Files
web-terminal/docs/plans/w6-project-git-panel.md
Yaojia Wang 8fe1f52e5d feat(projects): show real git state on the project detail page
The page carried one bare `●` for "dirty" and nothing else, so "do I have
commits I haven't pushed" and "which worktree am I in" still meant dropping
into a terminal. Design mock: docs/mockups/project-detail-git.html; plan and
task breakdown (G1-G7): docs/plans/w6-project-git-panel.md.

One rule drives the whole feature: ahead/behind compare against `@{u}`, a
LOCALLY CACHED remote ref that only a fetch moves. This repo was the live
example while building — `↑9` true, `↓0` false, because FETCH_HEAD had not
moved in 19 days. So:

  - `ahead` needs only local refs and is never flagged.
  - `behind` is flagged `stale` once FETCH_HEAD is older than an hour.
  - Exactly ONE state may render green: ↑0 ↓0 AND a fresh fetch. Green means
    "I checked, ignore this"; getting it wrong is lying to the user.
  - No upstream (the normal state of a fresh worktree branch) leaves ahead and
    behind undefined — it renders an explicit `no upstream`, never the green
    path. That fall-through is the easiest bug to ship here.

What landed:

G1  SyncState (upstream/ahead/behind/lastFetchMs/detached) + ProjectDetail.sync
    and .dirtyCount. All additive and optional — the Android and iOS clients
    decode these shapes. The ahead/behind helper already existed for the list
    view; buildProjectDetail had simply never called it.
    Fixes a pre-existing bug on the way: readBranch read <repo>/.git/HEAD
    directly, so it returned nothing inside a LINKED worktree, where .git is a
    file. resolveGitDirs now resolves both the per-worktree gitdir (HEAD) and
    the shared common dir (FETCH_HEAD).

G2  POST /projects/git/fetch. Same discipline as push: the remote is derived
    server-side and no remote or refspec is ever read from the body, so a
    client cannot aim it at an arbitrary URL. Touches refs/remotes only — no
    working tree, no index, no merge; it is not a pull. Own rate-limit bucket
    so refreshes cannot eat the budget a real push needs. On failure
    lastFetchMs is left alone, so the UI keeps saying "stale" instead of
    pretending it refreshed.

G3  makeSyncBand replaces the bare dot: upstream name, ↑n, ↓n, stale flag,
    dirty count, Fetch button (disabled on a detached HEAD).

G4  The commit list marks unpushed commits and draws the upstream boundary
    once, after the last of them. Marking is server-side from `rev-list`,
    deliberately NOT "the first N rows": `git log` is date-ordered, so merging
    an older branch interleaves unpushed commits BELOW pushed ones, and that
    shortcut fails in the dangerous direction — calling an unpushed commit
    pushed. A regression test builds exactly that backdated-merge shape.

G5  The worktree section is always "Worktrees (n)" (it used to rename itself
    to "Branch" at n=1) and the current row carries its own state chips.

G6  Cost control. The plan called for a .git-mtime cache; that was dropped
    during implementation because a fingerprint over HEAD/index/reflog does
    NOT move when push updates a remote-tracking ref — the cached `ahead`
    would still claim "9 to push" right after a successful push, which is the
    exact lie the feature exists to prevent. Replaced with three measures that
    cannot go stale: in-flight coalescing (N devices watching one repo cost
    one probe, entry dropped as it settles, nothing cached across time),
    skipping the re-render when the payload is byte-identical (this also stops
    the 5 s re-mount of the commit log, two more git spawns per tick), and
    pausing the timer while the document is hidden.

G7  Per-worktree state via GET /projects/worktree/state, kept narrower than
    /projects/detail so N rows do not pay for worktree listing and CLAUDE.md
    reads nothing renders. Needed an unplanned prerequisite: ProjectSessionRef
    carried no cwd, so sessions could not be attributed to a worktree. Added
    it, plus countSessionsByWorktree, which matches DEEPEST-first because
    .claude/worktrees/<name> lives INSIDE the main checkout and prefix
    matching would count every worktree session against the parent repo too.

Out of scope, unchanged: no reset, no checkout, no clean, no rebase, no
force-push. stage/commit/push stay exactly as they were.

Verified: tsc and build clean; 46 new tests.
2026-07-29 17:12:00 +02:00

236 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# w6 — Project detail: the Git panel
Design mock: [`docs/mockups/project-detail-git.html`](../mockups/project-detail-git.html)
Make "do I have commits I haven't pushed / which worktree am I in" **ambient** on the
project detail page — visible without typing a git command. Read-mostly: the panel's
job is to *tell you what happened*, not to replace the editor in handling it.
## Design rule that drives everything (read first)
`ahead`/`behind` compare against `@{u}` — a **locally cached** remote ref. Without a
fetch it never changes. Right now this repo reports `↓0` while `.git/FETCH_HEAD` is
19 days old: the number is a lie, and a naive panel would render it as a confident
"in sync".
Therefore:
- **`↑ ahead` is always trustworthy** (it only needs local refs) → never flagged.
- **`↓ behind` is only as fresh as the last fetch** → flagged `stale` once
`FETCH_HEAD` mtime exceeds `FETCH_STALE_MS` (1 h).
- **Exactly one state may render green: `↑0 ↓0` AND fetch within the threshold.**
Everything else is neutral or warning. Green here means "I checked, ignore this" —
getting it wrong is lying to the user.
- **No upstream ⇒ no `↑↓` at all**, rendered as an explicit `no upstream` chip.
A new worktree branch has no upstream, so `ahead`/`behind` are `undefined` — never
let "no number" fall through to the green path. This is the single easiest bug to ship.
## Contract
### Types (`src/types.ts`)
New, and **additive only**`android/api-client/.../Projects.kt` and
`ios/App/WebTerm/ViewModels/ProjectDetailViewModel.swift` decode these shapes.
Never rename or remove an existing field; every new field is optional.
```ts
/** G1: upstream sync state for one repo/worktree. Every field degrades
* independently (no upstream / detached HEAD / empty repo / never fetched). */
export interface SyncState {
upstream?: string; // "origin/develop"; undefined ⇒ no upstream configured
ahead?: number; // commits on HEAD not on @{u}
behind?: number; // commits on @{u} not on HEAD — only as fresh as lastFetchMs
lastFetchMs?: number; // .git/FETCH_HEAD mtime; undefined ⇒ never fetched
detached?: boolean; // HEAD is detached ⇒ no branch, no ahead/behind
}
```
`ProjectDetail` gains:
```ts
sync?: SyncState; // git repos only; undefined for non-git dirs
dirtyCount?: number; // `git status --porcelain` line count (gated by projectDirtyCheck)
```
`dirty?: boolean` **stays** (mobile clients read it). `dirtyCount` is the additive refinement.
### Log response (`src/http/git-log.ts`)
`CommitLogEntry` gains `unpushed?: boolean`; `GitLogResult` gains `upstream?: string`.
**Marking happens server-side**, not on the client. (The first draft of this plan
shipped the SHA set to the client and matched there — wrong split: `%h` is
abbreviated while `rev-list` yields full SHAs, and the two can disagree on width,
so the matching rule belongs next to the data that defines it.) The server matches
by prefix, bounded by `GIT_LOG_MAX × UNPUSHED_MAX` comparisons.
> **Why a second spawn and not "the first `ahead` rows".** `git log` is date-ordered;
> an unpushed merge can pull in commits with older dates that interleave below pushed
> ones. "First N are unpushed" is wrong in exactly the repo shape we use (merge-per-worktree),
> and it fails in the dangerous direction — labelling an unpushed commit as pushed.
> Use `git rev-list @{u}..HEAD --max-count=200`.
The client draws the boundary only when `upstream` is present, exactly once, after
the last marked row; `unpushed` is honoured only when strictly `true`.
### New route (`src/server.ts` + `src/http/git-ops.ts`)
`POST /projects/git/fetch``{ path }``{ ok, lastFetchMs }`.
- Same three-prong path check as the other git routes (absolute + dir + has `.git`, SEC-H7).
- **Server-derived target**, exactly like `push`: never accept a remote or refspec
from the client. `git fetch --no-tags --quiet` (current branch's remote, else the
sole remote; ≥2 remotes without an upstream → 400).
- `GIT_TERMINAL_PROMPT=0` in the child env, plus an explicit timeout — a fetch that
hits a credential prompt otherwise hangs the request forever. (**Correction:**
`push` already had this via `NO_PROMPT_ENV`; only `fetch` needed wiring.)
- Fetch only updates remote-tracking refs: no working tree, no index, no merge.
It stays inside the "no destructive ops" line.
### Out of scope (unchanged)
No `reset`, no branch `checkout`, no `clean`, no rebase, no force-push.
`stage` / `commit` / `push` stay exactly as they are in `diff.ts`.
## Tasks
| ID | What | Owns |
|----|------|------|
| **G1** | `SyncState` + `dirtyCount`; wire `readSync` into `buildProjectDetail`; add `readUpstream` / `readLastFetch` | `src/types.ts`, `src/http/projects.ts` |
| **G2** | `POST /projects/git/fetch` + `fetch()` in the git write engine | `src/http/git-ops.ts`, `src/server.ts` |
| **G3** | Sync band in the detail header (replaces the bare `●`) + staleness rule | `public/projects.ts`, `public/style.css` |
| **G4** | Unpushed marking + `origin/x` boundary in the commit list | `src/http/git-log.ts`, `public/git-log.ts`, `public/style.css` |
| **G5** | Worktree panel: always "Worktrees", per-row branch/path/dirty/↑↓ | `public/projects.ts`, `public/style.css` |
| **G6** | Cost controls: per-repo dedupe, `.git` mtime gate, pause while hidden | `src/http/projects.ts`, `public/projects.ts` |
| **G7** | Per-worktree dirty/↑↓ — **lazy**, only for an expanded row | `src/http/worktrees.ts`, `public/projects.ts` |
Order: **G1 → G2 → (G3, G4) → G5 → G6 → G7.** G3 needs G1's data and G2's button target.
G7 is the most expensive and least essential — it ships last or not at all.
### Status (2026-07-29) — G1G7 all shipped
tsc + build clean; **46 new tests, all green** (2128 → 2174), and `npm test` is
green end to end.
**Correction to an earlier note in this file:** the three residual
`test/integration/server.test.ts` failures were first written off as "environment,
not code" because of their `[needs real PTY (sandbox-off)]` labels. That was wrong
`PTY_AVAILABLE` is **true** here, so those cases were running, not skipping, and
failing on timing. See "Test-suite flakiness" below.
Deviations worth knowing:
- `readBranch` read `<repo>/.git/HEAD` directly, so it returned nothing inside any
linked worktree (there `.git` is a file). Fixed by `resolveGitDirs()` — a
pre-existing bug this feature would otherwise have inherited.
- **G6 dropped the `.git`-mtime cache from the plan.** A fingerprint over
HEAD/index/reflog does *not* move when `git push` updates a remote-tracking ref,
so a cached `ahead` would keep claiming "9 to push" right after a successful
push — the exact confident lie this feature exists to prevent. Replaced with two
measures that cannot go stale: **in-flight coalescing** (N devices watching one
repo cost one probe, entry dropped the instant it settles) and **skip-render when
the payload is byte-identical** (which also stops the 5 s re-mount of the commit
log, i.e. two `git` spawns per tick). Plus pausing the timer while the document
is hidden.
- **G7 needed an unplanned prerequisite:** `ProjectSessionRef` carried no `cwd`, so
sessions could not be attributed to a worktree. Added it, plus
`countSessionsByWorktree` — which does DEEPEST-match, because
`.claude/worktrees/<name>` lives *inside* the main checkout and prefix matching
would count every worktree session against the parent repo too.
- G7 probes each non-current worktree once per opened project rather than
on-expand: with the state cached client-side and skipped on unchanged ticks, the
cost is the same and the information is there without a click.
- **Test-suite flakiness (pre-existing, now fixed).** Two independent causes:
1. *Fixtures outgrew vitest's 5 s default.* The git ones spawn 612 sequential
`git` processes; the real-server ones boot a server and a shell. Gave those
`describe`s an explicit `{ timeout: 30_000 }` and the real-PTY waits a named
`PTY_WAIT_MS`. 11 failures → ~1.
2. *The real-PTY E2E file cannot share the machine with the rest of the suite.*
`test/integration/server.test.ts` passed alone (27/27, repeatedly) and flaked
in the full run: it asserts on real prompt output within seconds, which is not
achievable while ~8 workers saturate the box. Raising the numbers further only
moved the flake around, so `npm test` now runs **two passes**`test:unit`
(everything else, parallel) then `test:e2e` (that file alone). `vitest run`
still runs everything at once for anyone who wants it.
- Also bounded the `srv.close()` in H1's `finally`. Unbounded, it swallowed the
body's real error and reported a bare timeout instead — the diagnosis above
only became possible after making the failure legible.
## Cost control (G6 — the real risk)
Only two reads are free: branch (`.git/HEAD`) and last-fetch (`FETCH_HEAD` mtime).
Everything else spawns `git`, on the same machine that is running Claude Code and builds.
**As shipped:**
1. **In-flight coalescing by repo path** — concurrent probes of one repo share a
single promise, dropped as soon as it settles. N devices ⇒ one probe, and
nothing is cached across time.
2. **Skip the render when the payload is unchanged** — the 5 s tick used to rebuild
the whole subtree, re-mounting the commit log and re-running `/projects/log`
(two more spawns) to redraw identical rows.
3. **Pause while hidden**`document.visibilityState`; a backgrounded tab or a
phone with the screen off stops probing entirely.
4. Per-worktree state (G7) is probed once per opened project, never per tick.
**Rejected: the `.git` mtime gate from the original plan.** A fingerprint over
HEAD / index / reflog does not change when `push` moves a remote-tracking ref, so
the cached `ahead` would still read "9 to push" immediately after pushing. Cheaper
than coalescing, and wrong in precisely the way this whole feature is built to
avoid.
## TDD steps
Repo style: pure-unit in `test/*.test.ts`, real-server in `test/integration/*`.
**G1**`test/projects.test.ts`
1. RED: `buildProjectDetail` on a fixture repo with an upstream → `sync.ahead === n`, `sync.upstream === 'origin/<b>'`.
2. RED: repo with **no upstream**`sync.upstream === undefined` **and** `sync.ahead === undefined` (guards the green-path bug).
3. RED: detached HEAD → `sync.detached === true`, no branch, no ahead/behind.
4. RED: never fetched → `sync.lastFetchMs === undefined` (not `0`, not `Date.now()`).
5. RED: `dirtyCount` matches porcelain line count; `dirty` still boolean-true alongside.
6. RED: non-git dir → `sync === undefined`, and no `git` is spawned.
**G2**`test/http/git-ops.test.ts` + `test/integration/projects-endpoint.test.ts`
7. RED: fetch rejects a path outside the allowed roots (SEC-H7) → 400, no spawn.
8. RED: client-supplied `remote` / `refspec` in the body is **ignored**, not forwarded.
9. RED: ≥2 remotes and no upstream → 400 with a plain message (never raw stderr, SEC-M10).
10. RED: success → `lastFetchMs` advances.
**G3**`test/projects-panel.test.ts`
11. RED: `↑0 ↓0` + fresh fetch → the single green "in sync" chip.
12. RED: `↑0 ↓0` + `lastFetchMs` older than `FETCH_STALE_MS`**not** green; `↓` carries `stale`.
13. RED: `upstream === undefined``no upstream` chip, **no** `↑↓`, **not** green.
14. RED: `detached === true` → short SHA, no branch chip, fetch button disabled.
15. RED: `dirtyCount === 3``● 3`, not a bare dot.
**G4**`test/git-log.test.ts`
16. RED: entries whose SHA is in `unpushed` get the marker; the boundary renders exactly once, after the last of them.
17. RED: `unpushed` empty → no boundary, no markers.
18. RED: `upstream === undefined` → no boundary at all (nothing to draw it against).
19. RED: an unpushed commit that sorts *below* a pushed one (merge of an older branch) is still marked — the Set is authoritative, not the row index.
**G5**`test/projects-panel.test.ts`
20. RED: 1 worktree still renders the section (title always "Worktrees"), with its path and session count.
21. RED: a worktree with no upstream shows `no upstream`, never a green chip.
**G6**`test/projects.test.ts`
22. RED: two `buildProjectDetail` calls for the same repo with an unchanged `.git` mtime spawn `git` once.
## Edge cases
- Empty repo (no commits): no `↑↓`, no commit list, no boundary; not green.
- Shallow clone: `rev-list` counts are still meaningful vs `@{u}`; don't special-case.
- `@{u}` set but the remote branch was deleted upstream → `rev-list` fails → all
fields `undefined` → renders `no upstream`. Acceptable; do not invent a number.
- Fetch in flight: disable the button, don't queue a second one.
- Fetch failing (offline / auth) → surface a short message, leave `lastFetchMs` unchanged
so the `stale` flag stays on. Never silently mark it fresh.
## Security
- New route reuses the existing gate: auth token check + the three-prong path check.
- No client-controlled remote/refspec, no shell, `--` terminators, `GIT_TERMINAL_PROMPT=0`.
- Errors are plain sentences, never raw git stderr (SEC-M10).
- Adds **no new capability** — anyone who can reach the port already has a shell.