Uh oh!
There was an error while loading. Please reload this page.
Fix background task liveness probe after session resume - #215
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d795398521
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
d795398 to
8929fb0CompareFix A - TaskStop completion recognition: list_pending_background_task_ids now treats a TaskStop tool_result (.toolUseResult.message "Successfully stopped task: <id>") as a terminal completion event, because many Claude Code builds do not also emit a task_notification system record for a TaskStop. The id is read from .task_id, falling back to parsing it out of the message text when that field is absent. The message field is coerced with tostring before contains() so unrelated tool results carrying a structured (object/array/number) .message do not error and wipe the whole completed set under pipefail. Fix B - cross-session liveness glob: is_bg_task_alive now searches sibling session dirs under the same project slug when neither the transcript-recorded output path nor the session-derived path exists. This only expands where we look, so a genuinely alive task is never wrongly pruned; a file missing everywhere still fails open as before. Builds on the extract_bg_task_output_path_from_transcript helper landed in PolyArch#215. Tests: 51/51. Renames existing AC-NN cases to descriptive names and adds regression coverage for TaskStop completion, the cross-session glob, and non-string .message poisoning.
zevorn
commented
Jul 7, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8929fb042c
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
The path extractor required the trailing 'You will be notified when it completes.' sentence. When a Claude Code launch record omits it, the regex returned no match, so is_bg_task_alive fell back to the derived current-session path and dead/orphaned tasks stayed pending forever on session resume. Bound the match by the closing JSON string quote ([^"]*) instead of the suffix sentence, and strip the optional suffix afterwards. Add AC-25c covering the suffix-omitted launch form. Discussion: PolyArch#215 (comment)
zevorn
commented
Jul 14, 2026
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b079f84ea8
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
The path extractor used grep -F "$task_id", a substring match. Probing "bash_1" also matched an earlier "bash_10" launch line, so head -n1 selected bash_10's output path; lsof then saw a dead file and pruned the still-running bash_1, letting the stop hook reach Codex before bash_1 finished. Anchor the match on the exact backgroundTaskId JSON value by wrapping the id in literal double quotes: "bash_1" cannot match "bash_10", and the output path never contains a double quote so the boundary cannot fall inside a path value. Add AC-25d (selective lsof mock: bash_1.output alive, others dead; bash_10 launched before bash_1) asserting the alive bash_1 still short-circuits. Discussion: PolyArch#215 (comment)
zevorn
commented
Jul 15, 2026
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:1034c93ad2
ℹ️ 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".
| match=$(grep -F "\"$task_id\"" "$transcript_path" 2>/dev/null \ | ||
| | grep -oE 'Output is being written to: [^"]*' \ | ||
| | head -n1) || true |
There was a problem hiding this comment.
Decode JSON before using recorded output paths
When the recorded output path contains JSON-escaped characters, such as a project slug with a literal backslash, this pipeline reads the raw JSONL text and returns the escaped spelling (for example /tmp/.../foo\\bar/...) rather than the actual filesystem path (/tmp/.../foo\bar/...). [[ -f "$output_file" ]] then fails and the dead Bash task is treated as still running forever; this also regresses same-session probes that previously used the correctly derived path. Extract the text via jq/JSON decoding before passing the path to lsof.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Root cause: extract_bg_task_output_path_from_transcript read the raw JSONL line and regex-extracted the path with grep -oE 'Output is being written to: [^"]*'. The path lives inside a JSON string value, so any character JSON must escape comes through in its escaped spelling — a literal backslash in the path is serialized as "\". [[ -f "$output_file" ]] then checked the doubled-backslash path, missed the real file, is_bg_task_alive failed open, and the dead task was treated as alive forever. The stop hook never reached Codex, and same-session probes that previously used the correctly derived path regressed too, as you noted.
Fix: keep the quote-anchored grep -F ""$task_id"" that selects the launch record by exact backgroundTaskId (so bash_1 is still not confused with bash_10), then decode that record's text field with jq -r '.message.content[]?.content[]? | select(.type == "text") | .text' so the extracted path matches the filesystem. jq is already a hard dependency of every validator hook; when it is absent the function falls back to the derived tasks_dir path (the pre-transcript-extraction behavior), preserving the fail-open contract.
Regression coverage: added AC-25e — a dead task whose recorded output path contains a literal backslash (tasks/with\backslash/...) is pruned via the decoded path. Confirmed the test has teeth: with the old raw-grep extraction AC-25e fails (escaped path → [[ -f ]] fails → treated alive → stop hook short-circuits and never reaches Codex), and with the fix it passes. Full suite 46/46.
@zevorn could you take another look?
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
extract_bg_task_output_path_from_transcript read the raw JSONL text and regex-extracted the path, so JSON-escaped characters came through in their escaped spelling: a literal backslash in the path was doubled to "\\". [[ -f ]] then missed the real file, is_bg_task_alive failed open, and the dead task was treated as alive forever -- the stop hook never reached Codex and same-session probes regressed. Keep the quote-anchored grep that selects the launch record by exact backgroundTaskId, then decode the record's text field with jq -r so the extracted path matches the filesystem. Require jq (already a hard dependency of every validator hook) and fall back to the derived tasks_dir path when it is absent. Regression coverage: AC-25e -- a dead task whose recorded output path contains a literal backslash is pruned via the decoded path. Old raw-grep code fails AC-25e (escaped path -> [[ -f ]] fails -> treated alive); the fix passes. Full suite 46/46.
zevorn
commented
Jul 17, 2026
LGTM! Thanks for the contribution. |
Uh oh!
There was an error while loading. Please reload this page.
Fix A - TaskStop completion recognition: list_pending_background_task_ids now treats a TaskStop tool_result (.toolUseResult.message "Successfully stopped task: <id>") as a terminal completion event, because many Claude Code builds do not also emit a task_notification system record for a TaskStop. The id is read from .task_id, falling back to parsing it out of the message text when that field is absent. The message field is coerced with tostring before contains() so unrelated tool results carrying a structured (object/array/number) .message do not error and wipe the whole completed set under pipefail. Fix B - cross-session liveness glob: is_bg_task_alive now searches sibling session dirs under the same project slug when neither the transcript-recorded output path nor the session-derived path exists. This only expands where we look, so a genuinely alive task is never wrongly pruned; a file missing everywhere still fails open as before. Builds on the extract_bg_task_output_path_from_transcript helper landed in PolyArch#215. Tests: 51/51. Renames existing AC-NN cases to descriptive names and adds regression coverage for TaskStop completion, the cross-session glob, and non-string .message poisoning.
Problem
When a Claude Code session is resumed or continued, the current transcript may have a different session id from the session that originally launched a
background task. The task's
.outputfile physically lives under the old session directory, but the stop hook's liveness probe derived the output pathfrom the current transcript's session id. As a result, dead/orphaned tasks were never pruned and the loop was blocked from reaching Codex review.
Fix
Teach
is_bg_task_aliveto look up the real output file path recorded in the transcript's launch message:extract_bg_task_output_path_from_transcript()to parse the "Output is being written to: " line.tasks_dirpath when the transcript is unreadable or the message is missing.transcript_pathargument throughprune_dead_bg_task_idsandlist_pending_background_task_ids.Test
Add regression test AC-25: simulate a session resume with a new-session transcript that records an old-session output path, then assert the dead task is
pruned using the real path rather than the derived new-session path.
42/42 tests pass.