Repository navigation
fix(adapters): count subagent transcript writes as idle-notice activity; name the waited-on tool - #896
Conversation
…ty (#858) A parent blocked on a foreground subagent writes nothing to its own transcript, so the #680 idle detector journaled a false session-idle. - Add the CLIProfile field `subagent_transcript_dir` (appended): None derives from usage_parser (`{stem}/subagents` for claude-jsonl), "" turns it off; an explicit template must use exactly `{stem}`, stay relative, and is refused on hookless profiles. - Walk that tree stat-only and recursively (`_subagent_tree_files` / `_subagent_tree_key`: count, total bytes, newest mtime) and OR its movement into the idle clock's change test. `idle.last_key` stays the parent's key, since it is the #727 scan offset. - A raising walk degrades the tick to parent-only, crumbed as subagent-scan-failed / subagent-scan-recovered.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughIdle tracking now includes profile-declared subagent transcript trees. Claude ChangesSubagent-aware session idle tracking
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Sequence Diagram(s)sequenceDiagram
participant GenericAdapter
participant CLIProfile
participant TranscriptFiles
participant RunJournal
participant TUI
GenericAdapter->>CLIProfile: Resolve subagent_transcript_root
GenericAdapter->>TranscriptFiles: Stat parent and subagent transcripts
TranscriptFiles-->>GenericAdapter: Activity keys and open tool calls
GenericAdapter->>RunJournal: Record session-idle with in_tool when available
TUI->>RunJournal: Read current session entries
RunJournal-->>TUI: session-idle and in_tool
TUI->>TUI: Display idle duration and tool label
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies both primary changes: counting subagent transcript writes as idle activity and naming the awaited tool. |
| Linked Issues check | |
Issue #858 requires foreground subagent transcript writes to prevent false session-idle notices. The PR adds subagent_transcript_dir, derives {stem}/subagents for claude-jsonl, recursively sta… |
| Out of Scope Changes check | |
The profile field, recursive scan, fault handling, in_tool journal field, TUI label, documentation, and tests all support issue #858 or the idle-notice behavior described by the PR. The available ch… |
| Docstring Coverage | |
Docstring coverage is 82.93% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 9 files. (1 skipped: 1 … |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the transcript trail
And counts each subagent’s tale
A tool name joins the idle line
While busy files keep time just fine
The TUI glows; the clocks align
Comment @coderabbitai help to get the list of available commands.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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: 972f17b1cd
ℹ️ 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".
| elif item.get("type") == "tool_result": | ||
| call_id = item.get("tool_use_id") | ||
| if isinstance(call_id, str): | ||
| open_calls.pop(call_id, None) |
There was a problem hiding this comment.
When a forked Claude subagent copies the parent's still-open tool_use, the parent can later contain its corresponding tool_result while the child retains the copied use. Because files are processed parent-first, this line removes the call while reading the parent, but the subsequently scanned child re-adds the older copied use, so a later idle stretch can incorrectly report in_tool: Agent instead of none. Merge events by timestamp, or prevent an older use from overriding a newer cross-file result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in git rev-parse --short=8 HEAD. A tool_result now closes its call in every file, whatever order the files are read in. The scan collects results across all transcripts first, then decides which calls are still open.
For the record: real fork transcripts do copy the parent's Agent tool_use under the same id and timestamp. Today the child closes that copy itself with a synthetic "Fork started" result about 10ms later, so the stale label could only appear in that window. The fix removes the dependence on read order anyway. Regression test: test_open_tool_call_closes_a_forked_copy_from_the_parent (it fails against the old per-file close).
…le (#858) A forked subagent's transcript copies the parent's open Agent tool_use under the same id. The open-call scan closed calls per file in read order, so a parent-side tool_result could be undone by the child's copy read afterwards. Collect results across all files before deciding which calls are still open.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 283b5be163
ℹ️ 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".
| "transcript's filename stem), with no other field, format spec or " | ||
| f"conversion: got {subdir!r}" | ||
| ) | ||
| if is_absolute_path(subdir) or has_parent_ref(subdir): |
There was a problem hiding this comment.
A profile can set subagent_transcript_dir = "{stem}\u0000/subagents": TOML accepts the escape, and it passes the placeholder, absolute-path, and parent-reference checks here. On the first idle sample, _subagent_tree_files() passes the rendered path to os.scandir(), which raises ValueError: embedded null character; _sample_subagent_tree() catches only OSError, so the exception escapes the wait loop and crashes the run instead of being rejected as a ProfileError. Reject NUL characters at this profile boundary.
AGENTS.md reference: AGENTS.md:L86-L87
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cfe6adf: validate_profile now rejects a NUL in subagent_transcript_dir with a ProfileError before parsing the template. Test: a "{stem}\u0000/x" row in test_malformed_subagent_transcript_dir_is_a_profile_error (ablated: it fails without the check).
| if not path.is_file(): | ||
| continue | ||
| with path.open("rb") as stream: |
There was a problem hiding this comment.
When a transcript path is replaced concurrently after is_file() returns but before open(), the replacement can be a FIFO with no writer, causing this synchronous open("rb") to block indefinitely. Because _open_tool_call() runs inside the session wait loop, that prevents subsequent Stop, window-death, timeout, and hard-stop handling despite the stated FIFO guard. Open nonblocking and validate the resulting descriptor with fstat() so a raced non-regular file degrades through the existing observable fault path.
AGENTS.md reference: AGENTS.md:L86-L87
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cfe6adf: _open_tool_call keeps the is_file() prefilter, then opens with os.open(O_RDONLY | O_NONBLOCK) and skips the file unless fstat says S_ISREG. A FIFO swapped in between neither blocks nor gets read. New test test_open_tool_call_skips_a_fifo_swapped_in_after_the_check (ablated: dropping O_NONBLOCK and dropping the fstat check each fail it).
…#858) Reject a NUL in subagent_transcript_dir at profile load: TOML accepts a \u0000 escape, and the rendered path made the tree walk raise ValueError, which escaped the wait loop instead of degrading like an OSError. Open transcripts in _open_tool_call non-blocking and re-check the opened descriptor with fstat, so a FIFO swapped in after the is_file check neither blocks the wait loop nor gets read.
|
@codex review |
|
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". |
1 participant
Summary
The #680 idle detector stats only the parent transcript. Claude Code writes nothing to the parent
<sid>.jsonlwhile a foreground subagent runs, so a session blocked on a subagent journals a falsesession-idle. Under bmad-loop that is the normal case, becauseclaude.tomlsetsCLAUDE_CODE_DISABLE_BACKGROUND_TASKS=1and every subagent is foreground.This PR does two things:
subagent_transcript_dirprofile field names a subagent transcript tree. A stat-only recursive walk of that tree feeds the idle clock only.session-idleon aclaude-jsonlsession carriesin_tool, the unanswered tool call (for exampleBash). The TUI renders· idle 71m · in Bash.Two corrections to the issue's suggested fix
<sid>/subagents/workflows/wf_<id>/agent-*.jsonl, the reasontokens.tallyalready usesrglobon the same anchor. Sibling*.meta.jsonfiles are not transcripts.idle.last_key. The wait loop'ssample_transcriptreadslast_key[1]as the parent's byte size. That value is the A dev session waiting on an interactive permission prompt is recorded ascrashed, andretryburns the last attempt #727 growth test and the scan offset for_transcript_has_assistant_activity. A combined key would credit subagent bytes as parent growth and scan from a wrong offset. The tree gets its own_IdleTracker.sub_key, OR-ed only into the idle clock's change test.Design
subagent_transcript_dir(CLIProfile, appended field):None(the default) derives the value fromusage_parser:{stem}/subagentsforclaude-jsonl, else off.""turns it off, and any other string is an explicit template.claude.tomlis untouched.{stem}is required (without it, sibling sessions' trees would mask idleness). The value must be relative with no.., and it is refused on hookless profiles.os.scandirstack. It skips symlinks and never opens a file (a FIFO cannot block). A missing root reads as empty, and the walk is uncapped. The key is an aggregate(count, total_bytes, newest_mtime_ns).subagent-scan-failed/subagent-scan-recovered.in_toolonsession-idle:_IDLE_TOOL_SLUGS,mcp(MCP server names stay out of the journal),other,none(nothing open), orunknown(the read raised). Never transcript text. Declared benign in the journal field guard.session-idleappend. A silent transcript holds still, so the state read then holds for the whole stretch: atool_resultlanding moves the file and closes the stretch. The reader reads the parent plus every subagent-tree jsonl and names the newest open call by entry timestamp. A parent's openAgenttherefore loses to its subagent's laterBash, which is the innermost wait.claude-jsonlonly (the tool_use/tool_result shape is Claude's). Every other parser omits the field, so an absent field means "this CLI cannot tell".unknown, nevernone. It is crumbed astool-scan-failed/tool-scan-recovered.trunk checkand a hungdocker runleave identical transcripts (session-end journaled 2h19 after session_timeout_min fires when the session is wedged inside a tool call #157). Suppressing the notice during a tool call would hide exactly the hung-tool case [BUG] Stall detection cannot see a session idling inside a tool call: pane-log re-arm keeps the grace alive throughsleep#680 exists for. The label tells an operator which one to suspect.session-activeand the heartbeat are unchanged.Evidence
A read-only scan of local
~/.claude/projectstranscripts (Claude Code 2.1.273–2.1.296) found 196 foreground agents that would trip a falsesession-idleat the 600 s default. The longest false stretch was 9572 s, with zero parent writes inside any ≥600 s gap. Classifying every ≥600 s silence by the unanswered tool call when the gap began:AgentAskUserQuestion/ExitPlanModeBashSo the waited-on call is recoverable from the transcript, and the remaining gaps get a label.
Residuals
usage_parseraway fromclaude-jsonlmust setsubagent_transcript_diritself, and it gets noin_toollabel.Out of scope
SubagentStart/SubagentStophook latch. "A subagent is open" is not "it is working", and the latch would reopen a completion hazard.Test plan
tests/test_profile.py: derived default, explicit""disables, malformed values refused (each check ablated), hookless refusal, claude's root matches thetokens.tallyanchor.tests/test_generic_tmux.py, subagent fold: direct and workflow-nested writes keep a static parent active; a tree appearing mid-run closes the stretch; a silent tree still opens one stretch; re-point rebaselines the tree; subagent growth is not A dev session waiting on an interactive permission prompt is recorded ascrashed, andretryburns the last attempt #727 work; scan faults crumb once and degrade to the parent; the streak survives a re-point; the fold is off without a layout; a declared dir folds underusage_parser = "none"; final-interval activity closes the stretch; direct_subagent_tree_keywalk rules.tests/test_generic_tmux.py,in_tool: names the open call;nonewhen nothing is open; innermost wait across parent and tree; MCP maps tomcpwith no server name; omitted for gemini/codex/copilot; read faults and tree-walk faults readunknownwith one crumb pair;session-activefields unchanged; the tolerant reader skips torn and garbage lines, skips a FIFO, and propagates a read fault.tests/test_tui_data.py/tests/test_tui_app.py:idle_toolderivation, its close bysession-active, non-string values read as None; header renders· in Bash/· in unknown, and nothing fornoneor an unlabelled stretch.uv run pytest -q -n logical: 14413 passed, 113 skipped.uv run pyright: 0 errors.trunk check: clean.Closes #858
Summary by CodeRabbit