feat: inherit parent context for sub-agent sessions#632
feat: inherit parent context for sub-agent sessions#632NicholaiVogel wants to merge 1 commit intomainfrom
Conversation
25a6b31 to
c15f71c
Compare
|
Review posted through GitHub's PR review UI for commit |
PR-Reviewer-Ant
left a comment
There was a problem hiding this comment.
Review metadata
- Reviewer: pr-reviewer
- Model:
gpt-5.5 - Commit:
c15f71cc
I found one correctness issue in the fallback transcript search path: the SQL parameter order does not match the generated placeholders, so non-FTS searches can fail or return no results even when matching transcripts exist.
Confidence: High [sufficient_diff_evidence, targeted_context_included] - The bug is directly visible in the new searchSessionTranscripts fallback query: placeholders in SELECT come before the WHERE agent_id placeholder, but args pushes LIKE parameters first and agentId afterward. This affects the new /api/sessions/search and session_search MCP behavior added by the PR.
| if (targetSessionKey) { | ||
| parts.push("AND st.session_key = ?"); | ||
| args.push(targetSessionKey); | ||
| } else if (params.currentSessionKey) { |
There was a problem hiding this comment.
The fallback LIKE query binds parameters in the wrong order. The generated SQL starts with ${score} AS rank, which contains the first N ? placeholders, then WHERE st.agent_id = ?; however args is initialized with the LIKE patterns and only then pushes params.agentId. That part is fine for the score placeholders, but after appending AND (${any}), the code pushes the second set of LIKE patterns at the very end, after optional session_key, project, and limit placeholders have already been added. With a session/project filter, those filters receive LIKE values and the final LIKE clauses receive session/project/limit values, so searches can silently miss valid transcript hits. Build the SQL and args in exact placeholder order, or split score/where args so optional filters don't get interleaved incorrectly.
c15f71c to
02788fe
Compare
PR-Reviewer-Ant
left a comment
There was a problem hiding this comment.
Review metadata
- Reviewer: pr-reviewer
- Model:
gpt-5.5 - Commit:
02788fec
I found one OpenCode lifecycle bug: inherited session-start context is queued too late to be injected into the child session's first prompt, which undercuts the PR's main continuity claim for that harness.
Confidence: Medium [sufficient_diff_evidence, missing_runtime_repro] - The OpenCode hook path reads and deletes pending injection before calling ensureSessionStarted, while ensureSessionStarted writes the inherited context back into pendingInject. That ordering is visible in integrations/opencode/plugin/src/index.ts, though I did not run OpenCode to confirm the exact hook timing.
Since my last review:
- Fixed or likely addressed: 1
- Still blocking: 0
- Newly found: 1
- Suppressed due to human rebuttal/out-of-scope: 0
| @@ -205,6 +244,7 @@ export const SignetPlugin: Plugin = async ({ directory, client: oc }) => { | |||
| pendingInject.delete(input.sessionID); | |||
There was a problem hiding this comment.
This posts the per-session session-start request after the hook has already read and deleted pendingInject for input.sessionID, so any inherited parent context returned by ensureSessionStarted() is queued for a later prompt instead of the child session's first prompt. For sub-agent continuity, the first prompt is the important one: the sub-agent starts working before the inherited context is visible. The fix should make ensureSessionStarted(input.sessionID) run before consuming pendingInject, or have it return the inject text directly so this same hook invocation can append it.
02788fe to
6db5688
Compare
PR-Reviewer-Ant
left a comment
There was a problem hiding this comment.
Review metadata
- Reviewer: pr-reviewer
- Model:
gpt-5.5 - Commit:
6db56882
I found one correctness issue in the OpenCode lifecycle path: the newly fetched session-start context for a per-session start is queued after the prompt handler has already consumed pending injections, so the first prompt of a child session misses the inherited parent context.
Confidence: Medium [sufficient_diff_evidence, missing_cross_module_context] - The diff shows prompt.before deleting pendingInject for input.sessionID before calling ensureSessionStarted, while ensureSessionStarted queues the session-start result.inject back into pendingInject. That makes the inherited context unavailable to the prompt currently being handled. I could not inspect the full local repository in this read-only review environment, so the confidence is medium rather than high, but the ordering problem is visible in the changed OpenCode plugin code.
Since my last review:
- Fixed or likely addressed: 1
- Still blocking: 0
- Newly found: 1
- Suppressed due to human rebuttal/out-of-scope: 0
| @@ -205,6 +244,7 @@ export const SignetPlugin: Plugin = async ({ directory, client: oc }) => { | |||
| pendingInject.delete(input.sessionID); | |||
There was a problem hiding this comment.
ensureSessionStarted() can enqueue the per-session session-start injection, including inherited parent context, but this runs after prompt.before has already read and deleted pendingInject for this sessionID. That means a newly created OpenCode child session will not receive the inherited parent context on its first prompt; it only becomes pending for a later prompt. Since this PR’s stated behavior is sub-agent context continuity at session start, the session-start call needs to happen before consuming pending injections, or its returned inject needs to be appended directly to the current output.context path.
Summary
PR Readiness Checklist
INDEX.md+dependencies.yaml)Validation
bun scripts/spec-deps-check.ts./node_modules/.bin/biome check docs/API.md docs/CONFIGURATION.md docs/HARNESSES.md docs/HOOKS.md docs/MCP.md docs/specs/INDEX.md docs/specs/dependencies.yaml docs/specs/approved/sub-agent-context-continuity.md integrations/claude-code/connector/src/index.ts integrations/opencode/plugin/src/index.ts platform/core/src/types.ts platform/daemon/src/hooks.test.ts platform/daemon/src/hooks.ts platform/daemon/src/mcp/tools.test.ts platform/daemon/src/mcp/tools.ts platform/daemon/src/memory-config.ts platform/daemon/src/routes/session-routes.ts platform/daemon/src/subagent-context.ts surfaces/cli/src/commands/hook.test.ts surfaces/cli/src/commands/hook.tsgit diff --check HEAD~1..HEADbun run build:corebun test platform/daemon/src/hooks.test.ts platform/daemon/src/mcp/tools.test.ts surfaces/cli/src/commands/hook.test.tscd integrations/opencode/plugin && bun run typecheck && bun run buildcd integrations/claude-code/connector && bun run typecheck && bun run buildcd platform/daemon && bun run build.tscd surfaces/cli && bun run build:cliNotes
bun run lintstill reports pre-existing package.json formatting issues outside this patch; scoped Biome is clean for the touched files.Closes #315