From e5608fc0ef5de4bf7df3c863e5c216ea47b4f25f Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Thu, 10 Sep 2026 17:48:42 -0700 Subject: [PATCH] Avoid redundant Pi local-file child sessions --- .../src/bridge/bridge.lifecycle.test.ts | 47 ++++++-- .../src/bridge/bridge.local-file.test.ts | 111 ------------------ .../src/bridge/bridge.round2.test.ts | 22 +++- .../provider-pi/src/bridge/turn-input.test.ts | 20 ++++ 4 files changed, 71 insertions(+), 129 deletions(-) delete mode 100644 plugins/provider-pi/src/bridge/bridge.local-file.test.ts diff --git a/plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts b/plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts index d016ccd9fd..e1a9a76775 100644 --- a/plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts +++ b/plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts @@ -79,19 +79,39 @@ async function startThread(threadId: string): Promise { expect(response.result).toMatchObject({ providerThreadId: threadId }); } -it("stop{release} ends the child", async () => { - await startThread("thr_lc_release"); - await harness.request((nextId += 1), "turn/start", { - threadId: "thr_lc_release", - providerThreadId: "thr_lc_release", +it("stop{release} ends the child after a local-file-only turn", async () => { + const threadId = "thr_lc_release"; + const filePath = join(harness.workspaceDir, "notes.md"); + await startThread(threadId); + const turn = await harness.request((nextId += 1), "turn/start", { + threadId, + providerThreadId: threadId, clientRequestId: "creq_ab23456789", - input: [{ type: "text", text: "hello", mentions: [] }], + input: [ + { + type: "localFile", + path: filePath, + name: "notes.md", + sizeBytes: 6, + mimeType: "text/markdown", + }, + ], options: FULL_PERMISSION_OPTIONS, }); - await harness.waitForTurnBoundary("thr_lc_release", 0); + expect(turn.result).toEqual({ threadId }); + await harness.waitForTurnBoundary(threadId, 0); + expect( + harness + .deltasOf(threadId) + .some( + (delta) => + delta.kind === "item.textDelta" && + String(delta.text).includes(`[Attached file: ${filePath}]`), + ), + ).toBe(true); const stop = await harness.request((nextId += 1), "thread/stop", { - threadId: "thr_lc_release", - providerThreadId: "thr_lc_release", + threadId, + providerThreadId: threadId, intent: "release", activeTurnId: null, }); @@ -238,9 +258,12 @@ it("accepts a prompt containing only a local image", async () => { expect(response.result).toEqual({ threadId }); await harness.waitForTurnBoundary(threadId); expect( - harness.deltasOf(threadId).some( - (delta) => delta.kind === "item.textDelta" && delta.text === "Response to: ", - ), + harness + .deltasOf(threadId) + .some( + (delta) => + delta.kind === "item.textDelta" && delta.text === "Response to: ", + ), ).toBe(true); }); diff --git a/plugins/provider-pi/src/bridge/bridge.local-file.test.ts b/plugins/provider-pi/src/bridge/bridge.local-file.test.ts deleted file mode 100644 index c72a6508a9..0000000000 --- a/plugins/provider-pi/src/bridge/bridge.local-file.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { join } from "node:path"; -import { afterEach, beforeEach, expect, it } from "vitest"; -import { - FULL_PERMISSION_OPTIONS, - type FakePiBridgeHarness, - startFakePiBridge, -} from "./test-support.js"; - -let harness: FakePiBridgeHarness; - -beforeEach(async () => { - harness = await startFakePiBridge({ - prefix: "bb-pi-local-file-", - initialize: true, - }); -}); - -afterEach(async () => { - await harness.teardown(); -}); - -function localFile(path: string) { - return { - type: "localFile" as const, - path, - name: "notes.md", - sizeBytes: 6, - mimeType: "text/markdown", - }; -} - -it("includes local file paths in turn prompts", async () => { - const threadId = "thr_local_file_turn"; - const path = join(harness.workspaceDir, "notes.md"); - const marker = `[Attached file: ${path}]`; - await harness.startThread(threadId); - - const response = await harness.request(1, "turn/start", { - threadId, - providerThreadId: threadId, - clientRequestId: "creq_ab23456789", - input: [ - { type: "text", text: "Read this file.", mentions: [] }, - localFile(path), - ], - options: FULL_PERMISSION_OPTIONS, - }); - - expect(response.error).toBeUndefined(); - expect(response.result).toEqual({ threadId }); - await harness.waitForTurnBoundary(threadId); - expect( - harness.deltasOf(threadId).some( - (delta) => - delta.kind === "item.textDelta" && - String(delta.text).includes(`Read this file.\n${marker}`), - ), - ).toBe(true); -}); - -it("accepts a turn prompt that contains only a local file", async () => { - const threadId = "thr_local_file_only"; - const path = join(harness.workspaceDir, "notes.md"); - await harness.startThread(threadId); - - const response = await harness.request(2, "turn/start", { - threadId, - providerThreadId: threadId, - clientRequestId: "creq_cd23456789", - input: [localFile(path)], - options: FULL_PERMISSION_OPTIONS, - }); - - expect(response.error).toBeUndefined(); - expect(response.result).toEqual({ threadId }); -}); - -it("includes local file paths in steer prompts", async () => { - const threadId = "thr_local_file_steer"; - const path = join(harness.workspaceDir, "notes.md"); - const marker = `[Attached file: ${path}]`; - await harness.startThread(threadId); - await harness.request(3, "turn/start", { - threadId, - providerThreadId: threadId, - clientRequestId: "creq_ef23456789", - input: [{ type: "text", text: "/hold", mentions: [] }], - options: FULL_PERMISSION_OPTIONS, - }); - await harness.waitForDelta(threadId, (delta) => delta.kind === "turn.open"); - - const response = await harness.request(4, "turn/steer", { - threadId, - providerThreadId: threadId, - expectedTurnId: "turn-1", - clientRequestId: "creq_gh23456789", - input: [localFile(path)], - options: FULL_PERMISSION_OPTIONS, - }); - - expect(response.error).toBeUndefined(); - expect(response.result).toEqual({ threadId }); - await harness.waitForTurnBoundary(threadId); - expect( - harness.deltasOf(threadId).some( - (delta) => - delta.kind === "item.textDelta" && - String(delta.text).includes(marker), - ), - ).toBe(true); -}); diff --git a/plugins/provider-pi/src/bridge/bridge.round2.test.ts b/plugins/provider-pi/src/bridge/bridge.round2.test.ts index 45d39d17dc..f0ea6a4947 100644 --- a/plugins/provider-pi/src/bridge/bridge.round2.test.ts +++ b/plugins/provider-pi/src/bridge/bridge.round2.test.ts @@ -1,4 +1,10 @@ -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; @@ -160,6 +166,7 @@ it("refuses a manual compaction while pi reports a run still streaming", async ( it("a steer consumed by the run is reported accepted and named in the reply", async () => { const threadId = "thr_r2_steer_ok"; + const filePath = join(harness.workspaceDir, "notes.md"); await harness.startThread(threadId); turnStart(threadId, "/hold", "creq_ab23456789"); await harness.waitForDelta(threadId, (d) => d.kind === "turn.open"); @@ -168,7 +175,10 @@ it("a steer consumed by the run is reported accepted and named in the reply", as providerThreadId: threadId, expectedTurnId: "turn-1", clientRequestId: "creq_cd23456789", - input: [{ type: "text", text: "take the left path", mentions: [] }], + input: [ + { type: "text", text: "take the left path", mentions: [] }, + { type: "localFile", path: filePath }, + ], options: FULL_PERMISSION_OPTIONS, }); expect(steer.result).toMatchObject({ threadId }); @@ -188,7 +198,9 @@ it("a steer consumed by the run is reported accepted and named in the reply", as .some( (d) => d.kind === "item.textDelta" && - String(d.text).includes("Steered: take the left path"), + String(d.text).includes( + `Steered: take the left path\n[Attached file: ${filePath}]`, + ), ), ).toBe(true); expect(harness.messages.some((m) => m.method === "error")).toBe(false); @@ -428,9 +440,7 @@ it("resumes at bb's requested cwd when the session header's cwd was removed", as turnStart(threadId, '/tool bash {"command":"pwd"}', "creq_rsm2345678"); await harness.waitForDelta(threadId, (d) => d.kind === "item.close"); - const opened = harness - .deltasOf(threadId) - .find((d) => d.kind === "item.open"); + const opened = harness.deltasOf(threadId).find((d) => d.kind === "item.open"); expect(opened?.item).toMatchObject({ type: "command", command: "pwd", diff --git a/plugins/provider-pi/src/bridge/turn-input.test.ts b/plugins/provider-pi/src/bridge/turn-input.test.ts index 1950582e42..37632e8c0a 100644 --- a/plugins/provider-pi/src/bridge/turn-input.test.ts +++ b/plugins/provider-pi/src/bridge/turn-input.test.ts @@ -32,6 +32,26 @@ function extractText(input: PromptInput[]): string | undefined { return extractPiPromptInput(input).text; } +it("preserves local file paths with and without text", () => { + const path = "/workspace/notes.md"; + const marker = `[Attached file: ${path}]`; + const file = { + type: "localFile" as const, + path, + name: "notes.md", + sizeBytes: 6, + mimeType: "text/markdown", + }; + + expect( + extractText([ + { type: "text", text: "Read this file.", mentions: [] }, + file, + ]), + ).toBe(`Read this file.\n${marker}`); + expect(extractText([file])).toBe(marker); +}); + it("invokes a selected skill through Pi's native command", () => { expect( extractText([