Skip to content

VC-861: Let Ticket parents steer their delegated subagents - #938

Merged
hussainph merged 8 commits into
mainfrom
volli/VC-861-steer-subagents-a-parent-session-can-session-sen
Oct 20, 2026
Merged

hussainph merged 8 commits into
mainfrom
volli/VC-861-steer-subagents-a-parent-session-can-session-sen

Conversation

@hussainph

@hussainph hussainph commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Problem

Ticket Sessions can delegate a helper but cannot redirect it. Merely adding session_send to their bundle would permit project-wide steering: the send operation previously resolved any structured Session in the project.

Fix

  • Offer session_send to newly created Ticket Sessions and pin the updated birth surface baseline.
  • Bind the caller's Role at the tool door. Ticket callers may steer only their own delegated subagents; Board callers retain project-wide steering. Refusals name the rule.
  • Keep existing Ticket membership frozen, including across additive refresh, cold turns, compaction, model switches, reattachment and pre-surface legacy backfill.
  • Teach session_delegate to steer drifting helpers, and document the scope in session_send and CLI help.
  • Preserve steering attribution, opened/joined receipts and delegation reporting. Active delegates report through their existing answer; other targets retain the automatic watch.

Decisions

  • Require both role === subagent and parentSessionId === caller: a Ticket Session started via an own-ticket grant is not a delegated helper.
  • Exclude a newly bundled send from old Ticket refresh candidates; changing only the birth bundle would otherwise grant it on the next cold refresh.
  • Apply the Ticket scope even to an existing explicit session.send grant: a grant does not make a Ticket caller a Board supervisor.
  • Preserve the conservative new-Sessions-only rule on legacy backfill too: a pre-surface Ticket Session gets no new send, while a Board Session keeps its existing send.
  • Qualify delegate steering advice with “if you hold session_send” so it remains accurate for frozen legacy Ticket Sessions without the tool.
  • No migration, protocol/schema change, or change to VC-809: End a completed subagent's shells and stop re-arming its delegation on host-notice turns #935 re-arm/report identity semantics.
  • No local desktop/dev build or live-app claim; verification uses the real runtime/Pi codec with scripted external replies, plus CI, per the ticket's machine limits.

Tests

  • Regression first on origin/main 67d49aa: /tmp/volli-serial.sh packages/host-core pnpm exec vp test run src/agent-tool-door.test.ts --maxWorkers=1 failed the 3 new unauthorized Ticket steering cases (63 passed).
  • Host-core door, supervision, birth baseline and assembly files: 105 passed after the fix, single worker.
  • Shared agent-tool-surface and verb-registry files: 117 passed, single worker.
  • CLI reference: 2 passed; expected help snapshot regenerated.
  • /tmp/volli-serial.sh packages/host-core pnpm exec vp test run src/session-runtime/delegate-browser-cleanup.integration.test.ts --maxWorkers=1: 6 passed. Both Board and Ticket parents steer an active delegated child through the real runtime/Pi adapter/stream codec; the child reads the parent marker, stays in one turn, and the parent receives exactly one delegate-1:answer receipt.
  • Frozen Ticket refresh tests rechecked after merging current main: 2 passed, 14 unrelated tests skipped.
  • Serial vp check on the 14 touched TypeScript files: formatting and lint passed. git diff --check: passed.
  • Local-run disclosure: the initial assembly-file test run also executed its pre-existing refresh benchmark. The integration delegate additionally ran a host-core typecheck (reported exit 0) despite being asked not to run heavy commands. Neither was repeated; all parent-run tests/checks used the mandatory serial runner. No coverage, full suite, or build was run locally.
  • Initial CI run 39269662092: Check + Build, all desktop test/coverage and smoke lanes, all N-1 compatibility lanes, and host-container build passed. Test (packages) caught one stale MCP birth-surface expectation in mcp-surface.test.ts; updated it to include session.send in canonical order. This is a test correction, not a flake rerun.
  • /tmp/volli-serial.sh packages/shared pnpm exec vp test run src/mcp-surface.test.ts --maxWorkers=1: 9 passed. Serial vp check on that file: passed.
  • CI on 27b7e670a (run 39271331013): CI gate green, all required lanes passed. Independent read-only review then identified legacy-backfill and old-surface wording edge cases; addressed before handoff (no change to delegation reporting).
  • Additional regression-first legacy backfill test failed before the guard; /tmp/volli-serial.sh packages/host-core pnpm exec vp test run src/session-runtime/context.test.ts --maxWorkers=1: 15 passed after the guard (both Board and Ticket backfill/reattach covered). Shared surface/registry recheck: 117 passed. Serial vp check on all follow-up touched TypeScript files: passed.
  • Pre-review head b66c492354539b9834b837ce2ddd907b73224bdb, CI run 39272980424: CI gate green. Check + Build, Test (packages), all desktop test/coverage lanes, host-container build, all N-1 compatibility lanes and all smoke lanes passed. No flake reruns requested.

Review

  • SHOULD 1 addressed: session_send skips its automatic watch for the caller's pending delegates; the receipt names the delegation answer instead. Snapshot pending children before delivery so a delegate finishing during the send cannot accidentally arm a second report. Other targets retain their watch; no VC-809: End a completed subagent's shells and stop re-arming its delegation on host-notice turns #935 re-arm/report identifiers changed.
  • The integration now steers through createAgentToolDoor, wired to the real Watch registry, post-commit wake bus, runtime and Pi adapter. It drains the watch coalescing clock and delivery work, then asserts exactly one message notice in the parent's ledger (not merely one delegation-keyed receipt). Regression first: both Board and Ticket cases failed with an extra watch: command beside delegate-1:answer; both now pass.
  • SHOULD 2 addressed: help notes and tool description both qualify steering advice by holding session_send and the helper still working; regenerated CLI reference.
  • Cheap nits addressed: Board/new-Ticket availability wording; removed misleading “only” from Role docs; fail-closed caller-Role predicate with unknown-Role regression; pinned a literal pre-VC-861 headless Ticket surface in the baseline fixture and use it for old-Surface assembly tests rather than deriving it from today's resolver.
  • No review requests declined. Steering an already answered child retains the existing resume/report behavior; the one-answer wording now explicitly concerns a still-working delegation.
  • Review local checks (all via /tmp/volli-serial.sh, one test file per invocation, --maxWorkers=1): door 70 passed, supervision 18 passed, real-runtime integration 6 passed, frozen assembly cases 2 passed / 14 unrelated skipped, registry 78 passed, CLI reference 2 passed. Touched-file formatting/lint and git diff --check passed. No local coverage or build for this review follow-up.
  • Review CI run 39276465356 caught TS2352 on the deliberately partial test double; corrected its explicit test-only assertion. As allowed after a CI type error, one /tmp/volli-serial.sh packages/host-core pnpm exec tsc --noEmit -p tsconfig.json completed successfully; serial formatting/lint on the corrected file passed. The same run's package lane timed out in the unrelated worktree/watch.test.ts five-second retention test; no changes there and no flake rerun requested. All desktop, smoke, host-container and N-1 lanes passed.
  • Corrected review head 214de639d110e08cb7b46c3f5eb1249d58b9b4fb, run 39278520935: Check + Build and all N-1 lanes passed. The bounded 30-minute plain gh pr checks 938 --watch timed out while Test (packages), desktop 3/4 and 4/4, host-container, core smoke and smoke shard 3 remained in progress; coverage and CI gate remain queued. Verification blocked pending CI completion; not claiming review-head CI green. No action reruns or concurrent watchers started.

Overlap

The specified PRs #909, #919, #920, #921 and #922 are already merged. git merge-tree --write-tree is clean against current origin/main and each specified PR head; no stack is required. Target: main.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 1796b947-7171-4dc6-8731-e2c409475cf2
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hussainph

Copy link
Copy Markdown
Owner Author

Merge-readiness review found and fixed an authority edge case in c317f81: after its Ticket is deleted, a Ticket Session attaches with a project-shaped runtime identity. Using that identity for steering would allow project-wide sends. Steering now reads the caller’s durable Session Role, preserving own-subagent-only scope after deletion and refusing missing/out-of-project callers. No change to the orphan attachment behavior or delegation reporting.

Regression-first: both unauthorized orphan-Ticket steering cases failed before the fix. Focused verification: 99 tests passed across agent-tool-door.test.ts, supervise-session.test.ts, and delegate-browser-cleanup.integration.test.ts; host-core typecheck, touched-file vp check, and git diff --check passed. The prior head’s CI gate was green; the new head needs its own CI gate.

@hussainph
hussainph merged commit c07f887 into main Oct 20, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant