From d119700bab7117503ccc5901746ea362f263d65b Mon Sep 17 00:00:00 2001 From: narghev Date: Thu, 7 May 2026 23:48:31 +0400 Subject: [PATCH] Update the skill to handle natural language diffs, change the logic of the server to read the diff from a temp file and not staged/unstaged git invocations --- .claude/skills/askdiff-dev/SKILL.md | 150 +++++++++++++++++- .claude/skills/askdiff/SKILL.md | 142 ++++++++++++++++- .gitignore | 2 + packages/cli/src/index.ts | 16 ++ packages/protocol/src/schemas.test.ts | 20 +++ packages/protocol/src/schemas.ts | 4 + packages/server/src/index.ts | 32 ++-- packages/server/src/main.ts | 13 ++ packages/server/src/util/constants.ts | 3 - packages/server/src/util/diff.test.ts | 52 +++++- packages/server/src/util/diff.ts | 32 ++-- packages/server/src/util/git.ts | 87 ---------- packages/server/src/util/helpers.ts | 4 - packages/ui-browser/src/components/TopBar.tsx | 11 ++ packages/ui-browser/src/lib/store.ts | 10 +- packages/ui-browser/src/lib/ws.ts | 2 +- 16 files changed, 446 insertions(+), 134 deletions(-) delete mode 100644 packages/server/src/util/git.ts delete mode 100644 packages/server/src/util/helpers.ts diff --git a/.claude/skills/askdiff-dev/SKILL.md b/.claude/skills/askdiff-dev/SKILL.md index c4f656f..3a7d21e 100644 --- a/.claude/skills/askdiff-dev/SKILL.md +++ b/.claude/skills/askdiff-dev/SKILL.md @@ -5,21 +5,148 @@ user-invocable: true allowed-tools: Bash --- -Local-development variant of `/askdiff`. Starts the WS server **and** -the browser UI's Vite dev server (with HMR). Vite is configured to +Local-development variant of `/askdiff`. Starts the WS server **and** the +browser UI's Vite dev server (with HMR), and exercises the in-repo +TypeScript instead of the published npm package. Vite is configured to proxy `/ws` to the WS server, so the UI uses the same same-origin `new WebSocket('ws://host/ws')` URL in dev as in prod. The `ASKDIFF_DEV_WS_TARGET` env var tells Vite which port to forward to. -Use this when editing `packages/ui-browser` and you want changes to -reload instantly instead of rebuilding the npm package. +Use this when editing `packages/server` or `packages/ui-browser` and you +want changes to reload instantly instead of rebuilding/republishing. + +> **Keep Step 1–3 in sync with `.claude/skills/askdiff/SKILL.md`.** The +> diff-resolution flow (interpret → git → temp file → label) must behave +> identically in both skills; only Step 4 (launch) differs. If you change +> the table or the bash blocks below, change them in the user-facing +> `askdiff` skill too. + +## Step 1 — figure out which diff the user wants + +Look at the message that invoked this skill. Anything after `/askdiff-dev` +is the user's diff description (may be empty). + +| User said | git command | Suggested label | +|---|---|---| +| `/askdiff-dev` (no args) | working tree — see Step 2 | `Working tree` | +| `/askdiff-dev last commit` | `git diff HEAD~1 HEAD` | `HEAD~1..HEAD` | +| `/askdiff-dev last 3 commits` | `git diff HEAD~3 HEAD` | `HEAD~3..HEAD` | +| `/askdiff-dev the 5th latest commit` | `git diff HEAD~5 HEAD~4` | `HEAD~5..HEAD~4` | +| `/askdiff-dev current branch against feature/test` | `git diff feature/test...HEAD` (three-dot, PR semantics) | `feature/test…HEAD` | +| `/askdiff-dev main vs my branch` | `git diff main...HEAD` | `main…HEAD` | +| `/askdiff-dev abc123 vs def456` | `git diff abc123 def456` | `abc123..def456` | +| `/askdiff-dev staged` | `git diff --cached` | `staged` | + +Defaults when the user is ambiguous: +- "branch X against branch Y" / "X vs Y" between two named refs ⇒ three-dot + (`git diff X...Y`) — matches how GitHub renders PRs. +- Two arbitrary commits ⇒ two-dot (`git diff A B`). +- "Nth latest commit" ⇒ that single commit's changes + (`git diff HEAD~N HEAD~(N-1)`). + +### When the description is vague + +If the description doesn't fit the table — e.g. "the commit where I added +the favicon", "the last commit by my coworker David", "where we ripped out +the old auth code", "the commit that broke CI last week" — pin down a +single commit with the ladder below, then diff `^..` (same shape +as the "Nth latest commit" pattern). Try in order until exactly one commit +matches; if several match, pick the most recent and **tell the user which +one you chose**; if none match, stop and ask — do not guess. + +1. **Author.** "by ", "'s last", "by my coworker": + ```bash + git log --author= -i -1 --format='%H %an %s' + ``` + +2. **Commit message.** "the migration commit", "where I bumped deps": + ```bash + git log --grep= -i -1 --format='%H %s' + ``` + +3. **Diff content.** "where I added/removed/touched ". `-S` matches + when a string's count changed in any file; `-G` is a regex over the + diff text: + ```bash + git log -S"" -1 --format='%H %s' + git log -G"" -1 --format='%H %s' + ``` + +4. **File history.** When you can identify the file but not the commit + (e.g. "where the homepage was added" — search the working tree for a + plausible path first, then ask git): + ```bash + git ls-files | grep -i # find candidate path + git log --follow -1 --format='%H %s' -- # most recent touch + git log --follow --diff-filter=A -1 --format='%H %s' -- # commit that introduced it + ``` + +Once a SHA is in hand, build the label as `: ` +(e.g. `d0b332b: add favicon`) and use `git diff ^ ` as the +diff command. If the user's count and description disagree (e.g. "my 3rd +previous commit, where I added a favicon" but the favicon is at HEAD~2), +trust the description over the count and **flag the off-by-one to the +user** so they know what you picked. + +**Validate every ref first.** Run `git rev-parse --verify ^{commit}` for +each ref the user named directly. If any fails, stop and tell the user +which ref didn't resolve — do not launch the server. (Refs returned by the +search ladder are already validated by virtue of `git log` finding them.) + +## Step 2 — write the diff to a temp file + +```bash +diff_file=$(mktemp /tmp/askdiff-diff.XXXXXX) +``` + +(macOS `mktemp` only substitutes trailing X's, so the template can't have +a `.diff` suffix. The server doesn't care about the extension.) + +**Working tree (no description).** Untracked files don't appear in +`git diff HEAD`, so we union them in via `--no-index`: + +```bash +{ + git -C "$project_cwd" diff HEAD --no-color + git -C "$project_cwd" ls-files --others --exclude-standard -z \ + | while IFS= read -r -d '' f; do + git -C "$project_cwd" diff --no-index --no-color -- /dev/null "$f" || true + done +} > "$diff_file" +``` + +(In an empty repo with no HEAD, replace `HEAD` with the empty-tree SHA +`4b825dc642cb6eb9a060e54bf8d69288fbee4904`.) + +**Description path.** Just run the resolved command: -Run this as a single Bash command so discovered values survive into the -launch: +```bash +git -C "$project_cwd" diff --no-color > "$diff_file" +``` + +For the description path, if the resulting file is empty, **stop** — tell the +user the requested diff is empty and don't launch. The working-tree path +*can* legitimately be empty (clean tree); launch anyway and the UI will +show "No changes." + +## Step 3 — pick a short label + +Use the "Suggested label" column above. For the working-tree case, use +`Working tree`. Keep it under ~40 chars. This becomes `ASKDIFF_DIFF_LABEL`. + +## Step 4 — launch (in-repo) + +Run as a single Bash command so the discovered values survive into the +launch. Substitute `EXTRA_DIFF_FILE` and `EXTRA_DIFF_LABEL` literally with +the values from Step 2/3. ``` set +e +# Filled in by Step 2/3. +EXTRA_DIFF_FILE="" +EXTRA_DIFF_LABEL="" + # 1. Free port for the WS server (default 7837, bump until free). port=7837 while lsof -iTCP:$port -sTCP:LISTEN -t >/dev/null 2>&1; do @@ -36,8 +163,14 @@ if [ -f "$session_file" ]; then [ -n "$manifest_cwd" ] && project_cwd="$manifest_cwd" fi -# 3. Start the WS server. -cd "$project_cwd" && PORT=$port ASKDIFF_SESSION_ID="$session_id" ASKDIFF_PROJECT_CWD="$project_cwd" nohup pnpm --filter @askdiff/server exec tsx src/main.ts > /tmp/askdiff.log 2>&1 & +# 3. Start the WS server (in-repo via tsx). +cd "$project_cwd" \ + && PORT=$port \ + ASKDIFF_SESSION_ID="$session_id" \ + ASKDIFF_PROJECT_CWD="$project_cwd" \ + ASKDIFF_DIFF_FILE="$EXTRA_DIFF_FILE" \ + ASKDIFF_DIFF_LABEL="$EXTRA_DIFF_LABEL" \ + nohup pnpm --filter @askdiff/server exec tsx src/main.ts > /tmp/askdiff.log 2>&1 & disown sleep 1.5 head -5 /tmp/askdiff.log @@ -81,6 +214,7 @@ echo "UI: $ui_url" Then tell the user: - the WS server port (visible in the `listening on ws://...` line) - the resolved Claude session ID (from the `claude session:` line) +- the diff label (always set) - the WS log file: `/tmp/askdiff.log` - the Vite log file: `/tmp/askdiff-ui.log` - the UI URL (last echoed line) — already opened in their default browser diff --git a/.claude/skills/askdiff/SKILL.md b/.claude/skills/askdiff/SKILL.md index c0b9037..36cb768 100644 --- a/.claude/skills/askdiff/SKILL.md +++ b/.claude/skills/askdiff/SKILL.md @@ -5,12 +5,137 @@ user-invocable: true allowed-tools: Bash --- -Start the published `askdiff` CLI in the background. Before launching, -check whether a newer version is available on npm. If so, halt with -the line `UPDATE_AVAILABLE: pinned=X latest=Y` so we can ask the user -whether to upgrade or proceed on the pinned version. +Compute the unified diff the user wants to review, write it to a temp file, +then launch the published `askdiff` CLI in the background pointing at that +file. The server is intentionally git-illiterate — it only reads the file +you produce and serves it to the browser. Skipping the file is a startup +error. -Run this as a single Bash command: +> **Keep Step 1–3 in sync with `.claude/skills/askdiff-dev/SKILL.md`.** The +> diff-resolution flow (interpret → git → temp file → label) must behave +> identically in both skills; only Step 4 (launch) differs. If you change +> the table or the bash blocks below, change them in the dev skill too. + +## Step 1 — figure out which diff the user wants + +Look at the message that invoked this skill. Anything after `/askdiff` is the +user's diff description (may be empty). + +| User said | git command | Suggested label | +|---|---|---| +| `/askdiff` (no args) | working tree — see Step 2 | `Working tree` | +| `/askdiff last commit` | `git diff HEAD~1 HEAD` | `HEAD~1..HEAD` | +| `/askdiff last 3 commits` | `git diff HEAD~3 HEAD` | `HEAD~3..HEAD` | +| `/askdiff the 5th latest commit` | `git diff HEAD~5 HEAD~4` | `HEAD~5..HEAD~4` | +| `/askdiff current branch against feature/test` | `git diff feature/test...HEAD` (three-dot, PR semantics) | `feature/test…HEAD` | +| `/askdiff main vs my branch` | `git diff main...HEAD` | `main…HEAD` | +| `/askdiff abc123 vs def456` | `git diff abc123 def456` | `abc123..def456` | +| `/askdiff staged` | `git diff --cached` | `staged` | + +Defaults when the user is ambiguous: +- "branch X against branch Y" / "X vs Y" between two named refs ⇒ three-dot + (`git diff X...Y`) — matches how GitHub renders PRs. +- Two arbitrary commits ⇒ two-dot (`git diff A B`). +- "Nth latest commit" ⇒ that single commit's changes + (`git diff HEAD~N HEAD~(N-1)`). + +### When the description is vague + +If the description doesn't fit the table — e.g. "the commit where I added +the favicon", "the last commit by my coworker David", "where we ripped out +the old auth code", "the commit that broke CI last week" — pin down a +single commit with the ladder below, then diff `^..` (same shape +as the "Nth latest commit" pattern). Try in order until exactly one commit +matches; if several match, pick the most recent and **tell the user which +one you chose**; if none match, stop and ask — do not guess. + +1. **Author.** "by ", "'s last", "by my coworker": + ```bash + git log --author= -i -1 --format='%H %an %s' + ``` + +2. **Commit message.** "the migration commit", "where I bumped deps": + ```bash + git log --grep= -i -1 --format='%H %s' + ``` + +3. **Diff content.** "where I added/removed/touched ". `-S` matches + when a string's count changed in any file; `-G` is a regex over the + diff text: + ```bash + git log -S"" -1 --format='%H %s' + git log -G"" -1 --format='%H %s' + ``` + +4. **File history.** When you can identify the file but not the commit + (e.g. "where the homepage was added" — search the working tree for a + plausible path first, then ask git): + ```bash + git ls-files | grep -i # find candidate path + git log --follow -1 --format='%H %s' -- # most recent touch + git log --follow --diff-filter=A -1 --format='%H %s' -- # commit that introduced it + ``` + +Once a SHA is in hand, build the label as `: ` +(e.g. `d0b332b: add favicon`) and use `git diff ^ ` as the +diff command. If the user's count and description disagree (e.g. "my 3rd +previous commit, where I added a favicon" but the favicon is at HEAD~2), +trust the description over the count and **flag the off-by-one to the +user** so they know what you picked. + +**Validate every ref first.** Run `git rev-parse --verify ^{commit}` for +each ref the user named directly. If any fails, stop and tell the user +which ref didn't resolve — do not launch the server. (Refs returned by the +search ladder are already validated by virtue of `git log` finding them.) + +## Step 2 — write the diff to a temp file + +```bash +diff_file=$(mktemp /tmp/askdiff-diff.XXXXXX) +``` + +(macOS `mktemp` only substitutes trailing X's, so the template can't have +a `.diff` suffix. The server doesn't care about the extension.) + +**Working tree (no description).** Untracked files don't appear in +`git diff HEAD`, so we union them in via `--no-index`: + +```bash +{ + git -C "$project_cwd" diff HEAD --no-color + git -C "$project_cwd" ls-files --others --exclude-standard -z \ + | while IFS= read -r -d '' f; do + git -C "$project_cwd" diff --no-index --no-color -- /dev/null "$f" || true + done +} > "$diff_file" +``` + +(In an empty repo with no HEAD, replace `HEAD` with the empty-tree SHA +`4b825dc642cb6eb9a060e54bf8d69288fbee4904`.) + +**Description path.** Just run the resolved command: + +```bash +git -C "$project_cwd" diff --no-color > "$diff_file" +``` + +`` is whatever you resolved in Step 1 (e.g. `HEAD~1 HEAD` or +`feature/test...HEAD` or `--cached`). + +For the description path, if the resulting file is empty, **stop** — tell the +user the requested diff is empty and don't launch. (`/askdiff HEAD vs HEAD` +is the canonical empty case.) The working-tree path *can* legitimately be +empty (clean tree); launch anyway and the UI will show "No changes." + +## Step 3 — pick a short label + +Use the "Suggested label" column above. For the working-tree case, use +`Working tree`. Keep it under ~40 chars. This becomes `ASKDIFF_DIFF_LABEL`. + +## Step 4 — launch + +Run as a single Bash command. Substitute `EXTRA_DIFF_FILE` and +`EXTRA_DIFF_LABEL` literally with the values from Step 2/3. ``` set +e @@ -20,6 +145,10 @@ set +e # this repo always pulls the newest published version). ASKDIFF_VERSION="latest" +# Filled in by Step 2/3. +EXTRA_DIFF_FILE="" +EXTRA_DIFF_LABEL="" + # 1. Resolve parent Claude Code session + cwd. session_file="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/sessions/$PPID.json" session_id="" @@ -46,6 +175,8 @@ fi cd "$project_cwd" \ && ASKDIFF_SESSION_ID="$session_id" \ ASKDIFF_PROJECT_CWD="$project_cwd" \ + ASKDIFF_DIFF_FILE="$EXTRA_DIFF_FILE" \ + ASKDIFF_DIFF_LABEL="$EXTRA_DIFF_LABEL" \ nohup npx -y askdiff@"$ASKDIFF_VERSION" --no-open > /tmp/askdiff.log 2>&1 & disown @@ -83,6 +214,7 @@ echo "UI: $url" launch already happened. Tell the user: - the WS server URL (the `listening on http://...` line) - the resolved Claude session ID (the `claude session:` line) +- the diff label (always set) - the log file: `/tmp/askdiff.log` - the UI URL (last echoed line) — already opened in their default browser diff --git a/.gitignore b/.gitignore index 9f9a89c..ecf7ce8 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,5 @@ coverage/ # Local skill management — not part of askdiff .agents/ skills-lock.json + +*.mp4 diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 43a2149..35262af 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -78,6 +78,8 @@ async function runServer(opts: RunOptions): Promise { await startServer({ cwd: resolved.cwd, sessionId: resolved.session, + diffFile: resolved.diffFile, + ...(resolved.diffLabel !== undefined ? { diffLabel: resolved.diffLabel } : {}), httpServer, onListening: (resolvedPort) => { const url = `http://localhost:${String(resolvedPort)}/`; @@ -87,6 +89,8 @@ async function runServer(opts: RunOptions): Promise { console.log( ` claude session: ${resolved.session ?? "(none — set ASKDIFF_SESSION_ID or use --session)"}`, ); + console.log(` diff file: ${resolved.diffFile}`); + if (resolved.diffLabel) console.log(` diff label: ${resolved.diffLabel}`); console.log(` websocket: ws://localhost:${String(resolvedPort)}${WS_PATH}`); }, }); @@ -142,6 +146,8 @@ interface ResolvedOptions { open: boolean; session: string | null; cwd: string; + diffFile: string; + diffLabel?: string; } async function resolveOptions(opts: RunOptions): Promise { @@ -161,12 +167,22 @@ async function resolveOptions(opts: RunOptions): Promise { const port = opts.port ?? (Number(process.env["PORT"]) || DEFAULT_PORT); + const diffFile = process.env["ASKDIFF_DIFF_FILE"]; + if (!diffFile) { + throw new Error( + "ASKDIFF_DIFF_FILE is required. The askdiff skill must compute the diff and pass the path before launching the server.", + ); + } + const diffLabel = process.env["ASKDIFF_DIFF_LABEL"]; + return { port, host: opts.host, open: opts.open, session, cwd, + diffFile, + ...(diffLabel ? { diffLabel } : {}), }; } diff --git a/packages/protocol/src/schemas.test.ts b/packages/protocol/src/schemas.test.ts index 1b1f815..bf614c0 100644 --- a/packages/protocol/src/schemas.test.ts +++ b/packages/protocol/src/schemas.test.ts @@ -217,6 +217,26 @@ describe("DiffMessageSchema", () => { }); expect(result.success).toBe(true); }); + + it("accepts an optional label", () => { + const result = DiffMessageSchema.safeParse({ + type: "diff", + raw: "", + files: [], + label: "HEAD~1..HEAD", + }); + expect(result.success).toBe(true); + }); + + it("rejects a non-string label", () => { + const result = DiffMessageSchema.safeParse({ + type: "diff", + raw: "", + files: [], + label: 42, + }); + expect(result.success).toBe(false); + }); }); describe("ChunkMessageSchema", () => { diff --git a/packages/protocol/src/schemas.ts b/packages/protocol/src/schemas.ts index 89873ed..a4553ee 100644 --- a/packages/protocol/src/schemas.ts +++ b/packages/protocol/src/schemas.ts @@ -61,6 +61,10 @@ export const DiffMessageSchema = z.object({ type: z.literal("diff"), raw: z.string(), files: z.array(DiffFileSchema), + // Short human description of *what* this diff shows ("HEAD~1..HEAD", + // "main…feature/x", "Working tree"). The skill sets it via the + // ASKDIFF_DIFF_LABEL env var; absent for legacy clients. + label: z.string().optional(), }); export const ChunkMessageSchema = z.object({ diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 0b89186..b340b44 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -7,8 +7,7 @@ import { import type { Socket } from "node:net"; import { WebSocketServer, type WebSocket } from "ws"; import { ClaudeCliError, streamAnswer } from "./claude"; -import { getDiff } from "./util/diff"; -import { GitError } from "./util/git"; +import { DiffError, getDiff } from "./util/diff"; import { PROTOCOL_VERSION, parseClientMessage, type AskMessage } from "@askdiff/protocol"; import { DEFAULT_HOST, DEFAULT_IDLE_SHUTDOWN_MS, PROJECT_NAME } from "./util/constants"; import { isValidSessionId, sessionExists } from "./util/session"; @@ -20,11 +19,19 @@ export interface ServerState { cwd: string; claudeSessionId: string | null; clients: Set; + diffFile: string; + diffLabel?: string; } export interface StartServerOptions { cwd: string; sessionId: string | null; + // Path to a unified diff file the skill produced (e.g. via `git diff + // HEAD~1 HEAD > $diffFile`). The server treats this as the single source + // of truth — it never invokes git itself. + diffFile: string; + // Short human description of the diff for the UI badge. + diffLabel?: string; // Standalone mode: pass `port` (and optionally `host`) and the server // creates its own HTTP listener. port?: number; @@ -42,13 +49,18 @@ export interface ServerHandle { close: () => Promise; } -async function sendDiff(ws: WebSocket, cwd: string): Promise { +async function sendDiff(ws: WebSocket, state: ServerState): Promise { try { - const { raw, files } = await getDiff(cwd); - send(ws, { type: "diff", raw, files }); + const { raw, files } = await getDiff(state.diffFile); + send(ws, { + type: "diff", + raw, + files, + ...(state.diffLabel !== undefined ? { label: state.diffLabel } : {}), + }); } catch (err) { const message = - err instanceof GitError ? err.message : `unexpected diff error: ${String(err)}`; + err instanceof DiffError ? err.message : `unexpected diff error: ${String(err)}`; send(ws, { type: "error", message }); } } @@ -123,7 +135,7 @@ async function handleSetSession( function errorMessage(err: unknown): string { if (err instanceof ClaudeCliError) return err.message; - if (err instanceof GitError) return err.message; + if (err instanceof DiffError) return err.message; if (err instanceof Error) return err.message; return String(err); } @@ -135,6 +147,8 @@ export async function startServer(opts: StartServerOptions): Promise { const text = Buffer.isBuffer(data) @@ -220,7 +234,7 @@ export async function startServer(opts: StartServerOptions): Promise { const port = Number.parseInt(process.env["PORT"] ?? "0", 10) || DEFAULT_PORT; const idleShutdownMs = readIdleShutdownMs(); + const diffFile = process.env["ASKDIFF_DIFF_FILE"]; + if (!diffFile) { + console.error( + "fatal: ASKDIFF_DIFF_FILE is required. The askdiff skill must compute the diff and pass the path before launching the server.", + ); + process.exit(1); + } + const diffLabel = process.env["ASKDIFF_DIFF_LABEL"]; + const initialSession = await resolveInitialSessionId(cwd); await startServer({ cwd, sessionId: initialSession, + diffFile, + ...(diffLabel ? { diffLabel } : {}), port, idleShutdownMs, onListening: (resolvedPort) => { @@ -37,6 +48,8 @@ async function main(): Promise { console.log( ` claude session: ${initialSession ?? "(none — send set_session before asking)"}`, ); + console.log(` diff file: ${diffFile}`); + if (diffLabel) console.log(` diff label: ${diffLabel}`); if (idleShutdownMs > 0) { console.log( ` idle shutdown: ${String(Math.round(idleShutdownMs / 1000))}s after last client`, diff --git a/packages/server/src/util/constants.ts b/packages/server/src/util/constants.ts index a98872d..9dd9fbd 100644 --- a/packages/server/src/util/constants.ts +++ b/packages/server/src/util/constants.ts @@ -5,8 +5,5 @@ export const DEFAULT_PORT = 7837; export const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; -export const MAX_DIFF_BUFFER = 64 * 1024 * 1024; -export const EMPTY_TREE_SHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; - export const DEFAULT_HOST = "127.0.0.1"; export const DEFAULT_IDLE_SHUTDOWN_MS = 5 * 60_000; diff --git a/packages/server/src/util/diff.test.ts b/packages/server/src/util/diff.test.ts index 1735adc..69ad805 100644 --- a/packages/server/src/util/diff.test.ts +++ b/packages/server/src/util/diff.test.ts @@ -1,5 +1,8 @@ -import { describe, expect, it } from "@jest/globals"; -import { parseUnifiedDiff } from "./diff"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "@jest/globals"; +import { DiffError, getDiff, parseUnifiedDiff } from "./diff"; describe("parseUnifiedDiff", () => { it("returns an empty array for empty input", () => { @@ -205,3 +208,48 @@ describe("parseUnifiedDiff", () => { expect(files[0]?.hunks).toHaveLength(1); }); }); + +describe("getDiff", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "askdiff-test-")); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("reads a unified diff from disk and parses it", async () => { + const path = join(dir, "good.diff"); + const raw = [ + "diff --git a/src/x.ts b/src/x.ts", + "--- a/src/x.ts", + "+++ b/src/x.ts", + "@@ -1,1 +1,1 @@", + "-old", + "+new", + "", + ].join("\n"); + writeFileSync(path, raw, "utf8"); + + const result = await getDiff(path); + expect(result.raw).toBe(raw); + expect(result.files).toHaveLength(1); + expect(result.files[0]?.path).toBe("src/x.ts"); + }); + + it("returns an empty result for an empty file", async () => { + const path = join(dir, "empty.diff"); + writeFileSync(path, "", "utf8"); + + const result = await getDiff(path); + expect(result.raw).toBe(""); + expect(result.files).toEqual([]); + }); + + it("throws DiffError when the file is missing", async () => { + const path = join(dir, "does-not-exist.diff"); + await expect(getDiff(path)).rejects.toBeInstanceOf(DiffError); + }); +}); diff --git a/packages/server/src/util/diff.ts b/packages/server/src/util/diff.ts index e7f3035..50ad714 100644 --- a/packages/server/src/util/diff.ts +++ b/packages/server/src/util/diff.ts @@ -1,21 +1,31 @@ +import { readFile } from "node:fs/promises"; import type { DiffFile, DiffHunk } from "@askdiff/protocol"; -import { EMPTY_TREE_SHA } from "./constants"; -import { hasHead, getTrackedDiff, getUntrackedDiff } from "./git"; + +// Failure to read or parse the diff file. The skill is the only producer of +// these files; surfacing this distinct class lets the WS layer attach a +// stable error message when something goes wrong on read. +export class DiffError extends Error { + constructor(message: string, override readonly cause?: unknown) { + super(message); + this.name = "DiffError"; + } +} export interface DiffResult { raw: string; files: DiffFile[]; } -export const getDiff = async (cwd: string): Promise => { - const base = (await hasHead(cwd)) ? "HEAD" : EMPTY_TREE_SHA; - const [tracked, untracked] = await Promise.all([ - getTrackedDiff(cwd, base), - getUntrackedDiff(cwd), - ]); - - const raw = [tracked, untracked].filter((s) => s.length > 0).join(""); - +export const getDiff = async (path: string): Promise => { + let raw: string; + try { + raw = await readFile(path, "utf8"); + } catch (err) { + throw new DiffError( + `failed to read diff file ${path}: ${err instanceof Error ? err.message : String(err)}`, + err, + ); + } return { raw, files: parseUnifiedDiff(raw) }; }; diff --git a/packages/server/src/util/git.ts b/packages/server/src/util/git.ts deleted file mode 100644 index bc95354..0000000 --- a/packages/server/src/util/git.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { MAX_DIFF_BUFFER } from "./constants"; -import { execFileAsync } from "./helpers"; - -export class GitError extends Error { - constructor(message: string, override readonly cause?: unknown) { - super(message); - this.name = "GitError"; - } -} - -interface ExecFileErrorLike extends Error { - code?: number; - stdout?: string; - stderr?: string; -} - -export const hasHead = async (cwd: string): Promise => { - try { - await execFileAsync("git", ["rev-parse", "--verify", "HEAD"], { cwd }); - return true; - } catch { - return false; - } -} - -export const getTrackedDiff = async (cwd: string, base: string): Promise => { - try { - const { stdout } = await execFileAsync( - "git", - ["diff", base, "--no-color"], - { cwd, maxBuffer: MAX_DIFF_BUFFER }, - ); - return stdout; - } catch (err) { - throw new GitError( - `git diff failed: ${err instanceof Error ? err.message : String(err)}`, - err, - ); - } -} - -const diffNoIndex = async (cwd: string, path: string): Promise => { - try { - const { stdout } = await execFileAsync( - "git", - ["diff", "--no-index", "--no-color", "--", "/dev/null", path], - { cwd, maxBuffer: MAX_DIFF_BUFFER }, - ); - - return stdout; - } catch (err) { - const e = err as ExecFileErrorLike; - if (e.code === 1 && typeof e.stdout === "string") - return e.stdout; - - return ""; - } -} - -export const getUntrackedDiff = async (cwd: string): Promise => { - let listing: string; - - try { - const { stdout } = await execFileAsync( - "git", - ["ls-files", "--others", "--exclude-standard", "-z"], - { cwd, maxBuffer: MAX_DIFF_BUFFER }, - ); - listing = stdout; - } catch (err) { - throw new GitError( - `git ls-files failed: ${err instanceof Error ? err.message : String(err)}`, - err, - ); - } - - const paths = listing.split("\0").filter((p) => p.length > 0); - if (paths.length === 0) return ""; - - const parts: string[] = []; - for (const path of paths) { - const piece = await diffNoIndex(cwd, path); - if (piece.length > 0) parts.push(piece); - } - - return parts.join(""); -} diff --git a/packages/server/src/util/helpers.ts b/packages/server/src/util/helpers.ts deleted file mode 100644 index 15885a8..0000000 --- a/packages/server/src/util/helpers.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; - -export const execFileAsync = promisify(execFile); diff --git a/packages/ui-browser/src/components/TopBar.tsx b/packages/ui-browser/src/components/TopBar.tsx index 91d88ee..8a22265 100644 --- a/packages/ui-browser/src/components/TopBar.tsx +++ b/packages/ui-browser/src/components/TopBar.tsx @@ -22,6 +22,12 @@ const ConnectionDot = () => { export const TopBar = () => { const project = useStore((s) => s.project); const protocol = useStore((s) => s.protocol); + const diffLabel = useStore((s) => s.diff?.label); + + // Always advertise *what* the user is reviewing — when the skill didn't + // pre-compute a diff, the server falls back to the working tree, so that + // becomes the implicit label. + const label = diffLabel ?? (protocol ? "Working tree" : undefined); return (
@@ -32,6 +38,11 @@ export const TopBar = () => { {protocol} )} + {label && ( + + {label} + + )} {project && (
diff --git a/packages/ui-browser/src/lib/store.ts b/packages/ui-browser/src/lib/store.ts index 9b3fc84..fbf4038 100644 --- a/packages/ui-browser/src/lib/store.ts +++ b/packages/ui-browser/src/lib/store.ts @@ -40,7 +40,7 @@ type Store = { sessionId: string | null; // diff - diff?: { raw: string; files: DiffFile[] }; + diff?: { raw: string; files: DiffFile[]; label?: string }; selectedFile?: string; // per-file UI state (path → flag) @@ -70,7 +70,7 @@ type Store = { setProtocol: (p: string) => void; setProject: (p: string) => void; setSessionId: (sid: string | null) => void; - setDiff: (raw: string, files: DiffFile[]) => void; + setDiff: (raw: string, files: DiffFile[], label?: string) => void; toggleViewed: (path: string) => void; toggleCollapsed: (path: string) => void; toggleTreeNode: (path: string) => void; @@ -139,11 +139,13 @@ export const useStore = create((set, get) => ({ setProject: (p) => set({ project: p }), setSessionId: (sid) => set({ sessionId: sid }), - setDiff: (raw, files) => { + setDiff: (raw, files, label) => { const s = get(); const prev = s.selectedFile; const stillExists = prev !== undefined && files.some((f) => f.path === prev); - const next: Partial = { diff: { raw, files } }; + const next: Partial = { + diff: { raw, files, ...(label !== undefined ? { label } : {}) }, + }; if (stillExists) { next.selectedFile = prev; } else { diff --git a/packages/ui-browser/src/lib/ws.ts b/packages/ui-browser/src/lib/ws.ts index 988dd57..6395357 100644 --- a/packages/ui-browser/src/lib/ws.ts +++ b/packages/ui-browser/src/lib/ws.ts @@ -45,7 +45,7 @@ const dispatch = (msg: ServerMessage) => { s.setSessionId(msg.session_id); return; case "diff": - s.setDiff(msg.raw, msg.files); + s.setDiff(msg.raw, msg.files, msg.label); return; case "chunk": s.appendChunk(msg.id, msg.delta);