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.
This commit is contained in:
Yaojia Wang
235
docs/plans/w6-project-git-panel.md
Normal file
235
docs/plans/w6-project-git-panel.md
Normal file
@@ -0,0 +1,235 @@
|
||||
# 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) — G1–G7 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 6–12 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.
|
||||
Reference in New Issue
Block a user