diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index c85c594c..f94a2ad4 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -113,6 +113,12 @@ reset this file. permissions are still bypassed only when the user explicitly selects bypass mode. - Removed the `piAgents` flag after Pi native chat shipped default-on in v0.2.0. +- A `pickforge-lanes` MCP call in chat now expands to the run's lane cards — + the same cards the Settings panel shows, not a second copy (#362 PR 3, behind + the default-off `mcpToolDetail` flag). Lanes are live while the call is in + flight, so watching a `lanes_wait` is worth something, and frozen once it + finishes so a replayed message does not rewrite itself from a run that has + moved on. Abandon stays in Settings. - The working row now names what is running and how long it has been running, behind the new default-off `turnActivity` flag (#365). During a long MCP call the chat used to show a bare "Working" dot with no way to tell thinking from diff --git a/src/components/chat/McpCard.tsx b/src/components/chat/McpCard.tsx index 57a40730..616e90ea 100644 --- a/src/components/chat/McpCard.tsx +++ b/src/components/chat/McpCard.tsx @@ -3,6 +3,8 @@ import { compactInline, hasHiddenDetail } from "../../lib/chatDisplay"; import { IconChevronRight } from "../icons"; import type { ToolCallStatus } from "../../stores/agentChat"; import { Disclosure, StatusPill, type StatusIntent } from "../ui"; +import { pikitRowIsLive, pikitRunRef } from "../../lib/pikitRunRef"; +import { PiKitRunLanes } from "../pikit/PiKitRunLanes"; import "./chat.css"; const STATUS_INTENT: Record = { @@ -23,7 +25,15 @@ export function McpCard(props: { const open = () => props.open ?? localOpen(); const toggle = () => (props.onToggle ? props.onToggle() : setLocalOpen((v) => !v)); const detail = () => props.detail ?? ""; - const canExpand = () => hasHiddenDetail(detail(), 120); + const row = () => ({ + server: props.server, + tool: props.tool, + detail: props.detail, + status: props.status, + }); + const runRef = () => pikitRunRef(row()); + const live = () => pikitRowIsLive(row()); + const canExpand = () => hasHiddenDetail(detail(), 120) || runRef() !== null; return (
@@ -64,11 +74,18 @@ export function McpCard(props: { - -
+
+ {/* A pickforge-lanes call renders the run's lanes the way Settings + does — same component, so the two cannot drift (#362). Live while + the call is in flight, frozen afterwards, so a replayed row does + not quietly rewrite itself from a run that has moved on. */} + + {(run) => } + + 
{detail()}
-
- + +
); diff --git a/src/components/pikit/PiKitLanesPanel.tsx b/src/components/pikit/PiKitLanesPanel.tsx index b2cffe94..64f212a3 100644 --- a/src/components/pikit/PiKitLanesPanel.tsx +++ b/src/components/pikit/PiKitLanesPanel.tsx @@ -6,8 +6,9 @@ // token usage. import { For, Show, type JSX, createSignal, onCleanup, onMount } from "solid-js"; import { ConfirmDialog } from "../ConfirmDialog"; -import { Disclosure, ForgeEmptyState, MonoEyebrow } from "../ui"; -import { IconChevronRight, IconGrid, IconRefresh } from "../icons"; +import { RunCard, type AbandonTarget } from "./PiKitRunCard"; +import { ForgeEmptyState, MonoEyebrow } from "../ui"; +import { IconGrid, IconRefresh } from "../icons"; import type { PiKitRunEntry } from "../../lib/process"; import { loadAllPiKitRuns, @@ -20,145 +21,13 @@ import { startPiKitLanesPolling, stopPiKitLanesPolling, } from "../../stores/pikitLanes"; -import { - abandonDisabledReason, - abandonHint, - formatCost, - formatDuration, - formatTokens, - laneDetail, - laneStatusTone, - orphanNote, - runLabel, - runStatusTone, -} from "./pikitLaneDisplay"; import "./pikitLanes.css"; -interface AbandonTarget { - run: string; - lane: string | null; - label: string; -} - interface Notice { text: string; error: boolean; } -// eslint-disable-next-line max-lines-per-function -- single cohesive panel; see codebase-design note in PR. -function RunCard(props: { - entry: PiKitRunEntry; - onAbandon: (target: AbandonTarget) => void; -}): JSX.Element { - const [open, setOpen] = createSignal(false); - const entry = () => props.entry; - const status = () => entry().status; - const runAbandonReason = () => abandonDisabledReason(entry()); - const runAbandonHint = () => abandonHint(entry()); - - return ( -
- - -
- No status details available.
}> - {(s) => ( -
- - {(lane) => { - const reason = () => abandonDisabledReason(entry(), lane); - const hint = () => abandonHint(entry()); - return ( -
- - - {lane.state} - - - {lane.lane} - - {lane.model} · {lane.effort} · {formatTokens(lane.tokensIn)}/ - {formatTokens(lane.tokensOut)} tok · {formatCost(lane.cost)} ·{" "} - {formatDuration(lane.durationMs)} - - {laneDetail(lane)} - - -
- ); - }} - -
- )} - - - {(note) =>
{note()}
} - - 0}> - - - - -
- ); -} - /** The complete run list, behind the panel's "view all" affordance. Reuses * `ConfirmDialog`'s portal/backdrop shape rather than introducing a second * modal idiom (#363). */ diff --git a/src/components/pikit/PiKitRunCard.tsx b/src/components/pikit/PiKitRunCard.tsx new file mode 100644 index 00000000..808a27e2 --- /dev/null +++ b/src/components/pikit/PiKitRunCard.tsx @@ -0,0 +1,147 @@ +// One pi-kit run, rendered identically in Settings and in an agent chat's MCP +// row (#362). Extracted out of PiKitLanesPanel rather than written twice — +// a second lane card would drift from this one the first time either changed. +import { For, Show, type JSX, createSignal } from "solid-js"; +import { Disclosure } from "../ui"; +import { IconChevronRight, IconGrid } from "../icons"; +import type { PiKitRunEntry } from "../../lib/process"; +import { + abandonDisabledReason, + abandonHint, + formatCost, + formatDuration, + formatTokens, + laneDetail, + laneStatusTone, + orphanNote, + runLabel, + runStatusTone, +} from "./pikitLaneDisplay"; +import "./pikitLanes.css"; + +export interface AbandonTarget { + run: string; + lane: string | null; + label: string; +} + +// eslint-disable-next-line max-lines-per-function -- one cohesive card; splitting it would spread the lane row across files. +export function RunCard(props: { + entry: PiKitRunEntry; + onAbandon: (target: AbandonTarget) => void; + /** Hides the Abandon controls. Acting on a run belongs in the Settings panel + * that owns it — offering a destructive action two clicks from a replayed + * chat message would be a trap (#362). */ + readOnly?: boolean; +}): JSX.Element { + const [open, setOpen] = createSignal(false); + const entry = () => props.entry; + const status = () => entry().status; + const runAbandonReason = () => abandonDisabledReason(entry()); + const runAbandonHint = () => abandonHint(entry()); + + return ( +
+ + +
+ No status details available.
}> + {(s) => ( +
+ + {(lane) => { + const reason = () => abandonDisabledReason(entry(), lane); + const hint = () => abandonHint(entry()); + return ( +
+ + + {lane.state} + + + {lane.lane} + + {lane.model} · {lane.effort} · {formatTokens(lane.tokensIn)}/ + {formatTokens(lane.tokensOut)} tok · {formatCost(lane.cost)} ·{" "} + {formatDuration(lane.durationMs)} + + {laneDetail(lane)} + + + + +
+ ); + }} + +
+ )} + + + {(note) =>
{note()}
} + + 0}> + + + + +
+ ); +} + diff --git a/src/components/pikit/PiKitRunLanes.tsx b/src/components/pikit/PiKitRunLanes.tsx new file mode 100644 index 00000000..592a2f26 --- /dev/null +++ b/src/components/pikit/PiKitRunLanes.tsx @@ -0,0 +1,55 @@ +// One pi-kit run's lanes, rendered inside an agent chat's MCP row (#362). +// +// Renders the SAME `RunCard` Settings uses, so the two surfaces cannot drift. +// What differs is the lifecycle, not the rendering: +// +// live — the MCP call is in flight, so this polls while the row is open. +// That is what makes watching a `lanes_wait` worth anything. +// frozen — the call has finished, so it shows whatever the store already +// knows and never polls. A replayed row must not rewrite itself from +// a run that has since moved on, or blank out because the run was +// pruned. +// +// Polling is scoped to a MOUNTED, OPEN row: the card only mounts when its +// disclosure opens (`Disclosure` keeps closed bodies out of the DOM), so a +// transcript full of old lanes rows costs nothing. +import { Show, type JSX, onCleanup, onMount } from "solid-js"; +import { RunCard } from "./PiKitRunCard"; +import { + pikitRuns, + startPiKitLanesPolling, + stopPiKitLanesPolling, +} from "../../stores/pikitLanes"; + +export function PiKitRunLanes(props: { run: string; live: boolean }): JSX.Element { + onMount(() => { + if (!props.live) return; + // Idempotent: the Settings panel may already be polling, and this must not + // double the interval or stop it out from under that panel on cleanup — + // `startPiKitLanesPolling` no-ops when a poll is already running. + startPiKitLanesPolling(); + onCleanup(() => stopPiKitLanesPolling()); + }); + + const entry = () => pikitRuns().find((candidate) => candidate.run === props.run) ?? null; + + return ( + + {props.live ? "Waiting for lane status…" : `No lane status retained for ${props.run}.`} + + } + > + {(found) => ( +
+ {/* Abandon is deliberately inert here: acting on a run belongs in the + Settings panel that owns it, and offering it mid-transcript would + be a destructive action two clicks from a replayed message. */} + undefined} readOnly /> +
+ )} + + ); +} diff --git a/src/components/pikit/pikitLanes.css b/src/components/pikit/pikitLanes.css index 7761a036..fedbf9ad 100644 --- a/src/components/pikit/pikitLanes.css +++ b/src/components/pikit/pikitLanes.css @@ -236,3 +236,12 @@ overflow-y: auto; min-height: 0; } + +/* A run's lanes rendered inside a chat's MCP row (#362). The card keeps its own + frame; this just stops it inheriting the chat body's monospace tail styling + and gives it room from the argument summary below it. */ +.pf-pikit-inline { + display: flex; + flex-direction: column; + margin-bottom: var(--pf-space-sm); +} diff --git a/src/lib/pikitRunRef.ts b/src/lib/pikitRunRef.ts new file mode 100644 index 00000000..b39c098d --- /dev/null +++ b/src/lib/pikitRunRef.ts @@ -0,0 +1,46 @@ +// Correlating a `pickforge-lanes` MCP row to the pi-kit run it is about (#362). +// +// The row already carries everything needed — the parser attaches a compact +// summary of the tool input, and `lanes_wait`/`lanes_status` take a `run` +// argument. `lanes_spawn` has no run to name going in; its run id only exists +// in the result, which the same summary carries once the call completes. +// +// Kept as a pure function so the correlation is testable without a store, a +// timer, or a chat. + +/** The MCP server whose calls render lane cards. */ +export const PIKIT_LANES_SERVER = "pickforge-lanes"; + +/** pi-kit run ids are `run--`; matching that shape + * rather than "any token" keeps a stray word in a result from being read as a + * run id. */ +const RUN_ID = /\brun-[0-9A-Za-z]+(?:-[0-9A-Za-z]+)*\b/; + +export interface PiKitRowLike { + server: string; + tool: string; + detail?: string | null; + status?: "inProgress" | "completed" | "failed"; +} + +/** The run a `pickforge-lanes` row is about, or null when the row is not one + * of ours or has not named a run yet. + * + * A `lanes_spawn` row names no run until its result lands, so an in-flight + * spawn correctly resolves to null rather than guessing. */ +export function pikitRunRef(row: PiKitRowLike): string | null { + if (row.server !== PIKIT_LANES_SERVER) return null; + const detail = row.detail?.trim(); + if (!detail) return null; + return RUN_ID.exec(detail)?.[0] ?? null; +} + +/** Whether this row should poll for live lane state, or render the snapshot it + * captured when the call finished. + * + * Live while the call is in flight — that is what makes `lanes_wait` worth + * watching. Frozen once it completes, because a replayed row must not quietly + * rewrite itself from a run that has since moved on or been pruned. */ +export function pikitRowIsLive(row: PiKitRowLike): boolean { + return pikitRunRef(row) !== null && row.status === "inProgress"; +} diff --git a/tests/unit/PiKitRunLanes.test.tsx b/tests/unit/PiKitRunLanes.test.tsx new file mode 100644 index 00000000..5e2427a1 --- /dev/null +++ b/tests/unit/PiKitRunLanes.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { render } from "solid-js/web"; +import type { PiKitRunEntry } from "../../src/lib/process"; + +const testEnv = vi.hoisted(() => ({ invoke: vi.fn() })); +vi.mock("@tauri-apps/api/core", () => ({ invoke: testEnv.invoke })); + +const RUN = "run-20260726T101112-4821"; + +const ENTRY: PiKitRunEntry = { + run: RUN, + supported: true, + orphaned: false, + status: { + schemaVersion: 1, + revision: 3, + updatedAtMs: 1_000, + run: RUN, + state: "active", + durationMs: 5_000, + totals: { cost: 0.1, tokensIn: 100, tokensOut: 50 }, + lanes: [ + { + lane: "lane-1", + model: "openai-codex/gpt-5.6-sol", + effort: "medium", + mode: "read-only", + state: "running", + tokensIn: 100, + tokensOut: 50, + cost: 0.1, + context: 1200, + }, + ], + }, +}; + +let root: HTMLDivElement; +let dispose: (() => void) | undefined; + +beforeEach(() => { + testEnv.invoke.mockReset(); + vi.resetModules(); + vi.useFakeTimers(); + root = document.createElement("div"); + document.body.appendChild(root); +}); + +afterEach(() => { + dispose?.(); + dispose = undefined; + root.remove(); + vi.useRealTimers(); +}); + +async function mount(live: boolean) { + const { PiKitRunLanes } = await import("../../src/components/pikit/PiKitRunLanes"); + dispose = render(() => , root); + return root; +} + +function pollCount() { + return testEnv.invoke.mock.calls.filter((call) => String(call[0]).startsWith("list_pi_kit_run")).length; +} + +describe("PiKitRunLanes — live in flight, frozen after (#362)", () => { + it("polls while the call is in flight", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + expect(pollCount()).toBeGreaterThan(0); + expect(root.querySelector(".pf-pikit-card")).not.toBeNull(); + + const before = pollCount(); + await vi.advanceTimersByTimeAsync(4_000); + expect(pollCount()).toBeGreaterThan(before); + }); + + it("never polls once the call has finished", async () => { + // A replayed row must not rewrite itself from a run that has moved on. + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(false); + await vi.advanceTimersByTimeAsync(10_000); + + expect(pollCount()).toBe(0); + }); + + it("renders the same card component Settings uses", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + // The Settings panel's own selectors — proof it is one component, not a + // second lane card that will drift. + expect(root.querySelector(".pf-pikit-card")).not.toBeNull(); + expect(root.querySelector(".pf-pikit-summary")).not.toBeNull(); + }); + + it("offers no Abandon control in a transcript", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + root.querySelector(".pf-pikit-summary")!.click(); + + expect(root.querySelector(".pf-pikit-abandon")).toBeNull(); + expect(root.textContent).not.toContain("Abandon all lanes"); + }); + + it("says so plainly when a finished run's status is gone", async () => { + testEnv.invoke.mockResolvedValue({ runs: [], total: 0 }); + await mount(false); + await vi.advanceTimersByTimeAsync(0); + + expect(root.textContent).toContain("No lane status retained"); + expect(root.textContent).toContain(RUN); + }); + + it("shows a waiting note rather than an empty box while in flight", async () => { + testEnv.invoke.mockResolvedValue({ runs: [], total: 0 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + expect(root.textContent).toContain("Waiting for lane status"); + }); +}); diff --git a/tests/unit/pikitRunRef.test.ts b/tests/unit/pikitRunRef.test.ts new file mode 100644 index 00000000..dc4d21ee --- /dev/null +++ b/tests/unit/pikitRunRef.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; +import { pikitRowIsLive, pikitRunRef } from "../../src/lib/pikitRunRef"; + +const row = (over: Partial[0]> = {}) => ({ + server: "pickforge-lanes", + tool: "lanes_wait", + detail: "run: run-20260726T101112-4821", + status: "inProgress" as const, + ...over, +}); + +describe("pikitRunRef — correlating an MCP row to its pi-kit run (#362)", () => { + it("reads the run from a lanes_wait argument summary", () => { + expect(pikitRunRef(row())).toBe("run-20260726T101112-4821"); + }); + + it("reads the run out of a lanes_spawn result once it lands", () => { + // A spawn names no run going in; the id only exists in the result. + expect( + pikitRunRef(row({ tool: "lanes_spawn", detail: "started run-20260726T101112-4821 with 3 lanes" })), + ).toBe("run-20260726T101112-4821"); + }); + + it("resolves to nothing for an in-flight spawn that has not named a run", () => { + expect(pikitRunRef(row({ tool: "lanes_spawn", detail: "lanes: 3, model: sol" }))).toBeNull(); + }); + + it("ignores rows from any other MCP server", () => { + expect(pikitRunRef(row({ server: "github" }))).toBeNull(); + }); + + it("ignores a row with no detail at all", () => { + expect(pikitRunRef(row({ detail: null }))).toBeNull(); + expect(pikitRunRef(row({ detail: " " }))).toBeNull(); + }); + + it("does not mistake an ordinary word for a run id", () => { + expect(pikitRunRef(row({ detail: "waiting for the run to finish" }))).toBeNull(); + }); +}); + +describe("pikitRowIsLive — live in flight, frozen after (#362)", () => { + it("polls while the call is in flight", () => { + expect(pikitRowIsLive(row({ status: "inProgress" }))).toBe(true); + }); + + it("freezes once the call has completed or failed", () => { + // A replayed row must not rewrite itself from a run that has moved on. + expect(pikitRowIsLive(row({ status: "completed" }))).toBe(false); + expect(pikitRowIsLive(row({ status: "failed" }))).toBe(false); + }); + + it("never polls a row with no run to poll for", () => { + expect(pikitRowIsLive(row({ detail: null }))).toBe(false); + expect(pikitRowIsLive(row({ server: "github" }))).toBe(false); + }); + + it("does not poll a row whose status is unknown (pre-flag history)", () => { + expect(pikitRowIsLive(row({ status: undefined }))).toBe(false); + }); +});