diff --git a/apps/server/test/public/public-thread-compaction.test.ts b/apps/server/test/public/public-thread-compaction.test.ts index d2f2398c05..07b16ba5d1 100644 --- a/apps/server/test/public/public-thread-compaction.test.ts +++ b/apps/server/test/public/public-thread-compaction.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - getLatestThreadSequence, - listQueuedThreadMessages, -} from "@bb/db"; +import { getLatestThreadSequence, listQueuedThreadMessages } from "@bb/db"; import { createStandaloneBuiltinCompactCommandInput, turnScope, @@ -141,6 +138,44 @@ describe("public thread compaction", () => { }); }); + it("routes ACP agent compaction onto the bridge's /compact turn", async () => { + await withTestHarness(async (harness) => { + const { host, session, thread } = seedCompactableThread(harness, { + providerId: "acp-omp", + providerThreadId: "provider-thread-acp", + }); + const responder = registerSuccessfulTurnResponder(harness, { + hostId: host.id, + sessionId: session.id, + }); + + const response = await harness.app.request( + `/api/v1/threads/${thread.id}/compact`, + { method: "POST" }, + ); + expect( + response.status, + JSON.stringify(await readJson(response.clone())), + ).toBe(200); + const turnSubmitRequests = responder.requests.filter( + ({ command }) => command.type === "turn.submit", + ); + expect(turnSubmitRequests).toHaveLength(1); + // The standalone builtin /compact mention rides the ordinary turn path + // to the provider-acp bridge, which runs it as the agent's own /compact + // maintenance prompt instead of model input. + expect(turnSubmitRequests[0]?.command).toMatchObject({ + type: "turn.submit", + threadId: thread.id, + input: createStandaloneBuiltinCompactCommandInput(), + resumeContext: { + providerId: "acp-omp", + providerThreadId: "provider-thread-acp", + }, + }); + }); + }); + it("queues sends and defers send-now while manual compaction is active", async () => { await withTestHarness(async (harness) => { const { host, session, thread } = seedCompactableThread(harness, { @@ -242,12 +277,14 @@ describe("public thread compaction", () => { }), ).toBe(true); expect(listQueuedThreadMessages(harness.db, thread.id)).toHaveLength(0); - await expect.poll( - () => - responder.requests.filter( - ({ command }) => command.type === "turn.submit", - ).length, - ).toBe(2); + await expect + .poll( + () => + responder.requests.filter( + ({ command }) => command.type === "turn.submit", + ).length, + ) + .toBe(2); }); }); diff --git a/apps/server/test/services/plugins/first-party-provider-plugins.test.ts b/apps/server/test/services/plugins/first-party-provider-plugins.test.ts index a7e9e2f58c..f3c527894b 100644 --- a/apps/server/test/services/plugins/first-party-provider-plugins.test.ts +++ b/apps/server/test/services/plugins/first-party-provider-plugins.test.ts @@ -92,7 +92,7 @@ const FIRST_PARTY_PROVIDER_DECLARATIONS = [ supportsThreadArchive: false, supportsThreadRename: false, fork: "tip", - supportsManualCompaction: false, + supportsManualCompaction: true, supportsUsage: false, visibility: "installed", hasLogo: true, diff --git a/packages/domain/src/plugin-sdk-version.ts b/packages/domain/src/plugin-sdk-version.ts index 232bbe94ac..cd0a914b39 100644 --- a/packages/domain/src/plugin-sdk-version.ts +++ b/packages/domain/src/plugin-sdk-version.ts @@ -16,7 +16,7 @@ // PLUGIN_SDK_MAJOR is 0, so the major-only artifact gate cannot distinguish // 0.x releases and is intentionally vacuous for them until a future 1.0. // Rebuildable artifacts still rebuild on the exact sdkVersion-differs trigger. -export const PLUGIN_SDK_VERSION = "0.4.22"; +export const PLUGIN_SDK_VERSION = "0.4.23"; /** Major of {@link PLUGIN_SDK_VERSION} — the plugin API compatibility number. */ export const PLUGIN_SDK_MAJOR = Number(PLUGIN_SDK_VERSION.split(".", 1)[0]); diff --git a/packages/plugin-sdk/package.json b/packages/plugin-sdk/package.json index ba46313380..f7d2089651 100644 --- a/packages/plugin-sdk/package.json +++ b/packages/plugin-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@get-bb/plugin-sdk", - "version": "0.4.22", + "version": "0.4.23", "homepage": "https://github.com/get-bb/bb#readme", "bugs": { "url": "https://github.com/get-bb/bb/issues" diff --git a/packages/provider-bridge-acp/src/bridge-protocol.ts b/packages/provider-bridge-acp/src/bridge-protocol.ts index 0a51a192b6..0eb4b8d7ab 100644 --- a/packages/provider-bridge-acp/src/bridge-protocol.ts +++ b/packages/provider-bridge-acp/src/bridge-protocol.ts @@ -6,8 +6,26 @@ * why they are schemas rather than ad-hoc objects. */ -import { acpNativeReasoningSchema as acpBridgeNativeReasoningSchema, acpPermissionCliSchema as acpBridgePermissionCliSchema, acpReasoningCliSchema as acpBridgeReasoningCliSchema } from "@bb/domain"; -import { initializeParamsSchema, providerInstallationRunParamsSchema, providerInstallationStatusParamsSchema, providerMaintenanceParamsSchema, modelListParamsSchema as canonicalModelListParamsSchema, skillsConfigureParamsSchema, threadDiscardParamsSchema as canonicalThreadDiscardParamsSchema, threadForkParamsSchema as canonicalThreadForkParamsSchema, threadResumeParamsSchema as canonicalThreadResumeParamsSchema, threadStartParamsSchema as canonicalThreadStartParamsSchema, threadStopParamsSchema as canonicalThreadStopParamsSchema, turnStartParamsSchema as canonicalTurnStartParamsSchema, turnSteerParamsSchema as canonicalTurnSteerParamsSchema } from "@bb/provider-bridge-protocol"; +import { + acpNativeReasoningSchema as acpBridgeNativeReasoningSchema, + acpPermissionCliSchema as acpBridgePermissionCliSchema, + acpReasoningCliSchema as acpBridgeReasoningCliSchema, +} from "@bb/domain"; +import { + initializeParamsSchema, + providerInstallationRunParamsSchema, + providerInstallationStatusParamsSchema, + providerMaintenanceParamsSchema, + modelListParamsSchema as canonicalModelListParamsSchema, + skillsConfigureParamsSchema, + threadDiscardParamsSchema as canonicalThreadDiscardParamsSchema, + threadForkParamsSchema as canonicalThreadForkParamsSchema, + threadResumeParamsSchema as canonicalThreadResumeParamsSchema, + threadStartParamsSchema as canonicalThreadStartParamsSchema, + threadStopParamsSchema as canonicalThreadStopParamsSchema, + turnStartParamsSchema as canonicalTurnStartParamsSchema, + turnSteerParamsSchema as canonicalTurnSteerParamsSchema, +} from "@bb/provider-bridge-protocol"; import { z } from "zod"; import { acpSessionUpdateSchema, acpStopReasonSchema } from "./wire.js"; @@ -159,6 +177,14 @@ export const acpCompactionCompletedNotificationParamsSchema = status: z.literal("interrupted"), }) .passthrough(), + z + .object({ + threadId: z.string().min(1), + status: z.literal("skipped"), + /** The agent's own reason the compaction was a no-op. */ + detail: z.string().min(1), + }) + .passthrough(), z .object({ threadId: z.string().min(1), diff --git a/packages/provider-bridge-acp/src/bridge/bridge.test.ts b/packages/provider-bridge-acp/src/bridge/bridge.test.ts index 95d56f907b..a915f344fc 100644 --- a/packages/provider-bridge-acp/src/bridge/bridge.test.ts +++ b/packages/provider-bridge-acp/src/bridge/bridge.test.ts @@ -2332,6 +2332,111 @@ describe("acp bridge", () => { expect(threadEventsOfType("thread/compacted")).toEqual([]); }); + it("fails the compaction turn when the agent reports the failure in an end-turn message", async () => { + const { providerThreadId } = await startThread({ + envVars: { + FAKE_ACP_COMPACT_AGENT_MESSAGE: + "Compaction failed: summary model rejected the request", + }, + }); + + const turnId = sendTurnRequest("turn/start", providerThreadId, { + input: compactCommandInput(), + }); + expect((await waitForResponse(turnId)).error).toBeUndefined(); + + // omp answers end_turn even when its /compact handler failed and said so + // in an ordinary agent message; that text must fail the turn instead of + // the end_turn being read as a shrunk context (#2290). + const completed = await waitForTurnCompleted(); + expect(completed).toMatchObject({ + status: "failed", + error: { + message: "Compaction failed: summary model rejected the request", + }, + }); + expect(threadEventsOfType("thread/compacted")).toEqual([]); + }); + + it("completes a no-op compaction turn without reporting a compacted context", async () => { + const { providerThreadId } = await startThread({ + envVars: { + FAKE_ACP_COMPACT_AGENT_MESSAGE: + "Compaction failed: Nothing to compact (session too small)", + }, + }); + + const turnId = sendTurnRequest("turn/start", providerThreadId, { + input: compactCommandInput(), + }); + expect((await waitForResponse(turnId)).error).toBeUndefined(); + + // A small session has nothing to compact: the turn ends cleanly with the + // agent's reason surfaced as a warning, and no `thread/compacted`. + const completed = await waitForTurnCompleted(); + expect(completed).toMatchObject({ status: "completed" }); + expect(threadEventsOfType("thread/compacted")).toEqual([]); + expect(threadEventsOfType("provider/warning").at(-1)).toMatchObject({ + category: "compaction-skipped", + summary: "Context compaction skipped", + details: "Compaction failed: Nothing to compact (session too small)", + }); + }); + + it("keeps classifying a no-op compaction when the agent rewords its prose", async () => { + const { providerThreadId } = await startThread({ + envVars: { + FAKE_ACP_COMPACT_AGENT_MESSAGE: + "compaction failed: nothing to compact — the session is still small", + }, + }); + + const turnId = sendTurnRequest("turn/start", providerThreadId, { + input: compactCommandInput(), + }); + expect((await waitForResponse(turnId)).error).toBeUndefined(); + + // The no-op phrasing is the agent's own prose, not a contract + // (can1357/oh-my-pi#9786): a reworded, lowercased reason must still + // complete the turn as a skip instead of failing it as an error. + const completed = await waitForTurnCompleted(); + expect(completed).toMatchObject({ status: "completed" }); + expect(threadEventsOfType("thread/compacted")).toEqual([]); + expect(threadEventsOfType("provider/warning").at(-1)).toMatchObject({ + category: "compaction-skipped", + summary: "Context compaction skipped", + details: + "compaction failed: nothing to compact — the session is still small", + }); + }); + + it("fails the compaction turn when the failure report is reworded or preceded by other text", async () => { + const { providerThreadId } = await startThread({ + envVars: { + FAKE_ACP_COMPACT_AGENT_MESSAGE: + "Tried shrinking the context.\nCompaction failed: session is locked by another compaction", + }, + }); + + const turnId = sendTurnRequest("turn/start", providerThreadId, { + input: compactCommandInput(), + }); + expect((await waitForResponse(turnId)).error).toBeUndefined(); + + // A failure sentence buried after other streamed text must still fail + // the turn; reading `end_turn` as success here would report a compacted + // context that never compacted. + const completed = await waitForTurnCompleted(); + expect(completed).toMatchObject({ + status: "failed", + error: { + message: + "Tried shrinking the context.\nCompaction failed: session is locked by another compaction", + }, + }); + expect(threadEventsOfType("thread/compacted")).toEqual([]); + }); + it("accepts turn input only after the prompt carrying it goes out", async () => { const { providerThreadId } = await startThread(); const turnId = sendTurnRequest("turn/start", providerThreadId, { diff --git a/packages/provider-bridge-acp/src/bridge/bridge.ts b/packages/provider-bridge-acp/src/bridge/bridge.ts index 2ea887865b..38663b482c 100644 --- a/packages/provider-bridge-acp/src/bridge/bridge.ts +++ b/packages/provider-bridge-acp/src/bridge/bridge.ts @@ -104,6 +104,8 @@ import { acpSessionForkResultSchema, acpSessionNewResultSchema, acpSessionNotificationParamsSchema, + acpAgentMessageChunkUpdateSchema, + extractAcpContentText, acpUsageUpdateSchema, type AcpConfigStateResult, type AcpSessionModels, @@ -196,6 +198,13 @@ interface AcpThreadSession { * the provider-local `"compaction"` maintenance prompt, or none. */ activePromptKind: "turn" | "compaction" | null; + /** + * Agent message text streamed during the compaction maintenance prompt. + * Some agents (omp) report a failed `/compact` as an ordinary agent + * message and still answer `end_turn`, so the prompt result alone cannot + * tell a shrunk context from a no-op. + */ + compactionAgentMessage: string; queuedInputs: AcpPendingTurnInput[]; /** True while a session/prompt request is outstanding. */ promptRequestPending: boolean; @@ -1925,6 +1934,7 @@ async function startAgentSession( }, pendingInstructions: params.instructions, activePromptKind: null, + compactionAgentMessage: "", queuedInputs: [], promptRequestPending: false, cancelRequested: false, @@ -2388,11 +2398,38 @@ function runTurn( * every other stop reason or prompt rejection fails the turn with the agent's * own reason rather than being reported as a shrunk context. */ +/** + * omp reports a failed `/compact` as an ordinary agent message and still + * answers `end_turn`, so an `end_turn` compaction prompt is only a shrunk + * context when the agent did not spend the turn reporting a failure. The + * phrasing is omp's own prose rather than a contract (can1357/oh-my-pi#9786 + * asks for a structured signal), so classification matches the failure + * opener and the no-op reasons semantically — case-insensitive, anywhere in + * the message — instead of pinning exact strings: a reworded reason stays a + * skip, and any compaction-failure sentence can never report a compacted + * context. The no-op reasons are the phrases pi prints (#1721). + */ +const COMPACTION_FAILURE_PATTERN = /\bcompaction failed\b/i; +const COMPACTION_NOOP_PATTERN = /\b(?:nothing to compact|already compacted)\b/i; + +function compactionOutcomeForEndTurn( + agentMessage: string, +): Record { + const text = agentMessage.trim(); + if (!COMPACTION_FAILURE_PATTERN.test(text)) { + return { status: "completed" }; + } + return COMPACTION_NOOP_PATTERN.test(text) + ? { status: "skipped", detail: text } + : { status: "failed", error: text }; +} + function startCompaction( session: AcpThreadSession, pending: AcpPendingTurnInput, ): void { session.activePromptKind = "compaction"; + session.compactionAgentMessage = ""; emitForSession(session, ACP_COMPACTION_STARTED_METHOD, { threadId: session.bbThreadId, }); @@ -2415,7 +2452,7 @@ function startCompaction( .then((result) => { finish( result.stopReason === "end_turn" - ? { status: "completed" } + ? compactionOutcomeForEndTurn(session.compactionAgentMessage) : result.stopReason === "cancelled" ? { status: "interrupted" } : { @@ -2541,6 +2578,15 @@ function handleAgentNotification( if (parsed.data.sessionId !== session.providerThreadId) { return; } + if (session.activePromptKind === "compaction") { + const chunk = acpAgentMessageChunkUpdateSchema.safeParse( + parsed.data.update, + ); + if (chunk.success) { + session.compactionAgentMessage += + extractAcpContentText(chunk.data.content) ?? ""; + } + } emitForSession(session, ACP_UPDATE_METHOD, update); } diff --git a/packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs b/packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs index 9b786c814b..f5f6dee3da 100755 --- a/packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs +++ b/packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs @@ -424,7 +424,13 @@ async function handlePrompt(message) { } if (text === "/compact") { - // OpenCode treats this exact prompt as a provider-local control. + // OpenCode treats this exact prompt as a provider-local control. omp + // instead runs the command and reports a failure as an ordinary agent + // message while still answering end_turn (get-bb/bb#2290). + const compactMessage = process.env.FAKE_ACP_COMPACT_AGENT_MESSAGE; + if (compactMessage !== undefined) { + notifyUpdate(messageChunk(compactMessage)); + } } else if (text.includes("request-external-directory-permission")) { // opencode's external_directory permission: the running edit tool asks // with the generic kind "other", a bare directory title, and diff --git a/packages/provider-bridge-acp/src/delta-translation.test.ts b/packages/provider-bridge-acp/src/delta-translation.test.ts index cb0bbe8014..e68bca5f90 100644 --- a/packages/provider-bridge-acp/src/delta-translation.test.ts +++ b/packages/provider-bridge-acp/src/delta-translation.test.ts @@ -454,6 +454,34 @@ describe("acp delta translation (moved from the legacy adapter suite)", () => { ]); }); + it("translates a skipped maintenance prompt into a warning and a clean turn end", () => { + const harness = createHarness(); + harness.translate(compactionStartedEvent()); + const turnId = harness.openTurnId(); + + expect( + harness.translate( + compactionCompletedEvent({ + status: "skipped", + detail: "Compaction failed: Nothing to compact (session too small)", + }), + ), + ).toEqual([ + expect.objectContaining({ + type: "provider/warning", + scope: turnScope(turnId), + category: "compaction-skipped", + summary: "Context compaction skipped", + details: "Compaction failed: Nothing to compact (session too small)", + }), + expect.objectContaining({ + type: "turn/completed", + scope: turnScope(turnId), + status: "completed", + }), + ]); + }); + it("completes streamed items before ending a compaction turn", () => { const harness = createHarness(); harness.translate(compactionStartedEvent()); diff --git a/packages/provider-bridge-acp/src/delta-translation.ts b/packages/provider-bridge-acp/src/delta-translation.ts index d54877b942..bd472b6db3 100644 --- a/packages/provider-bridge-acp/src/delta-translation.ts +++ b/packages/provider-bridge-acp/src/delta-translation.ts @@ -1106,16 +1106,31 @@ export function createAcpDeltaTranslator( return []; } const status = params.data.status; + // A skipped compaction is a clean no-op (nothing to shrink), so its + // turn ends completed with the agent's reason surfaced as a warning — + // but like a failed or interrupted one it must never report + // `thread/compacted`, which only a genuine compaction earns. + const turnStatus: ThreadEventTurnStatus = + status === "skipped" ? "completed" : status; return [ - ...flushOpenTurnWork(context, status), - // Only a completed maintenance prompt actually shrank the context; a - // failed or interrupted one must never report `thread/compacted`. + ...flushOpenTurnWork(context, itemStatusForTurnStatus(turnStatus)), ...(status === "completed" ? ([{ kind: "context.compacted" }] as ThreadDelta[]) : []), + ...(status === "skipped" + ? ([ + { + kind: "provider.warning", + category: "compaction-skipped", + summary: "Context compaction skipped", + details: params.data.detail, + vouchedTurn: true, + }, + ] as ThreadDelta[]) + : []), { kind: "turn.boundary", - status, + status: turnStatus, ...(status === "failed" ? { error: { message: params.data.error } } : {}), diff --git a/plugins/provider-acp/src/known-agents.ts b/plugins/provider-acp/src/known-agents.ts index c55750f0f7..13dad27a8b 100644 --- a/plugins/provider-acp/src/known-agents.ts +++ b/plugins/provider-acp/src/known-agents.ts @@ -157,6 +157,11 @@ export const KNOWN_ACP_AGENTS: readonly AcpAgentDefinition[] = [ signInCommand: "omp login", installUrl: "https://github.com/can1357/omp", visibility: "installed", + // omp runs its builtin `/compact` from ACP prompt text + // (executeAcpBuiltinSlashCommand) and advertises it in + // available_commands_update, so the bridge's /compact maintenance turn + // works end to end (#2290). + supportsManualCompaction: true, // Unverified; the ACP tier's value (see acp-opencode). fork: "tip", launch: {