From d2e747251b15674ef061f539a6b037af18164dec Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Fri, 28 Aug 2026 03:01:06 -0400 Subject: [PATCH 1/2] drive: WP-11: repair PR #9 under review before anything else --- ops/NEXT.md | 286 +++++++++++++++++++++------- ops/reviews/20260828-0258-review.md | 276 +++++++++++++++++++++++++++ 2 files changed, 497 insertions(+), 65 deletions(-) create mode 100644 ops/reviews/20260828-0258-review.md diff --git a/ops/NEXT.md b/ops/NEXT.md index 4abee4387..f500b3cbb 100644 --- a/ops/NEXT.md +++ b/ops/NEXT.md @@ -1,65 +1,221 @@ -# NEXT — WP-9 merge handoff for PR #8 - -Written by the Relayflow Lead on 2026-08-27 for branch -`flow/drive-57e923c-08271542` and existing PR #8. - -## Current state - -WP-9 has no remaining product work. The branch contains `origin/main` commit -`6366943`, including the repository's evidence-capture standard. Gate 1's -covenant-2 preflight implementation remains on PR #8 and is not on `main`. - -The package-mandated deterministic-command limitation is public in -`docs/SURFACE.md`: an unresolved bare command receives the typed -`command_unresolved` warning rather than a refusal because `/bin/sh -c` may -supply a builtin, function, or assignment. The narrower unresolved path-like -case remains filed under “Close the deterministic-command preflight gap” in -`ops/BACKLOG.md`. - -The review-repair chronology is append-only under `ops/reviews/`. Rejected -WP-9 heads and their repairs are: - -- `18f03be`: the new surface paragraph named the wrong warning kind; - `c04d388` corrected it to `command_unresolved`. -- `c04d388`: the gate scoreboard retained the pre-WP-8 SDK count; - `fa19df1` corrected 130 to the reproduced 131 without changing Gate 1's - AMBER state. -- `385763a`: the branch still carried a WP-7 selector; `2b117ae` replaced it - with the supplied WP-9 assessment. -- `2b117ae`: that copied assessment still queued already-completed work; - `80aa711` replaced it with a present-tense merge handoff. -- `80aa711`: the handoff understated its own rejection chronology; this - revision removes the count and records the immediate prior rejection. - -The final changed-head review transcripts after this handoff are the merge -evidence. Each must name the same reviewed SHA, end in `REVIEW_PASSED`, and -the aggregate must return `SWARM_PASSED`. Only those transcript commits may -follow the reviewed handoff head. - -## Captured verification - -`ops/DRIVE-LOG.md` contains the literal WP-9 verification output: - -- kernel: 72 passed, 0 failed; -- Clippy with `-D warnings`: exit 0; -- Rust formatting: exit 0 with empty output; -- SDK: 131 passed, 0 failed; -- clean-room install/build: `dist` and `node_modules` moved aside - recoverably, `npm ci && npm run build && test -x dist/cli.js`: exit 0; -- largest Rust file: 468 lines. - -The destructive `rm -rf` spelling in the package was rejected by the worker -safety layer before process launch. The recoverable move established the same -absence precondition; the exact substituted command and output are preserved -in `ops/DRIVE-LOG.md` and the PR body. - -## Next action - -PR #8 remains OPEN. A human reviews and merges it if satisfied. After merge, a -new tick re-runs the Gate 1 verification on merged `main` and only then may -move `ops/SCOREBOARD.md` from AMBER to GREEN. - -The Lead does not merge, does not start Gate 2+ work while PR #8 is open, and -does not add product features to this branch. - -END_HANDOFF +# NEXT — WP-11: repair PR #9 under review before anything else + +Written by the Relayflow Lead on 2026-08-28, assessing at `615f97d` +(`flow/drive-615f97d-08280219`); `origin/main` is `173423c`. + +## Why this and nothing else + +`ops/DIRECTIVES.md` carries no standing directive, so the backlog governs — +but the backlog does not get a turn. **PR #9 is open with four untriaged +reviewer findings at HEAD `f435545`, three of them P1.** The operating rule is +explicit: no new work over unfinished work. WP-11 is the repair of PR #9. + +I read all four findings against the code at `f435545` and **confirmed every +one**. They are not bot noise. Two of them break the surface the PR exists to +ship: as merged today, `flows run` reports a protocol error for any flow that +successfully dispatches to a live worker, and for any run lasting longer than +30 seconds. + +There is also **no adversarial review transcript for PR #9** under +`ops/reviews/` — the newest entries are `20260828-0127-cloud-execution.md` and +the PR #8 rounds. The tick's own gate has not run on this PR. Per +`ops/RUN-CONTRACT.md` §3.3, that alone disqualifies it from the merge bar, +independent of the findings. + +## Current state, verified now (not carried from a report) + +Both suites are green at the assess head `615f97d`: + +```text +$ (cd sdk && npm test) + Test Files 8 passed (8) + Tests 131 passed (131) + +$ (cd kernel && ../ops/cargo.sh test --workspace) +test result: ok. 18 passed; 0 failed; ... +test result: ok. 0 passed; 0 failed; ... +test result: ok. 19 passed; 0 failed; ... +test result: ok. 26 passed; 0 failed; ... +test result: ok. 3 passed; 0 failed; ... +test result: ok. 6 passed; 0 failed; ... +``` + +That is 72 kernel tests and 131 SDK tests on the tick head. PR #9 claims 73 +kernel (a new `spec_parity` case) and adds `sdk/tests/live-kernel.test.ts`. + +PR #9 mechanics: `MERGEABLE` / `CLEAN`, CodeRabbit and Devin checks `SUCCESS`. +Per RUN-CONTRACT §3.1 those green checks are **not** review signal — CodeRabbit +posted only a run-configuration summary and Devin produced no findings. The +only substantive external review is Codex's, and it is the four findings below. +The branch is **6 commits behind `origin/main`** (`8c7285b`, `0edbe99`, +`96c3abb`, `0dac7d2`, `615f97d`, `173423c`). + +## Objective + +Bring PR #9 to a state that genuinely meets `ops/RUN-CONTRACT.md` §3: every +inline finding fixed or refused-with-reasoning **at HEAD**, each with a reply +recording the audit; the branch rebased onto current `main`; the full DoD +re-run on the rebased head; and an adversarial review transcript on disk. + +## The four findings, as I verified them + +**F1 (P1) — `flows run` dies at 30 seconds.** `sdk/src/cli/run.ts:65` awaits +`client.runStart(spec)` as a single request. `sdk/src/journal-client.ts:49` +sets `requestTimeoutMs = options.requestTimeoutMs ?? 30_000`, and `run.start` +does not return until the daemon has driven the run to a terminal or parked +state. Two sequential 20-second deterministic steps are each individually +valid and make `flows run` reject after 30s with `protocol_error` while +`relayflowd` keeps driving the journaled run to completion. The CLI reports a +failure for a run that succeeds — the exact inversion of "report honestly." +Fix by giving the run lifecycle a lifecycle-appropriate (or absent) timeout, +or by submitting and following `run.watch`; a bare timeout bump is not a fix, +because any constant is wrong for a durable-timer flow. + +**F2 (P1) — a successful dispatch is reported as a protocol error.** +`kernel/relayflowd/src/engine/drive.rs:120-138` returns +`RunStatus::Parked` after dispatch *whether or not a worker took the step* — +the registry row distinguishes them (`waiting_worker` vs `parked`), the wire +outcome does not. `findParkedStep` (`sdk/src/cli/run.ts:168`) then looks for a +step whose `run.get` state is `Runnable`; a dispatched step is `Running`, so +no step matches, and `classifyOutcome` falls through to `protocolFailure`. +Every CLI-started flow that reaches a live `llm` or `agent` worker is reported +as a protocol error. +*The repo already contains the correct answer to copy:* +`kernel/relayflowd/src/server/client.rs:98` handles a `Parked` outcome by +consulting the registry and branching on `waiting_worker` (keep following the +lease) versus `parked` (nothing is coming). The TypeScript surface needs the +same distinction. **It must not read the sqlite registry** — AGENTS.md rule 3 +makes the journal protocol the boundary. `RunGetResult.steps` is already +`Record` and carries `Running`, so this is expressible over the +existing protocol with no wire change: an out-of-band step in `Running` is +"waiting on a worker," not "parked with nothing coming." + +**F3 (P1) — the crash-recovery test injects no crash.** +`sdk/tests/live-kernel.test.ts:281` names itself "surface resume after a real +daemon kill," but the run is interrupted by a *separate* +`relayflowd run --stop-after 1` process that exits cleanly (the test asserts +`status === 0` and `initial.status === 'interrupted'`). Only then is +`firstDaemon` spawned, and it does nothing but read the already-finished +journal before being SIGKILLed. Killing an idle daemon interrupts no step and +no boundary; the test passes whether or not socket-started crash recovery +works. This is the "a gate that runs nothing fails" family (RUN-CONTRACT §4) +and it sits directly on gate 1's done-when. Fix: start the run through the +daemon that gets killed, and inject `SIGKILL` while that run is genuinely +mid-flight. +*Calibration, stated so the fix is not oversold:* the underlying property is +already covered kernel-side by +`sigkill_under_serve_resumes_the_socket_started_run`. What is unproven is the +**CLI surface** claim this test makes. Fix the test; do not claim it uncovered +a kernel regression. + +**F4 (P2) — `run_unavailable` is asserted about errors it cannot see.** +`resumeFlow`'s catch (`sdk/src/cli/run.ts:86`) maps *every* `run.resume` +rejection to exit 2 / `run_unavailable`. `docs/SURFACE.md:134` defines exit 2 +as "refused **before a journal write**." A request timeout, a dropped socket, +or a daemon `journal_write_failed` after resume processing began all now tell +automation that nothing ran when the journal may already have changed. Two +things must land together: +1. `JournalClient` currently **discards the structured code** — line 109 + rejects with `new Error(\`${res.error.code}: ${res.error.message}\`)`. It + must reject with a typed error carrying `code`, or classification is + impossible by construction. +2. The kernel has **no `run_not_found` code**. `server.rs` maps an unknown run + on `run.resume` through `internal_error` → `internal` + (`kernel/relayflowd/src/server.rs:166-179`, `:438-448`); the closed set is + `bad_request`, `protocol_mismatch`, `invalid_spec`, `unsupported_verb`, + `lease_conflict`, `journal_write_failed`, `internal`. So "this run does not + exist" is presently indistinguishable from "something broke." Introducing a + typed `run_not_found` is **in scope** — gate 1's done-when requires the + failure taxonomy to be closed and to "never [terminate in] a raw error," + and `internal` for a missing run is exactly the raw error it forbids. + Transport and runtime errors then route through `protocolFailure`. + +## Files in scope + +- `sdk/src/cli/run.ts` — F1, F2, F4 classification. +- `sdk/src/journal-client.ts` — F1 timeout policy; F4 typed protocol error. +- `sdk/src/failure-kinds.ts` — only if F2/F4 add a kind to `RUN_FAILURE_KINDS`. +- `sdk/tests/live-kernel.test.ts` — F3; plus new live coverage for F1 and F2. +- `sdk/tests/cli.test.ts`, `sdk/tests/journal-client-loopback.ts` — unit-level + coverage for the new classification and the typed error. +- `kernel/relayflowd/src/server.rs` — F4 only: a typed `run_not_found`. +- `docs/SURFACE.md` — reconcile the exit-code table with what the code now + does. If a documented meaning and the code disagree after the fixes, the + document is wrong until proven otherwise. +- `ops/DRIVE-LOG.md`, `ops/reviews/` — evidence. + +## Definition of done + +Every command below runs on the **rebased** head and its literal output is +captured (AGENTS.md, "evidence is captured, not narrated" — paste the output, +not a summary of it). + +1. Rebase `flow/drive-77b2457-08280058` onto `origin/main` (`173423c` or + later) in a scratch worktree — RUN-CONTRACT §4: the tick owns its checkout + and all other repo operations go elsewhere. +2. Each of F1–F4 has a test that **fails before the fix and passes after**. + For F2 and F3 this is mandatory and mechanical: F2 needs a live worker + attached while `flows run` starts the flow; F3 needs the kill to land on + the daemon actually driving the run. Mutation verification has one meaning + here (AGENTS.md §2) — revert, capture the failure, restore byte-for-byte, + capture the pass, paste both. +3. Build first, then test — `sdk/tests/live-kernel.test.ts` hard-fails in + `beforeAll` if either binary is missing, so ordering is load-bearing: + ``` + (cd kernel && ../ops/cargo.sh build) + (cd sdk && npm ci && npm run build) + ``` +4. `(cd kernel && ../ops/cargo.sh test --workspace)` — 0 failed. Report the + new total; do not restate 72 or 73 without reproducing it. +5. `(cd kernel && ../ops/cargo.sh clippy --workspace -- -D warnings)` — exit 0. +6. `(cd kernel && ../ops/cargo.sh fmt --check)` — exit 0, empty output. +7. `(cd sdk && npm test)` — 0 failed, `live-kernel.test.ts` among the files + that **ran** (a skipped live suite is a failed DoD, not a pass). +8. `git status --porcelain` — empty, before and after. +9. Each of the four Codex comments receives a **reply on the PR at HEAD** + naming the fixing commit, or an explicit reasoned refusal. Never silently + waved, never silently dismissed (RUN-CONTRACT §3.2). +10. An adversarial review transcript lands in `ops/reviews/` naming the + reviewed SHA and ending in a verdict token. PR #9 currently has none. +11. `ops/DRIVE-LOG.md` carries this tick's literal transcript, including any + failure. A tick that fails verification opens no PR and logs the failure. + +Merging is governed by RUN-CONTRACT §3 and is **not** part of this package's +done: if the full bar holds after the above, the merge is permitted under the +§2 grant; if any clause is short, PR #9 stays open with the reason recorded. + +## Explicitly OUT of scope for this tick + +- **Any gate 2–9 work**, including gate 6 integrations, even though the + scoreboard marks gate 6 "next up." PR #9 is unfinished work; gate 6 waits. +- **Every backlog item**, specifically: the deterministic-command preflight + gap, the release pipeline, the `steps: []` asymmetry, `f.browser`, the + regression suite, and the cloud-schedule re-registration. +- **The cloud sandbox verify gaps** (`ops/cargo.sh` losing its exec bit, + absent `node_modules`) filed at `615f97d`. Real and blocking for unattended + cloud ticks — and still not this tick. +- **The eight findings carried from PR #8** at squash. They stay in + `ops/BACKLOG.md`; do not fold them into this branch. +- **Any new product surface on PR #9.** Repair only. A feature added during a + review-repair round restarts the review and is how a PR stops converging. +- **Editing `docs/RFC-0001`** or `ops/DIRECTIVES.md` — Khaliq's, by PR. + +## One inconsistency, flagged not silently resolved + +`charter/LEAD.md` says "**You never merge.** You open PRs and report." +`ops/RUN-CONTRACT.md` §2 records a later verbatim grant from Khaliq — +"u have permissions to merge moving forward if all green and pr feedback is +addressed" — and §3 defines the bar. I am treating the later explicit grant as +operative and the charter line as stale text. This is not blocking and needs +no answer to proceed; it should be settled in the charter by PR when +convenient, so the two documents stop contradicting each other. + +## Assessment note on stale state + +`ops/NEXT.md` as it stood on `main` described WP-9 and PR #8 as open. PR #8 +merged at `9e1d9eb` on 2026-08-28 and gate 1 is GREEN on the scoreboard. This +file replaces that handoff. `ops/DRIVE-LOG.md` on `main` likewise ends on the +WP-9 entry; the WP-10 entry exists only on PR #9's branch and reaches `main` +when #9 does. + +END_ASSESSMENT diff --git a/ops/reviews/20260828-0258-review.md b/ops/reviews/20260828-0258-review.md new file mode 100644 index 000000000..7a6d5594a --- /dev/null +++ b/ops/reviews/20260828-0258-review.md @@ -0,0 +1,276 @@ +# Adversarial review — WP-11 assessment (`ops/NEXT.md`) + +**Reviewer:** non-interactive review agent +**Reviewed tree:** `615f97d` + uncommitted working-tree change to `ops/NEXT.md` +(HEAD == `main` == `615f97d`; the branch `flow/drive-615f97d-08280219` carries +no commits ahead of `main`, so `git diff main` is the working-tree diff). +**Diff under review:** `ops/NEXT.md` only — 1 file, +221 / −65. +**Bar applied:** `ops/NEXT.md` (scope), `AGENTS.md` (standards), +`docs/RFC-0001-everything-is-a-relayflow.md` (semantics), +`ops/RUN-CONTRACT.md` (operating contract). + +--- + +## 0. What the diff is + +The diff replaces the WP-9 / PR #8 merge handoff on `main` with a WP-11 +assessment: repair PR #9 before anything else. No product code, no test, no +gate, no RFC, no `ops/DIRECTIVES.md` is touched. Reviewing it means reviewing +whether its *claims* reproduce, since the document's entire value is that the +next tick can act on it without re-deriving anything. + +Note on ordering: `ops/NEXT.md` was last written at 02:24 EDT. PR #9's head at +that moment was `f435545` (authored 01:18). The repair commits `c83a367` +(02:43) and `3616c0a` (02:49) landed **after** the assessment was written, so +PR #9's present head (`3616c0a`) is not evidence against the document — it is +the document being executed. Every claim below is therefore judged as of +`f435545`, which is the head the document names and the head Codex reviewed. + +## 1. Refutation attempts — claims I tried to break and could not + +Every check below was run; the command and its result are recorded. + +### 1.1 The pasted test evidence reproduces + +`AGENTS.md` §"Evidence is captured, not narrated" is the standard six PR #8 +rounds were rejected against, so this was the first thing I attacked. I re-ran +both suites on the reviewed tree: + +``` +$ (cd sdk && npm test) + ✓ tests/preflight.test.ts (12 tests) ✓ tests/journal-client.test.ts (12 tests) + ✓ tests/validate.test.ts (36 tests) ✓ tests/hello-deterministic.test.ts (5 tests) + ✓ tests/deterministic-llm.test.ts (5) ✓ tests/spec-parity.test.ts (12 tests) + ✓ tests/cli.test.ts (42 tests) ✓ tests/bin.test.ts (7 tests) + + Test Files 8 passed (8) + Tests 131 passed (131) +``` + +``` +$ (cd kernel && ../ops/cargo.sh test --workspace) +test result: ok. 18 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.55s +test result: ok. 0 passed; 0 failed; ... +test result: ok. 19 passed; 0 failed; ... +test result: ok. 26 passed; 0 failed; ... +test result: ok. 3 passed; 0 failed; ... +test result: ok. 6 passed; 0 failed; ... +test result: ok. 0 passed; 0 failed; ... <- not shown in NEXT.md +test result: ok. 0 passed; 0 failed; ... <- not shown in NEXT.md +test result: ok. 0 passed; 0 failed; ... <- not shown in NEXT.md +``` + +SDK: 8 files / 131 tests — byte-for-byte what `ops/NEXT.md:31-32` pastes. +Kernel: 18 + 0 + 19 + 26 + 3 + 6 = **72**, exactly the total claimed. The +elision is recorded as F5 below; the numbers themselves are honest. + +### 1.2 Every code citation resolves at `f435545` + +| Claim | Cite | Verified | +|---|---|---| +| `run.start` is one awaited request | `sdk/src/cli/run.ts:65` | ✅ exact — `const outcome = await client.runStart(spec);` | +| 30s default request timeout | `sdk/src/journal-client.ts:49` | ✅ exact — `this.requestTimeoutMs = options.requestTimeoutMs ?? 30_000;` | +| `run.start` returns only at terminal/parked | `kernel/.../server.rs:160-164` | ✅ `engine.start(...)` is synchronous inside the request | +| Dispatch returns `Parked` either way | `.../engine/drive.rs:120-138` | ✅ registry gets `waiting_worker` vs `parked` (129-137); wire gets `RunStatus::Parked` unconditionally (138) | +| Kernel already solves this | `.../server/client.rs:98` | ✅ branches on the registry, follows the lease | +| `findParkedStep` requires `Runnable` | `sdk/src/cli/run.ts:197` | ✅ (anchor imprecise — F2 below) | +| `resumeFlow` catch → exit 2 / `run_unavailable` | `sdk/src/cli/run.ts:84-94` | ✅ (anchor `:86` is `exitCode: 2`) | +| `JournalClient` discards the structured code | `sdk/src/journal-client.ts:109` | ✅ exact — `new Error(\`${res.error.code}: ${res.error.message}\`)` | +| exit 2 = "refused before a journal write" | `docs/SURFACE.md:134` | ✅ exact | +| `run.resume` unknown run → `internal_error` | `.../server.rs:166-179`, `:438-448` | ✅ exact | +| closed set is the 7 named codes | `server.rs` | ✅ `bad_request`, `protocol_mismatch`, `invalid_spec`, `unsupported_verb`, `lease_conflict`, `journal_write_failed`, `internal` — no others emitted | +| kernel has no `run_not_found` | — | ✅ `git grep run_not_found f435545 -- kernel/` is empty (the only hit is a *mock* in `sdk/tests/cli.test.ts:471`, which strengthens F4) | +| the F3 test injects no real crash | `sdk/tests/live-kernel.test.ts:270-303` | ✅ separate `--stop-after 1` process asserted `status === 0` and `'interrupted'`; `firstDaemon` spawned at 277 only reads the finished journal before SIGKILL at 285 | +| two 20s deterministic steps are valid | `sdk/src/validate.ts:227-234` | ✅ `timeoutMs` is bounded only by "positive integer" — no cap | + +The F2 failure mode was traced end to end and is real: a dispatched step's +`run.get` state is not `Runnable`, `findParkedStep` returns `undefined`, and +`classifyOutcome` falls to `protocolFailure` (`run.ts:183-185`). + +### 1.3 Every PR and repo fact holds + +- `origin/main` is `173423c` ✅. +- PR #9 `OPEN`, `MERGEABLE` / `CLEAN`, head branch `flow/drive-77b2457-08280058` + ✅ (`gh pr view 9`) — matching the branch named in DoD step 1. +- Checks: `CodeRabbit SUCCESS`, `Devin Review SUCCESS` ✅. +- "A green check is not review signal" holds on the evidence: CodeRabbit's only + comment is a rate-limit notice + run configuration ("**Review limit reached** + … Next included review available in 54 minutes"), and Devin posted nothing. + ✅ — and this is the §3.1 precedent, not a rationalization. +- **Four** Codex inline findings on commit `f435545`, **three P1 + one P2** ✅, + and they map one-to-one onto F1–F4 including priority: + `run.ts:65` "Keep long run requests alive through completion" (P1) → F1; + "Handle dispatched parked outcomes without calling them protocol errors" (P1) + → F2; `live-kernel.test.ts` "Kill the daemon that is actually driving the test + run" (P1) → F3; `run.ts:89` "Reserve run_unavailable for missing resume + targets" (P2) → F4. +- Untriaged at authoring time ✅ — Codex posted 05:23Z, the replies are 06:43Z + (= 02:43 EDT), 19 minutes after `ops/NEXT.md` was written. +- 6 commits behind `origin/main` ✅ — `git rev-list --count f435545..origin/main` + = 6, and the six SHAs listed are exactly right and in order. +- No PR #9 adversarial transcript ✅ — `ops/reviews/` holds 72 files on `main` + and 71 at `f435545`; newest is `20260828-0127-cloud-execution.md`; nothing + greps for PR #9 / WP-10 / live-kernel. The §3.3 disqualification stands. +- "PR #9 claims 73 kernel (a new `spec_parity` case)" ✅ — the branch's own + DRIVE-LOG shows `tests/spec_parity.rs` at **4 passed** where `main` reproduces + **3**; 18+0+19+26+4+6 = 73. Precise and correct. +- PR #8 merged at `9e1d9eb` ✅; gate 1 GREEN on the scoreboard ✅; gate 6 marked + "next up" ✅. +- `main`'s DRIVE-LOG ends on WP-9; the WP-10 entry exists only on the PR branch + (line 1990 at `f435545`, absent on `main`) ✅. +- `ops/DIRECTIVES.md` carries no standing directive ✅ (preamble only). +- Out-of-scope enumeration ✅ — every named item exists in `ops/BACKLOG.md`, and + the PR #8 carry-over is exactly **eight** findings (F1, F2, F3, F5, F6, F8b, + F9, F10). The cloud-sandbox gaps are the section filed at `615f97d`. +- Both quotations are verbatim: RUN-CONTRACT §2's merge grant, and + `charter/LEAD.md`'s "**You never merge.** You open PRs and report." ✅ The + flagged contradiction between them is real, correctly characterized as + non-blocking, and correctly *not* resolved unilaterally. + +### 1.4 Standards and rails + +- **Scope:** one ops document. No gate edited (`AGENTS.md` rail 2), no RFC or + DIRECTIVES edited, no `main` commit (rail 1), no product surface added. +- **RFC vocabulary:** `deterministic` / `llm` / `agent` and `completionReason` + used correctly throughout (`AGENTS.md` §7). +- **Fail closed:** F2's fix explicitly forbids reading the sqlite registry from + TypeScript, citing `AGENTS.md` §3 (the journal protocol is the boundary). + That is the right call and the hardest one to get right here. +- **Calibration:** the F3 paragraph volunteers that the underlying property is + already covered kernel-side by `sigkill_under_serve_resumes_the_socket_started_run` + and instructs the implementer not to claim a kernel regression. That is + `AGENTS.md` §4 ("prefer a smaller true claim") applied against the author's + own interest. + +## 2. Findings + +### F1 — RFC citation overstates its mandate for `run_not_found` (substantive, non-blocking) + +`ops/NEXT.md:127-131` justifies introducing a typed `run_not_found` on the +grounds that "gate 1's done-when requires the failure taxonomy to be closed and +to 'never [terminate in] a raw error,' and `internal` for a missing run is +exactly the raw error it forbids." + +`docs/RFC-0001-everything-is-a-relayflow.md:100` scopes that clause to the +journal: "the failure taxonomy is closed — every failed **run's journal** +terminates in a declared failure kind, never a raw error." The `run.resume` +protocol error set (`kernel/relayflowd/src/server.rs:433-448`) is a different +taxonomy, and a `run.resume` for a nonexistent run never produces a journal +termination at all — there is no run to terminate. + +The change may well be correct under `AGENTS.md` §4 (fail closed, no silent +fallbacks) and it is honestly ranked P2, but the RFC does not compel it. An +implementer reading this as an RFC mandate would treat a discretionary kernel +change as obligatory. State the justification on the fail-closed rail instead. + +### F2 — F2's prescribed fix rests on a mischaracterized wire value (substantive, non-blocking) + +`ops/NEXT.md:88-91`: "`RunGetResult.steps` is already `Record` +and carries `Running`, so this is expressible over the existing protocol with +no wire change." + +The conclusion is right; the premise as written is not. `snapshot_from_state` +serializes step state with `format!("{:?}", runtime.state)` +(`kernel/relayflowd/src/engine/model.rs:62`), and `StepState::Running` is a +*struct* variant carrying `attempt`, `lease_deadline_ms` and `idempotency_key` +(`kernel/relayflowd-core/src/state.rs:23-27`). The wire value is therefore +`Running { attempt: 1, lease_deadline_ms: …, idempotency_key: "…" }`, never the +bare string `"Running"`. An implementer mirroring the adjacent +`snapshot.steps[step.id] === 'Runnable'` check would write `=== 'Running'`, +which silently never matches — reproducing the exact false-negative F2 exists +to fix, but harder to see. + +This is confirmed empirically: the repair that followed had to write +`state === 'Running' || state?.startsWith('Running {')` (`c83a367`, +`sdk/src/cli/run.ts`). The document should have said so. + +### F3 — `sdk/tests/live-kernel.test.ts:281` is the wrong anchor (minor) + +`ops/NEXT.md:94`: "`sdk/tests/live-kernel.test.ts:281` names itself 'surface +resume after a real daemon kill'". Line 281 is `beforeClient.close();`. The +`describe` bearing that title is at **line 248**, the `it` at 249; Codex +anchored the same finding at 338. The described behavior reproduces exactly +(§1.2), so this is an anchor slip, not a fabrication — but `AGENTS.md` §3 +("cite paths that exist; a wrong one reads as fabrication even when the work is +real") applies to line anchors a reviewer will actually open. + +### F4 — `sdk/src/cli/run.ts:168` is the wrong anchor (minor) + +`ops/NEXT.md:78` attributes the `Runnable` lookup to `run.ts:168`. Line 168 is +`if (parkedStep !== undefined) {`; the call is at 167, the function at 188, and +the `=== 'Runnable'` predicate at 197. Same class as F3. + +### F5 — the kernel test block is a filtered rendering presented as literal output (minor) + +`ops/NEXT.md:29-38` prints the block under a `$ (cd kernel && ../ops/cargo.sh +test --workspace)` prompt. Re-running that exact command on the reviewed tree +emits **nine** `test result:` lines (18, 0, 19, 26, 3, 6, 0, 0, 0); the document +shows six, and truncates each with `...`. Nothing is overstated — the total is +72 either way and reproduces — but `AGENTS.md` is explicit: "Not a summary of +the output — the output." The per-line `...` marks elision; the three dropped +lines do not. Paste the full block or say it is filtered. + +### F6 — DoD step 1 leaves the worktree boundary ambiguous (minor) + +`ops/NEXT.md:153-156` instructs the next tick to rebase "in a scratch worktree — +RUN-CONTRACT §4: the tick owns its checkout and all other repo operations go +elsewhere." That paraphrase matches §4 as it stands in *this* checkout +(`615f97d`). §4 on `origin/main` — `173423c`, the very commit the document names +as the rebase target — was rewritten to read: "**The tick owns the primary +checkout; I stay out of it.** A tick's `sync` does `git checkout main`, so ticks +MUST run from `~/Projects/AgentWorkforce/flows` — do not 'fix' this by launching +a tick from a worktree (tried 2026-08-28; `sync` fails with `'main' is already +used by worktree`)." + +The paraphrase is still defensible (the rebase is a repo operation, not a tick +launch), but DoD steps 3–8 (build, test, clippy, fmt, `git status --porcelain`) +never say *where* they run relative to that scratch worktree. That is precisely +the ambiguity `173423c` was authored to remove, and it is load-bearing: build +in the worktree, verify in the checkout, and the two heads differ. + +## 3. What I could not refute + +I attacked the evidence first, on the theory that this repo's recurring failure +mode is a true change wrapped in a claim that does not reproduce. It did not +work here. Both suites reproduce to the exact file, test, and per-crate counts. +All fourteen code citations resolve to code that says what the document says it +says. All four Codex findings exist, at the named commit, with the named +priorities, and were genuinely untriaged when this was written. The +"no adversarial transcript" claim — the one that would have been easiest to +assert loosely — is verifiable against two separate trees and is exactly right. +The out-of-scope list is enumerated against a backlog that actually contains +every item, at the right count. + +Two things I specifically expected to be soft, and were not: the +"6 commits behind" list is correct commit-for-commit and in order, and the +"PR #9 claims 73 kernel (a new `spec_parity` case)" attribution is correct down +to which crate's count moved. That is the difference between reading the branch +and paraphrasing its report. + +The self-flagged charter/RUN-CONTRACT contradiction and the F3 calibration note +are both cases where the document volunteers something against its own +convenience. That is the behavior `AGENTS.md` is trying to buy. + +## 4. Verdict rationale + +Six findings, none blocking. F1 and F2 are substantive and should be corrected +before the next tick executes — F2 in particular would cost an implementer a +debugging cycle, and F1 mislabels a discretionary kernel change as +RFC-mandated. But neither is a scope violation and neither makes a false claim +about work performed: both are errors in *forward-looking prescription*, in a +document whose backward-looking claims all reproduce. + +F3–F6 are hygiene: two anchor slips, one filtered-output presentation, one +ambiguity in an executable DoD. + +The diff stays in scope (one ops document; no gate, RFC, directive, product +surface, or `main` commit), applies the right rail for the right reason (no new +work over unfinished work), and — the thing that matters most under this repo's +standard — makes no verification claim that fails to reproduce. + +Recommend correcting F1 and F2 in place before the implementation tick begins. +They do not gate this assessment. + +**Reviewed:** working tree at `615f97d` (`ops/NEXT.md`, +221/−65). + +REVIEW_PASSED From 2da6a92b785da2341e443d927ff331e8c5b496e7 Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Fri, 28 Aug 2026 03:04:55 -0400 Subject: [PATCH 2/2] =?UTF-8?q?drive:=20WP-11=20tick=20log=20=E2=80=94=20a?= =?UTF-8?q?ssessment=20gated,=20PR=20#9=20short=20of=20the=20bar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tick's own product was the WP-11 assessment and its adversarial gate, not the repair; the repair ran on PR #9's branch and its evidence stays there. Verify re-executed on this head (72 kernel / 131 SDK, all nine test-result lines unfiltered) proves this head clean and nothing about PR #9's 73/147 claim. Recorded against this tick: the review landed 15 minutes after the implementation it was meant to steer. Recorded against PR #9: its transcript names c83a367, not head 3616c0a; the branch is one commit behind main; and the live suite has not run on a merge candidate. Co-Authored-By: Claude Opus 5 --- ops/DRIVE-LOG.md | 188 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) diff --git a/ops/DRIVE-LOG.md b/ops/DRIVE-LOG.md index 2e4983063..3de068fca 100644 --- a/ops/DRIVE-LOG.md +++ b/ops/DRIVE-LOG.md @@ -1986,3 +1986,191 @@ $ find kernel -name '*.rs' -not -path '*/target/*' | xargs wc -l | sort -nr | he PR #8 remains open. Gate 1 remains AMBER until a human merges it and re-verifies merged `main`; the Lead does not merge. + +## 2026-08-28 03:03 EDT — WP-11: assess PR #9, gate the assessment, log the tick (`flow/drive-615f97d-08280219`, base `615f97d`, head `d2e7472`) + +### Work package + +WP-11 as this tick executed it was **not** the repair itself. It was the +assessment that names the repair and the gate on that assessment: + +- `ops/NEXT.md` rewritten (+221 / −65) from the stale WP-9 / PR #8 handoff to + "repair PR #9 under review before anything else" — the four Codex findings + (F1 30s `run.start` timeout, F2 dispatched-park misreported as + `protocol_error`, F3 crash test that injects no crash, F4 `run_unavailable` + asserted about errors it cannot see), each re-derived against the code at + PR #9's then-head `f435545` rather than accepted from the bot. +- `ops/reviews/20260828-0258-review.md` — the adversarial transcript for that + assessment. +- Committed as `d2e7472`, opened as PR #11. + +The **repair of PR #9 itself ran on PR #9's own branch** +(`flow/drive-77b2457-08280058`), as `c83a367` (02:43 EDT) and `3616c0a` +(02:49 EDT). Its evidence lives in that branch's `ops/DRIVE-LOG.md` under +"WP-11 — repair PR #9 findings F1–F4 on rebased head." This tick did not +produce that evidence and does not claim it. + +### Verify — re-executed in this tick, on this tick's head `d2e7472` + +This head is documentation-only relative to `615f97d`; the expected result is +that both suites reproduce unchanged, and they do. Verbatim, no filtering — +all nine kernel `test result:` lines are shown (the previous entry's elision +was flagged as F5 in this tick's review): + +```text +$ (cd kernel && ../ops/cargo.sh test --workspace) +test result: ok. 18 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.54s +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s +test result: ok. 19 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.81s +test result: ok. 26 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s +test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s +test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s +EXIT=0 +``` + +72 kernel tests (18 + 0 + 19 + 26 + 3 + 6, plus three empty doc-test targets). + +```text +$ (cd kernel && ../ops/cargo.sh clippy --workspace -- -D warnings) + Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.13s +CLIPPY_EXIT=0 + +$ (cd kernel && ../ops/cargo.sh fmt --check) +FMT_EXIT=0 +fmt stdout bytes: 0 stderr bytes: 0 +``` + +```text +$ (cd sdk && npm test) +SDK_EXIT=0 + +> @relayflows/sdk@0.1.0 test +> tsc --noEmit && vitest run + + RUN v2.1.9 /Users/khaliqgant/Projects/AgentWorkforce/flows/sdk + + ✓ tests/preflight.test.ts (12 tests) 7ms + ✓ tests/deterministic-llm.test.ts (5 tests) 20ms + ✓ tests/validate.test.ts (36 tests) 30ms + ✓ tests/hello-deterministic.test.ts (5 tests) 35ms + ✓ tests/journal-client.test.ts (12 tests) 38ms + ✓ tests/spec-parity.test.ts (12 tests) 37ms + ✓ tests/cli.test.ts (42 tests) 594ms + ✓ tests/bin.test.ts (7 tests) 1087ms + + Test Files 8 passed (8) + Tests 131 passed (131) + Start at 03:02:46 + Duration 1.36s (transform 282ms, setup 0ms, collect 887ms, tests 1.85s, environment 1ms, prepare 529ms) +``` + +```text +$ git status --porcelain +(empty) +``` + +**What this verify does and does not prove.** It proves this tick's head is +clean and that the numbers `ops/NEXT.md` pastes reproduce byte-for-byte. It +proves **nothing about PR #9's repair**, which is on a different branch and +claims different totals: 73 kernel (the `spec_parity` target moves 3 → 4) and +147 SDK across 9 files including all six `live-kernel` tests (33400ms). Those +figures are read from `origin/flow/drive-77b2457-08280058`, not re-executed +here. They must be re-run on the merge candidate before #9 is merged. + +### Review verdict — `REVIEW_PASSED` + +`ops/reviews/20260828-0258-review.md`, reviewing the working tree at `615f97d` +(`ops/NEXT.md`, +221 / −65). Six findings, none blocking: + +- **F1** (substantive) — the assessment cited RFC-0001's closed-taxonomy clause + as compelling a typed `run_not_found`. The RFC scopes that clause to a run's + *journal*; `run.resume`'s protocol error set is a different taxonomy. The + change may be right on the fail-closed rail, but it is discretionary, not + RFC-mandated. +- **F2** (substantive) — the assessment claimed `RunGetResult.steps` carries + the bare string `Running`. It does not: `snapshot_from_state` uses + `format!("{:?}")` on a struct variant, so the wire value is + `Running { attempt: …, lease_deadline_ms: …, idempotency_key: … }`. An + implementer copying the adjacent `=== 'Runnable'` check would have written a + comparison that silently never matches. +- **F3–F6** (hygiene) — two imprecise line anchors, the filtered kernel output + noted above, and a DoD that says to rebase in a scratch worktree without + saying where steps 3–8 then run. + +The reviewer's own summary of what it could not break: both suites reproduce +to the exact file and per-crate counts, all fourteen code citations resolve, +all four Codex findings exist at the named commit with the named priorities and +were genuinely untriaged, and the "no adversarial transcript for PR #9" claim +verified against two separate trees. + +**Ordering fault, recorded against this tick.** The review recommends +correcting F1 and F2 "before the implementation tick begins." The +implementation tick had already begun — `c83a367` landed at 02:43, the review +at 02:58. The recommendation arrived after the work it was meant to steer. It +cost nothing this time (the repair independently wrote +`state === 'Running' || state?.startsWith('Running {')`, which is F2's correct +form, and the reviewer confirmed that empirically), but the gate ran behind the +thing it was gating. `ops/NEXT.md` still carries the two wrong justifications +and should be corrected in place before anyone reads it as doctrine. + +### PRs + +- **This tick — PR #11**: — + OPEN, head `d2e747251b15674ef061f539a6b037af18164dec`, `MERGEABLE` / `CLEAN`. + CodeRabbit and Devin both SUCCESS; per RUN-CONTRACT §3.1 that is not review + signal, and the substantive gate is the transcript above. Docs-only diff + (`ops/NEXT.md`, `ops/reviews/`). +- **Under repair — PR #9**: — + OPEN, head `3616c0a521dfdcba41d94139b54bf3915f3891af`, `MERGEABLE` / `CLEAN`. + All four Codex inline comments now have replies naming `c83a367` and the + audit performed (verified through the API at 03:02, not assumed). Its own + adversarial transcript `ops/reviews/20260828-0244-pr9-adversarial.md` ends + `REVIEW_PASSED`. + +### Honest state of the gate + +**Gate 1 — GREEN on `main`, and this tick did not change that.** It closed at +`9e1d9eb` (PR #8, merged by Khaliq) and was verified on a clean worktree off +`origin/main`. Nothing in this tick or in PR #9 touches that basis. What PR #9 +adds is the *CLI surface* over the live kernel, and that surface is still +unmerged. + +Three things are short of the merge bar for PR #9 and are recorded rather than +waved: + +1. **The adversarial transcript reviewed `c83a367`, not the current head + `3616c0a`.** RUN-CONTRACT §3.2/§3.3 wants the verdict at HEAD. `3616c0a` + adds only `ops/DRIVE-LOG.md` and the transcript file itself — evidence, no + product code — so the exposure is small, but the transcript does not name + the SHA that would be merged. +2. **The branch is one commit behind `origin/main`** (`b2535aa`, "verdict picks + the review by filename, not mtime"). GitHub reports `CLEAN`, but §3.5 + requires the bar to be re-verified on the serialized head, and the 73/147 + figures were produced against `173423c`. +3. **The suite that proves the repair has not been re-executed by this tick.** + `live-kernel.test.ts` takes ~33s and hard-fails in `beforeAll` without both + binaries built; a skipped live suite is a failed DoD, not a pass, so it has + to actually run on the merge candidate. + +Gate 6 remains RED and marked "next up" on the scoreboard. It is not next up in +practice: no new work over unfinished work, and PR #9 is unfinished work. + +### Likely next package + +**WP-12 — land PR #9 or say precisely why not.** Rebase +`flow/drive-77b2457-08280058` onto `origin/main` at `b2535aa` in a scratch +worktree; build kernel then SDK (ordering is load-bearing for the live suite); +re-run the full DoD on the rebased head and capture it unfiltered — kernel +`test --workspace`, clippy `-D warnings`, `fmt --check`, and `npm test` with +`live-kernel.test.ts` among the files that **ran**; land an adversarial +transcript naming the rebased SHA; then merge under the §2 grant if every +clause holds, or leave #9 open with the failing clause named. + +Two smaller items ride along and should not be allowed to displace it: +correcting the F1/F2 justifications in `ops/NEXT.md` (PR #11), and the cloud +verify gaps filed at `615f97d` — `ops/cargo.sh` losing its exec bit in the +snapshot and `node_modules` absent — which block unattended cloud ticks and +are still unaddressed.