Fix app-server thread request boundaries

This commit is contained in:
murashit 2026-06-26 17:14:12 +09:00
parent c35a07bc9c
commit 638cecb991
9 changed files with 89 additions and 38 deletions

View file

@ -25,11 +25,12 @@ import type { AppServerResourceEvent } from "../actions/metadata";
import { classifyAppServerLog } from "./app-server-logs";
import { type ChatNotificationEffect, planChatNotification } from "./notification-plan";
import {
routeServerRequest,
serverRequestApprovalResponse,
serverRequestCurrentTimeResponse,
serverRequestMcpElicitationResponse,
serverRequestUserInputResponse,
} from "./server-requests/responses";
import { routeServerRequest } from "./server-requests/routing";
} from "./server-requests/adapter";
function cannotSendApprovalResponseMessage(): string {
return "Could not send approval response because Codex app-server is not connected.";
@ -235,7 +236,7 @@ function respondToCurrentTimeRequest(
context: ChatInboundHandlerContext,
request: Extract<ServerRequest, { method: "currentTime/read" }>,
): void {
if (!context.actions.respondToServerRequest(request.id, { currentTimeAt: Math.floor(Date.now() / 1000) })) {
if (!context.actions.respondToServerRequest(request.id, serverRequestCurrentTimeResponse(Date.now()))) {
addSystemMessage(context, cannotSendCurrentTimeMessage());
}
}

View file

@ -1,10 +1,20 @@
import type { ServerRequest } from "../../../../../app-server/connection/rpc-messages";
import {
appServerApprovalRequest,
appServerApprovalResponse,
appServerMcpElicitationRequest,
appServerMcpElicitationResponse,
appServerUserInputRequest,
appServerUserInputResponse,
} from "../../../../../app-server/protocol/server-requests";
import type { PendingApproval, PendingMcpElicitation, PendingUserInput } from "../../../../../domain/pending-requests/model";
import type {
ApprovalAction,
McpElicitationAction,
McpElicitationContentValue,
PendingApproval,
PendingMcpElicitation,
PendingUserInput,
} from "../../../../../domain/pending-requests/model";
import {
type ActiveRouteScope,
fallbackMessageScope,
@ -89,6 +99,25 @@ export function routeServerRequest(request: ServerRequest, scope: ActiveRouteSco
}
}
export function serverRequestApprovalResponse(approval: PendingApproval, action: ApprovalAction): unknown {
return appServerApprovalResponse(approval, action);
}
export function serverRequestUserInputResponse(questions: readonly { id: string }[], answers: Record<string, string>): unknown {
return appServerUserInputResponse(questions, answers);
}
export function serverRequestMcpElicitationResponse(
action: McpElicitationAction,
content: Record<string, McpElicitationContentValue> | null,
): unknown {
return appServerMcpElicitationResponse(action, content);
}
export function serverRequestCurrentTimeResponse(currentTimeMs: number): unknown {
return { currentTimeAt: Math.floor(currentTimeMs / 1000) };
}
function serverRequestScope(request: ServerRequest): MessageScope {
if (!isServerRequest(request)) return fallbackMessageScope(request);
const extractor = SERVER_REQUEST_SCOPE_EXTRACTORS[request.method] as (request: ServerRequest) => MessageScope;

View file

@ -1,26 +0,0 @@
import {
appServerApprovalResponse,
appServerMcpElicitationResponse,
appServerUserInputResponse,
} from "../../../../../app-server/protocol/server-requests";
import type {
ApprovalAction,
McpElicitationAction,
McpElicitationContentValue,
PendingApproval,
} from "../../../../../domain/pending-requests/model";
export function serverRequestApprovalResponse(approval: PendingApproval, action: ApprovalAction): unknown {
return appServerApprovalResponse(approval, action);
}
export function serverRequestUserInputResponse(questions: readonly { id: string }[], answers: Record<string, string>): unknown {
return appServerUserInputResponse(questions, answers);
}
export function serverRequestMcpElicitationResponse(
action: McpElicitationAction,
content: Record<string, McpElicitationContentValue> | null,
): unknown {
return appServerMcpElicitationResponse(action, content);
}

View file

@ -173,12 +173,13 @@ async function forkThreadFromTurn(
try {
const sourceName = inheritedForkThreadName(threadId, threadManagementState(host).threadList.listedThreads);
const forkedThread = await forkThreadOnAppServer(scope.client, threadId, host.vaultPath);
let forkedThread = await forkThreadOnAppServer(scope.client, threadId, host.vaultPath);
if (threadManagementScopeClientStale(host, scope)) return;
const forkedThreadId = forkedThread.id;
if (turnsToDrop > 0) {
await rollbackThreadOnAppServer(scope.client, forkedThreadId, turnsToDrop);
const snapshot = await rollbackThreadOnAppServer(scope.client, forkedThreadId, turnsToDrop);
if (threadManagementScopeClientStale(host, scope)) return;
forkedThread = snapshot.thread;
}
host.applyThreadCatalogEvent({ type: "thread-forked", thread: forkedThread });
if (!threadManagementScopeStillTargetsOriginalPanel(host, scope)) return;

View file

@ -123,9 +123,11 @@ function applyThreadCatalogEvent(
store.setArchivedThreads(event.threads);
return;
case "thread-started":
case "thread-forked":
upsertActiveThread(store, activeFacts, event.thread, acknowledgeByThreadId);
return;
case "thread-forked":
upsertActiveThread(store, activeFacts, event.thread, acknowledgedByThreadVersion(event.thread));
return;
case "thread-touched":
applyThreadTouchedEvent(store, activeFacts, event.threadId, event.recencyAt);
return;
@ -329,6 +331,15 @@ function acknowledgedByName(name: string | null): (thread: Thread) => boolean {
return (thread) => thread.name === name;
}
function acknowledgedByThreadVersion(reference: Thread): (thread: Thread) => boolean {
return (thread) =>
thread.updatedAt > reference.updatedAt ||
(thread.updatedAt === reference.updatedAt &&
thread.preview === reference.preview &&
thread.name === reference.name &&
thread.recencyAt === reference.recencyAt);
}
function acknowledgedByRecency(recencyAt: number | null): (thread: Thread) => boolean {
return (thread) => thread.recencyAt === recencyAt;
}

View file

@ -8,7 +8,7 @@ import {
ROUTED_SERVER_NOTIFICATION_METHODS_BY_ROUTE_KIND,
routeServerNotification,
} from "../../../../../src/features/chat/app-server/inbound/notification-routing";
import { routeServerRequest } from "../../../../../src/features/chat/app-server/inbound/server-requests/routing";
import { routeServerRequest } from "../../../../../src/features/chat/app-server/inbound/server-requests/adapter";
import { chatStateFixture, chatStateWith } from "../../support/state";
const activeScope = { activeThreadId: "thread-active", activeTurnId: "turn-active" };

View file

@ -164,6 +164,19 @@ describe("thread management actions", () => {
it("forks from a selected turn by dropping later turns on the fork", async () => {
const client = clientMock();
client.forkThread.mockResolvedValue({
thread: {
...archivedThread(),
id: "forked",
sessionId: "forked",
name: "Fork before rollback",
preview: "Pre-rollback",
updatedAt: 10,
},
});
client.rollbackThread.mockResolvedValue({
thread: { ...rollbackThread(), preview: "Post-rollback", updatedAt: 20 },
});
const host = hostMock({ client, items: turnItems() });
const controller = threadManagementActions(host);
@ -173,7 +186,12 @@ describe("thread management actions", () => {
expect(client.rollbackThread).toHaveBeenCalledWith("forked", 2);
expect(host.applyThreadCatalogEvent).toHaveBeenCalledWith({
type: "thread-forked",
thread: expect.objectContaining({ id: "forked" }),
thread: expect.objectContaining({
id: "forked",
name: "Rolled Back Thread",
preview: "Post-rollback",
updatedAt: 20,
}),
});
expect(host.openThreadInNewView).toHaveBeenCalledWith("forked");
expect(client.archiveThread).not.toHaveBeenCalled();

View file

@ -351,7 +351,7 @@ export function timestamp(): number {
APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE,
APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE,
]);
expect(pluginMessages(report, "src/features/chat/app-server/inbound/server-requests/responses.ts")).toEqual([
expect(pluginMessages(report, "src/features/chat/app-server/inbound/server-requests/adapter.ts")).toEqual([
APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE,
]);
});
@ -661,7 +661,7 @@ export type Item = TurnItem;
`.trimStart(),
);
await writeFile(
path.join(cwd, "src/features/chat/app-server/inbound/server-requests/responses.ts"),
path.join(cwd, "src/features/chat/app-server/inbound/server-requests/adapter.ts"),
`
import { appServerUserInputResponse } from "../../../../../app-server/protocol/server-requests";
@ -825,7 +825,7 @@ export type AppServerThreadResumeClient = Pick<AppServerClient, "resumeThread">;
"src/features/chat/ui/protocol-leak.tsx",
"src/features/chat/app-server/inbound/app-server-logs.ts",
"src/features/chat/app-server/mappers/message-stream/turn-items.ts",
"src/features/chat/app-server/inbound/server-requests/responses.ts",
"src/features/chat/app-server/inbound/server-requests/adapter.ts",
"src/domain/threads/model.ts",
"src/domain/threads/format.ts",
"src/features/chat/domain/message-stream/selectors.ts",

View file

@ -176,6 +176,23 @@ describe("ThreadCatalog", () => {
expect(catalog.activeSnapshot()).toEqual([thread("other")]);
});
it("keeps rollback fork metadata until active snapshots catch up to the rollback version", () => {
const { catalog } = catalogFixture();
const forkBeforeRollback = thread("forked", false, { name: "Before", preview: "Before rollback", updatedAt: 20 });
const forkAfterRollback = thread("forked", false, { name: "After", preview: "After rollback", updatedAt: 20 });
const forkAfterFutureUpdate = thread("forked", false, { name: "Future", preview: "Future update", updatedAt: 21 });
catalog.apply({ type: "active-list-snapshot-received", threads: [thread("existing")] });
catalog.apply({ type: "thread-forked", thread: forkAfterRollback });
catalog.apply({ type: "active-list-snapshot-received", threads: [forkBeforeRollback, thread("existing")] });
expect(catalog.activeSnapshot()).toEqual([forkAfterRollback, thread("existing")]);
catalog.apply({ type: "active-list-snapshot-received", threads: [forkAfterFutureUpdate, thread("existing")] });
expect(catalog.activeSnapshot()).toEqual([forkAfterFutureUpdate, thread("existing")]);
});
it("keeps app-server rename facts when an older active list resolves later", async () => {
const staleRefresh = deferred<readonly Thread[]>();
const fetchThreads = vi