feat(subagent): link child sessions to parent and support resume (#87)#92
Conversation
- Stamp parent_session_id, parent_session, and subagent_task into new subagent session headers when the parent is session-backed, enabling bidirectional session-tree navigation of delegated work - Add SubagentConfig.SessionID to resume an existing subagent session so follow-up tasks reuse the child's accumulated context; expose it as an optional session_id parameter on the subagent tool - Add SubagentConfig.ParentSessionID override and propagate the previously-unused extension ParentSessionID through extbridge - Add TreeManager.SetParentLink (in-place header rewrite) and FindSessionPathByID (header-only session lookup by UUID) - Document the resume workflow in the tool description, SDK README, extension skill, and docs site Fixes #87
|
Connected to Huly®: KIT-93 |
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds resumable subagent sessions and parent-child session linking. It extends subagent configuration and tool inputs, adds session lookup and session-header rewrite support, wires Kit.Subagent to resume or link sessions, propagates the new fields through extbridge, and updates tests and docs. ChangesResumable subagent sessions
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/session/store.go`:
- Around line 385-386: Wrap the scanner error returned from readSessionHeader so
it follows the same error-handling pattern as extractSessionInfo. Update the
scanner.Err() branch in readSessionHeader to return a contextual fmt.Errorf with
the session-file scanning message and %w wrapping, instead of returning err
directly.
In `@internal/session/tree_manager.go`:
- Around line 315-363: SetParentLink in TreeManager rewrites the session file
non-atomically, so a failed write can truncate the original data and corrupt the
on-disk session. Update the rewrite path to write the new header and entries to
a temporary file first, flush and close it successfully, then atomically replace
the original file with rename/replace semantics. Keep the existing header
rewrite flow in SetParentLink, but ensure tm.file, tm.writer, and tm.filePath
are only switched to the new file after the temp file has been fully written.
In `@pkg/kit/kit.go`:
- Around line 2352-2359: The session resume path lookup in the `cfg.SessionID`
block ignores the error from `os.Getwd()`, so `session.FindSessionPathByID` may
run with an invalid base directory. Update this flow in `kit.go` to check the
`os.Getwd()` result before calling `session.FindSessionPathByID`, and return a
wrapped `fmt.Errorf("context: %w", err)`-style error if it fails so `resumePath`
is only used when `cwd` is valid.
In `@www/pages/advanced/subagents.md`:
- Line 50: The description in subagents.md overstates capabilities by saying
child Kit instances have “full tool access”; update the wording in the subagents
section to reflect that they inherit the parent’s active tools minus subagent,
and that named-agent presets and allowlists may further restrict access. Use the
existing subagents explanation as the anchor and rewrite only that sentence to
be accurate and precise.
In `@www/pages/sessions.md`:
- Around line 22-23: The session documentation wording in the subagents section
overstates the behavior of parent headers; update the sentence so it only claims
parent_session_id, parent_session, and subagent_task are recorded when the
parent is session-backed. Keep the change localized to the affected prose in the
sessions guide and align the phrasing with the actual behavior described by the
subagents feature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18387294-3fd0-4c7d-abe4-6642fca6ae9e
📒 Files selected for processing (15)
internal/core/subagent.gointernal/core/subagent_test.gointernal/extbridge/extbridge.gointernal/extensions/subagent.gointernal/session/parent_link_test.gointernal/session/store.gointernal/session/tree_manager.gopkg/kit/README.mdpkg/kit/kit.gopkg/kit/subagent_session_test.goskills/kit-extensions/SKILL.mdwww/pages/advanced/subagents.mdwww/pages/extensions/capabilities.mdwww/pages/sdk/overview.mdwww/pages/sessions.md
) - Make SetParentLink's header rewrite atomic: write to a temp file and rename over the original so a partial write can never truncate or corrupt the session (Major); extend test to assert no temp file leaks - Check and wrap the os.Getwd error in Kit.Subagent's resume path instead of discarding it (Minor) - Wrap scanner.Err in readSessionHeader per error-handling guidelines (Minor) - Docs: qualify tool-access wording on the subagents page and the parent-header claim in sessions.md to match actual behavior (Minor)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/session/tree_manager.go (1)
371-376: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional:
Sync()the temp file before rename for crash durability.
w.Flush()pushes the buffer to the OS but does not force it to stable storage. If the process/host crashes between rename and the OS flushing the temp file's data blocks, the renamed file can contain the new metadata entry but zero-length data on some filesystems. AddingtmpFile.Sync()beforeClose()(and optionally an fsync of the parent dir) closes this window. The atomicity guarantee against partial-write corruption already holds via rename; this only strengthens durability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/tree_manager.go` around lines 371 - 376, The temp-file rewrite flow in tree_manager.go should strengthen crash durability by syncing the file before rename. In the rewrite path around the existing w.Flush() and tmpFile.Close() handling, call tmpFile.Sync() after flushing buffered writes and before closing, so the temp file’s contents are forced to stable storage before the atomic rename. Keep the current rewriteErr handling in the same rewrite helper logic, and if you choose to harden further, also sync the parent directory after the rename.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/session/tree_manager.go`:
- Around line 371-376: The temp-file rewrite flow in tree_manager.go should
strengthen crash durability by syncing the file before rename. In the rewrite
path around the existing w.Flush() and tmpFile.Close() handling, call
tmpFile.Sync() after flushing buffered writes and before closing, so the temp
file’s contents are forced to stable storage before the atomic rename. Keep the
current rewriteErr handling in the same rewrite helper logic, and if you choose
to harden further, also sync the parent directory after the rename.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9abbabfe-79ca-4802-9462-d34be197eac3
📒 Files selected for processing (6)
internal/session/parent_link_test.gointernal/session/store.gointernal/session/tree_manager.gopkg/kit/kit.gowww/pages/advanced/subagents.mdwww/pages/sessions.md
✅ Files skipped from review due to trivial changes (2)
- www/pages/sessions.md
- www/pages/advanced/subagents.md
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/session/parent_link_test.go
- internal/session/store.go
- pkg/kit/kit.go
- Address CodeRabbit nitpick: w.Flush() reaches the OS cache only; a crash between rename and writeback could leave a renamed-but-empty session file on some filesystems. Sync before close closes the window.
1 participant
Description
Subagent runs currently create disconnected child sessions, and every invocation is one-shot — follow-up questions re-establish context from scratch. This PR implements both halves of #87:
1. Parent-child session linking. When a subagent is spawned from a session-backed parent, the child session's header now records
parent_session_id(parent UUID),parent_session(parent file path), andsubagent_task(the original prompt). Combined with the existingsubagent_session_idtool-response metadata, links are now bidirectional, enabling session-tree navigation of delegated work. A newSubagentConfig.ParentSessionIDallows an explicit override, and the previously-unused extensionParentSessionIDfield is now propagated throughextbridge.2. Resumable subagents.
SubagentConfiggains aSessionIDfield (exposed to the LLM as an optionalsession_idparameter on thesubagenttool). When set, the child Kit opens the existing session instead of starting fresh, so follow-up tasks reuse the subagent's accumulated context:The tool description documents the resume workflow so the model knows to pass
subagent_session_idback.Fixes #87
Type of Change
Checklist
go test -race; only pre-existing environment-dependentactivate_skillfailures remain, reproduced identically on cleanmaster)golangci-lint runreports 0 issues on all touched packagesAdditional Information
New primitives
internal/session/tree_manager.go—TreeManager.SetParentLink(): stamps the parent reference into the header via in-place JSONL rewrite (header is line 1); in-memory sessions update in placeinternal/session/store.go—FindSessionPathByID(): header-only (first-line) session lookup by UUID, cwd session dir first with fallback scan of~/.kit/sessionsPlumbing
pkg/kit/kit.go—SubagentConfig.SessionID/ParentSessionID;Kit.Subagent()resolves resume paths up front (fast-fail on unknown IDs), stamps the parent link on newly created child sessionsinternal/core/subagent.go—session_idtool parameter, threaded throughSubagentSpawnRequest; updated tool descriptioninternal/extensions/subagent.go+internal/extbridge/extbridge.go— extensionSubagentConfig.SessionIDadded;SessionID/ParentSessionIDpropagated into the SDK pathTests —
internal/session/parent_link_test.go(5),pkg/kit/subagent_session_test.go(2), plus 2 ininternal/core/subagent_test.goDocs —
pkg/kit/README.md,skills/kit-extensions/SKILL.md, and docs site (www/pages/advanced/subagents.md— new "Session linking and resuming" section — plussdk/overview.md,extensions/capabilities.md,sessions.md);tome buildpassesBackward compatibility
omitemptyheader fields)SessionID+NoSessionis rejected as mutually exclusive; unknown session IDs fail fast with an actionable errorRelated: #84 (named agents), #86 (background subagent handle) share these code paths.
Summary by CodeRabbit
session_id, returning asubagent_session_idfor follow-up runs.