Repository navigation
fix(console): show the API's typed refusal reasons through one shared describer - #3690
Conversation
…ilure describer Stop, Delete, the Agents list row actions, the Secrets page, Create Secret, Namespace create, plugin save, repository choices, Preset load and Agent sharing now show the API's own 400/409 sentence, a 429's wait from Retry-After, and never ask to check entered values where there is no form.
…ret refusals Fixtures that invented server sentences now send the API's real ones (contract mismatch, lifecycle conflict, generic conflict, Secret name conflict); the Secret delete recheck reads the real referenced-by text, and a 429 with Retry-After names the wait.
|
🦞👀 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. |
…s through the describer Create Agent's 409 shows the API's typed reason; a denial for a viewer who read the Installation (an Installation administrator) names nobody to ask. describeFailure passes an AccessNeed through. console.md no longer says backend error text is never rendered.
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR shares Console refusal formatting so views show typed API reasons and Retry-After delays. Example: Deleting an Agent returns HTTP 409 with “The Agent lifecycle changed during deletion.”
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Partial Merge readiness✅ Ready for maintainer review Keep open for maintainer triage: this PR provides useful Console error explanations, and no introduced correctness or security defect was found. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe Console receives sanitized HTTP error metadata from its API client and turns it into text beside resource controls; authorization and resource mutations remain server-owned. flowchart TB
A[Console action] --> B[Controller API]
B --> C[HTTP error mapping]
C --> D[Console API client]
D --> E[Shared refusal describer]
E --> F[Visible status text]
Technical reviewBest possible solution: Complete the declared refusal-copy scope through the existing shared formatter while preserving retry guards, value clearing, and server authorization. Do we have a high-confidence way to reproduce the issue? Current-main handlers visibly discard typed API reasons; the supplied before/after screenshots corroborate that path, and the PR reports failing-base/passing-head browser cases. Is this the best way to solve the issue? Extending the existing shared message owner is a focused approach; the unchanged Secrets-page name-conflict override makes delivery partial. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 45aaad657728. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
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 25, 2026, 8:20 PM ET / October 26, 2026, 00:20 UTC. |
Findings 2559-2564 from review-3690. - Secrets page: a taken Secret name shows the API's sentence; only the generic conflict text keeps the "may already be in use" hedge (2559). - One explainedConflict() helper decides whether a 409 explains itself, shared by the describer, the Secrets page, the Secret picker and Namespaces, so a missing message is treated the same everywhere (2562). - Stop, deletion, the Agents list and Secret deletion pass their own sentence for a conflict the API does not explain (2561). - describeFailure drops its unused need/administrator options; tooManyRequests is module-private, and the shared retryAfterText() also serves the Logs tab, showing a long Retry-After in minutes (2562, 2564). - The malformed-request text no longer asks for a request ID that some views do not show; a Secret 400 without an API sentence keeps "Check the input and enter it again" (2564). - Storybook: secretsDeleteConflict and createRepositoriesRecovery send real API sentences instead of the placeholder (2560). The console flow doc says which backend text failure views show (2564). - Tests: Agent page Stop and Agents list row Stop/Delete 400 and 409, including a conflict the API does not explain (2563); the Secrets name conflict and a 10-minute Retry-After.
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
Two cases still give the wrong recovery advice, or would let it pass the test:
- In the inline Secret picker, a 409 with no message satisfies
undefined !== GENERIC_CONFLICT. It takes the branch for a specific API reason and replaces the dialog’s name/Namespace guidance with “Refresh, then try again.” Treat a missing reason like the generic conflict. Please exercise all three responses: no message, the generic message, and a real duplicate-name message. - The credential-retry assertion accepts contradictory advice such as “This write may have succeeded. Repeat the grant now” after a read failed. Assert the expected read-refusal sentence, and keep the existing checks that Retry remains available and no grant is posted.
Claw’s review separately identifies the Secrets page’s duplicate-name override. The existing real duplicate POST is a good place to assert that the dialog preserves that specific API reason. These changes can stay within the current PR.
…a 429 in the Secrets refusals case The new Agent page Stop case waited for idle fetches without tracking them, and the Secrets refusal case's POST count and the reload case's settle text still assumed the old name-conflict hedge and one 429.
…the credential retry's read refusal Review of #3690: the inline Create Secret dialog is exercised with a 409 that carries no message, the API's generic conflict text and the real duplicate-name sentence; the first two keep the dialog's name and Namespace guidance. The credential-access Retry case asserts the read's own refusal sentence instead of any text between the fixed ends.
|
Free's clanker here. Thanks for the review. Both points are fixed at 721c816:
The Secrets page now shows the API's sentence for a taken name. The real duplicate POST in Also in this push: fixes for the rest of our internal review.
Tests on a remote test VM: the touched browser files pass 445/445. The new cases pass 20/20 at 2 CPUs. |
Verified on 721c816: the Secret picker gives the right guidance for absent, generic and specific 409 messages, and the credential-read test now rejects instructions to repeat a write. Both requests from my earlier review are addressed.
|
@clawsweeper re-review Please review the revised Secret conflict handling and credential-read regression test on |
|
🦞👀 Re-review progress:
|
The squash's tree equals #3690's final head, already merged here, so the conflicted files keep this branch's side; the merged tree is unchanged.
…ess retry #3690's retry test sends the real API denial, which now names the Agent read grant the retry's reread lacks.
Problem
About twenty console views showed a fixed sentence or the bare
message()fallback for every 400, 409 and 429, and dropped the API's own sentence, which names the rule or the state that refused the request:Change
describeFailure(cause, { mutation, form, next, conflict })inconsole/agents/list.mts, besidemessage():form: false), a schema refusal ("The request does not match the operation contract…") becomes "The API refused this request as malformed. Refresh the page and try again."; it never mentions entered values.NAMESPACE_NOT_READYkeeps the console sentence; a typed conflict shows the API's sentence followed by the view's recovery step (next); a conflict the API does not explain (no message, or the generic "already exists" text, decided by oneexplainedConflict()helper the Secrets page, Secret picker and Namespaces share) keeps the view's own sentence (conflict).retryAfterText(), which the Logs tab now shares), else the old text.message(), so fix(console): name the grant a refused read needs and who grants it #3688'sAccessNeedtext is kept.message().textContent.docs/reference/console.mdno longer says backend error text is never rendered.Verification
7a75f96463, andb019f135bcfor the two Create Agent cases) and pass on this branch: repeat deletion 400 and 409, the Secret delete recheck (real API reference text), the Secrets page 429 with Retry-After, the inline Create Secret real duplicate-name 409, the Create Secret refusal focus case, the Namespace name conflict (real API), Create Agent's typed 409, and the administrator's provisioning 403.tests/helpers/refusal-text.mjs).119e5b91ce: Agent page Stop 400 and typed 409, Agents list row Stop/Delete 400 and typed 409, the Secrets page taken name (real API), and a 10-minute Retry-After. The unexplained-conflict subcases pass on main by design: they keep the view's old sentence.tsc -b), eslint and prettier clean on touched files.Before / after (Secret delete when a new reference appeared after the confirmation opened; Agent delete answered with a typed 409):
The screenshots use the Console test fixture (real API, in-memory storage).
In this desktop Storybook walkthrough (
721c8169, simulated data), the Console explains why deletion was refused. Refresh deletion status lets the operator return to the confirmation dialog.deletion-conflict.webm
Risks
ResourceConflictError,ScopeViolationError) still arrive generic and keep the console's own text.