Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions apps/server/test/public/public-thread-compaction.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 28 additions & 2 deletions packages/provider-bridge-acp/src/bridge-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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),
Expand Down
60 changes: 56 additions & 4 deletions packages/provider-bridge-acp/src/bridge/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import { fileURLToPath } from "node:url";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createStandaloneBuiltinCompactCommandInput } from "@bb/domain";
import type { DynamicTool, ReasoningLevel } from "@bb/domain";
import { PROVIDER_BRIDGE_PROTOCOL_VERSION, THREAD_DELTA_NOTIFICATION_METHOD } from "@bb/provider-bridge-protocol";
import {
PROVIDER_BRIDGE_PROTOCOL_VERSION,
THREAD_DELTA_NOTIFICATION_METHOD,
} from "@bb/provider-bridge-protocol";
import {
assembleCapturedThreadEvents,
captureBridgeJsonRpcOutput,
Expand Down Expand Up @@ -355,9 +358,7 @@ function deltaKindsOf(message: BridgeJsonRpcOutputMessage): string[] {
if (message.method !== THREAD_DELTA_NOTIFICATION_METHOD) {
return [];
}
const params = message.params as
| { deltas?: { kind?: string }[] }
| undefined;
const params = message.params as { deltas?: { kind?: string }[] } | undefined;
return (params?.deltas ?? []).map((delta) => delta.kind ?? "");
}

Expand Down Expand Up @@ -2085,6 +2086,57 @@ 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("accepts turn input only after the prompt carrying it goes out", async () => {
const { providerThreadId } = await startThread();
const turnId = sendTurnRequest("turn/start", providerThreadId, {
Expand Down
85 changes: 79 additions & 6 deletions packages/provider-bridge-acp/src/bridge/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,37 @@
* workspace write policy on client `fs/write_text_file` requests.
*/

import { isStandaloneBuiltinCompactCommand, pendingInteractionResolutionSchema, reasoningEffortsForLevels } from "@bb/domain";
import {
isStandaloneBuiltinCompactCommand,
pendingInteractionResolutionSchema,
reasoningEffortsForLevels,
} from "@bb/domain";
import type { AvailableModel, PromptInput, ReasoningLevel } from "@bb/domain";
import { acpLaunchSpecSchema, type AcpLaunchSpec } from "../launch-spec.js";
import { BRIDGE_INBOUND_REQUEST_METHODS, BRIDGE_JSON_RPC_ERRORS, BRIDGE_NOTIFICATION_METHODS, PROVIDER_BRIDGE_PROTOCOL_VERSION, THREAD_DELTA_GRAMMAR_V3, THREAD_DELTA_NOTIFICATION_METHOD } from "@bb/provider-bridge-protocol";
import type { InitializeResult, ThreadDelta } from "@bb/provider-bridge-protocol";
import { BridgeRecoveryError, bridgeRequestEnvelopeSchema, createBridgeIo, createBridgeLineHandler, decodeBridgeJsonRpcResponse, decodeToolCallResponsePayload, experimental_defineProviderBridge, mimeTypeFromExtension, runBridgeRequest, withoutBridgeRuntimeEnv } from "@bb/provider-bridge-protocol/bridge-kit";
import {
BRIDGE_INBOUND_REQUEST_METHODS,
BRIDGE_JSON_RPC_ERRORS,
BRIDGE_NOTIFICATION_METHODS,
PROVIDER_BRIDGE_PROTOCOL_VERSION,
THREAD_DELTA_GRAMMAR_V3,
THREAD_DELTA_NOTIFICATION_METHOD,
} from "@bb/provider-bridge-protocol";
import type {
InitializeResult,
ThreadDelta,
} from "@bb/provider-bridge-protocol";
import {
BridgeRecoveryError,
bridgeRequestEnvelopeSchema,
createBridgeIo,
createBridgeLineHandler,
decodeBridgeJsonRpcResponse,
decodeToolCallResponsePayload,
experimental_defineProviderBridge,
mimeTypeFromExtension,
runBridgeRequest,
withoutBridgeRuntimeEnv,
} from "@bb/provider-bridge-protocol/bridge-kit";
import type { BridgeJsonRpcResponse } from "@bb/provider-bridge-protocol/bridge-kit";
import { execFile } from "node:child_process";
import { randomBytes } from "node:crypto";
Expand Down Expand Up @@ -79,6 +104,8 @@ import {
acpSessionForkResultSchema,
acpSessionNewResultSchema,
acpSessionNotificationParamsSchema,
acpAgentMessageChunkUpdateSchema,
extractAcpContentText,
acpUsageUpdateSchema,
type AcpConfigStateResult,
type AcpSessionModels,
Expand Down Expand Up @@ -171,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;
Expand Down Expand Up @@ -1828,6 +1862,7 @@ async function startAgentSession(
},
pendingInstructions: params.instructions,
activePromptKind: null,
compactionAgentMessage: "",
queuedInputs: [],
promptRequestPending: false,
cancelRequested: false,
Expand Down Expand Up @@ -2012,7 +2047,10 @@ async function startAgentSession(
sendThreadDeltas(bbThreadId, [{ kind: "session.reset" }]);
session.deferStartEmit = undefined;
for (const deferred of deferredEmits) {
if (deferred.sessionId !== undefined && deferred.sessionId !== sessionId) {
if (
deferred.sessionId !== undefined &&
deferred.sessionId !== sessionId
) {
continue;
}
emitForSession(session, deferred.method, deferred.params);
Expand Down Expand Up @@ -2288,11 +2326,37 @@ 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 two
* no-op messages are the same strings pi prints (#1721): the compaction
* completed cleanly but had nothing to do, while any other "Compaction
* failed:" text is a real failure the thread must surface.
*/
const ACP_COMPACTION_NOOP_MESSAGES: Record<string, true> = {
"Compaction failed: Nothing to compact (session too small)": true,
"Compaction failed: Already compacted": true,
};

function compactionOutcomeForEndTurn(
agentMessage: string,
): Record<string, unknown> {
const text = agentMessage.trim();
if (!text.startsWith("Compaction failed:")) {
return { status: "completed" };
}
return ACP_COMPACTION_NOOP_MESSAGES[text] === true
? { 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,
});
Expand All @@ -2315,7 +2379,7 @@ function startCompaction(
.then((result) => {
finish(
result.stopReason === "end_turn"
? { status: "completed" }
? compactionOutcomeForEndTurn(session.compactionAgentMessage)
: result.stopReason === "cancelled"
? { status: "interrupted" }
: {
Expand Down Expand Up @@ -2441,6 +2505,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);
}

Expand Down
18 changes: 15 additions & 3 deletions packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ const authMethods = (process.env.FAKE_ACP_AUTH_METHODS ?? "")
const authOptional = process.env.FAKE_ACP_AUTH_OPTIONAL === "1";
const sessionNewError = process.env.FAKE_ACP_SESSION_NEW_ERROR;
const exitOnSessionNew = process.env.FAKE_ACP_EXIT_ON_SESSION_NEW;
const sessionNewDelayMs = Number(process.env.FAKE_ACP_SESSION_NEW_DELAY_MS ?? "0");
const sessionNewDelayMs = Number(
process.env.FAKE_ACP_SESSION_NEW_DELAY_MS ?? "0",
);
const updatesWithSessionResponse =
process.env.FAKE_ACP_UPDATES_WITH_SESSION_RESPONSE === "1";
const ignoreCancel = process.env.FAKE_ACP_IGNORE_CANCEL === "1";
Expand Down Expand Up @@ -233,7 +235,11 @@ function configState() {
}

function requireAuthenticated(message) {
if (authMethods.length === 0 || authOptional || authenticatedMethod !== null) {
if (
authMethods.length === 0 ||
authOptional ||
authenticatedMethod !== null
) {
return true;
}
// ACP's reserved auth-required error: code -32000 with this message.
Expand Down Expand Up @@ -328,7 +334,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
Expand Down
Loading
Loading