From 9b98cc7767df49e6791ff7d3c2b86819f211e955 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:50:35 +0530 Subject: [PATCH 1/5] fix(review): preserve historical change snapshots --- src/review-checkpoints.test.ts | 50 ++++++++++++ src/review-checkpoints.ts | 144 +++++++++++++++++++++++++++++---- 2 files changed, 177 insertions(+), 17 deletions(-) diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index 499cdb9b..6e9d0963 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -62,12 +62,58 @@ test("show_changes reports and advances the last-shown checkpoint", async (t) => markReviewed: true, }); assert.equal(markedReviewed.summary.files, 2); + assert.match(markedReviewed.reviewRef, /^[0-9a-f]{40,64}$/); + + const restored = await manager.reviewByRef({ + workspaceId: "ws_incremental", + root, + reviewRef: markedReviewed.reviewRef, + }); + assert.deepEqual(restored.summary, markedReviewed.summary); + assert.deepEqual(restored.files, markedReviewed.files); + assert.equal(restored.patch, markedReviewed.patch); const afterReviewed = await manager.reviewChanges({ workspaceId: "ws_incremental", root }); assert.equal(afterReviewed.summary.files, 0); assert.equal(afterReviewed.patch, ""); }); +test("historical review refs survive later reviews and manager restarts", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_history", root }); + + await writeFile(join(root, "README.md"), "hello\nfirst\n"); + const first = await manager.reviewChanges({ workspaceId: "ws_history", root }); + + await writeFile(join(root, "README.md"), "hello\nfirst\nsecond\n"); + const second = await manager.reviewChanges({ workspaceId: "ws_history", root }); + assert.notEqual(first.reviewRef, second.reviewRef); + + const restarted = createReviewCheckpointManager(); + const restoredFirst = await restarted.reviewByRef({ + workspaceId: "ws_history", + root, + reviewRef: first.reviewRef, + }); + assert.deepEqual(restoredFirst.summary, first.summary); + assert.equal(restoredFirst.patch, first.patch); + assert.match(restoredFirst.patch, /\+first/); + assert.doesNotMatch(restoredFirst.patch, /\+second/); +}); + +test("review refs are scoped to the workspace review history", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_scoped", root }); + + const head = await gitOutput(root, ["rev-parse", "HEAD"]); + await assert.rejects( + () => manager.reviewByRef({ workspaceId: "ws_scoped", root, reviewRef: head }), + /Unknown review reference/, + ); +}); + test("review checkpoints survive a manager restart", async (t) => { const root = await committedRepository(t); const manager = createReviewCheckpointManager(); @@ -246,3 +292,7 @@ async function deleteReviewRef( async function git(cwd: string, args: string[]): Promise { await execFileAsync("git", args, { cwd }); } + +async function gitOutput(cwd: string, args: string[]): Promise { + return (await execFileAsync("git", args, { cwd })).stdout.trim(); +} diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index 21a0d660..037ca8c5 100644 --- a/src/review-checkpoints.ts +++ b/src/review-checkpoints.ts @@ -20,6 +20,7 @@ export interface ReviewFile { } export interface ReviewChangesResult { + reviewRef: string; result: string; summary: ReviewSummary; files: ReviewFile[]; @@ -48,6 +49,11 @@ export interface ReviewCheckpointManager { since?: ReviewSince; markReviewed?: boolean; }): Promise; + reviewByRef(input: { + workspaceId: string; + root: string; + reviewRef: string; + }): Promise; } const REVIEW_REF_PREFIX = "refs/devspace/review"; @@ -113,15 +119,8 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { const baselineRef = effectiveSince === "workspace_open" ? state.openRef : state.baselineRef; const baseline = (await git(state.gitRoot, ["rev-parse", "--verify", `${baselineRef}^{commit}`])).stdout.trim(); - const current = await createWorkingTreeSnapshot(state.gitRoot); - const patch = (await git(state.gitRoot, ["diff", "--binary", "--no-color", baseline, current], { - maxBuffer: 50 * 1024 * 1024, - })).stdout; - const numstat = (await git(state.gitRoot, ["diff", "--numstat", "-z", baseline, current], { - maxBuffer: 50 * 1024 * 1024, - })).stdout; - const files = parseNumstat(numstat); - const summary = summarizeFiles(files); + const current = await createWorkingTreeSnapshot(state.gitRoot, baseline); + const review = await readReviewBetween(state.gitRoot, baseline, current); if (markReviewed) { await git(state.gitRoot, ["update-ref", state.baselineRef, current]); @@ -132,19 +131,66 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { ? ` The last-shown checkpoint was missing, so changes were compared from workspace open${markReviewed ? " and the baseline was re-established" : ""}.` : ""; return { + reviewRef: current, result: `${ - summary.files === 0 + review.summary.files === 0 ? `No changes since ${effectiveSince === "workspace_open" ? "workspace open" : "last shown changes"}.` - : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).` + : formatChangedFiles(review.summary) }${fallbackNote}`, - summary, - files, - patch, + ...review, }; }, + + async reviewByRef({ workspaceId, root, reviewRef }) { + let state = states.get(workspaceId); + assertWorkspaceRoot(state, workspaceId, root); + if (!isReadyState(state)) { + await this.initializeWorkspace({ workspaceId, root }); + state = states.get(workspaceId); + } + assertWorkspaceRoot(state, workspaceId, root); + + if (!state?.gitRoot) { + throw new Error(state?.diagnostic ?? "show_changes requires a Git workspace in this version."); + } + + const [openCommit, baselineCommit, reviewCommit] = await Promise.all([ + commitForRef(state.gitRoot, state.openRef), + commitForRef(state.gitRoot, state.baselineRef), + resolveReviewCommitOrUndefined(state.gitRoot, reviewRef), + ]); + if ( + !openCommit + || !baselineCommit + || !reviewCommit + || reviewCommit === openCommit + ) { + throw new Error(`Unknown review reference for workspace ${workspaceId}: ${reviewRef}`); + } + + const [isAfterOpen, isBeforeBaseline] = await Promise.all([ + isAncestor(state.gitRoot, openCommit, reviewCommit), + isAncestor(state.gitRoot, reviewCommit, baselineCommit), + ]); + if (!isAfterOpen || !isBeforeBaseline) { + throw new Error(`Unknown review reference for workspace ${workspaceId}: ${reviewRef}`); + } + + return readReviewCommit(state.gitRoot, reviewCommit); + }, }; } +export async function readReviewRef(root: string, reviewRef: string): Promise { + const eligibility = await getGitEligibility(root); + if (!eligibility.ok || !eligibility.gitRoot) { + throw new Error(eligibility.message ?? "show-changes requires a Git workspace."); + } + + const commit = await resolveReviewCommit(eligibility.gitRoot, reviewRef); + return readReviewCommit(eligibility.gitRoot, commit); +} + function assertWorkspaceRoot( state: WorkspaceReviewState | undefined, workspaceId: string, @@ -181,7 +227,8 @@ async function initializeWorkspaceState( ]); if (!openCommit && !baselineCommit) { - const initialCommit = await createWorkingTreeSnapshot(eligibility.gitRoot); + const head = (await git(eligibility.gitRoot, ["rev-parse", "--verify", "HEAD^{commit}"])).stdout.trim(); + const initialCommit = await createWorkingTreeSnapshot(eligibility.gitRoot, head); await git(eligibility.gitRoot, ["update-ref", state.openRef, initialCommit]); await git(eligibility.gitRoot, ["update-ref", state.baselineRef, initialCommit]); state.openRefAvailable = true; @@ -230,7 +277,7 @@ function reviewRefs( }; } -async function createWorkingTreeSnapshot(gitRoot: string): Promise { +async function createWorkingTreeSnapshot(gitRoot: string, parent: string): Promise { const tempDir = await mkdtemp(join(tmpdir(), "devspace-review-index-")); const indexPath = join(tempDir, "index"); const env = checkpointEnv(indexPath); @@ -239,13 +286,76 @@ async function createWorkingTreeSnapshot(gitRoot: string): Promise { await git(gitRoot, ["read-tree", "HEAD"], { env }); await git(gitRoot, ["add", "-A", "--", "."], { env }); const tree = (await git(gitRoot, ["write-tree"], { env })).stdout.trim(); - const parent = (await git(gitRoot, ["rev-parse", "--verify", "HEAD^{commit}"])).stdout.trim(); return (await git(gitRoot, ["commit-tree", tree, "-p", parent, "-m", "DevSpace review snapshot"], { env })).stdout.trim(); } finally { await rm(tempDir, { recursive: true, force: true }); } } +async function readReviewCommit(gitRoot: string, reviewRef: string): Promise { + const parent = (await git(gitRoot, ["rev-parse", "--verify", `${reviewRef}^1`])).stdout.trim(); + const review = await readReviewBetween(gitRoot, parent, reviewRef); + return { + reviewRef, + result: review.summary.files === 0 ? "No changes in this review." : formatChangedFiles(review.summary), + ...review, + }; +} + +async function readReviewBetween( + gitRoot: string, + before: string, + after: string, +): Promise> { + const patch = (await git(gitRoot, ["diff", "--binary", "--no-color", before, after], { + maxBuffer: 50 * 1024 * 1024, + })).stdout; + const numstat = (await git(gitRoot, ["diff", "--numstat", "-z", before, after], { + maxBuffer: 50 * 1024 * 1024, + })).stdout; + const files = parseNumstat(numstat); + return { + summary: summarizeFiles(files), + files, + patch, + }; +} + +async function resolveReviewCommit(gitRoot: string, reviewRef: string): Promise { + if (!isReviewRef(reviewRef)) { + throw new Error(`Invalid review reference: ${reviewRef}`); + } + return (await git(gitRoot, ["rev-parse", "--verify", `${reviewRef}^{commit}`])).stdout.trim(); +} + +async function resolveReviewCommitOrUndefined( + gitRoot: string, + reviewRef: string, +): Promise { + try { + return await resolveReviewCommit(gitRoot, reviewRef); + } catch { + return undefined; + } +} + +async function isAncestor(gitRoot: string, ancestor: string, descendant: string): Promise { + try { + await git(gitRoot, ["merge-base", "--is-ancestor", ancestor, descendant]); + return true; + } catch { + return false; + } +} + +function isReviewRef(value: string): boolean { + return /^[0-9a-f]{40,64}$/.test(value); +} + +function formatChangedFiles(summary: ReviewSummary): string { + return `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).`; +} + function checkpointEnv(indexPath: string): NodeJS.ProcessEnv { return { GIT_INDEX_FILE: indexPath, From e70c2b1585212045c4c962cdc4a247776baf5686 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:50:35 +0530 Subject: [PATCH 2/5] fix(ui): restore cards from durable tool output --- package.json | 2 +- src/server.test.ts | 70 +++++++++-- src/server.ts | 47 +++---- src/ui/card-types.test.ts | 7 -- src/ui/card-types.ts | 8 -- src/ui/tool-result.test.ts | 137 ++++++++++++++++++++ src/ui/tool-result.ts | 250 +++++++++++++++++++++++++++++++++++++ src/ui/vite-env.d.ts | 7 ++ src/ui/workspace-app.tsx | 160 +++++++++++++++++------- 9 files changed, 593 insertions(+), 95 deletions(-) create mode 100644 src/ui/tool-result.test.ts create mode 100644 src/ui/tool-result.ts diff --git a/package.json b/package.json index dfa5cf55..6d60fee7 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "postinstall": "node scripts/fix-node-pty-permissions.mjs", "schema:config": "tsx scripts/generate-config-schema.ts", "start": "node dist/cli.js serve", - "test": "tsx src/user-config.test.ts && tsx src/config.test.ts && tsx src/onboarding.test.ts && tsx src/cli-workspace.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-config.test.ts && tsx src/local-agent-catalog.test.ts && tsx src/local-agent-presentation.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-daemon-lifecycle.test.ts && tsx src/local-agent-daemon-protocol.test.ts && tsx src/local-agent-daemon.test.ts && tsx src/local-agent-codex.test.ts && tsx src/local-agent-opencode.test.ts && tsx src/local-agent-acp.test.ts && tsx src/local-agent-grok.test.ts && tsx src/local-agent-pi-sandbox.test.ts && tsx src/local-agent-pi.test.ts && tsx src/local-agent-claude.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/local-agent-manager.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", + "test": "tsx src/user-config.test.ts && tsx src/config.test.ts && tsx src/onboarding.test.ts && tsx src/cli-workspace.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/tool-result.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-config.test.ts && tsx src/local-agent-catalog.test.ts && tsx src/local-agent-presentation.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-daemon-lifecycle.test.ts && tsx src/local-agent-daemon-protocol.test.ts && tsx src/local-agent-daemon.test.ts && tsx src/local-agent-codex.test.ts && tsx src/local-agent-opencode.test.ts && tsx src/local-agent-acp.test.ts && tsx src/local-agent-grok.test.ts && tsx src/local-agent-pi-sandbox.test.ts && tsx src/local-agent-pi.test.ts && tsx src/local-agent-claude.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/local-agent-manager.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli-show-changes.test.ts && tsx src/cli.test.ts", "typecheck": "tsx src/config-schema.test.ts && tsc -p tsconfig.json --noEmit" }, "keywords": [], diff --git a/src/server.test.ts b/src/server.test.ts index b7d8d351..127abba1 100644 --- a/src/server.test.ts +++ b/src/server.test.ts @@ -74,7 +74,7 @@ test("open_workspace reports aggregate review availability", async (t) => { assert.deepEqual(gitReview, { available: true }); }); -test("show_changes exposes the aggregate diff to plain MCP hosts", async (t) => { +test("show_changes keeps model output compact and preserves the rich review card", async (t) => { const context = await fixture(t, { git: true, uiEnabled: false }); const opened = structuredContent( await callOpen(context.client, context.project, "review"), @@ -88,13 +88,22 @@ test("show_changes exposes the aggregate diff to plain MCP hosts", async (t) => arguments: { workspaceId }, }); const structured = structuredContent(review); + assert.equal((review._meta as Record | undefined)?.tool, undefined); - assert.deepEqual(structured.summary, { + assert.equal(structured.workspaceId, workspaceId); + assert.match(structured.reviewRef as string, /^[0-9a-f]{40,64}$/); + assert.match(structured.result as string, /Changed 1 file \(\+1 -1\)/); + assert.equal("summary" in structured, false); + assert.equal("files" in structured, false); + assert.equal("patch" in structured, false); + + const card = responseCard(review); + assert.deepEqual(card.summary, { files: 1, additions: 1, removals: 1, }); - assert.deepEqual(structured.files, [ + assert.deepEqual(card.files, [ { path: "README.md", type: "change", @@ -102,14 +111,59 @@ test("show_changes exposes the aggregate diff to plain MCP hosts", async (t) => removals: 1, }, ]); - assert.match(structured.patch as string, /-hello\n\+goodbye/); + assert.match( + ((card.payload as { patch?: string } | undefined)?.patch) ?? "", + /-hello\n\+goodbye/, + ); const tools = await context.client.listTools(); const outputProperties = tools.tools.find((tool) => tool.name === "show_changes") ?.outputSchema?.properties; - assert.ok(outputProperties && "summary" in outputProperties); - assert.ok(outputProperties && "files" in outputProperties); - assert.ok(outputProperties && "patch" in outputProperties); + assert.ok(outputProperties && "workspaceId" in outputProperties); + assert.ok(outputProperties && "reviewRef" in outputProperties); + assert.equal(outputProperties && "summary" in outputProperties, false); + assert.equal(outputProperties && "files" in outputProperties, false); + assert.equal(outputProperties && "patch" in outputProperties, false); + const inputProperties = tools.tools.find((tool) => tool.name === "show_changes") + ?.inputSchema?.properties; + assert.equal(inputProperties && "reviewRef" in inputProperties, false); +}); + +test("show_changes can reopen a historical review without advancing the checkpoint", async (t) => { + const context = await fixture(t, { git: true }); + const workspaceId = structuredContent( + await callOpen(context.client, context.project, "review-history"), + ).workspaceId; + assert.equal(typeof workspaceId, "string"); + + await writeFile(join(context.project, "README.md"), "first\n"); + const first = structuredContent(await context.client.callTool({ + name: "show_changes", + arguments: { workspaceId }, + })); + const reviewRef = first.reviewRef; + assert.equal(typeof reviewRef, "string"); + + await writeFile(join(context.project, "README.md"), "second\n"); + const reopened = await context.client.callTool({ + name: "show_changes", + arguments: { workspaceId }, + _meta: { "devspace/reviewRef": reviewRef }, + } as Parameters[0]); + assert.equal(structuredContent(reopened).reviewRef, reviewRef); + assert.match( + (((responseCard(reopened).payload as { patch?: string } | undefined)?.patch) ?? ""), + /\+first/, + ); + + const current = await context.client.callTool({ + name: "show_changes", + arguments: { workspaceId }, + }); + assert.match( + (((responseCard(current).payload as { patch?: string } | undefined)?.patch) ?? ""), + /-first\n\+second/, + ); }); test("open_workspace keeps lifecycle flags out of model output and preserves complete card metadata", async (t) => { @@ -119,6 +173,8 @@ test("open_workspace keeps lifecycle flags out of model output and preserves com }); const first = await callOpen(context.client, context.project, "chat-1"); const repeated = await callOpen(context.client, context.project, "chat-1"); + assert.equal((first._meta as Record | undefined)?.tool, undefined); + assert.equal((repeated._meta as Record | undefined)?.tool, undefined); const tools = await context.client.listTools(); const openTool = tools.tools.find((tool) => tool.name === "open_workspace"); diff --git a/src/server.ts b/src/server.ts index dd0a15e8..9e7ded7f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -167,20 +167,6 @@ const workspaceAvailableAgentsFileOutputSchema = z.object({ path: z.string(), }); -const reviewFileOutputSchema = z.object({ - path: z.string(), - previousPath: z.string().optional(), - type: z.enum(["change", "rename-pure", "rename-changed", "new", "deleted"]), - additions: z.number(), - removals: z.number(), -}); - -const reviewSummaryOutputSchema = z.object({ - files: z.number(), - additions: z.number(), - removals: z.number(), -}); - function sendJsonRpcError( res: Response, status: number, @@ -504,7 +490,6 @@ export function createMcpServer( return { content: resultContent, _meta: { - tool: "open_workspace", card: { workspaceId: workspace.id, root: workspace.root, @@ -653,21 +638,29 @@ export function createMcpServer( workspaceId: z.string().describe(workspaceIdDescription), }, outputSchema: resultOutputSchema({ - summary: reviewSummaryOutputSchema, - files: z.array(reviewFileOutputSchema), - patch: z.string(), + workspaceId: z.string(), + reviewRef: z.string().regex(/^[0-9a-f]{40,64}$/), }), ...workspaceAppDescriptorMeta(config), annotations: { readOnlyHint: true }, }, - async ({ workspaceId }) => { + async ({ workspaceId }, { _meta }) => { const startedAt = performance.now(); const workspace = workspaces.getWorkspace(workspaceId); - const review = await reviewCheckpoints.reviewChanges({ - workspaceId, - root: workspace.root, - markReviewed: true, - }); + const reviewRef = typeof _meta?.["devspace/reviewRef"] === "string" + ? _meta["devspace/reviewRef"] + : undefined; + const review = reviewRef + ? await reviewCheckpoints.reviewByRef({ + workspaceId, + root: workspace.root, + reviewRef, + }) + : await reviewCheckpoints.reviewChanges({ + workspaceId, + root: workspace.root, + markReviewed: true, + }); const content = [textBlock(review.result)]; logToolCall(config, { @@ -680,7 +673,6 @@ export function createMcpServer( return { content, _meta: { - tool: "show_changes", card: { workspaceId, summary: review.summary, @@ -691,10 +683,9 @@ export function createMcpServer( }, }, structuredContent: { + workspaceId, + reviewRef: review.reviewRef, result: contentText(content), - summary: review.summary, - files: review.files, - patch: review.patch, }, }; }, diff --git a/src/ui/card-types.test.ts b/src/ui/card-types.test.ts index 2ae9f55e..19e5b582 100644 --- a/src/ui/card-types.test.ts +++ b/src/ui/card-types.test.ts @@ -3,15 +3,8 @@ import test from "node:test"; import { isExpandableCard, isInitiallyExpandedCard, - isToolName, } from "./card-types.js"; -test("only UI-backed tools are recognized as card tools", () => { - assert.equal(isToolName("open_workspace"), true); - assert.equal(isToolName("show_changes"), true); - assert.equal(isToolName("read"), false); -}); - test("aggregate review opens when a patch is available", () => { const card = { tool: "show_changes" as const, diff --git a/src/ui/card-types.ts b/src/ui/card-types.ts index 5ab0a532..6c84d0fa 100644 --- a/src/ui/card-types.ts +++ b/src/ui/card-types.ts @@ -67,14 +67,6 @@ export interface ToolResultCard { instruction?: string; } -export function isToolName(value: unknown): value is ToolName { - return value === "open_workspace" || value === "show_changes"; -} - -export function isToolResultCard(value: unknown): value is Omit { - return Boolean(value && typeof value === "object"); -} - export function summaryNumber( summary: Record | undefined, key: string, diff --git a/src/ui/tool-result.test.ts b/src/ui/tool-result.test.ts new file mode 100644 index 00000000..4687e028 --- /dev/null +++ b/src/ui/tool-result.test.ts @@ -0,0 +1,137 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { + decodeToolResult, + toolResultFromChatGptGlobals, +} from "./tool-result.js"; + +test("workspace cards can be rebuilt from structured content without result metadata", () => { + const decoded = decodeToolResult({ + content: [], + structuredContent: { + workspaceId: "ws_1", + root: "/tmp/project", + mode: "checkout", + skills: [{ name: "tdd", description: "Tests first", path: "/tmp/tdd/SKILL.md" }], + agentsFiles: [{ path: "AGENTS.md", content: "instructions" }], + review: { available: true }, + instruction: "Reuse this workspace.", + }, + }); + + assert.equal(decoded.kind, "card"); + if (decoded.kind !== "card") return; + assert.equal(decoded.card.tool, "open_workspace"); + assert.equal(decoded.card.workspaceId, "ws_1"); + assert.equal(decoded.card.summary?.skills, 1); + assert.equal(decoded.card.summary?.agentsFiles, 1); +}); + +test("review results use rich metadata when the host provides it", () => { + const decoded = decodeToolResult({ + content: [], + structuredContent: { + workspaceId: "ws_1", + reviewRef: "a".repeat(40), + result: "Changed 1 file (+1 -0).", + }, + _meta: { + card: { + workspaceId: "ws_1", + summary: { files: 1, additions: 1, removals: 0 }, + files: [{ path: "new.txt", type: "new", additions: 1, removals: 0 }], + payload: { patch: "diff --git ..." }, + }, + }, + }); + + assert.equal(decoded.kind, "card"); + if (decoded.kind !== "card") return; + assert.equal(decoded.card.tool, "show_changes"); + assert.equal(decoded.card.files?.[0]?.path, "new.txt"); + assert.equal(decoded.card.payload?.patch, "diff --git ..."); +}); + +test("review structured content becomes a reload reference when metadata is missing", () => { + const decoded = decodeToolResult({ + content: [], + structuredContent: { + workspaceId: "ws_1", + reviewRef: "b".repeat(40), + result: "Changed 1 file (+1 -0).", + }, + }); + + assert.deepEqual(decoded, { + kind: "review-reference", + workspaceId: "ws_1", + reviewRef: "b".repeat(40), + }); +}); + +test("older review results can reload from their structured patch", () => { + const decoded = decodeToolResult({ + content: [], + structuredContent: { + result: "Changed 1 file (+1 -0).", + summary: { files: 1, additions: 1, removals: 0 }, + files: [{ path: "new.txt", type: "new", additions: 1, removals: 0 }], + patch: "diff --git a/new.txt b/new.txt", + }, + }); + + assert.equal(decoded.kind, "card"); + if (decoded.kind !== "card") return; + assert.equal(decoded.card.tool, "show_changes"); + assert.equal(decoded.card.files?.[0]?.path, "new.txt"); + assert.equal(decoded.card.payload?.patch, "diff --git a/new.txt b/new.txt"); +}); + +test("ChatGPT globals restore structured output and hidden MCP result metadata together", () => { + const fullResult: CallToolResult = { + content: [{ type: "text", text: "Changed 1 file." }], + structuredContent: { stale: true }, + _meta: { card: { workspaceId: "ws_1", payload: { patch: "patch" } } }, + }; + const restored = toolResultFromChatGptGlobals({ + toolOutput: { + workspaceId: "ws_1", + reviewRef: "c".repeat(40), + result: "Changed 1 file.", + }, + toolResponseMetadata: { + mcp_tool_result: fullResult, + }, + }); + + assert.deepEqual(restored?.structuredContent, { + workspaceId: "ws_1", + reviewRef: "c".repeat(40), + result: "Changed 1 file.", + }); + assert.deepEqual(restored?._meta, fullResult._meta); +}); + +test("ChatGPT globals also accept result metadata exposed directly", () => { + const restored = toolResultFromChatGptGlobals({ + toolOutput: { + workspaceId: "ws_1", + reviewRef: "d".repeat(40), + result: "Changed 1 file.", + }, + toolResponseMetadata: { + card: { + workspaceId: "ws_1", + summary: { files: 1, additions: 1, removals: 0 }, + }, + }, + }); + + assert.deepEqual(restored?._meta, { + card: { + workspaceId: "ws_1", + summary: { files: 1, additions: 1, removals: 0 }, + }, + }); +}); diff --git a/src/ui/tool-result.ts b/src/ui/tool-result.ts new file mode 100644 index 00000000..d029eaf1 --- /dev/null +++ b/src/ui/tool-result.ts @@ -0,0 +1,250 @@ +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import type { ReviewFileType, ToolResultCard } from "./card-types.js"; + +export type DecodedToolResult = + | { kind: "card"; card: ToolResultCard } + | { kind: "review-reference"; workspaceId: string; reviewRef: string } + | { kind: "invalid" }; + +export interface ChatGptToolGlobals { + toolOutput?: unknown; + toolResponseMetadata?: unknown; +} + +export function decodeToolResult(result: CallToolResult): DecodedToolResult { + const structured = asRecord(result.structuredContent); + const metaCard = cardFields(asRecord(asRecord(result._meta)?.card)); + + if (structured) { + const workspaceId = stringField(structured.workspaceId); + const reviewRef = stringField(structured.reviewRef); + if (workspaceId && reviewRef) { + if (metaCard) { + return { + kind: "card", + card: { + ...metaCard, + tool: "show_changes", + workspaceId, + }, + }; + } + return { kind: "review-reference", workspaceId, reviewRef }; + } + + if (typeof structured.patch === "string" && Array.isArray(structured.files)) { + const legacyCard = cardFields({ + ...structured, + payload: { patch: structured.patch }, + }); + if (legacyCard) { + return { kind: "card", card: { ...legacyCard, tool: "show_changes" } }; + } + } + + const root = stringField(structured.root); + const mode = workspaceMode(structured.mode); + if (workspaceId && root && mode) { + const structuredCard = cardFields(structured) ?? {}; + return { + kind: "card", + card: { + ...structuredCard, + ...metaCard, + tool: "open_workspace", + workspaceId, + root, + mode, + summary: metaCard?.summary ?? workspaceSummary(structuredCard), + }, + }; + } + } + + // Existing conversations created before reviewRef was added can still render + // while the host supplies their live MCP Apps result metadata. + if (metaCard?.workspaceId && (metaCard.files?.length || metaCard.payload?.patch)) { + return { kind: "card", card: { ...metaCard, tool: "show_changes" } }; + } + if (metaCard?.workspaceId && metaCard.root && metaCard.mode) { + return { kind: "card", card: { ...metaCard, tool: "open_workspace" } }; + } + + return { kind: "invalid" }; +} + +export function toolResultFromChatGptGlobals( + globals: ChatGptToolGlobals | undefined, +): CallToolResult | undefined { + if (!globals) return undefined; + + const responseMetadata = asRecord(globals.toolResponseMetadata); + const metadataResult = mcpToolResult(globals.toolResponseMetadata); + const structuredContent = asRecord(globals.toolOutput) + ?? asRecord(metadataResult?.structuredContent); + const resultMeta = asRecord(metadataResult?._meta) + ?? directResultMeta(responseMetadata); + if (!metadataResult && !structuredContent && !resultMeta) return undefined; + + return { + ...(metadataResult ?? { content: [] }), + ...(structuredContent ? { structuredContent } : {}), + ...(resultMeta ? { _meta: resultMeta } : {}), + } as CallToolResult; +} + +function directResultMeta( + metadata: Record | undefined, +): Record | undefined { + if (!metadata) return undefined; + return "card" in metadata ? metadata : undefined; +} + +function mcpToolResult(value: unknown): CallToolResult | undefined { + const metadata = asRecord(value); + if (!metadata) return undefined; + + const direct = asRecord(metadata.mcp_tool_result); + if (direct) return direct as CallToolResult; + + const callToolResult = asRecord(metadata.call_tool_result); + const nested = asRecord(callToolResult?.mcp_tool_result); + return nested ? nested as CallToolResult : undefined; +} + +function cardFields(record: Record | undefined): Partial | undefined { + if (!record) return undefined; + + const agentsFiles = arrayRecords(record.agentsFiles)?.map((item) => ({ + path: stringField(item.path), + content: stringField(item.content), + })); + const availableAgentsFiles = arrayRecords(record.availableAgentsFiles)?.map((item) => ({ + path: stringField(item.path), + })); + const skills = arrayRecords(record.skills)?.map((item) => ({ + name: stringField(item.name), + description: stringField(item.description), + path: stringField(item.path), + })); + const agentProviders = arrayRecords(record.agentProviders)?.map((item) => ({ + id: stringField(item.id), + model: stringField(item.model), + effort: stringField(item.effort), + note: stringField(item.note), + })); + const agents = arrayRecords(record.agents)?.map((item) => ({ + name: stringField(item.name), + description: stringField(item.description), + provider: stringField(item.provider), + model: stringField(item.model), + effort: stringField(item.effort), + })); + const files = arrayRecords(record.files)?.map((item) => ({ + path: stringField(item.path), + previousPath: stringField(item.previousPath), + type: reviewFileType(item.type), + additions: numberField(item.additions), + removals: numberField(item.removals), + })); + const worktreeRecord = asRecord(record.worktree); + const reviewRecord = asRecord(record.review); + const summary = asRecord(record.summary); + const payloadRecord = asRecord(record.payload); + + return definedFields({ + workspaceId: stringField(record.workspaceId), + path: stringField(record.path), + root: stringField(record.root), + workspaceReused: booleanField(record.workspaceReused), + includeBootstrapContext: booleanField(record.includeBootstrapContext), + mode: workspaceMode(record.mode), + sourceRoot: stringField(record.sourceRoot), + worktree: worktreeRecord + ? { + path: stringField(worktreeRecord.path), + baseRef: stringField(worktreeRecord.baseRef), + baseSha: stringField(worktreeRecord.baseSha), + dirtySource: booleanField(worktreeRecord.dirtySource), + detached: booleanField(worktreeRecord.detached), + managed: booleanField(worktreeRecord.managed), + } + : undefined, + review: reviewAvailability(reviewRecord), + summary, + files, + payload: payloadRecord ? { patch: stringField(payloadRecord.patch) } : undefined, + agentsFiles, + availableAgentsFiles, + skills, + agentProviders, + agents, + instruction: stringField(record.instruction), + }); +} + +function workspaceSummary(card: Partial): Record { + return { + mode: card.mode, + agentsFiles: card.agentsFiles?.length ?? 0, + availableAgentsFiles: card.availableAgentsFiles?.length ?? 0, + skills: card.skills?.length ?? 0, + agentProviders: card.agentProviders?.length ?? 0, + agents: card.agents?.length ?? 0, + }; +} + +function reviewAvailability( + record: Record | undefined, +): ToolResultCard["review"] { + if (!record || typeof record.available !== "boolean") return undefined; + if (record.available) return { available: true }; + const reason = stringField(record.reason); + return reason ? { available: false, reason } : undefined; +} + +function reviewFileType(value: unknown): ReviewFileType | undefined { + return value === "change" + || value === "rename-pure" + || value === "rename-changed" + || value === "new" + || value === "deleted" + ? value + : undefined; +} + +function workspaceMode(value: unknown): ToolResultCard["mode"] { + return value === "checkout" || value === "worktree" ? value : undefined; +} + +function arrayRecords(value: unknown): Array> | undefined { + if (!Array.isArray(value)) return undefined; + return value.flatMap((item) => { + const record = asRecord(item); + return record ? [record] : []; + }); +} + +function asRecord(value: unknown): Record | undefined { + return value !== null && typeof value === "object" + ? value as Record + : undefined; +} + +function stringField(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +function numberField(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) ? value : undefined; +} + +function booleanField(value: unknown): boolean | undefined { + return typeof value === "boolean" ? value : undefined; +} + +function definedFields>(record: T): T { + return Object.fromEntries( + Object.entries(record).filter(([, value]) => value !== undefined), + ) as T; +} diff --git a/src/ui/vite-env.d.ts b/src/ui/vite-env.d.ts index cbe652db..e224b6eb 100644 --- a/src/ui/vite-env.d.ts +++ b/src/ui/vite-env.d.ts @@ -1 +1,8 @@ declare module "*.css"; + +interface Window { + openai?: { + toolOutput?: unknown; + toolResponseMetadata?: unknown; + }; +} diff --git a/src/ui/workspace-app.tsx b/src/ui/workspace-app.tsx index c3c6f36e..bc101cb3 100644 --- a/src/ui/workspace-app.tsx +++ b/src/ui/workspace-app.tsx @@ -8,11 +8,8 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { isExpandableCard, isInitiallyExpandedCard, - isToolName, - isToolResultCard, summaryNumber, type HostContext, - type ToolName, type ToolResultCard, } from "./card-types.js"; import { getProviderLogo, renderIcon, toolIcons, type ToolIcon } from "./icons.js"; @@ -20,6 +17,11 @@ import { getFileChangePathDisplay, getPatchDisplayParts, } from "./patch-display.js"; +import { + decodeToolResult, + toolResultFromChatGptGlobals, + type ChatGptToolGlobals, +} from "./tool-result.js"; import "./workspace-app.css"; interface CardDisplay { @@ -51,6 +53,8 @@ let currentPayload: MountedPayload | null = null; let currentPayloadContainer: HTMLElement | null = null; let openWorkspaceInstructionKey: string | null = null; let showAvailableWorkspaceInstructions = false; +let pendingToolResult: CallToolResult | null = null; +let pendingReviewKey: string | null = null; const maybeAppRoot = document.querySelector("#app"); @@ -71,32 +75,11 @@ async function boot(): Promise { ); app.ontoolresult = (result) => { - const structuredContent = getStructuredContent>(result); - const metaCard = cardFromMeta(result); - const structured = metaCard - ? { ...structuredContent, ...metaCard } - : structuredContent; - const tool = toolNameFromMeta(result); - - if (!tool || !isToolResultCard(structured)) { - card = null; - expanded = false; - reviewFilesExpanded = false; - openWorkspaceInstructionKey = null; - showAvailableWorkspaceInstructions = false; - errorMessage = "No result card is available for this tool result."; - render(); + if (!connected) { + pendingToolResult = result; return; } - - const nextCard = { ...structured, tool }; - card = nextCard; - expanded = isInitiallyExpandedCard(nextCard); - reviewFilesExpanded = false; - openWorkspaceInstructionKey = null; - showAvailableWorkspaceInstructions = false; - errorMessage = null; - render(); + void applyToolResult(result); }; app.onhostcontextchanged = (ctx) => { @@ -111,6 +94,7 @@ async function boot(): Promise { }; app.onteardown = async () => { + window.removeEventListener("openai:set_globals", handleChatGptGlobalsChanged); unmountPayload(); return {}; }; @@ -121,15 +105,114 @@ async function boot(): Promise { if (initialContext) hostContext = initialContext; applyHostContext(); connected = true; + window.addEventListener("openai:set_globals", handleChatGptGlobalsChanged); } catch (connectError) { connectionError = connectError instanceof Error ? connectError.message : String(connectError); } + const initialResult = pendingToolResult ?? chatGptRestoredResult(); + pendingToolResult = null; + if (initialResult) { + await applyToolResult(initialResult); + } else { + render(); + } +} + +async function applyToolResult(result: CallToolResult): Promise { + const decoded = decodeToolResult(result); + if (decoded.kind === "card") { + setCard(decoded.card); + return; + } + if (decoded.kind === "invalid") { + clearCard("No result card is available for this tool result."); + return; + } + + const reviewKey = `${decoded.workspaceId}:${decoded.reviewRef}`; + pendingReviewKey = reviewKey; + card = null; + errorMessage = null; + resetCardInteractions(); + render(); + + try { + const restored = await reopenReview(decoded.workspaceId, decoded.reviewRef); + if (pendingReviewKey !== reviewKey) return; + + const restoredResult = decodeToolResult(restored); + if (restoredResult.kind !== "card" || restoredResult.card.tool !== "show_changes") { + throw new Error("The host returned an incomplete historical review."); + } + setCard(restoredResult.card); + } catch (reviewError) { + if (pendingReviewKey !== reviewKey) return; + clearCard( + reviewError instanceof Error + ? reviewError.message + : String(reviewError), + ); + } +} + +function setCard(nextCard: ToolResultCard): void { + pendingReviewKey = null; + card = nextCard; + expanded = isInitiallyExpandedCard(nextCard); + reviewFilesExpanded = false; + openWorkspaceInstructionKey = null; + showAvailableWorkspaceInstructions = false; + errorMessage = null; render(); } +function clearCard(message: string): void { + pendingReviewKey = null; + card = null; + errorMessage = message; + resetCardInteractions(); + render(); +} + +function resetCardInteractions(): void { + expanded = false; + reviewFilesExpanded = false; + openWorkspaceInstructionKey = null; + showAvailableWorkspaceInstructions = false; +} + +async function reopenReview( + workspaceId: string, + reviewRef: string, +): Promise { + if (!app) throw new Error("The app bridge is not connected."); + if (!app.getHostCapabilities()?.serverTools) { + throw new Error("This host cannot reload historical review details."); + } + + return app.callServerTool({ + name: "show_changes", + arguments: { workspaceId }, + _meta: { "devspace/reviewRef": reviewRef }, + }); +} + +function chatGptRestoredResult(): CallToolResult | undefined { + return toolResultFromChatGptGlobals(window.openai); +} + +function handleChatGptGlobalsChanged(event: Event): void { + if (!connected || card) return; + + const customEvent = event as CustomEvent<{ globals?: ChatGptToolGlobals }>; + const restored = toolResultFromChatGptGlobals(customEvent.detail?.globals) + ?? chatGptRestoredResult(); + if (restored) void applyToolResult(restored); +} + function applyHostContext(): void { if (hostContext?.theme) applyDocumentTheme(hostContext.theme); if (hostContext?.styles?.variables) { @@ -401,9 +484,14 @@ function toolCardClassName(display: CardDisplay): string { function cardDisplay(card: ToolResultCard): CardDisplay { if (card.tool === "open_workspace") { + const title = card.workspaceReused === true + ? "Reused workspace" + : card.workspaceReused === false + ? "Opened workspace" + : "Workspace"; return { icon: card.mode === "worktree" ? toolIcons.gitBranch : toolIcons.folderOpen, - title: `${card.workspaceReused ? "Reused" : "Opened"} workspace`, + title, label: card.root ?? card.path, tone: "workspace", }; @@ -837,22 +925,6 @@ function renderWorkspaceChips(chips: WorkspaceChip[]): HTMLElement { return list; } -function toolNameFromMeta(result: CallToolResult): ToolName | undefined { - const meta = result._meta as Record | undefined; - const tool = meta?.tool; - return isToolName(tool) ? tool : undefined; -} - -function cardFromMeta(result: CallToolResult): Partial | undefined { - const meta = result._meta as Record | undefined; - const metaCard = meta?.card; - return metaCard && typeof metaCard === "object" ? metaCard : undefined; -} - -function getStructuredContent(result: CallToolResult): T | undefined { - return result.structuredContent as T | undefined; -} - function element( tag: K, options: { From 60f7f2795fcfad71196124c3a4c55eaab8e684d5 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:50:35 +0530 Subject: [PATCH 3/5] feat(cli): inspect Git-backed reviews --- docs/chatgpt-coding-workflow.md | 10 +++++ docs/gotchas.md | 5 +++ src/cli-show-changes.test.ts | 72 +++++++++++++++++++++++++++++++++ src/cli.ts | 40 +++++++++++++++++- 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 src/cli-show-changes.test.ts diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index 1cb43e7a..f6826617 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -192,6 +192,16 @@ that changes files. It shows the combined changes for that turn and advances the review point automatically. Reusing a workspace does not change this workflow. +The model-facing result stays compact: DevSpace returns the workspace ID, a +Git-backed `reviewRef`, and the summary text. MCP Apps hosts receive the full +file list and patch in result metadata for immediate rendering. If a host later +restores only the structured result, the review card can reopen that exact +`reviewRef` from DevSpace's Git review history without advancing the current +review point. + +For local inspection, run `devspace show-changes `. Add `--json` to +include the parsed summary, file list, and patch. + ## Shell Use The shell tool is for commands that belong in a terminal: diff --git a/docs/gotchas.md b/docs/gotchas.md index 2f609355..5f628867 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -251,3 +251,8 @@ metadata and only show text results; `show_changes` remains available there. If both cards are missing in ChatGPT, confirm that `ui.enabled` is not `false` in `~/.devspace/config.jsonc` and reconnect the MCP server. + +Historical `show_changes` cards use the `reviewRef` in their structured result +to recover the exact Git-backed review when a host reloads the app without its +original result metadata. `open_workspace` can rebuild its card directly from +its structured result. diff --git a/src/cli-show-changes.test.ts b/src/cli-show-changes.test.ts new file mode 100644 index 00000000..6d4d9ef5 --- /dev/null +++ b/src/cli-show-changes.test.ts @@ -0,0 +1,72 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createRequire } from "node:module"; +import test from "node:test"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { promisify } from "node:util"; +import { createReviewCheckpointManager } from "./review-checkpoints.js"; +import { writeTestDevspaceConfig } from "./test-support/config.test.js"; + +const execFileAsync = promisify(execFile); +const require = createRequire(import.meta.url); +const tsxLoader = pathToFileURL(require.resolve("tsx")).href; +const cliPath = fileURLToPath(new URL("./cli.ts", import.meta.url)); + +test("show-changes prints a Git-backed historical review", async (t) => { + const root = await mkdtemp(join(tmpdir(), "devspace-cli-show-changes-")); + t.after(() => rm(root, { recursive: true, force: true })); + const project = join(root, "project"); + await execFileAsync("git", ["init", project]); + await git(project, ["config", "user.email", "devspace@example.com"]); + await git(project, ["config", "user.name", "DevSpace Test"]); + await writeFile(join(project, "README.md"), "hello\n"); + await git(project, ["add", "README.md"]); + await git(project, ["commit", "-m", "Initial commit"]); + + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_cli", root: project }); + await writeFile(join(project, "README.md"), "hello\nreview me\n"); + const review = await manager.reviewChanges({ workspaceId: "ws_cli", root: project }); + + const configDir = join(root, ".devspace"); + const env = writeTestDevspaceConfig(configDir, { + workspaces: { allowedRoots: [project] }, + storage: { stateDir: join(root, ".state") }, + }); + const cliArgs = ["--import", tsxLoader, cliPath, "show-changes", review.reviewRef]; + const plain = await execFileAsync("node", cliArgs, { + cwd: project, + env: { + ...process.env, + ...env, + DEVSPACE_WORKSPACE_ID: "", + DEVSPACE_WORKSPACE_ROOT: "", + }, + encoding: "utf8", + }); + assert.match(plain.stdout, /\+review me/); + + const json = await execFileAsync("node", [...cliArgs, "--json"], { + cwd: project, + env: { + ...process.env, + ...env, + DEVSPACE_WORKSPACE_ID: "", + DEVSPACE_WORKSPACE_ROOT: "", + }, + encoding: "utf8", + }); + const parsed = JSON.parse(json.stdout) as { + reviewRef: string; + patch: string; + }; + assert.equal(parsed.reviewRef, review.reviewRef); + assert.equal(parsed.patch, review.patch); +}); + +async function git(cwd: string, args: string[]): Promise { + await execFileAsync("git", args, { cwd }); +} diff --git a/src/cli.ts b/src/cli.ts index 92248151..b521556a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -50,9 +50,18 @@ import { writeDevspaceAuth, } from "./user-config.js"; import { expandHomePath } from "./roots.js"; +import { readReviewRef } from "./review-checkpoints.js"; import { shutdownHttpServer } from "./server-shutdown.js"; -type Command = "serve" | "init" | "doctor" | "config" | "agents" | "help" | "version"; +type Command = + | "serve" + | "init" + | "doctor" + | "config" + | "agents" + | "show-changes" + | "help" + | "version"; const require = createRequire(import.meta.url); const SUPPORTED_NODE_RANGE = ">=20.12 <27"; @@ -79,6 +88,9 @@ async function main(argv: string[]): Promise { case "agents": await runAgentsCommand(args); return; + case "show-changes": + await runShowChanges(args); + return; case "help": printHelp(); return; @@ -90,7 +102,13 @@ async function main(argv: string[]): Promise { function normalizeCommand(command: string | undefined): Command { if (!command || command === "serve" || command === "start") return "serve"; - if (command === "init" || command === "doctor" || command === "config" || command === "agents") return command; + if ( + command === "init" + || command === "doctor" + || command === "config" + || command === "agents" + || command === "show-changes" + ) return command; if (command === "help" || command === "--help" || command === "-h") return "help"; if (command === "version" || command === "--version" || command === "-v") return "version"; throw new Error(`Unknown command: ${command}`); @@ -391,6 +409,7 @@ function printHelp(): void { " devspace doctor Show config, runtime, and native dependency status", " devspace config get Print persisted config", " devspace config set publicBaseUrl ", + " devspace show-changes [--json]", " devspace agents ls List subagent sessions", " devspace agents run [--model ] [--effort ] ", " devspace agents continue [--model ] [--effort ] ", @@ -405,6 +424,23 @@ function printHelp(): void { ); } +async function runShowChanges(args: string[]): Promise { + const { args: commandArgs, json } = extractJsonOption(args); + const [reviewRef, ...extra] = commandArgs; + if (!reviewRef || extra.length > 0) { + throw new Error("Usage: devspace show-changes [--json]"); + } + + const config = loadConfig(); + const scope = resolveCliWorkspaceContext(config.allowedRoots); + const review = await readReviewRef(scope.workspaceRoot, reviewRef); + if (json) { + printJson(review); + return; + } + console.log(review.patch || review.result); +} + async function runAgentsCommand(args: string[]): Promise { const [subcommand, ...rest] = args; const { args: commandArgs, json } = extractJsonOption(rest); From ac1b6161b9c33742d724b9d52715d962c36641ca Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Wed, 26 Aug 2026 01:12:08 +0530 Subject: [PATCH 4/5] fix(review): harden historical review restore --- src/cli-show-changes.test.ts | 41 +++++++++++++++++++++++++++++----- src/review-checkpoints.test.ts | 6 ++++- src/review-checkpoints.ts | 39 ++++++++++++++++++++++++++++++++ src/ui/tool-result.test.ts | 18 +++++++++++++++ src/ui/tool-result.ts | 17 +++++++++++++- 5 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/cli-show-changes.test.ts b/src/cli-show-changes.test.ts index 6d4d9ef5..7ee900b1 100644 --- a/src/cli-show-changes.test.ts +++ b/src/cli-show-changes.test.ts @@ -1,21 +1,26 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; import test from "node:test"; -import { fileURLToPath, pathToFileURL } from "node:url"; +import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; import { writeTestDevspaceConfig } from "./test-support/config.test.js"; const execFileAsync = promisify(execFile); const require = createRequire(import.meta.url); -const tsxLoader = pathToFileURL(require.resolve("tsx")).href; -const cliPath = fileURLToPath(new URL("./cli.ts", import.meta.url)); +const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); +const cliPath = join(repoRoot, "dist", "cli.js"); +const tscPath = require.resolve("typescript/bin/tsc"); test("show-changes prints a Git-backed historical review", async (t) => { + await execFileAsync(process.execPath, [tscPath, "-p", join(repoRoot, "tsconfig.build.json")], { + cwd: repoRoot, + }); + const root = await mkdtemp(join(tmpdir(), "devspace-cli-show-changes-")); t.after(() => rm(root, { recursive: true, force: true })); const project = join(root, "project"); @@ -36,7 +41,7 @@ test("show-changes prints a Git-backed historical review", async (t) => { workspaces: { allowedRoots: [project] }, storage: { stateDir: join(root, ".state") }, }); - const cliArgs = ["--import", tsxLoader, cliPath, "show-changes", review.reviewRef]; + const cliArgs = [cliPath, "show-changes", review.reviewRef]; const plain = await execFileAsync("node", cliArgs, { cwd: project, env: { @@ -65,6 +70,30 @@ test("show-changes prints a Git-backed historical review", async (t) => { }; assert.equal(parsed.reviewRef, review.reviewRef); assert.equal(parsed.patch, review.patch); + + const head = (await execFileAsync("git", ["rev-parse", "HEAD"], { + cwd: project, + encoding: "utf8", + })).stdout.trim(); + await assert.rejects( + execFileAsync("node", [cliPath, "show-changes", head], { + cwd: project, + env: { + ...process.env, + ...env, + DEVSPACE_WORKSPACE_ID: "", + DEVSPACE_WORKSPACE_ROOT: "", + }, + encoding: "utf8", + }), + (error: unknown) => { + assert.match( + (error as { stderr?: string }).stderr ?? "", + /Unknown DevSpace review reference/, + ); + return true; + }, + ); }); async function git(cwd: string, args: string[]): Promise { diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index 6e9d0963..6707c6ea 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import test, { type TestContext } from "node:test"; import { promisify } from "node:util"; -import { createReviewCheckpointManager } from "./review-checkpoints.js"; +import { createReviewCheckpointManager, readReviewRef } from "./review-checkpoints.js"; const execFileAsync = promisify(execFile); @@ -112,6 +112,10 @@ test("review refs are scoped to the workspace review history", async (t) => { () => manager.reviewByRef({ workspaceId: "ws_scoped", root, reviewRef: head }), /Unknown review reference/, ); + await assert.rejects( + () => readReviewRef(root, head), + /Unknown DevSpace review reference/, + ); }); test("review checkpoints survive a manager restart", async (t) => { diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index 037ca8c5..6f687275 100644 --- a/src/review-checkpoints.ts +++ b/src/review-checkpoints.ts @@ -188,6 +188,9 @@ export async function readReviewRef(root: string, reviewRef: string): Promise { + const refs = (await git(gitRoot, [ + "for-each-ref", + "--format=%(refname)\t%(objectname)", + REVIEW_REF_PREFIX, + ])).stdout.trim(); + if (!refs) return false; + + const histories = new Map(); + for (const line of refs.split("\n")) { + const [ref, commit] = line.split("\t"); + if (!ref || !commit) continue; + + const match = ref.match(/^refs\/devspace\/review\/(.+)\/(open|baseline)$/); + if (!match) continue; + const [, workspace, kind] = match; + if (!workspace || !kind) continue; + + const history = histories.get(workspace) ?? {}; + history[kind as "open" | "baseline"] = commit; + histories.set(workspace, history); + } + + const memberships = await Promise.all( + [...histories.values()].map(async ({ open, baseline }) => { + if (!open || !baseline || reviewCommit === open) return false; + const [isAfterOpen, isBeforeBaseline] = await Promise.all([ + isAncestor(gitRoot, open, reviewCommit), + isAncestor(gitRoot, reviewCommit, baseline), + ]); + return isAfterOpen && isBeforeBaseline; + }), + ); + return memberships.some(Boolean); +} + function isReviewRef(value: string): boolean { return /^[0-9a-f]{40,64}$/.test(value); } diff --git a/src/ui/tool-result.test.ts b/src/ui/tool-result.test.ts index 4687e028..b7c5315d 100644 --- a/src/ui/tool-result.test.ts +++ b/src/ui/tool-result.test.ts @@ -70,6 +70,24 @@ test("review structured content becomes a reload reference when metadata is miss }); }); +test("incomplete review metadata falls back to the durable review reference", () => { + const decoded = decodeToolResult({ + content: [], + structuredContent: { + workspaceId: "ws_1", + reviewRef: "e".repeat(40), + result: "Changed 1 file (+1 -0).", + }, + _meta: { card: {} }, + }); + + assert.deepEqual(decoded, { + kind: "review-reference", + workspaceId: "ws_1", + reviewRef: "e".repeat(40), + }); +}); + test("older review results can reload from their structured patch", () => { const decoded = decodeToolResult({ content: [], diff --git a/src/ui/tool-result.ts b/src/ui/tool-result.ts index d029eaf1..efd1bdb4 100644 --- a/src/ui/tool-result.ts +++ b/src/ui/tool-result.ts @@ -19,7 +19,7 @@ export function decodeToolResult(result: CallToolResult): DecodedToolResult { const workspaceId = stringField(structured.workspaceId); const reviewRef = stringField(structured.reviewRef); if (workspaceId && reviewRef) { - if (metaCard) { + if (isCompleteReviewCard(metaCard)) { return { kind: "card", card: { @@ -73,6 +73,21 @@ export function decodeToolResult(result: CallToolResult): DecodedToolResult { return { kind: "invalid" }; } +function isCompleteReviewCard( + card: Partial | undefined, +): card is Partial & { + files: NonNullable; + payload: { patch: string }; + summary: Record; +} { + if (!card || !Array.isArray(card.files) || typeof card.payload?.patch !== "string") { + return false; + } + return numberField(card.summary?.files) !== undefined + && numberField(card.summary?.additions) !== undefined + && numberField(card.summary?.removals) !== undefined; +} + export function toolResultFromChatGptGlobals( globals: ChatGptToolGlobals | undefined, ): CallToolResult | undefined { From 809f1a4492de3cd3218981fc6f844d48e36d31c3 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Wed, 26 Aug 2026 01:19:10 +0530 Subject: [PATCH 5/5] test(cli): bind review smoke to package entrypoint --- src/cli-show-changes.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cli-show-changes.test.ts b/src/cli-show-changes.test.ts index 7ee900b1..40763b86 100644 --- a/src/cli-show-changes.test.ts +++ b/src/cli-show-changes.test.ts @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; +import { readFileSync } from "node:fs"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; @@ -12,8 +13,14 @@ import { writeTestDevspaceConfig } from "./test-support/config.test.js"; const execFileAsync = promisify(execFile); const require = createRequire(import.meta.url); -const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); -const cliPath = join(repoRoot, "dist", "cli.js"); +const packageJsonPath = fileURLToPath(new URL("../package.json", import.meta.url)); +const repoRoot = dirname(packageJsonPath); +const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")) as { + bin: { devspace: string }; +}; +// This verifies the compiled entrypoint declared for the installed `devspace` +// command. npm's package-install shim itself is outside this focused test. +const cliPath = join(repoRoot, packageJson.bin.devspace); const tscPath = require.resolve("typescript/bin/tsc"); test("show-changes prints a Git-backed historical review", async (t) => {