fix: the holes the first validated run found - #125
Merged
Conversation
Four defects, all of which made the harness quieter than it should have been about work it was losing. **The lease is now held for the whole attempt (#123).** It was renewed only between stages, which made it a bound on the longest stage rather than on how long a worker may be absent — and the two stages that matter, an agent thinking and a full check suite, are both routinely longer than it. A 915s agent run against a 900s lease was enough: the item was retired underneath a worker that was still going, and every later write was owner-scoped to an owner the row no longer had, so the work, the reason it ended and its worktree were all lost silently. A daemon heartbeat now covers the attempt, and a release that finds itself owner-less says so instead of returning quietly. **An item's `latest` event comes from the audit store (#122).** Nothing ingests `events.jsonl` into the `EventStore` under `serve`, so the projection that reads it reported `latest: null` for every item forever. Grouped in the store rather than scanned, because a scan drops exactly the long-quiet item whose status someone is trying to read. **Reconcile can recover a pull request whose URL was dropped (#124).** It was driven entirely by the URLs the queue had recorded, so the one kind of pull request most in need of being found was the one it could never see. The head branch is the link that was always there. **Base checks no longer block the request (#121).** Running a whole build inside the HTTP request repeated the mistake `stop` had just been fixed for: the request outlived every proxy timeout while the build carried on with nowhere to report, and retrying started a second one. A run is now started, joined if one is in flight, and read back; `check_base=true` reports the last run and never starts one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #121. Closes #122. Closes #123. Closes #124.
All four were found by running the previous uplift on Node B against NGMS and
watching what it actually did. Each one made the harness quieter than it
should have been about work it was losing.
#123 — the lease is held for the whole attempt
The lease was renewed only between stages, which made it a bound on the
longest stage rather than on how long a worker may be absent. The two stages
that matter — an agent thinking, and a full check suite — are both routinely
longer than it.
One 915s agent run against a 900s lease was enough. The lease lapsed 15s
before the agent returned, a second worker's claim scan retired the item
underneath it, and because every later write is owner-scoped to an owner the
row no longer had, the 15 minutes of work, the reason the item ended and its
13G worktree all disappeared without an event.
LeaseHeartbeatnow beats on a daemon thread for the duration of an attempt,in both executors. Daemon on purpose: a worker that dies must take its
heartbeat with it, because that is what makes the lease expire and the item
recoverable.
release()finding itself owner-less now logs the discardedresult rather than returning silently.
The regression test is end-to-end and fails without the fix — an agent slower
than the lease, and a competing claim while it runs.
#122 —
latestcomes from the store the fleet writes toNothing calls
ingest()underserve, so theEventStorethe itemprojections read is empty in every supervised deployment and
latestwasnullforever. It now prefers the audit store, grouped in SQL rather thanscanned: a "last N events" scan drops precisely the long-quiet item whose
status someone is trying to read.
#124 — a dropped
pr_urlis recoverableReconciliation was driven by the URLs the queue had already recorded, so a
pull request whose URL was lost was invisible to the one thing meant to find
it. Matching on the head branch recovers it. Bounded by construction: only a
branch an item itself recorded can match, so a human's PR cannot be adopted.
#121 — base checks do not block the request
Running the suite inside the request repeated the mistake
stophad just beenfixed for. It 502'd at the proxy's 15s timeout while the build carried on with
nowhere to report, and the obvious response — retry — started a second
concurrent build. Two 502s left 26G building invisibly.
POST /api/projects/{id}/preflight/basestarts a run and returns; a secondcall joins the one in flight;
GETpolls it.check_base=truenow reportsthe latest run and never starts one, so a readiness read stays a read.
Gates
548 tests, ruff,
mypy .— all green.