Repository navigation
fix(console): use the console's own required-field check on the remaining forms - #2918
Merged
Merged
Conversation
…ning forms Plugin selections, Slack channel credentials, the authentication source form and the workspace file editor still used reportValidity, so a refused field showed the browser's transient bubble. They now use validateFields with novalidate, as #2907 did for the other forms (finding 1979). The Create Agent invalid listener is removed: that form is novalidate and nothing calls checkValidity on it, so it never ran. A refused or unknown Plugins save now takes focus only when it is on the page or the disabled Save button, not when the reader moved it to another control in the section while the save ran (finding 1978).
…ms and the Plugins focus guard Each test fails on main: Plugin selections JSON, Slack channel credentials, the authentication source form and the workspace editor fired the browser's invalid event (its bubble) and showed no error under the field, and a Plugins save refusal or unknown outcome pulled focus off the Plugin selections JSON the reader had moved to. Mapped to checks-browser-2.
…e field error The error now sits under Plugin selections JSON, so the grey status line no longer says it a second time.
…es the text Reload sets the editor without an input event, so the error under the field and aria-invalid stayed on a file that was now valid. The read paths that reset the custom validity now clear the field error too (internal review).
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:44 — with 
GitHub Actions
Error
GitHub Actions
Error
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:44 — with
GitHub Actions
Error
GitHub Actions
Error
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:44 — with
GitHub Actions
Error
GitHub Actions
Error
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:47 — with
GitHub Actions
Error
GitHub Actions
Error
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:47 — with
GitHub Actions
Error
GitHub Actions
Error
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 07:47 — with
GitHub Actions
Failure
GitHub Actions
Failure
clawsweeper
Bot
added
clawsweeper:intake:bug
Contributor
|
Codex review: needs changes before merge. What this changesThis PR replaces transient browser validation bubbles with persistent field errors on four remaining console forms and preserves focus during failed Plugins saves. Example: Save AGENTS.md containing 16,385 ASCII characters.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Needs changes before merge - 3 items remain Keep open: this PR usefully completes the existing console validation pattern, but its Slack test and UI evidence handling need focused repairs. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe OCC console validates form input before submitting existing Agent and workspace API requests; these changes affect field feedback and focus without changing server authorization or persistence. flowchart TD
User[Console user] --> Forms[Agent forms]
Forms --> Validation[Shared field validation]
Validation --> Errors[Persistent errors and field focus]
Forms --> API[Existing OCC API]
API --> Outcome[Save outcome and focus guard]
Technical reviewBest possible solution: Retain the shared validation changes, repair the unsupported Slack test, add the affected Storybook scenarios, and publish compliant UI evidence outside the source tree. Do we have a high-confidence way to reproduce the issue? Current main still calls reportValidity in the affected owners and scopes Plugins outcome focus to the entire section; supplied before/after images corroborate the visible validation problem, but no target code was run during this review. Is this the best way to solve the issue? Reusing validateFields and the existing focus helpers is the smallest appropriate production fix; the fabricated Slack response does not establish a supported regression scenario. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 6e6b95815617. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 19, 2026, 4:00 AM ET / 08:00 UTC. |
…ies for the field errors The Slack channel test rewrote a Configuration into a cross-Namespace Secret binding, which the API refuses, so it proved no reachable state: Save stays disabled until both tokens are bound, and a bound picker passes. The switch to validateFields stays for consistency. Storybook gains Plugin selections JSON error, Authentication source required fields and Workspace file too large under Pages / Agent detail, with workflow steps in form-field-workflow.md (review feedback).
freeqaz
temporarily deployed
to
integration-qa-pr
October 19, 2026 08:16 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
freeqaz
temporarily deployed
to
integration-qa-pr
October 19, 2026 08:16 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
freeqaz
had a problem deploying
to
integration-qa-pr
October 19, 2026 08:16 — with
GitHub Actions
Failure
GitHub Actions
Failure
This was referenced Oct 19, 2026
This was referenced Oct 19, 2026
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
#2907 (UX plan C3) moved most console forms to the console's own required-field check, but four forms still called
reportValidity(finding 1979):On these forms, a refused field showed the browser's transient bubble. Nothing was left under the field,
aria-invalidwas not set, and the workspace editor's message came back as a bubble on every later Save.A second Plugins tab problem (finding 1978): a refused save, or one with an unknown outcome, moved focus to the refusal or to Reload even when the reader had moved focus to another control in the section while the save ran, such as Plugin selections JSON. Only the page and the disabled Save button should give up focus.
Change
validateFields(andnovalidatewhere there is a<form>), as feat(console): form field pattern with linked help and errors #2907 did. Each refused field shows its error under the field, linked witharia-describedbyandaria-invalid, and gets focus. A closed Plugin selections JSON section opens first. Errors clear when the field is fixed, and a workspace file's error also clears when Reload replaces the text.invalidlistener is gone. That form isnovalidateand nothing callscheckValidityon it, so the listener never ran. Removing it adds no hunk that fix(console): let the create preflight carry any configuration both creates accept #2833 or fix(console): grant Agent access to accepted Configuration Secrets #2255 conflicts with.revealStatus) and the unknown-outcome Reload (focusSaveOutcome) are now scoped to the Save button, not the section. They take focus only from the page or from the disabled Save button. Every message string is unchanged.scripts/console-storybook/form-field-workflow.md, steps 5-7.console.mjsstill usesreportValidity. Sign-in is out of scope for this change and stays as it is.UX plan C3 follow-up. Fixes findings 1979 and 1978.
Verification
tests/browser/console-form-fields-2.test.mjs(mapped tochecks-browser-2, suite audit has no issues). It has 8 tests:invalidevent (its bubble) and show no field error. In both "moved" focus tests, focus is pulled off Plugin selections JSON. The two "takes focus from the Save button" tests pass on main, as intended: they check that the guard is not too strict.taskset -c 14-15, one test at a time). That run still included the Slack case, which has since been removed.docs-compute-matrix, which needsgithub-slugger; that package is not installed locally. After the final review fixes, I re-ran the touched Plugins, save-refusal, detail and drafts files (84/84 passed) and every workspace and credentials file (221/221 passed).prettier --checkandeslint --max-warnings 0pass on the touched files.Open-PR conflicts. For each PR touching these files, I compared
git merge-treeagainst main withgit merge-treeagainst this head:scenarios.mjshunk is byte-identicalvalidateFieldslineMedia. These are the real console after a refused Save, at 1440 and 390 px. "Before" shows the browser bubble: it is gone on the next click or keypress, and nothing stays under the field. "After" shows the error under the field. The media commit is removed again before landing, as the maintainer workflow requires: committed media is linked by SHA, then deleted. The Storybook stories above show the same states. I rendered them locally at 1440 and 390 px from the prepared fixture assets, and each shows its error under the field.
Media commit
101672499.Picker listbox open over the error right after the check (finding 1988, as on Create Agent since #2907):
Risks
Low. A note on 1978: in browsers where clicking a button does not focus it (Safari and Firefox on macOS), focus stays on whatever the reader last used. If that was Plugin selections JSON, the refusal now leaves focus there instead of taking it. The refusal is still a
role=alertblock, so it is announced. The changes are client-side only and keep every string. One visible difference: when a required Secret picker gets focus from the check, its listbox opens over the new error, as it already does on Create Agent since #2907 (filed as finding 1988). In the authentication source form, the Service Accounts method shows the error "API key Secret is required." under a field labelled "Service account token Secret". That message already existed and is now persistent instead of a bubble. It has the same cause as finding 1987.