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".
d795398 to
8929fb0
Compare
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.
|
@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".
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)
|
@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".
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)
|
@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 👍 / 👎.
|
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". |
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.