diff --git a/.agents/skills/e2e/SKILL.md b/.agents/skills/e2e/SKILL.md index be43453ef0..7132d25754 100644 --- a/.agents/skills/e2e/SKILL.md +++ b/.agents/skills/e2e/SKILL.md @@ -70,6 +70,40 @@ cd apps && pnpm --filter @kandev/web e2e -- tests/task/my-test.spec.ts # single cd apps && pnpm --filter @kandev/web e2e -- --grep "task creation" # by name ``` +### Flake reproduction + +Start by matching CI as closely as possible, then add pressure deliberately: + +1. Run the exact failed shard in the CI runtime image with CI env enabled: + ```bash + docker run --rm --ipc=host -v "$PWD":/work -w /work/apps/web \ + -e CI=true -e GITHUB_ACTIONS=true -e GITHUB_WORKSPACE=/work \ + -e NODE_OPTIONS=--dns-result-order=ipv4first \ + -e PLAYWRIGHT_BROWSERS_PATH=/ms-playwright \ + ghcr.io/kdlbs/kandev-ci:runtime-latest \ + bash -lc 'git config --global --add safe.directory /work 2>/dev/null; npx playwright test --config e2e/playwright.config.ts --project=chromium --project=mobile-chrome --shard=10/10 --reporter=list' + ``` +2. If the exact shard passes, constrain container resources and repeat the + failing spec/test. GitHub-hosted runners can expose timing bugs that a roomy + local machine hides: + ```bash + docker run --rm --ipc=host --cpus=2 --memory=4g --memory-swap=4g \ + -v "$PWD":/work -w /work/apps/web \ + -e CI=true -e GITHUB_ACTIONS=true -e GITHUB_WORKSPACE=/work \ + -e NODE_OPTIONS=--dns-result-order=ipv4first \ + -e PLAYWRIGHT_BROWSERS_PATH=/ms-playwright \ + ghcr.io/kdlbs/kandev-ci:runtime-latest \ + bash -lc 'git config --global --add safe.directory /work 2>/dev/null; npx playwright test --config e2e/playwright.config.ts --project=mobile-chrome e2e/tests/terminal/mobile-terminal-keybar.spec.ts --grep "user presses an OS-keyboard letter while no modifier is active" --repeat-each=30 --reporter=list' + ``` +3. Preserve nearby test ordering when a single-test repeat stays green. Run the + full spec file or full shard with the same resource limits before declaring + a flake non-reproducible. + +Record the exact command, resource limits, repeat number, and failure artifact +path. Always inspect `error-context.md`; mobile/terminal flakes often show +state that the stack trace alone hides, such as duplicate active terminals or a +terminal stuck on "Starting terminal...". + **CRITICAL: E2E tests run against the production build** (`.next/standalone/`), not dev mode. After any frontend code change, you **must** rebuild before running tests (`pnpm e2e:run` does this for you): ```bash diff --git a/.agents/skills/fix/SKILL.md b/.agents/skills/fix/SKILL.md index 71249a3693..632d74c544 100644 --- a/.agents/skills/fix/SKILL.md +++ b/.agents/skills/fix/SKILL.md @@ -75,6 +75,13 @@ Before anything else, reproduce the bug reliably. Pick the right method based on If it can't be reproduced, add logging/assertions to gather more info — don't guess at a fix. Find the minimal reproduction case: strip away everything that isn't needed to trigger the bug. +For flaky Playwright failures, do not stop after one clean focused run. Use the +`/e2e` flake reproduction flow: run the exact CI shard in +`ghcr.io/kdlbs/kandev-ci:runtime-latest` with `CI=true`, then add resource +limits such as `--cpus=2 --memory=4g --memory-swap=4g` and repeat the failing +test or full spec file. Preserve nearby test ordering when a single-test repeat +passes, and inspect `error-context.md` from the failed repeat before fixing. + Mark task 1 as completed. --- diff --git a/apps/cli/src/ports.test.ts b/apps/cli/src/ports.test.ts index 0d06e7d664..06b2962c48 100644 --- a/apps/cli/src/ports.test.ts +++ b/apps/cli/src/ports.test.ts @@ -65,6 +65,15 @@ function closeServer(server: net.Server): Promise { return new Promise((resolve) => server.close(() => resolve())); } +async function findReportedAvailablePort(): Promise { + for (let attempt = 0; attempt < 10; attempt += 1) { + const { server, port } = await listenOn("127.0.0.1"); + await closeServer(server); + if (await __testing.isPortAvailable(port)) return port; + } + throw new Error("Unable to find a port reported available"); +} + describe("isPortAvailable", () => { const servers: net.Server[] = []; @@ -105,8 +114,7 @@ describe("isPortInUse", () => { describe("pickAvailablePort", () => { it("returns the preferred port when it is free", async () => { - const { server, port } = await listenOn("127.0.0.1"); - await closeServer(server); + const port = await findReportedAvailablePort(); expect(await pickAvailablePort(port)).toBe(port); }); diff --git a/apps/web/components/task/chat/messages/chat-message.test.tsx b/apps/web/components/task/chat/messages/chat-message.test.tsx index 4c65e919c6..fb363cb81f 100644 --- a/apps/web/components/task/chat/messages/chat-message.test.tsx +++ b/apps/web/components/task/chat/messages/chat-message.test.tsx @@ -3,7 +3,12 @@ import type { ReactNode } from "react"; import { fireEvent, render, screen } from "@testing-library/react"; import { StateProvider } from "@/components/state-provider"; import { ChatMessage } from "./chat-message"; -import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http"; +import { + sessionId as toSessionId, + taskId as toTaskId, + type Message, + type TaskSession, +} from "@/lib/types/http"; const SENDER_TASK_ID = "task-sender"; const SENDER_TITLE = "Fix login bug"; @@ -68,6 +73,36 @@ function renderWithSender( ); } +function renderAgentMessageWithSession(session: Partial, metadata = {}) { + const taskSession: TaskSession = { + id: toSessionId("sess-1"), + task_id: toTaskId("task-target"), + state: "COMPLETED", + started_at: "2026-05-04T00:00:00Z", + updated_at: "2026-05-04T00:00:00Z", + ...session, + }; + const Wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + + return render( + + + , + ); +} + describe("ChatMessage sender badge", () => { it("renders the sender badge when sender_task_id is present in metadata", () => { const { container } = renderWithSender([{ id: SENDER_TASK_ID, title: SENDER_TITLE }], { @@ -167,6 +202,151 @@ describe("ChatMessage sender badge", () => { }); }); +describe("ChatMessage agent session config metadata", () => { + it("shows session config options next to the model", () => { + renderAgentMessageWithSession({ + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + reasoning_effort: "high", + verbosity: "low", + }, + }, + }); + + expect(screen.getByText("gpt-5.5 · Reasoning effort: high · Verbosity: low")).not.toBeNull(); + }); + + it("prefers live runtime config over the profile snapshot", () => { + renderAgentMessageWithSession({ + metadata: { + runtime_config: { + model: "gpt-5.6", + mode: "accept-edits", + config_options: { + reasoning_effort: "medium", + }, + }, + }, + agent_profile_snapshot: { + model: "gpt-5.5", + mode: "default", + config_options: { + reasoning_effort: "high", + }, + }, + }); + + expect( + screen.getByText("gpt-5.6 · Mode: accept-edits · Reasoning effort: medium"), + ).not.toBeNull(); + }); + + it("merges runtime config options over snapshot options per option", () => { + renderAgentMessageWithSession({ + metadata: { + runtime_config: { + config_options: { + reasoning_effort: "medium", + }, + }, + }, + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + reasoning_effort: "high", + verbosity: "low", + }, + }, + }); + + expect( + screen.getAllByText("gpt-5.5 · Reasoning effort: medium · Verbosity: low").length, + ).toBeGreaterThan(0); + }); + + it("keeps merged runtime-only config options sorted", () => { + renderAgentMessageWithSession({ + metadata: { + runtime_config: { + config_options: { + reasoning_effort: "medium", + }, + }, + }, + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + verbosity: "low", + }, + }, + }); + + expect( + screen.getAllByText("gpt-5.5 · Reasoning effort: medium · Verbosity: low").length, + ).toBeGreaterThan(0); + }); +}); + +describe("ChatMessage agent session config metadata overrides", () => { + it("does not fall back to snapshot options when runtime options are explicitly empty", () => { + const { container } = renderAgentMessageWithSession({ + metadata: { + runtime_config: { + config_options: {}, + }, + }, + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + reasoning_effort: "high", + }, + }, + }); + + expect(screen.getByText("gpt-5.5")).not.toBeNull(); + expect(container.textContent).not.toContain("Reasoning effort"); + }); + + it("keeps message-level model attribution while showing session options", () => { + renderAgentMessageWithSession( + { + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + reasoning_effort: "high", + }, + }, + }, + { model: "gpt-5.5-mini" }, + ); + + expect(screen.getByText("gpt-5.5-mini · Reasoning effort: high")).not.toBeNull(); + }); + + it("uses message-level config options when message metadata provides them", () => { + const { container } = renderAgentMessageWithSession( + { + agent_profile_snapshot: { + model: "gpt-5.5", + config_options: { + reasoning_effort: "high", + }, + }, + }, + { + model: "gpt-5.5-mini", + config_options: { + reasoning_effort: "low", + }, + }, + ); + + expect(screen.getByText("gpt-5.5-mini · Reasoning effort: low")).not.toBeNull(); + expect(container.textContent).not.toContain("Reasoning effort: high"); + }); +}); + describe("ChatMessage image attachments", () => { it("opens image attachments in an in-app preview dialog", () => { const openSpy = vi.spyOn(window, "open").mockReturnValue(null); diff --git a/apps/web/components/task/chat/messages/message-actions.tsx b/apps/web/components/task/chat/messages/message-actions.tsx index d77d9c13f0..6ff669f90b 100644 --- a/apps/web/components/task/chat/messages/message-actions.tsx +++ b/apps/web/components/task/chat/messages/message-actions.tsx @@ -34,18 +34,145 @@ type MessageActionsProps = { hasNext?: boolean; }; -function useMessageModel(message: Message, showModel: boolean) { +type SessionConfigSource = { + model?: unknown; + mode?: unknown; + config_options?: unknown; + configOptions?: unknown; +}; + +type MessageSessionConfig = { + model: string | null; + mode: string | null; + configOptions: Array<{ label: string; value: string }>; + configOptionsSet: boolean; +}; + +function stringValue(value: unknown): string | null { + return typeof value === "string" && value.trim() !== "" ? value : null; +} + +function isStringConfigEntry(entry: [string, unknown]): entry is [string, string] { + const [key, optionValue] = entry; + return key !== "model" && key !== "mode" && stringValue(optionValue) !== null; +} + +function configOptionEntries(value: unknown): Array<{ label: string; value: string }> { + if (!value || typeof value !== "object" || Array.isArray(value)) return []; + return Object.entries(value) + .filter(isStringConfigEntry) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, optionValue]) => ({ + label: humanizeConfigKey(key), + value: optionValue, + })); +} + +function humanizeConfigKey(key: string): string { + return key + .replace(/[_-]+/g, " ") + .replace(/([a-z0-9])([A-Z])/g, "$1 $2") + .trim() + .replace(/^./, (char) => char.toUpperCase()); +} + +function configFromSource(source: SessionConfigSource | null | undefined): MessageSessionConfig { + if (!source) return emptySessionConfig(); + const configOptionsValue = source.config_options ?? source.configOptions; + return { + model: stringValue(source.model), + mode: stringValue(source.mode), + configOptions: configOptionEntries(configOptionsValue), + configOptionsSet: isConfigOptionsObject(configOptionsValue), + }; +} + +function emptySessionConfig(): MessageSessionConfig { + return { model: null, mode: null, configOptions: [], configOptionsSet: false }; +} + +function isConfigOptionsObject(value: unknown): boolean { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +function mergeSessionConfig( + primary: MessageSessionConfig, + fallback: MessageSessionConfig, +): MessageSessionConfig { + return { + model: primary.model ?? fallback.model, + mode: primary.mode ?? fallback.mode, + configOptions: mergedConfigOptions(primary, fallback), + configOptionsSet: primary.configOptionsSet || fallback.configOptionsSet, + }; +} + +function mergedConfigOptions( + primary: MessageSessionConfig, + fallback: MessageSessionConfig, +): MessageSessionConfig["configOptions"] { + if (!primary.configOptionsSet) return fallback.configOptions; + if (primary.configOptions.length === 0) return []; + const optionsByLabel = new Map(fallback.configOptions.map((option) => [option.label, option])); + for (const option of primary.configOptions) { + optionsByLabel.set(option.label, option); + } + return Array.from(optionsByLabel.values()).sort((a, b) => a.label.localeCompare(b.label)); +} + +function formatSessionConfig(config: MessageSessionConfig): string | null { + if (!config.model) return null; + const details = formatSessionConfigDetails(config); + return details ? `${config.model} · ${details}` : config.model; +} + +function formatSessionConfigDetails(config: MessageSessionConfig): string | null { + const details = [ + config.mode ? `Mode: ${config.mode}` : null, + ...config.configOptions.map((option) => `${option.label}: ${option.value}`), + ].filter(Boolean); + return details.length > 0 ? details.join(" · ") : null; +} + +function useMessageSessionConfigText(message: Message, showModel: boolean) { const sessionId = message.session_id; - const messageModel = showModel - ? (message.metadata as { model?: string } | undefined)?.model - : undefined; - const sessionModel = useAppStore((state) => { - if (!showModel || messageModel || !sessionId) return null; + const messageConfig = showModel + ? configFromSource(message.metadata as SessionConfigSource | undefined) + : null; + const [sessionConfigText, sessionDetailsText] = splitSessionConfigText( + useSessionConfigText(sessionId, showModel), + ); + if (!messageConfig?.model) return sessionConfigText || null; + const messageDetails = formatSessionConfigDetails(messageConfig); + const details = messageDetails ?? sessionDetailsText; + return details ? `${messageConfig.model} · ${details}` : messageConfig.model; +} + +function useSessionConfigText(sessionId: string | undefined, showModel: boolean) { + return useAppStore((state) => { + if (!showModel || !sessionId) return null; const session = state.taskSessions.items[sessionId]; - const snapshot = session?.agent_profile_snapshot as { model?: string } | null | undefined; - return snapshot?.model ?? null; + const snapshot = configFromSource( + session?.agent_profile_snapshot as SessionConfigSource | null | undefined, + ); + const metadata = session?.metadata as Record | null | undefined; + const runtime = configFromSource( + metadata?.runtime_config as SessionConfigSource | null | undefined, + ); + const config = mergeSessionConfig(runtime, snapshot); + return joinSessionConfigText(formatSessionConfig(config), formatSessionConfigDetails(config)); }); - return messageModel ?? sessionModel; +} + +function joinSessionConfigText(full: string | null, details: string | null): string { + return JSON.stringify([full, details]); +} + +function splitSessionConfigText(value: string | null): [string | null, string | null] { + if (!value) return [null, null]; + const parsed = JSON.parse(value) as [unknown, unknown]; + const [full, details] = parsed; + return [stringValue(full), stringValue(details)]; } function CopyButton({ copied, onCopy }: { copied: boolean; onCopy: () => void }) { @@ -141,19 +268,21 @@ function RawToggleButton({ function MessageMetaInfo({ showModel, - modelName, + sessionConfigText, showTimestamp, createdAt, }: { showModel: boolean; - modelName: string | null | undefined; + sessionConfigText: string | null; showTimestamp: boolean; createdAt: string; }) { return ( <> - {showModel && modelName && ( - {modelName} + {showModel && sessionConfigText && ( + + {sessionConfigText} + )} {showTimestamp && ( @@ -180,7 +309,7 @@ export function MessageActions({ hasNext = false, }: MessageActionsProps) { const { copied, copy } = useCopyToClipboard(); - const modelName = useMessageModel(message, showModel); + const sessionConfigText = useMessageSessionConfigText(message, showModel); const handleCopy = async () => { await copy(message.content); }; @@ -205,7 +334,7 @@ export function MessageActions({ )} diff --git a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts index d6c627b147..b703b5120d 100644 --- a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts +++ b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts @@ -37,6 +37,15 @@ export async function focusTerminalForTyping(testPage: Page): Promise { await testPage.getByTestId("terminal-panel").last().getByTestId("terminal-xterm-host").click(); } +async function remountTerminalPanel(testPage: Page): Promise { + const chatTab = testPage.getByRole("button", { name: "Chat" }); + if (await chatTab.isVisible()) { + await chatTab.tap(); + await testPage.waitForTimeout(250); + } + await switchToTerminalPanel(testPage); +} + /** * Wait for the mobile shell to be ready by tailing xterm's buffer until it has * any content (a prompt is enough). Mobile mounts the terminal lazily on tab @@ -50,10 +59,16 @@ export async function focusTerminalForTyping(testPage: Page): Promise { export async function waitForShellReady(testPage: Page, timeout = 45_000): Promise { const panel = testPage.getByTestId("terminal-panel"); const deadline = Date.now() + timeout; + let remounts = 0; + let nextRemountAt = Date.now() + 10_000; while (Date.now() < deadline) { if ((await readTerminalBuffer(testPage)).length > 0) return; if (!(await panel.isVisible())) { await switchToTerminalPanel(testPage); + } else if (remounts < 2 && Date.now() >= nextRemountAt) { + await remountTerminalPanel(testPage); + remounts += 1; + nextRemountAt = Date.now() + 15_000; } await testPage.waitForTimeout(1_000); } diff --git a/apps/web/hooks/domains/session/use-terminals-build.test.ts b/apps/web/hooks/domains/session/use-terminals-build.test.ts new file mode 100644 index 0000000000..4152329faa --- /dev/null +++ b/apps/web/hooks/domains/session/use-terminals-build.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { appendTerminalIfMissing } from "./use-terminals-build"; +import type { Terminal } from "./use-terminals-types"; + +function terminal(id: string, seq: number): Terminal { + return { + id, + type: "shell", + label: `Terminal ${seq}`, + closable: true, + kind: "ordinary", + seq, + }; +} + +describe("appendTerminalIfMissing", () => { + it("does not duplicate a terminal already inserted by shell-list sync", () => { + const existing = terminal("shell-1", 1); + const result = appendTerminalIfMissing([existing], terminal("shell-1", 1)); + expect(result).toEqual([existing]); + }); + + it("appends a distinct terminal", () => { + const existing = terminal("shell-1", 1); + const next = terminal("shell-2", 2); + expect(appendTerminalIfMissing([existing], next)).toEqual([existing, next]); + }); +}); diff --git a/apps/web/hooks/domains/session/use-terminals-build.ts b/apps/web/hooks/domains/session/use-terminals-build.ts index 523c9810e7..f4dff95d37 100644 --- a/apps/web/hooks/domains/session/use-terminals-build.ts +++ b/apps/web/hooks/domains/session/use-terminals-build.ts @@ -40,6 +40,11 @@ export function shellToTerminal(shell: UserShellInfo): Terminal { }; } +export function appendTerminalIfMissing(terminals: Terminal[], terminal: Terminal): Terminal[] { + if (terminals.some((item) => item.id === terminal.id)) return terminals; + return [...terminals, terminal]; +} + /** * Compute the effective active tab value, preferring store, then * sessionStorage, then fallback. diff --git a/apps/web/hooks/domains/session/use-terminals.ts b/apps/web/hooks/domains/session/use-terminals.ts index 2e906b80ae..f729b64bda 100644 --- a/apps/web/hooks/domains/session/use-terminals.ts +++ b/apps/web/hooks/domains/session/use-terminals.ts @@ -12,6 +12,7 @@ import { } from "@/lib/api/domains/user-shell-api"; import { useUserShells } from "./use-user-shells"; import { + appendTerminalIfMissing, buildParkedTerminals, buildTerminalsFromShells, computeTerminalTabValue, @@ -174,7 +175,7 @@ function useAddTerminal({ state: ordinary ? (result.state ?? "open") : result.state, ptyStatus: result.ptyStatus ?? "stopped", }; - setTerminals((prev) => [...prev, newTerm]); + setTerminals((prev) => appendTerminalIfMissing(prev, newTerm)); if (sessionId) setRightPanelActiveTab(sessionId, result.terminalId); } catch (error) { console.error("Failed to create user shell:", error); @@ -443,7 +444,7 @@ function useTerminalActions({ closable: true, kind: "script", }; - setTerminals((prev) => [...prev, newTerm]); + setTerminals((prev) => appendTerminalIfMissing(prev, newTerm)); if (sessionId) setRightPanelActiveTab(sessionId, result.terminalId); } catch (error) { console.error("Failed to create script terminal:", error); diff --git a/apps/web/hooks/use-plan-panel-auto-open.test.ts b/apps/web/hooks/use-plan-panel-auto-open.test.ts index 106fcb1329..d255e6e018 100644 --- a/apps/web/hooks/use-plan-panel-auto-open.test.ts +++ b/apps/web/hooks/use-plan-panel-auto-open.test.ts @@ -15,7 +15,15 @@ let mockLastSeen: string | undefined = undefined; let mockIsLoaded = true; let mockConnectionStatus = "connected"; let mockIsRestoringLayout = false; -let mockApi: { getPanel: typeof mockGetPanel } | null = { getPanel: mockGetPanel }; +let mockActivePanelId: string | null = "chat"; +let mockApi: { getPanel: typeof mockGetPanel; activePanel?: { id: string } | null } | null = null; + +function makeApi() { + return { + getPanel: mockGetPanel, + activePanel: mockActivePanelId ? { id: mockActivePanelId } : null, + }; +} function buildState() { return { @@ -77,7 +85,8 @@ describe("usePlanPanelAutoOpen", () => { mockIsLoaded = true; mockConnectionStatus = "connected"; mockIsRestoringLayout = false; - mockApi = { getPanel: mockGetPanel }; + mockActivePanelId = "chat"; + mockApi = makeApi(); mockGetPanel.mockReturnValue(null); mockGetTaskPlan.mockResolvedValue(null); }); @@ -140,7 +149,8 @@ describe("usePlanPanelAutoOpen — eager fetch", () => { mockIsLoaded = true; mockConnectionStatus = "connected"; mockIsRestoringLayout = false; - mockApi = { getPanel: mockGetPanel }; + mockActivePanelId = "chat"; + mockApi = makeApi(); mockGetPanel.mockReturnValue(null); mockGetTaskPlan.mockResolvedValue(null); }); @@ -160,14 +170,26 @@ describe("usePlanPanelAutoOpen — eager fetch", () => { expect(mockGetTaskPlan).not.toHaveBeenCalled(); }); - it("acknowledges the plan on hydrate when the panel was restored", () => { + it("acknowledges the plan on hydrate when the restored panel is active", () => { mockGetPanel.mockReturnValue({ id: "plan" }); + mockActivePanelId = "plan"; + mockApi = makeApi(); mockLastSeen = undefined; renderHook(() => usePlanPanelAutoOpen()); expect(mockMarkTaskPlanSeen).toHaveBeenCalledWith("task-1"); expect(mockAddPlanPanel).not.toHaveBeenCalled(); }); + it("does not acknowledge the plan on hydrate when the restored panel is inactive", () => { + mockGetPanel.mockReturnValue({ id: "plan" }); + mockActivePanelId = "chat"; + mockApi = makeApi(); + mockLastSeen = undefined; + renderHook(() => usePlanPanelAutoOpen()); + expect(mockMarkTaskPlanSeen).not.toHaveBeenCalled(); + expect(mockAddPlanPanel).not.toHaveBeenCalled(); + }); + it("does not acknowledge a live update when lastSeen is already recorded", () => { mockGetPanel.mockReturnValue({ id: "plan" }); mockLastSeen = TS; @@ -224,7 +246,8 @@ describe("usePlanPanelAutoOpen — race guards", () => { mockIsLoaded = true; mockConnectionStatus = "connected"; mockIsRestoringLayout = false; - mockApi = { getPanel: mockGetPanel }; + mockActivePanelId = "chat"; + mockApi = makeApi(); mockGetPanel.mockReturnValue(null); mockGetTaskPlan.mockResolvedValue(null); }); diff --git a/apps/web/hooks/use-plan-panel-auto-open.ts b/apps/web/hooks/use-plan-panel-auto-open.ts index f25911403b..fc2d26a5dc 100644 --- a/apps/web/hooks/use-plan-panel-auto-open.ts +++ b/apps/web/hooks/use-plan-panel-auto-open.ts @@ -90,14 +90,13 @@ export function usePlanPanelAutoOpen() { if (!api || isRestoringLayout) return; if (!plan || plan.created_by !== "agent") return; if (lastSeen === plan.updated_at) return; - if (api.getPanel("plan")) { + const planPanel = api.getPanel("plan"); + if (planPanel) { // Page-reload case: panel restored from saved layout and there is no - // recorded `lastSeen` (not persisted across sessions). We can't tell - // whether the plan was acknowledged before reload or not, so we - // optimistically mark it seen to avoid a stale indicator flash on - // every reload. When a live update arrives after the reload, - // `lastSeen` is defined so we don't suppress it — PlanTab reads the - // store directly and re-arms the indicator. + // recorded `lastSeen` (not persisted across sessions). Only mark the + // plan seen when the restored Plan panel is active, meaning the user is + // already looking at it. If Chat is active, the existing tab still needs + // its unseen indicator for a new or inactive plan. // // Guard on `addedPlanPanelRef`: only heal panels we did *not* add this // session. Otherwise this branch misfires when our own addPlanPanel @@ -105,7 +104,11 @@ export function usePlanPanelAutoOpen() { // resolves after the WS push and re-applies an equivalent plan object — // which would mark a freshly auto-opened plan seen and suppress the // indicator the user must see. - if (lastSeen === undefined && !addedPlanPanelRef.current) { + if ( + lastSeen === undefined && + !addedPlanPanelRef.current && + api.activePanel?.id === planPanel.id + ) { markTaskPlanSeen(plan.task_id); } return; diff --git a/apps/web/lib/state/slices/session/session-slice.ts b/apps/web/lib/state/slices/session/session-slice.ts index c7aeea4d44..f42753e41d 100644 --- a/apps/web/lib/state/slices/session/session-slice.ts +++ b/apps/web/lib/state/slices/session/session-slice.ts @@ -6,6 +6,7 @@ import { reconcileMessages } from "./message-signature"; import { migrateEnvKeyedData } from "@/lib/state/slices/session-runtime/session-runtime-slice"; import { prepareResultToSessionState } from "@/lib/state/slices/session-runtime/prepare-result"; import { createDebugLogger, isDebug } from "@/lib/debug/log"; +import { getPlanLastSeen, setPlanLastSeen } from "@/lib/local-storage"; const debugEnv = createDebugLogger("session:env-mapping"); @@ -132,6 +133,7 @@ export const defaultSessionState: SessionSliceState = { }; type ImmerSet = Parameters[0]; +type ImmerGet = () => SessionSlice; function buildMessageActions(set: ImmerSet) { return { @@ -226,14 +228,20 @@ function buildMessageActions(set: ImmerSet) { }; } -function buildTaskPlanActions(set: ImmerSet) { +function buildTaskPlanActions(set: ImmerSet, get: ImmerGet) { return { - setTaskPlan: (taskId: string, plan: Parameters[1]) => + setTaskPlan: (taskId: string, plan: Parameters[1]) => { + const shouldHydrateLastSeen = get().taskPlans.lastSeenUpdatedAtByTaskId[taskId] === undefined; + const storedLastSeen = shouldHydrateLastSeen ? getPlanLastSeen(taskId) : null; set((draft) => { draft.taskPlans.byTaskId[taskId] = plan; draft.taskPlans.loadingByTaskId[taskId] = false; draft.taskPlans.loadedByTaskId[taskId] = true; - }), + if (shouldHydrateLastSeen && storedLastSeen !== null) { + draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId] = storedLastSeen; + } + }); + }, setTaskPlanLoading: (taskId: string, loading: boolean) => set((draft) => { draft.taskPlans.loadingByTaskId[taskId] = loading; @@ -242,7 +250,8 @@ function buildTaskPlanActions(set: ImmerSet) { set((draft) => { draft.taskPlans.savingByTaskId[taskId] = saving; }), - clearTaskPlan: (taskId: string) => + clearTaskPlan: (taskId: string) => { + setPlanLastSeen(taskId, null); set((draft) => { // revisionContentCache is keyed by revisionId, so pick the IDs for this // task before deleting the revisions list and drop their cache entries. @@ -262,12 +271,16 @@ function buildTaskPlanActions(set: ImmerSet) { delete draft.taskPlans.previewRevisionIdByTaskId[taskId]; delete draft.taskPlans.comparePairByTaskId[taskId]; delete draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId]; - }), - markTaskPlanSeen: (taskId: string) => + }); + }, + markTaskPlanSeen: (taskId: string) => { + const plan = get().taskPlans.byTaskId[taskId]; + const lastSeen = plan?.updated_at ?? ""; + setPlanLastSeen(taskId, lastSeen); set((draft) => { - const plan = draft.taskPlans.byTaskId[taskId]; - draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId] = plan?.updated_at ?? ""; - }), + draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId] = lastSeen; + }); + }, setPlanRevisions: ( taskId: string, revisions: Parameters[1], @@ -427,7 +440,7 @@ export const createSessionSlice: StateCreator< [["zustand/immer", never]], [], SessionSlice -> = (set) => ({ +> = (set, get) => ({ ...defaultSessionState, ...buildMessageActions(set), addTurn: (turn) => @@ -472,7 +485,7 @@ export const createSessionSlice: StateCreator< set((draft) => { draft.activeModel.bySessionId[sessionId] = modelId; }), - ...buildTaskPlanActions(set), + ...buildTaskPlanActions(set, get), setQueueEntries: (sessionId, entries, meta) => set((draft) => { draft.queue.bySessionId[sessionId] = entries; diff --git a/apps/web/lib/state/slices/session/task-plans.test.ts b/apps/web/lib/state/slices/session/task-plans.test.ts index 9b7dbe482c..e72870fa96 100644 --- a/apps/web/lib/state/slices/session/task-plans.test.ts +++ b/apps/web/lib/state/slices/session/task-plans.test.ts @@ -1,10 +1,18 @@ -import { describe, it, expect } from "vitest"; +import { beforeEach, describe, it, expect, vi } from "vitest"; import { create } from "zustand"; import { immer } from "zustand/middleware/immer"; import { createSessionSlice } from "./session-slice"; import type { SessionSlice } from "./types"; import type { TaskPlan } from "@/lib/types/http"; +const mockGetPlanLastSeen = vi.fn(); +const mockSetPlanLastSeen = vi.fn(); + +vi.mock("@/lib/local-storage", () => ({ + getPlanLastSeen: (...args: unknown[]) => mockGetPlanLastSeen(...args), + setPlanLastSeen: (...args: unknown[]) => mockSetPlanLastSeen(...args), +})); + function makeStore() { return create()(immer(createSessionSlice)); } @@ -28,6 +36,11 @@ function makePlan(overrides: Partial = {}): TaskPlan { } describe("task plan slice", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetPlanLastSeen.mockReturnValue(null); + }); + it("markTaskPlanSeen writes the current plan updated_at", () => { const store = makeStore(); store.getState().setTaskPlan(TASK_ID, makePlan({ updated_at: TS_LATER })); @@ -35,6 +48,7 @@ describe("task plan slice", () => { store.getState().markTaskPlanSeen(TASK_ID); expect(store.getState().taskPlans.lastSeenUpdatedAtByTaskId[TASK_ID]).toBe(TS_LATER); + expect(mockSetPlanLastSeen).toHaveBeenCalledWith(TASK_ID, TS_LATER); }); it("markTaskPlanSeen with no plan writes an empty-string sentinel", () => { @@ -43,6 +57,16 @@ describe("task plan slice", () => { store.getState().markTaskPlanSeen("task-missing"); expect(store.getState().taskPlans.lastSeenUpdatedAtByTaskId["task-missing"]).toBe(""); + expect(mockSetPlanLastSeen).toHaveBeenCalledWith("task-missing", ""); + }); + + it("setTaskPlan hydrates stored lastSeenUpdatedAtByTaskId", () => { + mockGetPlanLastSeen.mockReturnValue(TS_LATER); + const store = makeStore(); + + store.getState().setTaskPlan(TASK_ID, makePlan({ updated_at: TS_LATER })); + + expect(store.getState().taskPlans.lastSeenUpdatedAtByTaskId[TASK_ID]).toBe(TS_LATER); }); it("setTaskPlan does not change lastSeenUpdatedAtByTaskId", () => { @@ -65,5 +89,6 @@ describe("task plan slice", () => { expect(store.getState().taskPlans.lastSeenUpdatedAtByTaskId[TASK_ID]).toBeUndefined(); expect(store.getState().taskPlans.byTaskId[TASK_ID]).toBeUndefined(); + expect(mockSetPlanLastSeen).toHaveBeenCalledWith(TASK_ID, null); }); });