fix(session-flow): distinguish a completed background continuation from a failed one - #2115
Conversation
…om a failed one find-handoff's background-delivery screening rechecked the continuation's current state before excluding a save-point, but keyed that recheck on `claude agents` presence and collapsed every absence into "keep the candidate, noting the failed background attempt". `claude agents --json` lists ACTIVE sessions only; a completed background session is excluded by the CLI and surfaces only under `--all`, carrying a `state` (observed: done, stopped) where a live one carries a `status` (observed: idle, busy). Verified this session against `claude agents --help` and a live `--json --all` sample; it is the same contract claude-ops' lane-launcher.sh (load_sessions) already relies on. A finished continuation therefore looked identical to a dead one: the ladder surfaced its save-point as a lost handoff labelled a failed attempt, inviting the operator to redo completed work and letting a recent completed continuation bury the older manual handoff they were actually looking for. The recheck now reads `claude agents --json --all` and resolves four ways instead of two: live (exclude, running), terminal-and-completed (exclude, FINISHED, point at that session's output), terminal-and-not-completed (keep, the restart artifact the recheck exists for), and absent even from --all (UNKNOWN, keep, never called a failure, since the --all history is bounded). Keyed on the launched sessionId where the transcript recorded one; the continue-<topic> slug stays an ambiguous key that routes to UNKNOWN. Stated once at the step-1 screening site and governing every screening site, with the prompt-only site and the Gotchas bullet aligned to it. evals.json case 8's stale expectation is corrected and a case is added for the completed-continuation branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 1m 16s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR is a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow in the diff. Specifically checked for and ruled out:
Nothing in scope for this lane (unpinned actions, dangerous triggers, excessive permissions, static template-injection) applies — there are no workflow files in this diff. |
|
Claude finished @kyle-sexton's task in 4m 54s —— View job Review complete
This PR discharges a stranded review thread from merged PR #1033: Changes are scoped entirely to documentation/config surfaces ( Posted 3 inline nits (all 🟡, non-blocking):
Verification note: the PR body's live-session verification of No correctness bugs or blocking issues found. Branch: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80f4febb12
ℹ️ 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".
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Claude finished @kyle-sexton's task in 51s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR is a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow file in the diff. Specifically checked for and ruled out:
The slug-only session-key ambiguity flagged by Codex ( |
|
Claude finished @kyle-sexton's task in 4m 10s —— View job Code review complete
Scope reviewed ( The fix itself is sound: replacing the old two-way "live vs. everything-else-is-failed" recheck with a four-way resolution (live / terminal-completed / terminal-not-completed / absent-even-from- Finding posted inline
Already-open, unresolved from the prior code-review-lane pass on this same commit (not re-posted to avoid duplicate inline threads, still outstanding)
Verification notes
No blocking issues. One 🟡 correctness nit posted inline; three prior nits remain open from the earlier pass on this same commit. |
Version collision only. Main shipped session-flow 0.22.0 (the bare-alias removal) while this branch held 0.21.4, so this entry takes 0.22.1 and both changelog entries are kept in full — they cover unrelated changes. Verified: no conflict markers remain, plugin.json parses, changelog versions are monotonic with no duplicates, no MD012. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 45s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR remains a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow file anywhere in the diff. Re-checked against the current HEAD (
The slug-only session-key ambiguity flagged in review threads ( |
|
Claude finished @kyle-sexton's task in 3m 12s —— View job Code review complete
Scope reviewed (
Note on the PR body: its version-bump claim (" Still open from the two prior code-review-lane passes on this same content (not re-posted, to avoid duplicate inline threads)
No new correctness, architecture, or maintainability issues found beyond what's already flagged above. The core fix (four-way resolution replacing the old two-way live/failed collapse) is sound and consistently applied across all three screening sites. Verification note: the PR body's live-session verification of |
… which id Two review nits on find-handoff, both still live at this head. The two observed terminal `state` values were listed side by side with nothing saying which one means the continuation finished. That mapping is what the four-way resolution turns on — `done` sends a candidate down the excluded "finished" branch and `stopped` down the kept "restart artifact" branch — and it was only recoverable implicitly, several lines later. Stated inline instead. `sessionId`/`id` is a `claude agents --json` field and collides by sight with the `session_id` this file uses everywhere else for transcript frontmatter. A reader keying the recheck on the wrong one gets no match and reports UNKNOWN. Named the source explicitly at the point of use. Prose only; no branch logic changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 48s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR is a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow file anywhere in the diff. Specifically checked and ruled out:
The slug-only session-key ambiguity flagged in earlier review passes on this same content ( |
|
Claude finished @kyle-sexton's task in 1m 41s —— View job Code review complete
Scope reviewed: the delta since the last review pass on this PR — commit Two open nits from earlier passes are now resolved
Both edits are prose-only, consistent with the rest of the four-way resolution, and don't touch branch logic — confirmed by reading the surrounding paragraph, not just the diff. Still open, not re-posted (avoids duplicate inline threads on unresolved findings)
No new issues found in this delta. Both fixed nits close the gap cleanly and are internally consistent with the surrounding four-way resolution. |
The recheck declared UNKNOWN only when a slug failed to resolve to exactly one session, which reads a unique match as identification. It is not. `--all`'s history is bounded, so the candidate's own session can age out while a newer same-topic continuation remains. The slug then matches exactly one entry and that entry is a DIFFERENT session wearing the same name — and the recheck would report its state as this candidate's, which is the one thing this four-way resolution exists to prevent. Uniqueness at snapshot time is not identity across time. UNKNOWN now holds whenever no `sessionId` was recorded, regardless of match count; only a recorded ID identifies. Prose only; no branch logic changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 49s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR remains a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow file anywhere in the diff. Re-checked against current HEAD (
The slug-only session-key ambiguity that Codex and prior Claude passes flagged on this file (the current-state recheck's fallback for an unrecorded launch ID) is addressed in this diff's new text — the recheck now states slug uniqueness "at snapshot time is not identity across time" and routes slug-only evidence to UNKNOWN regardless of match count. That's a correctness fix to a local, single-user, read-only screening heuristic (a wrong branch would at most cause an agent to under/over-surface its own save-point candidates) — not a security boundary, and out of scope for this security-focused lane either way. |
|
Claude finished @kyle-sexton's task in 2m 40s —— View job Code review complete
Scope reviewed: the delta since the last code-review pass on this PR ( The last open correctness nit is now fixed
All three findings raised on this PR's earlier commits are now resolved:
Still open, not re-posted (avoids a duplicate inline thread on an unchanged block)
No new issues found in this delta. The four-way resolution (live / terminal-completed / Verification note: the PR body's live-session verification of |
…the recheck Uniqueness at snapshot time is not identity across time. `--all`'s history is bounded — the same boundedness the UNKNOWN branch already rests on — so the session that produced a candidate can age out while a newer, unrelated launch reusing the `continue-<topic>` slug is still listed. The slug then resolves uniquely to the wrong session and its live-or-completed state is borrowed for the older candidate: the misattribution this rung exists to prevent, reached from the other direction. Only an exact `sessionId`/`id` match identifies a session now; slug-only evidence is always UNKNOWN, uniqueness notwithstanding. The slug-to-candidate-FILE rule at :119-121 and :475-476 is deliberately untouched. That one asks which file a continuation delivered, where same-topic files from one session all match — a different question from session identity, and correct as written. Adds the two eval scenarios the four-way resolution had no dedicated case for: absent-even-from-`--all` (id 14), which also pins the slug rule above, and terminal-and-not-completed (id 15). Both were previously reachable only as criteria inside another case's expected output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch had already shipped this fix as 2acaeb8 before I wrote my own — I edited from a stale local checkout. Origin's wording wins the conflict: it additionally disambiguates the `claude agents --json` `sessionId`/`id` field from the `session_id` this file uses for transcript frontmatter, which mine did not. What survives from my commit is the part origin does not have: eval cases 14 (absent-from-all is UNKNOWN, which also pins the slug rule) and 15 (terminal-not-completed is the restart artifact). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 54s —— View job Security review complete ✅
Scope reviewed (
FindingsNo security issues found. This PR remains a pure documentation/instruction-prose change to a Claude Code skill, plus matching eval fixtures and a changelog/version bump — there is no executable code, script, hook, or workflow file anywhere in the diff. The delta since the last review pass on this PR (commit Re-checked the full diff against the same criteria as every prior pass on this PR:
The slug-only session-key ambiguity raised by Codex and prior Claude passes on this file is now fixed in the diff's text (identity requires a recorded |
No linked issue
Summary
Discharges an unresolved review thread stranded on merged PR #1033
(
PRRT_kwDOTCGFQM6TybdT, "Distinguish completed agents before retaining handoffs"). Thefind-handoffskill's background-delivery screening could not tell a background continuation thatfinished the work from one that died, so it labelled a completed continuation's save-point a
failed background attempt and surfaced it as the lost handoff.
Fix
The screening rule rechecks a continuation's current state before excluding its save-point — a good
rule, kept. What was wrong is the state read. It keyed on
claude agentspresence and collapsedevery absence into one branch:
claude agents --jsonlists active sessions only. Verified live this session, not from recall:claude agents --help→--all With --json: also include completed background sessionsclaude agents --json --allon this machine returns 13 sessions; terminal ones carrystate(done,stopped) and nostatus, active ones carrystatus(idle,busy) and nostate.plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh(
load_sessions): "claude agents --json(no --all) lists ACTIVE sessions only … completed/terminalbackground sessions are excluded by the CLI (they surface only under --all …)".
So a successfully completed continuation is absent from the bare list and indistinguishable there
from a dead one. The consequence is the reviewer's: the operator is handed a finished
continuation's save-point, invited to redo completed work, and the recent completed candidate can
bury the older manual handoff they were actually looking for.
The recheck now reads
claude agents --json --alland resolves four ways instead of two:status)statereports completionstatedoes not report completion--all--allhistory is boundedKeyed on the launched
sessionId/idwhere the launch-verification listing recorded one; the--name "continue-<topic>"slug stays the ambiguous key it already is at launch time and routes toUNKNOWN. State values are read as reported rather than against a closed set.
Following the file's existing
"Succeeded" means verified-visiblepattern, the resolution is statedonce at the step-1 screening site and declared to govern every screening site; the prompt-only
screening site and the Gotchas bullet reference it instead of restating prose that can drift apart
again.
evals/evals.jsoncase 8 carried the defective doctrine verbatim in its expectations — it iscorrected, and case 13 (
completed-continuation-is-not-a-failed-one) is added for the branch.Note on the finding's pointers: the claim verified, the pointers did not. The thread anchors at
SKILL.md:84, which is the/loopre-arm paragraph, not the recheck rule; andlane-launcher.sh:254-257no longer holds the cited contract — it is now lines 506-510 of that file.The complaint was evaluated on its substance, and reproduced.
Verification
Pre-fix control (the complaint reproduces on the PRE-fix tree). Two fresh-context agents were
given only the pre-fix prose — the step-1 screening block, and the Gotchas bullet separately — plus
the scenario (verified-visible launch naming this exact file; continuation completed successfully;
claude agents --jsonreturns no entry), with no reviewer text and no hint of the expected answer:Continuation absent or failed now → keep the candidate. The onlyexclusion branch is
Continuation still live →… The label isnoting the failed background attempt… No such instruction exists [distinguishing failed from completed] … It doesnot [mention
--all, completed sessions, or terminal state]."the operator may resume it — re-running work that already finished hours ago."
Post-fix (same prompts, swapped text). The step-1 agent now refuses the bare-
--jsonreadingoutright — "Exclusion is only reachable via the Live or Terminal and completed branches,
both of which require a positive presence reading you do not have … re-run the lookup with
--all" — and, on absence from--all, reports "UNKNOWN, never 'failed.' … report that thecontinuation's outcome could not be determined instead of asserting a failed background attempt."
The complaint no longer reproduces.
An adversarial fresh-context verifier was also run against the diff with the rationale withheld and
instructed to refute that the finding is discharged.
scripts/affected-tests.sh --runfor this diff; per-plugin changelog parity satisfied(
session-flow0.21.3→0.21.4with its entry).Related
PRRT_kwDOTCGFQM6TybdT,chatgpt-codex-connector, 2026-07-25) — that PR is already merged, so this lands as a follow-uprather than closing an issue.
plugins/claude-ops/skills/lanes/scripts/lane-launcher.shload_sessions— the in-repo statementof the
claude agents --json/--allcontract this fix conforms to (unchanged here).plugins/session-flow/skills/continue-in-background— the producer whose launches this screeningcorrelates against (unchanged here).