[freeqaz]

Problem

Finding 2536, slice 3 (after #3725 and #3732). Some refusals still answered a generic 404, or a 500, although each comes after the caller's grants and is about the request or an Installation property:

• Credential source create with a Credential Proxy selected: a type the Proxy cannot protect (for example codex-pat) and a Secret reference to another Namespace both answered 404 The requested platform resource was not found.
• Prepared and credential-free source create: an Agent being deleted, or an embedded Agent, in config.agent_id answered the same 404.
• Withdrawal of a source the Agent's active version does not use answered 404.
• Agent save or deploy that binds a Proxy-owned source (prepared or credential-free) after the Installation dropped drivers.credential_proxy answered 500 INTERNAL_ERROR (a DriverSelectionError from the owner lookup). The intended sourcesWithoutProxy 409 was reachable only for live-adopted Gateway sources.
• The failed device-login 503 tells the caller to ask an operator to check the API logs, but the Refresh Driver's error was dropped in a bare catch {}, so the log record named only the 503 itself.
• Review of fix(api): name saved repository, Credential Proxy deploy and protected source refusals #3732 also found no test for sourcesWithoutProxy, dedicatedSandbox, gitAlone or the agents.defaults.model 400.

Change

• admitStaticEgressSource: a type the selected Proxy cannot protect is a 409 RESOURCE_CONFLICT that names the type and lists the types it protects; a Secret reference to another Namespace is the Credential Gateway path's 400 (Credential source Secrets cannot cross Namespaces.).
• admitPreparedTokenSource, admitCredentialFreeMetadata: after the Agent operate grant and lookup, a deleting Agent is 409 AGENT_DELETING and an embedded Agent a 409 that names config.agent_id. A missing Agent stays the generic 404. The prepared issuer reference to another Namespace is the same 400.
• withdrawAgentCredentialSource: a source the active revision does not use is a 409. The check reads only the revision's bindings, never the source, so any source ID gets the same answer.
• authorizeAgentCredentialSources (Agent create, update, deploy): a Proxy-owned source with no Proxy selected is CredentialProxyAdmissionError("sourcesWithoutProxy"), 409, instead of a 500.
• RetryLaterError("deviceLoginFailed", cause): the caught Refresh Driver error is the cause. The 503 log record keeps only a cause's class and code (dependencyUnavailableLogFields), never its message.
• Docs: a breaking-changes notice (the oldest entry moves to the earlier page to stay under the word limit) and the credential sources error table.

Not changed, recorded in the finding's site list:

• dedicatedSandbox is unreachable: selectedProxyOwner() refuses a Proxy without its paired Sandbox first, and every composition installs both together. No test, since no supported Installation reaches it.
• gitAlone is reachable only with protected repository Git on a qualified sender (an external Driver with exact dispatch and no policy reduction, or the OpenShell observed-hook preview) plus a protected source. No in-memory fixture deploys either with a source today, so it stays untested here; it belongs with the PostgreSQL external Git journey.
• The Compute protected-execution admission catch-all keeps its fixed 503: passing a Compute Driver's reason through needs a typed refusal in the Driver contract.

Verification

• New tests/integration/protected-source-refusals.test.mjs (checks-baseline-1): real app, IAM, controller and OpenShell Proxy and Gateway Drivers on the static egress fixture. It runs with prepared GitHub metadata enabled (the longest list of protected types) and asserts status, code and the full text (no cap ellipsis) for each create refusal, deleting, embedded and missing Agents for both credential-free-repository-metadata and github-app-metadata, the agents.defaults.model 400 and sourcesWithoutProxy for both a live-adopted Gateway source and a Proxy-owned source (a second controller opens the same state after the Installation drops the Proxy, as a new API process would; its answer goes through the real HTTP error mapper), and the withdrawal 409 for an existing and a missing source ID.
• Identical answers: a caller who may create sources but not operate the Agent gets the same 403 for an existing Agent and a missing one; withdrawal answers an unused source and a missing ID alike.
• The device-login test asserts the 503 log record names RetryLaterError with cause class TypeError and does not contain the private failure text. The prepared metadata test covers the issuer reference. The conformance withdrawal case asserts the mapped 409 for two IDs.
• Mutants, each killed: the cause dropped; each create retype reverted to the 404 (4); the withdrawal retype reverted (fails both files); the dropped-Proxy check removed (500); the admitProxyPlan check made to return; the model 400 reverted; the Agent check moved before the operate grant (identical-answer test: 409 against 403); the prepared issuer retype reverted; the Agent check removed from the prepared path; the type refusal text put back with the administrator after the type list.
• tsc --build, eslint and Prettier on the touched files; the docs word count (breaking-changes 2477, earlier page 2494).
• The touched test files pass 20/20 at 2 CPUs on a remote test VM.

Risks

• Status changes (404 to 409/400, 500 to 409) are in the breaking-changes notice.
• The 409 for a type the Proxy cannot protect lists the Proxy's protected types, an Installation property.
• Agent save of a binding to a Proxy-owned source after the Proxy is dropped now refuses with a 409 instead of a 500; no other caller of the owner lookup changes.

[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 gives protected credential-source refusals actionable HTTP errors and preserves safe diagnostic information for failed device logins.

Example: Create a codex-pat credential source while the Installation selects a Credential Proxy that cannot protect that type.

• Before: 404 NOT_FOUND with a generic resource-not-found message.
• After: 409 RESOURCE_CONFLICT naming codex-pat and listing the Proxy's supported types.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Focused production repairs with meaningful integration coverage; evidence remains fixture-based and the new suite needs inclusion in TypeScript checking.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored PR is outside the ordinary contributor proof gate; supplied verification exercises real API and OCC owners with fixture dependencies and reports repeated passing and mutation runs.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Authorized API callers receive misleading resource-not-found or internal-error answers for credential-source rules they can act on.
Reason: The collaborator-authored direction continues the existing named-refusal repair and keeps the generic missing-resource boundary.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep this useful, collaborator-authored PR open; the production changes preserve authorization boundaries, with one required correction to the new integration test.

Priority: P2
Reviewed head: 1c161eafb20fc73e823d0a22735be3ef87e7e7a7

Before merge

• Include the new integration suite in TypeScript checking (P3) - This newly added integration suite uses .test.mjs, contrary to AGENTS.md's explicit requirement that new integration tests use .test.ts with erasable types. tests/tsconfig.json selects **/*.test.ts, so the new suite is omitted from the repository's test typecheck. Convert it to .test.ts, add the necessary parameter types, and update both references in checks-baseline-1.json.

Findings

• [P3] Include the new integration suite in TypeScript checking — tests/integration/protected-source-refusals.test.mjs:1-4

Tests

• Missing end-to-end proof: Supplied verification uses in-process fixtures; no separately started API run is recorded, and the dropped-Proxy case invokes a second controller plus the HTTP mapper directly.

Agent review details

How this fits together

The API passes authenticated resource operations to OCC, which authorizes exact resources, checks credential-source and Driver ownership, and returns errors through the HTTP mapper.

flowchart LR
  Client[API client] --> API[HTTP API]
  API --> OCC[OCC admission]
  OCC --> IAM[IAM authorization]
  OCC --> State[Platform state]
  OCC --> Drivers[Credential Drivers]
  OCC --> Mapper[HTTP error mapper]
  Mapper --> Client

Loading

Technical review

Best possible solution:

Retain the focused OCC error changes and convert the new integration test to TypeScript with its CI references updated.

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

Current-main rejection sites and the HTTP mapper establish the generic responses and missing-owner failure without executing target code.

Is this the best way to solve the issue?

The patch uses existing OCC error classes and authorization owners, preserving rejection conditions while making their responses actionable.

Full review comments:

• [P3] Include the new integration suite in TypeScript checking — tests/integration/protected-source-refusals.test.mjs:1-4
  This newly added integration suite uses .test.mjs, contrary to AGENTS.md's explicit requirement that new integration tests use .test.ts with erasable types. tests/tsconfig.json selects **/*.test.ts, so the new suite is omitted from the repository's test typecheck. Convert it to .test.ts, add the necessary parameter types, and update both references in checks-baseline-1.json.
  Confidence: 1

Overall correctness: patch is correct
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Protected-source creation changes intended behavior with a stated reason (3115679: The Credential Proxy implementation required exact Namespace material, an authorized dedicated Agent, and trusted issuer origin.)
• Credential withdrawal keeps the original intent (feat: update credential sources and withdraw them from running Agents #553: Withdrawal applies only to a source held by the active revision; later work extended withdrawal to admitted successors.)
• RetryLaterError diagnostics keeps the original intent (c6e9704: Fixed retry and recovery messages were exposed after authorization while retaining dependency-error retry semantics.)
• Recorded credential-source ownership keeps the original intent (3115679: Sources retain their recorded Gateway or Proxy owner instead of silently moving between credential delivery paths.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

• node --test tests/integration/protected-source-refusals.test.ts
• pnpm typecheck
• node scripts/ci/run-tests.mjs audit
• pnpm lint
• pnpm format:check

What I checked:

• Pinned production changes: The introduced diff retypes existing rejection paths, retains Agent and source authorization before state disclosures, and checks for a missing Proxy before resolving a Proxy-owned source. (packages/occ/src/index.ts:11536, 1c161eafb20f)
• Current-main boundary: Fetched main still combines prepared-source Agent state and foreign Secret references into ScopeViolationError; the preceding merged slices do not implement this PR's central changes. (packages/occ/src/index.ts:5788, 8193a16a896e)
• New integration test misses the TypeScript project: AGENTS.md requires new integration tests to use .test.ts, while tests/tsconfig.json includes **/*.test.ts; the newly added .test.mjs file is outside that selection. (tests/tsconfig.json:15, 1c161eafb20f)
• Safe cause logging: RetryLaterError now retains the caught device-login error as its cause; the existing logger traverses bounded cause metadata without logging cause messages. (apps/controller/src/http/errors.ts:523, 1c161eafb20f)
• Owner direction and accepted response changes: The collaborator-authored body explicitly accepts changed HTTP statuses and disclosure of Installation-wide supported types, and the breaking-change notice documents the client impact. (docs/guides/deploy/breaking-changes.md:24, 1c161eafb20f)
• Discussion and related repairs: Discussion contains only review-status bot comments; fix(api): name repository, standing approval and credential source field refusals #3725 and fix(api): name saved repository, Credential Proxy deploy and protected source refusals #3732 establish the preceding named-error repairs and preserve generic answers for unauthorized or missing resources.

Review metrics

Metric	Value	Why it matters
Production and regression coverage	Production +76/-24 lines; tests and test helpers +381/-9 lines	Most growth exercises distinct refusal paths, authorization-equivalent answers, dropped-Driver state, and diagnostic redaction.

Labels

Label changes:

• add P2: The PR repairs misleading responses and an internal-error response in limited credential-source workflows.
• add merge-risk: 🚨 compatibility: Clients branching on the previous 404 or 500 responses now receive 400 or 409; the author explicitly accepts and documents this change.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P2: The PR repairs misleading responses and an internal-error response in limited credential-source workflows.
• merge-risk: 🚨 compatibility: Clients branching on the previous 404 or 500 responses now receive 400 or 409; the author explicitly accepts and documents this change.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• 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:49 AM ET / 07:49 UTC.