Skip to content
Merged
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
5 changes: 5 additions & 0 deletions LAWS/CHAT.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,8 @@

- A message that is not first in the queue MUST NOT steer the session.
- A steering result MUST affect only the message that produced it.

## Subagent activity

- Subagent activity MUST attribute the subagent when its identity is known.
- Subagent activity MUST describe the delegated task when it is known.
Comment thread
loganj marked this conversation as resolved.
191 changes: 191 additions & 0 deletions src/features/chat/acp/__tests__/acpNotificationHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,197 @@ describe("acpNotificationHandler", () => {
warnSpy.mockRestore();
});

it.each([
{ mode: "live", lateIdentity: false, configuredTask: false },
{ mode: "live", lateIdentity: true, configuredTask: false },
{ mode: "replay", lateIdentity: false, configuredTask: false },
{ mode: "replay", lateIdentity: true, configuredTask: false },
{ mode: "live", lateIdentity: false, configuredTask: true },
{ mode: "live", lateIdentity: true, configuredTask: true },
{ mode: "replay", lateIdentity: false, configuredTask: true },
{ mode: "replay", lateIdentity: true, configuredTask: true },
] as const)("retains async delegate identity and task in $mode when load identity is late=$lateIdentity and configured=$configuredTask", async ({
mode,
lateIdentity,
configuredTask,
}) => {
const sessionId = "acp-session";
if (mode === "live") {
registerPreparedSession(sessionId, "goose", "/Users/test");
setActiveMessageId(sessionId, "assistant-1");
} else {
markSessionReplayLoading(sessionId);
}

const replayMeta =
mode === "replay"
? { messageId: "assistant-1", created: 1_700_000_120 }
: {};
const toolMeta = (toolName: string) => ({
goose: {
...replayMeta,
toolCall: { toolName },
},
});

await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call",
toolCallId: "delegate-1",
title: "delegate",
rawInput: {
source: "Rivet",
...(!configuredTask ? { instructions: "Count markdown files" } : {}),
async: true,
},
_meta: toolMeta("delegate"),
},
} as never);
await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call_update",
toolCallId: "delegate-1",
status: "completed",
content: [
{
type: "content",
content: {
type: "text",
text: "Task 20260807_119 started in background",
},
},
],
_meta: toolMeta("delegate"),
},
} as never);

await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call",
toolCallId: "load-1",
title: "load",
rawInput: { source: "20260807_119" },
...(!lateIdentity ? { _meta: toolMeta("load") } : {}),
},
} as never);
if (lateIdentity) {
await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call_update",
toolCallId: "load-1",
_meta: toolMeta("load"),
},
} as never);
}

const messages =
mode === "live"
? useChatStore.getState().messagesBySession[sessionId]
: getReplayBuffer(sessionId);
const load = messages
?.flatMap((message) => message.content)
.find((block) => block.type === "toolRequest" && block.id === "load-1");
expect(load).toMatchObject({
type: "toolRequest",
toolName: "load",
subagentAgentName: "Rivet",
...(configuredTask
? { subagentTaskIsConfigured: true }
: { subagentTaskLabel: "Count markdown files" }),
});
});

it("retains codex-acp wire provenance on the rendered tool request", async () => {
registerPreparedSession("acp-session", "codex", "/Users/test");
setActiveMessageId("acp-session", "assistant-1");

await handleSessionNotification({
sessionId: "acp-session",
update: {
sessionUpdate: "tool_call",
toolCallId: "followup-1",
title: "Sending follow-up",
rawInput: {
prompt: "Re-check the cache boundary",
senderThreadId: "root",
receiverThreadIds: ["/root/reviewer"],
agentsStates: {},
model: "gpt-5",
reasoningEffort: "medium",
status: "running",
},
_meta: { codex: { collaboration: { tool: "followup_task" } } },
},
} as never);

const [message] = useChatStore.getState().messagesBySession["acp-session"];
expect(message.content[0]).toMatchObject({
type: "toolRequest",
id: "followup-1",
toolName: "followup_task",
arguments: {
prompt: "Re-check the cache boundary",
receiverThreadIds: ["/root/reviewer"],
},
subagentAgentName: "/root/reviewer",
subagentTaskLabel: "Re-check the cache boundary",
});
});

it.each([
"live",
"replay",
] as const)("retains codex-acp provenance when identity arrives late in %s", async (mode) => {
const sessionId = "acp-session";
if (mode === "live") {
registerPreparedSession(sessionId, "codex", "/Users/test");
setActiveMessageId(sessionId, "assistant-1");
} else {
markSessionReplayLoading(sessionId);
}

await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call",
toolCallId: "followup-1",
title: "Sending follow-up",
rawInput: {
prompt: "Re-check the cache boundary",
receiverThreadIds: ["/root/reviewer"],
},
},
} as never);
await handleSessionNotification({
sessionId,
update: {
sessionUpdate: "tool_call_update",
toolCallId: "followup-1",
_meta: { codex: { collaboration: { tool: "followup_task" } } },
},
} as never);

const messages =
mode === "live"
? useChatStore.getState().messagesBySession[sessionId]
: getReplayBuffer(sessionId);
const request = messages
?.flatMap((message) => message.content)
.find(
(block) => block.type === "toolRequest" && block.id === "followup-1",
);
expect(request).toMatchObject({
type: "toolRequest",
toolName: "followup_task",
subagentAgentName: "/root/reviewer",
subagentTaskLabel: "Re-check the cache boundary",
});
});

it("preserves ACP tool kind and locations on tool requests", async () => {
registerPreparedSession("acp-session", "goose", "/Users/test");
setActiveMessageId("acp-session", "assistant-1");
Expand Down
71 changes: 43 additions & 28 deletions src/features/chat/acp/acpNotificationHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ import {
getToolCallIdentity,
getToolChainSummary,
} from "@/shared/api/acpToolCallIdentity";
import { resolveSubagentLabel } from "@/features/chat/lib/subagentToolCalls";
import {
getSubagentToolCallContext,
resolveSubagentContext,
} from "@/features/chat/lib/subagentToolCalls";
import { applyChatSessionConfigOptionsSnapshot } from "./sessionConfigSnapshotAdapter";
import { perfLog } from "@/shared/lib/perfLog";
import {
Expand Down Expand Up @@ -431,11 +434,13 @@ function handleReplay(sessionId: string, update: SessionUpdate): void {
getReplayAssistantMessageMetadata(sessionId, update),
);
const replayArguments = rawInputToArguments(update.rawInput);
const replaySubagentLabel = resolveSubagentLabel(
identity.toolName,
replayArguments,
getReplayBuffer(sessionId) ?? [],
);
const replaySubagentContext =
getSubagentToolCallContext(identity.toolName, replayArguments) ??
resolveSubagentContext(
identity.toolName,
replayArguments,
getReplayBuffer(sessionId) ?? [],
);
msg.content.push({
type: "toolRequest",
id: update.toolCallId,
Expand All @@ -446,7 +451,7 @@ function handleReplay(sessionId: string, update: SessionUpdate): void {
...toolCallUpdatePatch(update),
startedAt: created ?? Date.now(),
...(chainSummary ? { chainSummary } : {}),
...(replaySubagentLabel ? { subagentLabel: replaySubagentLabel } : {}),
...(replaySubagentContext ?? {}),
});
break;
}
Expand Down Expand Up @@ -494,13 +499,19 @@ function handleReplay(sessionId: string, update: SessionUpdate): void {
// The wire tool name can arrive after the initial tool_call
// (identity patched in by a later update); resolve the subagent
// label now that we know what the tool is.
if (identity.toolName && tc.subagentLabel === undefined) {
const lateLabel = resolveSubagentLabel(
tc.toolName,
tc.arguments,
getReplayBuffer(sessionId) ?? [],
);
if (lateLabel) tc.subagentLabel = lateLabel;
if (
identity.toolName &&
(tc.subagentAgentName === undefined ||
tc.subagentTaskLabel === undefined)
) {
const lateContext =
getSubagentToolCallContext(tc.toolName, tc.arguments) ??
resolveSubagentContext(
tc.toolName,
tc.arguments,
getReplayBuffer(sessionId) ?? [],
);
if (lateContext) Object.assign(tc, lateContext);
}
}
}
Expand Down Expand Up @@ -628,11 +639,13 @@ function handleLive(sessionId: string, update: SessionUpdate): void {
const chainSummary = getToolChainSummary(update);

const liveArguments = rawInputToArguments(update.rawInput);
const liveSubagentLabel = resolveSubagentLabel(
identity.toolName,
liveArguments,
useChatStore.getState().messagesBySession[sessionId] ?? [],
);
const liveSubagentContext =
getSubagentToolCallContext(identity.toolName, liveArguments) ??
resolveSubagentContext(
identity.toolName,
liveArguments,
useChatStore.getState().messagesBySession[sessionId] ?? [],
);
const toolRequest: ToolRequestContent = {
type: "toolRequest",
id: update.toolCallId,
Expand All @@ -643,7 +656,7 @@ function handleLive(sessionId: string, update: SessionUpdate): void {
...toolCallUpdatePatch(update),
startedAt: Date.now(),
...(chainSummary ? { chainSummary } : {}),
...(liveSubagentLabel ? { subagentLabel: liveSubagentLabel } : {}),
...(liveSubagentContext ?? {}),
};
store.setStreamingMessageId(sessionId, messageId);
store.appendToStreamingMessage(sessionId, toolRequest);
Expand Down Expand Up @@ -674,13 +687,17 @@ function handleLive(sessionId: string, update: SessionUpdate): void {
// The wire tool name can arrive after the initial tool_call
// (identity patched in by a later update); resolve the subagent
// label now that we know what the tool is.
const lateSubagentLabel = identity.toolName
? resolveSubagentLabel(
const storedArguments = identity.toolName
? (findLiveToolRequest(sessionId, messageId, update.toolCallId)
?.arguments ?? {})
: {};
const lateSubagentContext = identity.toolName
? (getSubagentToolCallContext(identity.toolName, storedArguments) ??
resolveSubagentContext(
identity.toolName,
findLiveToolRequest(sessionId, messageId, update.toolCallId)
?.arguments ?? {},
storedArguments,
useChatStore.getState().messagesBySession[sessionId] ?? [],
)
))
: undefined;
store.updateMessage(sessionId, messageId, (msg) => ({
...msg,
Expand All @@ -692,9 +709,7 @@ function handleLive(sessionId: string, update: SessionUpdate): void {
...identity,
...patch,
...(chainSummary ? { chainSummary } : {}),
...(lateSubagentLabel && c.subagentLabel === undefined
? { subagentLabel: lateSubagentLabel }
: {}),
...(lateSubagentContext ?? {}),
}
: c,
),
Expand Down
Loading