From 52925c14059f1cb0d60aacc03940afd16df99f1b Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 11 Jul 2026 15:56:20 +0200 Subject: [PATCH] fix(sessions): restrict delegation tools in tracked children --- .changeset/clarify-subsession-waiting.md | 2 +- .../piSessionService.delegationTools.test.ts | 109 ++++++++ .../piSessionService.spawnSession.test.ts | 3 + .../piSessionService.spawnSubsession.test.ts | 14 +- .../sessions/piSessionService.testSupport.ts | 2 + src/server/sessions/piSessionService.ts | 243 +++++++++++++----- src/server/sessions/spawnSessionTool.test.ts | 9 +- src/server/sessions/spawnSessionTool.ts | 4 +- .../sessions/spawnSubsessionTool.test.ts | 25 +- src/server/sessions/spawnSubsessionTool.ts | 26 +- 10 files changed, 341 insertions(+), 96 deletions(-) create mode 100644 src/server/sessions/piSessionService.delegationTools.test.ts diff --git a/.changeset/clarify-subsession-waiting.md b/.changeset/clarify-subsession-waiting.md index cb3dd4c..2ebeec3 100644 --- a/.changeset/clarify-subsession-waiting.md +++ b/.changeset/clarify-subsession-waiting.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Clarify tracked-subsession guidance so agents continue independent work or end their turn instead of polling while child sessions run. +Keep delegation tools available to human-created and independently spawned sessions, remove them from tracked child sessions, and make delegation tool contracts capability-focused. diff --git a/src/server/sessions/piSessionService.delegationTools.test.ts b/src/server/sessions/piSessionService.delegationTools.test.ts new file mode 100644 index 0000000..0e1f0f5 --- /dev/null +++ b/src/server/sessions/piSessionService.delegationTools.test.ts @@ -0,0 +1,109 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createPiWebCustomToolDefinitions, sessionAllowsDelegationTools, type PiSessionManager } from "./piSessionService.js"; +import type { SubsessionToolDeps } from "./spawnSubsessionTool.js"; +import { fakeSessionManager } from "./piSessionService.testSupport.js"; + +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((path) => rm(path, { recursive: true, force: true }))); +}); + +function delegationDeps() { + const spawn = vi.fn(() => Promise.resolve({ sessionId: "independent-1", cwd: "/workspace" })); + const subsessions: SubsessionToolDeps = { + spawn: vi.fn(() => Promise.resolve({ sessionId: "child-1", cwd: "/workspace" })), + list: vi.fn(() => Promise.resolve([])), + check: vi.fn(() => Promise.resolve({ sessionId: "child-1", cwd: "/workspace", status: "idle" as const, finalText: "", messageCount: 0 })), + read: vi.fn(() => Promise.resolve({ sessionId: "child-1", cwd: "/workspace", status: "idle" as const, entries: [], total: 0, matched: 0, start: 0, hasMore: false })), + }; + return { spawn, subsessions }; +} + +function toolNames(definitions: ReturnType): string[] { + return definitions.map((definition) => definition.name); +} + +function manager(id: string, file: string | undefined, entries: readonly unknown[] = []): PiSessionManager { + return fakeSessionManager("/workspace", { + getSessionId: () => id, + getSessionFile: () => file, + getEntries: () => entries, + }); +} + +describe("delegation tool capability boundary", () => { + it("provides every globally enabled delegation tool to unrestricted sessions", () => { + const { spawn, subsessions } = delegationDeps(); + + expect(toolNames(createPiWebCustomToolDefinitions("/workspace", true, spawn, subsessions))).toEqual([ + "edit", + "spawn_session", + "spawn_subsession", + "list_subsessions", + "check_subsession", + "read_subsession", + ]); + }); + + it("continues to honor global delegation feature flags for unrestricted sessions", () => { + const { spawn } = delegationDeps(); + + expect(toolNames(createPiWebCustomToolDefinitions("/workspace", true, spawn))).toEqual(["edit", "spawn_session"]); + expect(toolNames(createPiWebCustomToolDefinitions("/workspace", true))).toEqual(["edit"]); + }); + + it("removes every delegation tool but retains ordinary tools for restricted tracked children", () => { + const { spawn, subsessions } = delegationDeps(); + + expect(toolNames(createPiWebCustomToolDefinitions("/workspace", false, spawn, subsessions))).toEqual(["edit"]); + }); + + it.each(["human-created", "spawn_session-created"])("allows delegation for a %s session without tracked-child provenance", async () => { + const sessionManager = manager("session-1", undefined); + const open = vi.fn(() => { throw new Error("no parent session should be opened"); }); + + await expect(sessionAllowsDelegationTools(sessionManager, { open })).resolves.toBe(true); + expect(open).not.toHaveBeenCalled(); + }); + + it("removes delegation when persisted records verify exact tracked-child provenance", async () => { + const dir = await mkdtemp(join(tmpdir(), "pi-web-delegation-provenance-")); + tempDirs.push(dir); + const parentFile = join(dir, "parent.jsonl"); + const childFile = join(dir, "child.jsonl"); + await writeFile(parentFile, `${JSON.stringify({ type: "session", version: 3, id: "parent-1", timestamp: "2026-01-01T00:00:00.000Z", cwd: "/workspace" })}\n`, "utf8"); + await writeFile(childFile, `${JSON.stringify({ type: "session", version: 3, id: "child-1", timestamp: "2026-01-01T00:00:00.000Z", cwd: "/workspace", parentSession: parentFile })}\n`, "utf8"); + + const childManager = manager("child-1", childFile, [ + { type: "custom", customType: "pi-web.subsession.spawned", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1" } }, + ]); + const parentManager = manager("parent-1", parentFile, [ + { type: "custom", customType: "pi-web.subsession.link", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1", spawnedSessionFile: childFile, cwd: "/workspace" } }, + ]); + + await expect(sessionAllowsDelegationTools(childManager, { open: () => parentManager })).resolves.toBe(false); + }); + + it("does not treat a copied child marker as tracked provenance without an exact reciprocal file link", async () => { + const dir = await mkdtemp(join(tmpdir(), "pi-web-delegation-copy-")); + tempDirs.push(dir); + const parentFile = join(dir, "parent.jsonl"); + const originalChildFile = join(dir, "original-child.jsonl"); + const copiedChildFile = join(dir, "copied-child.jsonl"); + await writeFile(parentFile, `${JSON.stringify({ type: "session", version: 3, id: "parent-1", timestamp: "2026-01-01T00:00:00.000Z", cwd: "/workspace" })}\n`, "utf8"); + await writeFile(copiedChildFile, `${JSON.stringify({ type: "session", version: 3, id: "child-1", timestamp: "2026-01-01T00:00:00.000Z", cwd: "/workspace", parentSession: parentFile })}\n`, "utf8"); + + const copiedChildManager = manager("child-1", copiedChildFile, [ + { type: "custom", customType: "pi-web.subsession.spawned", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1" } }, + ]); + const parentManager = manager("parent-1", parentFile, [ + { type: "custom", customType: "pi-web.subsession.link", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1", spawnedSessionFile: originalChildFile, cwd: "/workspace" } }, + ]); + + await expect(sessionAllowsDelegationTools(copiedChildManager, { open: () => parentManager })).resolves.toBe(true); + }); +}); diff --git a/src/server/sessions/piSessionService.spawnSession.test.ts b/src/server/sessions/piSessionService.spawnSession.test.ts index 8f3e51d..0f1244b 100644 --- a/src/server/sessions/piSessionService.spawnSession.test.ts +++ b/src/server/sessions/piSessionService.spawnSession.test.ts @@ -33,9 +33,11 @@ describe("PiSessionService", () => { const fake = fakeRuntime("spawned-1", { sessionFile: "/tmp/spawned-1.jsonl" }); const model = testModel(); let initialModel: PiAgentSession["model"]; + let delegationToolsEnabled: boolean | undefined; const createAgentRuntime: RuntimeCreator = async (_createRuntime, options) => { await Promise.resolve(); initialModel = options.initialModel; + delegationToolsEnabled = options.delegationToolsEnabled; return fake.runtime; }; const service = new PiSessionService(new CapturingSessionEventHub(), { @@ -48,6 +50,7 @@ describe("PiSessionService", () => { await service.spawnSession({ spawningCwd: "/workspace", prompt: "continue", cwd: "/workspace-feature", model }); expect(initialModel).toBe(model); + expect(delegationToolsEnabled).toBe(true); await service.dispose(); }); diff --git a/src/server/sessions/piSessionService.spawnSubsession.test.ts b/src/server/sessions/piSessionService.spawnSubsession.test.ts index b695529..0ba16fa 100644 --- a/src/server/sessions/piSessionService.spawnSubsession.test.ts +++ b/src/server/sessions/piSessionService.spawnSubsession.test.ts @@ -55,16 +55,18 @@ describe("PiSessionService", () => { await service.dispose(); }); - it("uses the parent session's model as the tracked child's initial model", async () => { + it("uses the parent model and disables delegation before creating the tracked child runtime", async () => { const parent = fakeRuntime("parent-1", { sessionFile: "/tmp/parent-1.jsonl" }); const child = fakeRuntime("child-1", { sessionFile: "/tmp/child-1.jsonl", sessionManager: fakeSessionManager("/workspace-feature") }); const model = testModel(); const initialModels: PiAgentSession["model"][] = []; + const delegationCapabilities: boolean[] = []; const runtimes = [parent.runtime, child.runtime]; let index = 0; const createAgentRuntime: RuntimeCreator = async (_createRuntime, options) => { await Promise.resolve(); initialModels.push(options.initialModel); + delegationCapabilities.push(options.delegationToolsEnabled); const runtime = runtimes[index] ?? child.runtime; index += 1; return runtime; @@ -81,6 +83,7 @@ describe("PiSessionService", () => { await service.spawnSubsession({ spawningCwd: "/workspace", parentSessionId: "parent-1", parentSessionFile: "/tmp/parent-1.jsonl", prompt: "do the slice", cwd: "/workspace-feature", model }); expect(initialModels).toEqual([undefined, model]); + expect(delegationCapabilities).toEqual([true, false]); await service.dispose(); }); @@ -306,19 +309,25 @@ describe("PiSessionService", () => { try { const childManager = fakeSessionManager("/workspace-feature", { + getSessionId: () => "child-1", + getSessionFile: () => childFile, getHeader: () => ({ parentSession: parentFile }), getEntries: () => [{ type: "custom", customType: "pi-web.subsession.spawned", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1" } }], }); const parentManager = fakeSessionManager("/workspace", { + getSessionId: () => "parent-1", + getSessionFile: () => parentFile, getEntries: () => [{ type: "custom", customType: "pi-web.subsession.link", data: { version: 1, spawnedBySessionId: "parent-1", spawnedSessionId: "child-1", spawnedSessionFile: childFile, cwd: "/workspace-feature" } }], }); const child = fakeRuntime("child-1", { sessionFile: childFile, sessionManager: childManager }); const parent = fakeRuntime("parent-1", { sessionFile: parentFile, sessionManager: parentManager }); const runtimes = [child.runtime, parent.runtime]; + const delegationCapabilities: boolean[] = []; let index = 0; const open = vi.fn((path: string) => path === parentFile ? parentManager : childManager); const service = new PiSessionService(new CapturingSessionEventHub(), { - createAgentRuntime: () => { + createAgentRuntime: (_createRuntime, options) => { + delegationCapabilities.push(options.delegationToolsEnabled); const runtime = runtimes[index] ?? parent.runtime; index += 1; return Promise.resolve(runtime); @@ -342,6 +351,7 @@ describe("PiSessionService", () => { expect(parent.calls.sendCustomMessage).toHaveLength(1); expect(parent.calls.sendCustomMessage[0]?.message.content).toContain("Subsession child-1 stopped working"); + expect(delegationCapabilities).toEqual([false, true]); expect(open).toHaveBeenCalledWith(parentFile); await service.dispose(); } finally { diff --git a/src/server/sessions/piSessionService.testSupport.ts b/src/server/sessions/piSessionService.testSupport.ts index bd7f9c0..0281cef 100644 --- a/src/server/sessions/piSessionService.testSupport.ts +++ b/src/server/sessions/piSessionService.testSupport.ts @@ -33,6 +33,8 @@ export interface TestSession extends PiAgentSession { export function fakeSessionManager(cwd = "/workspace", patch: Partial = {}): PiSessionManager { return { getCwd: () => cwd, + getSessionId: () => "session-1", + getSessionFile: () => undefined, getBranch: () => [], getLeafId: () => "leaf-1", ...patch, diff --git a/src/server/sessions/piSessionService.ts b/src/server/sessions/piSessionService.ts index fa5f783..3affafe 100644 --- a/src/server/sessions/piSessionService.ts +++ b/src/server/sessions/piSessionService.ts @@ -104,11 +104,17 @@ interface PersistedChildSubsessionLink { spawnedSessionId: string; } +type SessionCreationProvenance = "tracked-subsession"; + interface StartSessionOptions { parentSession?: string; initialModel?: AgentModel; } +interface InternalStartSessionOptions extends StartSessionOptions { + creationProvenance?: SessionCreationProvenance; +} + function requirePromptText(value: unknown): string { if (typeof value !== "string") throw new Error("Prompt text is required"); return value; @@ -167,6 +173,8 @@ type ModelRegistryInstance = ReturnType; export interface PiSessionManager { getCwd(): string; + getSessionId(): string; + getSessionFile(): string | undefined; getBranch(): unknown[]; getEntries?(): readonly unknown[]; getLeafId(): string | null; @@ -257,18 +265,24 @@ interface CreateAgentRuntimeOptions { cwd: string; agentDir: string; sessionManager: PiSessionManager; + delegationToolsEnabled: boolean; initialModel?: AgentModel; } +type PiWebRuntimeFactoryOptions = Parameters[0] & { + delegationToolsEnabled?: boolean; + initialModel?: AgentModel; +}; + type PiWebCreateAgentSessionRuntimeFactory = ( - options: Parameters[0] & { initialModel?: AgentModel } + options: PiWebRuntimeFactoryOptions ) => ReturnType; type CreateAgentRuntime = (createRuntime: PiWebCreateAgentSessionRuntimeFactory, options: CreateAgentRuntimeOptions) => Promise; function defaultCreateAgentRuntime(createRuntime: PiWebCreateAgentSessionRuntimeFactory, options: CreateAgentRuntimeOptions): Promise { if (!(options.sessionManager instanceof SessionManager)) throw new Error("Default runtime creation requires an SDK SessionManager"); - const runtimeFactory = createRuntimeWithOneShotInitialModel(createRuntime, options.initialModel); + const runtimeFactory = createRuntimeWithOneShotSessionOptions(createRuntime, options.initialModel, options.delegationToolsEnabled); return createAgentSessionRuntime(runtimeFactory, { cwd: options.cwd, agentDir: options.agentDir, @@ -276,30 +290,55 @@ function defaultCreateAgentRuntime(createRuntime: PiWebCreateAgentSessionRuntime }); } -function createRuntimeWithOneShotInitialModel(createRuntime: PiWebCreateAgentSessionRuntimeFactory, initialModel: AgentModel | undefined): CreateAgentSessionRuntimeFactory { - // The inherited model belongs only to the session being spawned. Do not keep - // reapplying it if that runtime later creates/forks/switches sessions itself. +function createRuntimeWithOneShotSessionOptions( + createRuntime: PiWebCreateAgentSessionRuntimeFactory, + initialModel: AgentModel | undefined, + delegationToolsEnabled: boolean, +): CreateAgentSessionRuntimeFactory { + // These inputs belong only to the session being opened. A later runtime + // replacement resolves its own model and delegation capability. let pendingInitialModel = initialModel; + let pendingDelegationToolsEnabled: boolean | undefined = delegationToolsEnabled; return async (options) => { const model = pendingInitialModel; + const toolsEnabled = pendingDelegationToolsEnabled; pendingInitialModel = undefined; + pendingDelegationToolsEnabled = undefined; return createRuntime({ ...options, ...(model === undefined ? {} : { initialModel: model }), + ...(toolsEnabled === undefined ? {} : { delegationToolsEnabled: toolsEnabled }), }); }; } type SpawnSessionFn = (input: SpawnSessionInvocation) => Promise; -function createDefaultRuntimeFactory(authStorage: AuthStorage, modelRegistry: ModelRegistryInstance, spawn?: SpawnSessionFn, subsessions?: SubsessionToolDeps): PiWebCreateAgentSessionRuntimeFactory { - return async ({ cwd, agentDir, sessionManager, sessionStartEvent, initialModel }) => { +export function createPiWebCustomToolDefinitions( + cwd: string, + delegationEnabled: boolean, + spawn?: SpawnSessionFn, + subsessions?: SubsessionToolDeps, +) { + return [ + createPiWebEditToolDefinition(cwd), + ...(delegationEnabled && spawn !== undefined ? [createSpawnSessionToolDefinition(cwd, { spawn })] : []), + ...(delegationEnabled && subsessions !== undefined ? createSubsessionToolDefinitions(cwd, subsessions) : []), + ]; +} + +function createDefaultRuntimeFactory( + authStorage: AuthStorage, + modelRegistry: ModelRegistryInstance, + sessionManagers: Pick, + spawn?: SpawnSessionFn, + subsessions?: SubsessionToolDeps, +): PiWebCreateAgentSessionRuntimeFactory { + return async ({ cwd, agentDir, sessionManager, sessionStartEvent, initialModel, delegationToolsEnabled }) => { const services = await createAgentSessionServices({ cwd, agentDir, authStorage, modelRegistry }); - const customTools = [ - createPiWebEditToolDefinition(cwd), - ...(spawn === undefined ? [] : [createSpawnSessionToolDefinition(cwd, { spawn })]), - ...(subsessions === undefined ? [] : createSubsessionToolDefinitions(cwd, subsessions)), - ]; + const resolvedDelegationToolsEnabled = delegationToolsEnabled + ?? await sessionAllowsDelegationTools(sessionManager, sessionManagers); + const customTools = createPiWebCustomToolDefinitions(cwd, resolvedDelegationToolsEnabled, spawn, subsessions); const result = await createAgentSessionFromServices({ services, sessionManager, @@ -345,17 +384,16 @@ export interface PiSessionServiceDependencies { heartbeatIntervalMs?: number; workspaceActivity?: Pick; /** - * When provided, the `spawn_session` tool is registered on every session, - * letting the LLM start new sessions scoped to its project's workspaces. - * Omit to keep the capability disabled (the tool is never registered). + * When provided, `spawn_session` is available to sessions whose creation + * provenance permits delegation, scoped to the project's workspaces. + * Omit to keep the capability disabled. */ spawnTargets?: SpawnTargetResolver; /** * Beta: when true (and `spawnTargets` is provided), the tracked-subsession - * tools (`spawn_subsession`, `list_subsessions`, `check_subsession`, - * `read_subsession`) are - * registered on every session. Off by default so the capability can ship in - * main without being exposed in releases. + * tools are available to sessions whose creation provenance permits + * delegation. Off by default so the capability can ship in main without + * being exposed in releases. */ subsessionsEnabled?: boolean; /** Structured logger for notable runtime events (e.g. spawns). */ @@ -411,6 +449,7 @@ export class PiSessionService { this.createRuntime = deps.createRuntime ?? createDefaultRuntimeFactory( this.modelRegistry.authStorage, this.modelRegistry, + this.sessionManager, this.spawnTargets === undefined ? undefined : (input) => this.spawnSession(input), !subsessionsActive ? undefined : { spawn: (input) => this.spawnSubsession(input), @@ -531,10 +570,17 @@ export class PiSessionService { } async start(cwd: string, options: StartSessionOptions = {}): Promise { + return this.startSession(cwd, options); + } + + private async startSession(cwd: string, options: InternalStartSessionOptions): Promise { const active = await this.create( this.sessionManager.create(cwd, options.parentSession === undefined ? undefined : { parentSession: options.parentSession }), cwd, - options.initialModel === undefined ? {} : { initialModel: options.initialModel }, + { + ...(options.initialModel === undefined ? {} : { initialModel: options.initialModel }), + ...(options.creationProvenance === undefined ? {} : { creationProvenance: options.creationProvenance }), + }, ); const { session } = active.runtime; const created: ClientSession = { @@ -584,9 +630,10 @@ export class PiSessionService { if (this.spawnTargets === undefined) throw new Error("Spawning sessions is disabled"); const decision = await this.spawnTargets.resolveSpawnTarget(input.spawningCwd, input.cwd); if (!decision.allowed) throw spawnTargetError(decision); - const created = await this.start(decision.cwd, { + const created = await this.startSession(decision.cwd, { ...(input.parentSessionFile === undefined ? {} : { parentSession: input.parentSessionFile }), ...(input.model === undefined ? {} : { initialModel: input.model }), + creationProvenance: "tracked-subsession", }); const parentSessionFile = nonEmptyString(input.parentSessionFile); const link: TrackedSubsessionLink = { @@ -795,53 +842,13 @@ export class PiSessionService { this.registerVerifiedSubsession(link); } - private async verifiedSubsessionLinkFromOpenedChild(session: PiAgentSession): Promise { - // Child markers are only hints; the current child header and reciprocal - // parent custom link must agree on the exact ids and files before relinking. - const entries = session.sessionManager.getEntries?.() ?? session.sessionManager.getBranch(); - let marker: PersistedChildSubsessionLink | undefined; - for (const entry of entries) { - const parsed = parsePersistedChildSubsessionLink(entry); - if (parsed?.spawnedSessionId === session.sessionId) marker = parsed; - } - if (marker === undefined) return undefined; - - const childSessionFile = nonEmptyString(session.sessionFile); - if (childSessionFile === undefined) return undefined; - const childHeader = await readSessionHeaderSummary(childSessionFile); - if (childHeader?.id !== session.sessionId) return undefined; - const parentSessionFile = nonEmptyString(childHeader.parentSession); - if (parentSessionFile === undefined) return undefined; - const parentHeader = await readSessionHeaderSummary(parentSessionFile); - if (parentHeader?.id !== marker.spawnedBySessionId) return undefined; - - const parentLink = this.findReciprocalParentSubsessionLink(parentSessionFile, marker.spawnedBySessionId, session.sessionId, childSessionFile); - if (parentLink === undefined) return undefined; - return { - parentSessionId: marker.spawnedBySessionId, - childSessionId: session.sessionId, - childSessionFile, - parentSessionFile, - cwd: parentLink.cwd ?? session.sessionManager.getCwd(), - }; - } - - private findReciprocalParentSubsessionLink(parentSessionFile: string, parentSessionId: string, childSessionId: string, childSessionFile: string): PersistedParentSubsessionLink | undefined { - let parentManager: PiSessionManager; - try { - parentManager = this.sessionManager.open(parentSessionFile); - } catch { - return undefined; - } - const entries = parentManager.getEntries?.() ?? parentManager.getBranch(); - for (const entry of entries) { - const link = parsePersistedParentSubsessionLink(entry); - if (link === undefined) continue; - if (link.spawnedBySessionId !== parentSessionId || link.spawnedSessionId !== childSessionId) continue; - if (link.spawnedSessionFile === undefined || !sessionPathsEqual(link.spawnedSessionFile, childSessionFile)) continue; - return link; - } - return undefined; + private verifiedSubsessionLinkFromOpenedChild(session: PiAgentSession): Promise { + return verifiedTrackedSubsessionLink(this.sessionManager, { + sessionId: session.sessionId, + sessionFile: session.sessionFile, + sessionManager: session.sessionManager, + cwd: session.sessionManager.getCwd(), + }); } private async getOrOpenTrackedSubsession(sessionId: string): Promise { @@ -898,7 +905,7 @@ export class PiSessionService { const status: SubsessionStatus = this.activities.get(childId)?.phase === "error" ? "error" : "idle"; const finalText = finalAssistantText(historyMessages(session)); const preview = finalText === "" ? "(no output)" : truncateForNotification(finalText); - const text = `Subsession ${childId} stopped working (status: ${status}). Latest output:\n\n${preview}\n\nUse check_subsession with sessionId "${childId}" for its status and latest output, or read_subsession to look through its full transcript.`; + const text = `Subsession ${childId} stopped working (status: ${status}). Latest output:\n\n${preview}\n\nStatus and latest output are available through check_subsession with sessionId "${childId}"; its full transcript is available through read_subsession.`; void this.notifyParentOfSubsession(link.parentSessionId, childId, text); } @@ -1592,11 +1599,18 @@ export class PiSessionService { return undefined; } - private async create(sessionManager: PiSessionManager, cwd: string, options: Pick = {}): Promise> { + private async create( + sessionManager: PiSessionManager, + cwd: string, + options: Pick = {}, + ): Promise> { + const delegationToolsEnabled = options.creationProvenance !== "tracked-subsession" + && await sessionAllowsDelegationTools(sessionManager, this.sessionManager); const runtime = await this.createAgentRuntime(this.createRuntime, { cwd, agentDir: this.agentDir, sessionManager, + delegationToolsEnabled, ...(options.initialModel === undefined ? {} : { initialModel: options.initialModel }), }); await this.bindSessionExtensions(runtime.session); @@ -2095,6 +2109,95 @@ function isDefined(value: T | undefined): value is T { return value !== undefined; } +interface TrackedSubsessionSessionIdentity { + sessionId: string; + sessionFile: string | undefined; + sessionManager: PiSessionManager; + cwd: string; +} + +/** + * Resolve the delegation capability from server-owned, persisted session + * provenance. A copied marker is not enough: the child header and reciprocal + * parent link must identify the exact same session files. + */ +export async function sessionAllowsDelegationTools( + sessionManager: PiSessionManager, + managers: Pick, +): Promise { + const trackedLink = await verifiedTrackedSubsessionLink(managers, { + sessionId: sessionManager.getSessionId(), + sessionFile: sessionManager.getSessionFile(), + sessionManager, + cwd: sessionManager.getCwd(), + }); + return trackedLink === undefined; +} + +async function verifiedTrackedSubsessionLink( + managers: Pick, + session: TrackedSubsessionSessionIdentity, +): Promise { + // Child markers are only hints; the current child header and reciprocal + // parent custom link must agree on the exact ids and files before relinking. + const entries = session.sessionManager.getEntries?.() ?? session.sessionManager.getBranch(); + let marker: PersistedChildSubsessionLink | undefined; + for (const entry of entries) { + const parsed = parsePersistedChildSubsessionLink(entry); + if (parsed?.spawnedSessionId === session.sessionId) marker = parsed; + } + if (marker === undefined) return undefined; + + const childSessionFile = nonEmptyString(session.sessionFile); + if (childSessionFile === undefined) return undefined; + const childHeader = await readSessionHeaderSummary(childSessionFile); + if (childHeader?.id !== session.sessionId) return undefined; + const parentSessionFile = nonEmptyString(childHeader.parentSession); + if (parentSessionFile === undefined) return undefined; + const parentHeader = await readSessionHeaderSummary(parentSessionFile); + if (parentHeader?.id !== marker.spawnedBySessionId) return undefined; + + const parentLink = findReciprocalParentSubsessionLink( + managers, + parentSessionFile, + marker.spawnedBySessionId, + session.sessionId, + childSessionFile, + ); + if (parentLink === undefined) return undefined; + return { + parentSessionId: marker.spawnedBySessionId, + childSessionId: session.sessionId, + childSessionFile, + parentSessionFile, + cwd: parentLink.cwd ?? session.cwd, + }; +} + +function findReciprocalParentSubsessionLink( + managers: Pick, + parentSessionFile: string, + parentSessionId: string, + childSessionId: string, + childSessionFile: string, +): PersistedParentSubsessionLink | undefined { + let parentManager: PiSessionManager; + try { + parentManager = managers.open(parentSessionFile); + } catch { + return undefined; + } + const entries = parentManager.getEntries?.() ?? parentManager.getBranch(); + for (const entry of entries) { + const link = parsePersistedParentSubsessionLink(entry); + if (link === undefined) continue; + if (link.spawnedBySessionId !== parentSessionId || link.spawnedSessionId !== childSessionId) continue; + if (link.spawnedSessionFile === undefined || !sessionPathsEqual(link.spawnedSessionFile, childSessionFile)) continue; + return link; + } + return undefined; +} + function trackedSubsessionLinkFromParentLink(parentSessionId: string, link: PersistedParentSubsessionLink, parentSessionFile: string): TrackedSubsessionLink { return { parentSessionId, diff --git a/src/server/sessions/spawnSessionTool.test.ts b/src/server/sessions/spawnSessionTool.test.ts index ea19549..3c23363 100644 --- a/src/server/sessions/spawnSessionTool.test.ts +++ b/src/server/sessions/spawnSessionTool.test.ts @@ -17,7 +17,14 @@ describe("createSpawnSessionToolDefinition", () => { expect(spawn).toHaveBeenCalledWith({ spawningCwd: "/repos/a", prompt: "do the thing", cwd: "/repos/a-feature", model: dispatchModel }); expect(result.details).toEqual({ sessionId: "new-1", cwd: "/repos/a-feature" }); - expect(result.content[0]).toMatchObject({ type: "text", text: "Started session new-1 in /repos/a-feature." }); + expect(result.content[0]).toMatchObject({ type: "text", text: "Started independent session new-1 in /repos/a-feature." }); + }); + + it("describes the independent-session capability without workflow policy", () => { + const tool = createSpawnSessionToolDefinition("/repos/a", { spawn: vi.fn() }); + + expect(tool.description).toBe("Start a new independent pi-web session and send it an initial prompt. The session is not tracked by the caller, can be opened by a human, and runs without returning its later output to the caller."); + expect(tool.description).not.toMatch(/use this|continue work|follow a plan|relay/i); }); it("forwards omitted cwd as undefined and omits a missing dispatching model", async () => { diff --git a/src/server/sessions/spawnSessionTool.ts b/src/server/sessions/spawnSessionTool.ts index 9696520..43cbb1f 100644 --- a/src/server/sessions/spawnSessionTool.ts +++ b/src/server/sessions/spawnSessionTool.ts @@ -41,7 +41,7 @@ export function createSpawnSessionToolDefinition(spawningCwd: string, deps: Spaw return defineTool({ name: "spawn_session", label: "Spawn session", - description: "Start a new, independent pi-web session and send it an initial prompt. Use this to dispatch a fresh agent to continue work or follow a plan. The new session runs on its own and a human can interact with it; you do not receive its output.", + description: "Start a new independent pi-web session and send it an initial prompt. The session is not tracked by the caller, can be opened by a human, and runs without returning its later output to the caller.", promptSnippet: "spawn_session: start a new independent session with a first prompt", parameters: SpawnSessionParams, async execute(_toolCallId, params, _signal, _onUpdate, ctx) { @@ -55,7 +55,7 @@ export function createSpawnSessionToolDefinition(spawningCwd: string, deps: Spaw ...(ctx.model === undefined ? {} : { model: ctx.model }), }); return { - content: [{ type: "text", text: `Started session ${result.sessionId} in ${result.cwd}.` }], + content: [{ type: "text", text: `Started independent session ${result.sessionId} in ${result.cwd}.` }], details: result, }; }, diff --git a/src/server/sessions/spawnSubsessionTool.test.ts b/src/server/sessions/spawnSubsessionTool.test.ts index da9f746..c49999b 100644 --- a/src/server/sessions/spawnSubsessionTool.test.ts +++ b/src/server/sessions/spawnSubsessionTool.test.ts @@ -49,20 +49,31 @@ describe("createSubsessionToolDefinitions", () => { model: dispatchModel, }); expect(result.details).toEqual({ sessionId: "child-1", cwd: "/repos/a-feature" }); - expect(firstText(result.content)).toContain("Started subsession child-1"); + expect(firstText(result.content)).toContain("Started tracked subsession child-1"); }); - it("tells the parent to work independently or end its turn instead of polling", async () => { + it("describes tracked dispatch and notification without workflow policy", async () => { const { spawn: spawnTool } = tools({ spawn: vi.fn(() => Promise.resolve({ sessionId: "child-1", cwd: "/repos/a-feature" })), }); - expect(spawnTool.description).toContain("Do not poll or sleep while waiting"); + expect(spawnTool.description).toBe("Start a tracked child session and send it an initial prompt. The call returns after dispatch; the parent is notified when the child stops working and can inspect its status, latest output, and transcript."); - const result = await spawnTool.execute("call-guidance", { prompt: "do it" }, undefined, undefined, ctxFor("parent-1", undefined)); + const result = await spawnTool.execute("call-contract", { prompt: "do it" }, undefined, undefined, ctxFor("parent-1", undefined)); const message = firstText(result.content); - expect(message).toContain("Continue independent work or end this turn if blocked; do not poll"); - expect(message).toContain("You will be resumed when it stops working"); + expect(message).toBe("Started tracked subsession child-1 in /repos/a-feature. The parent will be notified when it stops working."); + expect(`${spawnTool.description}\n${message}`).not.toMatch(/do not poll|continue (?:useful|independent) work|end (?:this|the) turn|relay/i); + }); + + it("keeps all subsession tool descriptions capability-oriented", () => { + const definitions = tools({}); + + expect(definitions.list.description).toBe("List tracked child sessions owned by the calling session, with each child's current status (working, idle, error, or unknown)."); + expect(definitions.check.description).toBe("Return a tracked subsession's current status, message count, and most recent assistant output."); + expect(definitions.read.description).toBe("Return a filtered, paginated transcript of a tracked subsession. Filters select message roles and content kinds, search full message content, optionally include raw tool arguments, and cap or page the returned entries."); + for (const definition of Object.values(definitions)) { + expect(definition.description).not.toMatch(/use this|do not poll|continue working|start narrow|for just the final|relay/i); + } }); it("spawn_subsession omits the inherited model when the dispatching session has no current model", async () => { @@ -100,7 +111,7 @@ describe("createSubsessionToolDefinitions", () => { it("list_subsessions reports an empty state", async () => { const { list: listTool } = tools({ list: vi.fn(() => Promise.resolve([])) }); const result = await listTool.execute("call-3", {}, undefined, undefined, ctxFor("parent-1", undefined)); - expect(result.content[0]).toMatchObject({ type: "text", text: "You have not spawned any subsessions." }); + expect(result.content[0]).toMatchObject({ type: "text", text: "No tracked subsessions." }); }); it("check_subsession scopes by parent and returns the final result", async () => { diff --git a/src/server/sessions/spawnSubsessionTool.ts b/src/server/sessions/spawnSubsessionTool.ts index 6498bf7..1bb0cf1 100644 --- a/src/server/sessions/spawnSubsessionTool.ts +++ b/src/server/sessions/spawnSubsessionTool.ts @@ -78,13 +78,13 @@ const ListSubsessionsParams = Type.Object({}); const CheckSubsessionParams = Type.Object({ sessionId: Type.String({ - description: "Id of a subsession you spawned (as returned by spawn_subsession or list_subsessions).", + description: "Id of a tracked subsession owned by the calling session, as returned by spawn_subsession or list_subsessions.", }), }); const ReadSubsessionParams = Type.Object({ sessionId: Type.String({ - description: "Id of a subsession you spawned (as returned by spawn_subsession or list_subsessions).", + description: "Id of a tracked subsession owned by the calling session, as returned by spawn_subsession or list_subsessions.", }), roles: Type.Optional(Type.Array( Type.Union([Type.Literal("assistant"), Type.Literal("user"), Type.Literal("tool"), Type.Literal("system"), Type.Literal("custom")]), @@ -126,7 +126,7 @@ function renderEntry(entry: TranscriptEntry): string { function clipNotice(part: TranscriptEntry["parts"][number]): string { if ((part.kind === "text" || part.kind === "thinking" || part.kind === "tool_result") && part.truncated !== undefined) { - return ` [+${String(part.truncated.full - part.truncated.shown)} chars truncated; re-read with a larger maxChars]`; + return ` [+${String(part.truncated.full - part.truncated.shown)} chars truncated]`; } return ""; } @@ -153,15 +153,15 @@ function renderTranscript(result: SubsessionReadResult): string { ? "no messages matched your filters" : `no messages in this window (${String(result.matched)} matched outside it)`) : `messages ${String(result.start)}–${String(last.index)} of ${String(result.total)} (${String(result.matched)} matched)`; - const more = result.hasMore ? `\n\nMore matching messages exist earlier; page back with before: ${String(result.start)}.` : ""; + const more = result.hasMore ? `\n\nEarlier matching messages exist before index ${String(result.start)}.` : ""; // Empty entries with matches means the `before` cursor excluded every match // (they all sit at index >= before): the agent paged too far back and should // raise `before` or omit it, not page back further. const body = result.entries.length > 0 ? result.entries.map(renderEntry).join("\n\n") : (result.matched === 0 - ? "(nothing matched; try widening roles/include, dropping search, or raising limit)" - : `(no messages before index ${String(result.start)}; all ${String(result.matched)} matches are later — raise 'before' or omit it)`); + ? "(no messages matched the filters)" + : `(no messages before index ${String(result.start)}; all ${String(result.matched)} matches have later indexes)`); return `Subsession ${result.sessionId} [${result.status}] — ${range}:\n\n${body}${more}`; } @@ -178,7 +178,7 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse const spawnTool = defineTool({ name: "spawn_subsession", label: "Spawn subsession", - description: "Start an asynchronous tracked child session. The call returns after dispatch. When the child becomes idle or errors, a notification starts a new parent turn or queues behind the current one. Do not poll or sleep while waiting: continue useful independent work, or end this turn normally if blocked. Inspect only when immediately actionable.", + description: "Start a tracked child session and send it an initial prompt. The call returns after dispatch; the parent is notified when the child stops working and can inspect its status, latest output, and transcript.", promptSnippet: "spawn_subsession: start a tracked child session you will be notified about", parameters: SpawnSubsessionParams, async execute(_toolCallId, params, _signal, _onUpdate, ctx) { @@ -193,7 +193,7 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse ...(ctx.model === undefined ? {} : { model: ctx.model }), }); return { - content: [{ type: "text", text: `Started subsession ${result.sessionId} in ${result.cwd}. Continue independent work or end this turn if blocked; do not poll. You will be resumed when it stops working.` }], + content: [{ type: "text", text: `Started tracked subsession ${result.sessionId} in ${result.cwd}. The parent will be notified when it stops working.` }], details: result, }; }, @@ -202,7 +202,7 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse const listTool = defineTool({ name: "list_subsessions", label: "List subsessions", - description: "List the tracked subsessions you spawned, with their current status (working, idle, error, or unknown).", + description: "List tracked child sessions owned by the calling session, with each child's current status (working, idle, error, or unknown).", promptSnippet: "list_subsessions: see the tracked child sessions you spawned", parameters: ListSubsessionsParams, async execute(_toolCallId, _params, _signal, _onUpdate, ctx) { @@ -210,8 +210,8 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse const parentSessionFile = ctx.sessionManager.getSessionFile() ?? undefined; const subsessions = await deps.list(parentSessionId, parentSessionFile); const text = subsessions.length === 0 - ? "You have not spawned any subsessions." - : `Your subsessions:\n${subsessions.map(statusLine).join("\n")}`; + ? "No tracked subsessions." + : `Tracked subsessions:\n${subsessions.map(statusLine).join("\n")}`; return { content: [{ type: "text", text }], details: { subsessions } }; }, }); @@ -219,7 +219,7 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse const checkTool = defineTool({ name: "check_subsession", label: "Check subsession", - description: "Quick glance at a subsession you spawned: its current status and most recent assistant output. Use this to react to what a subsession produced. When the summary is not enough, use read_subsession to look through its full transcript.", + description: "Return a tracked subsession's current status, message count, and most recent assistant output.", promptSnippet: "check_subsession: glance at a subsession's status and latest output", parameters: CheckSubsessionParams, async execute(_toolCallId, params, _signal, _onUpdate, ctx) { @@ -237,7 +237,7 @@ export function createSubsessionToolDefinitions(spawningCwd: string, deps: Subse const readTool = defineTool({ name: "read_subsession", label: "Read subsession", - description: "Read through the transcript of a subsession you spawned. Returns its messages filtered and paginated however you ask: choose which roles (assistant, user, tool, system, custom) and content kinds (text, thinking, tool_call, tool_result, image) to include, search for a substring (always over full content), cap each value's length with maxChars (omit for full text; clipped parts are flagged so truncation is never silent), optionally include raw tool args, and page backward with 'before'/'limit'. Start narrow (e.g. assistant text with a small maxChars) and widen the filters, raise maxChars, or page further back if you don't find what you need. For just the final result, use check_subsession instead.", + description: "Return a filtered, paginated transcript of a tracked subsession. Filters select message roles and content kinds, search full message content, optionally include raw tool arguments, and cap or page the returned entries.", promptSnippet: "read_subsession: read through a subsession's transcript with filters", parameters: ReadSubsessionParams, async execute(_toolCallId, params, _signal, _onUpdate, ctx) {