From d7bb49efbebc2cab158da62d3bf0278f77da9fca Mon Sep 17 00:00:00 2001 From: murashit Date: Fri, 26 Jun 2026 08:32:35 +0900 Subject: [PATCH] Refine chat app-server protocol lint boundaries --- biome.jsonc | 22 +-- ...-app-server-protocol-boundary-imports.grit | 12 +- ...t-app-server-request-protocol-imports.grit | 19 -- ...chat-app-server-turn-protocol-imports.grit | 19 -- .../chat/app-server/inbound/handler.ts | 18 +- .../app-server/inbound/notification-plan.ts | 11 +- .../{routing.ts => notification-routing.ts} | 179 ++---------------- .../chat/app-server/inbound/route-scope.ts | 40 ++++ .../inbound/server-requests/responses.ts | 26 +++ .../inbound/server-requests/routing.ts | 112 +++++++++++ .../mappers/message-stream/turn-items.ts | 15 +- .../chat/protocol/inbound/routing.test.ts | 4 +- tests/scripts/grit-policy.test.mjs | 98 ++++------ 13 files changed, 270 insertions(+), 305 deletions(-) delete mode 100644 scripts/lint/no-chat-app-server-request-protocol-imports.grit delete mode 100644 scripts/lint/no-chat-app-server-turn-protocol-imports.grit rename src/features/chat/app-server/inbound/{routing.ts => notification-routing.ts} (60%) create mode 100644 src/features/chat/app-server/inbound/route-scope.ts create mode 100644 src/features/chat/app-server/inbound/server-requests/responses.ts create mode 100644 src/features/chat/app-server/inbound/server-requests/routing.ts diff --git a/biome.jsonc b/biome.jsonc index b49789db..bb4746b1 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -13,27 +13,7 @@ }, { "path": "./scripts/lint/no-app-server-protocol-boundary-imports.grit", - "includes": [ - "**/src/**/*.ts", - "**/src/**/*.tsx", - "!**/src/app-server/**/*.ts", - "!**/src/app-server/**/*.tsx", - "!**/src/features/chat/app-server/inbound/notification-plan.ts", - "!**/src/features/chat/app-server/mappers/message-stream/turn-items.ts", - "!**/src/features/chat/app-server/inbound/handler.ts", - "!**/src/features/chat/app-server/inbound/routing.ts" - ] - }, - { - "path": "./scripts/lint/no-chat-app-server-turn-protocol-imports.grit", - "includes": [ - "**/src/features/chat/app-server/inbound/notification-plan.ts", - "**/src/features/chat/app-server/mappers/message-stream/turn-items.ts" - ] - }, - { - "path": "./scripts/lint/no-chat-app-server-request-protocol-imports.grit", - "includes": ["**/src/features/chat/app-server/inbound/handler.ts", "**/src/features/chat/app-server/inbound/routing.ts"] + "includes": ["**/src/**/*.ts", "**/src/**/*.tsx", "!**/src/app-server/**/*.ts", "!**/src/app-server/**/*.tsx"] }, { "path": "./scripts/lint/no-generated-app-server-boundary-imports.grit", diff --git a/scripts/lint/no-app-server-protocol-boundary-imports.grit b/scripts/lint/no-app-server-protocol-boundary-imports.grit index a130238d..e31037d1 100644 --- a/scripts/lint/no-app-server-protocol-boundary-imports.grit +++ b/scripts/lint/no-app-server-protocol-boundary-imports.grit @@ -12,7 +12,15 @@ private pattern js_module_reference() { js_module_reference() as $stmt where { $stmt <: contains `$source` where { - $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol|src/app-server/protocol)/[^/\"']+(?:/.*)?[\"']$" + $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol|src/app-server/protocol)/[^/\"']+(?:/.*)?[\"']$", + not { + $filename <: r".*/src/features/chat/app-server/mappers/message-stream/turn-items\.ts$", + $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol/turn|src/app-server/protocol/turn)(?:/.*)?[\"']$" + }, + not { + $filename <: r".*/src/features/chat/app-server/inbound/server-requests/.*", + $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol/server-requests|src/app-server/protocol/server-requests)(?:/.*)?[\"']$" + } }, - register_diagnostic(span=$stmt, message="Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat ingestion and message-stream conversion may consume app-server turn protocol at the boundary; feature state and UI must use Panel-owned models.", severity="error") + register_diagnostic(span=$stmt, message="Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat turn-item conversion may consume turn protocol, and chat request handling may consume server request protocol at their app-server boundaries; feature state and UI must use Panel-owned models.", severity="error") } diff --git a/scripts/lint/no-chat-app-server-request-protocol-imports.grit b/scripts/lint/no-chat-app-server-request-protocol-imports.grit deleted file mode 100644 index de505aab..00000000 --- a/scripts/lint/no-chat-app-server-request-protocol-imports.grit +++ /dev/null @@ -1,19 +0,0 @@ -language js - -private pattern js_module_reference() { - or { - JsImport(), - JsExportNamedFromClause(), - JsExportFromClause(), - TsImportType(), - JsImportCallExpression() - } -} - -js_module_reference() as $stmt where { - $stmt <: contains `$source` where { - $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol|src/app-server/protocol)/[^/\"']+(?:/.*)?[\"']$", - not { $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol/server-requests|src/app-server/protocol/server-requests)(?:/.*)?[\"']$" } - }, - register_diagnostic(span=$stmt, message="Chat app-server request handling may consume server request protocol projections only. Convert app-server payloads to chat pending request domain models at this boundary.", severity="error") -} diff --git a/scripts/lint/no-chat-app-server-turn-protocol-imports.grit b/scripts/lint/no-chat-app-server-turn-protocol-imports.grit deleted file mode 100644 index 56b5a4b4..00000000 --- a/scripts/lint/no-chat-app-server-turn-protocol-imports.grit +++ /dev/null @@ -1,19 +0,0 @@ -language js - -private pattern js_module_reference() { - or { - JsImport(), - JsExportNamedFromClause(), - JsExportFromClause(), - TsImportType(), - JsImportCallExpression() - } -} - -js_module_reference() as $stmt where { - $stmt <: contains `$source` where { - $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol|src/app-server/protocol)/[^/\"']+(?:/.*)?[\"']$", - not { $source <: r"^[\"'](?:(?:\.\./)+app-server/protocol/turn|src/app-server/protocol/turn)(?:/.*)?[\"']$" } - }, - register_diagnostic(span=$stmt, message="Chat app-server ingestion and message-stream conversion may consume the app-server turn protocol only. Convert other protocol payloads to local or domain models at the boundary.", severity="error") -} diff --git a/src/features/chat/app-server/inbound/handler.ts b/src/features/chat/app-server/inbound/handler.ts index 4be2a84a..c09feeea 100644 --- a/src/features/chat/app-server/inbound/handler.ts +++ b/src/features/chat/app-server/inbound/handler.ts @@ -1,9 +1,4 @@ import type { RequestId, ServerNotification, ServerRequest } from "../../../../app-server/connection/rpc-messages"; -import { - appServerApprovalResponse, - appServerMcpElicitationResponse, - appServerUserInputResponse, -} from "../../../../app-server/protocol/server-requests"; import { type ApprovalAction, contentForPendingMcpElicitation, @@ -29,7 +24,12 @@ import { import type { AppServerResourceEvent } from "../actions/metadata"; import { classifyAppServerLog } from "./app-server-logs"; import { type ChatNotificationEffect, planChatNotification } from "./notification-plan"; -import { routeServerRequest } from "./routing"; +import { + serverRequestApprovalResponse, + serverRequestMcpElicitationResponse, + serverRequestUserInputResponse, +} from "./server-requests/responses"; +import { routeServerRequest } from "./server-requests/routing"; function cannotSendApprovalResponseMessage(): string { return "Could not send approval response because Codex app-server is not connected."; @@ -181,7 +181,7 @@ function handleAppServerLog(context: ChatInboundHandlerContext, message: string) function resolveApproval(context: ChatInboundHandlerContext, requestId: PendingRequestId, action: ApprovalAction): void { const approval = pendingApproval(context, requestId); if (!approval) return; - if (!context.actions.respondToServerRequest(approval.requestId, appServerApprovalResponse(approval, action))) { + if (!context.actions.respondToServerRequest(approval.requestId, serverRequestApprovalResponse(approval, action))) { addSystemMessage(context, cannotSendApprovalResponseMessage()); return; } @@ -191,7 +191,7 @@ function resolveApproval(context: ChatInboundHandlerContext, requestId: PendingR function resolveUserInput(context: ChatInboundHandlerContext, requestId: PendingRequestId, answers: Record): void { const input = pendingUserInput(context, requestId); if (!input) return; - if (!context.actions.respondToServerRequest(input.requestId, appServerUserInputResponse(input.params.questions, answers))) { + if (!context.actions.respondToServerRequest(input.requestId, serverRequestUserInputResponse(input.params.questions, answers))) { addSystemMessage(context, cannotSendUserInputMessage()); return; } @@ -220,7 +220,7 @@ function resolveMcpElicitation(context: ChatInboundHandlerContext, requestId: Pe const elicitation = pendingMcpElicitation(context, requestId); if (!elicitation) return; const content = action === "accept" ? contentForPendingMcpElicitation(elicitation, state(context).requests.mcpElicitationDrafts) : null; - if (!context.actions.respondToServerRequest(elicitation.requestId, appServerMcpElicitationResponse(action, content))) { + if (!context.actions.respondToServerRequest(elicitation.requestId, serverRequestMcpElicitationResponse(action, content))) { addSystemMessage(context, cannotSendMcpElicitationMessage()); return; } diff --git a/src/features/chat/app-server/inbound/notification-plan.ts b/src/features/chat/app-server/inbound/notification-plan.ts index 3feecc66..5d584b46 100644 --- a/src/features/chat/app-server/inbound/notification-plan.ts +++ b/src/features/chat/app-server/inbound/notification-plan.ts @@ -1,5 +1,4 @@ import type { ServerNotification } from "../../../../app-server/connection/rpc-messages"; -import { completedConversationSummaryFromTurnRecord, type TurnItem } from "../../../../app-server/protocol/turn"; import { threadFromAppServerRecord } from "../../../../app-server/threads"; import { threadTokenUsageFromRuntimeUsage } from "../../../../domain/runtime/metrics"; import { normalizeExplicitThreadName } from "../../../../domain/threads/model"; @@ -30,6 +29,8 @@ import { hookRunMessageStreamItem } from "../mappers/message-stream/hook-run-ite import { createAutoReviewResultItem, createReviewResultItem } from "../mappers/message-stream/review-result-items"; import { taskProgressMessageStreamItem } from "../mappers/message-stream/task-progress"; import { + type AppServerTurnItem, + completedConversationSummaryFromAppServerTurn, messageStreamItemFromTurnItem, messageStreamItemsFromTurns, shouldSuppressLifecycleItem, @@ -46,7 +47,7 @@ import { type TurnLifecycleNotificationMethod, type UserVisibleNoticeNotification, type UserVisibleNoticeNotificationMethod, -} from "./routing"; +} from "./notification-routing"; export type ChatNotificationEffect = | { type: "refresh-threads" } @@ -243,7 +244,7 @@ const TURN_LIFECYCLE_PLANNERS = { type: "maybe-name-thread", threadId: notification.params.threadId, turnId: notification.params.turn.id, - completedSummary: completedConversationSummaryFromTurnRecord(notification.params.turn), + completedSummary: completedConversationSummaryFromAppServerTurn(notification.params.turn), }, { type: "refresh-threads" }, ], @@ -427,13 +428,13 @@ function autoApprovalReviewPlan( }); } -function startedItemPlan(item: TurnItem, turnId: string): ChatNotificationPlan { +function startedItemPlan(item: AppServerTurnItem, turnId: string): ChatNotificationPlan { if (shouldSuppressLifecycleItem(item)) return EMPTY_PLAN; const streamItem = messageStreamItemFromTurnItem(item, turnId); return streamItem ? actionPlan({ type: "message-stream/item-upserted", item: streamItem }) : EMPTY_PLAN; } -function completedItemPlan(item: TurnItem, turnId: string): ChatNotificationPlan { +function completedItemPlan(item: AppServerTurnItem, turnId: string): ChatNotificationPlan { if (item.type === "userMessage") return EMPTY_PLAN; const streamItem = messageStreamItemFromTurnItem(item, turnId); if (!streamItem) return EMPTY_PLAN; diff --git a/src/features/chat/app-server/inbound/routing.ts b/src/features/chat/app-server/inbound/notification-routing.ts similarity index 60% rename from src/features/chat/app-server/inbound/routing.ts rename to src/features/chat/app-server/inbound/notification-routing.ts index 53268151..df53c757 100644 --- a/src/features/chat/app-server/inbound/routing.ts +++ b/src/features/chat/app-server/inbound/notification-routing.ts @@ -1,27 +1,13 @@ -import type { ServerNotification, ServerRequest } from "../../../../app-server/connection/rpc-messages"; +import type { ServerNotification } from "../../../../app-server/connection/rpc-messages"; import { - appServerApprovalRequest, - appServerMcpElicitationRequest, - appServerUserInputRequest, -} from "../../../../app-server/protocol/server-requests"; -import type { PendingApproval, PendingMcpElicitation, PendingUserInput } from "../../../../domain/pending-requests/model"; - -export interface ActiveRouteScope { - activeThreadId: string | null; - activeTurnId: string | null; -} - -export type ServerRequestRoute = - | { kind: "approval"; request: ServerRequest; approval: PendingApproval } - | { kind: "userInput"; request: ServerRequest; input: PendingUserInput } - | { kind: "mcpElicitation"; request: ServerRequest; elicitation: PendingMcpElicitation } - | { kind: "currentTime"; request: Extract } - | { kind: "unsupported"; request: ServerRequest } - | { kind: "unknown"; request: ServerRequest } - | { kind: "inactive"; request: ServerRequest }; + type ActiveRouteScope, + fallbackMessageScope, + isMessageScopeInActiveRouteScope, + isTurnScopedMessageForIdleActiveThread, + type MessageScope, +} from "./route-scope"; type ServerNotificationMethod = ServerNotification["method"]; -type ServerRequestMethod = ServerRequest["method"]; type RoutedNotification = Extract; export type StreamUpdateNotification = RoutedNotification; export type TurnLifecycleNotification = RoutedNotification; @@ -40,18 +26,9 @@ export type ServerNotificationRoute = | { kind: "unhandled"; notification: ServerNotification } | { kind: "inactive"; notification: ServerNotification }; -interface MessageScope { - threadId: string | null; - turnId: string | null; -} - -type ServerRequestRouteKindByMethod = Record>; type ServerNotificationScopeExtractors = { [Method in ServerNotificationMethod]: (notification: Extract) => MessageScope; }; -type ServerRequestScopeExtractors = { - [Method in ServerRequestMethod]: (request: Extract) => MessageScope; -}; const GLOBALLY_ROUTED_THREAD_CATALOG_NOTIFICATION_METHODS = [ "thread/started", @@ -202,70 +179,11 @@ const SERVER_NOTIFICATION_SCOPE_EXTRACTORS: ServerNotificationScopeExtractors = "account/login/completed": unscopedNotificationScope, }; -const SERVER_REQUEST_SCOPE_EXTRACTORS: ServerRequestScopeExtractors = { - "item/commandExecution/requestApproval": threadTurnRequestScope, - "item/fileChange/requestApproval": threadTurnRequestScope, - "item/tool/requestUserInput": threadTurnRequestScope, - "mcpServer/elicitation/request": threadTurnRequestScope, - "item/permissions/requestApproval": threadTurnRequestScope, - "item/tool/call": threadTurnRequestScope, - "account/chatgptAuthTokens/refresh": unscopedRequestScope, - "attestation/generate": unscopedRequestScope, - "currentTime/read": threadOnlyRequestScope, - applyPatchApproval: unscopedRequestScope, - execCommandApproval: unscopedRequestScope, -}; - -const SERVER_REQUEST_ROUTE_KIND_BY_METHOD: ServerRequestRouteKindByMethod = { - "item/commandExecution/requestApproval": "approval", - "item/fileChange/requestApproval": "approval", - "item/permissions/requestApproval": "approval", - "item/tool/requestUserInput": "userInput", - "mcpServer/elicitation/request": "mcpElicitation", - "item/tool/call": "unsupported", - "account/chatgptAuthTokens/refresh": "unsupported", - "attestation/generate": "unsupported", - "currentTime/read": "currentTime", - applyPatchApproval: "unsupported", - execCommandApproval: "unsupported", -}; - -export function routeServerRequest(request: ServerRequest, scope: ActiveRouteScope): ServerRequestRoute { - if (!isServerRequest(request)) { - if (!isMessageInActiveScope(request, scope)) return { kind: "inactive", request }; - if (isTurnScopedMessageForIdleActiveThread(request, scope)) return { kind: "inactive", request }; - return { kind: "unknown", request }; - } - if (!isMessageInActiveScope(request, scope)) return { kind: "inactive", request }; - if (isTurnScopedMessageForIdleActiveThread(request, scope)) return { kind: "inactive", request }; - - switch (SERVER_REQUEST_ROUTE_KIND_BY_METHOD[request.method]) { - case "approval": { - const approval = appServerApprovalRequest(request); - if (approval) return { kind: "approval", request, approval }; - return { kind: "unsupported", request }; - } - case "userInput": { - const input = appServerUserInputRequest(request); - if (input) return { kind: "userInput", request, input }; - return { kind: "unsupported", request }; - } - case "mcpElicitation": { - const elicitation = appServerMcpElicitationRequest(request); - if (elicitation) return { kind: "mcpElicitation", request, elicitation }; - return { kind: "unsupported", request }; - } - case "currentTime": - return { kind: "currentTime", request: request as Extract }; - case "unsupported": - return { kind: "unsupported", request }; - } -} - export function routeServerNotification(notification: ServerNotification, scope: ActiveRouteScope): ServerNotificationRoute { if (isThreadCatalogNotification(notification)) return { kind: "threadLifecycle", notification }; - if (!isMessageInActiveScope(notification, scope)) return { kind: "inactive", notification }; - if (isIdleThreadStreamUpdate(notification, scope)) return { kind: "inactive", notification }; + const messageScope = serverNotificationScope(notification); + if (!isMessageScopeInActiveRouteScope(messageScope, scope)) return { kind: "inactive", notification }; + if (isIdleThreadStreamUpdate(notification, messageScope, scope)) return { kind: "inactive", notification }; if (isStreamUpdateNotification(notification)) return { kind: "streamUpdate", notification }; if (isTurnLifecycleNotification(notification)) return { kind: "turnLifecycle", notification }; @@ -276,59 +194,14 @@ export function routeServerNotification(notification: ServerNotification, scope: return { kind: "unhandled", notification }; } -function isMessageInActiveScope(message: ServerNotification | ServerRequest, scope: ActiveRouteScope): boolean { - const threadId = messageThreadId(message); - // Scope identifiers are filters only when both the notification and the - // active panel have one. Thread catalog and idle-thread notifications often - // omit active turn scope, so missing ids stay eligible for the active route. - if (threadId && scope.activeThreadId && threadId !== scope.activeThreadId) return false; - - const turnId = messageTurnId(message); - if (turnId && scope.activeTurnId && turnId !== scope.activeTurnId) return false; - - return true; -} - -function messageThreadId(message: ServerNotification | ServerRequest): string | null { - if (isServerRequest(message)) return serverRequestScope(message).threadId; - if (isServerNotification(message)) return serverNotificationScope(message).threadId; - return fallbackMessageScope(message).threadId; -} - -function messageTurnId(message: ServerNotification | ServerRequest): string | null { - if (isServerRequest(message)) return serverRequestScope(message).turnId; - if (isServerNotification(message)) return serverNotificationScope(message).turnId; - return fallbackMessageScope(message).turnId; -} - function serverNotificationScope(notification: ServerNotification): MessageScope { + if (!isServerNotification(notification)) return fallbackMessageScope(notification); const extractor = SERVER_NOTIFICATION_SCOPE_EXTRACTORS[notification.method] as (notification: ServerNotification) => MessageScope; return extractor(notification); } -function serverRequestScope(request: ServerRequest): MessageScope { - const extractor = SERVER_REQUEST_SCOPE_EXTRACTORS[request.method] as (request: ServerRequest) => MessageScope; - return extractor(request); -} - -function isServerRequest(message: ServerNotification | ServerRequest): message is ServerRequest { - return Object.hasOwn(SERVER_REQUEST_SCOPE_EXTRACTORS, message.method); -} - -function isServerNotification(message: ServerNotification | ServerRequest): message is ServerNotification { - return Object.hasOwn(SERVER_NOTIFICATION_SCOPE_EXTRACTORS, message.method); -} - -function threadTurnRequestScope(request: { params: { threadId: string; turnId: string | null } }): MessageScope { - return { threadId: request.params.threadId, turnId: request.params.turnId }; -} - -function threadOnlyRequestScope(request: { params: { threadId: string | null } }): MessageScope { - return { threadId: request.params.threadId, turnId: null }; -} - -function unscopedRequestScope(): MessageScope { - return { threadId: null, turnId: null }; +function isServerNotification(notification: ServerNotification): boolean { + return Object.hasOwn(SERVER_NOTIFICATION_SCOPE_EXTRACTORS, notification.method); } function threadStartedNotificationScope(notification: { params: { thread: { id: string } } }): MessageScope { @@ -351,24 +224,6 @@ function unscopedNotificationScope(): MessageScope { return { threadId: null, turnId: null }; } -function fallbackMessageScope(message: ServerNotification | ServerRequest): MessageScope { - const params = messageParams(message); - return { - threadId: stringParam(params, "threadId"), - turnId: stringParam(params, "turnId"), - }; -} - -function messageParams(message: { params?: unknown }): Record | null { - const params = message.params; - return params !== null && typeof params === "object" && !Array.isArray(params) ? (params as Record) : null; -} - -function stringParam(params: Record | null, key: string): string | null { - const value = params?.[key]; - return typeof value === "string" && value.length > 0 ? value : null; -} - function isThreadCatalogNotification(notification: ServerNotification): notification is ThreadLifecycleNotification { return notificationMethodIn(notification.method, GLOBALLY_ROUTED_THREAD_CATALOG_NOTIFICATION_METHODS); } @@ -377,12 +232,8 @@ function isStreamUpdateNotification(notification: ServerNotification): notificat return notificationMethodIn(notification.method, STREAM_UPDATE_NOTIFICATION_METHODS); } -function isIdleThreadStreamUpdate(notification: ServerNotification, scope: ActiveRouteScope): boolean { - return isTurnScopedMessageForIdleActiveThread(notification, scope) && isStreamUpdateNotification(notification); -} - -function isTurnScopedMessageForIdleActiveThread(message: ServerNotification | ServerRequest, scope: ActiveRouteScope): boolean { - return scope.activeThreadId !== null && scope.activeTurnId === null && messageTurnId(message) !== null; +function isIdleThreadStreamUpdate(notification: ServerNotification, messageScope: MessageScope, scope: ActiveRouteScope): boolean { + return isTurnScopedMessageForIdleActiveThread(messageScope, scope) && isStreamUpdateNotification(notification); } function isTurnLifecycleNotification(notification: ServerNotification): notification is TurnLifecycleNotification { diff --git a/src/features/chat/app-server/inbound/route-scope.ts b/src/features/chat/app-server/inbound/route-scope.ts new file mode 100644 index 00000000..e0ff8f52 --- /dev/null +++ b/src/features/chat/app-server/inbound/route-scope.ts @@ -0,0 +1,40 @@ +export interface ActiveRouteScope { + activeThreadId: string | null; + activeTurnId: string | null; +} + +export interface MessageScope { + threadId: string | null; + turnId: string | null; +} + +export function isMessageScopeInActiveRouteScope(messageScope: MessageScope, activeScope: ActiveRouteScope): boolean { + // Scope identifiers are filters only when both the message and the active + // panel have one. Thread catalog and idle-thread notifications often omit + // active turn scope, so missing ids stay eligible for the active route. + if (messageScope.threadId && activeScope.activeThreadId && messageScope.threadId !== activeScope.activeThreadId) return false; + if (messageScope.turnId && activeScope.activeTurnId && messageScope.turnId !== activeScope.activeTurnId) return false; + return true; +} + +export function isTurnScopedMessageForIdleActiveThread(messageScope: MessageScope, activeScope: ActiveRouteScope): boolean { + return activeScope.activeThreadId !== null && activeScope.activeTurnId === null && messageScope.turnId !== null; +} + +export function fallbackMessageScope(message: { params?: unknown }): MessageScope { + const params = messageParams(message); + return { + threadId: stringParam(params, "threadId"), + turnId: stringParam(params, "turnId"), + }; +} + +function messageParams(message: { params?: unknown }): Record | null { + const params = message.params; + return params !== null && typeof params === "object" && !Array.isArray(params) ? (params as Record) : null; +} + +function stringParam(params: Record | null, key: string): string | null { + const value = params?.[key]; + return typeof value === "string" && value.length > 0 ? value : null; +} diff --git a/src/features/chat/app-server/inbound/server-requests/responses.ts b/src/features/chat/app-server/inbound/server-requests/responses.ts new file mode 100644 index 00000000..ba7c12ef --- /dev/null +++ b/src/features/chat/app-server/inbound/server-requests/responses.ts @@ -0,0 +1,26 @@ +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): unknown { + return appServerUserInputResponse(questions, answers); +} + +export function serverRequestMcpElicitationResponse( + action: McpElicitationAction, + content: Record | null, +): unknown { + return appServerMcpElicitationResponse(action, content); +} diff --git a/src/features/chat/app-server/inbound/server-requests/routing.ts b/src/features/chat/app-server/inbound/server-requests/routing.ts new file mode 100644 index 00000000..6a706fce --- /dev/null +++ b/src/features/chat/app-server/inbound/server-requests/routing.ts @@ -0,0 +1,112 @@ +import type { ServerRequest } from "../../../../../app-server/connection/rpc-messages"; +import { + appServerApprovalRequest, + appServerMcpElicitationRequest, + appServerUserInputRequest, +} from "../../../../../app-server/protocol/server-requests"; +import type { PendingApproval, PendingMcpElicitation, PendingUserInput } from "../../../../../domain/pending-requests/model"; +import { + type ActiveRouteScope, + fallbackMessageScope, + isMessageScopeInActiveRouteScope, + isTurnScopedMessageForIdleActiveThread, + type MessageScope, +} from "../route-scope"; + +export type ServerRequestRoute = + | { kind: "approval"; request: ServerRequest; approval: PendingApproval } + | { kind: "userInput"; request: ServerRequest; input: PendingUserInput } + | { kind: "mcpElicitation"; request: ServerRequest; elicitation: PendingMcpElicitation } + | { kind: "currentTime"; request: Extract } + | { kind: "unsupported"; request: ServerRequest } + | { kind: "unknown"; request: ServerRequest } + | { kind: "inactive"; request: ServerRequest }; + +type ServerRequestMethod = ServerRequest["method"]; +type ServerRequestRouteKindByMethod = Record>; +type ServerRequestScopeExtractors = { + [Method in ServerRequestMethod]: (request: Extract) => MessageScope; +}; + +const SERVER_REQUEST_SCOPE_EXTRACTORS: ServerRequestScopeExtractors = { + "item/commandExecution/requestApproval": threadTurnRequestScope, + "item/fileChange/requestApproval": threadTurnRequestScope, + "item/tool/requestUserInput": threadTurnRequestScope, + "mcpServer/elicitation/request": threadTurnRequestScope, + "item/permissions/requestApproval": threadTurnRequestScope, + "item/tool/call": threadTurnRequestScope, + "account/chatgptAuthTokens/refresh": unscopedRequestScope, + "attestation/generate": unscopedRequestScope, + "currentTime/read": threadOnlyRequestScope, + applyPatchApproval: unscopedRequestScope, + execCommandApproval: unscopedRequestScope, +}; + +const SERVER_REQUEST_ROUTE_KIND_BY_METHOD: ServerRequestRouteKindByMethod = { + "item/commandExecution/requestApproval": "approval", + "item/fileChange/requestApproval": "approval", + "item/permissions/requestApproval": "approval", + "item/tool/requestUserInput": "userInput", + "mcpServer/elicitation/request": "mcpElicitation", + "item/tool/call": "unsupported", + "account/chatgptAuthTokens/refresh": "unsupported", + "attestation/generate": "unsupported", + "currentTime/read": "currentTime", + applyPatchApproval: "unsupported", + execCommandApproval: "unsupported", +}; + +export function routeServerRequest(request: ServerRequest, scope: ActiveRouteScope): ServerRequestRoute { + const messageScope = serverRequestScope(request); + if (!isServerRequest(request)) { + if (!isMessageScopeInActiveRouteScope(messageScope, scope)) return { kind: "inactive", request }; + if (isTurnScopedMessageForIdleActiveThread(messageScope, scope)) return { kind: "inactive", request }; + return { kind: "unknown", request }; + } + if (!isMessageScopeInActiveRouteScope(messageScope, scope)) return { kind: "inactive", request }; + if (isTurnScopedMessageForIdleActiveThread(messageScope, scope)) return { kind: "inactive", request }; + + switch (SERVER_REQUEST_ROUTE_KIND_BY_METHOD[request.method]) { + case "approval": { + const approval = appServerApprovalRequest(request); + if (approval) return { kind: "approval", request, approval }; + return { kind: "unsupported", request }; + } + case "userInput": { + const input = appServerUserInputRequest(request); + if (input) return { kind: "userInput", request, input }; + return { kind: "unsupported", request }; + } + case "mcpElicitation": { + const elicitation = appServerMcpElicitationRequest(request); + if (elicitation) return { kind: "mcpElicitation", request, elicitation }; + return { kind: "unsupported", request }; + } + case "currentTime": + return { kind: "currentTime", request: request as Extract }; + case "unsupported": + return { kind: "unsupported", request }; + } +} + +function serverRequestScope(request: ServerRequest): MessageScope { + if (!isServerRequest(request)) return fallbackMessageScope(request); + const extractor = SERVER_REQUEST_SCOPE_EXTRACTORS[request.method] as (request: ServerRequest) => MessageScope; + return extractor(request); +} + +function isServerRequest(request: ServerRequest): boolean { + return Object.hasOwn(SERVER_REQUEST_SCOPE_EXTRACTORS, request.method); +} + +function threadTurnRequestScope(request: { params: { threadId: string; turnId: string | null } }): MessageScope { + return { threadId: request.params.threadId, turnId: request.params.turnId }; +} + +function threadOnlyRequestScope(request: { params: { threadId: string | null } }): MessageScope { + return { threadId: request.params.threadId, turnId: null }; +} + +function unscopedRequestScope(): MessageScope { + return { threadId: null, turnId: null }; +} diff --git a/src/features/chat/app-server/mappers/message-stream/turn-items.ts b/src/features/chat/app-server/mappers/message-stream/turn-items.ts index 66dd247c..fc6cf327 100644 --- a/src/features/chat/app-server/mappers/message-stream/turn-items.ts +++ b/src/features/chat/app-server/mappers/message-stream/turn-items.ts @@ -1,7 +1,12 @@ -import type { TurnItem } from "../../../../../app-server/protocol/turn"; -import { turnUserItemText } from "../../../../../app-server/protocol/turn"; +import { + completedConversationSummaryFromTurnRecord, + type TurnItem, + type TurnRecord, + turnUserItemText, +} from "../../../../../app-server/protocol/turn"; import type { HistoricalTurn } from "../../../../../domain/threads/history"; import { referencedThreadMetadataFromPrompt } from "../../../../../domain/threads/reference"; +import type { ThreadConversationSummary } from "../../../../../domain/threads/transcript"; import { jsonPreview } from "../../../../../shared/text/preview"; import { commandExecutionState, @@ -41,6 +46,12 @@ interface TurnItemSourceFields { sourceItemId: string; } +export type AppServerTurnItem = TurnItem; + +export function completedConversationSummaryFromAppServerTurn(turn: TurnRecord): ThreadConversationSummary | null { + return completedConversationSummaryFromTurnRecord(turn); +} + export function messageStreamItemsFromTurns(turns: readonly HistoricalTurn[]): MessageStreamItem[] { const sortedTurns = [...turns].sort((a, b) => (a.startedAt ?? 0) - (b.startedAt ?? 0)); const items: MessageStreamItem[] = []; diff --git a/tests/features/chat/protocol/inbound/routing.test.ts b/tests/features/chat/protocol/inbound/routing.test.ts index 7b257c1c..ba3531ed 100644 --- a/tests/features/chat/protocol/inbound/routing.test.ts +++ b/tests/features/chat/protocol/inbound/routing.test.ts @@ -7,8 +7,8 @@ import { import { ROUTED_SERVER_NOTIFICATION_METHODS_BY_ROUTE_KIND, routeServerNotification, - routeServerRequest, -} from "../../../../../src/features/chat/app-server/inbound/routing"; +} from "../../../../../src/features/chat/app-server/inbound/notification-routing"; +import { routeServerRequest } from "../../../../../src/features/chat/app-server/inbound/server-requests/routing"; import { chatStateFixture, chatStateWith } from "../../support/state"; const activeScope = { activeThreadId: "thread-active", activeTurnId: "turn-active" }; diff --git a/tests/scripts/grit-policy.test.mjs b/tests/scripts/grit-policy.test.mjs index 560f3f4e..e093b3f3 100644 --- a/tests/scripts/grit-policy.test.mjs +++ b/tests/scripts/grit-policy.test.mjs @@ -14,6 +14,8 @@ const projectPluginByName = new Map( return [path.basename(pluginPath), plugin]; }), ); +const APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE = + "Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat turn-item conversion may consume turn protocol, and chat request handling may consume server request protocol at their app-server boundaries; feature state and UI must use Panel-owned models."; describe("GritQL source policy", () => { it("keeps project-wide source-shape policies enforceable as Biome plugin diagnostics", async () => { @@ -466,6 +468,28 @@ export const response = appServerUserInputResponse; import { appServerUserInputResponse } from "../../../../app-server/protocol/server-requests"; export const response = appServerUserInputResponse; +`.trimStart(), + ); + await writeFile( + path.join(cwd, "src/features/chat/app-server/mappers/message-stream/turn-items.ts"), + ` +import type { TurnItem } from "../../../../../app-server/protocol/turn"; +import { toolInventoryAppsFromAppInfos } from "../../../../../app-server/protocol/tool-inventory"; + +const toolInventory = await import("../../../../../app-server/protocol/tool-inventory"); + +export const convert = [toolInventoryAppsFromAppInfos, toolInventory] satisfies unknown[]; +export type Item = TurnItem; +`.trimStart(), + ); + await writeFile( + path.join(cwd, "src/features/chat/app-server/inbound/server-requests/responses.ts"), + ` +import { appServerUserInputResponse } from "../../../../../app-server/protocol/server-requests"; + +const runtimeMetrics = await import("../../../../../app-server/protocol/runtime-metrics"); + +export const response = [appServerUserInputResponse, runtimeMetrics] satisfies unknown[]; `.trimStart(), ); @@ -475,82 +499,31 @@ export const response = appServerUserInputResponse; "src/features/chat/application/threads/history-controller.ts", "src/features/chat/panel/surface/message-stream-presenter.ts", "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", ], cwd, ); expect(pluginMessages(report, "src/features/chat/application/pending-requests/pending-request-actions.ts")).toEqual([ - "Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat ingestion and message-stream conversion may consume app-server turn protocol at the boundary; feature state and UI must use Panel-owned models.", + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); expect(pluginMessages(report, "src/features/chat/application/threads/history-controller.ts")).toEqual([ - "Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat ingestion and message-stream conversion may consume app-server turn protocol at the boundary; feature state and UI must use Panel-owned models.", + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); expect(pluginMessages(report, "src/features/chat/panel/surface/message-stream-presenter.ts")).toEqual([ - "Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat ingestion and message-stream conversion may consume app-server turn protocol at the boundary; feature state and UI must use Panel-owned models.", + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); expect(pluginMessages(report, "src/features/chat/app-server/inbound/app-server-logs.ts")).toEqual([ - "Source modules outside app-server must use domain models and app-server services instead of app-server protocol modules. Chat ingestion and message-stream conversion may consume app-server turn protocol at the boundary; feature state and UI must use Panel-owned models.", + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); - }); - - it("keeps chat app-server ingestion on app-server turn protocol only", async () => { - const cwd = await tempBiomeWorkspace(["no-chat-app-server-turn-protocol-imports.grit"]); - await writeFile( - path.join(cwd, "src/features/chat/app-server/inbound/notification-plan.ts"), - ` -import { toolInventoryAppsFromAppInfos } from "../../../../app-server/protocol/tool-inventory"; - -const toolInventory = await import("../../../../app-server/protocol/tool-inventory"); - -export const convert = [toolInventoryAppsFromAppInfos, toolInventory]; -`.trimStart(), - ); - await writeFile( - path.join(cwd, "src/features/chat/app-server/mappers/message-stream/turn-items.ts"), - ` -import type { TurnItem } from "../../../../../app-server/protocol/turn"; - -export type Item = TurnItem; -`.trimStart(), - ); - - const report = biomeLint( - ["src/features/chat/app-server/inbound/notification-plan.ts", "src/features/chat/app-server/mappers/message-stream/turn-items.ts"], - cwd, - ); - - expect(pluginMessages(report, "src/features/chat/app-server/inbound/notification-plan.ts")).toEqual([ - "Chat app-server ingestion and message-stream conversion may consume the app-server turn protocol only. Convert other protocol payloads to local or domain models at the boundary.", - "Chat app-server ingestion and message-stream conversion may consume the app-server turn protocol only. Convert other protocol payloads to local or domain models at the boundary.", + expect(pluginMessages(report, "src/features/chat/app-server/mappers/message-stream/turn-items.ts")).toEqual([ + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); - expect(pluginDiagnostics(report, "src/features/chat/app-server/mappers/message-stream/turn-items.ts")).toEqual([]); - }); - - it("keeps chat app-server request handling on server request protocol only", async () => { - const cwd = await tempBiomeWorkspace(["no-chat-app-server-request-protocol-imports.grit"]); - await writeFile( - path.join(cwd, "src/features/chat/app-server/inbound/handler.ts"), - ` -const runtimeMetrics = await import("../../../../app-server/protocol/runtime-metrics"); - -export const response = runtimeMetrics; -`.trimStart(), - ); - await writeFile( - path.join(cwd, "src/features/chat/app-server/inbound/routing.ts"), - ` -import { appServerUserInputResponse } from "../../../../app-server/protocol/server-requests"; - -export const response = appServerUserInputResponse; -`.trimStart(), - ); - - const report = biomeLint(["src/features/chat/app-server/inbound/handler.ts", "src/features/chat/app-server/inbound/routing.ts"], cwd); - - expect(pluginMessages(report, "src/features/chat/app-server/inbound/handler.ts")).toEqual([ - "Chat app-server request handling may consume server request protocol projections only. Convert app-server payloads to chat pending request domain models at this boundary.", + expect(pluginMessages(report, "src/features/chat/app-server/inbound/server-requests/responses.ts")).toEqual([ + APP_SERVER_PROTOCOL_BOUNDARY_MESSAGE, ]); - expect(pluginDiagnostics(report, "src/features/chat/app-server/inbound/routing.ts")).toEqual([]); }); it("keeps lower-level source independent from connection and feature layers", async () => { @@ -862,6 +835,7 @@ async function tempBiomeWorkspace(plugins) { await mkdir(path.join(cwd, "src/features/chat/application/threads"), { recursive: true }); await mkdir(path.join(cwd, "src/features/chat/application/pending-requests"), { recursive: true }); await mkdir(path.join(cwd, "src/features/chat/app-server/inbound"), { recursive: true }); + await mkdir(path.join(cwd, "src/features/chat/app-server/inbound/server-requests"), { recursive: true }); await mkdir(path.join(cwd, "src/features/chat/app-server/mappers/message-stream"), { recursive: true }); await mkdir(path.join(cwd, "src/features/chat/host"), { recursive: true }); await mkdir(path.join(cwd, "src/features/chat/panel"), { recursive: true });