Skip to content

fix(console): disabled reasons name the grant and who gives it - #3695

Merged
freeqaz merged 29 commits into
mainfrom
fix/console-access-reasons
Oct 26, 2026
Merged

freeqaz merged 29 commits into
mainfrom
fix/console-access-reasons

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Follows #3690 (landed), which added the shared failure describer.

Problem

Disabled controls and refused requests often did not say which grant was missing, or who could give it (findings 2539, 2545, 2548, 2549):

  • Agent detail sent readers to an "Agent administrator" for read access to versions. No such role grants that. Each version is its own AgentRevision, with its own read grant, and an Installation administrator binds it after each deployment.
  • The Agent page's 403 said "Ask its owner to share it with you". Owners cannot grant access; Namespace IAM writes need Installation administration.
  • On Agents list rows the note was "No access to stop or delete" / "No access to delete", with no grant and no who. Stop also stayed after a failed first deployment, though nothing was running and the detail page already hides it (catalog rows 49 and 58).
  • Secrets:
    • Create, read, replace and delete denials named no grant.
    • An empty list said "No Secrets yet" to a member who simply cannot see the Namespace's Secrets.
    • A bound Secret the reader cannot read showed Bound Secret · sec_… · Metadata unavailable (access denied).
  • The network policy said "You do not have permission to …" with no grant. Next to it was an internal "No Driver configurations are available to this account for this Agent."
  • The Namespaces empty state, workspace saves, Run diagnostics and the Agent-side save refusals said "ask an administrator", without saying which one.

Change

  • New agents/access-reasons.mts is the one module that words who grants access. It owns the ask clause (askForGrant "Ask an Installation administrator for .", askAdministratorTo for a step only that role can take, and the clause for use inside a sentence), notIncluded (what the reader's access does not include, then the grant), and the create reasons. list.mts's accessDenied, AGENT_CREATE_DENIED and save refusals, and the inline copies in workspace, diagnostics, detail, create, secret-picker, presets, namespaces, secrets and console.mjs, now build their text from it. The ask ends as the API's named denials from fix(iam): name the missing grant in a caller's own denial #3699 do. Each grant is the action the API checks for that request. I read each handler or route to confirm it:
    • Agent operate (stop, workspace saves, diagnostics)
    • Agent delete
    • AgentRevision read, per version
    • Configuration update
    • Secret read, update, delete and operate
    • SandboxPolicy read (the policy and its suggestions)
    • Agent manage_policy plus SandboxPolicy update (validate, save, apply)
    • Agent read_logs (Sandbox log text)
  • Where no Namespace Role can grant the action, the reason says who can do it instead:
    • Creating Agents, Secrets and Namespaces: only an Installation administrator can do it (one shared shape).
    • The policy editing setting needs Installation administer and Agent administer.
  • Agent detail: the hidden-version panel, the new-version hint and the empty history all use AgentRevision read from an Installation administrator. Configuration read-only reasons name Configuration update, or Secret operate on each bound Secret.
  • Agent page 403: "Ask an Installation administrator for Agent read on it."
  • Agents list:
    • Confirmations use the shared reasons.
    • The row's one reserved note line names the missing grant ("Needs Agent delete", "Needs Agent operate", "Needs operate and delete"). The whole reason, with who gives it, is the line's tooltip and what the icons' description reads. "Needs Agent operate and delete" overflows the reserved line at 1280 px (169 px in 147 px), so the combined line keeps the short form; a test asserts that the line fits.
    • Stop is hidden while the row's latest deployment read shows a failed first deployment with no current version. Each read decides again, so a list kept on Back rereads and shows Stop for a retried first deployment that is queued.
  • Secrets:
    • Each denial names its grant.
    • The empty heading reads "No Secrets are shared with you" when create is denied.
    • A bound Secret the reader cannot read says "A Secret is bound. Its name is hidden because your access does not include reading it. Ask an Installation administrator for Secret read on it." The ID moves to the tooltip.
  • Network policy: each 403 names its grant, and the Driver configurations sentence is gone.
  • Device login: a 403 names what that request checks (packages/occ/src/index.ts, authorizeDeviceAuthorizationScope and the start, poll and cancel paths):
    • Start on Create Agent: Agent create, Secret create and credential source create, which only an Installation administrator holds.
    • Start on Agent detail: Agent read and update on the Agent, plus Secret create and credential source create, which only an Installation administrator holds.
    • Poll and cancel: the same Agent check, plus Secret operate on the login's Secret (and, for poll, operate on its credential source). On Agent detail these are grants a Namespace Role can give, so the text asks for them.
  • Create Agent: a refused Configuration create says that only an Installation administrator holds Configuration create, instead of "ask an administrator for Configuration create access".
  • Namespaces: a member's empty state reads "No Namespaces are shared with you. Ask an Installation administrator to give you access to one." A known administrator's reads "Create a Namespace to get started." When the administrator answer is unknown, it reads "Create a Namespace if your access allows it, or ask an Installation administrator to give you access to one." A refused create and the disabled control use one shared sentence.
  • Docs: docs/reference/console.md and docs/reference/console/versions-and-channel-drafts.md. The Storybook scenario step text is updated too.
  • No API change.

Left alone, because other open work owns these lines:

Verification

  • tsc (console and tests), eslint and prettier are clean on the touched files.
  • Browser suites: the 18 touched or related browser files pass 585/585 on head 1c4224af (merged with main bcddbeae, which includes fix(iam): name the missing grant in a caller's own denial #3699), on a remote test VM in a private network namespace. tests/integration/caller-denial-api.test.ts passes on the merge. Head cf034d58 changes only tests/conformance/http-errors.test.mjs (a fix(iam): name the missing grant in a caller's own denial #3699 case imported the removed accessDenied); it passes 122/122, and the console conformance and integration files pass 89/89.
  • The new cases fail on the head before these fixes (5bc2b940): both device login cases (Create Agent start; Agent detail start, poll and cancel), the Agents list Back case for a retried first deployment, and the unknown-administrator Namespaces case.
  • Device login cases use the real Native IAM Driver: the administrator's Secret create or Secret operate grant is removed from its Role, and the API refuses the start, poll or cancel with 403.
  • Pinned 20/20 at 2 CPUs (netns), 6 cases each run: the two device login cases, the failed-first-deployment list case with its Back step, the unknown-administrator Namespaces case and the row reasons case (now also asserting that the row line fits).
  • Earlier runs on the previous head: 22 browser files 694/694; 12 cases pinned 20/20. The media below is unchanged: the row line still reads "Needs operate and delete".

Agents list row for a reader (before, after):

agents-row-c6-before
agents-row-c6-after

Risks

…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.
…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.
Findings 2539, 2545, 2548, 2549. One module, agents/access-reasons, holds
the shared reasons: what the reader's access does not include, then
"Ask an Installation administrator for <grant>", naming the action the
API checks (Agent operate to stop, Agent delete to delete, AgentRevision
read per version, Configuration update, Secret read/update/delete/operate,
SandboxPolicy read, Agent manage_policy and SandboxPolicy update).

- Agent detail: the hidden-version panel, new-version hint and empty
  history no longer send readers to an "Agent administrator".
- Agent page 403 says Agent read from an Installation administrator
  instead of "ask its owner to share it".
- Agents list: row notes say which grant is missing; Stop hides when a
  first deployment failed and nothing runs.
- Secrets: create, read, replace and delete denials name their grant;
  an empty list without create says no Secrets are shared with you; a
  bound Secret the reader cannot read says its name is hidden instead of
  "Metadata unavailable (access denied)".
- Network policy 403s name their grant; the editing setting says only an
  Installation administrator who administers the Agent can change it;
  the internal "No Driver configurations" sentence is gone.
- Namespaces empty state, workspace, diagnostics and save refusals use
  the same Installation administrator wording.
…e list's hidden Stop

The network policy reason lost its typographic apostrophe when it moved
to access-reasons, so the page no longer matched the existing copy. A new
case pins that the Agents list offers no Stop once a failed first
deployment left nothing running, and the policy denial case checks the
internal Driver configurations sentence is gone.
@freeqaz
freeqaz requested a review from a team as a code owner October 26, 2026 00:22
@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.

…s its tooltip

The row's reserved note line is one short line, so a full reason was cut
to "Your access does not incl…". The line now says which grant is
missing ("Needs Agent delete", "Needs Agent operate", "Needs operate
and delete"); the whole reason, with who gives it, is its tooltip and
what the icons' description reads.
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.
…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.
…console-6

# Conflicts:
#	apps/controller/src/console/secrets.mts
…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.
@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. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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 names missing grants and the Installation administrator who can provide them, and hides Stop after an observed failed first deployment.

Example: A reader lacks operate and delete access to Guarded Agent.

  • Before: The row says “No access to stop or delete”.
  • After: The row says “Needs operate and delete”, with the full grant explanation and Installation administrator guidance in its tooltip and accessible description.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) Useful access guidance has browser coverage and partial visual evidence, but the existing Stop finding remains unresolved.
Proof confidence 🦪 silver shellfish (2/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 vague access refusals or are directed to someone who cannot grant the required permission.
Reason: The maintainer-authored direction corrects actionable permission guidance without changing IAM contracts.

Merge readiness

⛔ Needs changes before merge - 1 item remains

The shared grant wording is useful, but the previously reported Stop regression remains reachable when a newer deployment is unreadable.

Priority: P2
Reviewed head: cf034d580c06c7b7e6840b5108a7b4a60eea6fea

Before merge

  • Preserve Stop when a newer deployment may be unreadable (P2) - The previously reported failure remains: give a caller Agent operate and read of failed v1, then queue v2 without granting that caller AgentRevision read on v2. latestReadableRevision still selects v1, and this branch records a failed first deployment because no revision is active yet. rowActions consequently removes Stop even though stopAgent can cancel the queued deployment using Agent operate alone. Clearing the set after a readable queued retry does not address this case. Remove suppression based on the filtered deployment read unless an authoritative observation establishes that no stoppable deployment exists.

Findings

  • [P2] Preserve Stop when a newer deployment may be unreadable — apps/controller/src/console/agents/collection.mts:385-388

Provenance

  • Earliest Secret-picker and device-login refusal wording: original intent not found (unknown: Scoped git log -S searches encountered unavailable historical blobs; the earliest wording rationale remains unknown.)

Tests

  • Missing end-to-end proof: The supplied evidence lacks the required UI walkthrough, and the added retry test does not demonstrate a base/head regression run with an unreadable newer queued revision.
Agent review details

How this fits together

The browser Console combines advisory permissions, filtered revision lists, and API responses into controls and recovery guidance; the controller authorizes each operation independently.

flowchart TD
User[Console user] --> Console[Browser Console]
Console --> API[Controller API]
API --> IAM[Selected IAM Driver]
API --> State[Agent and revision state]
API --> Responses[Permissions and filtered revisions]
Responses --> Console
Loading

Technical review

Best possible solution:

Keep the shared grant guidance and remove the Stop suppression based on filtered revision history.

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

Source establishes the failing path: a caller with Agent operate can read failed v1 but not queued v2, so the list hides Stop despite an operation the caller can cancel; no target code was executed.

Is this the best way to solve the issue?

Centralizing the access wording fits the existing Console, but filtered revision history cannot establish that no deployment remains stoppable.

Full review comments:

  • [P2] Preserve Stop when a newer deployment may be unreadable — apps/controller/src/console/agents/collection.mts:385-388
    The previously reported failure remains: give a caller Agent operate and read of failed v1, then queue v2 without granting that caller AgentRevision read on v2. latestReadableRevision still selects v1, and this branch records a failed first deployment because no revision is active yet. rowActions consequently removes Stop even though stopAgent can cancel the queued deployment using Agent operate alone. Clearing the set after a readable queued retry does not address this case. Remove suppression based on the filtered deployment read unless an authoritative observation establishes that no stoppable deployment exists.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • List actions and disabled reasons changes intended behavior with a stated reason (e8185da: feat(console): stop and delete Agents from the Agents list #2524 added direct list actions with advisory permission explanations and independent server authorization.)
  • Hidden-version guidance keeps the original intent (37c4ffb: Distinguishes hidden revision history from a never-deployed Agent; the new wording preserves that distinction and corrects the granting role.)
  • Named read refusals keeps the original intent (b019f13: fix(console): name the grant a refused read needs and who grants it #3688 names the grant checked by a refused read and directs the caller to Installation administration.)
  • Secret binding and save guidance keeps the original intent (02e0eb2: Existing controls prevent binding readable Secrets without operate permission; this PR retains those gates and makes the recovery instruction specific.)
  • Workspace and policy permission controls keeps the original intent (4945ae9: Workspace saves are unavailable without Agent operate, while policy controls retain their existing permission gates and independent log reads.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

Acceptance criteria:

  • node --test tests/browser/console-agent-overview.test.mjs tests/browser/console-agent-state.test.mjs
  • pnpm lint
  • pnpm format:check
  • pnpm check:workspace

What I checked:

  • Introduced Stop suppression: A failed deployment read with no active revision populates failedFirstDeployments, which removes the row's Stop control; the retry revision remains permission-filtered. (apps/controller/src/console/agents/collection.mts:385, cf034d580c06)
  • Revision visibility does not establish stoppability: listRevisions skips revisions without exact read permission, while stopAgent authorizes Agent operate and cancels provisioning without requiring revision read. (packages/occ/src/index.ts:4100, cf034d580c06)
  • Prior finding checked against the revised head: Compared with the earlier reviewed head, the patch clears suppression after a readable queued retry, but still infers failure from the latest readable revision; the earlier unreadable-newer-revision finding remains unfixed. (apps/controller/src/console/agents/collection.mts:385, cf034d580c06)
  • Current main preserves Stop: Current main suppresses Stop for stopped Agents without an active revision, without treating an older readable failure as proof that nothing remains stoppable. (apps/controller/src/console/agents/collection.mts:664, 6eb7ad4dd34d)
  • Visual evidence inspected: The supplied before/after images show the revised row label; the other prepared images belong to the earlier list-action implementation at feat(console): stop and delete Agents from the Agents list #2524. The manifest contains no recording of this PR's changed interaction. (cf034d580c06)
  • Policy and architecture applied: Read the complete root AGENTS.md; no nested AGENTS.md files or maintainer-notes directory were present in the touched trees. The implemented architecture retains controller authorization, and repository policy requires screenshots and a short UI walkthrough before merging. (AGENTS.md, cf034d580c06)

Likely related people:

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: Clearer permission guidance has normal-priority value, and the introduced Stop regression affects callers with incomplete revision visibility.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-26T00:50:12.313Z sha eec00bf :: needs changes before merge. :: [P2] Preserve Stop when a newer deployment may be unreadable

Reviewed October 25, 2026, 10:10 PM ET / October 26, 2026, 02:10 UTC (Revision 2).

…n administrator holds that grant

Namespace IAM Roles cannot grant create, so asking "an administrator for
Configuration create access" pointed at a grant nobody else can give.
Every device-authorization route authorizes Agent create in the
Namespace, which no Namespace Role grants.
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.
…ach request's grants

- access-reasons.mts owns the ask clause (askForGrant, askAdministratorTo, notIncluded) and
  the create reasons; list.mts, workspace, diagnostics, detail, create, secret-picker,
  presets, namespaces, secrets and console.mjs build their text from it (finding 2578).
- Device login 403s name what each request checks: Create Agent's start, Agent detail's
  start, poll and cancel (finding 2577).
- The Agents list decides a failed first deployment again on each deployment read, so a
  retained list keeps Stop for a retried first deployment (finding 2580).
- An unknown administrator with no Namespaces keeps the access hint (finding 2583).
- Nits: drop unused AGENT_DEPLOY_DENIED, 'Needs Agent operate and delete', Storybook step,
  Sandbox log grant comment (finding 2582).
- Tests take the new reasons from tests/helpers/refusal-text.mjs (finding 2579).
…d first deployment

The retried deployment's record is routed as queued, as the in-memory fixture runs no
deployment work. The unknown-administrator case denies the bootstrapped Namespace so the
empty state shows. The row line stays 'Needs operate and delete': with 'Agent' it overflows
the reserved line at 1280 pixels (169 vs 147), and the test now asserts that it fits.
@freeqaz

freeqaz commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 26, 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. labels Oct 26, 2026
…asons

# Conflicts:
#	apps/controller/src/console/agents/deletion.mts
#	apps/controller/src/console/agents/list.mts
#	docs/reference/console.md
#	tests/browser/console-agent-detail.test.mjs
#	tests/browser/console-agent-draft-guards.test.mjs
…t list.mts

#3699 added a case that imported accessDenied from list.mts; this branch folded it into
access-reasons.mts (notIncluded), so the file failed to load in Checks and Conformance 1.
The case now compares with namespaceListDenied, the written-out expectation.
@clawsweeper clawsweeper Bot removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 26, 2026
@freeqaz
freeqaz merged commit 0a17e75 into main Oct 26, 2026
47 of 48 checks passed
@freeqaz
freeqaz deleted the fix/console-access-reasons branch October 26, 2026 02:19
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: ⏳ 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