boocode/docs/features/git-diff-panel/artifacts/synthesis-input.md

# 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.