Skip to content

fix(console): say why a read failed, name settings by path, one request id and not-ready sentence - #3733

Merged
freeqaz merged 9 commits into
mainfrom
errcopy-console-1026
Oct 26, 2026
Merged

freeqaz merged 9 commits into
mainfrom
errcopy-console-1026

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Console error text still left readers guessing in several places (error-copy audit, finding 2538):

  • Some catches said "unavailable, refresh to retry" for a denial and an outage alike (runtime images, Driver
    configurations, plugin policy controls, the deployment overview, the Secret deletion check, workspace files). The
    plugin tab also said "This installation does not support plugin policy editing" when only the read had failed.
  • Some refusals used the bare fallback: a refused Stop or Delete status read said "Access denied" without the grant;
    removing a failed credential registration added a Credential Gateway hint to every refusal; the credential-access
    retry and a credential registration problem after Create Agent said "Check the entered values" where nothing was
    entered.
  • Validation lists showed JSON pointers (/additions/0: ...) or bare codes; unknown runtime causes, log gap reasons
    and plugin warnings showed only their code.
  • The request ID had four renderings (one labelled "Request:"). Three different Namespace-not-ready sentences used
    "tenant RoleBindings". "OCC" appeared in user copy. docs/reference/console.md said other backend error text is
    not rendered, which is false.

Change

  • agents/list.mts: describeFailure takes an AccessNeed; new readFailureReason/readFailure choose the text by
    status. A 403 names the grant and the Installation administrator, a 429 how long to wait, an outage says to ask an
    operator if it continues, and a 400/409 shows the API's reason.
  • Status-aware catches: runtime images (Agent read / Namespace read), Agent and Installation Driver configurations
    (Agent read / Installation administer), plugin capabilities (Agent update; the field no longer claims missing
    support after a failed read), the deployment overview reread, the Secret deletion check (Secret read), and workspace
    files (Agent read; 409/429 through the describer).
  • Generic 403 and "entered values" fallbacks: Stop and Delete status reads name Agent read. Credential registration
    check names Namespace read; its removal names credential source delete and gives the Gateway hint only when the
    request got no answer. The credential-access retry and the registration problem show the API's own reason.
  • No pointers or codes: field errors name the setting as Configuration setting additions[0], and a detail without
    a message gets a sentence. Unknown runtime causes, log gaps and plugin warnings get a sentence.
  • Consistency: requestIdText/requestIdLine/withRequestId in dom.mts replace the per-view renderings (one
    label, "Request ID:"). One NAMESPACE_NOT_READY_MESSAGE, which names who finishes the setup. No "OCC" in copy
    (Stop notices, harness-auth hints, live-serving note, cluster RBAC refusal, debug build title).
  • docs/reference/console.md: says which API text the console shows and what a generic answer gets.
  • Tests read the new sentences from tests/helpers/refusal-text.mjs.

Verification

Audit sites checked on main: item 3, 19 sites (15 already fixed by #3688/#3690, 2 changed here, 2 left on purpose); item 7, 6 sites (3 changed here); item 8, 9 sites (all changed); item 9, 5 sites (all changed); item 10, 4 topics (3 changed, plus the doc).

  • New assertions, one or more per item, each failing on main (37f66025e8) with the tests copied over: 21 tests in 10 browser files fail on main and pass on this branch (a remote test VM, private network namespace).
  • Mutants (each reverts one change), each caught: 7 of 7 (Stop status read need, outage reason, workspace 429 branch, dotted setting path, request ID label, shared not-ready sentence in the Secret picker, credential source delete refusal).
  • Touched browser files, 20/20 at 2 CPUs on a remote test VM (private network namespace): 11 of 11 (console-agent-detail, -detail-2, -deploy-again, -overview, -draft-guards, -presets, console-agents-2, console-create-agent-provisioning-400, console-form-fields, console-runtime-credentials, console-secrets).
  • tsc --build and eslint apps/controller/src/console: clean. checks-browser, checks-browser-2 on the same VM: pass (head 6807635, after merging main).

Risks

  • Copy only, plus one new optional setCapabilities(value, unread) argument; no request, retry or state change.
  • Visible sentences change in about 20 places; tests that matched old wording were updated.
  • credentials.mts keeps its own request ID text (same label) to avoid a conflict with fix(console): read the default slack account's token keys and show channel refusals on credentials #3686.
  • Two audit rows stay as they are on purpose: the generic text after a partial Agent share (pinned by an existing test)
    and the Agent reread before a credential-access retry, where the API's named denial already names Agent read.
  • UI media: the changes are failure sentences only; the browser tests above assert each one.

@freeqaz
freeqaz requested a review from a team as a code owner October 26, 2026 06:48
@clawsweeper

clawsweeper Bot commented Oct 26, 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.

@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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 26, 2026
@clawsweeper clawsweeper Bot added 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 26, 2026
@clawsweeper

clawsweeper Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

What this changes

This PR makes Console failures status-aware, standardizes request IDs and Namespace readiness guidance, and displays configuration errors as setting paths.

Example: Refresh stop status receives HTTP 403.

  • Before: The Console reports a generic access denial.
  • After: The Console names Agent read and directs the reader to an Installation administrator.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) Useful shared-owner repair with passing head checks, one concrete diagnostic defect, and unmet repository visual-verification requirements.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Readers receive generic or misleading explanations for denied, rate-limited, or failed Console operations.
Reason: The collaborator-authored direction improves existing recovery paths through their established owners without adding configuration or platform capabilities.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep this PR open: it improves useful Console recovery guidance, but introduces one misleading workspace failure diagnosis.

Priority: P2
Reviewed head: 6807635120cdd77fc2c945d0dd058c40f3120706

Before merge

  • Describe interrupted workspace reads without asserting non-delivery (P2) - A workspace GET can reach the API, receive headers, and then lose its response body or exceed the deadline. api-client.mts propagates those cases as a statusless TypeError or TimeoutError, as the existing real-socket conformance cases demonstrate. This branch now tells the reader that the request never reached the API and directs them to check their connection, although neither conclusion follows. Use neutral interrupted-read guidance and cover an after-headers failure alongside the existing pre-response abort case.
  • Explain or restore the original intent of Workspace read failure classification - fix(console): preserve interrupted response body errors #1891: Preserve transport and deadline errors after response headers so callers report interrupted reads; such errors do not establish that the request failed to reach the API.

Findings

  • [P2] Describe interrupted workspace reads without asserting non-delivery — apps/controller/src/console/agents/workspace.mts:72-73

Provenance

  • Workspace read failure classification changes intended behavior without addressing why it exists (fix(console): preserve interrupted response body errors #1891: Preserve transport and deadline errors after response headers so callers report interrupted reads; such errors do not establish that the request failed to reach the API.)

Tests

  • Missing end-to-end proof: Current-source visual evidence is missing for the changed failure and recovery states, and the workspace browser extension does not cover a disconnect or timeout after response headers.
Agent review details

How this fits together

Console views receive failures from the authenticated API client and turn their status, error details, and request ID into visible explanations and recovery actions.

flowchart LR
  U[User] --> V[Console views]
  V --> C[API client]
  C --> A[Controller API]
  A --> C
  C --> F[Failure descriptions]
  F --> V
Loading

Technical review

Best possible solution:

Keep the shared status-aware descriptions and use neutral interrupted-read guidance when a workspace transport failure cannot establish whether the API received the request.

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

Source establishes the generic failure-copy paths and the introduced interruption misclassification; existing socket tests demonstrate the relevant error shapes, but no tests were executed during this read-only review.

Is this the best way to solve the issue?

Extending the existing failure describer is appropriate, provided transport errors describe the observed interruption without inferring where the request stopped.

Full review comments:

  • [P2] Describe interrupted workspace reads without asserting non-delivery — apps/controller/src/console/agents/workspace.mts:72-73
    A workspace GET can reach the API, receive headers, and then lose its response body or exceed the deadline. api-client.mts propagates those cases as a statusless TypeError or TimeoutError, as the existing real-socket conformance cases demonstrate. This branch now tells the reader that the request never reached the API and directs them to check their connection, although neither conclusion follows. Use neutral interrupted-read guidance and cover an after-headers failure alongside the existing pre-response abort case.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

  • node --test tests/conformance/console-api-client.test.mjs
  • node --test --test-name-pattern='explicit missing-file readback' tests/browser/console-agent-detail-2.test.mjs
  • pnpm lint
  • pnpm format:check
  • pnpm check:workspace

What I checked:

Likely related people:

  • Free Wortley: Raw commit d28c024 adds apps/controller/src/console/agents/list.mts:414 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: d28c0241e506; files: apps/controller/src/console/agents/list.mts)

Labels

Label changes:

  • add P2: The PR improves recovery guidance across existing Console workflows; its introduced defect misdiagnoses interrupted workspace reads without changing authorization or persisted state.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P2: The PR improves recovery guidance across existing Console workflows; its introduced defect misdiagnoses interrupted workspace reads without changing authorization or persisted state.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

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.

Reviewed October 26, 2026, 3:20 AM ET / 07:20 UTC.

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: 🧂 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant