indifferentketchup
ca028a4024
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>
98 lines
7.4 KiB
Markdown
98 lines
7.4 KiB
Markdown
# 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` D1–D18, `team-findings.md` F1–F21). 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`).
|