Repository navigation
Commit ce37a2a
feat(api): compare-failure logging + GitHub check runs for Deploy-on-GitHub builds (#195)
* docs(api): plan 0075 — compare-failure logging + GitHub check-runs
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(api): log branch-head compare failures instead of silent null
getBranchHeadComparisonStatus collapsed five failure causes (token-mint
throw, network, 4xx, 5xx, exhausted) into a silent string|null. Add
area:'github' console.warn JSON logging at each point and wrap the token
mint in try/catch so the documented never-throws contract holds. Return
contract, retry count, delay, and 4xx-vs-5xx semantics are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(api): format plan 0075 doc (oxfmt)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(api): add builds.check_run_id column + migration 0028
Additive nullable text column to persist the GitHub Check Run id for a
build (Feature B). NULL = no check-run (Actions builds, pre-create-recheck
failures, pre-column rows, or a best-effort create failure); finalizers
no-op on NULL. No index (no query filters by it). Backward compatible:
old code never references it, new code tolerates NULL on existing rows.
Migration 0028_check_runs stamped after 0027 (when 1782220390482).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(api): add GitHub check-run helper (create + complete)
New src/github/check-run.ts with best-effort createCheckRun (POST,
in_progress) and completeCheckRun (PATCH, terminal conclusion). Both never
throw: retry transient 5xx/network/token-mint, return immediately on 4xx,
and log every failure via the shared area:'github' warn shape. Uses the
App's existing Checks:Write; executor-agnostic (plain args). Exports
GITHUB_API_BASE/GITHUB_API_VERSION from installation-token.ts so the API
version string has a single source of truth.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(api): add setBuildCheckRunId db helper
Persists the GitHub Check Run id on the build row (Feature B). Unguarded
by status — the build's own Durable Object is the only writer of its
check_run_id, so there is no concurrent writer to CAS against.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(api): refine plan 0075 Task 5 — close two check-run gaps
Found by reading the actual build-manager control flow:
1. startBuild's own catch (non-capacity) must finalize the check-run as
'failure' — the check-run is created before signDeployToken/start, and a
throw there marks the build failed with no onStop (force-stop → terminal
guard no-op), stranding the check-run in_progress.
2. Create must be idempotent across a capacity-requeue (same DO re-runs
startBuild) via DO storage, else a duplicate 'Void Build' check-run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(api): wire GitHub check-run into the build lifecycle (container)
Create a 'Void Build' check-run once all pre-start rechecks pass (SHA-
guarded; idempotent across a capacity-requeue via DO storage), persist the
id to DO storage (input-gated) + Postgres (best-effort), and finalize it at
every terminal site: onStop success/failure, onError, enforceTimeout,
cancelBuild (cancelled), and startBuild's own non-capacity catch. The
last is required because every force-stop path sets the local terminal
status before forceStopContainer(), so the onStop it triggers is a
TERMINAL_STATUSES no-op — finalize must live at each site. All best-effort:
a check-run API failure never fails or perturbs the build/deploy, and the
finally{pokeScheduler} always runs. Container executor only (Actions
deferred). 20 integration tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(api): document the 'Void Build' check-run + Checks:Write usage
README Deploy-on-GitHub section gains a Check Run bullet (container-only,
best-effort, details_url convention, Actions deferred, no check_run_id
index). operations.md reconciles the existing Checks note to state
Checks:Write is now exercised by the published check-run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): harden check-run lifecycle against concurrency (codex findings)
Three verified concurrency hardenings for the GitHub check-run wiring:
1. Best-effort DO-storage cache — wrap the create's `ctx.storage.put` in
try/catch so a cache-write rejection can't enter the build-start catch
and fail the build.
2. CAS-aware finalize — every terminal site (onStop x2, onError,
enforceTimeout, cancelBuild, startBuild non-capacity catch) now captures
the `markBuildTerminalIfActive` boolean and, on a LOST CAS (row
terminalized out-of-band), finalizes the check-run to the AUTHORITATIVE
recorded DB status via the new `finalizeCheckRunToStatus` helper — so
GitHub never contradicts the DB.
3. Post-create cancellation recheck — a cancel landing during the
createCheckRun POST is caught by re-reading the row after the POST; if no
longer `starting`, finalize the just-created run to the recorded outcome
and return WITHOUT starting a container (onStop cannot self-heal this —
its terminal guard no-ops).
+4 integration scenarios (CAS-lost cancelled/failed, cancel-during-create,
best-effort put rejection). Full api suite 1518/1518. Plan 0075 residual
notes corrected (the original "onStop self-heals" claim was false).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): finalize check-run from persisted column on DO-storage miss
Closes the two narrow residuals from the round-2 adversarial review:
- F1: finalizeCheckRun now falls back to builds.check_run_id (best-effort,
caught) when DO storage has no id — so a DO-storage miss (eviction, a
rejected cache put, transient loss) can no longer strand an
already-created run in_progress. Gives the persisted column its purpose
(it was written but never read).
- F3: the 6 lost-CAS finalize sites now route through
finalizeCheckRunToRecordedOutcome, which wraps the authoritative getBuild
best-effort so a transient read can't skip the PATCH or propagate out of
the terminal handler.
+2 integration scenarios (DO-miss recovery PATCHes; lost-CAS getBuild throw
swallowed, slot still freed). full api suite 1520/1520.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(api): record check-run finalize residuals in plan 0075
DO-storage-miss fallback + lost-CAS getBuild wrap (CLOSED in fix wave);
cancel-before-this.start() window documented as pre-existing, deploy-token
gated at 3 layers, bounded by enforceTimeout — not re-fixed (owner decision).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): prefer in-hand check-run id; reuse persisted id on requeue
Closes two Cursor Bugbot findings — both cosmetic and near-unreachable
(gated on a DO-storage put rejection) but real symmetry gaps in the F1 fix:
- #1 requeue duplicates: the create reuse-gate now falls back to the
builds.check_run_id already carried on the pre-create-recheck row, so a
requeue whose best-effort DO cache write was lost REUSES the run instead
of POSTing a second one (which orphaned the first in_progress). Zero new
reads — the row was already fetched.
- #2 post-create cancel miss: finalizeCheckRun takes an optional explicit
id; the post-create cancel recheck passes the in-hand local id, so the
just-created run is finalized even when BOTH the DO put and the PG persist
failed (id lived only in the local variable).
+2 integration scenarios. full api suite 1522/1522.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): in-memory check-run id fallback for dual-persist failure
Closes Cursor Bugbot #3 (cosmetic, near-impossible — both the DO put AND the
Postgres persist must fail in one startBuild; pre-existing). When the build
CONTINUES (no cancel) after a dual-persist failure, the just-created id lived
only in startBuild's local variable, so the eventual terminal finalizer — a
SEPARATE invocation with no access to that var — could not find it and the run
stayed in_progress. (The earlier explicitId fix only covered the
cancel-during-create branch.)
Add an in-memory `activeCheckRunId` field, set right after createCheckRun,
consulted by finalizeCheckRun (behind explicitId) and the create reuse-gate. It
survives create→terminal within the build's lifetime (the DO is actively running
the container) and is strictly additive — lost on eviction, behavior is
identical to today. This is the root-cause fix for the whole dual-persist class;
explicitId + builds.check_run_id remain the in-window and durable layers.
+1 integration scenario. full api suite 1523/1523.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): finalize prior-attempt check run on pre-start exits; single-shot create
Two GitHub-Codex review findings (both P2, cosmetic, NEW, reachable via normal
operational paths — not the prior near-impossible DO-storage rejection):
- A: the pre-start terminalizing returns in startBuild (freshness aged-out,
non-identical head, !covered, and the cancel recheck) did NOT finalize a
check-run created on a PRIOR attempt. A capacity requeue re-runs startBuild and
builds.check_run_id survives the requeue, so an early terminal exit left the run
stuck in_progress. Each terminal pre-start return now calls
finalizeCheckRunToStatus(config, <status>) before returning — it no-ops on a
first attempt (no id anywhere) and finalizes on a requeue (id recovered from
builds.check_run_id).
- B: createCheckRun retried the create POST on 5xx/network, but POST /check-runs
is not idempotent (external_id is a reference field, not an idempotency key), so
an ambiguous retry (run created server-side, response lost) could duplicate the
run and orphan the first in_progress. The create is now single-shot: only the
pre-POST token mint is retried; any non-success POST returns null.
+4 integration/unit scenarios (incl. first-attempt no-op + no-retry on 5xx/network);
2 stale tests rewritten to the single-shot contract. full api suite 1527/1527.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(api): CAS-aware finalize on the 3 pre-start terminalize paths
The freshness aged-out, non-identical-head, and uncovered-repo early exits
in BuildManager.startBuild called markBuildTerminalIfActive then finalized a
prior-attempt check-run to a HARDCODED conclusion, discarding the CAS result.
Those paths await getBranchHeadComparisonStatus / installationCoversRepo —
non-storage awaits that release the DO input gate — so a concurrent
terminalizer (terminateBuildsForProject commits an out-of-band terminal
status, then the cancelBuild RPC) can win the CAS with a DIFFERENT status. On
a lost CAS GitHub got the assumed conclusion while Postgres held the real one.
Capture the won boolean and, on a loss, finalize via the recorded outcome —
exactly mirroring the catch site and the 6 terminal sites. setStatus/broadcast
stay hardcoded, matching the accepted catch-site precedent (DO-local + transient
only; the durable, externally-visible check-run is what must match the DB).
Adds 5 integration scenarios (lost-CAS + won for all three paths).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>1 parent c42c30f commit ce37a2a
16 files changed
Lines changed: 5422 additions & 14 deletions
File tree
- meta
- design-docs
- packages/api
- drizzle
- meta
- src
- github
- test
- integration
- unit
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
313 | 313 | | |
314 | 314 | | |
315 | 315 | | |
316 | | - | |
| 316 | + | |
| 317 | + | |
317 | 318 | | |
318 | 319 | | |
319 | 320 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
162 | 162 | | |
163 | 163 | | |
164 | 164 | | |
| 165 | + | |
165 | 166 | | |
166 | 167 | | |
167 | 168 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
0 commit comments