feat(specialist): add exec metadata flags#104
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe PR adds CLI flags and wiring so specialist/session metadata (--calling-session-id, --calling-tool-use-id, --tag, --depth, --session-title, --init-session-id) are parsed, validated, used for session creation/title, and passed through to agent execution with tests and updated help text. ChangesSpecialist Session Metadata Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
internal/cli/exec.go (1)
167-170: ⚡ Quick winUse one computed session title value across session prep and agent metadata.
Right now session creation uses
execSessionTitle(...), while agent metadata uses rawoptions.sessionTitle. When no explicit title is provided, those diverge (session has generated title; agent metadata is empty).Suggested fix
prompt, err := resolveExecPrompt(options, workspaceRoot, deps.stdin) if err != nil { return writeExecFormatUsageError(stdout, stderr, options.outputFormat, err.Error()) } + sessionTitle := execSessionTitle(options, prompt) overrides := config.Overrides{} @@ if shouldUseExecSession(options) { preparedSession, err = sessions.PrepareExec(sessions.PrepareExecOptions{ SessionID: options.initSessionID, - Title: execSessionTitle(options, prompt), + Title: sessionTitle, Cwd: workspaceRoot, @@ result, err := agent.Run(context.Background(), agentPrompt, provider, agent.Options{ @@ - SessionTitle: options.sessionTitle, + SessionTitle: sessionTitle,Also applies to: 211-217
🤖 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/cli/exec.go` around lines 167 - 170, Compute the session title once and reuse it for both session preparation and agent metadata: call execSessionTitle(...) once into a local variable (e.g., computedTitle) before invoking sessions.PrepareExec and then pass that same computedTitle into PrepareExecOptions.Title and wherever agent metadata currently reads options.sessionTitle; update the other occurrence (the similar block around the 211-217 region) to use the same computedTitle so generated and explicit titles never diverge.
🤖 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/cli/exec_parse.go`:
- Around line 374-377: The invalid-format error message returned by
parseExecOutputFormat omits the accepted "debug" alias; update the error text in
the default branch (where execOutputStreamJSON and "debug" are handled) to list
"debug" alongside text, json, and stream-json so the hint matches actual
accepted values (refer to parseExecOutputFormat, execOutputStreamJSON and the
"debug" case).
---
Nitpick comments:
In `@internal/cli/exec.go`:
- Around line 167-170: Compute the session title once and reuse it for both
session preparation and agent metadata: call execSessionTitle(...) once into a
local variable (e.g., computedTitle) before invoking sessions.PrepareExec and
then pass that same computedTitle into PrepareExecOptions.Title and wherever
agent metadata currently reads options.sessionTitle; update the other occurrence
(the similar block around the 211-217 region) to use the same computedTitle so
generated and explicit titles never diverge.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ac3420cf-880e-4fea-81d5-f0cf66622ae1
📒 Files selected for processing (6)
internal/agent/types.gointernal/cli/app.gointernal/cli/exec.gointernal/cli/exec_parse.gointernal/cli/exec_sessions.gointernal/cli/exec_test.go
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Review — clean, approving with a couple of notes
Nice, focused PR — the flag parsing is consistent (both --flag value and --flag=value forms, shared requiredInlineFlagValue/nextFlagValue helpers), validation is solid (--depth rejects non-int/negative, --init-session-id validated via sessions.ValidSessionID, sensible conflict checks), execSessionTitle/shouldUseExecSession are correct, and it's well covered by TestParseExecSpecialistMetadataFlags / ...RejectsInvalidValues / TestRunExecUsesInitSessionIDAndSessionTitle. Build + go test ./internal/cli ./internal/agent green. Approving.
Non-blocking notes
-
debugalias missing from the error hint (exec_parse.go:377, as CodeRabbit noted) —parseExecOutputFormatacceptsdebugbut the invalid-format error still says "Expected text, json, or stream-json." One-word fix: add, or debugso the guidance matches behavior. -
⚠️ agent.Options.Depthcoordination — heads-up from working in the adjacent area: the upcoming sub-agent (task) runtime also introducesagent.Options.Depthwith the same meaning ("sub-agent nesting level; child = Depth+1; refuse at/above maxTaskDepth"). This PR addsDepth(plusCallingSessionID/CallingToolUseID) as exec metadata. They're the same concept — worth syncing with whoever owns the task-runtime slice so you converge on oneDepthfield with agreed semantics, otherwise the second PR to land hits a duplicate-field/merge collision. Same spirit for the other calling-* metadata. -
Plumbed-but-not-yet-consumed —
CallingSessionID,CallingToolUseID,Tag,SessionTitle,Depthare set onagent.Optionsbut nothing ininternal/agentreads them yet (matches your "later slices" note). Fine as staging; just flagging so it's a conscious choice — ideally these land with their consumer (or keep the doc comment so they're not mistaken for dead fields).
None of these block — they're a one-liner + coordination. 👍
🤖 Verified review via Claude Code.
2 participants
Summary:
Scope:
Tests:
Summary by CodeRabbit
New Features
--calling-session-id,--calling-tool-use-id,--tag,--depth,--session-title, and--init-session-idtozero exec; session titles can be set via--session-title.--output-format debugadded as an alias for JSON streaming.Documentation
zero exechelp text to document the new flags and aliases.Bug Fixes / Validation
--depth(non-negative) and--init-session-idusage (only when creating/forking).Tests