Skip to content

fix(console): secret picker errors name the field and stay visible - #2922

Open
freeqaz wants to merge 8 commits into
mainfrom
ux-fixes-128-secret
Open

freeqaz wants to merge 8 commits into
mainfrom
ux-fixes-128-secret

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Three lows from the live walk on oce-dogfood-b and the form-field lanes (#2907, #2918), all in the Secret picker (secret-picker.mjs):

  • 1987: Create Agent's model credential picker is labelled "API key Secret" or "Service account token Secret", but its required error said "Model credential Secret is required.", a field the page does not show. The authentication source form said "API key Secret is required." under a field labelled "Service account token Secret".
  • 1988: the picker opens its list on focus. When the console's own check (validateFields) moves focus to a required picker, the list covers the new error under the field until Escape.
  • 1989 (D648): a Create Secret refusal other than a name conflict (seen live: 409 NAMESPACE_NOT_READY on a provisioning Namespace; also 400/403/404/429) disabled the submit while it ran, so focus dropped to the page body behind the open dialog.

Change

  • Error names the shown label. The required message is built from the label node the page shows, so a form that renames the field by method gets "API key Secret is required." or "Service account token Secret is required.". A shown error follows a renamed label when the method changes. No caller changes.
  • No list on the check's focus. A picker that shows its error (aria-invalid="true") does not open its list on focus. Click (new listener), typing and ArrowDown still open it, as before. A valid picker still opens on focus as today.
  • Refusal keeps focus in the dialog. After any refusal other than a name conflict (which still focuses Name), the refusal block takes focus with revealStatus(feedback, dialog), in view. An unknown outcome is handled the same way.
  • Storybook: new Pages / Create Agent / Model credential Secret required and Pages / Create Agent / Create Secret refused; form-field-workflow.md steps 6-8 updated (the list no longer needs Escape).

Presentation only: no API or authorization change.

Verification

Media (Storybook fixtures on the merged head)

These are the author's Storybook screenshots and walkthrough. We reproduced the three states at 1440 × 900 and 390 × 844 in Chromium, with the picker from dd92bd62a6dd for the before comparison and the PR at 3110ba7c75f5 for the after comparison. We checked the field label and error, whether the list obscured the error, and where focus landed after the simulated refusal. The captures show Console behavior with fixture data. They do not exercise live Secret creation or storage.

Model credential Secret required (Create Agent, Service Accounts): before, the open list covers the field's error; after, the error names the shown field ("Service account token Secret is required.") and the list stays closed

1440 390
before createModelSecretRequired before 1440 createModelSecretRequired before 390
after createModelSecretRequired after 1440 createModelSecretRequired after 390

Authentication source required fields: before, the list opens over the error; after, it stays closed

1440 390
before agentAuthenticationRequiredFields before 1440 agentAuthenticationRequiredFields before 390
after agentAuthenticationRequiredFields after 1440 agentAuthenticationRequiredFields after 390

Create Secret refused (409 NAMESPACE_NOT_READY): before, focus is on Name, not the refusal; after, the refusal has focus inside the dialog

1440 390
before createSecretRefused before 1440 createSecretRefused before 390
after createSecretRefused after 1440 createSecretRefused after 390

Stories: Pages / Create Agent / Model credential Secret required, Pages / Create Agent / Create Secret refused, Pages / Agent detail / Authentication source required fields.

Recording: walkthrough (1280x800, webm): Create Agent's Service account token Secret error with the list closed, the label and error following the method ("API key Secret is required."), click and ArrowDown opening the list, then the Create Secret refusal taking focus in the dialog.

Risks

Low. Keyboard readers who Tab into a picker that shows its error now press ArrowDown or type to open the list; one without an error still opens on focus. Findings 1987, 1988, 1989; UX plan C3 follow-up. Finding 1993 (Share Agent explainer) is left for #906, which rewrites access.mjs.

The required Secret picker's error now names the label the page shows
(API key Secret or Service account token Secret) on Create Agent and the
authentication source form, and a shown error follows a renamed label
(finding 1987). A picker showing its error no longer opens its list on
focus, so the console's own check does not cover the error; click,
typing and ArrowDown still open it (finding 1988). A Create Secret
refusal other than a name conflict takes focus inside the dialog, on the
refusal, instead of dropping it to the page (finding 1989).
@freeqaz
freeqaz requested a review from a team as a code owner October 19, 2026 09:16
@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.

@freeqaz
freeqaz deployed to integration-qa-pr October 19, 2026 09:22 — with GitHub Actions Active
@freeqaz
freeqaz deployed to integration-qa-pr October 19, 2026 09:22 — with GitHub Actions Active
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment 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 changes before merge.

What this changes

Secret-picker validation names the visible field, keeps its error unobscured, and focuses creation refusals inside the dialog.

Example: Create Research assistant using Service Accounts without selecting a Secret.

  • Before: The error names Model credential Secret, and the automatically opened list covers it.
  • After: Service account token Secret is required appears beneath the focused field with the list closed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with meaningful browser coverage and directly inspected UI screenshots; the attachment handoff remains a merge requirement.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (screenshot): Inspected Storybook screenshots exercise the actual picker UI and show corrected labels, unobscured errors at 1440 and 390 pixels, and focused refusal feedback inside the dialog; they establish simulated UI behavior only. Recording inspection was blocked by reviewer infrastructure, which does not invalidate the visible evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Credential errors can name a different field, disappear beneath its options, or leave keyboard focus behind an open refusal dialog.
Reason: The collaborator-authored direction addresses observed accessibility and recovery defects without adding configuration or changing authorization.

Merge readiness

⛔ Needs changes before merge - 1 item remains

This PR fixes concrete Secret-picker usability defects with a focused shared-component change; no introduced correctness or security defect was found.

Priority: P2
Reviewed head: 3110ba7c75f5e54f83f4f65ffa848715287a34b6

Before merge

  • Complete next step - Upload the existing screenshots and walkthrough as native GitHub attachments in Verification, and confirm they open and show the final UI, as AGENTS.md requires.

Findings

None.

Agent review details

How this fits together

The console’s shared Secret picker serves Agent creation and credential editing, translating selections, validation results, and Secret API responses into bindings and accessible feedback.

flowchart TD
  A[Agent creation form] --> C[Shared Secret picker]
  B[Credentials form] --> C
  C --> D[Field validation helpers]
  C --> E[Namespace Secret API]
  E --> F[Creation dialog feedback]
  D --> G[Visible accessible errors]
  F --> G
Loading

Technical review

Best possible solution:

Land the focused shared-picker repair after the existing native-media handoff is completed.

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

Current-main source exposes the reported paths, and before screenshots corroborate the obscured validation error; no target code was executed during this review.

Is this the best way to solve the issue?

The patch repairs the existing shared owner using established validation and focus helpers while preserving selection, keyboard access, and name-conflict recovery.

AGENTS.md: found and applied where relevant.

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

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Labels

Label changes:

No label changes.

Label justifications:

  • P2: Misnamed or obscured field errors and misplaced dialog focus create recoverable friction in Agent credential setup.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.
  • proof: sufficient: Contributor real behavior proof is sufficient.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.

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 (3 earlier review cycles)
  • reviewed 2026-10-19T09:34:34.735Z sha f87ac2b :: needs maintainer review before merge. :: none
  • reviewed 2026-10-19T09:42:40.975Z sha 6375aa9 :: needs changes before merge. :: none
  • reviewed 2026-10-19T10:00:54.950Z sha d70cc91 :: needs changes before merge. :: none

Reviewed October 19, 2026, 5:36 PM ET / 21:36 UTC (Revision 4).

@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-environment Preliminary submission classification; full source review pending. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 19, 2026
@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 19, 2026
@freeqaz

freeqaz commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks. The only before-merge item is the native-attachment upload. This host can't upload native GitHub attachments, so the media follows the maintainers' committed-media flow: the screenshots (1a58165ef) and the walkthrough recording (7a1bdaf3c) are committed under .pr-media/2922/, linked by SHA in Verification, and removed again before the squash (the head has no .pr-media/). I'm leaving the native upload to a maintainer and not landing this while the verdict is "needs changes".

@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:

@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-environment Preliminary submission classification; full source review pending. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 19, 2026
@freeqaz-openai

Copy link
Copy Markdown
Collaborator

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

This branch was successfully deployed

1 active (outdated) deployment
integration-qa-pr — f87ac2b3 Deployed Oct 19, 2026 by freeqaz via QA advisory (kubernetes) #2541
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. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants