[freeqaz]

Problem

Every Agent PATCH (updateAgent in packages/occ/src/index.ts) checks the caller's grant on each source that the Agent and its Configuration bind now, on top of Agent update and Configuration read:

• Secret operate on every Secret that the Configuration binds;
• the Harness authentication source: Secret operate (API key, Service Accounts token), credential source operate (ChatGPT OAuth), or ServiceAccount read (ChatGPT service account);
• credential source operate on every credential source that the Agent lists.

The console gated the Plugins, authentication source and repository access editors only on Agent update. A member with Agent update and Configuration read but no operate on, for example, a bound Slack token or the model API key Secret got enabled editors, and every Save answered 403 after the click (finding 2441). #3611 added configurationSecretsOperate, but that bit is answered only to callers with Configuration update and does not cover the Agent's own sources.

Change

• API: GET agents/{id}/permissions adds agentSourcesOperate: whether the IAM Driver allows every grant listed above now. true when nothing is bound.
  • Answered only to a caller with Agent update and Configuration read whose Agent settings are readable; otherwise false.
  • No new disclosure: the Agent read already returns harnessAuth and credentialSources, and Configuration read already returns the Secret bindings. A real PATCH reports the same decision, with an audit row.
  • No audit row or side effect. One IAM call for each distinct grant, shared with configurationSecretsOperate (bindings are capped at 64, credential sources at their own limit).
  • Server authorization is unchanged.
• Console: when the answer is false, the Plugins, authentication source and repository access editors are disabled with a note that names the missing grant: "Saving these settings needs operate on each Secret and credential source this Agent and its Configuration bind, which your access does not include. Ask an administrator for that access." A missing Agent update still shows its own note first. The Plugins tab does not read the plugin policy capabilities in that state, because no save could keep a policy edit. A draft refresh (for example Reload authentication source) reads the answer again and redraws the editors, as for Agent update (finding 1538).
• Why a new field, not a wider configurationSecretsOperate: an Agent update checks more than the Configuration's bindings (the Harness source and credential sources), and it needs Agent update, not Configuration update.
• Contracts, OpenAPI and docs/reference/api.md are regenerated. The flow doc and both console guides are updated. The flow doc stays at 2,484 words because two 10-24 changelog entries moved to its history page.
• Storybook: a new scenario, "Model authentication, editor cannot operate a bound source". Existing permission summaries carry the new field.

Verification

• Integration, tests/integration/console-api.test.mjs: the new test "the Agent sources answer matches what a real Agent update checks". Two Configuration Secrets (one bound under two names) plus the Harness API key Secret. Each step checks the answer against the same no-op Agent PATCH (403 or 200):

  • operate on all sources but no Configuration read: false and 403;
  • with Configuration read: true and 200;
  • operate on only one of the two Configuration Secrets: false and 403;
  • every Configuration Secret but not the Harness Secret: false and 403.
  • The answers add no audit denial; the refused PATCH adds one.

• Integration, tests/integration/device-authorization-api.test.mjs: a ChatGPT OAuth Agent (Harness credential source, listed in credentialSources). Without credential source operate: false and 403. With it: true and 200.

• Browser, tests/browser/console-agent-save-refusals.test.ts:

  • The new test "an Agent editor without operate on the bound Secrets sees the Agent-side saves disabled": on all three tabs the note shows, Save is disabled, no field takes an edit, and nonAuthWriteRequests is empty.
  • The Agent update test now shares the same helper.
  • The reread test now walks from the Agent update note, to the sources note, to an enabled Save as each grant is added.

• Mutants, each run against the integration tests (all fail): every changed to some; the Agent's sources dropped; the Configuration bindings dropped; the Configuration read gate removed; the wrong Harness Secret id; credential sources checked as Secrets; the field removed (that is, main).

• Remote test VM:

  • The full checks-browser lane passed.
  • The full checks-browser-2 lane ran once before the last test fix; its only failure was the new test's repository step. That step is fixed: an Agent editor's repository tab offers no Save without a repository Driver, so the test now checks only the note and the disabled fields there. After the fix, console-agent-save-refusals.test.ts passed 15/15.
  • The four Agent-side save tests passed 20 of 20 runs at 2 CPUs in a private network namespace.
  • A console mutant that ignores the sources answer fails the new browser test and the reread test.

• Local: the two integration tests passed 20 of 20 runs at 2 CPUs.

• Media (simulated UI proof from the browser test fixture): a member with Agent update, Configuration read and Secret read, but no operate on the Agent's API key Secret. Before, Save authentication source was enabled and answered 403. After, it is disabled and the note names the grant. Before = main 403639fffb, after = this branch. No video.

  	Before	After
  Authentication source, 1440 px
  Authentication source, 390 px
  Plugins, 390 px

• Storybook: "Components/Credentials / Model authentication, editor cannot operate a bound source" (new). CI builds Storybook; I did not build it locally.

• Local: tsc --build (all projects and tests), eslint and prettier on the touched files, openapi:check.

Risks

• Advisory only. Lifecycle state and record checks (a deleted or foreign-driver Secret) can still refuse a save. So can sources that the save adds: a new authentication source or credential source is checked at Save.
• The answer is false when the reason is missing Configuration read, and the note then names source operate. In practice that caller cannot load the draft's Configuration.
• The ServiceAccount Harness branch (read) has no dedicated test. It mirrors authorizeBoundHarnessAuth and goes through the same helper as the Secret and credential source branches, which are tested.

[clawsweeper]

🦞👀
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]

Codex review: needs changes before merge.

What this changes

This PR disables Agent settings editors when the caller lacks grants required by the Agent’s currently bound sources.

Example: A member can update an Agent and read its Configuration but cannot operate its bound API key Secret.

• Before: Save authentication source is enabled, and saving returns 403.
• After: The authentication fields and Save are disabled with permission guidance.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A useful, focused permission fix with meaningful integration coverage and visible UI evidence, held by incorrect recovery guidance and incomplete repository-required visual verification.
Proof confidence	🦐 gold shrimp (3/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: Members receive editable Agent settings and enabled Save controls even when existing source permissions guarantee a refused save.
Reason: The maintainer-authored direction extends the existing permission-hint pattern to a verified gap without changing mutation authorization.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep this PR open: it addresses a current Console permission mismatch, but its new denial message gives incorrect recovery guidance for ServiceAccount-backed Agents.

Priority: P2
Reviewed head: c39b476545b78f08471b27e7330328dadf8411e4

Before merge

• Include ServiceAccount read in the disabled-save guidance (P2) - For an Agent whose Harness uses a ServiceAccount, a member can have Agent update, Configuration read, and every Secret/credential-source operate grant while lacking only ServiceAccount read. The new agentBoundSourceGrants check then returns agentSourcesOperate=false and disables all three editors, but this message tells the member to request operate on Secrets and credential sources they already have. Following that instruction cannot restore saving. Include ServiceAccount read in the shared explanation and affected guide text, and cover this persona so the guidance matches the actual refusal.

Findings

• [P2] Include ServiceAccount read in the disabled-save guidance — apps/controller/src/console/agents/list.mts:230-231

Tests

• Missing end-to-end proof: Provide the repository-required walkthrough of disabled editors and grant recovery, plus browser inspection of the rebuilt Storybook scenario.

Agent review details

How this fits together

The Console reads advisory permissions from the controller, which checks exact resources through IAM; actual Agent updates independently enforce authorization in OCC.

flowchart LR
  Console[Console editors] --> Permissions[Controller permission summary]
  Permissions --> IAM[IAM Driver]
  Permissions --> Console
  Console --> Save[Agent update]
  Save --> OCC[OCC authorization and persistence]
  OCC --> IAM

Loading

Technical review

Best possible solution:

Keep the advisory permission check and correct the shared denial text and affected guides to include ServiceAccount read.

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

Current main checks only Agent update before enabling these editors, while OCC also requires bound-source grants; source inspection establishes the mismatch without executing target code.

Is this the best way to solve the issue?

Extending the existing advisory summary is appropriate and preserves server enforcement, but its shared explanation must cover every grant that can make the new answer false.

Full review comments:

• [P2] Include ServiceAccount read in the disabled-save guidance — apps/controller/src/console/agents/list.mts:230-231
  For an Agent whose Harness uses a ServiceAccount, a member can have Agent update, Configuration read, and every Secret/credential-source operate grant while lacking only ServiceAccount read. The new agentBoundSourceGrants check then returns agentSourcesOperate=false and disables all three editors, but this message tells the member to request operate on Secrets and credential sources they already have. Following that instruction cannot restore saving. Include ServiceAccount read in the shared explanation and affected guide text, and cover this persona so the guidance matches the actual refusal.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Configuration bound-Secret permission summary keeps the original intent (1065fce: Expose exact bound-Secret operate requirements before Configuration saves; the predecessor explicitly left Agent saves for follow-up.)
• Disabled Agent editors and permission refresh keeps the original intent (df01c5f: Prevent unsaveable edits from blocking navigation and reread grants when rebuilding the draft.)
• Plugin capability reads for denied editors keeps the original intent (c000db1: Show saved selections read-only and avoid a capability request that a known read-only caller cannot use.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

Acceptance criteria:

• node --test tests/integration/console-api.test.mjs
• node --test tests/browser/console-agent-save-refusals.test.ts
• pnpm lint
• pnpm format:check
• pnpm docs:check

What I checked:

• Pinned change and current-main gap: The introduced diff adds agentSourcesOperate and applies it to Plugins, authentication, and repository editors; fetched main still gates these editors on Agent update alone. (apps/controller/src/console/agents/detail.mts:586, c39b476545b7)
• ServiceAccount recovery mismatch: The new predicate checks ServiceAccount read, but the displayed denial names only Secret and credential-source operate; existing conformance coverage establishes that missing ServiceAccount read independently refuses Agent updates. (apps/controller/src/console/agents/list.mts:231, c39b476545b7)
• Real authorization owner remains intact: OCC updateAgent separately authorizes current Harness sources, listed credential sources, Configuration read, and bound Configuration Secrets before saving; this PR changes advisory answers rather than mutation authority. (packages/occ/src/index.ts:9230, c39b476545b7)
• Distinct predecessor: fix(console): say up front when a configuration save needs secret operate on its bound secrets #3611 implemented Configuration-save hints and explicitly deferred Agent-save hints because their grants differ. (apps/controller/src/index.ts:2460, 1065fce57eab)
• Visual evidence and applicable gate: Inspected the downloaded desktop and phone authentication screenshots: the after views show disabled controls and the new note. The full PR body explicitly supplies no video and reports no local Storybook build; root policy requires a walkthrough video and browser inspection of rebuilt Storybook. (AGENTS.md, c39b476545b7)
• Policy and architecture: Read the complete root policy and implementation status; no nested AGENTS.md files or maintainer-notes directory were present in the inspected trees. Applied test and documentation guidance without running tests or review helpers. (docs/design.md:16, c39b476545b7)

Likely related people:

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

Labels

Label changes:

• add P2: This improves permission guidance for existing Agent editors with limited blast radius; server authorization already refuses unauthorized saves.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp 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: This improves permission guidance for existing Agent editors with limited blast radius; server authorization already refuses unauthorized saves.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp 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 25, 2026, 7:31 AM ET / 11:31 UTC.