Compare commits

...

6 Commits

Author SHA1 Message Date
d10d79399b fix(docker): trust bind-mounted repos via git safe.directory
The container runs as root over uid-1000-owned host repos; git's dubious-
ownership guard made every project read as not-a-repo, hiding the git diff
panel's Git tab and nulling the branch indicator. Bakes safe.directory='*'
into the runtime image. Applied live to the running container too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-03 08:29:33 +00:00
aeb2777ad4 docs: changelog for v2.7.14-backlog-hardening + v2.7.15-git-diff-panel
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-03 03:42:40 +00:00
2c58f2b3d3 Merge epic-backlog-and-gitdiff: v2.7.14 backlog hardening + v2.7.15 git diff panel
Two plans delivered via paseo-epic in an isolated worktree, audited green:
- v2.7.14: post-review backlog (external task-cancel + finalization, tool-call
  parser prune + pino logging, BooChat stall-timeout, view_session_history MCP
  tool, retire the :9502 fallback SPA).
- v2.7.15: git diff panel (Files/Git tab in the file browser with stage/commit/
  discard, server-side argv-safe git, sessionEvent-driven refresh).
2026-06-03 03:41:12 +00:00
d8bb2dabfe feat: git diff panel (Files/Git tab in the file browser)
Adds a Git tab to the right-side file panel that shows the project
repository's diff and lets the user stage, unstage, commit, and discard
whole files in-session. Two comparison modes (Uncommitted vs HEAD, and the
branch vs its base — upstream tracking branch else default branch), auto-
selected by repo state on first open and pinned after explicit choice;
per-file expand/collapse with lazy syntax-highlighted diffs, +/- stats, and
binary/large-file placeholders. All git read and write logic lives in
apps/server via a new git_diff service: argv-safe execFile only (never a
shell), per-file paths validated repo-relative through pathGuard with a
realpath symlink-escape check, server-derived commit identity (the request
carries no author fields), and the write endpoints are deliberately absent
from the assistant tool registry. Reads are bounded (30s deadline, 10MB);
an index lock or an in-progress merge/rebase/cherry-pick/bisect surfaces as
"repository busy" and disables writes. The panel stays current via a client
git_diff_refresh session event (no new wire contract) coalesced across tab
open, mutations, turn completion, and pending-change apply. Discard is an
irrecoverable hard-delete behind a plain confirm that distinguishes
reverting a tracked file from deleting an untracked one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-03 03:18:41 +00:00
ca028a4024 docs: add git-diff-panel implementation planning artifacts
Implementation decision log, iteration history, synthesis input, the
implementation plan, and discovery notes for the git-diff-panel feature.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 02:26:04 +00:00
3e7115afad docs: record @boocode/contracts SSOT + schema-migration learnings in CLAUDE.md
Add the packages/contracts package to the monorepo list, a consolidated
@boocode/contracts conventions bullet (subpaths, build-first, web-local strict
WsFrame union, built-dist consumption), and the `SELECT *` view / DROP COLUMN
(2BP01) schema gotcha that crash-looped boocoder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 02:25:59 +00:00
22 changed files with 2973 additions and 57 deletions

View File

@@ -2,6 +2,18 @@
All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch. All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch.
## v2.7.16-container-git-safedir — 2026-06-03
Hotfix that makes the `v2.7.15-git-diff-panel` work in production. The `boocode` container runs as root but bind-mounts host project repos owned by uid 1000, so git rejected them with "detected dubious ownership" and the diff route reported every project as not-a-repo — which hid the Git tab entirely (and had been silently nulling the existing branch indicator too). Adds `git config --system --add safe.directory '*'` to the Dockerfile runtime stage so the container's git trusts the mounted repos; applied live to the running container and baked into the image for future rebuilds. Surfaced by a live smoke immediately after the v2.7.14/v2.7.15 deploy.
## v2.7.15-git-diff-panel — 2026-06-03
A Files / Git tab in the right-side file panel (the file-browser sidebar) that shows the project repository's git diff and lets the user stage, unstage, commit, and discard whole files in-session — modeled on Paseo's diff view, scoped and planned through the `plan-a-feature``plan-implementation` skills, then built and audited via `paseo-epic` in an isolated worktree. Two comparison modes (Uncommitted vs HEAD, and the current branch vs its base — the upstream tracking branch else `origin/HEAD`), auto-selected by repo dirty-state on first open and pinned after an explicit choice; per-file expand/collapse with lazy Shiki `lang:'diff'` highlighting, +/- stats, and binary/too-large placeholders. All git read and write logic lives in `apps/server` (new `git_diff.ts` + routes on `projects.ts`) — the read-only-server posture governs the assistant's tools, not the user's own actions, and the container already mounts `/opt` read-write while `project_bootstrap` already commits via `execFile`. Every write uses the safe `execFile` argv pattern (never a shell string) with `--` operand separators, per-file `pathGuard` + realpath symlink-escape validation, server-derived `-c` commit identity (the request body is `.strict()` and carries no author fields), and the write endpoints are deliberately absent from the assistant tool registry. Reads are bounded (30s deadline, 10MB); an index lock or an in-progress merge/rebase/cherry-pick/bisect surfaces as "repository busy" and disables writes. The panel stays current via a client `git_diff_refresh` sessionEvent (no new wire contract) coalesced across tab-open, mutations, turn completion, and pending-change apply; discard is an irrecoverable hard-delete behind a plain confirm distinguishing a tracked revert from an untracked delete. New `git_diff` pure-helper + temp-repo integration tests (59 cases); server 630 tests green, web tsc clean. Pairs with `v2.7.14-backlog-hardening` (shipped together).
## v2.7.14-backlog-hardening — 2026-06-03
Five independent items from the second external-code-review backlog (`boocode_code_review_v2.md`), each built and audited as its own phase via `paseo-epic`. **External task-cancel** now actually works: Stop on an opencode/goose/qwen/claude task aborts the running child via a per-task `AbortController` registry reachable from the cancel route and finalizes the assistant message as `cancelled` — fixing two latent bugs (catch blocks left the message `streaming`; warm success-paths wrote `complete` on an aborted turn); warm pools/worktrees are preserved (abort the prompt only, never the pooled process) and the native boocode path is unchanged. **Parser prune**: the tool-call parser drops to its two load-bearing exports (eight zero-caller symbols unexported, a gate test added for the `<invoke>`-as-text fallback) with no live-path behavior change, and placeholder-rejection logging moves to pino. **BooChat stall-timeout**: a 90s per-chunk deadline wraps native inference's `fullStream` via `AbortSignal.any` so a hung local stream finalizes the message instead of hanging — no retry, since re-running re-emits already-streamed deltas (a pure `classifyStreamError` helper is added). **view_session_history**: a read-only MCP tool returning the newest-N transcript (role≠system) in chronological order. **Retire :9502**: the unused `apps/coder/web` fallback SPA is removed (package, static-serve block, build step, Dockerfile copy, `@fastify/static`), keeping every API/WS/health/MCP route. F1 added an optional `status` field to the shared `message_complete` contracts frame (so a deploy rebuilds `@boocode/contracts` first, as the sequence already does). Server 630 / coder 360 tests green.
## v2.7.13-contracts-ssot — 2026-06-02 ## v2.7.13-contracts-ssot — 2026-06-02
Creates `@boocode/contracts` (`packages/contracts`), a new workspace package that becomes the single source of truth for every cross-app wire contract — reversing the decision recorded in `v2.5.12-provider-lifecycle-phase4` that declined a shared types package as not worth the Docker/build-order risk at solo scale; a live `AgentSessionConfig` drift that had since appeared between `apps/coder` and `apps/web` justified the investment. Six contracts are now defined exactly once: the `WsFrameSchema` Zod runtime schema, the provider snapshot types (`ProviderSnapshotEntry` and family), the Zod provider-config schemas, `MessageMetadata` + `ErrorReason`, `AgentSessionConfig`, and `WorktreeRiskReport`; both Zod-backed contracts use `z.infer` so validator and type derive from the same definition and cannot drift independently. All four consumers — `apps/server`, `apps/web`, `apps/coder`, and the fallback SPA `apps/coder/web` — import via `workspace:*` through a per-subpath exports map consuming built dist only (no tsconfig project references); the hand-synced copies and their parity tests (`provider-types-parity.test.ts`; the ws-frames byte-parity assertion) are deleted while the KNOWN_FRAME_TYPES drift test and broker fail-closed tests are preserved. Build order is inverted in the root build script, Dockerfile, and coder deploy docs; `apps/coder/web`'s migration also removed dead `pending_change_*` reducer arms (no frame publisher exists for these — pending changes are HTTP-delivered), closing a latent missing-default-arm crash, and reconciled field-type conflicts with the canonical `WsFrame`; zod is pinned to a single version across the workspace. Server 543 / coder 293 / contracts 11 tests passing; human smoke verified on the live stack 2026-06-02. Creates `@boocode/contracts` (`packages/contracts`), a new workspace package that becomes the single source of truth for every cross-app wire contract — reversing the decision recorded in `v2.5.12-provider-lifecycle-phase4` that declined a shared types package as not worth the Docker/build-order risk at solo scale; a live `AgentSessionConfig` drift that had since appeared between `apps/coder` and `apps/web` justified the investment. Six contracts are now defined exactly once: the `WsFrameSchema` Zod runtime schema, the provider snapshot types (`ProviderSnapshotEntry` and family), the Zod provider-config schemas, `MessageMetadata` + `ErrorReason`, `AgentSessionConfig`, and `WorktreeRiskReport`; both Zod-backed contracts use `z.infer` so validator and type derive from the same definition and cannot drift independently. All four consumers — `apps/server`, `apps/web`, `apps/coder`, and the fallback SPA `apps/coder/web` — import via `workspace:*` through a per-subpath exports map consuming built dist only (no tsconfig project references); the hand-synced copies and their parity tests (`provider-types-parity.test.ts`; the ws-frames byte-parity assertion) are deleted while the KNOWN_FRAME_TYPES drift test and broker fail-closed tests are preserved. Build order is inverted in the root build script, Dockerfile, and coder deploy docs; `apps/coder/web`'s migration also removed dead `pending_change_*` reducer arms (no frame publisher exists for these — pending changes are HTTP-delivered), closing a latent missing-default-arm crash, and reconciled field-type conflicts with the canonical `WsFrame`; zod is pinned to a single version across the workspace. Server 543 / coder 293 / contracts 11 tests passing; human smoke verified on the live stack 2026-06-02.

View File

@@ -23,13 +23,12 @@ pnpm -C apps/server build # server only (tsc + copy schema.sql)
pnpm -C apps/web build # web only (vite) pnpm -C apps/web build # web only (vite)
# Type checking (no emit) # Type checking (no emit)
npx tsc --noEmit # project references (root) # Per-app is authoritative. There is NO root tsconfig.json (only tsconfig.base.json),
npx tsc -p apps/web/tsconfig.app.json --noEmit # web app specifically # so a bare `npx tsc --noEmit` at root compiles nothing.
npx tsc -p apps/web/tsconfig.app.json --noEmit # web (authoritative)
# IMPORTANT: root tsc --noEmit uses project references and can miss errors pnpm -C apps/server build # server typecheck (tsc + copy schema)
# that the per-app tsconfig catches. Always verify with the per-app command pnpm -C apps/coder build # coder typecheck
# when editing web code. The server build (pnpm -C apps/server build) is pnpm -C apps/booterm typecheck # booterm typecheck
# authoritative for server code.
# Production # Production
docker compose build --no-cache boocode && docker compose up -d docker compose build --no-cache boocode && docker compose up -d
@@ -39,7 +38,7 @@ Tests: `pnpm -C apps/server test` (vitest); `apps/coder` has its own suite — `
## Architecture ## Architecture
**Monorepo**: pnpm workspaces with `apps/server` (Fastify + postgres), `apps/web` (React + Vite), `apps/booterm` (Fastify + node-pty + tmux), `apps/coder` (BooCoder, host service). **Monorepo**: pnpm workspaces with `apps/server` (Fastify + postgres), `apps/web` (React + Vite), `apps/booterm` (Fastify + node-pty + tmux), `apps/coder` (BooCoder, host service), `packages/contracts` (`@boocode/contracts`, cross-app wire-contract SSOT — builds FIRST).
### Per-app deep references ### Per-app deep references
@@ -69,6 +68,8 @@ Schema CHECK migration order when renaming allowed values: (1) `ALTER TABLE ...
**`CREATE OR REPLACE VIEW` can't reorder/rename columns** (Postgres `42P16`): append a new `messages_with_parts` column at the END of the SELECT — a mid-list insert shifts an existing column → crash-loops boot. Add it to each explicit read SELECT too (`routes/messages.ts`/`chats.ts`/`ws.ts`). **`CREATE OR REPLACE VIEW` can't reorder/rename columns** (Postgres `42P16`): append a new `messages_with_parts` column at the END of the SELECT — a mid-list insert shifts an existing column → crash-loops boot. Add it to each explicit read SELECT too (`routes/messages.ts`/`chats.ts`/`ws.ts`).
**A `SELECT *` view pins every column** (`2BP01`): `DROP COLUMN` on the table fails while such a view exists. `human_inbox` is `SELECT * FROM tasks` — to drop a `tasks` column, `DROP VIEW IF EXISTS human_inbox` first, drop the column(s), then recreate the view (idempotent). Bites existing DBs only; a fresh DB never had the column, so fresh-DB testing misses it.
## Environment ## Environment
Required: `DATABASE_URL`, `LLAMA_SWAP_URL`. Optional: `PORT` (3000), `HOST` (0.0.0.0), `PROJECT_ROOT_WHITELIST` (/opt, read-only add-existing scope), `BOOTSTRAP_ROOT` (/opt/projects, writable bootstrap mkdir target — host must `mkdir -p` it before container start), `DEFAULT_MODEL`, `LOG_LEVEL`, `SEARXNG_URL` (default `http://100.114.205.53:8888` — internal Tailscale; the public host is behind Authelia, unusable from server context), `BOOCODE_TOOLS` (`core`|`standard`|`all`, default `all`; a ceiling, never expands an agent's whitelist), `MCP_CONFIG_PATH` (default `/data/mcp.json`, opencode `mcpServers` shape; missing = no MCP), `CONTEXT7_API_KEY` (the Context7 MCP key, referenced from `data/mcp.json` as `"{env:CONTEXT7_API_KEY}"`). `data/mcp.json` is **gitignored** but no longer holds secrets — string values support opencode-style `{env:VAR}` substitution (`mcp-config.ts:substituteEnvVars`, applied before Zod validation; unset var → `''` + warn), so real keys live in `.env`; template `data/mcp.example.json`. A config-only edit there needs only `docker compose restart boocode` (data/ is bind-mounted); changing a referenced secret edits `.env`. MCP loads at server startup with per-server graceful degradation; the coder does NOT load MCP (BooChat only). Required: `DATABASE_URL`, `LLAMA_SWAP_URL`. Optional: `PORT` (3000), `HOST` (0.0.0.0), `PROJECT_ROOT_WHITELIST` (/opt, read-only add-existing scope), `BOOTSTRAP_ROOT` (/opt/projects, writable bootstrap mkdir target — host must `mkdir -p` it before container start), `DEFAULT_MODEL`, `LOG_LEVEL`, `SEARXNG_URL` (default `http://100.114.205.53:8888` — internal Tailscale; the public host is behind Authelia, unusable from server context), `BOOCODE_TOOLS` (`core`|`standard`|`all`, default `all`; a ceiling, never expands an agent's whitelist), `MCP_CONFIG_PATH` (default `/data/mcp.json`, opencode `mcpServers` shape; missing = no MCP), `CONTEXT7_API_KEY` (the Context7 MCP key, referenced from `data/mcp.json` as `"{env:CONTEXT7_API_KEY}"`). `data/mcp.json` is **gitignored** but no longer holds secrets — string values support opencode-style `{env:VAR}` substitution (`mcp-config.ts:substituteEnvVars`, applied before Zod validation; unset var → `''` + warn), so real keys live in `.env`; template `data/mcp.example.json`. A config-only edit there needs only `docker compose restart boocode` (data/ is bind-mounted); changing a referenced secret edits `.env`. MCP loads at server startup with per-server graceful degradation; the coder does NOT load MCP (BooChat only).
@@ -116,6 +117,7 @@ Cross-cutting only. Per-app conventions live in the matching `apps/*/CLAUDE.md`.
- **Adding a new WS frame type** (cross-app): add it to `WsFrameSchema` in `packages/contracts/src/ws-frames.ts` (single source of truth; rebuild with `pnpm -C packages/contracts build`). The server's `InferenceFrame` loose union (`services/inference/turn.ts`) and the web's strict `WsFrame` discriminated union (`apps/web/src/api/types.ts`) still exist separately and also need updating. Server publish is permissive; the frontend type is the wire-format gate — missing the web side silently drops the frame at JSON-parse. - **Adding a new WS frame type** (cross-app): add it to `WsFrameSchema` in `packages/contracts/src/ws-frames.ts` (single source of truth; rebuild with `pnpm -C packages/contracts build`). The server's `InferenceFrame` loose union (`services/inference/turn.ts`) and the web's strict `WsFrame` discriminated union (`apps/web/src/api/types.ts`) still exist separately and also need updating. Server publish is permissive; the frontend type is the wire-format gate — missing the web side silently drops the frame at JSON-parse.
- **Sentinels** (cross-app) are `role='system'` rows with structured `metadata.kind` (`cap_hit`, `doom_loop`). UI-only — `buildMessagesPayload` strips them via `isAnySentinel` so the LLM never sees them. `MessageMetadata` is single-sourced in `@boocode/contracts` (`packages/contracts/src/message-metadata.ts`). A new kind requires updating that file and rebuilding the package, plus a render branch in `apps/web/src/components/MessageBubble.tsx`. - **Sentinels** (cross-app) are `role='system'` rows with structured `metadata.kind` (`cap_hit`, `doom_loop`). UI-only — `buildMessagesPayload` strips them via `isAnySentinel` so the LLM never sees them. `MessageMetadata` is single-sourced in `@boocode/contracts` (`packages/contracts/src/message-metadata.ts`). A new kind requires updating that file and rebuilding the package, plus a render branch in `apps/web/src/components/MessageBubble.tsx`.
- **Provider snapshot types** (`ProviderSnapshotEntry`, `ProviderModel`, `ProviderMode`, `ThinkingOption`, `AgentCommand`, `ProviderSnapshotStatus`) are single-sourced in `@boocode/contracts` (`packages/contracts/src/provider-snapshot.ts`); `apps/coder/src/services/provider-types.ts` re-exports them. Edit the package source; there is no hand-synced web copy to update. - **Provider snapshot types** (`ProviderSnapshotEntry`, `ProviderModel`, `ProviderMode`, `ThinkingOption`, `AgentCommand`, `ProviderSnapshotStatus`) are single-sourced in `@boocode/contracts` (`packages/contracts/src/provider-snapshot.ts`); `apps/coder/src/services/provider-types.ts` re-exports them. Edit the package source; there is no hand-synced web copy to update.
- **`@boocode/contracts`** single-sources cross-app wire contracts via per-subpath built-dist exports, consumed by all four apps (incl. `apps/coder/web`): `./ws-frames`, `./provider-snapshot`, `./provider-config` (Zod schemas), `./message-metadata` (`MessageMetadata`/`ErrorReason`/`AgentSessionConfig`), `./worktree-risk`. It builds BEFORE every consumer (root build, Dockerfile, coder deploy). Its `WsFrame` is the loose `z.infer` of `WsFrameSchema` (payloads `unknown`); the web's richer strict `WsFrame` union is **deliberately web-local** (`apps/web/src/api/types.ts`), bridged to the validated frame by a cast — don't move it into the package. Consume built `dist` via the exports map; never add the package to a tsconfig `references` array.
- **JSONB columns**: use `sql.json(value as never)` — NOT `${JSON.stringify(value)}::jsonb` which double-serializes (stores a JSON string instead of an object/array). Pattern in `parts.ts`, `settings.ts`. - **JSONB columns**: use `sql.json(value as never)` — NOT `${JSON.stringify(value)}::jsonb` which double-serializes (stores a JSON string instead of an object/array). Pattern in `parts.ts`, `settings.ts`.
- Skills live in `data/skills/<vendor>/`; Sam's own namespace is `boocode/` (`committing-changes`, `using-worktrees`, `improving-boocode-guidance`, `systematic-debugging`) — `SKILL.md` + optional `eval.yaml` (gerund names; eval = `skill:` + `tasks:` of `prompt`+`grader`, incl. a negative-trigger task). `data/skills/` is canonical; a divergent mirror at `/opt/skills/` exists. - Skills live in `data/skills/<vendor>/`; Sam's own namespace is `boocode/` (`committing-changes`, `using-worktrees`, `improving-boocode-guidance`, `systematic-debugging`) — `SKILL.md` + optional `eval.yaml` (gerund names; eval = `skill:` + `tasks:` of `prompt`+`grader`, incl. a negative-trigger task). `data/skills/` is canonical; a divergent mirror at `/opt/skills/` exists.

View File

@@ -24,6 +24,9 @@ RUN pnpm deploy --filter=@boocode/server --prod --legacy /out/server
FROM node:20-alpine AS runtime FROM node:20-alpine AS runtime
RUN apk add --no-cache ripgrep git openssh-client RUN apk add --no-cache ripgrep git openssh-client
# The container runs as root but bind-mounts host project repos owned by uid 1000;
# trust them so git read/write tools (git_status, the git diff panel) work over the mount.
RUN git config --system --add safe.directory '*'
RUN mkdir -p /root/.ssh && ssh-keyscan -p 2222 -H 100.114.205.53 git.indifferentketchup.com >> /root/.ssh/known_hosts && chmod 700 /root/.ssh && chmod 600 /root/.ssh/known_hosts RUN mkdir -p /root/.ssh && ssh-keyscan -p 2222 -H 100.114.205.53 git.indifferentketchup.com >> /root/.ssh/known_hosts && chmod 700 /root/.ssh && chmod 600 /root/.ssh/known_hosts
WORKDIR /app WORKDIR /app

View File

@@ -10,6 +10,18 @@ import { resolveProjectRoot, PathScopeError } from '../services/path_guard.js';
import { listDir, viewFile } from '../services/file_ops.js'; import { listDir, viewFile } from '../services/file_ops.js';
import { getProjectFiles } from '../services/file_index.js'; import { getProjectFiles } from '../services/file_index.js';
import { getGitMeta } from '../services/git_meta.js'; import { getGitMeta } from '../services/git_meta.js';
import {
getGitDiff,
stageFiles,
unstageFiles,
commitFiles,
discardFiles,
detectInProgress,
isRepoDirty,
autoSelectMode,
GitWriteError,
} from '../services/git_diff.js';
import type { GitDiffMode } from '../services/git_diff.js';
import { import {
bootstrapProject, bootstrapProject,
BootstrapNameError, BootstrapNameError,
@@ -453,6 +465,178 @@ export function registerProjectRoutes(
} }
); );
// GET /api/projects/:id/git/diff?mode=uncommitted|committed
// Returns the structured diff payload for the project repository. mode param
// selects the comparison: uncommitted (working tree vs HEAD) or committed
// (branch vs its upstream/default-branch base). When mode is absent the server
// auto-selects based on dirty state (FIX 1: dirty → uncommitted, clean → committed).
// Always includes auto_mode (the dirty-state-derived mode) so the client can
// show a suggestion when a pinned mode diverges from what would be auto-selected.
// Returns { git_repo: false } when the path is not a git repository.
app.get<{ Params: { id: string }; Querystring: { mode?: string } }>(
'/api/projects/:id/git/diff',
async (req, reply) => {
const { id } = req.params;
const rawMode = req.query.mode;
const projectPath = await selectProjectPath(sql, id);
if (projectPath === null) {
reply.code(404);
return { error: 'not found' };
}
let projectRoot: string;
try {
projectRoot = await resolveProjectRoot(projectPath);
} catch (err) {
if (err instanceof PathScopeError) {
reply.code(404);
return { error: err.message };
}
throw err;
}
// Always detect dirty state: used for auto-select (FIX 1) and suggestion (FIX 4).
const dirty = await isRepoDirty(projectRoot);
const auto_mode = autoSelectMode(dirty);
const mode: GitDiffMode =
rawMode === 'committed' ? 'committed' :
rawMode === 'uncommitted' ? 'uncommitted' :
auto_mode; // no mode param → auto-select (FIX 1)
const result = await getGitDiff(projectRoot, mode);
if (result === null) {
return { git_repo: false, mode, auto_mode, base_label: null, in_progress_op: null, files: [] };
}
return { git_repo: true, ...result, auto_mode };
}
);
// ── Git write routes (Phase 2) ─────────────────────────────────────────────
// These are user UI actions — NOT registered in the assistant tool registry.
// D-3: argv-safe runGit/execFile with -- separators (never shell strings).
// D-4: per-file pathGuard validation via validateWritePath.
// D-5: commit identity server-derived; request body .strict(), no author fields.
// D-7: index-lock → 409; in-progress op → 409.
// D-13: NOT in ALL_TOOLS.
const GitFilesBody = z.object({ files: z.array(z.string().min(1)).min(1) });
const GitCommitBody = z
.object({
message: z.string().min(1),
files: z.array(z.string().min(1)).optional(),
})
.strict();
const GitDiscardBody = z.object({
files: z.array(
z
.object({
path: z.string().min(1),
change_type: z.string().min(1),
staged: z.boolean(),
})
.strict(),
).min(1),
});
// POST /api/projects/:id/git/stage — stage whole files
app.post<{ Params: { id: string } }>(
'/api/projects/:id/git/stage',
async (req, reply) => {
const body = GitFilesBody.safeParse(req.body);
if (!body.success) { reply.code(400); return { error: body.error.message }; }
const { id } = req.params;
const projectPath = await selectProjectPath(sql, id);
if (!projectPath) { reply.code(404); return { error: 'not found' }; }
let root: string;
try { root = await resolveProjectRoot(projectPath); }
catch (err) { if (err instanceof PathScopeError) { reply.code(404); return { error: (err as Error).message }; } throw err; }
const inProg = await detectInProgress(root);
if (inProg) { reply.code(409); return { error: `git ${inProg} in progress — write actions disabled` }; }
try {
await stageFiles(root, body.data.files);
return { ok: true };
} catch (err) {
if (err instanceof GitWriteError) { reply.code(err.busy ? 409 : 500); return { error: err.message }; }
throw err;
}
},
);
// POST /api/projects/:id/git/unstage — unstage whole files
app.post<{ Params: { id: string } }>(
'/api/projects/:id/git/unstage',
async (req, reply) => {
const body = GitFilesBody.safeParse(req.body);
if (!body.success) { reply.code(400); return { error: body.error.message }; }
const { id } = req.params;
const projectPath = await selectProjectPath(sql, id);
if (!projectPath) { reply.code(404); return { error: 'not found' }; }
let root: string;
try { root = await resolveProjectRoot(projectPath); }
catch (err) { if (err instanceof PathScopeError) { reply.code(404); return { error: (err as Error).message }; } throw err; }
const inProg = await detectInProgress(root);
if (inProg) { reply.code(409); return { error: `git ${inProg} in progress — write actions disabled` }; }
try {
await unstageFiles(root, body.data.files);
return { ok: true };
} catch (err) {
if (err instanceof GitWriteError) { reply.code(err.busy ? 409 : 500); return { error: err.message }; }
throw err;
}
},
);
// POST /api/projects/:id/git/commit — commit staged files (identity server-derived)
app.post<{ Params: { id: string } }>(
'/api/projects/:id/git/commit',
async (req, reply) => {
const body = GitCommitBody.safeParse(req.body);
if (!body.success) { reply.code(400); return { error: body.error.message }; }
const { id } = req.params;
const projectPath = await selectProjectPath(sql, id);
if (!projectPath) { reply.code(404); return { error: 'not found' }; }
let root: string;
try { root = await resolveProjectRoot(projectPath); }
catch (err) { if (err instanceof PathScopeError) { reply.code(404); return { error: (err as Error).message }; } throw err; }
const inProg = await detectInProgress(root);
if (inProg) { reply.code(409); return { error: `git ${inProg} in progress — write actions disabled` }; }
try {
await commitFiles(root, body.data.message, body.data.files);
return { ok: true };
} catch (err) {
if (err instanceof GitWriteError) { reply.code(err.busy ? 409 : 500); return { error: err.message }; }
throw err;
}
},
);
// POST /api/projects/:id/git/discard — discard file changes (irrecoverable)
app.post<{ Params: { id: string } }>(
'/api/projects/:id/git/discard',
async (req, reply) => {
const body = GitDiscardBody.safeParse(req.body);
if (!body.success) { reply.code(400); return { error: body.error.message }; }
const { id } = req.params;
const projectPath = await selectProjectPath(sql, id);
if (!projectPath) { reply.code(404); return { error: 'not found' }; }
let root: string;
try { root = await resolveProjectRoot(projectPath); }
catch (err) { if (err instanceof PathScopeError) { reply.code(404); return { error: (err as Error).message }; } throw err; }
const inProg = await detectInProgress(root);
if (inProg) { reply.code(409); return { error: `git ${inProg} in progress — write actions disabled` }; }
try {
await discardFiles(root, body.data.files);
return { ok: true };
} catch (err) {
if (err instanceof GitWriteError) { reply.code(err.busy ? 409 : 500); return { error: err.message }; }
throw err;
}
},
);
// GET /api/projects/:id/files // GET /api/projects/:id/files
app.get<{ Params: { id: string } }>( app.get<{ Params: { id: string } }>(
'/api/projects/:id/files', '/api/projects/:id/files',

View File

@@ -0,0 +1,346 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { realpath } from 'node:fs/promises';
import { execFile } from 'node:child_process';
import { promisify } from 'node:util';
import {
parseNameStatus,
splitDiffByFile,
classifyDiffBody,
autoSelectMode,
detectInProgress,
resolveCommittedBase,
canCommit,
getGitDiff,
} from '../git_diff.js';
import type { GitDiffFile } from '../git_diff.js';
const execFileAsync = promisify(execFile);
// ── T1: parseNameStatus ────────────────────────────────────────────────────
describe('parseNameStatus', () => {
it('parses modified file', () => {
const files = parseNameStatus('M\tsrc/foo.ts\n');
expect(files).toHaveLength(1);
expect(files[0]).toMatchObject({ path: 'src/foo.ts', change_type: 'modified', old_path: null });
});
it('parses added file', () => {
const files = parseNameStatus('A\tnewfile.ts\n');
expect(files).toHaveLength(1);
expect(files[0]).toMatchObject({ path: 'newfile.ts', change_type: 'added' });
});
it('parses deleted file', () => {
const files = parseNameStatus('D\tremoved.ts\n');
expect(files).toHaveLength(1);
expect(files[0]).toMatchObject({ path: 'removed.ts', change_type: 'deleted' });
});
it('parses renamed file with similarity score', () => {
const files = parseNameStatus('R100\told.ts\tnew.ts\n');
expect(files).toHaveLength(1);
expect(files[0]).toMatchObject({ path: 'new.ts', old_path: 'old.ts', change_type: 'renamed' });
});
it('parses type-changed file as modified', () => {
const files = parseNameStatus('T\tsymlink.ts\n');
expect(files).toHaveLength(1);
expect(files[0]).toMatchObject({ path: 'symlink.ts', change_type: 'modified' });
});
it('parses multiple files from multiline output', () => {
const output = 'M\ta.ts\nA\tb.ts\nD\tc.ts\n';
const files = parseNameStatus(output);
expect(files).toHaveLength(3);
expect(files.map((f) => f.change_type)).toEqual(['modified', 'added', 'deleted']);
});
it('ignores blank lines', () => {
const files = parseNameStatus('\n\nM\ta.ts\n\n');
expect(files).toHaveLength(1);
});
it('returns empty array for empty input', () => {
expect(parseNameStatus('')).toHaveLength(0);
expect(parseNameStatus('\n')).toHaveLength(0);
});
});
// ── T2: splitDiffByFile ────────────────────────────────────────────────────
describe('splitDiffByFile', () => {
const FIXTURE = `diff --git a/src/a.ts b/src/a.ts
index abc1234..def5678 100644
--- a/src/a.ts
+++ b/src/a.ts
@@ -1,3 +1,4 @@
context
-old line
+new line
more context
diff --git a/src/b.ts b/src/b.ts
index 1111111..2222222 100644
--- a/src/b.ts
+++ b/src/b.ts
@@ -10,2 +10,3 @@
ctx
+added
`;
it('splits two-file diff into two entries', () => {
const map = splitDiffByFile(FIXTURE);
expect(map.size).toBe(2);
expect(map.has('src/a.ts')).toBe(true);
expect(map.has('src/b.ts')).toBe(true);
});
it('each segment starts with diff --git header', () => {
const map = splitDiffByFile(FIXTURE);
expect(map.get('src/a.ts')).toMatch(/^diff --git a\/src\/a\.ts/);
expect(map.get('src/b.ts')).toMatch(/^diff --git a\/src\/b\.ts/);
});
it('handles deleted file (no +++ b/ line)', () => {
const deleted = `diff --git a/gone.ts b/gone.ts
deleted file mode 100644
--- a/gone.ts
+++ /dev/null
@@ -1,2 +0,0 @@
-line1
-line2
`;
const map = splitDiffByFile(deleted);
expect(map.size).toBe(1);
expect(map.has('gone.ts')).toBe(true);
});
it('returns empty map for empty input', () => {
expect(splitDiffByFile('').size).toBe(0);
expect(splitDiffByFile('\n').size).toBe(0);
});
});
// ── T3: resolveCommittedBase (integration with temp git repo) ──────────────
describe('resolveCommittedBase', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-gitdiff-base-')));
await execFileAsync('git', ['init'], { cwd: tmp });
await execFileAsync('git', ['-c', 'user.email=test@test.com', '-c', 'user.name=Test',
'commit', '--allow-empty', '-m', 'init'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('returns null base when no upstream and no origin', async () => {
const { base, label } = await resolveCommittedBase(tmp);
expect(base).toBeNull();
expect(label).toBeTruthy(); // still has a descriptive label
});
});
// ── T4: autoSelectMode ────────────────────────────────────────────────────
describe('autoSelectMode', () => {
it('returns uncommitted when dirty', () => {
expect(autoSelectMode(true)).toBe('uncommitted');
});
it('returns committed when clean', () => {
expect(autoSelectMode(false)).toBe('committed');
});
});
// ── T5: classifyDiffBody ──────────────────────────────────────────────────
describe('classifyDiffBody', () => {
it('classifies a normal diff as diff', () => {
const body = `diff --git a/foo b/foo
--- a/foo
+++ b/foo
@@ -1 +1 @@
-old
+new
`;
expect(classifyDiffBody(body)).toBe('diff');
});
it('classifies binary diff as binary', () => {
const body = `diff --git a/image.png b/image.png
index abc..def 100644
Binary files a/image.png and b/image.png differ
`;
expect(classifyDiffBody(body)).toBe('binary');
});
it('classifies oversized diff as too_large', () => {
const big = 'a'.repeat(600 * 1024); // 600KB > default cap
expect(classifyDiffBody(big)).toBe('too_large');
});
it('respects custom cap', () => {
const body = 'a'.repeat(100);
expect(classifyDiffBody(body, 50)).toBe('too_large');
expect(classifyDiffBody(body, 200)).toBe('diff');
});
});
// ── T6: detectInProgress ──────────────────────────────────────────────────
describe('detectInProgress', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-inprogress-')));
await mkdir(join(tmp, '.git'));
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('returns null when no sentinel files present', async () => {
expect(await detectInProgress(tmp)).toBeNull();
});
it('detects merge via MERGE_HEAD', async () => {
await writeFile(join(tmp, '.git', 'MERGE_HEAD'), 'abc');
expect(await detectInProgress(tmp)).toBe('merge');
await rm(join(tmp, '.git', 'MERGE_HEAD'));
});
it('detects cherry-pick via CHERRY_PICK_HEAD', async () => {
await writeFile(join(tmp, '.git', 'CHERRY_PICK_HEAD'), 'abc');
expect(await detectInProgress(tmp)).toBe('cherry-pick');
await rm(join(tmp, '.git', 'CHERRY_PICK_HEAD'));
});
it('detects bisect via BISECT_LOG', async () => {
await writeFile(join(tmp, '.git', 'BISECT_LOG'), 'abc');
expect(await detectInProgress(tmp)).toBe('bisect');
await rm(join(tmp, '.git', 'BISECT_LOG'));
});
it('detects rebase via rebase-merge directory', async () => {
await mkdir(join(tmp, '.git', 'rebase-merge'));
expect(await detectInProgress(tmp)).toBe('rebase');
await rm(join(tmp, '.git', 'rebase-merge'), { recursive: true });
});
it('detects rebase via rebase-apply directory', async () => {
await mkdir(join(tmp, '.git', 'rebase-apply'));
expect(await detectInProgress(tmp)).toBe('rebase');
await rm(join(tmp, '.git', 'rebase-apply'), { recursive: true });
});
});
// ── T7: canCommit ─────────────────────────────────────────────────────────
describe('canCommit', () => {
const stagedFile: GitDiffFile = {
path: 'a.ts',
old_path: null,
change_type: 'modified',
added_lines: 1,
removed_lines: 0,
staged: true,
diff_body: '+new',
is_binary: false,
is_too_large: false,
};
const unstagedFile: GitDiffFile = { ...stagedFile, staged: false };
it('returns true when at least one file is staged', () => {
expect(canCommit([stagedFile, unstagedFile])).toBe(true);
});
it('returns false when no files are staged', () => {
expect(canCommit([unstagedFile])).toBe(false);
expect(canCommit([])).toBe(false);
});
});
// ── T8: getGitDiff integration test ───────────────────────────────────────
describe('getGitDiff integration (temp repo)', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-gitdiff-int-')));
// Init repo + initial commit
await execFileAsync('git', ['init'], { cwd: tmp });
await execFileAsync('git', ['config', 'user.email', 'test@test.com'], { cwd: tmp });
await execFileAsync('git', ['config', 'user.name', 'Test'], { cwd: tmp });
await writeFile(join(tmp, 'existing.ts'), 'const x = 1;\n');
await execFileAsync('git', ['add', '.'], { cwd: tmp });
await execFileAsync('git', ['commit', '-m', 'init'], { cwd: tmp });
// Modify existing file (unstaged)
await writeFile(join(tmp, 'existing.ts'), 'const x = 2;\n');
// Add new untracked file
await writeFile(join(tmp, 'untracked.ts'), 'export {};\n');
// Stage a new file
await writeFile(join(tmp, 'staged.ts'), 'export const y = 1;\n');
await execFileAsync('git', ['add', 'staged.ts'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('getGitDiff returns git_repo true for a git repo', async () => {
const result = await getGitDiff(tmp, 'uncommitted');
expect(result).not.toBeNull();
expect(result!.mode).toBe('uncommitted');
});
it('includes modified file in uncommitted mode', async () => {
const result = await getGitDiff(tmp, 'uncommitted');
const paths = result!.files.map((f: GitDiffFile) => f.path);
expect(paths).toContain('existing.ts');
});
it('includes staged file with staged=true', async () => {
const result = await getGitDiff(tmp, 'uncommitted');
const staged = result!.files.find((f: GitDiffFile) => f.path === 'staged.ts');
expect(staged).toBeDefined();
expect(staged!.staged).toBe(true);
expect(staged!.change_type).toBe('added');
});
it('includes untracked file with change_type=untracked', async () => {
const result = await getGitDiff(tmp, 'uncommitted');
const untracked = result!.files.find((f: GitDiffFile) => f.path === 'untracked.ts');
expect(untracked).toBeDefined();
expect(untracked!.change_type).toBe('untracked');
});
it('returns null for a non-git directory', async () => {
const nonGit = await realpath(await mkdtemp(join(tmpdir(), 'boocode-nongit-')));
try {
const result = await getGitDiff(nonGit, 'uncommitted');
expect(result).toBeNull();
} finally {
await rm(nonGit, { recursive: true, force: true });
}
});
it('returns in_progress_op when MERGE_HEAD exists', async () => {
await writeFile(join(tmp, '.git', 'MERGE_HEAD'), 'abc\n');
try {
const result = await getGitDiff(tmp, 'uncommitted');
expect(result!.in_progress_op).toBe('merge');
} finally {
await rm(join(tmp, '.git', 'MERGE_HEAD'));
}
});
});

View File

@@ -0,0 +1,379 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { mkdtemp, rm, writeFile, mkdir, access, symlink } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { realpath } from 'node:fs/promises';
import { execFile } from 'node:child_process';
import { promisify } from 'node:util';
import {
validateWritePath,
checkSymlinkEscape,
stageFiles,
unstageFiles,
commitFiles,
discardFiles,
deriveCommitIdentity,
GitWriteError,
getGitDiff,
} from '../git_diff.js';
const execFileAsync = promisify(execFile);
// ── T12: validateWritePath — pure validation ──────────────────────────────
describe('validateWritePath', () => {
const root = '/repo/root';
it('accepts a simple relative path', () => {
expect(() => validateWritePath(root, 'src/foo.ts')).not.toThrow();
});
it('accepts a nested path', () => {
expect(() => validateWritePath(root, 'a/b/c.ts')).not.toThrow();
});
it('rejects empty string', () => {
expect(() => validateWritePath(root, '')).toThrow(GitWriteError);
});
it('rejects path starting with - (flag injection)', () => {
expect(() => validateWritePath(root, '-flag')).toThrow(GitWriteError);
expect(() => validateWritePath(root, '--option')).toThrow(GitWriteError);
});
it('rejects "." (repo root discard)', () => {
expect(() => validateWritePath(root, '.')).toThrow(GitWriteError);
});
it('rejects absolute paths', () => {
expect(() => validateWritePath(root, '/etc/passwd')).toThrow(GitWriteError);
expect(() => validateWritePath(root, '/repo/root/file.ts')).toThrow(GitWriteError);
});
it('rejects ".." traversal escaping root', () => {
expect(() => validateWritePath(root, '../outside/file.ts')).toThrow(GitWriteError);
expect(() => validateWritePath(root, 'a/../../outside')).toThrow(GitWriteError);
});
it('rejects path resolving exactly to root', () => {
// e.g. "a/.." resolves to /repo/root which is the root itself
expect(() => validateWritePath(root, 'a/..')).toThrow(GitWriteError);
});
it('throws GitWriteError not just Error', () => {
try {
validateWritePath(root, '-bad');
} catch (err) {
expect(err).toBeInstanceOf(GitWriteError);
expect((err as GitWriteError).busy).toBe(false);
}
});
});
// ── Integration tests (temp git repo) ─────────────────────────────────────
async function initRepo(dir: string) {
await execFileAsync('git', ['init'], { cwd: dir });
await execFileAsync('git', ['config', 'user.email', 'test@test.com'], { cwd: dir });
await execFileAsync('git', ['config', 'user.name', 'Test User'], { cwd: dir });
}
async function fileExists(path: string): Promise<boolean> {
try {
await access(path);
return true;
} catch {
return false;
}
}
// ── T9: stage / unstage round-trip ────────────────────────────────────────
describe('stageFiles / unstageFiles round-trip (temp repo)', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-write-stage-')));
await initRepo(tmp);
await writeFile(join(tmp, 'initial.ts'), 'const a = 1;\n');
await execFileAsync('git', ['add', '.'], { cwd: tmp });
await execFileAsync('git', ['commit', '-m', 'init'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('staging an untracked file shows it as staged in diff', async () => {
await writeFile(join(tmp, 'new.ts'), 'export const x = 1;\n');
// Before staging
const before = await getGitDiff(tmp, 'uncommitted');
const untrackedBefore = before!.files.find((f) => f.path === 'new.ts');
expect(untrackedBefore?.change_type).toBe('untracked');
expect(untrackedBefore?.staged).toBe(false);
// Stage
await stageFiles(tmp, ['new.ts']);
// After staging
const after = await getGitDiff(tmp, 'uncommitted');
const stagedAfter = after!.files.find((f) => f.path === 'new.ts');
expect(stagedAfter?.staged).toBe(true);
expect(stagedAfter?.change_type).toBe('added');
});
it('unstaging removes file from staged set', async () => {
// new.ts is currently staged from the previous test
await unstageFiles(tmp, ['new.ts']);
const after = await getGitDiff(tmp, 'uncommitted');
const f = after!.files.find((f) => f.path === 'new.ts');
expect(f?.staged).toBe(false);
expect(f?.change_type).toBe('untracked');
});
it('stageFiles rejects a path starting with -', async () => {
await expect(stageFiles(tmp, ['-bad'])).rejects.toThrow(GitWriteError);
});
it('stageFiles rejects path traversal', async () => {
await expect(stageFiles(tmp, ['../outside.ts'])).rejects.toThrow(GitWriteError);
});
});
// ── T10: commit with server-derived identity ──────────────────────────────
describe('commitFiles with server-derived identity (temp repo)', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-write-commit-')));
await initRepo(tmp);
await writeFile(join(tmp, 'base.ts'), 'export const a = 1;\n');
await execFileAsync('git', ['add', '.'], { cwd: tmp });
await execFileAsync('git', ['commit', '-m', 'init'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('deriveCommitIdentity falls back when no git config set', async () => {
// New repo initialized without global user config — may or may not have local config.
// The function should always return a non-empty name and email.
const identity = await deriveCommitIdentity(tmp);
expect(identity.name).toBeTruthy();
expect(identity.email).toBeTruthy();
});
it('deriveCommitIdentity uses git config when set', async () => {
const identity = await deriveCommitIdentity(tmp);
// We set user.email/name in initRepo above
expect(identity.name).toBe('Test User');
expect(identity.email).toBe('test@test.com');
});
it('commit creates a new commit and the staged file is no longer in diff', async () => {
await writeFile(join(tmp, 'newfile.ts'), 'export const b = 2;\n');
await stageFiles(tmp, ['newfile.ts']);
const before = await getGitDiff(tmp, 'uncommitted');
expect(before!.files.find((f) => f.path === 'newfile.ts')).toBeDefined();
await commitFiles(tmp, 'add newfile');
const after = await getGitDiff(tmp, 'uncommitted');
expect(after!.files.find((f) => f.path === 'newfile.ts')).toBeUndefined();
});
it('commit with specific files only commits those files', async () => {
await writeFile(join(tmp, 'a.ts'), 'const a = 1;\n');
await writeFile(join(tmp, 'b.ts'), 'const b = 2;\n');
await stageFiles(tmp, ['a.ts', 'b.ts']);
await commitFiles(tmp, 'partial commit', ['a.ts']);
const after = await getGitDiff(tmp, 'uncommitted');
const aFile = after!.files.find((f) => f.path === 'a.ts');
const bFile = after!.files.find((f) => f.path === 'b.ts');
// a.ts was committed — should not appear in uncommitted diff
expect(aFile).toBeUndefined();
// b.ts is still staged
expect(bFile?.staged).toBe(true);
});
it('commit rejects a path starting with - in files list', async () => {
await expect(commitFiles(tmp, 'msg', ['-bad'])).rejects.toThrow(GitWriteError);
});
});
// ── T11: discard tracked vs untracked ─────────────────────────────────────
describe('discardFiles (temp repo)', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-write-discard-')));
await initRepo(tmp);
await writeFile(join(tmp, 'tracked.ts'), 'const orig = 1;\n');
await execFileAsync('git', ['add', '.'], { cwd: tmp });
await execFileAsync('git', ['commit', '-m', 'init'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('discarding a modified tracked file reverts its content', async () => {
await writeFile(join(tmp, 'tracked.ts'), 'const modified = 99;\n');
const before = await getGitDiff(tmp, 'uncommitted');
expect(before!.files.find((f) => f.path === 'tracked.ts')).toBeDefined();
await discardFiles(tmp, [{ path: 'tracked.ts', change_type: 'modified', staged: false }]);
const after = await getGitDiff(tmp, 'uncommitted');
expect(after!.files.find((f) => f.path === 'tracked.ts')).toBeUndefined();
});
it('discarding an untracked file removes it from disk', async () => {
await writeFile(join(tmp, 'untracked.ts'), 'orphan\n');
const exists = await fileExists(join(tmp, 'untracked.ts'));
expect(exists).toBe(true);
await discardFiles(tmp, [{ path: 'untracked.ts', change_type: 'untracked', staged: false }]);
expect(await fileExists(join(tmp, 'untracked.ts'))).toBe(false);
});
it('discarding a staged-addition file removes it from index and disk', async () => {
await writeFile(join(tmp, 'staged-add.ts'), 'new file\n');
await stageFiles(tmp, ['staged-add.ts']);
const before = await getGitDiff(tmp, 'uncommitted');
expect(before!.files.find((f) => f.path === 'staged-add.ts')?.staged).toBe(true);
await discardFiles(tmp, [{ path: 'staged-add.ts', change_type: 'added', staged: true }]);
const after = await getGitDiff(tmp, 'uncommitted');
expect(after!.files.find((f) => f.path === 'staged-add.ts')).toBeUndefined();
expect(await fileExists(join(tmp, 'staged-add.ts'))).toBe(false);
});
it('discardFiles rejects "." (repo root)', async () => {
await expect(
discardFiles(tmp, [{ path: '.', change_type: 'modified', staged: false }]),
).rejects.toThrow(GitWriteError);
});
it('discardFiles rejects path traversal', async () => {
await expect(
discardFiles(tmp, [{ path: '../outside', change_type: 'untracked', staged: false }]),
).rejects.toThrow(GitWriteError);
});
});
// ── Index-lock → busy error ────────────────────────────────────────────────
describe('index-lock detection', () => {
let tmp: string;
beforeAll(async () => {
tmp = await realpath(await mkdtemp(join(tmpdir(), 'boocode-write-lock-')));
await initRepo(tmp);
await writeFile(join(tmp, 'file.ts'), 'const x = 1;\n');
await execFileAsync('git', ['add', '.'], { cwd: tmp });
await execFileAsync('git', ['commit', '-m', 'init'], { cwd: tmp });
});
afterAll(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('stageFiles throws GitWriteError with busy=true when index.lock exists', async () => {
await writeFile(join(tmp, 'new.ts'), 'export {};\n');
// Simulate a lock by creating .git/index.lock
await mkdir(join(tmp, '.git'), { recursive: true });
await writeFile(join(tmp, '.git', 'index.lock'), '');
try {
await stageFiles(tmp, ['new.ts']);
// Should not reach here
expect(true).toBe(false);
} catch (err) {
expect(err).toBeInstanceOf(GitWriteError);
expect((err as GitWriteError).busy).toBe(true);
} finally {
try { await rm(join(tmp, '.git', 'index.lock')); } catch { /* already gone */ }
}
});
});
// ── Commit request schema: reject unknown author fields ───────────────────
describe('GitCommitBody schema strictness (unit)', () => {
it('rejects extra author/email fields via Zod strict', () => {
// We import Zod inline to mirror the route's schema
const { z } = require('zod');
const GitCommitBody = z
.object({
message: z.string().min(1),
files: z.array(z.string().min(1)).optional(),
})
.strict();
const result = GitCommitBody.safeParse({
message: 'test commit',
author: 'Evil <evil@hack.com>',
email: 'evil@hack.com',
});
expect(result.success).toBe(false);
});
it('accepts valid commit body with message only', () => {
const { z } = require('zod');
const GitCommitBody = z
.object({
message: z.string().min(1),
files: z.array(z.string().min(1)).optional(),
})
.strict();
const result = GitCommitBody.safeParse({ message: 'add feature' });
expect(result.success).toBe(true);
});
});
// ── T13: checkSymlinkEscape (FIX 3) ──────────────────────────────────────────
describe('checkSymlinkEscape', () => {
let repoDir: string;
let outsideDir: string;
beforeAll(async () => {
repoDir = await realpath(await mkdtemp(join(tmpdir(), 'boocode-symlink-repo-')));
outsideDir = await realpath(await mkdtemp(join(tmpdir(), 'boocode-symlink-outside-')));
await writeFile(join(outsideDir, 'secret.ts'), 'secret data\n');
// Symlink inside repo pointing to outside dir
await symlink(outsideDir, join(repoDir, 'evil'));
});
afterAll(async () => {
await rm(repoDir, { recursive: true, force: true });
await rm(outsideDir, { recursive: true, force: true });
});
it('rejects a path that escapes via a directory symlink', async () => {
await expect(checkSymlinkEscape(repoDir, 'evil/secret.ts')).rejects.toThrow(GitWriteError);
});
it('rejects a path that resolves to the symlink itself (outside)', async () => {
await expect(checkSymlinkEscape(repoDir, 'evil')).rejects.toThrow(GitWriteError);
});
it('accepts a path that resolves within the repo', async () => {
await writeFile(join(repoDir, 'legit.ts'), 'export {};\n');
await expect(checkSymlinkEscape(repoDir, 'legit.ts')).resolves.toBeUndefined();
});
it('accepts a non-existent path (new file being staged)', async () => {
await expect(checkSymlinkEscape(repoDir, 'brand-new-file-not-yet-created.ts')).resolves.toBeUndefined();
});
});

View File

@@ -0,0 +1,554 @@
import { execFile } from 'node:child_process';
import { promisify } from 'node:util';
import { stat, realpath } from 'node:fs/promises';
import { isAbsolute, join, resolve, sep } from 'node:path';
const execFileAsync = promisify(execFile);
const GIT_TIMEOUT_MS = 30_000;
const GIT_MAX_BUFFER = 10 * 1024 * 1024; // 10MB
const FILE_DIFF_CAP = 512 * 1024; // 512KB per-file display cap
export type GitDiffMode = 'uncommitted' | 'committed';
export type ChangeType = 'added' | 'modified' | 'deleted' | 'renamed' | 'untracked';
export interface GitDiffFile {
path: string;
old_path: string | null;
change_type: ChangeType;
added_lines: number;
removed_lines: number;
staged: boolean;
diff_body: string | null; // null when is_binary or is_too_large
is_binary: boolean;
is_too_large: boolean;
}
export interface GitDiffResult {
mode: GitDiffMode;
base_label: string | null;
in_progress_op: string | null;
files: GitDiffFile[];
}
// runGit with 30s deadline and 10MB buffer for diff payloads. Returns null on
// any failure so callers can degrade gracefully without surfacing git errors.
async function runGit(args: string[], cwd: string): Promise<string | null> {
try {
const { stdout } = await execFileAsync('git', args, {
cwd,
timeout: GIT_TIMEOUT_MS,
windowsHide: true,
maxBuffer: GIT_MAX_BUFFER,
});
return stdout.toString();
} catch {
return null;
}
}
// ── Pure helpers (unit-testable without spawning git) ──────────────────────
/** Parses a single `git diff --name-status` output line. Returns null on garbage. */
function parseNameStatusLine(line: string): {
path: string;
old_path: string | null;
change_type: ChangeType;
} | null {
const trimmed = line.trim();
if (!trimmed) return null;
const parts = trimmed.split('\t');
if (parts.length < 2) return null;
const code = parts[0] ?? '';
// Rename: R<score>\told\tnew Copy: C<score>\told\tnew
if (code.startsWith('R') || code.startsWith('C')) {
if (parts.length < 3) return null;
return { path: parts[2] ?? '', old_path: parts[1] ?? null, change_type: 'renamed' };
}
const path = parts[1] ?? '';
if (!path) return null;
switch (code[0]) {
case 'A': return { path, old_path: null, change_type: 'added' };
case 'M':
case 'T': // type changed
case 'U': // unmerged
return { path, old_path: null, change_type: 'modified' };
case 'D': return { path, old_path: null, change_type: 'deleted' };
default: return null;
}
}
/** Parses multi-line `git diff --name-status` output into a file list. */
export function parseNameStatus(output: string): {
path: string;
old_path: string | null;
change_type: ChangeType;
}[] {
return output
.split('\n')
.map((l) => parseNameStatusLine(l))
.filter((x): x is NonNullable<typeof x> => x !== null);
}
/** Parses a single `git diff --numstat` output line. */
export function parseNumStatLine(line: string): {
path: string;
added: number;
removed: number;
binary: boolean;
} | null {
const parts = line.trim().split('\t');
if (parts.length < 3) return null;
const [added, removed, path] = parts;
if (!path) return null;
if (added === '-' && removed === '-') {
return { path, added: 0, removed: 0, binary: true };
}
const a = parseInt(added ?? '', 10);
const r = parseInt(removed ?? '', 10);
if (isNaN(a) || isNaN(r)) return null;
return { path, added: a, removed: r, binary: false };
}
/** Splits a unified diff text into per-file bodies keyed by current path. */
export function splitDiffByFile(diffText: string): Map<string, string> {
const result = new Map<string, string>();
if (!diffText.trim()) return result;
// Split at each "diff --git" header (lookahead keeps the header with its section)
const sections = diffText.split(/(?=^diff --git )/m);
for (const section of sections) {
if (!section.trim()) continue;
// Current path: prefer "+++ b/<path>" (absent for pure renames / deleted files)
const pppMatch = section.match(/^\+{3} b\/(.+)$/m);
if (pppMatch) {
result.set((pppMatch[1] ?? '').trim(), section);
continue;
}
// Deleted file: "--- a/<path>" with "+++ /dev/null"
const mmmMatch = section.match(/^-{3} a\/(.+)$/m);
if (mmmMatch) {
const p = (mmmMatch[1] ?? '').trim();
if (p && p !== '/dev/null') {
result.set(p, section);
continue;
}
}
// Pure rename with no content change: extract from "diff --git a/... b/..."
// Take everything after the last " b/" on that line.
const gitLineMatch = section.match(/^diff --git a\/.+ b\/(.+)$/m);
if (gitLineMatch) {
result.set((gitLineMatch[1] ?? '').trim(), section);
}
}
return result;
}
/** Classifies a diff body segment as diff | binary | too_large. */
export function classifyDiffBody(body: string, cap = FILE_DIFF_CAP): 'diff' | 'binary' | 'too_large' {
if (/^Binary files /m.test(body)) return 'binary';
if (body.length > cap) return 'too_large';
return 'diff';
}
/** Returns the auto-selected diff mode based on dirty state. */
export function autoSelectMode(isDirty: boolean): GitDiffMode {
return isDirty ? 'uncommitted' : 'committed';
}
/** Returns true when at least one file is staged (commit is possible). */
export function canCommit(files: GitDiffFile[]): boolean {
return files.some((f) => f.staged);
}
/** Returns true when the working tree has uncommitted changes (staged or unstaged). */
export async function isRepoDirty(cwd: string): Promise<boolean> {
const gitRoot = await resolveGitRoot(cwd);
if (!gitRoot) return false;
const out = await runGit(['status', '--porcelain'], gitRoot);
if (out === null) return true; // can't determine — assume dirty
return out.trim().length > 0;
}
/**
* Async per-file symlink-escape guard (FIX 3 / D-4). Resolves the real path of
* the target (if it already exists on disk) and rejects when it falls outside
* the repo root. Non-existent paths (new files being staged) are allowed — there
* is no symlink to follow when the file hasn't been created yet.
*/
export async function checkSymlinkEscape(repoRoot: string, filePath: string): Promise<void> {
const resolved = resolve(repoRoot, filePath);
let real: string;
try {
real = await realpath(resolved);
} catch {
// File doesn't exist yet — no symlink to resolve, safe to proceed.
return;
}
if (real !== repoRoot && !real.startsWith(repoRoot + sep)) {
throw new GitWriteError(`path escapes repository root via symlink: ${filePath}`, false);
}
}
// ── Async helpers ──────────────────────────────────────────────────────────
/** Resolves the base ref for Committed mode with fallback chain. */
export async function resolveCommittedBase(
cwd: string,
): Promise<{ base: string | null; label: string }> {
// 1. Tracking branch (@{upstream})
const upstream = await runGit(
['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}'],
cwd,
);
if (upstream !== null) {
const trimmed = upstream.trim();
if (trimmed && !trimmed.includes('fatal')) {
return { base: trimmed, label: trimmed };
}
}
// 2. origin/HEAD (default branch)
const originHead = await runGit(['rev-parse', '--abbrev-ref', 'origin/HEAD'], cwd);
if (originHead !== null) {
const trimmed = originHead.trim();
if (trimmed && !trimmed.includes('fatal') && !trimmed.includes('unknown')) {
return { base: trimmed, label: trimmed };
}
}
return { base: null, label: 'uncommitted (no base found)' };
}
/** Detects in-progress git operations via .git sentinel files/dirs. */
export async function detectInProgress(repoRoot: string): Promise<string | null> {
const fileChecks: [string, string][] = [
['MERGE_HEAD', 'merge'],
['CHERRY_PICK_HEAD', 'cherry-pick'],
['BISECT_LOG', 'bisect'],
];
for (const [file, op] of fileChecks) {
try {
await stat(join(repoRoot, '.git', file));
return op;
} catch {
// sentinel not present — continue
}
}
for (const dir of ['rebase-merge', 'rebase-apply']) {
try {
await stat(join(repoRoot, '.git', dir));
return 'rebase';
} catch {
// not present — continue
}
}
return null;
}
// ── Read logic ─────────────────────────────────────────────────────────────
/** Resolves the git work-tree root for the given path. Returns null if not a repo. */
async function resolveGitRoot(cwd: string): Promise<string | null> {
const out = await runGit(['rev-parse', '--show-toplevel'], cwd);
return out !== null ? out.trim() : null;
}
function buildNumstatMap(
output: string,
): Map<string, { added: number; removed: number; binary: boolean }> {
const map = new Map<string, { added: number; removed: number; binary: boolean }>();
for (const line of output.split('\n')) {
const parsed = parseNumStatLine(line);
if (parsed) map.set(parsed.path, { added: parsed.added, removed: parsed.removed, binary: parsed.binary });
}
return map;
}
async function getUncommittedDiff(
gitRoot: string,
inProgress: string | null,
): Promise<GitDiffResult> {
const hasCommits = (await runGit(['rev-parse', '--verify', 'HEAD'], gitRoot)) !== null;
const [nameStatusOut, cachedNameStatusOut, untrackedOut, numstatOut, diffOut, cachedDiffOut] =
await Promise.all([
hasCommits
? runGit(['diff', '--name-status', 'HEAD'], gitRoot)
: Promise.resolve(''),
hasCommits
? runGit(['diff', '--cached', '--name-status', 'HEAD'], gitRoot)
: runGit(['diff', '--cached', '--name-status'], gitRoot),
runGit(['ls-files', '--others', '--exclude-standard'], gitRoot),
hasCommits ? runGit(['diff', '--numstat', 'HEAD'], gitRoot) : Promise.resolve(''),
hasCommits ? runGit(['diff', 'HEAD'], gitRoot) : Promise.resolve(''),
hasCommits
? runGit(['diff', '--cached', 'HEAD'], gitRoot)
: runGit(['diff', '--cached'], gitRoot),
]);
const allChanged = parseNameStatus(nameStatusOut ?? '');
const stagedSet = new Set(
parseNameStatus(cachedNameStatusOut ?? '').map((f) => f.path),
);
const untracked = (untrackedOut ?? '').split('\n').filter(Boolean);
const numstatMap = buildNumstatMap(numstatOut ?? '');
// Merge unstaged and staged diff maps
const diffMap = splitDiffByFile(diffOut ?? '');
const cachedDiffMap = splitDiffByFile(cachedDiffOut ?? '');
// Staged-only files won't be in diffOut; supplement from cachedDiffMap
for (const [k, v] of cachedDiffMap) {
if (!diffMap.has(k)) diffMap.set(k, v);
}
const files: GitDiffFile[] = [];
for (const entry of allChanged) {
const ns = numstatMap.get(entry.path);
const body = diffMap.get(entry.path) ?? null;
const kind = body !== null ? classifyDiffBody(body) : ns?.binary ? 'binary' : 'diff';
files.push({
path: entry.path,
old_path: entry.old_path,
change_type: entry.change_type,
added_lines: ns?.added ?? 0,
removed_lines: ns?.removed ?? 0,
staged: stagedSet.has(entry.path),
diff_body: kind === 'diff' ? body : null,
is_binary: kind === 'binary',
is_too_large: kind === 'too_large',
});
}
for (const p of untracked) {
files.push({
path: p,
old_path: null,
change_type: 'untracked',
added_lines: 0,
removed_lines: 0,
staged: false,
diff_body: null,
is_binary: false,
is_too_large: false,
});
}
return { mode: 'uncommitted', base_label: null, in_progress_op: inProgress, files };
}
async function getCommittedDiff(
gitRoot: string,
base: string,
label: string,
inProgress: string | null,
): Promise<GitDiffResult> {
const [nameStatusOut, numstatOut, diffOut] = await Promise.all([
runGit(['diff', '--name-status', base, 'HEAD'], gitRoot),
runGit(['diff', '--numstat', base, 'HEAD'], gitRoot),
runGit(['diff', base, 'HEAD'], gitRoot),
]);
const allChanged = parseNameStatus(nameStatusOut ?? '');
const numstatMap = buildNumstatMap(numstatOut ?? '');
const diffMap = splitDiffByFile(diffOut ?? '');
const files: GitDiffFile[] = allChanged.map((entry) => {
const ns = numstatMap.get(entry.path);
const body = diffMap.get(entry.path) ?? null;
const kind = body !== null ? classifyDiffBody(body) : ns?.binary ? 'binary' : 'diff';
return {
path: entry.path,
old_path: entry.old_path,
change_type: entry.change_type,
added_lines: ns?.added ?? 0,
removed_lines: ns?.removed ?? 0,
staged: false, // staged concept does not apply in committed mode
diff_body: kind === 'diff' ? body : null,
is_binary: kind === 'binary',
is_too_large: kind === 'too_large',
};
});
return { mode: 'committed', base_label: label, in_progress_op: inProgress, files };
}
/**
* Returns the structured git diff for the given directory and mode, or null if
* the directory is not a git repository. On a null committed-mode base, falls
* back to uncommitted and labels the result accordingly.
*/
export async function getGitDiff(cwd: string, mode: GitDiffMode): Promise<GitDiffResult | null> {
const gitRoot = await resolveGitRoot(cwd);
if (!gitRoot) return null;
const inProgress = await detectInProgress(gitRoot);
if (mode === 'uncommitted') {
return getUncommittedDiff(gitRoot, inProgress);
}
const { base, label } = await resolveCommittedBase(gitRoot);
if (!base) {
// Fall back to uncommitted with a descriptive label
const result = await getUncommittedDiff(gitRoot, inProgress);
return { ...result, base_label: label };
}
return getCommittedDiff(gitRoot, base, label, inProgress);
}
// ── Phase 2: Write helpers ─────────────────────────────────────────────────
// Fallback identity matching project_bootstrap.ts constants.
const GIT_USER_NAME = 'indifferentketchup';
const GIT_USER_EMAIL = 'samkintop@gmail.com';
export class GitWriteError extends Error {
constructor(
message: string,
public readonly busy: boolean,
) {
super(message);
this.name = 'GitWriteError';
}
}
/**
* Validates a per-file path argument for write operations.
* Rejects flag injection (leading `-`), repo-root discard (`.`), absolute
* paths, and `..` traversal without requiring the file to exist on disk.
*/
export function validateWritePath(repoRoot: string, filePath: string): void {
if (!filePath || typeof filePath !== 'string' || filePath.trim() === '') {
throw new GitWriteError('path is required', false);
}
if (filePath.startsWith('-')) {
throw new GitWriteError(`invalid path (flag injection): ${filePath}`, false);
}
if (filePath === '.') {
throw new GitWriteError('cannot operate on repository root (.)', false);
}
if (isAbsolute(filePath)) {
throw new GitWriteError(`path must be relative: ${filePath}`, false);
}
const resolved = resolve(repoRoot, filePath);
if (resolved === repoRoot || !resolved.startsWith(repoRoot + sep)) {
throw new GitWriteError(`path escapes repository root: ${filePath}`, false);
}
}
/** Reads git config user.name/email, falling back to bootstrap constants. */
export async function deriveCommitIdentity(
repoRoot: string,
): Promise<{ name: string; email: string }> {
const [nameOut, emailOut] = await Promise.all([
runGit(['config', 'user.name'], repoRoot),
runGit(['config', 'user.email'], repoRoot),
]);
return {
name: nameOut?.trim() || GIT_USER_NAME,
email: emailOut?.trim() || GIT_USER_EMAIL,
};
}
/** Runs a git write operation, propagating errors. Throws GitWriteError. */
async function runGitWrite(args: string[], cwd: string): Promise<void> {
try {
await execFileAsync('git', args, { cwd, timeout: GIT_TIMEOUT_MS, windowsHide: true });
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
const busy = msg.includes('index.lock') || msg.includes('Another git process');
throw new GitWriteError(busy ? 'repository is busy, try again' : msg, busy);
}
}
/** Stages the given files (`git add -- <files>`). */
export async function stageFiles(repoRoot: string, files: string[]): Promise<void> {
for (const f of files) {
validateWritePath(repoRoot, f);
await checkSymlinkEscape(repoRoot, f);
}
await runGitWrite(['add', '--', ...files], repoRoot);
}
/** Unstages the given files (`git restore --staged -- <files>`). */
export async function unstageFiles(repoRoot: string, files: string[]): Promise<void> {
for (const f of files) {
validateWritePath(repoRoot, f);
await checkSymlinkEscape(repoRoot, f);
}
await runGitWrite(['restore', '--staged', '--', ...files], repoRoot);
}
/** Commits staged files with a server-derived identity. */
export async function commitFiles(
repoRoot: string,
message: string,
files?: string[],
): Promise<void> {
if (files && files.length > 0) {
for (const f of files) {
validateWritePath(repoRoot, f);
await checkSymlinkEscape(repoRoot, f);
}
}
const id = await deriveCommitIdentity(repoRoot);
const args = ['-c', `user.name=${id.name}`, '-c', `user.email=${id.email}`, 'commit', '-m', message];
if (files && files.length > 0) args.push('--', ...files);
await runGitWrite(args, repoRoot);
}
export interface DiscardFileInfo {
path: string;
change_type: string;
staged: boolean;
}
/**
* Discards changes for the given files.
* - Untracked files: `git clean -f -- <path>`
* - Staged additions (new file staged, no HEAD version): unstage then clean
* - All other tracked files: `git restore HEAD -- <path>` (undoes staged + unstaged)
*/
export async function discardFiles(repoRoot: string, files: DiscardFileInfo[]): Promise<void> {
for (const { path } of files) {
validateWritePath(repoRoot, path);
await checkSymlinkEscape(repoRoot, path);
}
const untracked: string[] = [];
const stagedAdditions: string[] = [];
const tracked: string[] = [];
for (const f of files) {
if (f.change_type === 'untracked') {
untracked.push(f.path);
} else if (f.change_type === 'added' && f.staged) {
stagedAdditions.push(f.path);
} else {
tracked.push(f.path);
}
}
// Restore tracked files from HEAD (handles staged + unstaged modifications/deletions).
// git checkout HEAD -- <file> is the most portable form: resets index + worktree.
if (tracked.length > 0) {
await runGitWrite(['checkout', 'HEAD', '--', ...tracked], repoRoot);
}
// Staged additions: unstage first, then remove from working tree.
for (const p of stagedAdditions) {
await runGitWrite(['restore', '--staged', '--', p], repoRoot);
await runGitWrite(['clean', '-f', '--', p], repoRoot);
}
// Untracked files: clean (hard delete).
if (untracked.length > 0) {
await runGitWrite(['clean', '-f', '--', ...untracked], repoRoot);
}
}

View File

@@ -10,6 +10,9 @@ import type {
ViewFileResult, ViewFileResult,
AgentsResponse, AgentsResponse,
GitMeta, GitMeta,
GitDiffMode,
GitDiffResult,
GitDiscardFileInfo,
Skill, Skill,
ToolCostStat, ToolCostStat,
ProviderSnapshotEntry, ProviderSnapshotEntry,
@@ -151,6 +154,32 @@ export const api = {
request<{ files: string[] }>(`/api/projects/${id}/files`), request<{ files: string[] }>(`/api/projects/${id}/files`),
git: (id: string) => git: (id: string) =>
request<GitMeta>(`/api/projects/${id}/git`), request<GitMeta>(`/api/projects/${id}/git`),
gitDiff: (id: string, mode: GitDiffMode | null) =>
request<GitDiffResult>(
mode !== null
? `/api/projects/${id}/git/diff?mode=${mode}`
: `/api/projects/${id}/git/diff`,
),
gitStage: (id: string, files: string[]) =>
request<{ ok: boolean }>(`/api/projects/${id}/git/stage`, {
method: 'POST',
body: JSON.stringify({ files }),
}),
gitUnstage: (id: string, files: string[]) =>
request<{ ok: boolean }>(`/api/projects/${id}/git/unstage`, {
method: 'POST',
body: JSON.stringify({ files }),
}),
gitCommit: (id: string, body: { message: string; files?: string[] }) =>
request<{ ok: boolean }>(`/api/projects/${id}/git/commit`, {
method: 'POST',
body: JSON.stringify(body),
}),
gitDiscard: (id: string, files: GitDiscardFileInfo[]) =>
request<{ ok: boolean }>(`/api/projects/${id}/git/discard`, {
method: 'POST',
body: JSON.stringify({ files }),
}),
}, },
sessions: { sessions: {

View File

@@ -312,6 +312,39 @@ export interface GitMeta {
behind: number; behind: number;
} }
// git-diff-panel Phase 1: shapes returned by GET /api/projects/:id/git/diff.
export type GitDiffMode = 'uncommitted' | 'committed';
export type GitDiffChangeType = 'added' | 'modified' | 'deleted' | 'renamed' | 'untracked';
export interface GitDiffFile {
path: string;
old_path: string | null;
change_type: GitDiffChangeType;
added_lines: number;
removed_lines: number;
staged: boolean;
diff_body: string | null;
is_binary: boolean;
is_too_large: boolean;
}
export interface GitDiffResult {
git_repo: boolean;
mode: GitDiffMode;
/** Server-computed mode based on dirty state — used for auto-select (FIX 1) and mode suggestion (FIX 4). */
auto_mode?: GitDiffMode;
base_label: string | null;
in_progress_op: string | null;
files: GitDiffFile[];
}
// git-diff-panel Phase 2: per-file info for the discard endpoint.
export interface GitDiscardFileInfo {
path: string;
change_type: GitDiffChangeType;
staged: boolean;
}
// Batch 9.6: skill catalog row. Returned by GET /api/skills and consumed by // Batch 9.6: skill catalog row. Returned by GET /api/skills and consumed by
// the slash-command dropdown. `path` and `mtime` are exposed for debug surface // the slash-command dropdown. `path` and `mtime` are exposed for debug surface
// (/api/skills) but the dropdown only renders name + description. // (/api/skills) but the dropdown only renders name + description.

View File

@@ -0,0 +1,493 @@
import { useEffect, useRef, useState } from 'react';
import { ChevronDown, ChevronRight, GitBranch, RefreshCw, Trash2 } from 'lucide-react';
import { codeToHtml } from 'shiki';
import type { GitDiffFile, GitDiffMode, GitDiffResult, GitDiscardFileInfo } from '@/api/types';
import { cn } from '@/lib/utils';
interface WriteProps {
mutating: boolean;
mutateError: string | null;
onStage: (files: string[]) => Promise<boolean>;
onUnstage: (files: string[]) => Promise<boolean>;
onCommit: (message: string, files?: string[]) => Promise<boolean>;
onDiscard: (files: GitDiscardFileInfo[]) => Promise<boolean>;
}
interface Props extends WriteProps {
result: GitDiffResult | null;
loading: boolean;
error: string | null;
mode: GitDiffMode;
onSelectMode: (m: GitDiffMode) => void;
onRefresh: () => void;
/** FIX 4: non-null when the repo's dirty state suggests a different mode than the pinned one. */
modeSuggestion?: GitDiffMode | null;
/** FIX 5: pending-changes count from the Coder pane — shown in empty state as a hint. */
pendingCount?: number;
}
const CHANGE_TYPE_LABELS: Record<string, string> = {
added: 'A',
modified: 'M',
deleted: 'D',
renamed: 'R',
untracked: '?',
};
const CHANGE_TYPE_COLORS: Record<string, string> = {
added: 'text-green-500',
modified: 'text-yellow-500',
deleted: 'text-red-500',
renamed: 'text-blue-500',
untracked: 'text-muted-foreground',
};
interface DiscardConfirmState {
file: GitDiffFile;
}
function DiscardConfirmDialog({
state,
onConfirm,
onCancel,
}: {
state: DiscardConfirmState;
onConfirm: () => void;
onCancel: () => void;
}) {
const isUntracked = state.file.change_type === 'untracked';
return (
<div
role="dialog"
aria-modal="true"
className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 px-4"
>
<div className="bg-popover border rounded-lg shadow-lg max-w-sm w-full p-4 flex flex-col gap-3">
<p className="text-sm font-medium">
{isUntracked ? 'Permanently delete file?' : 'Discard changes?'}
</p>
<p className="text-xs text-muted-foreground">
{isUntracked
? `${state.file.path} will be permanently deleted. This cannot be undone.`
: `Changes to ${state.file.path} will be reverted to the last commit. This cannot be undone.`}
</p>
<div className="flex gap-2 justify-end">
<button
type="button"
onClick={onCancel}
className="text-xs px-3 py-1.5 rounded border hover:bg-muted max-md:min-h-[44px] max-md:min-w-[44px]"
>
Cancel
</button>
<button
type="button"
onClick={onConfirm}
className="text-xs px-3 py-1.5 rounded bg-destructive text-destructive-foreground hover:bg-destructive/90 max-md:min-h-[44px] max-md:min-w-[44px]"
>
{isUntracked ? 'Delete' : 'Discard'}
</button>
</div>
</div>
</div>
);
}
function FileDiffRow({
file,
uncommitted,
disabled,
onStage,
onUnstage,
onDiscardRequest,
}: {
file: GitDiffFile;
uncommitted: boolean;
disabled: boolean;
onStage: (path: string) => void;
onUnstage: (path: string) => void;
onDiscardRequest: (file: GitDiffFile) => void;
}) {
const [expanded, setExpanded] = useState(false);
const [html, setHtml] = useState<string | null>(null);
const [highlighting, setHighlighting] = useState(false);
const highlightRef = useRef<HTMLDivElement | null>(null);
useEffect(() => {
if (!expanded || !file.diff_body) return;
if (html !== null) return;
let cancelled = false;
setHighlighting(true);
void codeToHtml(file.diff_body, { lang: 'diff', theme: 'github-dark' })
.then((result) => { if (!cancelled) setHtml(result); })
.catch(() => { if (!cancelled) setHtml(null); })
.finally(() => { if (!cancelled) setHighlighting(false); });
return () => { cancelled = true; };
}, [expanded, file.diff_body, html]);
useEffect(() => {
if (highlightRef.current && html !== null) {
// Shiki generates sanitized HTML — not user-supplied content.
// eslint-disable-next-line no-unsanitized/property
highlightRef.current.innerHTML = html;
}
}, [html]);
const typeLabel = CHANGE_TYPE_LABELS[file.change_type] ?? '?';
const typeColor = CHANGE_TYPE_COLORS[file.change_type] ?? 'text-muted-foreground';
const displayPath = file.old_path ? `${file.old_path}${file.path}` : file.path;
return (
<li className="border-b border-border/30 last:border-0">
<div className="flex items-center group">
<button
type="button"
className="flex-1 flex items-center gap-1.5 px-2 py-1.5 text-xs hover:bg-muted/40 text-left max-md:min-h-[44px] min-w-0"
onClick={() => setExpanded((p) => !p)}
aria-expanded={expanded}
>
{expanded
? <ChevronDown size={10} className="shrink-0 text-muted-foreground" />
: <ChevronRight size={10} className="shrink-0 text-muted-foreground" />}
<span className={cn('font-mono font-bold w-3 shrink-0', typeColor)}>{typeLabel}</span>
<span className="truncate flex-1">{displayPath}</span>
{(file.added_lines > 0 || file.removed_lines > 0) && (
<span className="shrink-0 text-muted-foreground/70 font-mono text-[10px]">
{file.added_lines > 0 && <span className="text-green-500">+{file.added_lines}</span>}
{file.added_lines > 0 && file.removed_lines > 0 && <span className="mx-0.5">/</span>}
{file.removed_lines > 0 && <span className="text-red-500">-{file.removed_lines}</span>}
</span>
)}
{file.staged && (
<span className="shrink-0 text-[10px] bg-blue-500/15 text-blue-400 px-1 rounded">staged</span>
)}
</button>
{/* Write affordances — Uncommitted mode only */}
{uncommitted && (
<div className="flex items-center gap-0.5 px-1 shrink-0">
{/* Stage / Unstage toggle */}
{file.change_type !== 'deleted' && (
<button
type="button"
disabled={disabled}
onClick={() => file.staged ? onUnstage(file.path) : onStage(file.path)}
className="text-[10px] px-1.5 py-0.5 rounded border border-border/50 hover:bg-muted disabled:opacity-40 max-md:min-h-[44px] max-md:min-w-[44px]"
title={file.staged ? 'Unstage' : 'Stage'}
>
{file.staged ? '' : '+'}
</button>
)}
{/* Discard — separated secondary affordance */}
<button
type="button"
disabled={disabled}
onClick={() => onDiscardRequest(file)}
className="p-1 rounded hover:bg-destructive/15 hover:text-destructive text-muted-foreground/50 disabled:opacity-40 max-md:min-h-[44px] max-md:min-w-[44px]"
title={file.change_type === 'untracked' ? 'Delete file' : 'Discard changes'}
>
<Trash2 size={10} />
</button>
</div>
)}
</div>
{expanded && (
<div className="px-2 pb-2">
{file.is_binary && (
<p className="text-xs text-muted-foreground italic px-2 py-1">Binary file</p>
)}
{file.is_too_large && (
<p className="text-xs text-muted-foreground italic px-2 py-1">Diff too large to display</p>
)}
{file.change_type === 'untracked' && (
<p className="text-xs text-muted-foreground italic px-2 py-1">Untracked not yet staged</p>
)}
{!file.is_binary && !file.is_too_large && file.diff_body && (
<>
{highlighting && (
<p className="text-xs text-muted-foreground px-2 py-1">Highlighting</p>
)}
{!highlighting && html !== null ? (
<div
ref={highlightRef}
className="text-[11px] overflow-x-auto rounded bg-[#0d1117] [&_pre]:!p-2 [&_pre]:!m-0 [&_pre]:overflow-x-auto"
/>
) : (
!highlighting && (
<pre className="text-[11px] overflow-x-auto rounded bg-muted/30 p-2 whitespace-pre">
{file.diff_body}
</pre>
)
)}
</>
)}
</div>
)}
</li>
);
}
export function GitDiffView({
result,
loading,
error,
mode,
onSelectMode,
onRefresh,
mutating,
mutateError,
onStage,
onUnstage,
onCommit,
onDiscard,
modeSuggestion,
pendingCount,
}: Props) {
const [commitMessage, setCommitMessage] = useState('');
const [discardTarget, setDiscardTarget] = useState<DiscardConfirmState | null>(null);
const [lastAction, setLastAction] = useState<string | null>(null);
const lastActionTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
function flashAction(msg: string) {
setLastAction(msg);
if (lastActionTimer.current) clearTimeout(lastActionTimer.current);
lastActionTimer.current = setTimeout(() => setLastAction(null), 2000);
}
const uncommitted = mode === 'uncommitted';
const inProgress = result?.in_progress_op ?? null;
const writeDisabled = mutating || !!inProgress;
const stagedFiles = result?.files.filter((f) => f.staged) ?? [];
const canDoCommit = uncommitted && stagedFiles.length > 0 && commitMessage.trim().length > 0 && !writeDisabled;
async function handleStage(path: string) {
const ok = await onStage([path]);
if (ok) flashAction('Staged');
}
async function handleUnstage(path: string) {
const ok = await onUnstage([path]);
if (ok) flashAction('Unstaged');
}
function handleDiscardRequest(file: GitDiffFile) {
setDiscardTarget({ file });
}
async function handleDiscardConfirm() {
if (!discardTarget) return;
const { file } = discardTarget;
setDiscardTarget(null);
const info: GitDiscardFileInfo = {
path: file.path,
change_type: file.change_type,
staged: file.staged,
};
const ok = await onDiscard([info]);
if (ok) flashAction(file.change_type === 'untracked' ? 'Deleted' : 'Discarded');
}
async function handleCommit() {
const msg = commitMessage.trim();
if (!msg) return;
const ok = await onCommit(msg);
if (ok) {
setCommitMessage('');
flashAction('Committed');
}
}
if (loading && !result) {
return (
<div className="flex-1 flex items-center justify-center text-xs text-muted-foreground">
Loading diff
</div>
);
}
if (error) {
return (
<div className="flex-1 flex flex-col items-center justify-center gap-2 px-4 text-center">
<p className="text-xs text-destructive">{error}</p>
<button
type="button"
onClick={onRefresh}
className="text-xs text-muted-foreground hover:text-foreground flex items-center gap-1 max-md:min-h-[44px]"
>
<RefreshCw size={12} />
Refresh
</button>
</div>
);
}
if (!result || !result.git_repo) {
return (
<div className="flex-1 flex items-center justify-center text-xs text-muted-foreground px-4 text-center">
Not a git repository
</div>
);
}
const { files, base_label } = result;
return (
<div className="flex flex-col flex-1 overflow-hidden">
{/* Mode selector */}
<div className="flex items-center gap-1 px-2 py-1.5 border-b shrink-0">
<button
type="button"
onClick={() => onSelectMode('uncommitted')}
className={cn(
'text-xs px-2 py-0.5 rounded max-md:min-h-[44px]',
mode === 'uncommitted'
? 'bg-muted text-foreground font-medium'
: 'text-muted-foreground hover:text-foreground',
)}
>
Uncommitted
</button>
<button
type="button"
onClick={() => onSelectMode('committed')}
className={cn(
'text-xs px-2 py-0.5 rounded max-md:min-h-[44px]',
mode === 'committed'
? 'bg-muted text-foreground font-medium'
: 'text-muted-foreground hover:text-foreground',
)}
>
Committed
</button>
<div className="flex-1" />
{(loading || mutating) && (
<span className="text-[10px] text-muted-foreground">{mutating ? 'Working…' : 'Refreshing…'}</span>
)}
{lastAction && !mutating && (
<span className="text-[10px] text-green-500">{lastAction}</span>
)}
<button
type="button"
onClick={onRefresh}
disabled={loading || mutating}
className="p-1 rounded hover:bg-muted text-muted-foreground disabled:opacity-40 max-md:min-h-[44px] max-md:min-w-[44px]"
aria-label="Refresh diff"
title="Refresh"
>
<RefreshCw size={12} />
</button>
</div>
{/* Committed-mode base label */}
{result.mode === 'committed' && base_label && (
<div className="px-2 py-1 text-[10px] text-muted-foreground border-b flex items-center gap-1 shrink-0">
<GitBranch size={10} />
<span className="truncate">vs {base_label}</span>
</div>
)}
{/* FIX 2: Fallback label — committed was requested but no base branch found */}
{result.mode === 'uncommitted' && result.base_label && (
<div className="px-2 py-1 text-[10px] text-amber-600 dark:text-amber-400 border-b flex items-center gap-1 shrink-0">
<GitBranch size={10} />
<span className="truncate">{result.base_label}</span>
</div>
)}
{/* FIX 4: Mode suggestion — shown when pinned mode diverges from auto-selected mode */}
{modeSuggestion && (
<div className="px-2 py-1 text-[10px] text-muted-foreground border-b shrink-0 flex items-center gap-1">
<span>Repo is now {modeSuggestion === 'uncommitted' ? 'dirty' : 'clean'} </span>
<button
type="button"
onClick={() => onSelectMode(modeSuggestion)}
className="underline hover:text-foreground"
>
switch to {modeSuggestion === 'uncommitted' ? 'Uncommitted' : 'Committed'}
</button>
</div>
)}
{/* In-progress op banner */}
{inProgress && (
<div className="px-2 py-1 text-[10px] text-yellow-500 bg-yellow-500/10 border-b shrink-0">
{inProgress} in progress write actions disabled
</div>
)}
{/* Mutation error */}
{mutateError && (
<div className="px-2 py-1 text-[10px] text-destructive bg-destructive/10 border-b shrink-0 truncate">
{mutateError}
</div>
)}
{/* File list */}
<div className="flex-1 overflow-y-auto">
{files.length === 0 ? (
<div className="flex flex-col items-center justify-center px-4 py-8 text-xs text-muted-foreground text-center gap-1.5">
<span>{mode === 'uncommitted' ? 'No uncommitted changes' : 'No changes vs. the base branch'}</span>
{/* FIX 5: hint when pending changes exist in the Coder pane */}
{!!pendingCount && (
<span className="text-[10px]">
{pendingCount} pending {pendingCount === 1 ? 'change' : 'changes'} visible in the Coder pane
</span>
)}
</div>
) : (
<ul className="list-none">
{files.map((file) => (
<FileDiffRow
key={file.path}
file={file}
uncommitted={uncommitted}
disabled={writeDisabled}
onStage={handleStage}
onUnstage={handleUnstage}
onDiscardRequest={handleDiscardRequest}
/>
))}
</ul>
)}
</div>
{/* Commit panel — Uncommitted mode only */}
{uncommitted && (
<div className="shrink-0 border-t px-2 py-2 flex flex-col gap-1.5">
<textarea
value={commitMessage}
onChange={(e) => setCommitMessage(e.target.value)}
disabled={writeDisabled}
placeholder="Commit message…"
rows={2}
className="w-full text-xs rounded border bg-background px-2 py-1 resize-none focus:outline-none focus:ring-1 focus:ring-ring disabled:opacity-40 placeholder:text-muted-foreground"
/>
<div className="flex items-center gap-1.5">
<span className="text-[10px] text-muted-foreground flex-1">
{stagedFiles.length > 0
? `${stagedFiles.length} file${stagedFiles.length > 1 ? 's' : ''} staged`
: 'No files staged'}
</span>
<button
type="button"
disabled={!canDoCommit}
onClick={handleCommit}
className="text-xs px-3 py-1 rounded bg-primary text-primary-foreground hover:bg-primary/90 disabled:opacity-40 max-md:min-h-[44px]"
>
Commit
</button>
</div>
</div>
)}
{/* Discard confirmation dialog */}
{discardTarget && (
<DiscardConfirmDialog
state={discardTarget}
onConfirm={handleDiscardConfirm}
onCancel={() => setDiscardTarget(null)}
/>
)}
</div>
);
}

View File

@@ -6,7 +6,10 @@ import { inferLanguage } from '@/lib/attachments';
import { sessionEvents } from '@/hooks/sessionEvents'; import { sessionEvents } from '@/hooks/sessionEvents';
import { useRightRailDrawer } from '@/hooks/useRightRailDrawer'; import { useRightRailDrawer } from '@/hooks/useRightRailDrawer';
import { useViewport } from '@/hooks/useViewport'; import { useViewport } from '@/hooks/useViewport';
import { useProjectGit } from '@/hooks/useProjectGit';
import { useGitDiff } from '@/hooks/useGitDiff';
import { FileViewerOverlay } from '@/components/FileViewerOverlay'; import { FileViewerOverlay } from '@/components/FileViewerOverlay';
import { GitDiffView } from '@/components/GitDiffView';
import { Input } from '@/components/ui/input'; import { Input } from '@/components/ui/input';
import { Textarea } from '@/components/ui/textarea'; import { Textarea } from '@/components/ui/textarea';
import { Button } from '@/components/ui/button'; import { Button } from '@/components/ui/button';
@@ -21,6 +24,8 @@ import {
} from '@/components/ui/dialog'; } from '@/components/ui/dialog';
import { cn } from '@/lib/utils'; import { cn } from '@/lib/utils';
type RailTab = 'files' | 'git';
interface Props { interface Props {
projectId: string; projectId: string;
sessionId: string; sessionId: string;
@@ -45,12 +50,38 @@ export function RightRail({ projectId, sessionId }: Props) {
const [open, setOpen] = useState(() => { const [open, setOpen] = useState(() => {
try { return localStorage.getItem(`${STORAGE_KEY}.open`) !== 'false'; } catch { return true; } try { return localStorage.getItem(`${STORAGE_KEY}.open`) !== 'false'; } catch { return true; }
}); });
const [tab, setTab] = useState<RailTab>('files');
const [filter, setFilter] = useState(''); const [filter, setFilter] = useState('');
const [expandedDirs, setExpandedDirs] = useState<Set<string>>(new Set()); const [expandedDirs, setExpandedDirs] = useState<Set<string>>(new Set());
const [cache, setCache] = useState<Map<string, FileEntry[]>>(new Map()); const [cache, setCache] = useState<Map<string, FileEntry[]>>(new Map());
const [fullFileList, setFullFileList] = useState<string[] | null>(null); const [fullFileList, setFullFileList] = useState<string[] | null>(null);
const [viewerFile, setViewerFile] = useState<{ path: string; content: string } | null>(null); const [viewerFile, setViewerFile] = useState<{ path: string; content: string } | null>(null);
// Git metadata: dirty dot on the Git tab (no new fetch — reuses the 30s poll).
const git = useProjectGit(projectId);
const isDirty = git?.is_dirty ?? false;
// Git diff view state (Phase 2: includes write callbacks).
const { result: gitDiff, loading: gitLoading, error: gitError, mode: gitMode, selectMode, refresh: refreshDiff, mutating: gitMutating, mutateError: gitMutateError, stage: gitStage, unstage: gitUnstage, commit: gitCommit, discard: gitDiscard, modeSuggestion: gitModeSuggestion } = useGitDiff(projectId);
const showGitTab = gitDiff === null || gitDiff.git_repo;
// FIX 5: pending-changes count — fetched when git tab is active so the empty state
// can hint that unapplied pending changes exist in the Coder pane.
const [pendingCount, setPendingCount] = useState(0);
useEffect(() => {
if (tab !== 'git') return;
const check = () => {
fetch(`/api/coder/sessions/${sessionId}/pending`)
.then((r) => r.ok ? r.json() as Promise<Array<{ status: string }>> : [])
.then((data) => setPendingCount(data.filter((c) => c.status === 'pending').length))
.catch(() => {});
};
check();
return sessionEvents.subscribe((e) => {
if (e.type === 'git_diff_refresh') check();
});
}, [tab, sessionId]);
// New-file-from-pasted-text modal. Queues a pending_changes create via // New-file-from-pasted-text modal. Queues a pending_changes create via
// BooCoder; it then shows in the CoderPane DiffPanel for explicit apply. // BooCoder; it then shows in the CoderPane DiffPanel for explicit apply.
const [newFileOpen, setNewFileOpen] = useState(false); const [newFileOpen, setNewFileOpen] = useState(false);
@@ -167,6 +198,11 @@ export function RightRail({ projectId, sessionId }: Props) {
return []; return [];
}, [filterActive, trimmed, fullFileList]); }, [filterActive, trimmed, fullFileList]);
// Trigger a git diff refresh whenever the Git tab becomes active.
useEffect(() => {
if (tab === 'git') sessionEvents.emit({ type: 'git_diff_refresh' });
}, [tab]);
// Listen for open_file_in_browser events // Listen for open_file_in_browser events
useEffect(() => { useEffect(() => {
return sessionEvents.subscribe((event) => { return sessionEvents.subscribe((event) => {
@@ -206,17 +242,45 @@ export function RightRail({ projectId, sessionId }: Props) {
return ( return (
<> <>
<aside className={asideCls}> <aside className={asideCls}>
<div className="flex items-center gap-2 px-3 py-2 border-b shrink-0"> {/* Header: Files / Git tab strip, FilePlus (Files only), close */}
<span className="text-xs font-medium flex-1">Files</span> <div className="flex items-center gap-1 px-2 py-1.5 border-b shrink-0">
<button <button
type="button" type="button"
onClick={openNewFile} onClick={() => setTab('files')}
className="p-1 rounded hover:bg-muted text-muted-foreground max-md:min-h-[44px] max-md:min-w-[44px]" className={cn(
aria-label="New file from pasted text" 'text-xs px-2 py-0.5 rounded max-md:min-h-[44px]',
title="New file" tab === 'files' ? 'bg-muted text-foreground font-medium' : 'text-muted-foreground hover:text-foreground',
)}
> >
<FilePlus size={14} /> Files
</button> </button>
{showGitTab && (
<button
type="button"
onClick={() => setTab('git')}
className={cn(
'relative text-xs px-2 py-0.5 rounded max-md:min-h-[44px] flex items-center gap-1',
tab === 'git' ? 'bg-muted text-foreground font-medium' : 'text-muted-foreground hover:text-foreground',
)}
>
Git
{isDirty && (
<span className="w-1.5 h-1.5 rounded-full bg-yellow-400 shrink-0" aria-label="dirty" />
)}
</button>
)}
<div className="flex-1" />
{tab === 'files' && (
<button
type="button"
onClick={openNewFile}
className="p-1 rounded hover:bg-muted text-muted-foreground max-md:min-h-[44px] max-md:min-w-[44px]"
aria-label="New file from pasted text"
title="New file"
>
<FilePlus size={14} />
</button>
)}
<button <button
type="button" type="button"
onClick={closeRail} onClick={closeRail}
@@ -226,47 +290,73 @@ export function RightRail({ projectId, sessionId }: Props) {
<PanelRightClose size={14} /> <PanelRightClose size={14} />
</button> </button>
</div> </div>
<div className="px-2 py-1.5 shrink-0">
<Input {/* Files tab content */}
value={filter} {tab === 'files' && (
onChange={(e) => setFilter(e.target.value)} <>
placeholder="Filter files..." <div className="px-2 py-1.5 shrink-0">
className="h-7 text-xs" <Input
value={filter}
onChange={(e) => setFilter(e.target.value)}
placeholder="Filter files..."
className="h-7 text-xs"
/>
</div>
<div className="flex-1 overflow-y-auto px-1 py-1">
{filterActive ? (
filterResults.length > 0 ? (
<ul className="list-none space-y-0.5">
{filterResults.map((r) => (
<li key={r.path}>
<button
type="button"
className="w-full flex items-center gap-1 px-2 py-1 text-xs rounded hover:bg-muted/60 text-left"
onClick={() => void openFile(r.path)}
>
<FileText size={12} className="text-muted-foreground shrink-0" />
<span className="font-bold truncate">{r.name}</span>
<span className="text-muted-foreground ml-1 truncate">{r.path}</span>
</button>
</li>
))}
</ul>
) : (
<div className="text-xs text-muted-foreground px-2 py-4 text-center">No matches</div>
)
) : (
<TreeLevel
parentPath=""
entries={rootEntries}
cache={cache}
expanded={expandedDirs}
depth={0}
onToggleDir={toggleDir}
onSelectFile={(path) => void openFile(path)}
/>
)}
</div>
</>
)}
{/* Git tab content */}
{tab === 'git' && (
<GitDiffView
result={gitDiff}
loading={gitLoading}
error={gitError}
mode={gitMode}
onSelectMode={selectMode}
onRefresh={refreshDiff}
mutating={gitMutating}
mutateError={gitMutateError}
onStage={gitStage}
onUnstage={gitUnstage}
onCommit={gitCommit}
onDiscard={gitDiscard}
modeSuggestion={gitModeSuggestion}
pendingCount={pendingCount}
/> />
</div> )}
<div className="flex-1 overflow-y-auto px-1 py-1">
{filterActive ? (
filterResults.length > 0 ? (
<ul className="list-none space-y-0.5">
{filterResults.map((r) => (
<li key={r.path}>
<button
type="button"
className="w-full flex items-center gap-1 px-2 py-1 text-xs rounded hover:bg-muted/60 text-left"
onClick={() => void openFile(r.path)}
>
<FileText size={12} className="text-muted-foreground shrink-0" />
<span className="font-bold truncate">{r.name}</span>
<span className="text-muted-foreground ml-1 truncate">{r.path}</span>
</button>
</li>
))}
</ul>
) : (
<div className="text-xs text-muted-foreground px-2 py-4 text-center">No matches</div>
)
) : (
<TreeLevel
parentPath=""
entries={rootEntries}
cache={cache}
expanded={expandedDirs}
depth={0}
onToggleDir={toggleDir}
onSelectFile={(path) => void openFile(path)}
/>
)}
</div>
</aside> </aside>
{viewerFile && ( {viewerFile && (

View File

@@ -20,6 +20,7 @@ import { providerIcon, providerLabel } from '@/components/coder/providerIcons';
import { refreshAgentSessions } from '@/hooks/useAgentSessions'; import { refreshAgentSessions } from '@/hooks/useAgentSessions';
import { useAgentStatus, type AgentStatus, type AgentStatusEntry } from '@/hooks/useAgentStatus'; import { useAgentStatus, type AgentStatus, type AgentStatusEntry } from '@/hooks/useAgentStatus';
import { cn } from '@/lib/utils'; import { cn } from '@/lib/utils';
import { sessionEvents } from '@/hooks/sessionEvents';
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Types // Types
@@ -437,6 +438,7 @@ function usePendingChanges(sessionId: string) {
}); });
if (res.ok) { if (res.ok) {
setChanges((prev) => prev.map((c) => c.id === changeId ? { ...c, status: 'approved' } : c)); setChanges((prev) => prev.map((c) => c.id === changeId ? { ...c, status: 'approved' } : c));
sessionEvents.emit({ type: 'git_diff_refresh' });
} }
}, [sessionId]); }, [sessionId]);
@@ -446,6 +448,7 @@ function usePendingChanges(sessionId: string) {
}); });
if (res.ok) { if (res.ok) {
setChanges((prev) => prev.map((c) => c.id === changeId ? { ...c, status: 'rejected' } : c)); setChanges((prev) => prev.map((c) => c.id === changeId ? { ...c, status: 'rejected' } : c));
sessionEvents.emit({ type: 'git_diff_refresh' });
} }
}, [sessionId]); }, [sessionId]);

View File

@@ -178,6 +178,12 @@ export interface RefetchMessagesEvent {
type: 'refetch_messages'; type: 'refetch_messages';
} }
// git-diff-panel Phase 1: emitted client-side to trigger a panel refresh.
// Not a WS frame — no @boocode/contracts change required.
export interface GitDiffRefreshEvent {
type: 'git_diff_refresh';
}
export type SessionEvent = export type SessionEvent =
| SessionRenamedEvent | SessionRenamedEvent
| ProjectCreatedEvent | ProjectCreatedEvent
@@ -204,7 +210,8 @@ export type SessionEvent =
| ProjectUnarchivedEvent | ProjectUnarchivedEvent
| ProjectUpdatedEvent | ProjectUpdatedEvent
| ChatStatusEvent | ChatStatusEvent
| RefetchMessagesEvent; | RefetchMessagesEvent
| GitDiffRefreshEvent;
type Listener = (event: SessionEvent) => void; type Listener = (event: SessionEvent) => void;
const listeners = new Set<Listener>(); const listeners = new Set<Listener>();

View File

@@ -0,0 +1,114 @@
import { useCallback, useEffect, useRef, useState } from 'react';
import { api } from '@/api/client';
import type { GitDiffMode, GitDiffResult, GitDiscardFileInfo } from '@/api/types';
import { sessionEvents } from './sessionEvents';
export function useGitDiff(projectId: string | null | undefined) {
const [mode, setMode] = useState<GitDiffMode>('uncommitted');
const [pinned, setPinned] = useState(false);
const [result, setResult] = useState<GitDiffResult | null>(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
// FIX 4: non-null when user has pinned a mode that differs from the server's auto-selected mode.
const [modeSuggestion, setModeSuggestion] = useState<GitDiffMode | null>(null);
// Coalescence guard: absorb concurrent refresh triggers into the running request.
const inFlightRef = useRef(false);
const refresh = useCallback(() => {
if (!projectId || inFlightRef.current) return;
inFlightRef.current = true;
setLoading(true);
setError(null);
// FIX 1: when not pinned, omit mode param so the server auto-selects based on
// dirty state (dirty → uncommitted, clean → committed).
api.projects
.gitDiff(projectId, pinned ? mode : null)
.then((r) => {
if (!pinned) {
setMode(r.mode);
}
// FIX 4: if pinned and the server's auto-selected mode differs, surface a suggestion.
if (pinned && r.auto_mode && r.auto_mode !== mode) {
setModeSuggestion(r.auto_mode);
} else {
setModeSuggestion(null);
}
setResult(r);
})
.catch((err: unknown) => {
setError(err instanceof Error ? err.message : 'Failed to load diff');
})
.finally(() => {
inFlightRef.current = false;
setLoading(false);
});
}, [projectId, mode, pinned]);
// Re-run refresh when mode changes (user pinned a new mode).
useEffect(() => {
if (!projectId) {
setResult(null);
return;
}
refresh();
}, [projectId, mode]); // eslint-disable-line react-hooks/exhaustive-deps
// Subscribe to git_diff_refresh events (tab open, message_complete, manual).
useEffect(() => {
return sessionEvents.subscribe((event) => {
if (event.type === 'git_diff_refresh') refresh();
});
}, [refresh]);
const selectMode = useCallback((m: GitDiffMode) => {
setPinned(true);
setMode(m);
setModeSuggestion(null); // FIX 4: clear suggestion on explicit mode pick
}, []);
const [mutating, setMutating] = useState(false);
const [mutateError, setMutateError] = useState<string | null>(null);
const runMutation = useCallback(
async (fn: () => Promise<unknown>): Promise<boolean> => {
if (!projectId) return false;
setMutating(true);
setMutateError(null);
try {
await fn();
sessionEvents.emit({ type: 'git_diff_refresh' });
return true;
} catch (err) {
setMutateError(err instanceof Error ? err.message : 'Operation failed');
return false;
} finally {
setMutating(false);
}
},
[projectId],
);
const stage = useCallback(
(files: string[]) => runMutation(() => api.projects.gitStage(projectId!, files)),
[projectId, runMutation],
);
const unstage = useCallback(
(files: string[]) => runMutation(() => api.projects.gitUnstage(projectId!, files)),
[projectId, runMutation],
);
const commit = useCallback(
(message: string, files?: string[]) =>
runMutation(() => api.projects.gitCommit(projectId!, { message, files })),
[projectId, runMutation],
);
const discard = useCallback(
(files: GitDiscardFileInfo[]) => runMutation(() => api.projects.gitDiscard(projectId!, files)),
[projectId, runMutation],
);
return { result, loading, error, mode, selectMode, refresh, mutating, mutateError, stage, unstage, commit, discard, modeSuggestion };
}

View File

@@ -273,6 +273,10 @@ export function useSessionStream(sessionId: string | undefined) {
return; return;
} }
setState((s) => applyFrame(s, frame)); setState((s) => applyFrame(s, frame));
// Trigger git diff refresh after each completed assistant turn.
if (frame.type === 'message_complete') {
sessionEvents.emit({ type: 'git_diff_refresh' });
}
} catch (err) { } catch (err) {
console.warn('bad ws frame', err); console.warn('bad ws frame', err);
} }

View File

@@ -186,6 +186,8 @@ function applyEvent(prev: SidebarResponse, event: import('./sessionEvents').Sess
case 'chat_deleted': case 'chat_deleted':
case 'chat_status': case 'chat_status':
case 'refetch_messages': case 'refetch_messages':
case 'git_diff_refresh':
// Consumed by useGitDiff; no sidebar state change needed.
return prev; return prev;
case 'project_archived': { case 'project_archived': {
const next = prev.projects.filter((p) => p.id !== event.project_id); const next = prev.projects.filter((p) => p.id !== event.project_id);

View File

@@ -396,10 +396,13 @@ function SessionInner({ sessionId }: { sessionId: string }) {
<button <button
type="button" type="button"
onClick={toggleRightRail} onClick={toggleRightRail}
className="inline-flex items-center justify-center -mr-1 min-w-[44px] min-h-[44px] rounded text-muted-foreground hover:bg-muted hover:text-foreground shrink-0" className="relative inline-flex items-center justify-center -mr-1 min-w-[44px] min-h-[44px] rounded text-muted-foreground hover:bg-muted hover:text-foreground shrink-0"
aria-label="Toggle file browser" aria-label="Toggle file browser"
> >
<FolderTree className="size-5" /> <FolderTree className="size-5" />
{git?.is_dirty && (
<span className="absolute top-2 right-2 w-1.5 h-1.5 rounded-full bg-yellow-400" aria-hidden="true" />
)}
</button> </button>
</div> </div>

View File

@@ -0,0 +1,97 @@
# Discovery notes — git-diff-panel implementation
Single source of truth for project context. Specialists: read this first; do NOT re-grep what is here.
Source spec: `../feature-specification.md` (+ `decision-log.md` D1D18, `team-findings.md` F1F21). No
`feature-technical-notes.md` (no load-bearing mechanic qualified at spec time).
## Tech stack
- pnpm monorepo. `apps/web` (React 18 + Vite SPA), `apps/server` (BooChat — Fastify + postgres, native
inference, **read-only** file/git tools), `apps/coder` (BooCoder — host systemd service, port 9502,
write-capable, runs git writes today), `apps/booterm`. TS strict, NodeNext (`.js` import suffixes) on
server + coder. `@boocode/contracts` package single-sources WS frames + provider/message types.
- Tests: vitest ^3. server `pnpm -C apps/server test`; coder `pnpm -C apps/coder test` (`globals:false`
import describe/it/expect). Include glob `src/**/__tests__/**/*.test.ts`. No web test harness. DB-integration
tests opt-in via `DATABASE_URL` + `describe.runIf`.
- Deploy by surface: apps/coder change → `sudo systemctl restart boocoder`; apps/web|server change →
`docker compose up --build -d boocode` (rebuilds web+server from working tree; web HMR live on dev only).
- Shiki `^1.29.2` already in apps/web (`CodeBlock.tsx`, `FileViewerOverlay.tsx`); `lang:'diff'` is valid —
the path of least resistance for rendering a unified diff. No react-diff-viewer / diff2html installed.
## ADRs / coding standards
- No `docs/adr/`. Decisions live in `boocode_roadmap.md` (Decisions log) + per-app `CLAUDE.md` (auto-loaded
when editing that subtree) + `openspec/changes/archived/`.
- Coding standards: `docs/coding-standards/` (canonical), surfaced via `.claude/rules/coding-standards/`
path-scoped indexes. Cross-cutting conventions in root `CLAUDE.md`.
## Code touch points
### Git data sources today (read)
- `apps/server/src/services/git_meta.ts:44` `getGitMeta(rootPath)` — runs `runGit([...argv], rootPath)`
(SAFE: discrete argv, no shell), returns `{branch,is_dirty,ahead,behind}` only (NO diff text), 30s cache.
This is the read-side precedent for the new read route and the F2 argv-safety bar. Uses
`rev-list --left-right --count HEAD...@{upstream}` (the upstream-resolution precedent for D2/D13's base).
- `apps/server/src/routes/projects.ts:426` `GET /api/projects/:id/git` — returns the GitMeta shape (no diff).
`api.projects.git(id)` (`apps/web/src/api/client.ts:154`), polled 30s by `useProjectGit`. The new read
route slots beside this.
- `apps/coder/src/services/worktrees.ts:46` `diffWorktree(worktreePath, projectPath, {baseRef})` — produces
a real unified `git diff <base>...<head>` but via `hostExec(shell string)` + `shellEscape`, and commits
with per-invocation `-c user.email=boocoder@local -c user.name=BooCoder`. **Caution:** this is the SHELL
pattern F2 warns against; the new git-write ops should follow `git_meta.ts`'s argv `runGit`, not this.
But it IS the precedent that the coder/host can run git writes and inject identity per-invocation.
- `pending_changes.diff` is unified only for external-agent edits; native BooCode edits store `{old,new}` JSON.
The new Git panel is project-repo git state, complementary to the pending-changes panel (spec Coordinations).
### The "file browser" host (apps/web)
- `apps/web/src/components/RightRail.tsx` — the right-side file panel (NOT a workspace pane; the legacy
`file_browser` pane kind is dead). Header at ~:209 renders a static "Files" label + 2 icon buttons; desktop
`w-64`, mobile drawer `w-[85vw] max-w-sm` via `useRightRailDrawer`. Already applies `max-md:min-h-[44px]`
to header buttons (the 44px convention for D18). Fetches tree via `api.projects.files/listDir/viewFile`.
**The Files / Git tab (D1, D16) is added to THIS header** — and must fit one line (toolbar-fit rule, D18).
`open_file_in_browser` sessionEvent already opens the panel programmatically.
- Rendered in `App.tsx:~89` for every `/session/:id` (so the panel — and D8's Git tab — appears in all
session types). `Session.tsx:~397` mobile `FolderTree` toggle button (the dirty-indicator host for D17).
### Refresh-trigger plumbing (F20, D10)
- `message_complete` WS frame = "agent turn complete" trigger. Coder pending-changes refresh precedent:
`usePendingChanges` in `CoderPane.tsx:~786` refetches on message-complete. Pending-apply/discard has no
named frame — driven by the `refresh()` callback. Adding a new event/frame requires the CLAUDE.md parity
steps: a new WS frame → BOTH server/contracts `WsFrameSchema` AND web `WsFrame` (`apps/web/src/api/types.ts`);
a new sessionEvent → a `case` in `useSidebar.ts` `applyEvent`.
### Security surfaces
- `apps/server/src/services/path_guard.ts` `resolveProjectRoot(project.path)` — derives + scopes project
paths from the DB project row, never from the request (the F2 "server-derived root + relative-arg
validation" precedent). `secret_guard.ts` deny-list applies to the assistant's read tools (not the user's
git panel — spec D8). HTML artifacts run in a sandboxed iframe with `connect-src 'none'` (BOOCHAT.md) — the
evidence behind F1 (an artifact cannot POST to the new write endpoints). No app-layer auth (Authelia at the
proxy; `'default'` user key).
- BooCoder proxy: apps/server forwards `/api/coder/*` to apps/coder (`coder-proxy.ts`) — the route by which
a web client reaches coder (host) endpoints.
## Recent activity / precedent
- HEAD ~v2.7.11. Pure-helper + TDD precedent: `backends/turn-guard.ts`, `lifecycle-decisions.ts`,
`mistake-tracker.ts` (pure module + unit test, then wire). The diff-parse / base-resolution / mode-decision
logic should follow it (testable pure helpers).
- Sibling backlog plan at `docs/plans/post-review-backlog/` is the format precedent for the plan files.
## Enumerated gaps / open implementation questions for the team
1. **THE architecture decision (F18 / JD-005):** which service owns the new git operations? Options: (a) all
in apps/server (read + write) — but apps/server is "read-only" by posture (the git-write would be a new
write surface there); (b) read in apps/server (consistent with git_meta), write in apps/coder (the
write-capable host service, already runs git writes) via the `/api/coder/*` proxy; (c) all in apps/coder.
Note apps/coder runs on the host (can git-write to the project path); apps/server runs in Docker. The diff
panel appears in ALL sessions incl. plain BooChat — does that constrain which service answers? software-architect to settle.
2. No HTTP route returns a full working-tree git diff today — a new read route is needed regardless.
3. The diff-parse + per-file expand/collapse + staged/unstaged grouping + Shiki `lang:'diff'` rendering is
net-new UI in `RightRail.tsx`; no unified-diff renderer exists yet.
4. F2 argv-safety: the new write path must use discrete-argv git (like `git_meta.runGit`), NOT the
`hostExec(shell)` pattern `worktrees.ts` uses — concrete bar for the security/structural recommendation.
5. Committer identity (D6/D12/F3): server-derived. Precedents differ — `worktrees.ts` injects
`-c user.email=boocoder@local`; `project_bootstrap.ts` hardcodes `samkintop@gmail.com`. The plan must pick
the source for a USER commit (host git config vs a configured value) — not request-supplied.
6. Base resolution (D2/D13): `@{upstream}` precedent exists in git_meta; default-branch fallback unspecified
in code (needs `git symbolic-ref refs/remotes/origin/HEAD` or `rev-parse --abbrev-ref origin/HEAD`).

View File

@@ -0,0 +1,195 @@
# Implementation Decision Log: Git Diff Panel
<!--
This file records every implementation decision committed while planning the Git Diff Panel.
Behavioral and implementation statements live in [../feature-implementation-plan.md](../feature-implementation-plan.md) —
this file captures the question, rationale, evidence, and rejected alternatives for each decision.
Round-by-round history lives in [implementation-iteration-history.md](implementation-iteration-history.md).
Shared D-N counter across trivial and full sections. Decisions here are implementation HOW
decisions; the spec WHAT decisions (D1D18) live in [decision-log.md](decision-log.md) and are
inherited, not restated.
-->
## Trivial decisions
- D-9: No `@boocode/contracts` change — refresh is a client-side `sessionEvents` event, not a WS frame, so the contracts package (`packages/contracts/`) is not touched and not rebuilt. — Referenced in plan: Implementation Approach → External Interfaces, Operational Readiness.
- D-10: No DB schema change, no migration, no new env var — the panel reads git state at request time and writes the project repo directly; nothing is persisted in Postgres. — Referenced in plan: Implementation Approach → Data Model and Persistence, Operational Readiness.
- D-11: Per-file expand/collapse state is local React state inside `GitDiffView`, not a shared hook — one consumer exists. — Referenced in plan: Implementation Approach → Architecture and Integration Points, Deferred (YAGNI).
- D-12: `autoSelectMode` and `canCommit` are inline pure helpers inside `git_diff.ts` (unit-tested), not separate modules. — Referenced in plan: Implementation Approach → Runtime Behavior, Testing Strategy.
## Full decisions
### D-1: Both git read AND git write live in apps/server
- **Question:** Which service owns the new git read and git write operations — apps/server (BooChat, Docker), apps/coder (BooCoder, host service), or a read-in-server / write-in-coder split?
- **Decision:** Both the read route and the write routes (stage / unstage / commit / discard) live in **apps/server**. New `apps/server/src/services/git_diff.ts` holds the read logic and pure helpers plus the git-write helpers; new routes are added beside `GET /api/projects/:id/git` in `apps/server/src/routes/projects.ts`. No apps/coder changes. Single deploy surface: `docker compose up --build -d boocode`.
- **Rationale:** Writing in apps/server reuses the existing safe argv `runGit` pattern and the proven `project_bootstrap.ts` git-write precedent, and avoids a cross-service `/api/coder/*` proxy hop per write, a second deploy surface (coder restart), and making the host coder service a dependency for a plain BooChat-session commit. The "read-only" posture that motivated the split governs the **assistant's tool surface**, not the container filesystem or the user's own UI actions.
- **Evidence:**
- `docker-compose.yml:16` mounts `/opt:/opt` **read-write** into the boocode container — apps/server can already write project paths (refutes the "apps/server is read-only by posture" premise).
- `apps/server/src/services/project_bootstrap.ts` already runs git writes from apps/server: `git init` + commits via safe `execFile` with `-c user.name` / `-c user.email` (`GIT_USER_NAME='indifferentketchup'`, `GIT_USER_EMAIL='samkintop@gmail.com'` at :38-39, applied :122-123) — git-write from the server is an existing, proven pattern.
- `apps/server/src/services/git_meta.ts:30` has the safe `runGit` (discrete-argv `execFile`, no shell); `apps/coder/src/services/worktrees.ts` has only the **unsafe** `hostExec(shell string)` — writing in coder would require building a new safe wrapper there.
- Spec D8 ([decision-log.md](decision-log.md#d8--git-write-is-a-user-action-not-an-assistant-tool)): the read-only invariant is defined over the assistant's tool surface; the panel's actions are the human user's own UI actions.
- **Rejected alternatives:**
- Read-in-server / write-in-coder split (the Round-1 software-architect A1 recommendation) — rejected because its premise (apps/server is read-only and a write there is a new surface) is refuted by the `/opt` rw mount and the `project_bootstrap.ts` git-write precedent; the split adds a cross-service hop, a second deploy surface, and coder coupling for a BooChat-session commit.
- All-in-coder — rejected because it requires building a new safe argv git wrapper in coder (which only has the unsafe `hostExec`) and adds the same coder coupling and second deploy surface.
- **Specialist owner:** `software-architect`
- **Revisit criterion:** If a third consumer (beyond this feature in apps/server) needs the same git ops, reopen the shared-`packages/` extraction (see Deferred YAGNI); if apps/server's container ever loses the `/opt` rw mount, reopen the service-owner question.
- **Dissent (if any):** The Round-1 software-architect (A1) recommended the write-in-coder split; recorded under disagree-and-commit. Revisit if the refuting evidence (rw mount + bootstrap precedent) changes.
- **Driven by rounds:** R1
- **Dependent decisions:** D-2, D-3, D-4, D-5, D-8
- **Referenced in plan:** Implementation Approach → Architecture and Integration Points, Decomposition and Sequencing, Operational Readiness, Summary
### D-2: Read route + `git_diff.ts` pure helpers, TDD-first
- **Question:** What is the read-side shape, and how is the diff-parse / mode / base logic structured and tested?
- **Decision:** Add `GET /api/projects/:id/git/diff?mode=<uncommitted|committed>` beside `GET /api/projects/:id/git`. The route delegates to `apps/server/src/services/git_diff.ts`, which exposes pure helpers written test-first before wiring: `parseNameStatus` (porcelain → file list with change type), `splitDiffByFile` (unified diff text → per-file segments), `resolveCommittedBase` (base resolution, see D-6), `autoSelectMode`, the binary/large `classify`, and `detectInProgress` (in-progress git-state detection, see D-7). The route composes these over `runGit` argv calls.
- **Rationale:** The pure-helper-then-wire pattern is the project's established TDD precedent and keeps the parse/mode/base/classify logic unit-testable without spawning git. Slotting beside the existing git-meta route reuses its derivation and request shape.
- **Evidence:** `apps/server/src/services/git_meta.ts:44` `getGitMeta` + `apps/server/src/routes/projects.ts:426` `GET /api/projects/:id/git` (the read precedent the new route slots beside). Pure-helper TDD precedent: `apps/server/src/services/backends/turn-guard.ts`, `lifecycle-decisions.ts`, `mistake-tracker.ts` (pure module + unit test, then wire). test-engineer (T1T12).
- **Rejected alternatives:**
- Inline all parse/mode/base logic in the route handler — rejected because it cannot be unit-tested without spawning git and breaks the project's pure-helper precedent.
- A separate module per helper — rejected (see D-12); the helpers are small and cohesive in one tested module.
- **Specialist owner:** `software-architect`
- **Revisit criterion:** If `git_diff.ts` grows past a single cohesive module (e.g. distinct read vs write concerns each exceed a file), split along that seam.
- **Driven by rounds:** R1
- **Dependent decisions:** D-6, D-7
- **Referenced in plan:** Implementation Approach → Architecture and Integration Points, Implementation Approach → Runtime Behavior, Decomposition and Sequencing, Testing Strategy
### D-3: Write ops via argv-safe `runGit`/`execFile` with `--` separators
- **Question:** How are the write operations (stage / unstage / commit / discard) invoked safely?
- **Decision:** All write ops use the discrete-argv `runGit`/`execFile` pattern with explicit `--` separators between options and user-supplied paths, plus a flag-injection guard (reject path arguments beginning with `-`). They **never** use `hostExec(shell string)` or any shell-interpolated invocation. User-supplied text (commit message, file targets) is always passed as discrete argv, never built into a command string.
- **Rationale:** Discrete argv with `--` separators closes the command-injection and flag-injection vectors that a shell string opens (F2). The server already has the safe pattern; the coder's shell pattern is exactly what the spec security posture warns against.
- **Evidence:** `apps/server/src/services/git_meta.ts:30` `runGit` (safe discrete-argv `execFile`) is the bar. `apps/coder/src/services/worktrees.ts` `hostExec(shell)` + `shellEscape` is the anti-pattern F2 flags. Spec D11/D12 ([decision-log.md](decision-log.md#d11--all-git-operations-scoped-to-the-project-repository-path)), F2 ([team-findings.md](team-findings.md)).
- **Rejected alternatives:**
- Reuse the coder `hostExec(shell)` + `shellEscape` pattern — rejected because shell interpolation lets user-supplied content (commit message, filenames with special characters) be interpreted as flags or shell syntax; the argv pattern eliminates the class.
- **Specialist owner:** `adversarial-security-analyst`
- **Revisit criterion:** If a git operation genuinely cannot be expressed in discrete argv (none in the v1 set qualifies), reopen with a documented justification.
- **Driven by rounds:** R1
- **Dependent decisions:** D-4
- **Referenced in plan:** Implementation Approach → Architecture and Integration Points, Security Posture, Decomposition and Sequencing
### D-4: Path validation — repo-relative `pathGuard`, reject escape and repo-root discard
- **Question:** How are per-file path arguments validated before a write?
- **Decision:** Every per-file argument is validated by a `pathGuard(repoRoot, file)` that resolves the path and rejects it if it is absolute, contains `..` traversal, or resolves (including via symlink) outside the repository root. Additionally, discarding the repository root itself (`.`) is rejected. The repository root is derived server-side from the session's project record, never from the request.
- **Rationale:** Per-file arguments can escape the repo root via `../` traversal or a symlink even when the root is correct, so each argument needs independent validation (F2). Rejecting a repo-root discard prevents a single confirmation from wiping the whole working tree.
- **Evidence:** `apps/server/src/services/path_guard.ts` `resolveProjectRoot(project.path)` — derives + scopes project paths from the DB project row, never from the request (the F2 precedent). Spec D11/D12, F2 ([team-findings.md](team-findings.md)). Integration-test precedent: `path_guard.test.ts`.
- **Rejected alternatives:**
- Validate only the repository root, not per-file arguments — rejected (F2): a per-file `../` or symlink argument escapes a correctly-scoped root.
- Accept a caller-supplied repository path — rejected: needless write surface, no use case; root is server-derived.
- **Specialist owner:** `adversarial-security-analyst`
- **Revisit criterion:** If a legitimate write target outside the resolved root ever appears (none in scope), reopen.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Security Posture, Decomposition and Sequencing, Testing Strategy
### D-5: Commit identity server-derived; request schema `.strict()` with no author fields
- **Question:** Where does the commit author/committer identity come from, and what can the request set?
- **Decision:** Commit identity is server-derived via `-c user.name=… -c user.email=…` read from the repository's git config at commit time, falling back to the `project_bootstrap.ts` constants (`indifferentketchup` / `samkintop@gmail.com`) when git config yields nothing. The commit request schema is Zod `.strict()` and carries `{message, files?}` only — no `author`, `email`, `date`, or any identity field; unknown keys are rejected.
- **Rationale:** A server-derived identity passed via `-c` flags makes authorship un-spoofable from the request body (F3). `.strict()` ensures an attacker cannot smuggle an identity field. The `project_bootstrap.ts` `-c` precedent already establishes this exact mechanism for server-side commits.
- **Evidence:** `apps/server/src/services/project_bootstrap.ts:38-39,122-123` (server-side commit with `-c user.name`/`-c user.email`). Spec D6/D12, F3 ([team-findings.md](team-findings.md)). Zod is the project's request-validation library (apps/server CLAUDE.md).
- **Rejected alternatives:**
- Request-supplied commit identity — rejected (F3): allows authorship spoofing; no legitimate use in a single-user app.
- Hardcode the coder's `boocoder@local` identity — rejected: a user commit is not a coder commit; git config is the correct source for the human user, with the bootstrap constants as the fallback.
- A non-strict request schema — rejected: an unknown `author`-style key could slip through and influence the commit.
- **Specialist owner:** `adversarial-security-analyst`
- **Revisit criterion:** If a multi-identity requirement appears (e.g. co-author selection), reopen with a server-validated identity source, never a free-form request field.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Implementation Approach → Architecture and Integration Points, Security Posture, Decomposition and Sequencing, Testing Strategy
### D-6: Committed-mode base resolution via `@{upstream}` → `origin/HEAD` → null
- **Question:** How does Committed mode resolve the base it compares the current branch against?
- **Decision:** The pure helper `resolveCommittedBase` resolves the base in order: (1) `git rev-parse --abbrev-ref --symbolic-full-name @{upstream}` (the tracking branch; non-zero exit if none), then (2) `git rev-parse --abbrev-ref origin/HEAD` (the default branch), then (3) null. On null the panel falls back to Uncommitted and labels it as a fallback. The resolved base is returned in the read response so the header can label "Git — branch vs &lt;base&gt;".
- **Rationale:** Tracking-branch-first matches git's own upstream model and is the right base for a contributor whose tracking branch is a fork or PR target; `origin/HEAD` is the right fallback for the default branch; a labeled Uncommitted fallback is more useful than an error or a silent swap (F11).
- **Evidence:** `apps/server/src/services/git_meta.ts:58` already uses `HEAD...@{upstream}` (the upstream-resolution precedent). Discovery notes §6 (`rev-parse --abbrev-ref origin/HEAD` for the default-branch fallback). Spec D13, F11 ([team-findings.md](team-findings.md)).
- **Rejected alternatives:**
- Always compare against the default branch, ignoring the tracking branch — rejected (F11): wrong for contributors tracking a fork or PR target.
- Error when no base resolves — rejected (D13): leaves the panel useless; a labeled Uncommitted fallback is more helpful.
- `git symbolic-ref refs/remotes/origin/HEAD` — equivalent but `rev-parse --abbrev-ref origin/HEAD` is the simpler form already aligned with the git_meta precedent.
- **Specialist owner:** `software-architect`
- **Revisit criterion:** If repos without `origin/HEAD` set become common and the Uncommitted fallback is observed to confuse users, reopen the default-branch heuristic.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Implementation Approach → Runtime Behavior, Decomposition and Sequencing, Testing Strategy
### D-7: Read deadline 30s + maxBuffer 10MB; index-lock → HTTP 409; in-progress detection disables writes
- **Question:** What resilience bounds and failure semantics does the git surface commit to?
- **Decision:** (a) The read path uses a 30s execution deadline and a 10MB `maxBuffer` — both distinct from the D5 per-file display-size cap; a read that exceeds the deadline exits loading, shows an error, and offers Refresh. (b) A write that fails because the index is locked returns **HTTP 409 "repository busy"** with no server-side retry (retry is user-driven). (c) `detectInProgress` reads `.git` sentinel stats (`MERGE_HEAD`, `rebase-merge`/`rebase-apply`, `CHERRY_PICK_HEAD`, `BISECT_LOG`) and folds an in-progress flag into the read response; the client disables write affordances when set. (d) Untracked-file discard reports honest partial failure on refresh rather than claiming an unenforceable "state unchanged."
- **Rationale:** A slow git process can stall the panel even with small files, so a deadline distinct from the size cap is needed (F7). A server retry on a lock hides the busy state and adds timer state; a 409 lets the user retry (F5). Detecting in-progress states up front prevents raw git errors on stage/commit/discard (F9). Partial-failure honesty matches what the git layer can actually guarantee (F6).
- **Evidence:** `git_meta.ts:6-10` (the existing 2s timeout + 1MB buffer pattern this scales up for diff payloads). Spec D5/F7 (deadline distinct from size cap), F5 (index-lock), F9 (in-progress states), F6 (partial-failure wording) ([team-findings.md](team-findings.md)).
- **Rejected alternatives:**
- Server-side retry on index-lock — rejected (F5, YAGNI): "try again" is user-driven; a server retry hides the busy state and adds timer state. Reopen if lock contention is observed frequent.
- Combine the deadline and the display-size cap into one mechanism — rejected (F7/D5): they address different failure modes (slow process vs large output).
- Streaming diff reader that caps mid-stream — rejected (YAGNI): `execFile` maxBuffer 10MB covers any realistic single-user working-tree diff.
- **Specialist owner:** `on-call-engineer`
- **Revisit criterion:** Reopen the buffer/deadline if working-tree diffs routinely exceed 10MB or reads routinely exceed 30s; reopen the retry decision if index-lock contention is observed frequent in practice.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Implementation Approach → Runtime Behavior, On-Call Resilience Posture, Decomposition and Sequencing, Testing Strategy
### D-8: Refresh via client `git_diff_refresh` sessionEvent, coalesced — not a WS frame
- **Question:** How is the panel's refresh wired across its five triggers, and does it need a new WS frame?
- **Decision:** Refresh is a client-side `sessionEvents` event named `git_diff_refresh`, **not** a new WS frame. All five triggers are client-side: tab open, post-mutation, message_complete (from the existing WS frame), pending-change apply/discard, and the explicit Refresh control. The event is emitted from the `message_complete` handler and the CoderPane apply/discard callbacks; `useGitDiff` subscribes. A no-op `case 'git_diff_refresh'` is added to `useSidebar.ts` `applyEvent` per the apps/web parity rule. The client holds an in-flight coalescence ref so a refresh already running absorbs later triggers (no second concurrent read, no debounce).
- **Rationale:** No trigger requires a server push — `message_complete` already arrives as a WS frame and the rest are local UI events — so a new WS frame and a `@boocode/contracts` rebuild are unnecessary. A client coalescence ref settles the panel to a single final snapshot (F8).
- **Evidence:** Discovery notes §"Refresh-trigger plumbing": `message_complete` WS frame = turn-complete trigger; `usePendingChanges` in `CoderPane.tsx:~786` refetches on message-complete; new sessionEvent → a `case` in `useSidebar.ts` `applyEvent` (apps/web parity rule). Spec D10/F8 (coalescence), F20 (parity steps) ([team-findings.md](team-findings.md)).
- **Rejected alternatives:**
- A new WS frame for refresh — rejected (YAGNI): replaced by the simpler client `sessionEvents` event; no server push is needed, avoiding a `@boocode/contracts` rebuild and the dual server/web frame-type update.
- Per-trigger debounce instead of an in-flight coalescence ref — rejected (F8): a debounce delays the first read and can still spawn overlapping reads under bursts.
- **Specialist owner:** `software-architect`
- **Revisit criterion:** If a future trigger genuinely originates server-side (e.g. an out-of-band repo mutation the client cannot observe), reopen the WS-frame option.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Implementation Approach → Runtime Behavior, Implementation Approach → External Interfaces, Decomposition and Sequencing, On-Call Resilience Posture
### D-13: Write endpoints excluded from the assistant tool registry; artifact sandbox closes the indirect path
- **Question:** What prevents the assistant (directly or via a rendered artifact) from reaching the git-write endpoints?
- **Decision:** The git-write endpoints are HTTP routes only and are **never** registered in the assistant tool registry (`ALL_TOOLS` in `services/tools.ts`). The indirect path — an AI-emitted HTML artifact POSTing to the endpoints with the user's cookie — is already closed by the artifact iframe's `connect-src 'none'` sandbox. No new mitigation is built for the artifact path; the plan records the existing control as the evidence.
- **Rationale:** The panel's writes are the user's own UI actions (spec D8); keeping them out of `ALL_TOOLS` means no assistant tool surface exists for them, and the existing artifact sandbox closes the only indirect path (F1).
- **Evidence:** `apps/server/src/services/tools.ts` `ALL_TOOLS` (tool registry the endpoints are kept out of). Artifact iframe `connect-src 'none'` per BOOCHAT.md output-format section (the F1 evidence). Spec D8/D12, F1 ([team-findings.md](team-findings.md)).
- **Rejected alternatives:**
- Session-type gating (restrict the panel to write-capable sessions) — rejected (F1/D8): session type is the wrong layer; the artifact sandbox closes the actual indirect path, and the user's UI actions should not be gated by the assistant's permissions.
- A custom CSRF header on the write routes — rejected (YAGNI): `connect-src 'none'` on artifacts + the SameSite=Lax Authelia cookie + a same-origin SPA cover it at single-user scale. Reopen if the routes are exposed to a less-controlled origin.
- **Specialist owner:** `adversarial-security-analyst`
- **Revisit criterion:** If the write routes are ever exposed to an origin outside the same-origin SPA, reopen the CSRF-header decision.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Security Posture, Decomposition and Sequencing
### D-14: Two-phase build — read/display (Phase 1) then write actions (Phase 2)
- **Question:** How is v1 sequenced, given the write surface is the larger half?
- **Decision:** Two phases on the same deploy surface. **Phase 1 (read + display):** `git_diff.ts` pure helpers TDD-first → read route → client `useGitDiff` + RightRail Files/Git tab + `GitDiffView` read-only + the `git_diff_refresh` refresh wiring. **Phase 2 (write actions):** write helpers + write routes + stage/unstage/commit/discard affordances + in-progress disable. Both phases ship via `docker compose up --build -d boocode` (no coder restart).
- **Rationale:** The write surface is the larger, higher-risk half (F19); landing read/display first delivers reviewable value and de-risks the write phase. Because both read and write live in apps/server (D-1), the two phases share one deploy surface, so the split is sequencing, not two deploy targets.
- **Evidence:** Spec F19 (sequence diff-display before write actions within v1). D-1 (both in apps/server → single surface). junior-developer (coupling flag corrected to single-surface).
- **Rejected alternatives:**
- Ship read and write together as one slice — rejected (F19): the write surface is the larger half and benefits from landing display first for review.
- Phase the split across deploy surfaces — rejected (D-1): both halves are in apps/server, so there is one surface.
- **Specialist owner:** `software-architect`
- **Revisit criterion:** If Phase 1 review surfaces a read-path change that blocks the write design, fold the affected piece forward; otherwise the order holds.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Decomposition and Sequencing, Definition of Done
### D-15: Frontend — RightRail Files/Git tab, `GitDiffView` with lazy Shiki, dirty dot from `useProjectGit`
- **Question:** How does the panel fit into the existing right-rail file browser, and how are diffs rendered?
- **Decision:** The Files/Git tab strip is added to the existing `RightRail.tsx` header (replacing the static "Files" label), fitting one line (D18); the FilePlus button shows only on the Files tab. `GitDiffView` occupies the same slot as the file tree and renders unified diffs via Shiki `lang:'diff'`, highlighting a file's diff lazily only when expanded (per-file loading state; "expand all" lazily highlights per file as rendered). The dirty dot on the Git tab button and the `Session.tsx` FolderTree toggle is fed by `useProjectGit`'s existing `is_dirty` (no new fetch). D14's "briefly notes the change" is a non-blocking inline line inside the panel, not a toast (avoids mobile-drawer z-index collision).
- **Rationale:** The right rail is the file-browser host the spec scopes the panel to (D1); reusing its header and the existing `useProjectGit` dirty signal avoids new plumbing and a new fetch (D17). Lazy Shiki avoids highlighting unopened diffs (D9, perf). An inline note avoids the toast/drawer z-index collision on mobile.
- **Evidence:** `apps/web/src/components/RightRail.tsx` (~:209 header with static "Files" label + 2 icon buttons; `max-md:min-h-[44px]` already applied — the D18 44px convention). `Session.tsx:~397` mobile FolderTree toggle (D17 dirty host). `useProjectGit` polls `GET /api/projects/:id/git` 30s and already returns `is_dirty` (D17 reuse). Shiki `^1.29.2` already in apps/web (`CodeBlock.tsx`, `FileViewerOverlay.tsx`); `lang:'diff'` valid (discovery notes). Spec D1, D9, D16, D17, D18.
- **Rejected alternatives:**
- Eager-highlight all diffs on load — rejected (D9, perf): highlights diffs the user never expands.
- A new fetch for the dirty indicator — rejected (D17): `useProjectGit` already produces `is_dirty`.
- A toast for D14's mode-change note — rejected: collides with the mobile-drawer z-index; an inline panel line is non-blocking and in-context.
- A separate per-file expand-state hook — rejected (see D-11): one consumer; local state suffices.
- **Specialist owner:** `user-experience-designer`
- **Revisit criterion:** If a second component needs the diff-render or expand state, reopen the shared-hook extraction.
- **Driven by rounds:** R1
- **Dependent decisions:** —
- **Referenced in plan:** Implementation Approach → Architecture and Integration Points, Decomposition and Sequencing, Testing Strategy

View File

@@ -0,0 +1,64 @@
# Implementation iteration history — Git diff panel
Round-by-round record of the `plan-implementation` discussion for the git-diff-panel feature. The primary
plan is [`../feature-implementation-plan.md`](../feature-implementation-plan.md); decisions are in
[`implementation-decision-log.md`](implementation-decision-log.md). No `feature-technical-notes.md` exists
(no T# notes), so the spec-maturity gate reduces to the spec-level threshold alone.
Team size: **medium** (round cap 2). Converged in **1 round** — every open question resolved from evidence;
the spec-maturity gate did not trip.
---
## R1 — Parallel specialist review
**Specialists engaged:** software-architect, adversarial-security-analyst, on-call-engineer, test-engineer,
junior-developer (all sonnet, parallel). project-manager synthesized (Step 8).
**New input provided:** the feature spec + decision-log (D1D18) + team-findings (F1F21), the discovery
notes (`.discovery-notes.md`), and domain-scoped briefs. Mid-round the orchestrator verified the Docker
mount and project_bootstrap git-write precedent, which refuted the architect's service-split premise.
### Claim ledger
| # | Claim | State | Spec-maturity | Supporting |
|---|-------|-------|---------------|-----------|
| C1 | Read + write both in apps/server; architect's write-in-coder premise refuted by `/opt` rw mount + `project_bootstrap.ts` git-write precedent | Evidenced | plan-level | junior (coupling flag) + evidence; security (safe runGit only in server) |
| C2 | Read route + `git_diff.ts` pure helpers (parse, base-resolve, mode-select, classify, in-progress) — TDD-first | Evidenced | plan-level | architect, test-engineer |
| C3 | Write ops via argv-safe `runGit`/`execFile` + `--` separators; never `hostExec(shell)` | Evidenced | plan-level | architect, security |
| C4 | Path validation via `pathGuard` (reject `..`/abs/symlink-escape + repo-root discard) | Evidenced | plan-level | security |
| C5 | Commit identity server-derived (`-c` from git config, bootstrap fallback); `.strict()` request, no author fields | Evidenced | plan-level | security, on-call |
| C6 | Refresh = client `git_diff_refresh` sessionEvent + in-flight coalescence ref (no WS frame, no contracts rebuild) | Evidenced | plan-level | architect, on-call |
| C7 | Read deadline 30s + `maxBuffer` 10MB (distinct from D5 display cap) | Evidenced | plan-level | on-call |
| C8 | Index-lock → HTTP 409 "repository busy"; no server retry | Evidenced | plan-level | on-call |
| C9 | In-progress detection via `.git` sentinel `stat`s folded into read response → disable writes | Evidenced | plan-level | on-call, test-engineer |
| C10 | Write endpoints excluded from `ALL_TOOLS`; artifact sandbox `connect-src 'none'` blocks artifact→endpoint | Evidenced | plan-level | security |
| C11 | RightRail Files/Git tab + `GitDiffView` (Shiki `lang:'diff'` lazy-on-expand) + dirty dot from `useProjectGit` | Evidenced | plan-level | architect, junior |
| C12 | Two-phase build: read/display first, writes second (same deploy surface) | Evidenced | plan-level | architect, junior |
| C13 | Test plan T1T12: pure-helper units + temp-repo integration; skip Shiki/layout (no web harness) | Evidenced | plan-level | test-engineer |
### Open Questions and resolutions
| OQ | Question | Resolution source | Outcome |
|----|----------|-------------------|---------|
| OQ-1 | Which service owns the git routes? (raised by all five) | evidence | Read + write both in apps/server (D-1) — `/opt` rw mount + project_bootstrap precedent + D8 logic refute the coder-split premise |
| OQ-2 | Refresh-wiring mechanism across CoderPane↔RightRail subtrees | evidence | Client `git_diff_refresh` sessionEvent; no WS frame, no contracts rebuild (D-8) |
| OQ-6 | Commit identity source | evidence | Server-derived `-c` from git config, project_bootstrap constants fallback; request has no author fields (D-5) |
| OQ-7 | Committed-mode base resolution command | evidence | `@{upstream}``origin/HEAD` → null→Uncommitted fallback (D-6) |
| OQ-3/8/9 | Header condensation / dirty-indicator placement / D14 notification | evidence | Tab strip replaces "Files" label; FilePlus on Files tab only; dirty dot from `useProjectGit`; D14 = inline non-blocking line, not a toast (D-15) |
| OQ-4 | Is the write half gated/sequenced? | evidence | Two-phase build, read/display then writes (D-14) |
| OQ-5 | Shiki lazy vs eager | evidence | Lazy highlight on expand, per-file loading state (D-15) |
### Spec-maturity gate
**NOT tripped.** Zero spec-level findings — the spec committed every behavior (D1D18); all Round-1 findings
are plan-level HOW choices, each resolved from codebase evidence. No T#-contradictions (no T# notes exist).
### Next-step recommendation
**Go to synthesis.** All open questions resolved by evidence; no handoffs requested; no plan-level question
left unresolved.
**Decisions produced:** D-1 through D-15.
**Changed in plan:** Implementation Approach, Decomposition and Sequencing, Security Posture, On-Call
Resilience Posture, Operational Readiness, Testing Strategy, Deferred (YAGNI), UX Notes.

View File

@@ -0,0 +1,105 @@
# Synthesis input — Round 1 aggregation + resolutions (git-diff-panel impl)
Deterministic aggregation of Round-1 (software-architect, adversarial-security-analyst, on-call-engineer,
test-engineer, junior-developer). Team size: medium (round cap 2; converged in 1 — all open questions
resolved from evidence). Spec-maturity gate: NOT tripped (0 spec-level findings — the spec committed all
behaviors; every finding is a plan-level HOW). Next step: go to synthesis.
## THE corrected key decision (D-1)
**Both the git READ and the git WRITE routes live in apps/server, NOT a read-server/write-coder split.**
The software-architect (A1) recommended read-in-server / write-in-coder, on the premise that apps/server is
"read-only by posture" and adding writes there is a new surface. The junior-developer flagged the resulting
coupling (a plain BooChat session reaching the coder write service). Evidence REFUTES the architect's premise:
- `docker-compose.yml:16` mounts `/opt:/opt` **read-write** into the boocode container — apps/server can
already write project paths.
- `apps/server/src/services/project_bootstrap.ts` ALREADY runs git writes from apps/server: `git init` +
commits via safe `execFile` with `-c user.name/-c user.email` flags (`GIT_USER_NAME='indifferentketchup'`,
`GIT_USER_EMAIL='samkintop@gmail.com'` at :38-39, applied :122-123). Git-write from the server is an
existing, proven pattern — not new.
- D8's own logic: "read-only" governs the **assistant's tools**, not the container's filesystem or the
user's own UI actions. A user-driven git-write route is not an assistant tool.
- apps/server has the safe `runGit` (`git_meta.ts:30`, `execFile` discrete argv); apps/coder has only the
UNSAFE `hostExec(shell string)` (`worktrees.ts`). Writing in coder would require building a NEW safe
wrapper there; writing in server reuses the existing safe one and the project_bootstrap `-c` precedent.
- Writing in server avoids: a cross-service `/api/coder/*` proxy hop per write, a second deploy surface
(coder restart), and making coder a dependency for a BooChat-session commit.
**Decision:** read + write both in apps/server. New `apps/server/src/services/git_diff.ts` (read) +
git-write helpers, new routes beside `GET /api/projects/:id/git` in `routes/projects.ts`. No apps/coder
changes. Single deploy surface (`docker compose up --build -d boocode`). Rejected: A1's read-server/
write-coder split (premise refuted); all-in-coder (needs a new safe wrapper, adds coupling).
## Resolved open questions (all plan-level, settled by evidence)
- **OQ-1 service split** → D-1 above (read+write in apps/server).
- **OQ-2 refresh wiring (F20/D10)** → a client-side `sessionEvents` event `git_diff_refresh`, NOT a new WS
frame. The five triggers are all client-side (tab open, post-mutation, message_complete, pending-apply,
on-demand); no server push needed → no `@boocode/contracts` rebuild. Emit from the `message_complete`
handler + CoderPane apply/discard callbacks; `useGitDiff` subscribes; add a no-op `case` in
`useSidebar.ts:applyEvent` per the apps/web CLAUDE.md rule. (architect A4.)
- **OQ-6 commit identity (D12/F3)** → server-derived `-c user.name=… -c user.email=…`, read from git config
at commit time, fallback to the project_bootstrap constants. Request schema is `.strict()`,
`{message, files?}` only — no author/email/date fields. (security #4, project_bootstrap precedent.)
- **OQ-7 base resolution (D13)** → `git rev-parse --abbrev-ref --symbolic-full-name @{upstream}` (tracking
branch; non-zero if none) → else `git rev-parse --abbrev-ref origin/HEAD` (default branch) → else null →
fall back to Uncommitted, labeled. Pure helper `resolveCommittedBase`. (architect A2.)
- **OQ-3/8/9 UX placement** → adopt architect A4: the Files/Git tab strip replaces the static "Files" label;
the FilePlus button shows only on the Files tab (keeps header one line, D18); dirty dot on the Git tab
button + the `Session.tsx` FolderTree toggle, fed by `useProjectGit`'s existing `is_dirty` (no new fetch,
D17); D14's "briefly notes the change" is a non-blocking inline line inside the panel (NOT a toast —
avoids mobile-drawer z-index collision).
- **OQ-4 sequencing (F19)** → adopt architect A5 two-phase: Phase 1 = read + display (git_diff.ts pure
helpers TDD-first → read route → client + RightRail tab + GitDiffView read-only + refresh wiring); Phase 2
= write actions (write helpers + routes + stage/commit/discard affordances + in-progress disable). Both
phases are now the SAME deploy surface (server+web) since writes are in apps/server.
- **OQ-5 Shiki (D9)** → lazy: highlight a file's diff only when expanded, with a per-file loading state;
"expand all" triggers lazy highlight per file as rendered. (junior OQ-5.)
## Claim ledger (consolidated)
| # | Claim | State | Spec-maturity | Supporting |
|---|-------|-------|---------------|-----------|
| C1 | Read+write both in apps/server; architect's write-in-coder premise refuted by /opt rw mount + project_bootstrap git-write precedent | Evidenced | plan-level | junior (flag) + corrected via evidence; security (safe-runGit-only-in-server) |
| C2 | Read route + git_diff.ts pure helpers (parseNameStatus, splitDiffByFile, resolveCommittedBase, autoSelectMode, classify binary/large, detectInProgress) — TDD-first | Evidenced | plan-level | architect, test-engineer |
| C3 | Write ops via argv-safe runGit/execFile with `--` separators; NEVER hostExec(shell) | Evidenced | plan-level | architect, security |
| C4 | Path validation: pathGuard(repoRoot, file) rejects ../ + absolute + symlink-escape; also reject discard of repo-root (`.`) | Evidenced | plan-level | security |
| C5 | Commit identity server-derived (-c from git config, bootstrap fallback); request `.strict()` no author fields | Evidenced | plan-level | security, on-call OQ4 |
| C6 | Refresh = sessionEvent git_diff_refresh + client in-flight coalescence ref (no WS frame, no debounce) | Evidenced | plan-level | architect, on-call |
| C7 | Read deadline 30s + maxBuffer 10MB (distinct from D5 size cap); timeout→error+Refresh | Evidenced | plan-level | on-call |
| C8 | Index-lock → HTTP 409 "repository busy", NO server retry (user-driven retry) | Evidenced | plan-level | on-call |
| C9 | In-progress detection via .git sentinel stats (MERGE_HEAD/rebase-merge/CHERRY_PICK_HEAD/BISECT_LOG) folded into read response → disable writes | Evidenced | plan-level | on-call, test-engineer |
| C10 | Write endpoints NOT in the assistant tool registry (ALL_TOOLS); artifact sandbox connect-src 'none' already blocks artifact→endpoint | Evidenced | plan-level | security |
| C11 | RightRail Files/Git tab + GitDiffView (Shiki lang:'diff' lazy-on-expand) + dirty dot from useProjectGit | Evidenced | plan-level | architect, junior |
| C12 | Two-phase build: read/display first, writes second (same deploy surface) | Evidenced | plan-level | architect, junior |
| C13 | Test plan T1-T12: pure-helper units + temp-repo integration (mkdtemp/git init, like path_guard.test.ts); skip Shiki/layout (no web harness) | Evidenced | plan-level | test-engineer |
## YAGNI ledger
- **Shared `packages/` runGit extraction** → DEFER. Rule of Three not met (now only apps/server consumes it
for this feature; coder untouched). Reopen: a third consumer needs git ops. Source: architect.
- **New WS frame for refresh** → REPLACE with the simpler client `sessionEvents` event (no server push, no
contracts rebuild). Source: architect/junior OQ-2.
- **Server-side retry on index-lock** → DEFER. "Try again" is user-driven; a server retry hides the busy
state and adds timer state. Reopen: lock contention observed frequent in practice. Source: on-call F5.
- **Streaming diff reader (cap mid-stream)** → DEFER. `execFile` maxBuffer 10MB covers any realistic
single-user working-tree diff; streaming is ~40-50 LoC for a transient memory spike that doesn't matter at
this scale. Reopen: diffs routinely exceed 10MB. Source: on-call §6.
- **CSRF custom-header on the write routes** → DEFER. `connect-src 'none'` on artifacts + SameSite=Lax
Authelia cookie + same-origin SPA covers it at single-user scale. Reopen: routes exposed to a less-
controlled origin. Source: security #3.
- **Separate per-file expand-state hook** → REPLACE with local state in GitDiffView (one consumer). Reopen:
a second component needs the same expand state. Source: architect.
- **`autoSelectMode` / `canCommit` as separate files** → keep as inline pure helpers in git_diff.ts (tested),
not separate modules. Source: architect/test-engineer.
## Notes for synthesis
- No `feature-technical-notes.md` (no T# notes) — omit all T# handling.
- Honor: deploy-by-surface (now single surface: docker rebuild), stage-commits-by-path, WS-frame/sessionEvent
parity steps (only a sessionEvent here — `useSidebar.ts` case), pure-helper-then-wire TDD precedent,
`globals:false` + `.js` import suffixes + `src/**/__tests__/**` glob for tests.
- Security posture section: real content (argv-safety, path-traversal, identity, not-in-tool-registry,
artifact-sandbox). On-call resilience section: real content (deadline, index-lock, coalescence,
in-progress, partial-failure). Operational readiness: single deploy surface, no migration.

View File

@@ -0,0 +1,197 @@
# Feature Implementation Plan: Git Diff Panel
A Files / Git tab in the right-side file panel that reads the project repository's diff (two modes) and stages, unstages, commits, and discards whole files — built entirely in `apps/server` ([D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver)), shipped via a single docker rebuild, in two phases (read/display, then write actions) with no schema change, no migration, and no `@boocode/contracts` change.
## Source Specification
- **Feature specification:** [feature-specification.md](feature-specification.md)
- **Specification decision log:** [artifacts/decision-log.md](artifacts/decision-log.md)
- **Specification team findings:** [artifacts/team-findings.md](artifacts/team-findings.md)
- **Specification decisions this plan inherits:** D1D18
- **Specification open items this plan must respect or resolve:** None — the spec's only open item (commit identity) was settled at spec time (F3, D12).
## Outcome
When this plan is executed, the right-side file panel gains a Files / Git tab. Selecting **Git** shows the project repository's changed files (Uncommitted or Committed mode), each expandable to a syntax-highlighted unified diff. The user can stage, unstage, commit (with a server-derived identity), and discard whole files without leaving the session. The work is delivered by a new `apps/server/src/services/git_diff.ts` (read logic + git-write helpers + pure helpers), new read and write routes in `apps/server/src/routes/projects.ts`, and new `apps/web` UI (`GitDiffView`, a tab in `RightRail.tsx`, a `useGitDiff` hook, and a `git_diff_refresh` sessionEvent). No apps/coder change, no Postgres schema change, no new env var.
## Context
- **Driving constraint:** A direct user request (2026-06-02) to add a Paseo-style git diff panel shown "instead of the file browser." Not deadline-bound; scoped to ship as a coherent v1.
- **Stakeholders:** The single session user (reviews and commits their own repo changes in-session). The project's security posture (the write surface must not become an assistant-reachable path). On-call/operability (the panel must not stall on large or slow repos or fail opaquely under index contention).
- **Future-state concern:** The git-write surface now lives in `apps/server`. Watch for a third consumer of git ops (would trigger a shared-`packages/` extraction, [D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver)) and for working-tree diffs routinely exceeding the 10MB read buffer ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
- **Out-of-scope boundary:** No remote operations (push/pull/PR/merge), no side-by-side layout, no per-hunk staging, no continuous file-watch streaming, no rename of the pending-changes panel, no per-line review/re-prompt surface — all deferred in the spec under YAGNI.
## Team Composition and Participation
| Specialist | Status | Key Input |
|------------|--------|-----------|
| `project-manager` | Coordinator | Facilitated R1, corrected the service-owner decision against evidence, synthesized the plan. |
| `software-architect` | Active | Recommended a read-server/write-coder split (R1); the split's premise was refuted by evidence and recorded as the rejected alternative on [D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver). Owns the read shape, base resolution, refresh wiring, two-phase sequencing. |
| `adversarial-security-analyst` | Active | Argv-safety, path-traversal, server-derived identity, tool-registry exclusion, artifact-sandbox evidence ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators)[D-5](artifacts/implementation-decision-log.md#d-5-commit-identity-server-derived-request-schema-strict-with-no-author-fields), [D-13](artifacts/implementation-decision-log.md#d-13-write-endpoints-excluded-from-the-assistant-tool-registry-artifact-sandbox-closes-the-indirect-path)). |
| `on-call-engineer` | Active | Read deadline + maxBuffer, index-lock 409, refresh coalescence, in-progress detection, partial-failure honesty ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes), [D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame)). |
| `test-engineer` | Active | T1T12: pure-helper units + temp-repo integration; skip Shiki/layout (no web harness). |
| `junior-developer` | Reframer | Flagged the cross-service coupling of the write-in-coder split, which seeded the evidence correction on [D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver); confirmed the read-only-session Git tab needs no extra label (F21). |
## Implementation Approach
Both the git read and the git write operations live in `apps/server` ([D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver)) — reusing the existing safe argv `runGit` and the `project_bootstrap.ts` server-side git-write precedent, and avoiding a cross-service proxy hop, a second deploy surface, and coder coupling. The feature reuses Shiki (`lang:'diff'`) already in `apps/web`, the `useProjectGit` dirty signal, and the existing `RightRail.tsx` file-panel host; it introduces one new server service, new routes, one new web view + hook, and one new client `sessionEvents` event.
### Architecture and Integration Points
- **New `apps/server/src/services/git_diff.ts`** — read logic, git-write helpers, and TDD-first pure helpers (`parseNameStatus`, `splitDiffByFile`, `resolveCommittedBase`, `autoSelectMode`, `classify`, `detectInProgress`) ([D-2](artifacts/implementation-decision-log.md#d-2-read-route--git_diffts-pure-helpers-tdd-first)). `autoSelectMode` and `canCommit` are inline tested helpers, not separate modules ([D-12](artifacts/implementation-decision-log.md#trivial-decisions)).
- **New routes in `apps/server/src/routes/projects.ts`**, beside `GET /api/projects/:id/git`: read `GET /api/projects/:id/git/diff?mode=<uncommitted|committed>` ([D-2](artifacts/implementation-decision-log.md#d-2-read-route--git_diffts-pure-helpers-tdd-first)); writes for stage / unstage / commit / discard ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators)). The repository root is derived server-side via `path_guard.ts` `resolveProjectRoot`, never from the request ([D-4](artifacts/implementation-decision-log.md#d-4-path-validation--repo-relative-pathguard-reject-escape-and-repo-root-discard)).
- **Frontend** — a Files / Git tab in the existing `RightRail.tsx` header (replacing the static "Files" label, fitting one line); a new `GitDiffView` in the same slot as the file tree; a `useGitDiff` hook; the dirty dot fed by `useProjectGit`'s existing `is_dirty` ([D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)). Per-file expand/collapse is local state in `GitDiffView`, not a shared hook ([D-11](artifacts/implementation-decision-log.md#trivial-decisions)).
### Data Model and Persistence
None. The panel reads git state at request time and writes the project repository directly via git; nothing is persisted in Postgres — no schema change, no migration, no new env var ([D-10](artifacts/implementation-decision-log.md#trivial-decisions)).
### Runtime Behavior
- **Read:** the read route runs argv `runGit` calls under a 30s deadline and a 10MB `maxBuffer` ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)); `parseNameStatus` builds the file list, `splitDiffByFile` segments the unified diff, `classify` marks binary/large bodies, `detectInProgress` reads `.git` sentinels, and the response carries the resolved base label and the in-progress flag.
- **Mode/base:** `autoSelectMode` picks Uncommitted (dirty) or Committed (clean) on first open only ([D-12](artifacts/implementation-decision-log.md#trivial-decisions)); `resolveCommittedBase` resolves `@{upstream}``origin/HEAD` → null, falling back to a labeled Uncommitted on null ([D-6](artifacts/implementation-decision-log.md#d-6-committed-mode-base-resolution-via-upstream--originhead--null)).
- **Refresh:** the client `git_diff_refresh` sessionEvent fires on the five triggers, coalesced behind an in-flight ref so a running refresh absorbs later triggers ([D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame)).
- **Diff render:** `GitDiffView` highlights a file's diff via Shiki `lang:'diff'` lazily on expand, with a per-file loading state ([D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)).
### External Interfaces
New HTTP routes only (read + four writes) under `/api/projects/:id/git/…`. Refresh is a client-side `sessionEvents` event (`git_diff_refresh`), **not** a WS frame, so `@boocode/contracts` is not touched or rebuilt; the only parity step is a no-op `case 'git_diff_refresh'` in `useSidebar.ts` `applyEvent` ([D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame), [D-9](artifacts/implementation-decision-log.md#trivial-decisions)).
## Decomposition and Sequencing
Two phases on a single deploy surface ([D-14](artifacts/implementation-decision-log.md#d-14-two-phase-build--readdisplay-phase-1-then-write-actions-phase-2)); both ship via `docker compose up --build -d boocode`.
| # | Work Unit | Delivers | Depends On | Verification |
|---|-----------|----------|------------|--------------|
| 1 | `git_diff.ts` pure helpers (TDD-first) | `parseNameStatus`, `splitDiffByFile`, `resolveCommittedBase`, `autoSelectMode`, `classify`, `detectInProgress` ([D-2](artifacts/implementation-decision-log.md#d-2-read-route--git_diffts-pure-helpers-tdd-first), [D-6](artifacts/implementation-decision-log.md#d-6-committed-mode-base-resolution-via-upstream--originhead--null)) | — | Unit tests T1T7 |
| 2 | Read route `GET /api/projects/:id/git/diff?mode=` | Diff payload (files, counts, base label, in-progress flag) under 30s/10MB bounds ([D-2](artifacts/implementation-decision-log.md#d-2-read-route--git_diffts-pure-helpers-tdd-first), [D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)) | 1 | Temp-repo integration T8 |
| 3 | `useGitDiff` + RightRail Files/Git tab + dirty dot | Read-only panel, tab switch, dirty indicator from `useProjectGit` ([D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)) | 2 | Manual (no web harness) |
| 4 | `GitDiffView` read-only + lazy Shiki + refresh wiring | Per-file expand, lazy `lang:'diff'`, `git_diff_refresh` sessionEvent + `useSidebar.ts` no-op case, coalescence ref ([D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame), [D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)) | 3 | Manual |
| 5 | Write helpers + routes (stage/unstage/commit/discard) | Argv-safe writes with `--` separators, `pathGuard`, server-derived `-c` identity, `.strict()` commit schema, 409 on index-lock ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators), [D-4](artifacts/implementation-decision-log.md#d-4-path-validation--repo-relative-pathguard-reject-escape-and-repo-root-discard), [D-5](artifacts/implementation-decision-log.md#d-5-commit-identity-server-derived-request-schema-strict-with-no-author-fields), [D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)); endpoints kept out of `ALL_TOOLS` ([D-13](artifacts/implementation-decision-log.md#d-13-write-endpoints-excluded-from-the-assistant-tool-registry-artifact-sandbox-closes-the-indirect-path)) | 1, 2 | Temp-repo integration T9T12 |
| 6 | Write affordances + in-progress disable | Stage/unstage/commit/discard UI, tracked/untracked discard confirmation, in-progress disable ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes), [D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)) | 4, 5 | Manual |
Phase 1 = units 14 (read + display); Phase 2 = units 56 (write actions).
## RAID Log
### Assumptions
| ID | Assumption | What Changes If Wrong | Verifier | Status |
|----|------------|-----------------------|----------|--------|
| A1 | The boocode container keeps the `/opt:/opt` read-write mount, so apps/server can write project paths. | [D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver) reopens — writes would need a host service. | `docker-compose.yml:16` | Verified (R1) |
| A2 | A realistic single-user working-tree diff stays under 10MB. | The read truncates/errors; the streaming reader reopens (Deferred YAGNI). | `on-call-engineer` | Unverified — bounded by [D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes) |
### Dependencies
| ID | Dependency | Owner | Status |
|----|------------|-------|--------|
| Dep1 | Shiki `^1.29.2` (`lang:'diff'`) already in `apps/web`. | `apps/web` | Present (no install) |
| Dep2 | `useProjectGit` already returns `is_dirty` for the dirty dot. | `apps/web` | Present (no new fetch) |
## Testing Strategy
Sourced from `test-engineer` (T1T12). Server-side only — `apps/web` has no test harness, so Shiki rendering and layout/tab behavior are verified manually, not in the suite. Vitest conventions: `globals:false` (import `describe`/`it`/`expect`), `.js` import suffixes, include glob `src/**/__tests__/**/*.test.ts`.
- **Observable behaviors to test:** porcelain → file-list parse with change types (T1); unified-diff split per file (T2); base resolution `@{upstream}``origin/HEAD` → null (T3); auto-select mode by dirty/clean (T4); binary/large classify (T5); in-progress sentinel detection (T6); `canCommit` gating (T7); read route over a temp repo (T8); stage/unstage round-trip (T9); commit with server-derived identity (T10); discard tracked vs untracked (T11); path-guard rejection of `..`/absolute/symlink-escape and repo-root discard (T12).
- **Test doubles posture:** pure helpers tested directly with fixture strings (no git spawn); route/write tests use a real temp repo via `mkdtemp` + `git init` (the `path_guard.test.ts` integration pattern).
- **Edge cases requiring coverage:** binary file, oversized diff, no resolvable base, in-progress state, index-lock 409, untracked-discard partial failure, path-escape attempts ([D-4](artifacts/implementation-decision-log.md#d-4-path-validation--repo-relative-pathguard-reject-escape-and-repo-root-discard), [D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
- **Test levels:** unit (T1T7, pure helpers) + integration (T8T12, temp-repo). No end-to-end / web-layer automation.
## Security Posture
`adversarial-security-analyst` contributed the full write-surface posture. Threat vectors and the mitigations this plan commits to:
- **Command/flag injection** — every git invocation uses discrete argv with explicit `--` separators between options and user-supplied paths and a flag-injection guard (reject path args starting with `-`); never `hostExec(shell)` ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators)). User text (commit message, file targets) is always discrete argv.
- **Path traversal** — `pathGuard(repoRoot, file)` resolves each per-file argument and rejects absolute paths, `..` traversal, and symlink escape outside the server-derived root; discarding the repo root (`.`) is rejected ([D-4](artifacts/implementation-decision-log.md#d-4-path-validation--repo-relative-pathguard-reject-escape-and-repo-root-discard)). The root is derived from the project record via `path_guard.ts`, never the request.
- **Identity spoofing** — commit identity is server-derived (`-c user.name`/`-c user.email` from git config, falling back to the `project_bootstrap.ts` constants); the commit request schema is Zod `.strict()` with `{message, files?}` only — no author/email/date field can be supplied ([D-5](artifacts/implementation-decision-log.md#d-5-commit-identity-server-derived-request-schema-strict-with-no-author-fields)).
- **Assistant-driven invocation** — the write endpoints are never registered in `ALL_TOOLS`, so no assistant tool surface reaches them; the indirect artifact path is already closed by the artifact iframe's `connect-src 'none'` sandbox (F1), so no new CSRF mitigation is built ([D-13](artifacts/implementation-decision-log.md#d-13-write-endpoints-excluded-from-the-assistant-tool-registry-artifact-sandbox-closes-the-indirect-path)).
## Operational Readiness
- **Deploy surface:** single — `docker compose up --build -d boocode` (rebuilds web + server from the working tree). No `sudo systemctl restart boocoder`, because no apps/coder code changes ([D-1](artifacts/implementation-decision-log.md#d-1-both-git-read-and-git-write-live-in-appsserver)).
- **Schema / migration / env:** none — no Postgres change, no migration, no new env var ([D-10](artifacts/implementation-decision-log.md#trivial-decisions)).
- **Contracts package:** not touched — refresh is a client `sessionEvents` event, not a WS frame, so `packages/contracts/` is not rebuilt ([D-9](artifacts/implementation-decision-log.md#trivial-decisions)).
- **Feature flag / rollout:** none — the panel is additive and inert until the Git tab is opened; rollback is reverting the build.
## On-Call Resilience Posture
`on-call-engineer` contributed; each commitment maps to a flagged failure mode.
- **Timeouts and deadlines:** the read path runs under a 30s execution deadline and a 10MB `maxBuffer`, both distinct from the D5 per-file display-size cap; a read that exceeds the deadline exits loading, shows an error, and offers Refresh ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
- **Retry strategy:** none on the server. An index-lock write failure returns **HTTP 409 "repository busy"**; retry is user-driven, so no timer state or hidden busy state is introduced ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
- **Concurrency / coalescence:** the client holds an in-flight coalescence ref so concurrent `git_diff_refresh` triggers absorb into the running refresh — one read at a time, settling to a single final snapshot ([D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame)).
- **Graceful degradation:** `detectInProgress` (`.git` MERGE_HEAD / rebase / CHERRY_PICK_HEAD / BISECT_LOG sentinels) folds an in-progress flag into the read response; the client disables write affordances when set, so stage/commit/discard never fail with raw git errors mid-operation ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
- **Data integrity:** an untracked-file discard that fails partway reports honest partial failure on the next refresh rather than claiming an unenforceable "state unchanged" ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes)).
## Definition of Done
- [ ] Opening the Git tab shows the project repo's changed files in the auto-selected mode; switching to Files restores the tree in the same slot ([D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)).
- [ ] Committed mode labels its resolved base (`@{upstream}``origin/HEAD` → labeled Uncommitted fallback) ([D-6](artifacts/implementation-decision-log.md#d-6-committed-mode-base-resolution-via-upstream--originhead--null)).
- [ ] Stage / unstage / commit / discard operate at whole-file granularity in Uncommitted mode; discard prompts the tracked vs untracked confirmation; Committed mode is read-only ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators), [D-15](artifacts/implementation-decision-log.md#d-15-frontend--rightrail-filesgit-tab-gitdiffview-with-lazy-shiki-dirty-dot-from-useprojectgit)).
- [ ] Commit identity is server-derived; the request cannot set it; the schema is `.strict()` ([D-5](artifacts/implementation-decision-log.md#d-5-commit-identity-server-derived-request-schema-strict-with-no-author-fields)).
- [ ] Path-escape and repo-root discard are rejected; writes use argv + `--` separators ([D-3](artifacts/implementation-decision-log.md#d-3-write-ops-via-argv-safe-rungitexecfile-with----separators), [D-4](artifacts/implementation-decision-log.md#d-4-path-validation--repo-relative-pathguard-reject-escape-and-repo-root-discard)).
- [ ] Write endpoints are absent from `ALL_TOOLS` ([D-13](artifacts/implementation-decision-log.md#d-13-write-endpoints-excluded-from-the-assistant-tool-registry-artifact-sandbox-closes-the-indirect-path)).
- [ ] Read respects the 30s/10MB bounds; index-lock returns 409; in-progress disables writes; refresh coalesces ([D-7](artifacts/implementation-decision-log.md#d-7-read-deadline-30s--maxbuffer-10mb-index-lock--http-409-in-progress-detection-disables-writes), [D-8](artifacts/implementation-decision-log.md#d-8-refresh-via-client-git_diff_refresh-sessionevent-coalesced--not-a-ws-frame)).
- [ ] Tests T1T12 pass (`pnpm -C apps/server test`).
- [ ] Phase 1 ships and is reviewable before Phase 2 lands ([D-14](artifacts/implementation-decision-log.md#d-14-two-phase-build--readdisplay-phase-1-then-write-actions-phase-2)).
- [ ] Post-ship owner: the session user (single-user app); no separate on-call rotation.
## Specialist Handoffs for Implementation
- **`test-engineer`** — dispatch at the start of unit 1 to own T1T12; needs the helper signatures from `git_diff.ts` and the temp-repo integration pattern (`path_guard.test.ts`).
- **`adversarial-security-analyst`** — dispatch to review unit 5 before merge; needs the write-route handlers and the `.strict()` commit schema to confirm argv-safety, path-guard coverage, and identity derivation.
- **`user-experience-designer`** — dispatch during units 34 and 6; needs the `RightRail.tsx` header layout to confirm one-line fit (D18), tap-target minimum, and the tracked/untracked discard wording.
## Deferred (YAGNI)
### Shared `packages/` runGit extraction
- **Why deferred:** Rule of Three not met — only apps/server consumes the git ops for this feature (coder is untouched); a single-consumer abstraction is premature.
- **Reopen when:** a third consumer needs the same git ops.
- **Source:** R1, software-architect.
### New WS frame for refresh
- **Why deferred:** Replaced by the simpler client `sessionEvents` event `git_diff_refresh` — no trigger needs a server push, so a WS frame and a `@boocode/contracts` rebuild are unnecessary (simpler-version test).
- **Reopen when:** a refresh trigger genuinely originates server-side (an out-of-band repo mutation the client cannot observe).
- **Source:** R1, software-architect / junior-developer.
### Server-side retry on index-lock
- **Why deferred:** "Try again" is user-driven; a server retry hides the busy state and adds timer state (evidence test — no observed contention).
- **Reopen when:** index-lock contention is observed frequent in practice.
- **Source:** R1, on-call-engineer (F5).
### Streaming diff reader (cap mid-stream)
- **Why deferred:** `execFile` maxBuffer 10MB covers any realistic single-user working-tree diff; ~4050 LoC for a transient memory spike that does not matter at this scale (simpler-version test).
- **Reopen when:** working-tree diffs routinely exceed 10MB.
- **Source:** R1, on-call-engineer.
### CSRF custom-header on the write routes
- **Why deferred:** `connect-src 'none'` on artifacts + the SameSite=Lax Authelia cookie + a same-origin SPA cover it at single-user scale (evidence test — no exposed cross-origin path).
- **Reopen when:** the write routes are exposed to a less-controlled origin.
- **Source:** R1, adversarial-security-analyst.
### Separate per-file expand-state hook
- **Why deferred:** Replaced by local state in `GitDiffView` — one consumer (single-implementation abstraction).
- **Reopen when:** a second component needs the same expand state.
- **Source:** R1, software-architect.
### `autoSelectMode` / `canCommit` as separate modules
- **Why deferred:** Kept as inline tested pure helpers in `git_diff.ts` — splitting into modules adds files without a second consumer (simpler-version test).
- **Reopen when:** a second module needs to import them independently.
- **Source:** R1, software-architect / test-engineer.
## Open Items
- None. Every Round-1 open question resolved from evidence; the spec's only open item (commit identity) was settled at spec time. The plan is shippable as written.
## Summary
- **Outcome delivered:** A Files / Git tab in the right-side file panel that reads and (whole-file) stages, unstages, commits, and discards the project repository's changes, built entirely in `apps/server`.
- **Team size:** 6 specialists — see [artifacts/implementation-iteration-history.md](artifacts/implementation-iteration-history.md)
- **Rounds of facilitation:** 1 — see [artifacts/implementation-iteration-history.md](artifacts/implementation-iteration-history.md)
- **Decisions committed:** 15 — see [artifacts/implementation-decision-log.md](artifacts/implementation-decision-log.md)
- **Decisions settled by evidence:** 15 — see [artifacts/implementation-decision-log.md](artifacts/implementation-decision-log.md)
- **Decisions settled by junior-developer reframing:** 0 (the junior-developer's coupling flag seeded the evidence correction on D-1, but the resolution was evidence) — see [artifacts/implementation-decision-log.md](artifacts/implementation-decision-log.md)
- **Decisions settled by user input:** 0 — see [artifacts/implementation-decision-log.md](artifacts/implementation-decision-log.md)
- **Rejected alternatives recorded:** 22 — see [artifacts/implementation-decision-log.md](artifacts/implementation-decision-log.md)
- **Open items remaining:** 0
- **Recommendation:** Ship as planned — build Phase 1 (units 14) first, then Phase 2 (units 56).