GT-443: put the breaker where the runtime actually calls out, and run the load/chaos harness for the first time - #310
Merged
Conversation
release: align main — the CLI and MCP resolve the SDK that carries the security wave, 1.2.2 (GT-634)
A board row carries a STATUS but not who is working it or where, so two sessions can both read `PENDING` and both start, correctly. On 2026-07-30 that happened three times in a day. `50-validate-gap-claim` derives the claim set from OPEN PULL REQUESTS — every `GT-*` in a PR's title, body or branch name — and fails when one id is claimed by two of them, naming both with their branches and titles. DERIVED, NEVER HAND-WRITTEN, and the row asked for that on purpose: a hand-written claim goes stale in the direction that matters, because someone forgets to remove it and the next session works around a claim nobody holds. THREE CRITERIA OF FOUR, and the fourth is left open with its reason rather than stretched to fit. A claim list committed to the board would be derived from live GitHub state, so it would be stale the moment anyone opened a pull request — and GT-630's chain exists precisely to insist derived artifacts reach a fixed point. A perpetually-stale artifact inside that chain would be worse than none. Making the claim discoverable from the board needs something the board can render without committing, and that is not built. The row stays IN-PROGRESS. What it cannot see is in its own failure text, not implied: a branch with no open pull request claims nothing. Opening the PR early — draft is enough — is what makes the claim visible, which turns a convention into something with a check behind it. Anti-vacuous: a pull-request query that cannot be answered is a hard failure, not a quiet pass. A guard built because two sessions could not see each other must not stay silent when it cannot look. Zero open PRs is reported in words, because it looks identical to a broken query otherwise. Verified: 10/10 fixtures; 42-validate-guard-denominators 62 guards classified; 43-validate-guard-negative-fixtures 39/39 observed red, up from 38. Run against the real repository it reports 0 claims across 5 open PRs — checked rather than assumed: all five are Dependabot bumps naming no gap, so the zero is real and not an empty query. 08-validate-tracking (640 gaps), 01, 04, 41 within budget, 46 chain at a fixed point. Counters: 600/640, in-progress 14 -> 15, pending 22 -> 21. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(ci): one gap, one claim — the claim guard, advancing GT-639
`.git/EVOLITH_*` is a DIRECTORY only in the primary checkout; inside a `git worktree` it is a FILE holding a `gitdir:` pointer, so every write/read to `.git/EVOLITH_*` was ENOTDIR. In pre-commit that made `set -e` abort the commit outright; in pre-push the `rm -f` failed and TARGET stayed unread — together the reason `--no-verify` had become habitual from a worktree. `git rev-parse --git-dir` resolves both layouts; both hooks now go through it via a shared `write_push_intent`/read path, per-worktree, so the push intent belongs to the checkout that was committed in rather than the repo as a whole. Also in pre-push: `assert_checkoutable` refuses up front, before repo optimization or board sync run, when the merge target branch (main/develop) is already checked out in another worktree — `git checkout "$to"` would otherwise fail mid-dispatch with the push half-done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(ci): pre-commit/pre-push hooks assumed .git is always a directory
… where the runtime actually calls out (GT-443)
The k6 + chaos assets under product/infra/load/ ran in NO workflow
(`grep -rniE "k6|chaos" .github/workflows/` returned nothing), so their SLOs
gated nothing. This wires them up and executes them, which is how three
never-observed defects in the harness itself surfaced.
Circuit breaker — criterion 1, with the GT-560 tension resolved rather than
ignored. GT-560 was RIGHT about core-api: it is a stateless evaluation engine
(ADR-0101) that makes no outbound calls, and re-adding a breaker there would
recreate the orphan it deleted. But GT-560 scoped itself to core-api and said so
("move the breaker to where the real calls are"). The real calls are one layer
up: HttpCoreEvaluationAdapter.evaluate() (agent-runtime) had no timeout, no retry
and no breaker, and the production profile REFUSES TO BOOT without it
(runtime.factory.ts throws without AGENT_RUNTIME_CORE_ENDPOINT) — so a hung Core
stalls every governed request behind undici's 300 s default. Same for the Tracker
publish. Added a dependency-free CircuitBreaker (no opossum — GT-560 removed it)
that hands the call an AbortSignal AND races a deadline, so an uncooperative
callee cannot hang the runtime either. Wired ON BY DEFAULT in the factory, with a
spec asserting the wiring, because a breaker nobody injects protects nothing.
Deviation recorded in the source: ADR-0011 §1 mandates Redis-shared breaker
state; this is process-local, since sharing it would reintroduce the Redis
dependency GT-560 removed.
CI — criterion 2. New .github/workflows/reliability.yml (deliberately a new file:
ci-cd.yml and docker-images.yml are open in a PR). Boots core-api, runs the k6
smoke gate then the average-load profile, and publishes throughput, p95/p99 and
error rate against the declared thresholds into the step summary via
report-summary.mjs; a threshold breach fails the job.
Chaos — criterion 3. New chaos/kill-under-load.sh kills a dependency MID-RUN with
real callers on it and records what those callers saw; the drill plus the
total-outage MTTR run off the PR path.
What executing the harness found, all fixed here:
- smoke asserted `data.gates`, a field the evaluate response has never had (100 %
failure on first run, unnoticed because nothing ran it);
- the chaos scripts claimed `restart: unless-stopped` recovers their SIGKILL. It
does not: measured on Docker 29.4.3, an EXTERNAL kill leaves the container
Exited(137) with RestartCount=0, while a container whose own PID 1 dies is
restarted (RestartCount=2). The first drill sat in a 180 s outage and never
recovered. Scripts now restart the service the way a kubelet would;
- a 10-VU run returned 429 for 81 % of evaluations: core-api throttles at 100
req/60 s, so the profile was measuring the rate limiter. 429s now have their own
`throttled_429` threshold so a misconfigured target fails by name.
First numbers on record (laptop, single container, other workloads running —
re-baseline on the runner): smoke 50/50 checks, evaluate p95 719 ms; average load
10 VUs → 10.15 req/s, evaluate p95 2 545 ms / p99 6 524 ms with 0 % errors, i.e. a
latency SLO breach from saturation, not failure; SIGKILL under load → first
verdict back in 5 770 ms with 6.9 % of caller requests lost; total-outage MTTR
10 318 ms mean over 3/3 runs.
Criterion 4 (RTO/RPO in ADR-0013) is NOT met and ADR-0013 is deliberately
untouched: MTTR from a container restart on one host is not an RTO, and writing
it there would replace one unquantified claim with a mislabelled one. It needs a
real DR restore.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📊 Bilingual Coverage ImpactPR Changes
Repository Coverage
To create skeletons: node .harness/scripts/generate-es-skeleton.mjs <file.md>Generated by GitHub Actions |
This was referenced Jul 31, 2026
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
1 participant
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.
Advances GT-443. Criteria 1–3 met, criterion 4 blocked — the row stays open, deliberately.
Criterion 1 was in tension with GT-560, and the tension resolved as (a)
GT-560 was right about core-api, and no breaker was re-added there. Re-confirmed: core-api is a stateless engine (ADR-0101),
grep -rl "CircuitBreaker" srcis empty, its only outboundfetchcalls are two Dapr secret reads that are already cached, try/caught and env-fallback'd, and the OPA-sidecar HTTP branch is dead in deployment (values.yaml:3says "NO OPA sidecar";OPA_URLis set nowhere).But GT-560 scoped itself to core-api, and its own text names the remaining option — "move the breaker to infra-providers, where the real calls are." The real call is one layer up, and it is mandatory by design:
agent-runtime/src/adapters/core/http-core-evaluation.adapter.ts:47-56—evaluate()was a barefetchwith no timeout, no retry, no breaker (confirmed independently against the pre-change file).agent-runtime-api/src/agent-runtime/runtime.factory.ts:249-270— the production profile throws ifAGENT_RUNTIME_CORE_ENDPOINTis missing. A hung Core therefore stalls every governed request behind undici's 300 s default.adapters/tracker/http-tracker-trace.adapter.ts:36-45, whose header claims "best-effort" — untrue without a bound.http-slack-client.ts:70andGeminiProvider.tsalready use AbortController timeouts. These two adapters were the outliers.So the breaker lives where the call lives, is dependency-free (no opossum — GT-560 deleted it), forwards an
AbortSignaland races a deadline so an uncooperative callee cannot hang the runtime, and is on by default with a spec asserting the wiring. ADR-0011 §1's Redis-shared state is deliberately not implemented — that would reintroduce the dependency GT-560 removed — and the deviation is written in the source header rather than hidden.Proven by deletion: unwiring
this.options.breaker→ 4 tests fail, the hang case timing out at 20 s, which is exactly the unbounded behaviour; unwiring the factory → the wiring test fails. 5 integration tests drive open/half-open/closed against a realnode:httpdependency. Independently re-run by the integrating session: 5/5, and the full agent-runtime suite 411/411.Criterion 2: running the harness for the first time found three defects in the harness
New
.github/workflows/reliability.yml— a new file, so the two workflows contested by open PRs stay untouched.What executing it exposed: smoke asserted
data.gates, a field the API has never returned — 100 % failure, invisible because nothing ever ran it; a 10-VU run took 429 on 81 % of evaluations, i.e. the profile was measuring core-api's own 100-req/60 s throttler, now tracked under a separatethrottled_429threshold; anddocker compose upsilently reused a week-old image whose response contract had since changed.Criterion 3: met locally, and the recorded behaviour does NOT match ADR-0011
chaos/kill-under-load.shSIGKILLs core-api under 5 VUs: outage visible in 339 ms,/healthback in 5 538 ms, first governed verdict in 5 770 ms, callers lost 49/706 (6.9 %). Total-outage MTTR 3/3 recovered, mean 10 318 ms.The divergence is the finding: no gateway health-checking, single replica, no retry — and the scripts' claim that
restart: unless-stoppedself-heals is false. Measured on Docker 29.4.3: an externaldocker killleaves the containerExited(137)withRestartCount=0, while a container whose own PID 1 dies restarts. The first drill sat in a 180 s outage and never recovered. The scripts now restart the way a kubelet would, withRESTART_MODE=nonereproducing raw compose.Criterion 4: NOT met, and ADR-0013 is deliberately untouched
MTTR from a container restart on one host is not an RTO, and RPO requires a backup/restore data-loss window no local stack can produce. Writing 10 318 ms into ADR-0013 would swap one unquantified claim for a mislabelled one. This needs a real DR restore on real infrastructure — GT-448.
Caveats
The worktree's
node_modulesoriginally resolved into the main checkout; it was rebuilt worktree-local (verified withreadlink -fby the integrating session),dist/rebuilt, and everything re-run — only post-fix numbers appear above. Container figures come from an image rebuilt--no-cachefrom this branch. All absolutes were measured on a loaded laptop, not a runner; the workflow publishes its own per run.🤖 Generated with Claude Code