From 67d9f97f88545bf869207167db69b69242a3fe3f Mon Sep 17 00:00:00 2001 From: murashit Date: Tue, 19 May 2026 22:55:03 +0900 Subject: [PATCH] Attach prompt-submit hooks to pending turns --- src/display/blocks.ts | 25 ++-- src/panel/controller.ts | 29 +++- src/panel/hook-display.ts | 34 ++++- src/panel/state.ts | 8 + src/panel/view.ts | 17 +++ styles.css | 17 ++- tests/display/display-model.test.ts | 32 ++++ tests/panel/panel-controller.test.ts | 211 ++++++++++++++++++++++++++- 8 files changed, 353 insertions(+), 20 deletions(-) diff --git a/src/display/blocks.ts b/src/display/blocks.ts index b6bdbc75..5e2e8371 100644 --- a/src/display/blocks.ts +++ b/src/display/blocks.ts @@ -14,6 +14,7 @@ export function displayBlocksForItems( const autoReviewSummariesByTurn = autoReviewSummariesForTurns(visibleItems); const finalAssistantIdByTurn = finalAssistantItemsByTurn(visibleItems); const groupedTurnIds = new Set([...finalAssistantIdByTurn.keys()].filter((turnId) => turnId !== activeTurnId)); + const summaryAssistantIdByTurn = new Map([...finalAssistantIdByTurn].filter(([turnId]) => groupedTurnIds.has(turnId))); const groupedActivities = new Map(); for (const item of orderedItems) { @@ -23,27 +24,25 @@ export function displayBlocksForItems( groupedActivities.set(item.turnId, group); } - const emittedGroups = new Set(); const blocks: DisplayBlock[] = []; for (const item of orderedItems) { const turnId = item.turnId; if (turnId && groupedActivities.has(turnId) && isCompletedTurnDetailItem(item, finalAssistantIdByTurn)) { - if (!emittedGroups.has(turnId)) { - const groupItems = groupedActivities.get(turnId) ?? []; - blocks.push({ - type: "activityGroup", - id: `turn-${turnId}-activity`, - turnId, - summary: turnActivitySummary(groupItems), - items: groupItems, - }); - emittedGroups.add(turnId); - } continue; } + if (turnId && finalAssistantIdByTurn.get(turnId) === item.id && groupedActivities.has(turnId)) { + const groupItems = groupedActivities.get(turnId) ?? []; + blocks.push({ + type: "activityGroup", + id: `turn-${turnId}-activity`, + turnId, + summary: turnActivitySummary(groupItems), + items: groupItems, + }); + } blocks.push({ type: "item", - item: itemWithTurnSummaries(item, editedFilesByTurn, autoReviewSummariesByTurn, finalAssistantIdByTurn, turnDiffs), + item: itemWithTurnSummaries(item, editedFilesByTurn, autoReviewSummariesByTurn, summaryAssistantIdByTurn, turnDiffs), }); } diff --git a/src/panel/controller.ts b/src/panel/controller.ts index 021d9c92..ed2ff0da 100644 --- a/src/panel/controller.ts +++ b/src/panel/controller.ts @@ -28,7 +28,7 @@ import { clearActiveThreadState, type PanelState } from "./state"; import { userInputResponse, type PendingUserInput } from "../user-input/model"; import { jsonPreview } from "../utils"; import { classifyAppServerLog } from "./app-server-logs"; -import { hookRunDisplayItem } from "./hook-display"; +import { attachHookRunsToTurn, hookRunDisplayItem } from "./hook-display"; import { routeServerNotification, routeServerRequest } from "./inbound-routing"; import { clearUserInputDrafts, createApprovalResultItem, createUserInputResultItem } from "./request-state"; @@ -220,6 +220,7 @@ export class PanelController { if (method === "turn/started") { this.state.activeThreadId = params.threadId; this.state.activeTurnId = params.turn.id ?? this.state.activeTurnId; + if (params.turn.id) this.attachPendingPromptSubmitHooks(params.turn.id); this.state.busy = true; this.state.status = "Turn running..."; } else if (method === "turn/completed") { @@ -379,8 +380,30 @@ export class PanelController { turnId: string | null, status: string, ): void { - const item = hookRunDisplayItem(run, turnId, status); - if (item) this.state.displayItems = upsertDisplayItem(this.state.displayItems, item); + const resolvedTurnId = this.hookRunTurnId(run, turnId); + const item = hookRunDisplayItem(run, resolvedTurnId, status); + if (!item) return; + this.state.displayItems = upsertDisplayItem(this.state.displayItems, item); + if (!resolvedTurnId && this.state.pendingTurnStart && run?.eventName === "userPromptSubmit") { + const hookIds = this.state.pendingTurnStart.promptSubmitHookItemIds; + if (!hookIds.includes(item.id)) hookIds.push(item.id); + } + } + + private hookRunTurnId( + run: Extract["params"]["run"], + turnId: string | null, + ): string | null { + if (turnId) return turnId; + if (run?.eventName === "userPromptSubmit" && !this.state.pendingTurnStart) return this.state.activeTurnId; + return null; + } + + private attachPendingPromptSubmitHooks(turnId: string): void { + const pending = this.state.pendingTurnStart; + if (!pending) return; + this.state.displayItems = attachHookRunsToTurn(this.state.displayItems, turnId, pending.promptSubmitHookItemIds, pending.anchorItemId); + this.state.pendingTurnStart = null; } private handleMcpStartupStatus(params: Extract["params"]): void { diff --git a/src/panel/hook-display.ts b/src/panel/hook-display.ts index c47e36f2..1f0423b7 100644 --- a/src/panel/hook-display.ts +++ b/src/panel/hook-display.ts @@ -15,20 +15,50 @@ export function hookRunDisplayItem( ...(run.durationMs !== null && run.durationMs !== undefined ? [{ key: "duration", value: `${run.durationMs}ms` }] : []), ]; const details = [{ rows: metaRows }, ...(entries ? [{ title: "Hook output", body: entries }] : [])]; + const displayId = hookRunDisplayId(run); return { - id: `hook-${run.id}`, + id: displayId, kind: "hook", role: "tool", text: hookSummary(run.eventName, run.statusMessage), toolLabel: "hook", turnId: turnId ?? undefined, - itemId: `hook-${run.id}`, + itemId: displayId, status, details, output: "", }; } +function hookRunDisplayId(run: Extract["params"]["run"]): string { + return `hook-${run.id}-${run.startedAt.toString()}`; +} + +export function attachHookRunsToTurn( + items: DisplayItem[], + turnId: string, + hookItemIds: readonly string[], + afterItemId?: string | null, +): DisplayItem[] { + const hookIdSet = new Set(hookItemIds); + const attachedHooks = items.filter((item) => hookIdSet.has(item.id)).map((item) => ({ ...item, turnId })); + if (attachedHooks.length === 0) return items; + + const withoutAttachedHooks = items.filter((item) => !hookIdSet.has(item.id)); + const anchorItemId = afterItemId ?? lastUserMessageAnchorId(withoutAttachedHooks, turnId); + if (!anchorItemId) return [...withoutAttachedHooks, ...attachedHooks]; + const insertAfterIndex = withoutAttachedHooks.findIndex((item) => item.id === anchorItemId); + if (insertAfterIndex === -1) return [...withoutAttachedHooks, ...attachedHooks]; + return [...withoutAttachedHooks.slice(0, insertAfterIndex + 1), ...attachedHooks, ...withoutAttachedHooks.slice(insertAfterIndex + 1)]; +} + +function lastUserMessageAnchorId(items: DisplayItem[], turnId: string): string | null { + const anchor = [...items] + .reverse() + .find((item) => item.kind === "message" && item.role === "user" && (!item.turnId || item.turnId === turnId)); + return anchor?.id ?? null; +} + function hookSummary(eventName: string | null | undefined, statusMessage: string | null | undefined): string { const event = eventName?.trim() || "Hook"; const message = statusMessage?.trim(); diff --git a/src/panel/state.ts b/src/panel/state.ts index 27c8489b..ce0ce31e 100644 --- a/src/panel/state.ts +++ b/src/panel/state.ts @@ -16,6 +16,11 @@ import type { PendingUserInput } from "../user-input/model"; import type { ServiceTier } from "../app-server/service-tier"; import { defaultRuntimeOverride, type RuntimeOverride } from "../runtime/state"; +export interface PendingTurnStart { + anchorItemId: string; + promptSubmitHookItemIds: string[]; +} + export interface PanelState { status: string; effectiveConfig: ConfigReadResponse | null; @@ -35,6 +40,7 @@ export interface PanelState { rateLimit: RateLimitSnapshot | null; busy: boolean; displayItems: DisplayItem[]; + pendingTurnStart: PendingTurnStart | null; turnDiffs: Map; approvals: PendingApproval[]; pendingUserInputs: PendingUserInput[]; @@ -75,6 +81,7 @@ export function createPanelState(): PanelState { rateLimit: null, busy: false, displayItems: [], + pendingTurnStart: null, turnDiffs: new Map(), approvals: [], pendingUserInputs: [], @@ -99,6 +106,7 @@ export function createPanelState(): PanelState { export function clearActiveTurnState(state: PanelState): void { state.activeTurnId = null; state.busy = false; + state.pendingTurnStart = null; state.approvals = []; state.pendingUserInputs = []; state.userInputDrafts.clear(); diff --git a/src/panel/view.ts b/src/panel/view.ts index cc8aacc6..83197094 100644 --- a/src/panel/view.ts +++ b/src/panel/view.ts @@ -50,6 +50,7 @@ import { pendingRequestsSignature as requestStateSignature, userInputDraftKey, u import type { CodexPanelSettings } from "../settings/model"; import { questionDefaultAnswer, type PendingUserInput } from "../user-input/model"; import { PanelComposerController } from "./composer-controller"; +import { attachHookRunsToTurn } from "./hook-display"; import { clearActiveThreadState, clearConnectionScopedState, createPanelState, type PanelState } from "./state"; import { codexPanelDisplayTitle, getThreadTitle, inheritedForkThreadName, upsertThread } from "../threads/model"; import { exportArchivedThreadMarkdown } from "../threads/export"; @@ -404,6 +405,7 @@ export class CodexPanelView extends ItemView { ...(referencedThread ? { referencedThread } : {}), markdown: true, }); + this.state.pendingTurnStart = { anchorItemId: optimisticUserId, promptSubmitHookItemIds: [] }; this.forceMessagesToBottom(); this.composerController.setDraft(""); this.state.busy = true; @@ -428,13 +430,28 @@ export class CodexPanelView extends ItemView { this.state.requestedModel = commitRuntimeOverride(this.state.requestedModel); this.state.requestedReasoningEffort = commitRuntimeOverride(this.state.requestedReasoningEffort); this.state.activeTurnId = response.turn.id; + const pendingTurnStart = this.state.pendingTurnStart; this.state.displayItems = this.state.displayItems.map((item) => item.id === optimisticUserId ? { ...item, turnId: response.turn.id } : item, ); + if (pendingTurnStart) { + this.state.displayItems = attachHookRunsToTurn( + this.state.displayItems, + response.turn.id, + pendingTurnStart.promptSubmitHookItemIds, + pendingTurnStart.anchorItemId, + ); + this.state.pendingTurnStart = null; + } this.setStatus("Turn running..."); } catch (error) { this.state.busy = false; if (optimisticUserId) this.state.displayItems = this.state.displayItems.filter((item) => item.id !== optimisticUserId); + if (this.state.pendingTurnStart) { + const hookIds = new Set(this.state.pendingTurnStart.promptSubmitHookItemIds); + this.state.displayItems = this.state.displayItems.filter((item) => !hookIds.has(item.id)); + this.state.pendingTurnStart = null; + } this.composerController.setDraft(text); this.addSystemMessage(error instanceof Error ? error.message : String(error)); } diff --git a/styles.css b/styles.css index ff726e21..161af2c9 100644 --- a/styles.css +++ b/styles.css @@ -886,7 +886,22 @@ } .codex-panel__message--system { - border-left-color: var(--text-faint); + margin-bottom: 8px; + border-left-color: var(--background-modifier-border); + font-family: var(--font-interface); + line-height: var(--line-height-tight); +} + +.codex-panel__message--system .codex-panel__message-role { + font-size: var(--font-ui-smaller); + font-weight: 500; +} + +.codex-panel__message--system .codex-panel__message-content { + color: var(--text-muted); + font-family: var(--font-interface); + font-size: var(--font-ui-small); + line-height: var(--line-height-tight); } .codex-panel__message--review-result { diff --git a/tests/display/display-model.test.ts b/tests/display/display-model.test.ts index 10181d23..e7aef6e8 100644 --- a/tests/display/display-model.test.ts +++ b/tests/display/display-model.test.ts @@ -863,6 +863,19 @@ describe("display block grouping keeps work logs subordinate to conversation mes expect(blocks[2]).toMatchObject({ item: { id: "a3" } }); }); + it("keeps completed turn work details attached to the final response even when system messages are interleaved", () => { + const items: DisplayItem[] = [ + { id: "s1", kind: "system", role: "system", text: "debug before hook" }, + commandItem("c1", "npm test", "t1"), + { id: "s2", kind: "system", role: "system", text: "debug after hook" }, + { id: "a1", kind: "message", role: "assistant", text: "done", turnId: "t1" }, + ]; + + const blocks = displayBlocksForItems(items, null); + + expect(blocks.map((block) => (block.type === "item" ? block.item.id : block.id))).toEqual(["s1", "s2", "turn-t1-activity", "a1"]); + }); + it("keeps active turn activities expanded", () => { const items: DisplayItem[] = [ { id: "r1", kind: "reasoning", role: "tool", text: "thinking", turnId: "t1" }, @@ -1092,6 +1105,25 @@ describe("display block grouping keeps work logs subordinate to conversation mes item: { autoReviewSummaries: ["Auto-review approved: npm test", "Auto-review approved: npm test"] }, }); }); + + it("does not add edited file or auto-review summaries to active turn messages", () => { + const items: DisplayItem[] = [ + fileChangeItem("f1", "t1", "src/main.ts"), + { + id: "review-1", + kind: "reviewResult", + role: "tool", + text: "Auto-review approved: npm test", + turnId: "t1", + state: "completed", + }, + { id: "a1", kind: "message", role: "assistant", text: "still working", turnId: "t1" }, + ]; + + const assistantBlock = displayBlocksForItems(items, "t1").find((block) => block.type === "item" && block.item.id === "a1"); + expect(assistantBlock && assistantBlock.type === "item" ? assistantBlock.item : null).not.toHaveProperty("editedFiles"); + expect(assistantBlock && assistantBlock.type === "item" ? assistantBlock.item : null).not.toHaveProperty("autoReviewSummaries"); + }); }); describe("workspace path summaries stay readable without hiding audit paths", () => { diff --git a/tests/panel/panel-controller.test.ts b/tests/panel/panel-controller.test.ts index e7067a35..b3f57f00 100644 --- a/tests/panel/panel-controller.test.ts +++ b/tests/panel/panel-controller.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { PanelController } from "../../src/panel/controller"; +import { attachHookRunsToTurn } from "../../src/panel/hook-display"; import { createPanelState } from "../../src/panel/state"; import type { ServerNotification } from "../../src/generated/app-server/ServerNotification"; import type { ServerRequest } from "../../src/generated/app-server/ServerRequest"; @@ -210,7 +211,7 @@ describe("PanelController", () => { expect(state.displayItems).toMatchObject([ { - id: "hook-hook-1", + id: "hook-hook-1-1", kind: "hook", text: "postToolUse: Formatted 1 file.", toolLabel: "hook", @@ -231,6 +232,214 @@ describe("PanelController", () => { ]); }); + it("attaches unscoped hook runs to the active turn while streaming", () => { + const state = createPanelState(); + state.activeThreadId = "thread-active"; + state.activeTurnId = "turn-active"; + state.busy = true; + const controller = controllerForState(state); + + controller.handleNotification({ + method: "hook/completed", + params: { + threadId: "thread-active", + turnId: null, + run: { + id: "hook-1", + eventName: "userPromptSubmit", + handlerType: "command", + executionMode: "sync", + scope: "turn", + sourcePath: "/vault/.codex/hooks.json", + source: "project", + displayOrder: 1n, + status: "completed", + statusMessage: "Saving jj baseline", + startedAt: 1n, + completedAt: 2n, + durationMs: 1n, + entries: [], + }, + }, + } satisfies Extract); + + expect(state.displayItems[0]).toMatchObject({ id: "hook-hook-1-1", kind: "hook", turnId: "turn-active" }); + }); + + it("leaves non-prompt unscoped hook runs outside the active turn", () => { + const state = createPanelState(); + state.activeThreadId = "thread-active"; + state.activeTurnId = "turn-active"; + state.busy = true; + const controller = controllerForState(state); + + controller.handleNotification({ + method: "hook/completed", + params: { + threadId: "thread-active", + turnId: null, + run: { + id: "hook-1", + eventName: "postToolUse", + handlerType: "command", + executionMode: "sync", + scope: "turn", + sourcePath: "/vault/.codex/hooks.json", + source: "project", + displayOrder: 1n, + status: "completed", + statusMessage: "Rollback hook", + startedAt: 1n, + completedAt: 2n, + durationMs: 1n, + entries: [], + }, + }, + } satisfies Extract); + + expect(state.displayItems[0]).toMatchObject({ id: "hook-hook-1-1", kind: "hook" }); + expect(state.displayItems[0]?.turnId).toBeUndefined(); + }); + + it("keeps repeated hook runs with the same run id as separate display items", () => { + const state = createPanelState(); + state.activeThreadId = "thread-active"; + state.activeTurnId = "turn-active"; + const controller = controllerForState(state); + const baseRun: Extract["params"]["run"] = { + id: "hook-1", + eventName: "userPromptSubmit", + handlerType: "command", + executionMode: "sync", + scope: "turn", + sourcePath: "/vault/.codex/hooks.json", + source: "project", + displayOrder: 1n, + status: "completed", + statusMessage: "Saving jj baseline", + completedAt: 2n, + durationMs: 1n, + entries: [], + startedAt: 1n, + }; + + controller.handleNotification({ + method: "hook/completed", + params: { threadId: "thread-active", turnId: "turn-active", run: { ...baseRun, startedAt: 1n } }, + } satisfies Extract); + controller.handleNotification({ + method: "hook/completed", + params: { threadId: "thread-active", turnId: "turn-active", run: { ...baseRun, startedAt: 3n } }, + } satisfies Extract); + + expect(state.displayItems.map((item) => item.id)).toEqual(["hook-hook-1-1", "hook-hook-1-3"]); + }); + + it("attaches pre-turn prompt submit hook runs when the turn starts", () => { + const state = createPanelState(); + state.activeThreadId = "thread-active"; + state.pendingTurnStart = { anchorItemId: "local-user-1", promptSubmitHookItemIds: ["hook-hook-1-1"] }; + state.displayItems = [ + { id: "local-user-1", kind: "message", role: "user", text: "hello", markdown: true }, + { + id: "hook-hook-1-1", + kind: "hook", + role: "tool", + text: "userPromptSubmit: Saving jj baseline", + toolLabel: "hook", + status: "completed", + }, + ]; + const controller = controllerForState(state); + + controller.handleNotification({ + method: "turn/started", + params: { + threadId: "thread-active", + turn: { + id: "turn-active", + status: "inProgress", + startedAt: 1, + completedAt: null, + durationMs: null, + error: null, + itemsView: "full", + items: [], + }, + }, + } satisfies Extract); + + expect(state.displayItems.map((item) => item.id)).toEqual(["local-user-1", "hook-hook-1-1"]); + expect(state.displayItems[1]).toMatchObject({ id: "hook-hook-1-1", turnId: "turn-active" }); + expect(state.pendingTurnStart).toBeNull(); + }); + + it("moves pre-turn hook runs after the optimistic user message when a turn id is assigned", () => { + const items = attachHookRunsToTurn( + [ + { + id: "hook-old-1", + kind: "hook", + role: "tool", + text: "userPromptSubmit: Stale hook", + toolLabel: "hook", + status: "completed", + }, + { + id: "hook-hook-1-1", + kind: "hook", + role: "tool", + text: "userPromptSubmit: Saving jj baseline", + toolLabel: "hook", + status: "completed", + }, + { id: "local-user-1", kind: "message", role: "user", text: "hello", markdown: true }, + ], + "turn-active", + ["hook-hook-1-1"], + "local-user-1", + ); + + expect(items.map((item) => item.id)).toEqual(["hook-old-1", "local-user-1", "hook-hook-1-1"]); + expect(items[0]?.turnId).toBeUndefined(); + expect(items[2]).toMatchObject({ id: "hook-hook-1-1", turnId: "turn-active" }); + }); + + it("captures only prompt-submit hooks observed during the pending turn start", () => { + const state = createPanelState(); + state.activeThreadId = "thread-active"; + state.pendingTurnStart = { anchorItemId: "local-user-1", promptSubmitHookItemIds: [] }; + const controller = controllerForState(state); + + controller.handleNotification({ + method: "hook/completed", + params: { + threadId: "thread-active", + turnId: null, + run: { + id: "hook-1", + eventName: "userPromptSubmit", + handlerType: "command", + executionMode: "sync", + scope: "turn", + sourcePath: "/vault/.codex/hooks.json", + source: "project", + displayOrder: 1n, + status: "completed", + statusMessage: "Saving jj baseline", + startedAt: 1n, + completedAt: 2n, + durationMs: 1n, + entries: [], + }, + }, + } satisfies Extract); + + expect(state.displayItems[0]).toMatchObject({ id: "hook-hook-1-1", kind: "hook" }); + expect(state.displayItems[0]?.turnId).toBeUndefined(); + expect(state.pendingTurnStart?.promptSubmitHookItemIds).toEqual(["hook-hook-1-1"]); + }); + it("stores account rate limit updates outside thread scope", () => { const state = createPanelState(); const controller = controllerForState(state);