Skip to content

fix(api): name saved repository, Credential Proxy deploy and protected source refusals - #3732

Merged
freeqaz merged 5 commits into
mainfrom
errmap-2536b
Oct 26, 2026
Merged

freeqaz merged 5 commits into
mainfrom
errmap-2536b

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Finding 2536, second slice (first slice: #3725). Several refusals still reached clients as the generic 404 NOT_FOUND, 409 ...already exists or a retryable 503, although the caller could already read the resource and the cause was a rule they can act on:

  • Saved repository selections that repository policy no longer approves answered 400 on deploy and on GET .../repository-approval-scopes, as if the request had sent them (review of fix(api): name repository, standing approval and credential source field refusals #3725, nit 1).
  • A failed provisioning job said only Agent provisioning could not complete. for fixed, ID-free repository refusals (nit 2).
  • Agent POST create and POST provision had no test that a missing and an unreadable Namespace answer alike (nit 4).
  • Credential Proxy deploy admission answered 503 (retry later) for Installation configuration that never changes by retrying, and 404 for plan shapes the Proxy cannot serve.
  • Update and rotation of a prepared or credential-free protected source answered 404 for a source the caller had just read.
  • The registration-fence delete and a failed device-login poll had user-worded retry texts that the mapper replaced with A required platform dependency is unavailable.

Change

  • SavedRepositorySelectionError (409): stored-state callers (deploy, standing-approval scope read, Agent update approving, provisioning worker plan, repository credential admission) resolve with origin: "saved". Request-sent selections keep the 400 from fix(api): name repository, standing approval and credential source field refusals #3725. Text: The saved repository selections are no longer approved for this Namespace: repository policy changed after they were saved. Choose approved ones from the repository options, or ask an Installation administrator to approve them again.
  • ComputeRepositoryCredentialsUnsupportedError (409, the fix(api): name repository, standing approval and credential source field refusals #3725 text as a class), and provisioningFailureMessage passes it, RepositoryBindingSelectionError, SavedRepositorySelectionError and CredentialProxyAdmissionError through to the job status (precedent RepositoryDriverNotConfiguredError; all fixed texts that name no resource).
  • CredentialProxyAdmissionError (409) for the five Credential Proxy plan refusals in admitProxyPlan (Installation-wide access without a Proxy, Proxy sources without a selected Proxy, several sources without policy reduction, no Sandbox Driver, other sources beside repository Git). Each names the Installation rule and the fix, for example: Installation-wide repository access (profile github-installation) needs a Credential Proxy, and this Installation selects none. Choose another repository profile, or ask an Installation administrator to select drivers.credential_proxy. A live-adopted source without a provider/model default becomes a 400 naming agents.defaults.model.
  • Prepared or credential-free protected source update and rotate: 409 RESOURCE_CONFLICT, ...create a new source, bind it in place of this one, then delete this one.
  • RetryLaterError (a DependencyUnavailableError, still 503, message passed verbatim): Retry the deletion in 70 seconds; nothing else needs to change. and The sign-in service did not complete this device login, so it was cancelled. Start a new device login. ...
  • OpenAPI: the repository-approval-scopes route lists 409.
  • Docs: breaking-changes notice (status changes), reference pages for Agents and credential sources, repository credentials guide, egress flow. The oldest breaking-changes entry moved to the earlier page to keep the word cap.

Every retyped site throws only after the caller's grant and lookup succeed; plain ScopeViolationError stays the generic 404, so missing and unreadable resources still answer alike. The full site list, with what stays generic and why, is kept outside this repository; left for a follow-up are credential source create refusals and the Compute protected-execution admission catch-all (it swallows the Driver's reason). The Kubernetes Compute already exists family never reaches the HTTP mapper as 409 (runtime credential, log and diagnostic calls are wrapped), so it is not retyped here.

Verification

  • New and changed cases assert status and exact text through tests/helpers/refusal-text.mjs: saved selections on deploy and approval scopes (409), the installation-access Credential Proxy deploy (409, no revision), provisioning status for the Compute and saved-selection refusals (postgres), protected source update and rotate (postgres), the fence seconds (70 seconds, then 1 second), and the device-login poll.
  • Identical-answer tests (a known missing ID and an unreadable one give the same status and body): Agent POST create, POST provision, credential source update, rotate and delete, and the device-login poll. Deploy's was already covered.
  • Mutants, each killed by a test:
    • saved selections back to the request-sent 400: deploy and approval-scope tests fail;
    • the installation-access refusal back to a 503: the installation-access deploy test fails;
    • the fence back to a plain dependency error: the fence test fails on the generic text;
    • the protected update refusal moved ahead of the update grant and naming the source ID: the identical-answer test fails (409 against 403);
    • the fence refusal moved ahead of the delete grant and naming the source: the identical-answer test fails (503 against 403).
  • Touched test files 20/20 at 2 CPUs: repository-credentials-admission, credential-source-occ and openshell-device-authorization-api together; postgres-protected-credential-free and postgres-agent-provisioning each on a fresh database per run. The first CI run found credential-proxy-withdrawal still expecting the old 404 for the one-source refusal; it now asserts the 409 text, pinned 20/20 the same way.
  • tsc --build (sources and tests), eslint on touched files and prettier on touched docs and tests are clean.

Risks

  • Status changes: clients that branch on 400 for saved selections, or 503/404 for these deploy and protected-source refusals, see 409/400. The breaking-changes notice lists each.
  • Provisioning now fails the Credential Proxy configuration cases at once (PROVISIONING_REJECTED) instead of retrying them; they could not succeed without an Installation change.
  • A deployer now learns that the Installation selects no Credential Proxy or no Sandbox Driver. Both are Installation-wide facts the deployer needs in order to act, and the refusal names no other resource.

… refusals

Finding 2536, slice 2. Saved repository selections the policy no longer
approves answer 409 (the request sent none); provisioning status passes
the fixed repository and Credential Proxy texts through; protected deploy
admission, protected source update/rotate and the delete fence and device
login 503s name what blocks and what to do.
# Conflicts:
#	tests/integration/postgres-agent-provisioning.test.mjs
…e refusals

Breaking-changes notice for the status changes (saved selections 400 -> 409,
Credential Proxy deploy admission 503/404 -> 409, protected source update and
rotate 404 -> 409), with the oldest notice moved to the earlier page to stay
under the word cap. Reference and guide pages name the new answers.

Tests: the postgres provisioning file shares one repository policy helper;
the protected source rotate check sends no body.
@freeqaz
freeqaz requested a review from a team as a code owner October 26, 2026 06:45
@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.

The withdrawal test deployed two protected sources on a Proxy without
policy reduction and expected the old generic 404; deploy now answers
409 RESOURCE_CONFLICT with the CredentialProxyAdmissionError text.
@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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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 maintainer review before merge.

What this changes

This PR gives saved repository, protected deployment, credential-source, and device-login refusals specific status codes and recovery messages.

Example: Deploy an Agent saved with repository project and profile git-write after Namespace policy permits only git-read.

  • Before: 400 INVALID_REQUEST treats the saved selections as invalid request input.
  • After: 409 RESOURCE_CONFLICT explains that saved repository selections are no longer approved and offers recovery steps.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) A focused, useful repair with meaningful integration coverage and no identified defect; behavioral evidence remains harness-based, and no schema or serialized-format migration is needed.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; its API, worker, and credential-source behavior is supported by focused integration tests and exact-head CI rather than a supplied live-server artifact.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Authorized callers receive misleading not-found, already-exists, or retry-later responses for refusals that require a specific corrective action.
Reason: The maintainer-authored direction repairs existing diagnostics, documents compatibility consequences, and preserves the underlying admission rules.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR fixes misleading refusals still present on main, and no actionable introduced correctness or security defect was found.

Priority: P2
Reviewed head: d228c3a022dd33f41bf3a1784bbf96bfac4c31c5

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No separate running-server HTTP trace is supplied; reviewed evidence consists of registered-app and controller harnesses, PostgreSQL integration coverage, and reported mutation checks.
Agent review details

How this fits together

OCC authorizes resource operations and checks admission; typed errors leave through the controller's HTTP mapper or the durable provisioning status.

flowchart LR
  Request[API request] --> IAM[Exact operation authorization]
  IAM --> OCC[OCC admission]
  OCC --> Mapper[HTTP error mapper]
  Mapper --> Response[Status and recovery message]
  OCC --> Worker[Provisioning worker]
  Worker --> Status[Provisioning status]
Loading

Technical review

Best possible solution:

Land the focused typed-refusal repair with its documented status changes and existing authorization, cleanup, and resource-concealment boundaries preserved.

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

Current-main source shows the affected refusals reaching generic HTTP mappings or request-error classification; focused regression and mutation results are reported, but this read-only review ran no target code.

Is this the best way to solve the issue?

Typed fixed messages and explicit request-versus-saved origins repair the existing owners without globally exposing arbitrary errors or adding a parallel admission path.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Saved repository selections and Compute refusals changes intended behavior with a stated reason (ee9f23a: fix(api): name repository, standing approval and credential source field refusals #3725 introduced named repository refusals while retaining generic errors for missing or unreadable resources; this follow-up distinguishes saved state and exposes fixed worker messages.)
  • Credential Proxy plan admission changes intended behavior with a stated reason (3115679: Credential Proxy Implementation #2487 established protected-plan and source-composition restrictions; their predicates remain intact while permanent configuration refusals become named conflicts.)
  • Prepared and credential-free source update and rotation keeps the original intent (3115679: Protected grants require explicit new admission rather than in-place replacement; the patch retains rejection and explains the replacement workflow after authorization.)
  • Registration cleanup fence keeps the original intent (52ed4cd: A late registration must not leave credential material without a cleanup handle; the 70-second fence and retained record remain unchanged.)
  • Failed device-login poll keeps the original intent (unknown: Base source cancels uncertain exchanges rather than replaying them. Feature history includes device OAuth custody work, but older-content searches encountered unavailable blobs, leaving the precise refusal introduction unknown.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

  • Introduced changes: The pinned production diff changes error types, fixed messages, saved-selection classification, and the approval-scopes response contract while retaining admission predicates. (packages/occ/src/index.ts:5964, d228c3a022dd)
  • Current main retains the affected behavior: The relevant OCC, HTTP-mapping, and route-contract files are unchanged between the introduction base and fetched main; the central diagnostic repair remains necessary. (packages/occ/src/index.ts:5939, 80ee1d538b7b)
  • Authorization and disclosure boundaries: Source update and rotation still authorize before lookup and refusal; deployment authorizes bound sources before Proxy-plan admission, and added tests compare missing and unreadable identities. (packages/occ/src/index.ts:6381, d228c3a022dd)
  • Accepted compatibility consequences: The collaborator-authored PR explicitly accepts changed HTTP statuses, permanent rejection of fixed Proxy configuration failures, and disclosure of ID-free Installation facts; its breaking-change notice documents client handling. (docs/guides/deploy/breaking-changes.md:49, d228c3a022dd)
  • Focused integration coverage and exact-head CI: Changed scenarios cover API responses, worker terminal status, protected-source refusals, registration-fence timing, and device-login failure; their files are registered in CI, and CI Required plus PostgreSQL Platform and Application succeeded for the reviewed head. (tests/integration/postgres-agent-provisioning.test.mjs:1132, d228c3a022dd)
  • Registration-fence rationale: The original fence prevents a late credential registration from outliving its cleanup record; this PR retains that interval and deletion ordering and exposes the remaining wait. (packages/occ/src/index.ts:6836, 52ed4cdef0b6)

Labels

Label changes:

  • add P2: The repair improves diagnosis and recovery for rejected API and provisioning operations with limited blast radius.
  • add merge-risk: 🚨 compatibility: Clients matching the previous statuses or generic messages must adapt; the maintainer-authored PR explicitly accepts and documents this consequence.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: The repair improves diagnosis and recovery for rejected API and provisioning operations with limited blast radius.
  • merge-risk: 🚨 compatibility: Clients matching the previous statuses or generic messages must adapt; the maintainer-authored PR explicitly accepts and documents this consequence.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

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:10 AM ET / 07:10 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant