Skip to content

fix(console): announce danger feedback lines as alerts and pin the form refusals - #2931

Merged
freeqaz merged 8 commits into
mainfrom
test-gaps-127-refusals
Oct 19, 2026
Merged

freeqaz merged 8 commits into
mainfrom
test-gaps-127-refusals

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

bughunt-127 found the refusal blocks #2912 and #2923 introduced only partly pinned, and some still announced as statuses.

  • Finding 2004 (medium): the tests pinned only the preflight-conflict refusal of the authentication source form and the Repositories editor. Their 403 refusals (the PR's own media) could go back to the grey, unfocused hint, the original finding 1966 bug, with every test green. The unconfirmed Secret grant warning, the readBinding refusal, the rebuilt-tab tones and the progress line's reset were unpinned too.
  • Finding 2005: danger blocks still carried role=status in several places, against the design language (danger and warning blocks are role=alert): the authentication source and Repositories editor feedback lines, the repository picker's status line when it turns danger, the Secret picker's "Secrets unavailable", the channel Secrets reload/unknown-outcome lines, and the plugin feedback and tool-error lines.
  • Finding 2006: the tone and announcement role of the other sites fix(console): finish moving errors and notices to the status block #2912 migrated were unpinned.
  • Finding 2010: on the Configuration editor's pre-save conflict path the line became an alert only after its text was written, and the progress, Cancel and navigation resets were unpinned.

Change

Product (small):

  • setStatusTone (dom.mjs) now keeps an announcing line's role in step with its tone: a line with role=status or role=alert becomes an alert in the danger and warning tones and a status otherwise, the rule statusBlock already follows. Lines without a role are untouched.
  • Its callers in detail.mjs (authentication source say, Repositories editor refuse and resets, the retained warning/unknown lines, and the Configuration editor's refusals and resets) set the tone before writing the text, so a refusal is written into an alert and guidance into a status.
  • The fixed danger blocks for the channel Secrets reload and unknown-outcome lines (credentials.mjs) and the plugin selections feedback line (plugin-fields.mjs) are role=alert.
  • The plugin tool-error line stays role=status, with a comment: the details pane is rebuilt on every redraw (search typing, for one), so an alert would be announced again each time. The per-repository danger notes keep no role for the same reason, and the picker's status line does the announcing.
  • The repository status line drops its redundant aria-live="polite" (implied by role=status), so its alert role is assertive when it turns danger.

Tests:

  • console-status-block.test.mjs: the two existing conflict tests now expect an alert; new tests pin the authentication source 403 (focused danger alert, written as an alert, next save's progress a quiet status), the unfinished ChatGPT sign-in refusal, the unconfirmed Secret grant (focused warning alert, still a warning alert after the tab is rebuilt), the unknown outcome (still a danger alert after the rebuild), the Repositories editor 403 (focused alert; an edit and a held navigation turn it back into a status), the Configuration pre-save conflict (written into an alert; the navigation hold is a status) and a refused Configuration save's progress and Cancel resets.
  • console-agents.test.mjs: the repository-recovery test now tells the create form's refusal from the repository picker's own line, which is an alert too.
  • console-agent-draft-guards.test.mjs: the channel Secrets reload line is found as an alert.
  • Tone and role pins for the other migrated sites (finding 2006), added where existing tests already reach them: the repository choices refusal and load failure (alerts), the loading hint on Retry (a quiet status while the retry is held), the unavailable and unapproved repository lines (danger, no role), "Secrets unavailable" (alert), the Stop and deletion notices and the channel drawer notice (info statuses), the workspace explainer (info, no role), the Slack directory error (alert), and the plugin selections feedback (danger alert). One new channels test reaches the drawer's "Channels require Dedicated execution." refusal (danger alert, nothing written) with a real Embedded OpenClaw Agent whose Configuration was given an enabled Slack channel through the Configuration API (which accepts it; Agent save and deploy are what refuse it). No API response is rewritten.

Verification

Risks

  • setStatusTone is shared. It changes the role only on lines that already announce (role=status or role=alert), and only to follow the tone, so a line that is quiet progress stays a status. The other callers (create form, Share Agent) were already alerts in the danger tone.
  • A screen reader now hears these refusals assertively, and the unconfirmed grant warning too, as the design language asks.
  • The channel Secrets "Credential changes may have been saved…" line is now an alert as well, but no test reaches that state (it needs an unknown outcome followed by a definitive refusal); it is a static role change.

Closes findings 2004, 2005, 2006 and 2010 from bughunt-127 (UX plan follow-up).

A feedback line that switches between a quiet hint and a refusal kept
role=status in its danger and warning tones, against the status block
rule (danger and warning are alerts). setStatusTone now keeps an
announcing line's role in step with its tone, and the callers set the
tone before writing the text, so a refusal is written into an alert.
The fixed danger blocks for channel Secrets reload/unknown outcome and
the plugin feedback and tool error lines are alerts too. The repository
status line drops its redundant aria-live=polite so its alert role is
assertive.

Finding 2005, 2010.
Each migrated feedback line and notice now has its class, tone and role
asserted where an existing test reaches it: the repository choices
refusals and load failure, the loading hint on Retry, the unavailable
and unapproved repository lines, the Secrets unavailable line, the stop
and deletion notices, the channel drawer notice, the workspace
explainer, the Slack directory error and the plugin selections
feedback. A new channels test reaches the drawer's Dedicated execution
refusal for an Agent read as Embedded.

Finding 2006.
… blocks

The 403 refusals of the authentication source form and the Repositories
editor could go back to the grey, unfocused hint with every test green;
only the preflight conflict was pinned. New tests pin, for each form:
the API refusal as a focused danger alert, the unconfirmed Secret grant
as a warning alert (also after the tab is rebuilt), the unknown outcome
kept as a danger alert, the unfinished ChatGPT sign-in refusal, the next
save's progress as a quiet status, and the edit and navigation resets.
The Configuration editor's pre-save conflict is now checked to be
written into an alert, and its progress, Cancel and navigation resets
are pinned. The channel Secrets reload line and the plugin tool error
are checked as alerts.

Findings 2004, 2005, 2010.
@freeqaz
freeqaz requested a review from a team as a code owner October 19, 2026 11:25
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

…l error a status

Internal review nits: the Repositories editor and Configuration editor
resets now set the tone before writing their guidance, as the refusals
do. The plugin tool error stays role=status: the details pane is rebuilt
on every redraw, so an alert would be announced again on each keystroke.
The repository notes' no-role pins say why, and the plugin approvers
helper no longer splits a comment from its test.
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper clawsweeper Bot added clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR makes announcing danger and warning feedback use alert roles and adds browser coverage for refusals and status resets.

Example: Saving an Agent's authentication source receives HTTP 403.

  • Before: The danger feedback retains role=status and polite announcement semantics.
  • After: The feedback becomes role=alert before its refusal text is written, then returns to role=status during the next save.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The focused implementation and corrected fixtures support the repair, while supplied evidence does not satisfy the repository's explicit console verification requirement.
Proof confidence 🦪 silver shellfish (2/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Assistive technology receives some danger and warning feedback as polite status updates instead of the alerts specified by the existing component contract.
Reason: The maintainer-authored direction follows the established status-block behavior and protects consequential editor feedback.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it fixes a source-confirmed accessibility mismatch, and the earlier fixture finding is resolved with no remaining introduced code defect identified.

Priority: P2
Reviewed head: 12e9bbf2abd91b2742b9bef091aa2257c0d5a069

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The body reports base failures and mutation checks, but final-revision Storybook screenshots and a walkthrough showing refusal alert semantics followed by quiet progress are still absent.
Agent review details

How this fits together

The OCC console renders Agent editor feedback from validation and API outcomes; shared DOM helpers turn those outcomes into styled messages and accessibility announcements.

flowchart LR
  A[Agent editor action] --> B[Validation and API outcome]
  B --> C[Feedback caller]
  C --> D[Shared tone helper]
  D --> E[DOM live region]
  E --> F[Assistive technology]
Loading

Technical review

Best possible solution:

Keep announcement-role selection in the existing shared helper, with callers setting the tone before writing feedback.

Do we have a high-confidence way to reproduce the issue?

Pinned main leaves dynamically toned feedback at role=status, including authentication refusals; the relevant callers and existing alert contract establish the mismatch without executing target code.

Is this the best way to solve the issue?

The repair is appropriately centralized, preserves deliberately quiet nodes and plugin tool-error behavior, and avoids changing API or persistence contracts.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against f3a74292ab69.

Provenance checked

  • Shared status-block tone and role contract keeps the original intent (7474d8b: Consolidate error and notice presentation while defaulting danger and warning blocks to alerts and retaining deliberate caller overrides.)
  • Authentication and repository feedback changes intended behavior with a stated reason (9abddf7: Make refusals focused danger blocks and partial saves warnings while preserving existing status roles; this PR explicitly explains why announcing roles must now follow their tones.)
  • Configuration refusal and reset ordering keeps the original intent (b10157c: Expose refusals as alerts before writing their text and restore status semantics for progress, edits, cancellation, and navigation guidance.)
  • Plugin redraw announcement restraint keeps the original intent (6ec4e88: Avoid repeated live-region announcements during plugin redraws; the existing tool-error status role remains unchanged.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Review metrics

Metric Value Why it matters
Production and test changes Production +32/-18 lines; browser tests +572/-17 lines The small shared behavior repair has broader regression coverage for distinct refusals, retained warnings, and progress resets without adding production scaffolding.

Labels

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: Incorrect announcement urgency affects accessibility across Agent editors, with no demonstrated outage or blocked first-use workflow.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-19T11:54:26.143Z sha b36cede :: needs changes before merge. :: [P2] Create the channel-guard fixture through supported API operations

Reviewed October 19, 2026, 8:08 AM ET / 12:08 UTC (Revision 2).

Review: the guard test rewrote a Dedicated Codex Agent's read as
Embedded, a state the API never produces. It now creates an Embedded
OpenClaw Agent and enables Slack through the Configuration API, which
accepts it (Agent save and deploy are what refuse it), and asserts the
drawer's refusal with no response rewritten.
@freeqaz

freeqaz commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper clawsweeper Bot removed the clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. label Oct 19, 2026
@freeqaz
freeqaz merged commit acc8257 into main Oct 19, 2026
71 of 73 checks passed
@freeqaz
freeqaz deleted the test-gaps-127-refusals branch October 19, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant