diff --git a/src/domain/pending-requests/aggregate.ts b/src/domain/pending-requests/aggregate.ts new file mode 100644 index 00000000..fe1684f5 --- /dev/null +++ b/src/domain/pending-requests/aggregate.ts @@ -0,0 +1,42 @@ +export interface PendingRequestCounts { + readonly approvals: number; + readonly userInputs: number; + readonly mcpElicitations: number; + readonly actionable: number; +} + +interface PendingRequestQueueSource { + readonly approvals: readonly unknown[]; + readonly pendingUserInputs: readonly unknown[]; + readonly pendingMcpElicitations: readonly unknown[]; +} + +interface PendingRequestCountSource { + readonly pendingApprovals: number; + readonly pendingUserInputs: number; + readonly pendingMcpElicitations: number; +} + +export function pendingRequestCountsFromQueues(source: PendingRequestQueueSource): PendingRequestCounts { + return pendingRequestCounts({ + pendingApprovals: source.approvals.length, + pendingUserInputs: source.pendingUserInputs.length, + pendingMcpElicitations: source.pendingMcpElicitations.length, + }); +} + +export function pendingRequestCounts(source: PendingRequestCountSource): PendingRequestCounts { + const approvals = source.pendingApprovals; + const userInputs = source.pendingUserInputs; + const mcpElicitations = source.pendingMcpElicitations; + return { + approvals, + userInputs, + mcpElicitations, + actionable: approvals + userInputs + mcpElicitations, + }; +} + +export function hasPendingRequests(counts: PendingRequestCounts): boolean { + return counts.actionable > 0; +} diff --git a/src/domain/pending-requests/model.ts b/src/domain/pending-requests/model.ts index 6a4562f3..20f64391 100644 --- a/src/domain/pending-requests/model.ts +++ b/src/domain/pending-requests/model.ts @@ -218,10 +218,6 @@ export function mcpElicitationDraftKey(requestId: PendingRequestId, fieldId: str return pendingRequestDerivedKey(requestId, `mcp:${fieldId}`); } -export function approvalDetailsDisclosureId(requestId: PendingRequestId): string { - return pendingRequestDerivedKey(requestId, "details"); -} - export function pendingRequestDerivedKeyPrefix(requestId: PendingRequestId): string { return `${String(requestId)}:`; } diff --git a/src/features/chat/application/pending-requests/pending-request-actions.ts b/src/features/chat/application/pending-requests/pending-request-actions.ts index 4984a090..f44db9e6 100644 --- a/src/features/chat/application/pending-requests/pending-request-actions.ts +++ b/src/features/chat/application/pending-requests/pending-request-actions.ts @@ -1,7 +1,6 @@ import type { ChatStateStore } from "../state/store"; import { pendingRequestFocusSignature } from "../../domain/pending-requests/signatures"; import { - approvalDetailsDisclosureId, answersForPendingUserInput, type ApprovalAction, type McpElicitationAction, @@ -9,6 +8,7 @@ import { type PendingMcpElicitation, type PendingUserInput, } from "../../../../domain/pending-requests/model"; +import { approvalDetailsDisclosureId } from "../../domain/pending-requests/disclosure-ids"; import type { PendingRequestBlockActions, PendingRequestBlockState, PendingRequestId } from "./block"; import type { ChatState } from "../state/root-reducer"; diff --git a/src/features/chat/domain/pending-requests/disclosure-ids.ts b/src/features/chat/domain/pending-requests/disclosure-ids.ts new file mode 100644 index 00000000..ca99107b --- /dev/null +++ b/src/features/chat/domain/pending-requests/disclosure-ids.ts @@ -0,0 +1,5 @@ +import { pendingRequestDerivedKeyPrefix, type PendingRequestId } from "../../../../domain/pending-requests/model"; + +export function approvalDetailsDisclosureId(requestId: PendingRequestId): string { + return `${pendingRequestDerivedKeyPrefix(requestId)}details`; +} diff --git a/src/features/chat/domain/pending-requests/result-items.ts b/src/features/chat/domain/pending-requests/result-items.ts index 0fec5063..ccdbc5c1 100644 --- a/src/features/chat/domain/pending-requests/result-items.ts +++ b/src/features/chat/domain/pending-requests/result-items.ts @@ -12,9 +12,7 @@ import type { MessageStreamItem, MessageStreamUserInputQuestionResult } from ".. import { definedProp } from "../../../../utils"; export function createApprovalResultItem(approval: PendingApproval, action: ApprovalAction): MessageStreamItem { - const status = approvalResultStatus(action); const kind = approvalActionKind(action); - const scope = kind === "accept-session" ? "session" : "turn"; return { id: `approval-${String(approval.requestId)}`, kind: "approvalResult", @@ -24,8 +22,8 @@ export function createApprovalResultItem(approval: PendingApproval, action: Appr provenance: { source: "localUser", channel: "response", interaction: "approvalResponse", sourceId: String(approval.requestId) }, executionState: kind === "accept" || kind === "accept-session" ? "completed" : "failed", approval: { - status, - scope, + status: approvalResultStatus(kind), + scope: kind === "accept-session" ? "session" : "turn", request: approvalTitle(approval), auditFacts: approvalDetails(approval), }, @@ -76,19 +74,17 @@ export function createMcpElicitationResultItem( } function approvalResultText(approval: PendingApproval, action: ApprovalAction): string { - return `${approvalResultPrefix(action)}: ${approvalResultSummary(approval)}`; + return `${approvalResultPrefix(approvalActionKind(action))}: ${approvalResultSummary(approval)}`; } -function approvalResultPrefix(action: ApprovalAction): string { - const kind = approvalActionKind(action); +function approvalResultPrefix(kind: ReturnType): string { if (kind === "accept") return "Allowed"; if (kind === "accept-session") return "Allowed for this session"; if (kind === "cancel") return "Cancelled"; return "Denied"; } -function approvalResultStatus(action: ApprovalAction): string { - const kind = approvalActionKind(action); +function approvalResultStatus(kind: ReturnType): string { if (kind === "accept") return "allowed"; if (kind === "accept-session") return "allowed for session"; if (kind === "cancel") return "cancelled"; diff --git a/src/features/chat/domain/pending-requests/signatures.ts b/src/features/chat/domain/pending-requests/signatures.ts index 83f2e7e5..30ce8a43 100644 --- a/src/features/chat/domain/pending-requests/signatures.ts +++ b/src/features/chat/domain/pending-requests/signatures.ts @@ -1,4 +1,5 @@ import type { PendingApproval, PendingMcpElicitation, PendingUserInput } from "../../../../domain/pending-requests/model"; +import { hasPendingRequests, pendingRequestCountsFromQueues } from "../../../../domain/pending-requests/aggregate"; export function pendingRequestsSignature( approvals: readonly PendingApproval[], @@ -7,7 +8,11 @@ export function pendingRequestsSignature( drafts: ReadonlyMap, mcpDrafts: ReadonlyMap, ): string { - if (approvals.length === 0 && inputs.length === 0 && mcpElicitations.length === 0) return ""; + if ( + !hasPendingRequests(pendingRequestCountsFromQueues({ approvals, pendingUserInputs: inputs, pendingMcpElicitations: mcpElicitations })) + ) { + return ""; + } return JSON.stringify({ approvals: approvals.map((approval) => ({ id: approval.requestId, method: approval.method })), inputs: inputs.map((input) => ({ @@ -44,7 +49,11 @@ export function pendingRequestFocusSignature( inputs: readonly PendingUserInput[], mcpElicitations: readonly PendingMcpElicitation[], ): string { - if (approvals.length === 0 && inputs.length === 0 && mcpElicitations.length === 0) return ""; + if ( + !hasPendingRequests(pendingRequestCountsFromQueues({ approvals, pendingUserInputs: inputs, pendingMcpElicitations: mcpElicitations })) + ) { + return ""; + } return JSON.stringify({ approvals: approvals.map((approval) => ({ id: approval.requestId, method: approval.method })), inputs: inputs.map((input) => ({ id: input.requestId, method: input.method })), diff --git a/src/features/chat/host/session.ts b/src/features/chat/host/session.ts index 9280be81..5e59b91c 100644 --- a/src/features/chat/host/session.ts +++ b/src/features/chat/host/session.ts @@ -7,6 +7,7 @@ import { createChatViewDeferredTasks } from "./lifecycle"; import { ChatResumeWorkTracker, type ChatViewDeferredTasks } from "../application/lifecycle"; import { openPanelTurnLifecycle, parseRestoredThreadState, type ChatPanelSnapshot } from "../panel/snapshot"; import type { ChatState } from "../application/state/root-reducer"; +import { pendingRequestCountsFromQueues } from "../../../domain/pending-requests/aggregate"; import { renderChatPanelShell, unmountChatPanelShell } from "../panel/shell"; import { createChatStateStore, type ChatStateStore } from "../application/state/store"; import { createChatMessageScrollIntentState, type ChatMessageScrollIntentState } from "../panel/surface/message-stream-scroll"; @@ -89,13 +90,14 @@ export class ChatPanelSession implements ChatSurfaceHandle { } openPanelSnapshot(): ChatPanelSnapshot { + const pendingRequests = pendingRequestCountsFromQueues(this.state.requests); return { viewId: this.environment.obsidian.viewId, threadId: this.closing ? null : this.state.activeThread.id, turnLifecycle: openPanelTurnLifecycle(this.state.turn.lifecycle), - pendingApprovals: this.state.requests.approvals.length, - pendingUserInputs: this.state.requests.pendingUserInputs.length, - pendingMcpElicitations: this.state.requests.pendingMcpElicitations.length, + pendingApprovals: pendingRequests.approvals, + pendingUserInputs: pendingRequests.userInputs, + pendingMcpElicitations: pendingRequests.mcpElicitations, hasComposerDraft: this.state.composer.draft.trim().length > 0, connected: this.graph.connection.manager.isConnected(), }; diff --git a/src/features/chat/ui/message-stream/pending-request-block.tsx b/src/features/chat/ui/message-stream/pending-request-block.tsx index a2d8d524..813c2bad 100644 --- a/src/features/chat/ui/message-stream/pending-request-block.tsx +++ b/src/features/chat/ui/message-stream/pending-request-block.tsx @@ -1,7 +1,8 @@ import type { ComponentChild as UiNode } from "preact"; import { useEffect, useLayoutEffect, useRef, useState } from "preact/hooks"; -import { approvalDetailsDisclosureId } from "../../../../domain/pending-requests/model"; +import { hasPendingRequests, pendingRequestCountsFromQueues } from "../../../../domain/pending-requests/aggregate"; +import { approvalDetailsDisclosureId } from "../../domain/pending-requests/disclosure-ids"; import { type PendingApprovalViewModel, type PendingMcpElicitationFieldViewModel, @@ -70,7 +71,7 @@ function PendingRequestBlock({ if (!shouldFocus) return; focusPendingRequestControl(requestRef.current); }, [autoFocusRequested, consumeAutoFocus, autoFocusSignature]); - if (approvals.length === 0 && pendingUserInputs.length === 0 && pendingMcpElicitations.length === 0) return null; + if (!hasPendingRequests(pendingRequestCountsFromQueues({ approvals, pendingUserInputs, pendingMcpElicitations }))) return null; return (
Request
diff --git a/src/features/threads-view/state.ts b/src/features/threads-view/state.ts index 75bf0d07..c704e58a 100644 --- a/src/features/threads-view/state.ts +++ b/src/features/threads-view/state.ts @@ -1,5 +1,6 @@ import type { Thread } from "../../domain/threads/model"; import type { OpenCodexPanelSnapshot } from "../../workspace/panel-coordinator"; +import { hasPendingRequests, pendingRequestCounts } from "../../domain/pending-requests/aggregate"; import { threadRowCoreProjection, type ThreadRowCoreProjection } from "../threads/row-projection"; import { initialThreadRenameLifecycleState, @@ -10,13 +11,10 @@ import { type ThreadRenameLifecycleState as SharedThreadRenameLifecycleState, } from "../threads/rename-lifecycle"; -type ThreadsLiveStatus = "needs-input" | "approval" | "running" | "draft" | "offline" | "open"; +type ThreadsLiveStatus = "pending" | "running" | "draft" | "offline" | "open"; interface ThreadsLiveState { status: ThreadsLiveStatus; - label: string; - viewId: string; - openPanels: number; } export interface ThreadsRowModel extends ThreadRowCoreProjection { @@ -35,8 +33,7 @@ export type ThreadsRenameLifecycleEvent = | { type: "auto-name-finished"; generatingState: ThreadsGeneratingRenameState }; const STATUS_PRIORITY: Record = { - "needs-input": 5, - approval: 4, + pending: 4, running: 3, draft: 2, offline: 1, @@ -79,9 +76,6 @@ function liveStateForSnapshots(snapshots: OpenCodexPanelSnapshot[]): ThreadsLive const status = snapshotStatus(winner); return { status, - label: statusLabel(status), - viewId: winner.viewId, - openPanels: liveSnapshots.length, }; } @@ -131,27 +125,9 @@ function snapshotsForThreads(snapshots: OpenCodexPanelSnapshot[]): Map 0 || snapshot.pendingMcpElicitations > 0) return "needs-input"; - if (snapshot.pendingApprovals > 0) return "approval"; + if (hasPendingRequests(pendingRequestCounts(snapshot))) return "pending"; if (snapshot.turnLifecycle.kind !== "idle") return "running"; if (snapshot.hasComposerDraft) return "draft"; if (!snapshot.connected) return "offline"; return "open"; } - -function statusLabel(status: ThreadsLiveStatus): string { - switch (status) { - case "needs-input": - return "Needs input"; - case "approval": - return "Approval"; - case "running": - return "Running"; - case "draft": - return "Draft"; - case "offline": - return "Offline"; - case "open": - return "Open"; - } -} diff --git a/src/features/threads/row-projection.ts b/src/features/threads/row-projection.ts index 50364ec1..56329696 100644 --- a/src/features/threads/row-projection.ts +++ b/src/features/threads/row-projection.ts @@ -14,7 +14,6 @@ interface ThreadRowCoreArchiveConfirmProjection { } export interface ThreadRowCoreProjection { - readonly thread: Thread; readonly threadId: string; readonly title: string; readonly selected: boolean; @@ -31,7 +30,6 @@ export function threadRowCoreProjection(input: { }): ThreadRowCoreProjection { const rename = input.renameState; return { - thread: input.thread, threadId: input.thread.id, title: threadDisplayTitle(input.thread), selected: input.selected, diff --git a/src/styles/40-threads-view.css b/src/styles/40-threads-view.css index 8a43b24c..a7b480ff 100644 --- a/src/styles/40-threads-view.css +++ b/src/styles/40-threads-view.css @@ -70,8 +70,7 @@ font-weight: var(--nav-item-weight-active, var(--font-medium)); } -.codex-panel-threads__row--needs-input, -.codex-panel-threads__row--approval { +.codex-panel-threads__row--pending { --codex-panel-threads-gutter-color: color-mix(in srgb, var(--codex-panel-color-warning) 72%, transparent); } @@ -91,8 +90,7 @@ --codex-panel-threads-gutter-color: var(--codex-panel-border-muted-color); } -.codex-panel-threads__row--needs-input::before, -.codex-panel-threads__row--approval::before, +.codex-panel-threads__row--pending::before, .codex-panel-threads__row--running::before, .codex-panel-threads__row--draft::before, .codex-panel-threads__row--offline::before, diff --git a/src/workspace/panel-coordinator.ts b/src/workspace/panel-coordinator.ts index 4e2d5fe6..ee850d5a 100644 --- a/src/workspace/panel-coordinator.ts +++ b/src/workspace/panel-coordinator.ts @@ -4,6 +4,7 @@ import { VIEW_TYPE_CODEX_PANEL } from "../constants"; import { CodexChatView } from "../features/chat/host/view"; import type { ChatWorkspacePanelSurface } from "../features/chat/host/surface-handle"; import type { ChatPanelSnapshot } from "../features/chat/panel/snapshot"; +import { hasPendingRequests, pendingRequestCounts } from "../domain/pending-requests/aggregate"; const BOOT_RESTORED_PANEL_LOAD_DELAY_MS = 1_000; const BOOT_RESTORED_PANEL_LOAD_STAGGER_MS = 250; @@ -373,9 +374,7 @@ function isIdleEmptyPanelSnapshot(snapshot: ChatPanelSnapshot): boolean { return ( snapshot.threadId === null && snapshot.turnLifecycle.kind === "idle" && - snapshot.pendingApprovals === 0 && - snapshot.pendingUserInputs === 0 && - snapshot.pendingMcpElicitations === 0 && + !hasPendingRequests(pendingRequestCounts(snapshot)) && !snapshot.hasComposerDraft ); } diff --git a/tests/features/chat/protocol/server-requests/approval.test.ts b/tests/features/chat/protocol/server-requests/approval.test.ts index 83383dd6..2ad636d5 100644 --- a/tests/features/chat/protocol/server-requests/approval.test.ts +++ b/tests/features/chat/protocol/server-requests/approval.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { approvalResponse, toPendingApproval } from "../../../../../src/features/chat/app-server/requests/approval"; import { approvalDetails, approvalSummary, approvalTitle } from "../../../../../src/features/chat/domain/pending-requests/approval"; +import { createApprovalResultItem } from "../../../../../src/features/chat/domain/pending-requests/result-items"; import type { CommandApprovalDecision } from "../../../../../src/domain/pending-requests/model"; import { approvalActionOptions } from "../../../../../src/features/chat/presentation/pending-requests/approval-view"; import type { ServerRequest } from "../../../../../src/app-server/connection/rpc-messages"; @@ -116,10 +117,17 @@ describe("approval model", () => { const options = approvalActionOptions(approval); expect(options.map((option) => option.label)).toEqual(["Allow network rule", "Allow network rule", "Deny"]); + expect(options.map((option) => option.className)).toEqual(["", "", "mod-warning"]); expect(new Set(options.map((option) => option.id)).size).toBe(options.length); expect(approvalResponse(approval, expectPresent(options[0]).action)).toEqual({ decision: allowRegistryDecision }); expect(approvalResponse(approval, expectPresent(options[1]).action)).toEqual({ decision: allowApiDecision }); expect(approvalResponse(approval, expectPresent(options[2]).action)).toEqual({ decision: "decline" }); + expect(createApprovalResultItem(approval, expectPresent(options[0]).action)).toMatchObject({ + approval: { status: "allowed for session" }, + }); + expect(createApprovalResultItem(approval, expectPresent(options[2]).action)).toMatchObject({ + approval: { status: "denied" }, + }); }); it("keeps future simple command decisions renderable", () => { diff --git a/tests/features/threads-view/renderer.test.ts b/tests/features/threads-view/renderer.test.ts index 4ecd7289..e9042522 100644 --- a/tests/features/threads-view/renderer.test.ts +++ b/tests/features/threads-view/renderer.test.ts @@ -55,11 +55,10 @@ function threadFixture(overrides: Partial = {}): Thread { } function rowFixture(overrides: Partial = {}): ThreadsRowModel { - const thread = overrides.thread ?? threadFixture({ id: "thread", name: "Thread" }); - const title = overrides.title ?? thread.name ?? thread.preview; + const threadId = overrides.threadId ?? "thread"; + const title = overrides.title ?? "Thread"; return { - thread, - threadId: thread.id, + threadId, title, live: null, selected: false, @@ -92,13 +91,11 @@ describe("threads view renderer decisions", () => { [ openPanelSnapshot({ viewId: "open", threadId: "thread" }), openPanelSnapshot({ viewId: "running", threadId: "thread", turnLifecycle: { kind: "running", turnId: "turn" } }), - openPanelSnapshot({ viewId: "approval", threadId: "thread", pendingApprovals: 1 }), - openPanelSnapshot({ viewId: "input", threadId: "thread", pendingUserInputs: 1 }), - openPanelSnapshot({ viewId: "mcp", threadId: "thread", pendingMcpElicitations: 1 }), + openPanelSnapshot({ viewId: "pending", threadId: "thread", pendingApprovals: 1 }), ], new Map(), )[0]?.live, - ).toMatchObject({ status: "needs-input", label: "Needs input", viewId: "input", openPanels: 5 }); + ).toMatchObject({ status: "pending" }); expect( threadRows( @@ -108,7 +105,6 @@ describe("threads view renderer decisions", () => { )[0]?.live, ).toMatchObject({ status: "draft", - label: "Draft", }); expect( threadRows( @@ -118,7 +114,6 @@ describe("threads view renderer decisions", () => { )[0]?.live, ).toMatchObject({ status: "offline", - label: "Offline", }); expect( threadRows( @@ -155,7 +150,7 @@ describe("threads view renderer decisions", () => { renderThreadsView(parent, { status: "2 threads", loading: false, rows }, actions); expect(parent.querySelector(".codex-panel-threads__badge")).toBeNull(); - const main = expectPresent(parent.querySelector(".codex-panel-threads__row--approval")); + const main = expectPresent(parent.querySelector(".codex-panel-threads__row--pending")); const row = expectPresent(main.closest(".codex-panel-threads__row")); expect(row.classList.contains("codex-panel-ui__nav-row")).toBe(true); expect(main.classList.contains("codex-panel-ui__nav-item")).toBe(true); @@ -229,7 +224,6 @@ describe("threads view renderer decisions", () => { const parent = document.createElement("div"); const actions = threadsViewActions(); const row = rowFixture({ - thread: threadFixture({ id: "thread", name: "Old name" }), title: "Old name", rename: { active: true, draft: "Old name", generating: false }, }); @@ -256,7 +250,6 @@ describe("threads view renderer decisions", () => { const parent = document.createElement("div"); const actions = threadsViewActions(); const row = rowFixture({ - thread: threadFixture({ id: "thread", name: "Old name" }), title: "Old name", rename: { active: true, draft: "Old name", generating: false }, }); @@ -276,7 +269,6 @@ describe("threads view renderer decisions", () => { it("renders threads view rename auto-name loading state", () => { const parent = document.createElement("div"); const row = rowFixture({ - thread: threadFixture({ id: "thread", name: "Old name" }), title: "Old name", rename: { active: true, draft: "Old name", generating: true }, }); diff --git a/tests/features/threads-view/state.test.ts b/tests/features/threads-view/state.test.ts index ed2aabb3..7aec6b33 100644 --- a/tests/features/threads-view/state.test.ts +++ b/tests/features/threads-view/state.test.ts @@ -79,10 +79,10 @@ describe("threads view rename state", () => { expect(threadRows([thread({ name: null, preview: "" })], [], new Map())[0]?.rename.draft).toBe(""); }); - it("treats pending MCP elicitations as user input live state", () => { + it("treats pending MCP elicitations as pending live state", () => { const rows = threadRows([thread()], [openPanelSnapshot({ pendingMcpElicitations: 1 })], new Map()); - expect(rows[0]?.live).toMatchObject({ status: "needs-input", label: "Needs input" }); + expect(rows[0]?.live).toMatchObject({ status: "pending" }); }); });