Skip to content

fix(worker): hold a native git deployment for its first token and keep an existing agent running on quarantine - #3591

Merged
freeqaz merged 15 commits into
mainfrom
fix-2415
Oct 29, 2026
Merged

freeqaz merged 15 commits into
mainfrom
fix-2415

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

On the native prepared Git route (no Sandbox), a deployment could report succeeded with warnings: [] and error: null while the version had no repository Git token and every Git request answered a bare 503 (dogfood finding 2415: a rejected App key, and a stale grant after a registry switch). The worker log named the revision (PREPARED_GITHUB_RECOVERY_REQUIRED); nothing reached the deployment status, agent get or the console.

  • The worker asks GitHub for a version's first token only after the version's active pointer is published, so the deployment completed before any token existed.
  • A failed request quarantines the attempt, and that version is never prepared again. Only a new version heals it.

The first version of this PR failed such a deployment and then retired the version like any failed repository version. The earlier version had already stopped serving when the new one was published, so an existing Agent went fully offline, chat included (finding 2425).

Change

Owner decision 2026-10-28: a new Agent with configured Git repositories and no Git access fails; an existing Agent keeps running so it can report the issue.

  • Wait for the first token. PreparedRepositoryGitStore.revisionMaterial(revisionId) (native broker only, reads no material) answers ready, pending or quarantined. The worker checks it for a native repository deployment after it publishes the pointer and before it retires earlier versions: pending holds at REPOSITORY_GIT_MATERIAL_PENDING, quarantined fails with REPOSITORY_GIT_TOKEN_QUARANTINED. Maintenance claims and protected (OpenShell) admissions are not gated. An unfinished attempt stays pending; it ends at the convergence deadline.
  • New Agent: the version is retired, as before. "Existing" means an earlier revision of the same Agent was activated by its own deployment (REVISION_ACTIVATED or REVISION_ALREADY_ACTIVE, through controllerWorkDeploymentStatus). A deployment that completed stopped or superseded does not count.
  • Existing Agent: continueRevisionWithoutGit fails the deployment with continuingRevision and queues the version's maintenance in one transaction, under Namespace then Agent locks, after it rechecks the exact active, running revision, its service principal, a ready Namespace and the repository deadline. The version keeps running without Git, so users can still chat with the Agent and it can say Git is unavailable. Nothing in this path retires the predecessor.
  • Exhausted deployments: a published deployment that runs out of attempts keeps its runtime, as before, unless it is a new Agent's native version without a ready token. That version is now retired.
  • Convergence deadline (finding 2894): when the deployment's last wait was REPOSITORY_GIT_MATERIAL_PENDING and the token state read at the deadline is still pending (no attempt finished: an App key the worker cannot read after a Secret rotation, a worker restart during a request, an issuer change), an existing Agent's deployment fails with a new code, REPOSITORY_GIT_TOKEN_PENDING, through the same continueRevisionWithoutGit path (same locks, rechecks and maintenance in one transaction). A token state of quarantined at that moment uses REPOSITORY_GIT_TOKEN_QUARANTINED. A new Agent's version is still retired with CONVERGENCE_DEADLINE_EXCEEDED. The state is read again because the Sandbox install wait shares the pending code; a failed read keeps the deadline failure. If the token commits later, the kept version gets Git: its session stays open, and it is the running Agent's active, maintained version.
  • Maintenance accepts exactly these two codes (REPOSITORY_GIT_TOKEN_QUARANTINED, REPOSITORY_GIT_TOKEN_PENDING); it still refuses every other failed deployment, now pinned by a test.
  • controller-work.ts: two messages (249 and 239 characters, under the 256-character cap; both say to ask an Installation administrator) and deploy-again steps in deployment-failure-steps.ts. controllerWorkDeploymentStatus is exported from the OCC package index for the worker.
  • Docs: prepared-native-git.md ("Deployment and the first token": a new/existing table), the team runbook (rejected App key), and flow section 6. Older flow changelog entries moved to the history page to stay under the word limit.
  • Merged main (merge commit, no rebase). The two breaking-change pages main deleted in docs: remove upgrade notices kept for pre-release installs #3930 are dropped with this PR's notice.

Not done (needs an owner call or RFC): keeping the previous version serving until the new one gets Git. The Kubernetes Compute Driver stops a dedicated deployment's predecessors before it prepares the candidate (requiresStoppedPredecessors), and an embedded activation replaces the predecessor's Gateway. Holding the predecessor would change Driver behavior. This PR keeps the Agent available through the new version instead.

Verification

  • New and changed tests (shared fixtures in postgres-worker-revision-fixture.mjs and -support.mjs):
    • postgres-worker-agent-revision.test.mjs, a matrix: a new Agent with a ready, quarantined or pending (deadline) token; an existing Agent with a quarantined token or a token still pending at the deadline (REPOSITORY_GIT_TOKEN_PENDING): the deployment fails, the version stays active with its repository session open, is not stopped, the predecessor is not retired, its maintenance pass then succeeds with REVISION_ALREADY_ACTIVE, and a later version whose token is issued replaces it normally; and an Agent whose earlier version completed REVISION_STOPPED (treated as new, retired).
    • postgres-worker-agent-revision-teardown.test.mjs: a new Agent's published deployment that lost its lease (stale-claim recovery, ACTIVE_REVISION_RECOVERY) and then gets a quarantined token, or then exhausts its attempts on an unavailable dependency. Both fail and retire.
    • postgres-worker-agent-revision-teardown.test.mjs (exhausted published deployment, maintenance declared): maintenance queued later for a version whose deployment failed with another code (DEPENDENCY_UNAVAILABLE) is refused with REVISION_SUPERSEDED and prepares nothing.
    • postgres-worker-agent-revision.test.mjs, deadline guards: an existing Agent whose published version never activates (WORKSPACE_NODE_PENDING) while its token is pending, and one waiting at Compute's Sandbox install (REPOSITORY_GIT_MATERIAL_PENDING) with its token committed, still fail with CONVERGENCE_DEADLINE_EXCEEDED and retire the version: only the first-token wait keeps it.
    • Unchanged from earlier: the store's state test (postgres-prepared-github-git-use) and "maintenance ignores a quarantined token renewal".
  • Each new test fails without the change. On main plus the tests, all eight fail (no wait). On this branch before the owner change, the existing-Agent quarantine case and the new-Agent dependency-exhaustion case fail. On this branch before the deadline fix (finding 2894), the existing-Agent pending case fails (CONVERGENCE_DEADLINE_EXCEEDED instead of REPOSITORY_GIT_TOKEN_PENDING); every other test passes.
  • Mutants, each caught on a full postgres-application run: counting a raw succeeded work row as an earlier activation; locking the Agent before its Namespace (the fixture's lock-order check); retiring the predecessor in the kept path (now caught at once by a worker.error with CLAIM_LOST); keeping an exhausted new Agent's version without a token; maintenance refusing the kept version; queuing no maintenance in the kept path; maintenance exempting any failure code (the new refusal test); the deadline path retiring an existing Agent's version; maintenance not exempting REPOSITORY_GIT_TOKEN_PENDING. The deadline-guard tests catch two more: keeping the version for any last wait, and keeping it when the token state read at the deadline is ready. The earlier "accept another principal's activation" mutant no longer applies: the principal comparison it mutated was always true (a revision's principal is read from its Agent's row) and is removed; the earlier-activation check is scoped to the Agent.
  • On a remote test VM, at head 143acf9 (merged with main): postgres-application green, tsc --build clean, the matrix (6), the two new-Agent recovery tests and both exhausted-deployment tests pinned 20/20 at 2 CPUs (every run executed all of them), docs word count within limits (flow page 2477). ESLint and Prettier pass on the touched files.
  • Re-review on a remote test VM, at head 143acf9 merged with main 03372ea: tsc --build clean, postgres-application green, the matrix, recovery and exhausted-deployment tests 20/20 at 2 CPUs, all 9 mutants above caught again; the two deadline-guard tests 20/20 at 2 CPUs and each catches its mutant.
  • Live proof: not repeated for the owner change. The earlier live run on a remote k3d install (real GitHub, native route) covered the first-token wait and the quarantine failure for a first deployment, which is unchanged. The existing-Agent path has PostgreSQL integration proof only.

Risks

  • A native deployment finishes a few seconds later, after its first token commits.
  • Any failure of the first token request, including a transient one, quarantines the version and fails the deployment at once. An existing Agent then runs that version without Git until a new deployment succeeds; a new Agent has no running version until then.
  • An existing Agent's kept version has no Git and the deployment shows as failed while chat works. Its message says so and says to deploy a new version.
  • An existing Agent whose first token is still pending at the 15-minute convergence deadline now keeps that version running without Git (REPOSITORY_GIT_TOKEN_PENDING) instead of going offline. If the token commits later, the version gets Git without a new deployment. It is the Agent's active, authorized and maintained version, so that is the access a successful deployment would have given; the deployment still shows as failed.
  • The deadline decision reads the token state once more. If that read fails, the deadline failure stands and the version is retired, as before.
  • A published deployment whose lease is lost twice still ends LEASE_EXPIRED with its runtime kept, by queue recovery SQL that cannot read the token state (unchanged).
  • A deployment within about two minutes of an App key Secret change can still be signed with the old key.

…ail a quarantined one

Native prepared Git issues a version's first GitHub token only after its active
pointer is published, so a deployment reported succeeded with no warning while
every Git request answered 503 when GitHub refused the App key. The worker now
reads the version's token state from its prepared owner after activation: it
waits at REPOSITORY_GIT_MATERIAL_PENDING until the token commits and fails with
REPOSITORY_GIT_TOKEN_QUARANTINED when the preparation was quarantined, which no
later pass retries. Maintenance of running versions is unchanged.
…nt the deployment check

The status read now happens on the second check after the pointer is
published, once the first REPOSITORY_GIT_MATERIAL_PENDING deferral is recorded.
Document the first-token wait and the quarantined failure in the native Git
reference and the repository credential flow; move older flow changelog entries
to its history page to keep the page under the word limit.
@freeqaz
freeqaz requested a review from a team as a code owner October 25, 2026 06:21
@clawsweeper

clawsweeper Bot commented Oct 25, 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. 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. 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 25, 2026
@clawsweeper

clawsweeper Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

Native repository deployments wait for their first GitHub token and fail explicitly when token preparation is quarantined.

Example: GitHub returns HTTP 401 when preparing a native Agent revision's first repository token.

  • Before: The deployment reports succeeded while Git requests return HTTP 503.
  • After: The deployment reports REPOSITORY_GIT_TOKEN_QUARANTINED with recovery instructions; its published runtime remains running.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused repair with passing exact-head integration checks and no identified correctness defect; strongest behavior evidence remains component-level.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; supplied verification covers PostgreSQL-backed store behavior and worker decisions with an injected readiness callback, rather than a shipped-worker after-fix run.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A deployment reports success even though its requested repository access has no usable token and cannot recover within that revision.
Reason: The maintainer-authored direction makes deployment status reflect requested Git readiness and supplies actionable quarantine recovery without adding a configuration or public Driver contract.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it addresses a current deployment-status bug, follows an explicit maintainer decision, and has no concrete introduced defect identified in this review.

Priority: P2
Reviewed head: b7867a7c2bbdce99a9784526ed47d848795fcd6f

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No supplied after-fix trace exercises the shipped worker with its real prepared owner from pointer publication through token readiness or quarantine.
Agent review details

How this fits together

The controller worker reconciles admitted Agent revisions, reads token readiness from its prepared Git owner, and publishes deployment progress or failure through OCC.

flowchart TB
  A[Agent deployment] --> B[Controller worker]
  B --> C[Publish active revision]
  C --> D[Prepared Git owner]
  D --> E[GitHub token preparation]
  E --> F[PostgreSQL token metadata]
  F --> B
  B --> G[Deployment status]
Loading

Technical review

Best possible solution:

Land the focused readiness gate while preserving existing token issuance, authorization, cleanup, and published-runtime ownership.

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

Source confirms that native token discovery requires the published active pointer while main completes deployment without observing token readiness; no target code was executed during this review.

Is this the best way to solve the issue?

The existing prepared owner is the appropriate readiness source, and checking both publication paths fixes deployment reporting without moving issuance into reconciliation or changing send authorization.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Prepared native Git lifecycle keeps the original intent (3115679: Separate background token preparation from request handling and retain existing ownership through settlement, as described in Credential Proxy Implementation #2487.)
  • Deployment completion after active-pointer publication changes intended behavior with a stated reason (unknown: Existing worker code publishes before activation and preserves published runtimes after permanent activation failures; available history does not establish the original introduction of that ordering.)
  • Repository deployment progress and recovery copy changes intended behavior with a stated reason (3115679: The previous progress message described protected Sandbox installation; the new native wait requires wording that covers both delivery routes.)
  • Flow history placement keeps the original intent (bc1262a: Move older unpublished changelog entries verbatim into the existing history page to preserve records within documentation length limits.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 38ecfca adds apps/controller/src/backends/repository-credentials/prepared-git-owner.ts:361 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 38ecfca18762; files: apps/controller/src/backends/repository-credentials/prepared-git-owner.ts)

Labels

Label changes:

  • add P2: Fixes misleading success for the optional native repository credential path, with limited scope and no demonstrated urgent regression.
  • 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: Fixes misleading success for the optional native repository credential path, with limited scope and no demonstrated urgent regression.
  • 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.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Increase confidence with a shipped-worker trace showing pending progress followed by token readiness or quarantine failure.

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, 2:34 AM ET / 06:34 UTC.

- Name unreachable GitHub in REPOSITORY_GIT_TOKEN_QUARANTINED and say to deploy
  a new version when the key and registry are correct.
- Keep an unfinished issuing attempt pending: its own run can still commit it
  after the dispatch window (a suspended worker host or a delayed commit), and
  a false quarantine would fail the deployment for good. The reference now says
  other waits end at the convergence deadline.
- Test that a running version's maintenance pass ignores a quarantined renewal.
- Move the misplaced doc comment, use the store's material state type, and say
  that earlier versions stay until the next deployment.
- Breaking-change notice for native repository deployments.
The new 2026-10-25 entry put breaking-changes.md over the 2500-word limit.
A live run on a k3d install showed the PR's claim wrong: like any repository
version that fails for good, the failed version's work transfers to terminal
retirement, which stops its runtime and clears the Agent's active pointer. The
earlier version was already retired when the new one was published, so the
Agent has no running version until a deployment succeeds.

- Reference, flow, runbook and breaking-change notice now say so.
- The quarantined worker test waits for the retirement and asserts the stopped
  runtime and the cleared pointer instead of the pointer right after failure.
- The runbook says to wait about two minutes for the mounted App key Secret to
  update before deploying: a version deployed earlier can still be signed with
  the rejected key and fail again (seen live).
@freeqaz

freeqaz commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Free's clanker here (an AI agent working on Free's behalf).

Not landing this yet. What blocks it:

  1. Open behaviour decision (owner call). With this change, any failure of the first token request (including a transient GitHub outage or a timeout inside the 25-second budget) fails the deployment with REPOSITORY_GIT_TOKEN_QUARANTINED. A permanently failed repository version is then retired (runtime stopped, active pointer cleared), and the previous version was already retired when the new one was published. The Agent therefore goes fully offline, chat included, until a later deployment succeeds. Before this PR the same failure left chat running with Git broken. The options on the table:
    • (a) accept it: fail early, and the Agent stays offline until a good deployment;
    • (b) keep the runtime up on REPOSITORY_GIT_TOKEN_QUARANTINED so chat survives a GitHub failure (this changes the failed-version retirement in the worker);
    • (c) land only the REPOSITORY_GIT_MATERIAL_PENDING hold and treat a quarantine as a non-retiring failure.
      The recommendation is (b), or (c) as the smaller change. Unblocked by: a decision from @freeqaz.
  2. Merge conflicts with main (198 commits behind). These files conflict: packages/occ/src/state/controller-work.ts, docs/flows/agent-repository-credentials.md, docs/flows/agent-repository-credentials/history.md, docs/guides/deploy/breaking-changes.md and docs/guides/deploy/breaking-changes-earlier-2026-10-24.md. prepared-git-owner.ts merges cleanly. Once the decision in item 1 is made, merge main, re-check the docs word cap and wait for CI Required and First Agent Smoke on the new head.

The code review is done: no deadlock, a read-only query, and maintenance passes are not gated (a test pins this). The live checks on a remote test VM passed: a good key held briefly and then succeeded, a rejected key failed with the new code, and a restored key succeeded again. Also open: a docs follow-up. After a GitHub App key Secret changes, the worker can sign with the old key for one to two minutes, so a deployment in that window fails as quarantined. The runbook now tells operators to wait.

Resolve controller-work.ts on main's tightened messages, add
REPOSITORY_GIT_TOKEN_QUARANTINED to the failure-step table and fit its
message in the 256-character cap. Drop the two breaking-change pages main
deleted (#3930) and keep the flow changelog consistent with main.
…git token is quarantined

Owner decision 2026-10-28 (finding 2425): a new Agent with repository
access and no Git access fails; an existing Agent keeps running so it can
report the problem.

A quarantined first token now fails the deployment and, when an earlier
revision of the same service principal was activated by its own deployment
(stopped or superseded completions do not count), keeps the version running:
the deployment fails with continuingRevision and its maintenance chain starts
in one transaction under Namespace then Agent locks. Maintenance accepts that
failure code. A new Agent's version is still retired. An exhausted published
deployment of a new Agent without a token is retired instead of kept. A token
still pending at the convergence deadline retires the version for every Agent,
since it could still commit later.
…e flow under the word limit

The existing-Agent case now checks that nothing retired the predecessor only
after the failed pass's completion event, so a retirement made after the
failure commit cannot race the assertion (a retire-predecessor mutant
survived before). Move the flow's changelog entries dated 2026-10-21 to
2026-10-25 to its history page and tighten the first-token paragraph
(2477 words).
@freeqaz freeqaz changed the title fix(worker): hold a native git deployment until its first token and fail a quarantined one fix(worker): hold a native git deployment for its first token and keep an existing agent running on quarantine Oct 29, 2026
…git token is pending at the deadline

Finding 2894: a native deployment still waiting for its version's first GitHub token at the
convergence deadline ended CONVERGENCE_DEADLINE_EXCEEDED and retired the version for every
Agent, so an existing Agent went fully offline (owner decision 2026-10-28 says it stays running).

At the deadline, when the last wait was REPOSITORY_GIT_MATERIAL_PENDING and the token state is
still pending (or quarantined), the deployment goes through continueRevisionWithoutGit: an
existing Agent's deployment fails with REPOSITORY_GIT_TOKEN_PENDING, keeps the version and queues
its maintenance in one transaction; maintenance accepts that code as it does the quarantined one.
A new Agent's version is still retired with CONVERGENCE_DEADLINE_EXCEEDED. A token committed
later gives the kept version Git: its session stays open.

Finding 2895: drop the tautological principal check in activatedEarlierRevision (a revision's
principal is the Agent's own) and pin that maintenance refuses a version whose deployment failed
with any other code. Sharper existing-Agent assertion and a later fixed deployment step from the
independent review.
…ent's version at the deadline

The finding 2894 path keeps an existing Agent's version at the convergence
deadline only when the deployment's last wait was the first Git token and the
token is still pending. Two guards had no test: a published version whose
activation never completes (WORKSPACE_NODE_PENDING) with its token pending, and
Compute's Sandbox install wait (REPOSITORY_GIT_MATERIAL_PENDING, same code as
the token wait) once the token is committed. Both must still fail with
CONVERGENCE_DEADLINE_EXCEEDED and retire the version. Each case kills one
mutant that the existing suite let survive (dropping the last-wait check;
keeping the version when the re-read token state is ready).
@freeqaz
freeqaz merged commit 2eed3c3 into main Oct 29, 2026
42 of 43 checks passed
@freeqaz
freeqaz deleted the fix-2415 branch October 29, 2026 20:26
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: 🦐 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