Every push previously re-minted a fresh GitHub App installation token and
embedded it in the origin URL, so pushes routinely landed inside GitHub's
token-replication lag window (run 01M0DH033P2XSTHAGVBHG6922F failed
terminally on four consecutive fresh-token 404s). Reusing mature tokens
removes the failure trigger and saves two GitHub API calls plus one sandbox
exec per push.
- New fabro_github::token_source::InstallationTokenSource: one cached,
single-flight source per origin repo. Static credentials pass through
(generation 0); App credentials mint through the cache and reuse tokens
until REFRESH_MARGIN (10 min) before expiry. Every resolve returns a
non-secret TokenSnapshot (generation + Minted/Reused/Static provenance),
and the source logs mints at INFO and reuses at DEBUG.
- Docker and Daytona share the source through PushCredentialState: an embed
mutex serializes compare -> set-url -> record, a matching generation skips
the set-url exec, and the generation is recorded only after a successful
exec. The clone still mints its own token, but now seeds the source cache
(generation 1) and the last-embedded state, so a refresh mint failure
falls back to the known embedded token instead of believing nothing was
ever embedded.
- RefreshOutcome now reports the remote action (embedded/unchanged/none)
separately from the token snapshot; git_push_via_exec logs token age and
provenance with each push, and refresh failures log the last embedded
generation.
- The run-metadata writer resolves through the sandbox's shared source
instead of minting per snapshot (with its own cached source on resume).
- The ACP refresh-ahead loop reschedules from the embedded token's
expires_at minus the margin instead of a fixed 45-minute interval, which
a cached source would have broken for long turns; static credentials stop
the loop.
Plan: .ai/plans/git-push-token-resilience.md (PR 1: items 3 and 6).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Structural cleanup of the durable pull request creation feature, from a
three-agent review (reuse, quality, efficiency) of the branch:
- Move the supervisor out of handler/ into server/pull_request_supervisor.rs,
collapse its double bookkeeping into one task-id map, and fold the five
copy-pasted failure arms into attempt_pull_request_creation.
- Tag pull_request.failed events with the creation id they resolve, so a
publish-stage failure can never fail an unrelated explicit creation. The
reducer gains PullRequestCreation::succeed/fail transition methods.
- Scan pending creations through a narrow projection-cache accessor instead
of materializing every run summary, raise the scan interval to 30s (notify
covers the live path), and cap retries for runs whose worker cannot even
record a failure.
- Answer "creation already pending" POSTs before taking the per-run create
lock, which a worker can hold for the whole creation.
- Replace the hand-rolled per-run lock map with fabro_store::KeyedMutex.
- Reuse cheap Arc'd projections (cached_run_projection) on the poll endpoint
and in the worker instead of deep-cloning run summaries and diffs.
- Merge ExistingPullRequest into fabro_github::CreatedPullRequest and
extract one reconcile_existing_pull_request helper for both call sites.
- Give the client poll loop a 15-minute deadline; document that Retry-After
and the poll interval are the same constant.
- Resolve a wedged pending creation (run already has a pull request) as a
durable failure instead of skipping it forever.
- Tests: shared wait_for_pull_request_creation helper, a pinned generation-
failure assertion, and a new pipeline test proving reconciliation adopts
an existing PR without an LLM call or create request.
Verified: cargo build --workspace, cargo nextest run --workspace (7,767
passed), nightly clippy -D warnings, fmt --check, insta (no pending), bun
typecheck in fabro-api-client.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up cleanup on the publish-failures change.
Error model:
- Collapse `Error::{Engine, Publish, Handler}` into one `Error::Stage` with an
`ErrorStage` discriminator. The three shared a field shape and had to be
edited together in four match groups; nine near-identical constructors
become two private helpers.
- Add `Error::failure_reason()`, replacing the same error -> FailureReason
mapping written out in four places.
- Publish errors are now terminal. Publish runs once, after execution, so no
caller could ever act on the retryable classification.
Publish phase:
- Fix: a branch that was pushed is now still reported when pull request
creation fails afterwards. `PublishOutcome` records what happened and
carries the error separately, instead of hiding both behind a `Result`.
- Drop `PublishOutcome::NoChanges`, which no consumer distinguished from
`Published { pr_url: None }`.
- Move publish onto `Concluded` as methods and replace three near-identical
precondition guards with one `publish_target()`.
Pull requests:
- `maybe_open_pull_request` -> `open_pull_request` returning the record
directly. Both callers already reject empty diffs, so the `Ok(None)` path
was unreachable.
- Drop `CreatedPullRequest.head_sha`, which echoed back its own input.
GitHub client:
- Delete `branch_exists`, which had no callers and duplicated
`branch_head_sha`. Give `branch_head_sha` the `_with_client` split every
sibling has and port the tests to `MockHttpClient`.
- Collapse the copy-pasted credential match in `resolve_clone_credentials`.
Events:
- `PullRequestCreated.head_sha` is `Option<String>` instead of using an empty
string to mean absent.
- Centralize the run-branch refspec in `lifecycle::push_run_branch`, so
`git.push` reports a branch name from both emitters as documented.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
