ORB-163 PR3b: repair the tools the gates run (#668) - #668
Conversation
Section F says "use 5.50 for budgeting and freeze the definition in a committed tool", and section I requires its numbers generated by that tool rather than by hand. One round is one DISTINCT HEAD COMMIT that received a review submission from either review bot. The tool prints the four alternative definitions beside the frozen one, so two can never be merged into one figure again: that is how a mean of 4.60 and a worst of 13 came from different sets and understated rounds by 16 percent. The trap, confirmed against the live API before anything was encoded: the CI reviewer's login is `claude` in GraphQL, not `claude[bot]`. `[bot]` is the REST spelling and section F states its definition in REST terms, so a counter filtering on `claude[bot]` reports 0 rounds on a pull request carrying 15 bot submissions, and a plausible zero is indistinguishable from a real one. `claude[bot]` appears in zero of the 289 review nodes captured live and committed verbatim as the fixture. A bot is therefore resolved on GraphQL `__typename`, never on a login string, and a Bot login outside the known set is refused rather than counted. Reproduced exactly at the recorded cutoff over pull requests 641 to 661, which is 20 pull requests because #657 does not exist and both endpoints 404: 5.50 / 4.60 / 3.10 / 6.20 / 2.15 means, medians 5 / 4 / 2 / 6 / 2, worst 13 / 12 / 10 / 14 / 6. `--as-of` is inclusive, so the window that reproduces those figures is [2026-07-30T16:11:16Z, 2026-07-30T19:48:33Z), the same two instants recorded earlier with the brackets flipped. At an inclusive 19:48:33Z the frozen mean is 5.55, because a `claude` COMMENTED submission on #658 head 70aca7f is stamped exactly then. Both figures are right about different instants, which is why the tool prints the instant beside the numbers. A truncated review page is refused rather than paginated, and so are a failed, errored or unparseable lookup and a review missing its author, submittedAt or commit. Every response shape on the tool's path came from a real `gh api graphql` call or from GraphQL introspection. Withdrawn and encoded nowhere: Pearson 0.738, whose real value under the frozen definition is 0.496 with a matrix peak of 0.629, and "under 400 lines averaged 15.0 reviews, over 400 averaged 24.8", which has zero hits across 4 subset filters, 2 size measures, thresholds 100 to 800, 10 count quantities and both mean and median. The largest per-pull-request round count in the window is 14, so those cannot be rounds under any definition. 18 hermetic cases, every one proven red before green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
…ove the slices C1b. The budget fuse blocked on a number nobody derived. Measured against a copy of the production ledger: the codex seven-day window reads 1,188,429 tokens against a configured tokenBudget of 1,000,000, so the fuse refused EVERY launch on every tier, including one projecting zero additional tokens, while the provider's own authoritative reading over the same period is usedPercent 11 of 100 and did not move across the 2h22m session that produced most of that total. So the BLOCK keys on the provider's own weekly usage (claude weeklyPercent, codex usedPercent, through ai-quota.mjs) against automationBudget.accountUsedPercentCeiling, 85, with its derivation written beside the constant. The token budget is demoted to warning-only and the config says it is not derived from anything. Reserve, claim, cancel and record accounting is byte-for-byte unchanged: it is the audit trail and it is C4's proof of concurrency. The fallback is the common path, not the edge case. The claude arm reads its figure by scraping an Orca accessibility tree and flipped OK to UNAVAILABLE and back three times during this session on an idle machine. An unavailable reading falls back to the token budget, says so on stderr and in the emitted JSON as gate TOKEN_FALLBACK, and is never read as zero usage. The launcher's stricter exit 3 on an unreadable reading is relaxed to match, so the bounded fallback can actually fire, with both halves pinned by cases. ai-quota.mjs exits 1 while printing a complete valid object on stdout when both providers are unreachable, confirmed by running it that way. The budget tool reads stdout and ignores the exit code, deliberately, with that stated in the source. D3. .claude/orchestrator.json is resolved from origin/<base> rather than a working tree nothing updates. Three branches: no git repository uses the working copy; a HEAD that contains origin/<base> keeps its own newer copy, because refusing an ordinary pull request that edits the config would turn every tool red and that is how a gate gets switched off; a HEAD behind origin/<base> whose copies disagree refuses, naming every differing leaf and the remediation. An unresolvable ref fails closed after one explicit-refspec fetch and never falls back. B3. The strike count is durable. Clause 4's "two consecutive cycles fail on the same finding" was a string in the contract template whose only assertion matched the clause TEXT with a regex, and every relaunch is a fresh process, so the count reset and escalation degraded to unbounded retry. tools/lib/strike-ledger.mjs is an append-only store keyed by (scope, issue, key), isolated exactly as the budget ledger is, deliberately NOT the attempts:N Linear label, which counts a different thing. launch-worker --finding <id> exits 5 when the strikes are spent, meaning escalate, never retry. F2. Worker contract clause 12: post the approach as a pull request comment before writing code, with a behavioural assertion rather than a regex over the source, which is the A4 defect shape. Mirrored into AGENTS.md for the Codex cloud reviewer. C3. The launcher gained --wave-all, --wave-label and --wave-project, each consuming wave-plan.mjs --json and launching every ticket whose affected-file sets are pairwise disjoint while serialising the rest. It re-invokes itself once per ticket, so the budget fuse, the concurrency lock and maxParallelWorktrees are enforced in exactly one place. --wave-issues is refused by name: in that mode visibleWaves filters each wave BEFORE collisionsIn runs, so a collision with a ticket the operator did not name is invisible, and silence must not buy parallelism. Live on the real board, --wave-all --dry-run exits 0 with cap 4, 55 launchable, four launched and 51 serialised, 13 of them collision-deferred with their shared paths named. The affected-files parser is extracted to tools/lib/affected-files.mjs and imported by both wave-plan.mjs and check-ticket.mjs, so a wave's collision set and a ticket's validation can no longer disagree about what a ticket declares. wave-plan.mjs also stopped hardcoding the team key while the configuration declares linear.team, which is the stale-source root. C4. tools/check-slice-evidence.mjs proves a slice run was concurrent structurally, never by wall clock, because a hermetic test cannot invoke the model and a timing assertion flakes. Two or more rollouts for the ticket with overlapping intervals, plus one reserve and one record ledger row per slice, each a distinct reservation. It keys on the resolved engine and reads CODEX_HOME from the environment, confirmed unset in an ordinary shell and redirected by Orca, so it refuses rather than assuming ~/.codex. The four fixtures are shaped from two real rollouts read off this machine: on a subagent rollout payload.session_id carries the PARENT's id and payload.id the child's own, which an invented fixture would have had backwards. Stated as not confirmed: close_agent is a documented codex 0.146.0 tool that appears in zero of the 275 real rollouts on this machine, so its detection is implemented by symmetry with the observed spawn_agent shape and exercised against a planted event. The F-5 case that was owed: a stranded reservation found in the PRODUCTION ledger was never a worker, matching the harness's own fixture field for field. Two assertions now prove the isolation stays, one reading the ledger path a child actually receives and one bracketing the module with a size and mtime read of the real ledger. DEFAULT_AUTOMATION_BUDGET carries the account ceiling, so the two temporary bridges the slices needed are gone and runTrustScreen is a single shared helper again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
…uses
D1. The ledger-child gate had never executed. check-ticket.mjs read issue.parent, but
`orca linear issue <id> --full --json` returns 17 keys with no parent and relations: [], and
orca's relation vocabulary is {blocks, blocked-by, related, duplicate-of}, so parent is not
expressible as a relation and childOf appears in zero files of the installed bundle. The gate
was short-circuited rather than starved: a `parent === null ||` disjunct whitelisted the empty
case so readLinearParent, which works, never ran. On the same real ticket,
`--issue ORB-150` went from exit 0 "ticket ok" to exit 1 naming the missing fixed line.
The whole orca-derived parent path is deleted, not just the disjunct: with the dead
`relationship === "parent"` branch gone, parentFromOrca could return nothing, so it and its
three cases went too. Keeping a dead branch alive with harness cases is the harness proving a
belief rather than a behaviour, which is the invented-interface root.
The existing assertion that asserted the fail-open as CORRECT is inverted, with a companion
case proving a genuinely parentless issue still validates once the lookup answers, so the
inversion does not turn "no parent" into "always refuse".
D2. The dependency-prose guard fires wrong on main today. Its trailing /i made the identifier
class case-insensitive, so any lowercase hyphen-plus-digit token within 80 characters of a
signal word tripped it. Extracted verbatim and driven against strings this ticket itself
produced: "after feature/orb-163-c2-split-test-file", "once orb-164-c2-rubric-twin exists",
"depends on expo-sdk-54" and "Blocked by node-24" all tripped and now read clean, while
"after ORB-163" and "Blocked by ORB-164" still trip. The identifier alternation is built from
the team key the configuration declares rather than a hardcoded [A-Z][A-Z0-9]+, and a
non-alphanumeric team key exits 2 rather than being interpolated into a regex.
D5a. The filing gate is OPEN but REGISTERED. tools/harness-roots.json carries the five roots
fixed by the vault ADR, each with an id, a definition and the owner ticket. An unregistered id
is refused; a registered id whose owning ticket is still open is refused naming that ticket;
exempt claims nothing. Scoped by the harness Linear label. check-ticket.mjs acquires no write
capability: it exits 1 naming the already-filed ticket and stops, and the append stays with
/ticket and /feature.
A harness ticket with NO Root cause line is REFUSED, not warned about. The specification put
the obligation on the line rather than on its presence, and that left a hole: ORB-164 carried
the harness label and no such line when this work started, so the gate would have passed its
own paired ticket. The point is to force the classification, not to check the spelling of one
somebody volunteered. --file mode stays fail-open by necessity, because the label is genuinely
unreadable there, and --help says exactly that.
C3's other half: at least one PARSEABLE path is required under the affected-files heading, not
merely the heading, through the shared lib/affected-files.mjs, because a ticket with no path
list collides with everything and silence must not buy parallelism.
Fixed while in the file: the visible-effect word list used \b, which treats a hyphen as a
boundary, so a body reading "Root cause: string-not-act" made a pure tooling ticket demand
screenshots and a critique artifact.
Every gate proven red before green: restoring the disjunct turns 16 cases red, restoring the
/i turns exactly the four lowercase slug cases red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
…sing at liveness Section E. worker-status.mjs already derived delivery from artifacts and it earned its keep on #667: at 06:15Z that pull request read mergeStateStatus CLEAN, reviewDecision APPROVED, an approval on the head and zero threads, and a hand-built commit-anchored guard said OK, while worker-status.mjs returned exit 1 with unmet ["worktree-clean", ...] because the worktree carried two uncommitted files. A green pull request is not a delivered one. What it could not do was distinguish a worker still working from one that died. Liveness now comes from the real process table, using the launcher-written PID marker, ESRCH gone and EPERM alive and not ours. PID reuse fails CLOSED: a pid that answers alive but whose claim is older than the 16 hour backstop reads unknown, never alive, and so does a missing, unreadable or non-JSON marker, an unparseable or future claim time, and any other errno. Never a heartbeat: a worker that dies cannot write that it died. Six verdicts, DELIVERED, WORKING, STALLED, AWAITING-MERGE, IDLE and UNKNOWN. STALLED keys on the process being gone AND an open unapproved pull request, never on the Linear state, because a ticket shipping four sequential pull requests sits honestly In Progress while the tool assumed one pull request per ticket. No open pull request plus no live worker is IDLE, not stalled: one needs a relaunch and the other needs nothing, and conflating them costs a run in both directions. AWAITING-MERGE had to be named too, or #667 between its approval and its merge would have earned a false relaunch. The relaunch allowance keys on the HEAD SHA, so a push earns a fresh one and an unchanged head does not. It is spent through --consume-relaunch, exit 4 when refused, and it carries relaunch.findings so a relaunch ships the outstanding findings rather than the ticket body alone. It lives in the same lib/strike-ledger.mjs the clause 4 strikes use, under a distinct scope, so there is one store rather than two. worker-status.mjs also had NO unknown-flag rejection at all; its coverage entry passed only because --orbit-not-a-flag happened to fall through to the "--worktree is required" path. worker-watch.mjs no longer derives liveness itself. Its own alive() treated every non-ESRCH errno as alive with no reuse backstop and no UNKNOWN state, so it printed BUSY for a recycled pid: a tool reporting a state it never established, which is the defect this ticket exists to remove. It now renders worker-status.mjs's liveness and verdict verbatim, and a run that printed no JSON reads "verdict unavailable" rather than an invented state. Cost measured rather than guessed: 4.9s for one worktree and 9.2s for two, which an operator-invoked snapshot accepts, documented in the tool. A second defect found there: --repo filtered on a path prefix, and Orca does not put a child worktree under its repository path, so --repo ui printed an EMPTY FLEET over a live worker. It now resolves orca's repoId and refuses with exit 2 when a configured path is not an orca main worktree, rather than reporting an empty fleet it never checked. The old fixtures parked their worktrees inside the repo path, agreeing with the bug. The recycled-pid case is pinned at an absolute 17 hour claim age, never at BACKSTOP plus one, and was proven to go red against a 48 hour copy of the tool. A fixture that ages relative to the constant it measures can never fail when that constant moves, which already happened once on this ticket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
…old, wire the verdict D6. Harness Calibration was RED on main and GREEN in CI. Run bare it exited 1 with "calibration FAIL: 35/36 entries, missing entry: .claude/skills/quota/SKILL.md", while CI reported green because the guards.yml step runs --report-only, which exits 0 on drift by design. That is root cause 1 sitting inside the gate set PR3a just made blockable: a gate reporting green over a condition it never enforced. Three real defects. The quota skill arrived in 495f037 and was never stamped; its entry is written from actually reading the skill. The recorded invocation carried no exec, so the stamp no longer described how workers are invoked after PR1's A3a, and it is now re-read from .claude/orchestrator.json rather than typed by hand. Two content fingerprints were left stale by edits to orchestrate/SKILL.md and watch/SKILL.md; both verdicts genuinely stayed kept and both reasons stayed accurate, since neither file's effort frontmatter moved, only its body. The tool now exits 0 with "calibration PASS: 36/36 files". The new case that matters: a stamp whose invocation disagrees with .claude/orchestrator.json fails NAMING BOTH ARRAYS. The pre-existing case asserted only /invocation mismatch/, so it stayed green when the message was gutted to that bare string, which is exactly why defect 2 could ship the day PR1 landed. --report-only deliberately STAYS in guards.yml. Removing it is what makes the gate blocking, and that belongs with the branch-protection edit at the very end of the ticket, so a red calibration cannot brick main mid-sequence. G2a. The perf gate was a coin flip. Full history is 80 runs, 38 failure, 31 success, 11 cancelled, so 47.50 percent red and 55.07 percent excluding cancelled. All 38 failures are total-blocking-time; largest-contentful-paint and resource-summary appear in 0 of 80 logs, so two of the three assertions have never fired. For total-blocking-time, n = 70 with 38 exact medians plus 32 known-censored at or under 800, so percentiles above p46 are exact order statistics: p50 810.9, p75 852.0, p90 878.0, p95 906.5, p98 930.0, p99 and max 1093.0. The threshold of 800 sat at the MEDIAN. That is the whole defect. 800 becomes 950, between p97 and p98, projected 1 in 80. 1100 at p99 was rejected because it projects 0.0 percent, which is a gate that asserts nothing, and it would miss the 1093 observation, which came from a genuinely large design diff and is plausibly a real regression. LCP stays 7000 and script size stays 820000 with the reason in the file: n = 0 for both, lhci prints values only for assertions that fail, and any number would be fabricated. numberOfRuns 5 and aggregationMethod median stay, because the within-run spread across the 190 raw values inside the 38 failing runs has a median of 1807 ms and a max of 3392 ms. And the reason two thirds of that derivation was nearly impossible: the artifact upload has never once produced an artifact. Verbatim from the logs, include-hidden-files: false and "No files were found with the provided path: .lighthouseci". upload-artifact excludes dot paths by default and if-no-files-found: ignore swallowed it silently, so zero of 713 artifacts in this repository belong to any of the 80 perf runs. include-hidden-files is now true and if-no-files-found moves from ignore to error, because a warning leaves the job green over a missing artifact. perf.yml is NOT a required check and nothing here makes it one. lighthouserc.json is strict JSON, confirmed by reading how lhci loads it, so the derivation sits in a top-level thresholdDerivation key beside the threshold a reader would edit; flattenRcToConfig reads only ci, lhci, ci:client and ci:server, verified empirically, so it never reaches lhci. B4. orchestrate/SKILL.md now says the local /pr-review runs once, at the final head, and only when that head's diff touches tools/** or .claude/**. No token figure is claimed; every earlier one is withdrawn, not replaced. Its section 3 branches on the poll's verdict: DELIVERED is the only one that releases the slot, WORKING waits, STALLED relaunches, AWAITING-MERGE is bookkeeping, IDLE returns to the DAG, UNKNOWN acts on nothing. One correction to the plan: teardown cannot run on DELIVERED, because teardown-worktree.mjs requires the merge commit in the target branch and the issue Done, neither true of a DELIVERED, In Review, unmerged ticket. It also learns the launcher's wave mode and --finding, and states the two exhaustion codes plainly: exit 4 is the per-head relaunch allowance, exit 5 is the per-finding strike count. Three stale claims corrected while in the file: every workers entry must declare interactive: true, and codex is bare codex with no subcommand, are both false after PR1's A3a; and the trailing paragraph told the reader a worker never exits when the configured default now does. watch/SKILL.md described BUSY or IDLE from a PID probe, which worker-watch.mjs no longer does. It now describes ALIVE, GONE or UNKNOWN and the six delivery verdicts, and says why IDLE and STALLED had to be split. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
…rage baseline Documentation lands after the code it describes, never before it. PR2 raised a P1 from the per-commit cloud reviewer for exactly the inverse, and that finding was accurate about its commit and moot at the head, yet it still blocked the merge under the standing rule of an approving review with zero unresolved threads. Catalog rows for the three new files, review-rounds.mjs, check-slice-evidence.mjs and harness-roots.json, plus lib/affected-files.mjs and lib/strike-ledger.mjs. Amended rows for automation-budget.mjs, whose BLOCK now keys on the provider's own usage with the token budget demoted to a warning; check-ticket.mjs, which now requires a parseable path and reads the root registry; and worker-status.mjs, which now reports liveness and one of six verdicts. The coverage ratchet goes from 819 to 953 executed assertions across 39 tools, with zero drops. The reseed records growth so the next pull request ratchets against reality rather than against a baseline that already understates the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afb82a2e13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Review Complete
Scope: PR #668, thomasluizon/orbit-ui-mobile — "ORB-163 PR3b of 4: the tool behaviour" (base dcfa85da → head afb82a2e, 58 files, +8138/‑443)
Recommendation: REQUEST CHANGES
| Severity | Count |
|---|---|
| Critical | 0 |
| High | 1 |
| Medium | 4 |
| Low / Info | 4 |
Summary
This PR repairs the tools/ harness scripts behind the autonomous worker/orchestration loop: a durable per-finding strike ledger, a provider-quota-derived launch fuse, wave-mode launching, a STALLED/AWAITING-MERGE liveness split in worker-status.mjs/worker-watch.mjs, a shared Affected modules / files parser, a harness-root-cause filing registry in check-ticket.mjs, and two new read-only measurement tools (check-slice-evidence.mjs, review-rounds.mjs). No apps/*, orbit-api, or packages/shared/src/types file is touched — pure internal tooling, no user-facing surface. The engineering is careful and heavily self-documented, and CI's Harness Execution / Unit Tests / SonarCloud checks are all green on this head. One High finding survives adversarial verification.
Findings
Critical
None.
High
[HIGH] check-ticket.mjs --file mode applies the D5/D5a harness-root-cause gate to every ticket, not just harness ones — will false-reject ordinary bug tickets
- dimension: 1. Correctness
- location:
tools/check-ticket.mjs:350-367(themode === "--file"branch), regex attools/check-ticket.mjs:170 - issue:
--issuemode correctly scopes the check tolabels.includes(HARNESS_LABEL)(tools/check-ticket.mjs:404), proven by its own test (tools/__tests__/check-ticket.mjs:473: an unlabelled--issueticket with aRoot cause:line passes with status 0).--filemode has no such gate — it can't read labels from a drafted file — soROOT_CAUSE_LINE.exec(body)runs against every body. The regex matches any line starting with (optional bullet/bold)Root cause:anywhere in the body and captures the first token after the colon as a harness-root claim. But.claude/skills/ticket/SKILL.mdPhase D step 1 instructs, for every defect ticket regardless of theharnesslabel: "Root-cause hypotheses go in Technical details, labelled as hypotheses," and step 4 requiresnode tools/check-ticket.mjs --file <draft>before every ticket is created. A normal line like "Root cause: A race condition in the token refresh handler." capturesclaim = "A", unregistered intools/harness-roots.json, so the ticket is marked DEFECTIVE with a misleading error pointing at the wrong registry. - risk: Blocks ticket creation via
/ticket//featurefor ordinary non-harness tickets whose Technical Details section follows this repo's own writing convention — breaking the D1-D9 workflow's entry point. - fix: Scope
--filemode like--issuemode: either drop the unconditional check (rely on--issue-mode enforcement post-creation, which the doc string already concedes is partial), require an explicit harness-classification marker in the drafted body before applying the regex, or restrict the claim pattern to registry-id-shaped tokens (kebab-case) rather than any single word. Add a--filetest proving a non-harness ticket with a "Root cause:" line in Technical Details passes. - reference: CLAUDE.md rule 1 /
.claude/skills/ticket/SKILL.mdPhase D - Verified: an independent adversarial skeptic confirmed this by tracing the regex character-by-character, confirming the
--file/--issueasymmetry in the diff, and confirming no test covers the false-positive case. Verdict: CONFIRMED.
Medium
[MEDIUM] AWAITING-MERGE verdict has no test coverage — tools/worker-status.mjs:466-477. Of the six new verdict branches, only AWAITING-MERGE (dead process + open PR + approved head) has no driving test case in tools/__tests__/worker-status.mjs; it appears only in the --help doc regex. A ternary slip could silently fold it into STALLED (spending a relaunch allowance on nothing). Fix: add a case with a dead PID, open PR, and approved current head asserting verdict.verdict === "AWAITING-MERGE".
[MEDIUM] Relaunch allowance can key on local HEAD instead of the real PR head on an incomplete GraphQL read — tools/worker-status.mjs:484 (allowanceHead = prHead ?? localHead). prOpen comes from one query, prHead from a separate GraphQL call keyed by PR number, and its response is never checked for partial failure. If that second call resolves empty while the first still reports the PR OPEN, the allowance silently keys on local HEAD instead of the real PR head, breaking the "a push earns a fresh allowance" invariant. No test exercises prOpen === true with prHead === null. Fix: treat that combination as a refusal (fail(3, ...)) rather than a silent fallback.
[MEDIUM] New linear.team validation in check-ticket.mjs has no test coverage — tools/check-ticket.mjs:61-65. The same alphanumeric-team-key refusal added to wave-plan.mjs (and tested there, tools/__tests__/wave-plan.mjs:60-70) has no matching case in tools/__tests__/check-ticket.mjs. A hostile/absent linear.team would regress silently. Fix: port the two wave-plan.mjs cases over.
[MEDIUM] automation-budget.mjs's real (non-fixture) quota-reading path is never exercised — tools/automation-budget.mjs:357-368. Every test passes an explicit --quota <fixture>; the default spawnSync(ai-quota.mjs --json) path — what actually runs in production — and the "--quota file has invalid JSON" branch are both untested. Fix: add a case stubbing ai-quota.mjs as the real spawned child, plus one for malformed --quota JSON.
Low / Info
tools/worker-status.mjs:19,70spellsORBIT_WORKER_STRIKE_LEDGERas a literal instead of referencing the exportedSTRIKE_LEDGER_ENVconstant (tools/lib/strike-ledger.mjs:29).tools/__tests__/worker-status.mjsnever asserts the literal"DELIVERED"verdict via a live run (only implied byok === true).tools/lib/strike-ledger.mjs's empty-override guard (:37-40) and malformed-JSON-line guard (:55-60) are untested by any of its three consumers.tools/lib/affected-files.mjs:58-59: a colon-annotated bare domain (- config.example.com: staging host) is accepted as a declared path. Minor — only pollutes one ticket's own affected-file list.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/**/apps/mobile/** file changed |
| i18n-syncer | N/A — no user-facing string/locale file changed |
| contract-aligner | N/A — no packages/shared/src/types/*; orbit-api not checked out in this job |
| security-reviewer | N/A — no orbit-api code changed |
| design-reviewer | N/A — no apps/*/orbit-landing-page UI file changed |
Validation (from this PR's own CI run, read live)
| Check | Result |
|---|---|
| Lint / Type Check / Build | PASS |
| Unit Tests | PASS |
Harness Execution (test-tools.mjs + test-hooks.mjs) |
PASS |
| SonarCloud Analysis | PASS |
| Contract Drift / Harness Lockstep / Harness Calibration | PASS |
| Harness coverage ratchet | PASS — tools/harness-coverage-baseline.json 819 → 953, every tool grew, no shrink |
Deferred
- Dimensions 8-14 (DESIGN.md, Parity, i18n, Contract drift, Security-API-side, Backend hard rules, FEATURES.md): N/A — confirmed from the full changed-file list, no
apps/*/orbit-api/packages/shared/src/typessurface touched. - orbit-api side of dimensions 11/12: not verifiable — sibling repo not checked out in this job, per scoping instruction.
tools/review-rounds.mjsandtools/check-slice-evidence.mjs(new tools): read in full and confirmed registered intest-tools.mjs; not independently re-executed locally (Phase 6/7 skipped per caller instruction — CI's green Harness Execution run is cited instead).- Comment policy (dimension 4) across touched
tools/**files: not re-litigated.tools/is outside every ESLint workspace, and the WHY-style//comments without a URL are a pre-existing, established convention throughouttools/*.mjspredating this PR. - No prior review threads or earlier reviews from this reviewer exist on PR #668 (checked via
gh pr view --json reviews,comments— only a Vercel deployment comment present).
What's good
- The strike-ledger's
finding/relaunchscope separation is clean and durability-tested against a mid-flight kill. tools/lib/affected-files.mjsgenuinely eliminates a real duplicate-parser hazard betweencheck-ticket.mjsandwave-plan.mjs(confirmed by grep — no residual copy).accountUsedPercentCeiling's derivation in.claude/orchestrator.jsonis a cited real measurement, and its arithmetic matches the code's actual concurrency cap..claude/skills/watch/SKILL.mdandorchestrate/SKILL.mdwere updated in lockstep with the new verdict vocabulary, matchingworker-status.mjs's ternary exactly.
Recommendation
Fix the High finding before merge: scope check-ticket.mjs --file mode's root-cause check so it stops firing on ordinary, non-harness tickets that follow this repo's own root-cause-hypothesis guidance. The four Medium findings (three coverage gaps, one narrow GraphQL-partial-failure edge case) are cheap to close in the same PR given this repo's own dimension-15 bar, but are not release-blocking on their own.
## HIGH. The --file root-cause gate was unscoped and would have refused ordinary tickets --issue mode scoped the D5a check on the harness label; --file mode, which cannot read labels, ran it against every drafted body. The ticket skill's Phase D tells EVERY defect ticket to put root-cause hypotheses in Technical details and its step 4 runs the file validator before every creation, so an ordinary line reading "Root cause: A race condition in the token refresh handler." captured the claim "A", found it unregistered, and marked the ticket DEFECTIVE against a registry that has nothing to do with it. That is the entry point of the whole D1 to D9 workflow. Two halves, because scope alone was not enough. --file now applies the check only when the drafted body carries a Labels: line naming harness, which is the same predicate --issue uses read from the only place a draft can express intent; a body without the line skips and says so on stderr. And a captured claim must be lowercase kebab-case or it is refused with a message saying exactly that, never the misleading not-registered error. The same shape is enforced on the registry's own ids inside the reader, so what a claim is held to and what the registry may hold cannot drift apart. The finding exposed a gap nobody had named: neither ORB-163's nor ORB-164's body Labels line listed harness, though both carry the label in Linear, so the drafting convention and the live labels had already diverged on the two tickets this gate was built for. Both bodies are corrected in Linear, and /ticket and /feature now tell a drafter to write the line in full and what a harness ticket owes. ## The AWAITING-MERGE branch trapped pull requests that needed a worker A Codex P1, and the deeper half of the same area as the medium coverage finding. The verdict was headApproved alone, so a dead worker on an approved head with an unresolved thread read AWAITING-MERGE, and --consume-relaunch then refused because only STALLED is relaunchable: the pull request could not get the worker it needed to reconcile the thread that was blocking it. It is now headApproved AND zero outstanding review work, where the outstanding set is the review-activity inventory the tool already computes: unresolved threads, resolved without evidence, unacknowledged standalone items. Deliberately not the whole unmet list, which also carries commits, worktree-clean, branch-pushed, head-matches-pr, linear-in-review and the D7 evidence items, none of which a relaunched worker can act on beside an approved head without moving the head and voiding the approval the verdict keys on. Two cases, both directions: the clean shape asserts AWAITING-MERGE and that a relaunch is refused, the same fixture with one unresolved thread asserts STALLED and that a relaunch is granted. ## The relaunch allowance could key on a SHA GitHub never saw allowanceHead was prHead ?? localHead, and the pull request head comes from a separate GraphQL call whose partial failure was never checked. A pull request reported OPEN whose head could not be read now exits 3 naming the failed read, before any allowance arithmetic. The local-head path survives only where it is honest, which is the no-open-pull-request IDLE shape. ## The slice-evidence gate could be paid for by an earlier run's budget A Codex P1. The reservation lookup took every ledger identity prefixed by the issue, so on an issue with prior completed launches it assigned the earliest historical reservation to each new rollout, and a later fan-out that reserved nothing passed on spend records from a previous run. That defeats the only thing the gate proves. It now requires the reservation's reserve-to-record window to CONTAIN the slice process's start. Containment rather than an explicit run identity because there is no shared identity to key on: a ledger row carries no thread id and a rollout's session_meta carries no pid and no ledger identity, both confirmed by dumping a real parent and subagent pair, so inventing a field would have been the same defect one level down. A fixture carrying a prior run's closed reservations plus a fan-out that reserved nothing exits 0 before and 1 after, naming three shortfalls. ## close_agent is a REAL tool whose rollout event has never been emitted, and the signal is deleted A Codex P1, and the reviewer was right to refuse an unconfirmed external interface on a gate's decision path, which is the invented-interface root this pull request registers. Read from the installed engine rather than by running it, since the orchestrator guardrail refuses a codex invocation from this session, which is that gate working. codex 0.146.0 ships one Rust binary whose strings name core/src/tools/handlers/multi_agents/close_agent.rs and list the collaboration set as spawn_agent send_input resume_agent wait_agent close_agent. So the TOOL is real. The EVENT is not: across all 275 real rollouts on this machine, spawn_agent occurs 143 times in 36 files and close_agent occurs zero times, the only textual hits being this ticket's own body quoted inside a prompt. The call the model actually emits is wait_agent, in 67 rollouts. The inference was unsound even if the event existed, because closing a finished child before spawning the next is an ordinary concurrent pipeline, so a close between two spawns would have failed real concurrent runs as serial. The fallback shape now proves concurrency by overlapping child rollout intervals, the same signal the multi-process shape already used, from data every real rollout carries, and a fallback-serialised fixture asserts a non-overlapping fan-out fails. ## The production quota path was the untested one Every existing case passed an explicit --quota fixture, so the default spawn, which is what runs in production and which the BLOCK now keys on, was never exercised. Three cases drive the real spawn with the stub staged where the tool actually resolves its child, asserting the resolved source path so a fixture read cannot masquerade as a spawn: below the ceiling permits, above refuses with exit 4. A malformed --quota file already exited 3 and now has a case pinning it. A fourth went in unasked: a child that prints NOTHING must stay distinguishable from a child that prints an honest UNAVAILABLE, and the case asserts the two reasons differ. ## The shared parser admitted hostnames, and more widely than reported The reviewer found an annotated bare domain accepted. Measured, the bare-host rejection only ever caught a host carrying a slash, so config.example.com was admitted plain, backticked and annotated alike. The guard now runs against the token's first segment as well as the whole token, before every context branch. Two related corrections in the same six lines: a trailing slash is no longer a path separator, and a trailing full stop is stripped so a sentence-final path is not dropped. The residual ambiguity is documented rather than papered over. Once there is no slash a hostname and a repository-root file are the same shape, and a host under a country-code TLD that is also a file extension is not separable without a DNS or filesystem lookup, which a pure hermetic string parser cannot do. The generic-TLD list was chosen over a file-extension allowlist deliberately: an allowlist fails closed, and a dropped real path is the silent-parallelism failure C3 exists to remove, while an unknown TLD slipping through is the loud, serialising direction. Verified behaviour-neutral on the live board: 55 launchable and 101 collision pairs, byte-identical before and after across all 101 issues. ## The three remaining coverage findings The linear.team refusal is ported across from the wave-plan module. DELIVERED is asserted as a literal on a live run. Both strike-ledger guards, the empty override and the malformed line, are driven from their heaviest consumer. And ORBIT_WORKER_STRIKE_LEDGER now imports STRIKE_LEDGER_ENV rather than repeating the literal, because two spellings of one override is how isolation silently stops applying. ## state.type needed evidence, not a change The fourth Codex P1 asked for proof of Orca's response shape before gating on it. Re-read live: orca linear issue ORB-163 --json returns state as an object carrying id, name, type and color, with type "started". issueStateType already throws when type is absent and that becomes exit 2, so the string-shaped payload the reviewer describes fails closed with a named cause. No code changed; the evidence is in the pull request body where it belongs. Every new case was proven RED before green by breaking the thing it guards and restoring the file byte for byte. No fixture ages relative to a compiled-in constant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QX6iyQ4hRU4MiPvccAm35g
|
All ten findings are addressed in High: the
|
|
There was a problem hiding this comment.
Code Review: PR #668 (thomasluizon/orbit-ui-mobile)
Scope: PR #668 — "ORB-163 PR3b of 4: the tool behaviour" (base dcfa85da → head 86f09134). A prior review from this bot already exists on this PR (commit afb82a2e, CHANGES_REQUESTED: 1 High + 4 Medium + 4 Low, plus 4 Codex P1s). Per the caller's instruction, this pass focuses on the diff since that review: afb82a2e13a6..86f09134acd4 (19 non-fixture-jsonl files, +644/‑87) — the fix commit the PR author cites as closing all ten findings.
Recommendation: APPROVE
Summary
I read the actual diff between the reviewed commit and the current head, not the PR body's description of it. Every finding from the prior review is closed by a real code change plus a proportional test, not just an assertion: check-ticket.mjs --file now scopes the root-cause gate to drafts whose Labels: line names harness, and shares an id-shape check with --issue mode via one rootCauseProblem() helper; worker-status.mjs's AWAITING-MERGE/STALLED boundary now keys on a named OUTSTANDING_REVIEW_WORK set whose three members (review-threads, review-activity, resolved-thread-fixes) match the real check names in the checks array, and an OPEN PR whose head can't be read now fails closed (exit 3) before any allowance arithmetic instead of falling back to local HEAD; check-slice-evidence.mjs's reservation match now requires the slice's start to fall inside [reserved.startedAt, recorded.endedAt] rather than merely following the earliest unclaimed reservation, closing the stale-reservation hazard with a fixture built from a genuinely adversarial shape (a prior week's closed reservations sitting in front of a fan-out that reserved nothing); and affected-files.mjs now rejects a bare hostname in every context (list item, backticked, annotated), not only naked in prose. tools/harness-coverage-baseline.json's per-tool deltas (check-ticket.mjs +7, worker-status.mjs +7, automation-budget.mjs +5, wave-plan.mjs +5, check-slice-evidence.mjs +1 → 978 total) line up exactly with the new test cases actually added, so the coverage claim is countable, not asserted. No apps/*, orbit-api, or packages/shared/src/types file is touched by this diff range.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
[INFO] Minor discrepancy between the PR body's "980 assertions" and the coverage baseline's "978"
· dimension: 15. Harness changes need EXECUTED evidence
· location: PR body ("Harness at the fixed head: tools 980 assertions...") vs tools/harness-coverage-baseline.json:3 ("total": 978)
· issue: The two numbers differ by 2 — very likely two different counters (raw suite-wide assertion count vs. the per-tool coverage total the baseline tracks), not a fabricated figure; the per-tool deltas in the baseline diff match the new cases added file-for-file.
· risk: None on its own.
· fix: No action needed; a one-line note distinguishing "assertion count" from "coverage total" would remove the ambiguity for the next reviewer.
· reference: TESTING.md (Harness Execution job)
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/**/apps/mobile/** file in this diff range |
| i18n-syncer | N/A — no user-facing string/locale file in this diff range |
| contract-aligner | N/A — no packages/shared/src/types/*/endpoints.ts in this diff range; orbit-api not checked out |
| security-reviewer | N/A — no orbit-api code in this diff range |
| design-reviewer | N/A — no apps/*/orbit-landing-page UI file in this diff range |
Validation (Phase 6/7 skipped per caller instruction — Build/Unit Tests/SonarCloud gate separately)
Read live from the PR's own CI at review time:
| Check | Result |
|---|---|
| SonarCloud Analysis / SonarCloud Code Analysis | PASS |
| Harness Calibration, Harness Lockstep, Context Budget, Skill and Agent Frontmatter, Cross-Platform Parity | PASS (both matrix legs) |
| Contract Drift | PASS |
| Build | IN_PROGRESS at review time |
Harness Execution (test-tools.mjs + test-hooks.mjs) |
IN_PROGRESS at review time (both matrix legs) — see Deferred |
Deferred
- Dimensions 8–14 (DESIGN.md, Parity, i18n, Contract drift, Security-API-side, Backend hard rules, FEATURES.md): N/A for this diff range — confirmed from the 19-file changed-file list, no
apps/*/orbit-api/packages/shared/src/typessurface touched. - orbit-api side of dimensions 11/12: not verifiable — sibling repo not checked out in this job.
- Harness Execution CI job: still IN_PROGRESS at review time. The PR body's claimed local run (tools 980 assertions/0 failures exit 0; hooks exit 0) was read but not independently re-executed here (Phase 7 skipped per caller instruction). This required check still gates merge regardless of this review's verdict.
- The new
multi-stale-reservationsfixture set was spot-checked for internal consistency (reservation window vs. process timestamps) rather than every byte diffed against sibling fixtures. - Adversarial skeptic pass: not spawned — no surviving Critical/High finding in this diff range to challenge. The prior review's Medium/Low findings were independently re-derived from the current code (not trusted from the PR body) before being marked closed.
What's good
- All ten findings from the prior review are closed by actual code changes or actual new tests, verified against the diff itself.
rootCauseProblem()unifies the id-shape + registration check across--file/--issueinstead of duplicating logic (CLAUDE.md rule 10).- The
AWAITING-MERGE/STALLEDfix ships with tests driving both sides of the new boundary plus the relaunch grant, not just the happy path. - The Codex P1 on
close_agentwas resolved by measurement (grepping 275 real rollouts) rather than re-asserting the original assumption.
Recommendation
No Critical or High finding survives in the diff since the last review — approve. The one open item is procedural: Harness Execution was still running at review time; let it resolve before merge, since a red result there would be Critical per dimension 15 and this review cannot pre-empt it.
|
Acknowledgement of the remaining standalone automated activity on this pull request, so the delivery
One honest note about the delivery gate itself, rather than a silent pass. At this head
|
|
Auditable acknowledgement of every standalone automated item on this pull request, each named by its
One honest note about the gate rather than a silent pass. Its inventory counts every bot-authored The gate's other unmet item, |



ORB-163 PR3b of 4: the tool behaviour. PR1 (#665) landed the launcher and the budget lease, PR2
(#666) split the test file, PR3a (#667) made the deterministic gates able to block. This one repairs
the tools those gates run.
No lockstep twin is touched, so there is no paired orbit-api pull request for this one and no
Harness-Lockstep-Peer:directive.node tools/check-lockstep.mjsexits 0 against orbit-apimain,6 pairs checked. PR4 carries the twins.
Why this is a separate pull request from PR3a
PR1 was 2,555 lines and cost 9 review rounds. PR2 was 14,497 lines, 5.7 times larger, and cost 2.
Size alone does not predict rounds; the number of INDEPENDENT DECISIONS in the diff does, and PR1
carried eight interacting behaviour changes each of which every round re-reviewed. PR3 was split so
each half is one coherent set of decisions rather than a smaller pile of unrelated ones.
D6.
Harness Calibrationwas RED onmainand GREEN in CInode tools/check-calibration.mjs, no flags, onmain:CI reported green because the
guards.ymlstep runs--report-only, which exits 0 on drift bydesign. That is root cause 1 inside the gate set PR3a just made blockable: a gate reporting green
over a condition it never enforced. Three real defects, all fixed:
.claude/skills/quota/SKILL.mdarrived in495f037d(ORB-131, ORB-131: Build the two-sided quota ledger and routing fuse #644) and was never stamped. Theentry is written from actually reading the skill, with its own verdict and reason.
invocationcarried noexec, so the stamp no longer described how workers areinvoked after PR1's A3a. Re-read from
.claude/orchestrator.jsonrather than typed by hand.orchestrate/SKILL.md,watch/SKILL.md).Both verdicts genuinely stayed
keptand both reasons stayed accurate; neither file'seffort:frontmatter moved, only its body.
node tools/check-calibration.mjsnow exits 0,calibration PASS: 36/36 files.--report-onlydeliberately stays inguards.yml. Removing it is what makes the gate blocking,and that belongs with the branch-protection edit at the very end of the ticket, so a red calibration
cannot brick
mainmid-sequence.The new harness case is the one that matters: a stamp whose
invocationdisagrees with.claude/orchestrator.jsonnow fails naming both arrays. The pre-existing case asserted only/invocation mismatch/, so it stayed green when the message was gutted to that bare string, which iswhy defect 2 could ship the day PR1 landed.
C1b. The budget fuse blocked on a number nobody derived
Measured against a copy of the production ledger,
--reset-at 1785902976:The configured
tokenBudgetis 1,000,000, so the pre-C1b fuse refused every launch on everytier, including one projecting zero additional tokens. The provider's own authoritative reading over
the same period is codex
usedPercent11 of 100, and it did not move across the 2 h 22 m sessionthat produced most of that total.
So the BLOCK now keys on the provider's own weekly usage (
claude.weeklyPercent,codex.usedPercent, throughai-quota.mjs) againstautomationBudget.accountUsedPercentCeiling,85, with its derivation written beside the constant. The token budget is demoted to a
warning-only signal and the config now says it is not derived from anything. Reserve, claim, cancel
and record accounting is unchanged: it is the audit trail and it is C4's proof of concurrency.
Same ledger copy, same window,
--invocation-tokens 250000, which istier:deep:usedPercent11"gate":"ACCOUNT","usedPercent":11,"ceilingPercent":85, token total WARN onlygate TOKEN_FALLBACK, projected 1438429The fallback is the common path, not the edge case. The claude arm reads its figure by scraping
an Orca accessibility tree and flipped OK to UNAVAILABLE and back three times during this session on
an idle machine, on top of the two earlier observations. So an unavailable reading falls back to the
token budget, says so on stderr and in the emitted JSON as
gate: TOKEN_FALLBACK, and is never readas zero usage. The launcher's own stricter policy, which exited 3 whenever the reading was
unavailable and so could never let the fallback fire, is relaxed to match, with both halves pinned:
the fallback permits a launch and reaches
worktree create, and still refuses with exit 4 when thefallback would cross the budget.
ai-quota.mjsexits 1 while printing a complete valid object on stdout when both providers areunreachable, confirmed by running it that way. The budget tool reads stdout and ignores the exit
code, deliberately, with that stated in the source.
--helpproves a flag exists and nothing about aresponse body.
E. A green pull request is not a delivered one
worker-status.mjsalready derived delivery from artifacts and it earned its keep on #667: at 06:15Zthat pull request read
mergeStateStatusCLEAN,reviewDecisionAPPROVED, an approval on the headand zero threads, and a hand-built commit-anchored guard said OK, while
worker-status.mjsreturnedexit 1 with
{"ok": false, "unmet": ["worktree-clean", ...]}because the worktree carried twouncommitted files. What it could not do was distinguish a worker still working from one that died.
Three additions, and no rebuild of what was there:
ESRCHgone,EPERMalive and not ours. PID reuse fails CLOSED: a pid that answers alive but whose claim isolder than the 16 hour backstop reads
unknown, neveralive. So does a missing, unreadable ornon-JSON marker, and any other errno. Never a heartbeat: a worker that dies cannot write "I died".
DELIVERED,WORKING,STALLED,AWAITING-MERGE,IDLE,UNKNOWN.STALLEDkeys on the process being gone AND an open unapproved pull request, never on the Linear state,
because a ticket shipping four sequential pull requests sits honestly In Progress and the tool
assumed one pull request per ticket. No open pull request plus no live worker is
IDLE, notstalled: one needs a relaunch, the other needs nothing, and conflating them costs a run in both
directions.
AWAITING-MERGEhad to be named too, or ORB-163 PR3a: make the deterministic gates able to block a merge #667 between its approval and its merge wouldhave earned a false relaunch.
does not, spent through
--consume-relaunch, exit 4 when refused, carryingrelaunch.findingssoa relaunch ships the outstanding findings rather than the ticket body alone.
worker-status.mjsalso had no unknown-flag rejection at all; its coverage entry passed onlybecause
--orbit-not-a-flaghappened to fall through to the "--worktreeis required" path.B3. The strike count is durable now
Clause 4's "two consecutive cycles fail on the same finding" was a STRING in the contract template
whose only assertion matched the clause TEXT with a regex, and every relaunch is a fresh process, so
the count reset and escalation degraded to unbounded retry. That is what 19 rounds looks like.
tools/lib/strike-ledger.mjsis an append-only store keyed by(scope, issue, key), isolated byORBIT_WORKER_STRIKE_LEDGERexactly as the budget ledger is. Two scopes share one file:findingfor clause 4,
relaunchfor section E's per-head allowance. It is deliberately NOT theattempts:NLinear label, which counts a different thing.
launch-worker.mjs --finding <id>exits 5 when thestrikes are spent, which means escalate, never retry.
C3. Launch the wave, not the ticket
wave-plan.mjsalready computed the launchable set and refused to act, and that contract is sound,so the launcher gained the consumer rather than wave-plan gaining a launcher.
--wave-all,--wave-label,--wave-project, each consuming its--json, launching every ticketwhose affected-file sets are pairwise disjoint and serialising the rest. Live on the real board:
--wave-issuesis refused by name. In wave-plan's--issuesmodevisibleWavesfilters eachwave BEFORE
collisionsInruns, so a collision with a ticket the operator did not name is invisible.The other three partition identically, proven by one fixture run through all three. A mode that
silently partitions differently is silence buying parallelism, which is the thing this ticket removes.
The launcher re-invokes itself once per ticket, so the budget fuse, the concurrency lock and
maxParallelWorktreesare all enforced in exactly one place. Deferred tickets are reported, notlaunched: only a merge advances a wave (D3), unchanged.
C4. The slice-concurrency evidence gate
tools/check-slice-evidence.mjsproves a slice run was concurrent structurally, never by wallclock, because a hermetic test cannot invoke the model and a timing assertion flakes. Two or more
rollouts for the ticket with overlapping intervals, plus one reserve AND one record ledger row per
slice, each a distinct reservation. It keys on the RESOLVED engine and reads
CODEX_HOMEfrom theenvironment; confirmed live that it is unset in an ordinary shell and that Orca redirects it to
%APPDATA%\orca\codex-runtime-home\home, so the tool refuses rather than assuming~/.codex.Four fixtures, all derived from two REAL rollouts read off this machine rather than invented:
Fixture 3 is the one that would have caught the measured production blindness: 160,505 uncached input
tokens across three
spawn_agentchildren on ORB-153 that the ledger never saw.A real find from reading the rollouts: on a subagent rollout
payload.session_idcarries thePARENT's id and
payload.idthe child's own. An invented fixture would have had that backwards, andthe tool keys on
payload.id.The
close_agentsignal was REMOVED after review, and the investigation is the point. The toolis real: the installed codex 0.146.0 binary names
core/src/tools/handlers/multi_agents/close_agent.rsand lists the collaboration set as
spawn_agent send_input resume_agent wait_agent close_agent. TheEVENT is not: across all 275 real rollouts on this machine,
spawn_agentoccurs 143 times in 36 filesand
close_agentoccurs ZERO times, the only textual hits being this ticket own body quoted inside aprompt. The call the model actually emits is
wait_agent, in 67 rollouts. The inference was alsounsound even if the event existed, since closing a FINISHED child before spawning the next is an
ordinary concurrent pipeline. So the fallback shape now proves concurrency by overlapping child
rollout intervals, the same signal the multi-process shape already used, from data every real rollout
carries. Read from the installed binary rather than by running it, because the orchestrator guardrail
refuses a
codexinvocation from this session, which is that gate working.D1, D2, D5a. The ticket gates
D1, the ledger-child gate had never executed.
check-ticket.mjsreadissue.parent, butorca linear issue <id> --full --jsonreturns 17 keys with noparentandrelations: [], andorca's relation vocabulary is
{blocks, blocked-by, related, duplicate-of}. The gate wasshort-circuited rather than starved: a
parent === null ||disjunct whitelisted the empty case soreadLinearParent, which works, never ran. On the same real ticket:The whole orca-derived parent path is gone, not just the disjunct: with
relationship === "parent"deleted,
parentFromOrcacould return nothing, so it and its three cases went too. Keeping a deadbranch alive with two harness cases is the harness proving a belief rather than a behaviour.
The assertion that asserted the fail-open as correct is INVERTED, with a companion case proving a
genuinely parentless issue still validates once the lookup answers, so the inversion did not turn "no
parent" into "always refuse".
D2, the dependency-prose guard fires wrong on
maintoday. The trailing/imade the identifierclass case-insensitive. Extracted verbatim and run against the strings this ticket itself produced:
The identifier alternation is now built from the team key the configuration declares rather than a
hardcoded
[A-Z][A-Z0-9]+, and a non-alphanumeric team key exits 2 rather than being interpolatedinto a regex.
D5a, the filing gate is OPEN but REGISTERED.
tools/harness-roots.jsoncarries five roots fixedby the vault ADR of 2026-07-31, each with an
id, a definition and theownerticket. Anunregistered id is refused; a registered id whose owning ticket is still open is refused naming that
ticket;
exemptclaims nothing. Scoped by theharnessLinear label.check-ticket.mjsacquires nowrite capability: it exits 1 naming the already-filed ticket and stops, and the append is the
/ticketand/featureskills' job.A harness ticket with NO
Root cause:line is REFUSED, not warned about. The specification putthe obligation on the line rather than on its presence, and that left a hole: ORB-164 carried the
harnesslabel and no such line when this work started, so the gate would have passed its own pairedticket. The point is to force the classification, not to check the spelling of one somebody
volunteered.
--filemode stays fail-open by necessity, because the label is genuinely unreadablethere, and
--helpsays exactly that.Also fixed while in the file: the
visible-effectword list used\b, which treats a hyphen as aboundary, so
Root cause: string-not-actmade a pure tooling ticket demand screenshots.D3. Config read from a working tree nothing updates
.claude/orchestrator.jsonis now resolved fromorigin/<base>. Three branches, each a differentfact about the checkout: no git repository uses the working copy; a HEAD that CONTAINS
origin/<base>keeps its own newer copy, because refusing an ordinary pull request that edits theconfig would turn every tool red and that is how a gate gets switched off; a HEAD behind
origin/<base>whose copies disagree REFUSES, naming every differing leaf and the remediation. Anunresolvable ref fails CLOSED after one explicit-refspec fetch, and never falls back to the working
copy.
C3's other half, and one duplication removed at the root
check-ticket.mjsnow requires at least one PARSEABLE path under the affected-files heading, notmerely the heading, because a ticket with no path list collides with everything and silence must not
buy parallelism.
That parser lived in
wave-plan.mjs, which runs its whole body at import time and so could not beimported, so the first attempt copied it byte for byte and guarded the copy with an equality
assertion. That is a workaround.
tools/lib/affected-files.mjsnow owns it and both tools import it,so a wave's collision set and a ticket's validation can no longer disagree about what a ticket
declares.
wave-plan.mjsalso stopped hardcodingconst TEAM = "ORB"while the configurationdeclares
linear.team, which is thestale-sourceroot this pull request registers.F. The review-round definition is frozen in a tool
tools/review-rounds.mjs. One round is one DISTINCT HEAD COMMIT that received a review submissionfrom either bot, and it prints the four alternative definitions beside the frozen one so two can
never be merged into one figure again, which is how a mean of 4.60 and a worst of 13 came from
different sets and understated rounds by 16 percent.
The trap, and it caught a probe before it caught anyone else: the CI reviewer's login is
claudein GraphQL, not
claude[bot].[bot]is the REST spelling, and section F states its definition inREST terms. A counter filtering on
claude[bot]reports 0 rounds on a pull request carrying 15bot submissions, and a plausible zero is indistinguishable from a real one.
claude[bot]appears inzero of 289 review nodes captured live and committed verbatim. So the tool resolves a bot on
GraphQL
__typename, never on a login string, and refuses rather than counting when it meets a Botlogin outside the known set.
Reproduced exactly, at the recorded cutoff:
claudealoneOne correction to the recorded interval.
--as-ofis inclusive, which is the natural reading of"snapshot at T", so the reproducing window is
[2026-07-30T16:11:16Z, 2026-07-30T19:48:33Z), the sametwo instants with the brackets flipped. At an inclusive
19:48:33Zthe frozen mean is 5.55, because aclaudeCOMMENTED submission on #658 head70aca7feis stamped exactly then. Both figures are rightabout different instants, which is precisely why the tool prints the instant beside the numbers. Live
with no cutoff today the same 20 pull requests read 5.60 / 4.70 / 3.10 / 6.30 / 2.25.
A truncated review page is REFUSED rather than paginated, and so are a failed, errored or
unparseable lookup and a review missing its author,
submittedAtorcommit.WITHDRAWN and encoded nowhere: Pearson 0.738, whose real value under the frozen definition is
0.496 with a matrix peak of 0.629, and "under 400 lines averaged 15.0 reviews, over 400 averaged
24.8", which has zero hits across 4 subset filters, 2 size measures, thresholds 100 to 800, 10 count
quantities and both mean and median. The largest per-pull-request round count in the window is 14, so
those two cannot be rounds under any definition. What survives is enough: larger pull requests cost
roughly 1.6 to 2.1 times more review activity and the correlation is real and positive at about 0.5.
changed_filescorrelates between -0.02 and 0.18, essentially not at all.G2a. The perf gate was a coin flip
Full history: 80 runs, 38 failure, 31 success, 11 cancelled, so 47.50 percent red and 55.07 percent
excluding cancelled. All 38 failures are
total-blocking-time; the stringslargest-contentful-paintandresource-summaryappear in 0 of 80 logs, so two of the threeassertions have never fired.
total-blocking-time, n = 70 (38 exact medians plus 32 known-censored at or under 800, so percentilesabove p46 are exact order statistics): p50 810.9, p75 852.0, p90 878.0, p95 906.5, p98 930.0, p99 and
max 1093.0. The threshold of 800 sat at the median. That is the whole defect.
800 becomes 950, between p97 and p98, projected 1 in 80. 1100 at p99 was rejected: it projects 0.0
percent, which is a gate that asserts nothing, and it would miss the 1093 observation, which came
from a genuinely large design diff and is plausibly a real regression. LCP stays 7000 and script size
stays 820000 with the reason written in the file: n = 0 for both, lhci prints values only for
assertions that fail, and any number would be fabricated.
numberOfRuns: 5andaggregationMethod: medianstay. Runner noise, characterised from the 190 rawper-run values inside the 38 failing runs, has a within-run spread with a median of 1807 ms and a max
of 3392 ms; the median-of-5 is what makes the metric usable at all.
And the reason two thirds of that was nearly impossible: the artifact upload has never once
produced an artifact. Verbatim from the logs,
include-hidden-files: falseandNo files were found with the provided path: .lighthouseci.actions/upload-artifactexcludesdot-paths by default and
if-no-files-found: ignoreswallowed it silently, so zero of 713 artifactsin this repository belong to any of the 80 perf runs.
include-hidden-files: truenow, andif-no-files-foundmoves fromignoretoerror, because a warning leaves the job green over amissing artifact.
perf.ymlis NOT a required check and nothing here makes it one.lighthouserc.jsonis strict JSON, confirmed by reading how lhci loads it, so the derivation sits ina top-level
thresholdDerivationkey beside the threshold a reader would edit;flattenRcToConfigreads only
ci,lhci,ci:clientandci:server, verified empirically, so it never reaches lhci.B4, B2 and the /watch wiring
orchestrate/SKILL.mdnow says the local/pr-reviewruns once, at the final head, and only whenthat head's diff touches
tools/**or.claude/**. No token figure is claimed; every earlier oneis withdrawn, not replaced. Its B2 blocking wait was already landed by PR1; what was missing was
accuracy against the tool, so the
pr-watchinvocation is now written from reading it.Section 3 branches on the poll's verdict:
DELIVEREDis the only one that releases the slot,WORKINGwaits,STALLEDrelaunches,AWAITING-MERGEis bookkeeping,IDLEreturns to the DAG,UNKNOWNacts on nothing and surfaces. One correction to the plan: teardown cannot run onDELIVERED, because
teardown-worktree.mjsrequires the merge commit in the target branch and theissue Done, neither true of a DELIVERED, In Review, unmerged ticket. DELIVERED releases the slot;
section 4's merge is when teardown can actually pass.
worker-watch.mjsno longer derives liveness itself. Its ownalive()treated every non-ESRCHerrno as alive with no reuse backstop and no UNKNOWN state, so it printed BUSY for a recycled pid: a
tool reporting a state it never established. It now renders
worker-status.mjs's liveness andverdict verbatim, and a run that printed no JSON reads
verdict unavailablerather than an inventedstate. Cost measured rather than guessed: 4.9 s for one worktree, 9.2 s for two, which an
operator-invoked snapshot accepts and which is documented in the tool.
A second defect found there:
--repofiltered on a path prefix, and Orca does not put a childworktree under its repository path, so
--repo uiprinted an empty fleet over a live worker. Itnow resolves orca's
repoIdand REFUSES with exit 2 when a configured path is not an orca mainworktree, rather than reporting an empty fleet it never checked. The old fixtures parked their
worktrees inside the repo path, agreeing with the bug.
F-5's owed case, and the isolation nobody was proving
A stranded reservation found in the PRODUCTION ledger was never a worker: it matched the harness's
own fixture field for field, including a mocked
usedPercent === 10, with no worktree, transcript orsession behind it. A test fixture had written into the production ledger. The isolation exists; two
assertions now prove it stays, one reading the ledger path a child actually RECEIVES and one
bracketing the module with a size and mtime read of the real ledger.
Every new gate was proven RED before it was proven green
A gate that has never failed is unproven. Each was driven by breaking the thing it guards, watching
the case fail, and restoring the file byte for byte. Forty-odd mutations across the seven slices, the
load-bearing ones being: the D1 disjunct restored (16 red), the D2
/irestored (4 red, exactly thefour lowercase slugs), the calibration message reduced to a bare string (1 red, and the pre-existing
regex case stayed GREEN, which is the proof it never asserted the values),
isBotresolving on theREST spelling (5 red including the non-zero-count case), UNAVAILABLE read as zero usage (11 red), the
local liveness derivation restored (7 red), and the
--repopath filter restored (1 red).No relative-age fixture anywhere. Every case that measures against a compiled-in constant is
pinned at an ABSOLUTE value the test controls, so moving the constant turns the case red instead of
quietly following it. That defect shipped once on this ticket already and it is the reason a
regression stayed green across a constant change.
Evidence
Exit codes read from files, never through a pipe.
Both harnesses, run in full at this head:
node tools/test-tools.mjsEXIT=0 andnode .claude/hooks/test-hooks.mjsEXIT=0. Repo-tool appeal: the ticket's acceptance criteria require both harness invocations pasted with their real exit codes, and an evidence line that names no command proves nothing.The deterministic gates, each run at this head, gate name and real exit code:
calibration PASS: 36/36 files, model gpt-5.6-terra, date 2026-07-31frontmatter ok: 36 skill and agent files parseHARNESS LOCKSTEP OK: 6 pairs checkeddelta: +0 bytesEm dash count across all changed and new files: 0, checked by hand because
check-dashes.mjsskips
.claude/wholesale.Rollback
Reverting this pull request restores the previous tool behaviour, but two things do not come back
with it:
.claude/calibration.json's stamp would return to a state where the gate fails 35 of 36,and the branch-protection payload is not touched by any revert. A rollback that also needs the
protection state restored must apply the recorded baseline payloads by hand.
Not in this pull request
Section G's deletions, G4, G5 and D4 are PR4, which also carries the four lockstep twins and the
paired orbit-api pull request. Section H's vault record corrections land after PR4 merges. The
branch-protection edit is the last act of the whole ticket, in two parts, and the calibration step
keeps
--report-onlyuntil then.Review round 1: all ten findings closed in 86f0913
One High, four Medium, four Low from the CI reviewer, plus four P1s from the Codex reviewer. Every
one is addressed; nothing is deferred. The full answer is in the review reply comment on this pull
request and in that commit message. Two of them changed the design rather than adding a case: the
AWAITING-MERGE verdict was trapping pull requests that needed a worker, and the slice-evidence gate
could be satisfied by an earlier run reservations.
The High one is worth restating here because it would have broken the workflow entry point: the
--file root-cause gate was unscoped, so an ordinary ticket writing "Root cause: A race condition in
the token refresh handler." in Technical details, which the ticket skill instructs EVERY defect
ticket to do, captured the claim "A" and was marked defective against a registry that has nothing to
do with it.
Proof of the state.type shape the Codex reviewer asked for, read live:
Harness at the fixed head: tools 980 assertions, 0 failures, exit 0; hooks exit 0. Coverage baseline
reseeded 953 to 978, no tool lost coverage.