docs(plans): Wave 5 implementation plans (fan-out board, access token, android parity)
This commit is contained in:
Yaojia Wang
155
docs/plans/w5-access-token.md
Normal file
155
docs/plans/w5-access-token.md
Normal file
@@ -0,0 +1,155 @@
|
||||
# App-level access token (leave-the-LAN bar-raiser)
|
||||
|
||||
> **Feature id:** `w5-access-token` · **Effort: M** · Branch: `develop`
|
||||
> Adds an **optional** shared secret (`WEBTERM_TOKEN`) that gates remote HTTP + the WS handshake with a constant-time-compared, `HttpOnly`/`SameSite=Strict` cookie. **Unset ⇒ auth disabled**, preserving today's LAN zero-config. The token is **additive** — it sits *in front of* the existing Origin/CSRF model (`isOriginAllowed` at `src/http/origin.ts:22`, `requireAllowedOrigin` at `src/server.ts:480`), never replaces it.
|
||||
|
||||
---
|
||||
|
||||
## Honest tradeoff (read first — belongs in the PR description too)
|
||||
|
||||
This is a **bar-raiser, not a TLS/Tailscale substitute.** On bare LAN the terminal stream is still `ws://` (plaintext) — see `buildWsUrl` at `public/terminal-session.ts:47`, which only upgrades to `wss` when the *page* is HTTPS. When the token travels over `ws://`/`http://`, **anyone sniffing the LAN sees the cookie/token in cleartext.** The token only meaningfully hardens the **relay/tunnel path**, where the edge terminates TLS and the browser speaks `wss://`/`https://`. It is a *single shared secret* (no per-user identity, no revocation except changing the env var + restart, no lockout beyond rate-limiting). Ship it with that framing; do not let it read as "now it's safe on the public internet."
|
||||
|
||||
---
|
||||
|
||||
## Contract (routes / messages / env / types)
|
||||
|
||||
### Env (new — `src/config.ts`)
|
||||
|
||||
| Env var | Type | Default | Meaning |
|
||||
|---|---|---|---|
|
||||
| `WEBTERM_TOKEN` | string \| undefined | **unset** | Shared access token. **Unset/empty ⇒ auth DISABLED** (everything open, exactly as today). When set: **validated at load** — must match `^[A-Za-z0-9._~+/=-]{16,512}$` (cookie/URL-safe charset, min 16 chars). Invalid ⇒ **throw** (fail-fast, like `parsePort`). |
|
||||
| `WEBTERM_TOKEN_TTL` | number (sec) | `2592000` (30d) | *(optional, YAGNI-dial)* Cookie `Max-Age`. Parsed via the existing `parseNonNegativeInt` helper (`src/config.ts:83`). Ship the constant first; only wire the env if trivial. |
|
||||
|
||||
Charset validation is **not cosmetic**: it blocks `Set-Cookie` header/response-splitting injection and query-string ambiguity, and the ≥16 floor multiplies brute-force cost against the rate limiter.
|
||||
|
||||
### Config type (`src/types.ts`)
|
||||
|
||||
Add one field to `Config` (interface at `src/types.ts:21`, alongside the other secret-ish fields near lines 44–47 / 82):
|
||||
|
||||
```
|
||||
readonly webtermToken: string | undefined; // WEBTERM_TOKEN; undefined ⇒ auth disabled (SECRET — never log/expose)
|
||||
```
|
||||
|
||||
Follow the `vapidPrivateKey` precedent (`src/config.ts:349`): read as `env['WEBTERM_TOKEN'] || undefined`, **never** log it, **never** return it over `/config/ui`.
|
||||
|
||||
### New pure module `src/http/auth.ts` (mirrors the shape/discipline of `src/http/origin.ts`)
|
||||
|
||||
```
|
||||
export const AUTH_COOKIE_NAME = 'webterm_auth'
|
||||
export function isAuthEnabled(cfg: Config): boolean // webtermToken != null && != ''
|
||||
export function parseCookieHeader(header: string | undefined): Record<string, string>
|
||||
export function constantTimeEqual(a: string, b: string): boolean // SHA-256 both → timingSafeEqual (fixed-length guard)
|
||||
export function cookieIsAuthed(cfg: Config, cookieHeader: string | undefined): boolean
|
||||
export function buildSetCookie(cfg: Config, opts: { secure: boolean }): string // the Set-Cookie value
|
||||
export function isHttpsRequest(req: IncomingMessage): boolean // x-forwarded-proto==='https' || socket.encrypted
|
||||
```
|
||||
|
||||
- **`constantTimeEqual`** hashes *both* inputs with `crypto.createHash('sha256')` to a fixed 32 bytes, then `crypto.timingSafeEqual`. Hashing-to-fixed-length is the "fixed-length guard": it removes the length side-channel **and** avoids `timingSafeEqual`'s throw-on-length-mismatch. (Present-vs-absent, i.e. undefined/empty cookie, short-circuits to `false` — a missing cookie is not a secret-comparison oracle.)
|
||||
- **`buildSetCookie`** returns:
|
||||
`webterm_auth=<token>; Path=/; Max-Age=<ttl>; HttpOnly; SameSite=Strict` **+ `; Secure`** only when `opts.secure` is true. Dynamic `Secure` is required: a `Secure` cookie is never sent over `ws://`/`http://`, so forcing it would silently break LAN-over-HTTP auth; over the relay (`x-forwarded-proto: https`) it must be present.
|
||||
|
||||
### New/changed HTTP routes (`src/server.ts`)
|
||||
|
||||
| Route | Guard | Behavior |
|
||||
|---|---|---|
|
||||
| `GET /login` | **always reachable** (registered *before* the gate) | Serves the self-contained `public/login.html`. |
|
||||
| `POST /auth` | **always reachable**, rate-limited (`authLimiter`, 10/min/IP) | Body `token` (accepts `urlencoded` *and* `json`, `limit:'1kb'`). Valid ⇒ `Set-Cookie` (`buildSetCookie`) + **302 → `/`** (native-form path) or **204** for XHR. Invalid ⇒ **401** (+ `302 → /login?e=1` for form navigations). Over rate limit ⇒ **429**. |
|
||||
| `GET /?token=<t>` | handled **inside the gate**, rate-limited | Bootstrap link. Valid ⇒ `Set-Cookie` + **302 → same path with `token` stripped** (no token left in history; `Referrer-Policy: no-referrer` already set at `src/server.ts:307`). Invalid ⇒ **302 → /login**. |
|
||||
| **The global auth gate** (`app.use`, new) | — | Runs after the security-headers middleware (`src/server.ts:304`) and **before** `express.static` (`src/server.ts:317`). See allow-list below. |
|
||||
|
||||
### WS handshake (`src/server.ts` upgrade handler, `:1130`)
|
||||
|
||||
After the existing Origin check passes (`:1141–1146`) and **before** `wss.handleUpgrade` (`:1149`), insert:
|
||||
|
||||
> if `isAuthEnabled(cfg)` and **not** `cookieIsAuthed(cfg, req.headers['cookie'])` → `socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); socket.destroy(); return`.
|
||||
|
||||
The browser auto-sends the `HttpOnly` cookie on the same-origin WS handshake — **no frontend change to `buildWsUrl`/`connect()` is required** for the authed path.
|
||||
|
||||
### The gate's allow-list (single central policy point — the `origin.ts:13` idiom)
|
||||
|
||||
In order, the gate:
|
||||
1. `!isAuthEnabled(cfg)` → `next()` *(zero-config LAN unchanged)*.
|
||||
2. `isLoopback(req.socket.remoteAddress)` (`src/server.ts:160`) → `next()` *(loopback hook ingest — `POST /hook` `:533`, `/hook/permission` `:561`, `/hook/status` `:890` — has no cookie and must keep working; the token is about **remote** access).*
|
||||
3. `GET` with `?token=` → validate (rate-limited) → set-cookie+redirect, or → `/login`.
|
||||
4. `cookieIsAuthed(...)` → `next()`.
|
||||
5. else unauthed: `Accept: text/html` navigation → **302 → /login**; otherwise → **401** JSON `{ error: 'authentication required' }`.
|
||||
|
||||
**Scope note (decision to surface at review):** this gate is a **superset** of the ROADMAP's stated "WS + `requireAllowedOrigin` routes." It *also* gates the read-only GET side-channels (`/live-sessions`, `/projects`, **`/projects/diff` — which leaks source**, `/sessions` — which leaks prompts, `/config/ui`, etc.). That is deliberate: for a "don't-expose-a-shell-off-LAN" bar-raiser, leaving source/prompt reads open off-LAN is the bigger hole, and one central gate is simpler + DRYer than threading a token check through ~19 route handlers. `requireAllowedOrigin` is left **untouched** so the CSRF layer stays independent (defense in depth: gate = "are you authorized to be here", Origin = "is this request forged cross-site"). If a reviewer wants the strictly-minimal scope instead, the fallback is a `requireToken(req,res)` helper called only inside `requireAllowedOrigin` + the WS check — but the global gate is recommended.
|
||||
|
||||
---
|
||||
|
||||
## Files to change
|
||||
|
||||
| Path | Concrete change |
|
||||
|---|---|
|
||||
| `src/types.ts` | Add `readonly webtermToken: string \| undefined` to `Config` (interface `:21`, near the secret fields `:44–47`). **Coordination point** — this is the frozen shared contract; do it here, not locally. Optionally add `authRequired?: boolean` to `UiConfig` (`:656`) for the FE expiry hint. |
|
||||
| `src/config.ts` | In `loadConfig` (`:271`): read `WEBTERM_TOKEN` (`env['WEBTERM_TOKEN'] \|\| undefined`), validate charset+min-length when present (throw on bad, like `parsePort` `:170`), add `webtermToken` to the frozen return (`:474`). Never log it. *(Optional: parse `WEBTERM_TOKEN_TTL`.)* |
|
||||
| `src/http/auth.ts` | **NEW.** Pure, dependency-light helpers listed in Contract (`isAuthEnabled`, `parseCookieHeader`, `constantTimeEqual`, `cookieIsAuthed`, `buildSetCookie`, `isHttpsRequest`, `AUTH_COOKIE_NAME`). Imports `createHash`, `timingSafeEqual` from `node:crypto`. No Express/DOM types — keep it unit-testable like `origin.ts`. |
|
||||
| `src/server.ts` | (1) import the auth helpers (near `:36`). (2) Add `AUTH_RATE_MAX = 10` to the rate-limit constants (`:80–86`). (3) Instantiate `const authLimiter = createRateLimiter(AUTH_RATE_MAX, RATE_LIMIT_WINDOW_MS)` (near `:227`). (4) Register `GET /login` + `POST /auth` **then** `app.use(authGate)` between `:313` and `:317`. (5) `authGate` closure implements the allow-list (reuses `isLoopback` `:160`, `requireAllowedOrigin` untouched). (6) WS upgrade: insert the cookie check after Origin (`:1146`, before `:1149`). Serve `login.html` from a startup-cached read of `path.join(publicDir,'login.html')`. |
|
||||
| `public/login.html` | **NEW, fully self-contained.** A `<form method="POST" action="/auth">` with a `type="password"` token field + submit. **Inline `<style>` only, NO inline `<script>`** — the CSP at `src/server.ts:310` is `script-src 'self'` (blocks inline JS) but `style-src 'self' 'unsafe-inline'` (allows inline CSS). Native form POST → server 302 → cookie set → app loads; **needs zero JS**. Show an error banner when `?e=1`. |
|
||||
| `public/terminal-session.ts` | *(OPTIONAL polish, low priority)* On WS close-before-`attached` while auth is enabled, print a `statusLine` (`:38`) hint like "Locked — reload to sign in" instead of silent reconnect loops. Covers the cookie-expired-mid-session case. |
|
||||
| `docs/ROADMAP.md`, `CLAUDE.md`, `.env`/README env list | Tick the ROADMAP item (`:124`); document `WEBTERM_TOKEN` in the env-var list (CLAUDE.md "Planned Commands" block) with the honest-tradeoff sentence. |
|
||||
| `docs/PROGRESS_LOG.md` | Orchestrator appends the completion entry (not the builder). |
|
||||
|
||||
---
|
||||
|
||||
## TDD steps (ordered; matches repo vitest style — pure-unit + `test/integration/*` real-server)
|
||||
|
||||
**Write each test RED first, then implement to GREEN.** Follow AAA and descriptive names, per the repo's `test/origin.test.ts` / `test/integration/server.test.ts` conventions.
|
||||
|
||||
1. **`test/auth.test.ts` (unit, like `origin.test.ts`)**
|
||||
- `constantTimeEqual`: equal strings → `true`; unequal same-length → `false`; **different length → `false`** (no throw); empty/`undefined` → `false`.
|
||||
- `parseCookieHeader`: `'a=1; webterm_auth=xyz; b=2'` → map; missing/empty header → `{}`; malformed pairs ignored.
|
||||
- `isAuthEnabled`: undefined/empty → `false`; set → `true`.
|
||||
- `cookieIsAuthed`: correct cookie → `true`; wrong value → `false`; absent cookie → `false`; disabled cfg → design choice (assert `false` and gate short-circuits before calling it).
|
||||
- `buildSetCookie`: asserts substrings `HttpOnly`, `SameSite=Strict`, `Path=/`, `Max-Age=`; **`Secure` present iff `opts.secure`**; token value present.
|
||||
|
||||
2. **`test/config.test.ts` (extend existing)**
|
||||
- `WEBTERM_TOKEN` unset → `cfg.webtermToken === undefined`.
|
||||
- Valid token (≥16, safe charset) → stored verbatim.
|
||||
- Too short (`'abc'`) → `loadConfig` **throws**.
|
||||
- Bad charset (contains `;`, space, control char) → **throws**.
|
||||
|
||||
3. **`test/integration/auth.test.ts` (NEW real-server, pattern from `server.test.ts`)** — extend `makeTestConfig` to accept `WEBTERM_TOKEN`.
|
||||
- **Regression / disabled:** no token set → WS connects with Origin only; `DELETE /live-sessions` with valid Origin works; `GET /live-sessions` open. *(Proves zero-config LAN is untouched.)*
|
||||
- **WS enabled:** valid Origin + **no cookie → 401** (`waitForOpen` rejects, mirroring the `:262` bad-Origin test); valid Origin + **valid `Cookie: webterm_auth=<t>` → connects → `attached`**; valid Origin + **wrong cookie → 401**.
|
||||
- **`POST /auth`:** wrong token → 401; correct token → 302 + `Set-Cookie` (assert flags; **no `Secure` over http**); `>10` bad attempts/min → **429**.
|
||||
- **`x-forwarded-proto: https`** on `POST /auth` → `Set-Cookie` **includes `Secure`**.
|
||||
- **`GET /?token=<valid>`** → 302 to `/` (Location has no `token`) + `Set-Cookie`; **`?token=<invalid>`** → 302 `/login`, no cookie.
|
||||
- **`GET /login`** → 200 HTML, reachable while unauthed.
|
||||
- **Unauthed HTML nav** (`GET /`, `Accept: text/html`, no cookie) → 302 `/login`; **unauthed XHR** (`GET /live-sessions`, no cookie) → 401 JSON.
|
||||
- **Gate + CSRF stacking:** `DELETE /live-sessions` valid Origin, **no cookie → 401** (gate); with cookie → passes gate, then Origin check as before.
|
||||
- **Loopback bypass:** `POST /hook` from 127.0.0.1 with **no cookie** still returns 204 (hooks unaffected). *(Guard against regressing the side-channel.)*
|
||||
|
||||
4. Run `npm test` (vitest) + `npm run typecheck`; confirm the 80% coverage gate holds for `src/http/auth.ts` and the new server branches.
|
||||
|
||||
---
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **Cookie-less non-browser clients (curl/scripts):** already rejected by the WS Origin check (undefined Origin → `false`, `origin.ts:26`); the token gate adds a second wall for HTTP. Intended.
|
||||
- **Cookie expiry mid-session:** loaded FE keeps a live WS but new WS reconnects/`/config/ui` fetches start 401ing → the OPTIONAL `terminal-session.ts` hint tells the user to reload. No crash.
|
||||
- **`SameSite=Strict` + bootstrap link:** the `?token=` response *sets* the cookie and 302-redirects; the follow-up top-level navigation to `/` carries it (Strict permits top-level same-site sends). Works.
|
||||
- **Token with URL-reserved chars:** prevented by the config-load charset validation (no `%`-encoding ambiguity, no cookie/header injection).
|
||||
- **`?token=` on a non-GET or with a body:** gate only intercepts `?token=` for GET; other methods fall through to the cookie check.
|
||||
- **PWA offline shell after expiry:** `sw.js` may serve the cached shell offline (same-origin, no data) but the WS still 401s — no data leak.
|
||||
- **Rate-limiter memory:** reuses the existing in-memory sliding-window `createRateLimiter` (`:118`); per-IP arrays are pruned per call — no unbounded growth for the auth endpoint beyond active IPs.
|
||||
- **Login page assets vs. gate:** `login.html` must reference **no** external CSS/JS (inline-styles-only) so the gate can 401 everything else including `/build/main.js` while unauthed.
|
||||
- **Trailing-slash / `/index.html`:** treat both `/` and `/index.html` navigations the same in the "HTML nav → /login" branch.
|
||||
|
||||
## Security
|
||||
|
||||
- **Constant-time compare** via SHA-256→`timingSafeEqual` (no length or content timing oracle; no throw). ✔ security.md secret-handling.
|
||||
- **Cookie flags:** `HttpOnly` (JS can't read the token → XSS can't exfiltrate it), `SameSite=Strict` (cross-site pages can't ride the cookie — complements the Origin/CSWSH defense and `requireAllowedOrigin`), `Secure`-when-https, `Path=/`.
|
||||
- **Rate limiting** on `/auth` and `?token=` (10/min/IP) raises brute-force cost; combined with the ≥16-char token floor this is not trivially guessable. No account lockout (single shared secret) — documented.
|
||||
- **Secret hygiene:** `webtermToken` never logged (follows `vapidPrivateKey` at `:349`), never returned by `/config/ui` (`:1099`). Charset validation blocks `Set-Cookie`/response-splitting injection.
|
||||
- **Additive, non-breaking:** Origin check (`origin.ts:22`) and `requireAllowedOrigin` (`:480`) are unchanged; the gate is a new earlier layer. Loopback hook ingest is explicitly bypassed so the smart-features side-channel keeps working.
|
||||
- **Honest boundary (repeat in code comments + PR):** plaintext on bare `ws://` — token is a relay/tunnel hardener, **not** a TLS/Tailscale replacement. Keep the "never port-forward this raw" guidance from TECH_DOC §7 intact.
|
||||
- **Security-review trigger:** this is auth + cookie + crypto code → run the `security-reviewer` before merge per code-review.md.
|
||||
|
||||
## Effort & dependencies
|
||||
|
||||
- **Effort: M** (matches ROADMAP `:129`). No new npm deps — `node:crypto` (`timingSafeEqual`/`createHash`) and Express's built-in `urlencoded`/`json` parsers cover it. `~1` new pure module + `~1` new HTML file + surgical server wiring.
|
||||
- **Hard dependencies:** none — self-contained; does not block on the fan-out board or Android parity.
|
||||
- **Coordination:** touches two shared files — `src/types.ts` (frozen contract; the `Config.webtermToken` add is the coordination point) and `src/server.ts` (shared wiring). If run in parallel with the other Wave-5 tasks, use `isolation: worktree` and land the `types.ts` bump first so the server change type-checks. `PROGRESS_LOG.md` is orchestrator-written.
|
||||
- **Cross-cutting risk to watch:** the CSP `script-src 'self'` constraint (`:310`) forces the login page to be JS-free (native form) — easy to get wrong by reaching for inline `<script>`; the integration test `GET /login → 200` plus a manual load catches it.
|
||||
153
docs/plans/w5-android-parity.md
Normal file
153
docs/plans/w5-android-parity.md
Normal file
@@ -0,0 +1,153 @@
|
||||
# Android Projects / Diff / Worktree screens (client parity)
|
||||
|
||||
> **Feature id:** `w5-android-parity` · **Branch:** `develop` · **Server changes:** ZERO (every route below already ships from Wave 1-4).
|
||||
|
||||
## Reality check (read before planning — the task's premise is stale)
|
||||
|
||||
The task brief says the Android UI modules are "SDK-gated / commented out in `settings.gradle.kts`". **That is no longer true on `develop`.** Verified on disk:
|
||||
|
||||
- `android/settings.gradle.kts:50-53` **includes** `:app`, `:terminal-view`, `:host-registry`, `:client-tls-android` (uncommented). The `// TODO(android-sdk)` gating the README (`android/README.md:20-27`) still describes is stale doc drift.
|
||||
- The SDK **is installed** and proven: `android/local.properties` → `sdk.dir=/usr/local/share/android-commandlinetools`; `platforms/{android-35,android-36}` present. `PROGRESS_ANDROID.md:358-382` records a full green gate (`./gradlew test :app:assembleDebug koverVerify` → ~484 tests, APK builds). The whole Android client (A1–A36) is committed (`e254918`).
|
||||
- `:api-client` is **not** empty (only the `.gitkeep` placeholders remain beside real code): `models/Projects.kt`, `routes/{ApiClient,Endpoints,ApiRoute}.kt` etc. exist.
|
||||
|
||||
**So the actual parity gap is not "bring modules online" — it is: the shipped Android client was built to iOS P0+P1, which *deliberately excluded* the git-write surface.** `ANDROID_CLIENT_PLAN.md:79` — "*worktree-create — server has routes the iOS client does not consume; out of parity scope*"; `:233` lists `POST /projects/worktree` among routes "explicitly NOT consumed"; `:218` scopes diff to `staged=1|0` only (no `?base=`); `:221-227` lists only **3** guarded routes (kill / hook.decision / put-prefs). Everything Wave 1-4 added on the web/server is missing on Android:
|
||||
|
||||
| Server route (exists) | src/server.ts | Android today | Gap |
|
||||
|---|---|---|---|
|
||||
| `GET /projects/diff?…&base=<rev>` | 811-837 | `:app` `HttpDiffFetcher` sends `staged` only (`DiffViewModel.kt:313-322`) | **base-diff** |
|
||||
| `GET /projects/log?path=&n=` | 843-861 | — | **recent commits** |
|
||||
| `GET /projects/pr?path=` | 871-887 | — | **PR + CI chip** |
|
||||
| `POST /projects/worktree` | 908-934 | worktrees shown **read-only** (`ProjectDetailScreen.kt:116-159`) | **create** |
|
||||
| `DELETE /projects/worktree` | 937-964 | — | **remove** |
|
||||
| `POST /projects/worktree/prune` | 967-986 | — | **prune** |
|
||||
| `POST /projects/git/{stage,commit,push}` | 997-1096 | Diff is inert read-only | **stage/commit/push** |
|
||||
| `GET /projects` sync fields `ahead/behind/lastCommitMs` | types.ts:337-339 | `ProjectInfo` (Android) lacks them | **sync chip** |
|
||||
|
||||
This plan closes exactly that gap. It is the largest Wave 5 item but **narrower than "build the client"** — the foundation (grouping, prefs round-trip, detail page, diff flatten/render, adaptive nav, mTLS transport, design system) is done and reused verbatim.
|
||||
|
||||
---
|
||||
|
||||
## Contract (routes / messages / env / types)
|
||||
|
||||
No new server routes, no env vars, no `:wire-protocol` (frozen) changes. All additions are Kotlin client code.
|
||||
|
||||
### Route home decision (security-driven — do not deviate)
|
||||
The Origin header (CSWSH 铁律) is stamped in **exactly one place**: `ApiRoute.toHttpRequest()` when `originPolicy == GUARDED` (`android/api-client/.../routes/ApiRoute.kt:61-71`; ARCHITECTURE §4.3, checklist `:467`). Therefore **every state-changing route MUST be added to `:api-client`** (worktree create/remove/prune, git stage/commit/push) so it flows through that single Origin-stamping point. The RO additions (`/projects/pr`, `/projects/log`) also go in `:api-client` for consistency with the existing RO methods (`ApiClient.kt:34-96`). The one exception is **diff-vs-base**: the diff route already lives self-contained in `:app` (`DiffViewModel.kt` — A24 put it there because it is read-only and needs no Origin), so extend it in place rather than churning it into `:api-client`.
|
||||
|
||||
### New `:api-client` route builders — `routes/Endpoints.kt` (mirror the existing 26-93)
|
||||
```
|
||||
// RO (no Origin)
|
||||
fun projectPr(path: String): ApiRoute // GET /projects/pr?path=<enc> READ_ONLY
|
||||
fun projectLog(path: String, n: Int?): ApiRoute // GET /projects/log?path=<enc>[&n=<int>] READ_ONLY
|
||||
// G (Origin byte-equal) — bodies are application/json, encoded via ModelJson
|
||||
fun createWorktree(path, branch, base: String?): ApiRoute // POST /projects/worktree
|
||||
fun removeWorktree(path, worktreePath, force: Boolean): ApiRoute // DELETE /projects/worktree (BODY on DELETE — see Edge cases)
|
||||
fun pruneWorktrees(path): ApiRoute // POST /projects/worktree/prune
|
||||
fun gitStage(path, files: List<String>, stage: Boolean): ApiRoute // POST /projects/git/stage
|
||||
fun gitCommit(path, message: String): ApiRoute // POST /projects/git/commit
|
||||
fun gitPush(path): ApiRoute // POST /projects/git/push
|
||||
```
|
||||
Reuse the existing `percentEncode` (strict RFC 3986 unreserved, `Endpoints.kt:102-120`) for `path`/`n` query values and `ModelJson.encodeToString` for bodies. `n` clamps client-side to `1..GIT_LOG_MAX` mirror (server re-clamps; send it raw-but-bounded).
|
||||
|
||||
### New `:api-client` models (`models/`, all `@Serializable`, tolerant decode via `ModelJson` / `LossyDecode`, mirror types.ts)
|
||||
- `PrStatus` + `PrAvailability` (string-union → enum with unknown→`ERROR`) + `PrCheckSummary` — types.ts:560-588. **Every field except `availability` optional**; `availability` unknown/missing → `ERROR` (never throw).
|
||||
- `CommitLogEntry { hash, at: Long, subject }` + `GitLogResult { commits, truncated }` — types.ts:695-704 (list-lossy: drop malformed commit, keep rest).
|
||||
- `GitWriteOutcome` — a **client result union** for the guarded git ops carrying the server's *safe* body: `Ok(payload)` vs `Rejected(status, message)`. Server always returns `{ ok, … }` on 200 and `{ ok:false, error:"<safe string>" }` / `{ error }` on failure (git-ops.ts:81-119, worktrees.ts:201-341 — never raw stderr). Decode `error` as an **inert** string to display.
|
||||
- stage 200: `{ ok, staged: Boolean, count: Int }`
|
||||
- commit 200: `{ ok, commit: String }` (short sha; may be `""`)
|
||||
- push 200: `{ ok, branch, remote }`
|
||||
- worktree create 200: `{ ok, path, branch }`; remove 200: `{ ok, path }`; prune 200: `{ ok, pruned: List<String> }`
|
||||
- Extend `models/Projects.kt` `ProjectInfo` with `ahead: Int?`, `behind: Int?`, `lastCommitMs: Long?` (types.ts:337-339). Extend the `:app` `DiffResult` (`DiffViewModel.kt:215`) with `base: String? = null` (types.ts:553).
|
||||
|
||||
### New `ApiClient` methods (`routes/ApiClient.kt`) + status mapping
|
||||
- `suspend fun projectPr(path): PrStatus` — 200→tolerant decode; 400→`ProjectPathInvalid`; 404→`ProjectNotFound`; else `UnexpectedStatus`. (PR *degrade* — gh missing/unauth/no-PR — is `availability` inside a **200** body, not an HTTP status.)
|
||||
- `suspend fun projectLog(path, n): GitLogResult` — 200→decode; 400→`ProjectPathInvalid`; 404→`ProjectNotFound`; 500→`ProjectDetailUnavailable`(reuse) or new `GitLogUnavailable`.
|
||||
- Guarded ops return `GitWriteOutcome`: 200→`Ok(...)`; **403→`Rejected` reading body.error** (disabled *and* Origin-fail both 403 — see Security); 400/404/409→`Rejected(status, body.error)`; 429→`RateLimited`; else `UnexpectedStatus`. New `ApiClientError` cases as needed: `WorktreeDisabled`, `GitOpsDisabled` are **not** distinguishable from Origin-403 by status alone → prefer surfacing `Rejected(status, safeMessage)` over inventing typed variants (`ApiClientError.kt:11-48` is the sealed set to extend minimally).
|
||||
|
||||
### `:app` presenters (plain, JVM-testable — same pattern as `DiffViewModel`/`ProjectDetailViewModel`, NOT `androidx.lifecycle.ViewModel`)
|
||||
- `WorktreeViewModel(gateway, repoPath)`: `create(branch, base?)`, `remove(worktreePath, force)`, `prune()`. Each is a phase machine (`Idle/Working/Done/Failed(message)`) that **re-fetches project detail** on success so the worktree list refreshes. Branch name validated client-side (mirror `validateBranchName`, worktrees.ts:95) before any I/O.
|
||||
- Extend `ProjectsGateway` (`ProjectsViewModel.kt:400-413`) with the new gateway methods; `ApiClientProjectsGateway` delegates to the new `ApiClient` methods. (Keeps the VM JVM-tested against a fake.)
|
||||
- Extend `DiffViewModel` (`DiffViewModel.kt:47`): add `base: String?` state + `setBase(rev)`; when `base != null` the `staged` toggle is suppressed (server ignores `staged` when `base` set — server.ts:831 / public/diff.ts:112-129). Extend `diffUrl` (`:313`) to append `&base=<enc>` and **omit `staged`** in base mode. Add stage/commit/push: `toggleStage(file, staged)`, `commit(message)`, `push()` via the guarded gateway; these are **only offered in working/staged mode**, never base mode (parity with public/diff.ts:321).
|
||||
- `PrChipState` (fold into `ProjectDetailViewModel` or a small `PrViewModel`): fetch `/projects/pr` lazily on detail load; render one chip.
|
||||
|
||||
---
|
||||
|
||||
## Files to change
|
||||
|
||||
| Path | Concrete change |
|
||||
|---|---|
|
||||
| `android/api-client/.../routes/Endpoints.kt` | Add 8 builders (pr, log, worktree×3, git×3). RO ones `READ_ONLY`; write ones `GUARDED` with `ModelJson` bodies. Reuse existing `percentEncode`/`HEX`. |
|
||||
| `android/api-client/.../routes/ApiClient.kt` | Add `projectPr`, `projectLog`, `createWorktree`, `removeWorktree`, `pruneWorktrees`, `gitStage`, `gitCommit`, `gitPush` with the status mapping above. |
|
||||
| `android/api-client/.../routes/ApiClientError.kt` | Add minimal cases only if needed (`GitLogUnavailable`); prefer `Rejected(status,msg)` carried in `GitWriteOutcome` over per-error variants. |
|
||||
| `android/api-client/.../models/PrStatus.kt` (new) | `PrStatus`/`PrAvailability`/`PrCheckSummary` tolerant. |
|
||||
| `android/api-client/.../models/GitLog.kt` (new) | `CommitLogEntry`/`GitLogResult` list-lossy. |
|
||||
| `android/api-client/.../models/GitWrite.kt` (new) | `GitWriteOutcome` + per-op payloads; serializers registered in `models/Serializers.kt` if custom. |
|
||||
| `android/api-client/.../models/Projects.kt` | Add `ahead`/`behind`/`lastCommitMs` to `ProjectInfo`. |
|
||||
| `android/app/.../viewmodels/WorktreeViewModel.kt` (new) | Create/remove/prune phase machine; branch validation; re-fetch detail on success. |
|
||||
| `android/app/.../viewmodels/ProjectsViewModel.kt` | Extend `ProjectsGateway` + `ApiClientProjectsGateway` with the new methods; add sync-chip copy to `ProjectsCopy`. |
|
||||
| `android/app/.../viewmodels/ProjectDetailViewModel.kt` | Compose in worktree actions + PR chip fetch + recent-commits fetch (each independent, failure-isolated). |
|
||||
| `android/app/.../viewmodels/DiffViewModel.kt` | Add `base` mode (+`setBase`), extend `diffUrl` with `&base=`, add `toggleStage/commit/push` via a guarded `GitWriteGateway`; extend `DiffResult` with `base`. |
|
||||
| `android/app/.../screens/ProjectDetailScreen.kt` | Add: "New worktree" branch-input sheet (+optional base), per-worktree **remove** (force-confirm dialog, block `isMain`), **prune** button, a **PR chip** (tappable only if `url` parses `https`), a **recent commits** section. Keep all server strings inert `Text` (§8). |
|
||||
| `android/app/.../screens/DiffScreen.kt` | Add a base-rev input (a third mode beside Working/Staged), per-file **Stage/Unstage** buttons in working/staged only, a commit-message field + **Commit**/**Push** buttons with result banners. |
|
||||
| `android/app/.../screens/ProjectsScreen.kt` | Optional: render sync chip (ahead/behind) on project cards from the new `ProjectInfo` fields. |
|
||||
| `android/app/.../nav/NavGraph.kt` | The diff/detail routes already exist (`:94-131`); no new route needed — worktree/commit UIs are in-place sheets/dialogs, not new destinations. Verify `ProjectDetailPane`/`DiffPane` pass the new gateway. |
|
||||
| `android/app/.../designsystem/*` | Reuse existing tokens (`WebTermColors.statusWorking/statusStuck`, `WebTermCard`, `Spacing`). PR-check pass/fail/pending → existing status colors. No new tokens. |
|
||||
| test files (see TDD) | New/extended JVM unit tests under `api-client/src/test/...` and `app/src/test/.../viewmodels/`. |
|
||||
| `android/README.md` | Fix the stale "SDK-gated / COMMENTED OUT" section (`:12-27`) to match reality. |
|
||||
| `docs/PROGRESS_LOG.md` | Orchestrator appends the W5 entry (not the builder). |
|
||||
|
||||
---
|
||||
|
||||
## TDD steps (ordered — JUnit5 + `runTest`/`StandardTestDispatcher` + Turbine + MockK, matching `DiffViewModelTest`/`ApiRouteShapeTest`)
|
||||
|
||||
Write test → RED → implement → GREEN, bottom-up (pure `:api-client` first, then `:app` presenters, Compose deferred to device QA per plan §7).
|
||||
|
||||
**Phase A — `:api-client` routes + models (fully green + Kover ≥80% in one pass)**
|
||||
1. `models/PrStatusTest.kt` — decode a full `availability:"ok"` body; unknown `availability` → `ERROR`; missing fields tolerated; `PrCheckSummary` counts; non-object → error/`availability=ERROR` (mirror `HappyPathDecodeTest`/`TolerantDecodeTest`).
|
||||
2. `models/GitLogTest.kt` — decode `{commits,truncated}`; drop a commit missing `hash`/`at`, keep siblings; `truncated` passthrough.
|
||||
3. `models/GitWriteTest.kt` — decode each 200 payload; decode failure body `{ok:false,error:"…"}` → `Rejected(status,message)`.
|
||||
4. `routes/GitRouteShapeTest.kt` (extend `ApiRouteShapeTest`) — **Origin-iff-guarded**: `projectPr`/`projectLog` carry **no** Origin; worktree/git-write carry `Origin == endpoint.originHeader` byte-equal; assert method+path+query encoding (`?path=` strict-encoded; `&n=`); assert `DELETE /projects/worktree` **carries a JSON body**.
|
||||
5. `routes/ApiClientGitTest.kt` — with `FakeHttpTransport` queue canned responses: pr 200/400/404; log mapping; each guarded op 200→`Ok`, 403→`Rejected(body.error)`, 429→`RateLimited`, 409→`Rejected`. Assert the fake **received** the Origin header on writes and **not** on reads.
|
||||
6. Run `./gradlew :api-client:test :api-client:koverVerify` → green.
|
||||
|
||||
**Phase B — worktree + diff-base presenters (`:app`, green)**
|
||||
7. `viewmodels/WorktreeViewModelTest.kt` — invalid branch name → `Failed` with no I/O; create success → `Done` + detail re-fetch invoked; remove of `isMain` blocked; force flag threaded; 403-disabled → `Failed(safeMessage)`; 429 → rate-limited copy.
|
||||
8. Extend `DiffViewModelTest.kt` — `setBase("main")` builds `…/projects/diff?path=…&base=main` with **no** `staged`; base mode hides the staged toggle; `diffUrl` percent-encodes `base`; base-mode disables stage/commit/push.
|
||||
|
||||
**Phase C — git-write from diff + PR/log (`:app`, green)**
|
||||
9. Extend `DiffViewModelTest.kt` — `toggleStage(file,true)` posts `{path,files:[newPath],stage:true}` and refreshes; `commit("msg")` → `Ok(sha)` banner; empty message rejected client-side or surfaces server 400 message; `push()` → `Ok(branch,remote)`; 409 push-rejected → inert server message.
|
||||
10. `viewmodels/ProjectDetailPrLogTest.kt` — PR fetch failure does **not** fail the detail load (isolated); `availability != ok` renders degraded copy; recent-commits list decodes + isolates its own error.
|
||||
11. Run `./gradlew test :app:testDebugUnitTest :app:assembleDebug koverVerify` → green.
|
||||
|
||||
**Device QA (deferred — no emulator/Firebase/host here; append to `android/DEVICE_QA_CHECKLIST.md`):** worktree-create sheet + list refresh, remove force-confirm dialog, Stage/Unstage button layout, commit/push banners, PR chip tap→browser (https-only), base-rev input, sync-chip rendering, adaptive-pane behavior.
|
||||
|
||||
---
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **DELETE with a request body.** `DELETE /projects/worktree` reads `req.body` (server.ts:943-945), but the current Android transport has only tested body-less DELETE (`killSession`). **Verify `OkHttpHttpTransport` actually transmits a body on DELETE** (some stacks drop it) — add a transport-level test if not covered. This is the single highest-risk integration gotcha.
|
||||
- **403 is overloaded.** `requireAllowedOrigin` failure *and* `worktreeEnabled==false`/`gitOpsEnabled==false` both return **403** (server.ts:909-911, 938-939, 998-1000). The client cannot tell them apart by status → must decode `body.error` and surface it inertly; do **not** assume 403 means "re-pair / bad Origin".
|
||||
- **No capability-discovery endpoint.** `GET /config/ui` exposes only `allowAutoMode`/`costBudgetUsd` (server.ts:1099-1106) — it does **not** report `worktreeEnabled`/`gitOpsEnabled`/`GH_ENABLED`. So the client discovers "disabled" only via a 403 at action time; after a disabled-403, hide/disable that control for the session.
|
||||
- **PR degrade lives in the body, not the status.** `gh` missing/unauth/no-PR all return **200** with `availability∈{not-installed,unauthenticated,no-pr,disabled,error}` (types.ts:560-566). Render one chip from `availability`; never treat non-`ok` as an HTTP error.
|
||||
- **`staged` vs `base` precedence.** When `base` is set the server ignores `staged` (server.ts:831); mirror by omitting `staged` and hiding the toggle in base mode (public/diff.ts:112-129).
|
||||
- **Rate limits (429).** stage/commit share `gitWriteLimiter`; push has a tighter `gitPushLimiter` (server.ts:1003, 1075). Surface `RateLimited` copy; do not auto-retry.
|
||||
- **Empty commit sha.** `git commit` may return `{ok:true, commit:""}` (git-ops.ts:252) — render "committed" without a sha, don't crash on empty.
|
||||
- **Base-rev injection.** `?base=` must be inert and pass the server's `isPlausibleRev` (server.ts:826); the client sends it percent-encoded and lets the server 400 junk — surface that 400's message.
|
||||
- **Detail refresh race.** A worktree create/remove followed by a detail re-fetch can race a concurrent poll; cancel the in-flight fetch (the `job?.cancel()` pattern in `DiffViewModel.kt:78-82`).
|
||||
- **Force-remove of dirty worktree.** Server 409 "uncommitted changes — force required" (worktrees.ts:278) → the remove dialog must offer an explicit **Force** re-confirm, and must block `isMain` client-side (worktrees.ts:328).
|
||||
- **Tolerant decode everywhere.** A partial/malformed PR/log/diff body must degrade (drop-one-keep-rest), never throw — reuse `LossyDecode`/`ModelJson` discipline (`ApiClient.kt:26-28`).
|
||||
|
||||
## Security
|
||||
|
||||
- **Origin 铁律 preserved.** All six write routes go through `ApiRoute.toHttpRequest` GUARDED stamping (`ApiRoute.kt:61-71`) — the single CSWSH defense (ANDROID_CLIENT_PLAN `:467`). The two new reads (pr/log) MUST NOT carry Origin. A route-shape test asserts both directions (test step 4) so a future reclassification goes red, not silently wrong.
|
||||
- **Untrusted server strings stay inert.** Branch names, worktree paths, commit subjects, PR titles, diff lines, `error` messages are rendered as plain Compose `Text` — no `linkify`/Markdown/`AnnotatedString` autolink (ANDROID_CLIENT_PLAN `:476`, plan §8). The **PR chip is the one exception**: a tappable link **only when `url` parses as `https`** — validate scheme before making it clickable (`:476`).
|
||||
- **No credential leakage / no cache.** All calls ride the shared mTLS `OkHttpClient` with `.cache(null)` (`:470`) — diff/commit bodies can contain secrets; never persist them.
|
||||
- **Client-side pre-validation (defense in depth).** Branch name (`validateBranchName` mirror), non-empty commit message, `isAbsoluteCwd` for any path minted into an action (`ProjectsViewModel.kt:139-143` precedent). The server re-validates + realpath-contains regardless (git-ops.ts / worktrees.ts) — the client checks are UX, not the security boundary.
|
||||
- **Never log** worktree paths, commit messages, PR titles, or error bodies.
|
||||
|
||||
## Effort & dependencies
|
||||
|
||||
- **Effort: L (largest Wave 5 item), ~7-9 pd** for the buildable+tested layer — *below* the brief's 8-12 pd because the client scaffold, grouping, prefs round-trip, diff flatten/render, adaptive nav, mTLS transport and design system already exist and are reused verbatim (the app already builds to an APK). The remaining work is ~8 REST routes + ~6 models + 3-4 presenters + ~4 screen edits + ~40-50 tests. Add device-QA time separately (not doable in this env).
|
||||
- **Dependencies:** none external — every server route shipped in Wave 1-4 (server.ts:811-1096), so this is a pure client pass with **zero server change**. Depends only on the existing frozen `:wire-protocol` `HostEndpoint`/`HttpTransport` and `:api-client` (both stable). `develop` must contain the Android tree from `e254918` (it does).
|
||||
- **Can one builder pass get a full green Gradle build? Yes for the logic+compile layer, but phase it into 3 green checkpoints — do not attempt it monolithically.** A single pass can realistically finish **Phase A (api-client)** entirely green with Kover ≥80%. Phases B and C compile (`:app:assembleDebug`) and pass JVM unit tests, but their Compose surfaces (sheets, dialogs, buttons, chip, banners) are **device-QA-deferred by definition** (plan §7 — no emulator/Firebase/host in this env), exactly as every prior Android wave deferred rendering. So a realistic single-builder deliverable is: **`./gradlew test :app:assembleDebug koverVerify` green**, with the interactive git-write flows (create a worktree, stage→commit→push from a phone, tap a PR chip) recorded in `DEVICE_QA_CHECKLIST.md` rather than proven here. Recommended phasing: A (api-client routes/models/tests) → B (worktree + diff-base presenters) → C (git-write-from-diff + PR/log + screen wiring), each ending on a green build so review and rollback stay cheap. The stage→commit→push-from-the-diff-viewer flow (Phase C) is the largest and riskiest single chunk; keep it last and isolated.
|
||||
- **First action for the builder:** run `cd android && ./gradlew test :app:assembleDebug` to confirm the baseline is green on `develop` before adding anything (the toolchain is proven but unverified on this exact checkout), and fix the stale `README.md` no-SDK section as part of the pass.
|
||||
157
docs/plans/w5-fanout-board.md
Normal file
157
docs/plans/w5-fanout-board.md
Normal file
@@ -0,0 +1,157 @@
|
||||
# Worktree fan-out board (parallel agent lanes)
|
||||
|
||||
**Feature id:** `w5-fanout-board` · **Branch base:** `develop` · **Effort: L** · Depends on Wave 2 (PTY-inject / `initialInput`) + Wave 4 (worktree create + remove).
|
||||
|
||||
Fan **one task** across **N** branch/agent lanes of one repo: create N worktrees, spawn N Claude sessions each pre-injected with the same prompt, watch them race side-by-side in the existing split-grid board, approve/kill per lane, then **keep the winner** (its tab + worktree stay) while **losers get worktree-remove**. This is **≈90% composition of shipped parts**; the only genuinely new code is a small pure launch-command builder, an in-memory (localStorage-persisted) lane-group model, and one thin read-only grouping endpoint.
|
||||
|
||||
> **Byte-shuttle + no-clobber preserved:** the server never learns "fan-out" semantics on the terminal stream. Each lane is its **own worktree → own PTY** (`createWorktree` at `src/http/worktrees.ts:224` gives a fresh dir; `attach(null)` spawns a fresh PTY there), so two Claudes editing the same repo never touch the same working tree.
|
||||
|
||||
## New vs reused
|
||||
|
||||
| Concern | Reused (no change) | New (thin) |
|
||||
|---|---|---|
|
||||
| Spawn N worktrees | `createWorktree` (`worktrees.ts:224`) via `POST /projects/worktree` (`server.ts:908`) | orchestrator loops the route N× (sequential) |
|
||||
| Spawn N sessions w/ prompt | `openProject`→`addEntry`→`initialInput` (`tabs.ts:836,704,717`) → typed after `attached` (`terminal-session.ts:374`, `INITIAL_INPUT_DELAY_MS=700` `:30`); `claude "<prompt>"` pattern proven at `projects.ts:958` | `buildFanoutCmd(prompt,mode)` — pure, shell-quotes the prompt |
|
||||
| Watch board | `setGridLayout('grid-4'\|'grid-6')` (`tabs.ts:1020`), per-quadrant approve/maximize/monitor (`renderInlineApprove`/`toggleMaximize`/`toggleMonitor` `tabs.ts:1364,1081,1088`), statusLine gauges (`renderCell` `:1328`, `tab-gauge` `:1451`) | per-cell "🏆 Keep" button (same pattern as `cell-max` wiring `tabs.ts:776-801`) |
|
||||
| Discard losers | `removeWorktreeReq`/`confirmAndRemoveWorktree` (`projects.ts:299,349`) → `DELETE /projects/worktree` (`server.ts:937`) | batch-confirm loop in `keepFanoutWinner` |
|
||||
| Grouped discovery | `manager.list()`/`GET /live-sessions` (`manager.ts:185`, `server.ts:332`); `LiveSessionInfo.cwd` (`types.ts:299`) | `GET /live-sessions/grouped` + pure `groupSessionsByRepo` |
|
||||
|
||||
**Merge of the winner:** the winner's session/worktree **stays open** so the user runs the merge inside it (the ROADMAP + task design collapse "keep winner" to "winner session stays"). A one-click merge is **out of scope for v1** (needs conflict handling); an optional thin `merge()` in `git-ops.ts` is sketched under Effort as a follow-up.
|
||||
|
||||
---
|
||||
|
||||
## Contract
|
||||
|
||||
### New HTTP route (`src/server.ts`) — the one thin server piece
|
||||
|
||||
| Method / path | Guard | Response |
|
||||
|---|---|---|
|
||||
| `GET /live-sessions/grouped` | none (read-only aggregate, same threat model as `/live-sessions` `:332` and `/digest` `:340`) | `200 SessionGroup[]` |
|
||||
|
||||
Pure read over `manager.list()`; **no** Origin guard, **no** new write path. Grouping is **string-only** (no `git` exec): fan-out worktrees always live under `<repo>-worktrees/` (createWorktree base, `worktrees.ts:151-152`), so sessions whose `cwd` shares that parent cluster into one group. Registered beside `/live-sessions` (`server.ts:332`).
|
||||
|
||||
### `src/types.ts` (coordination edit — the frozen shared contract)
|
||||
|
||||
```ts
|
||||
// group of running sessions sharing a repo/worktree-root (fan-out discovery)
|
||||
export interface SessionGroup {
|
||||
repoRoot: string; // derived repo dir (parent of the *-worktrees folder, or the cwd)
|
||||
label: string; // basename(repoRoot)
|
||||
sessions: LiveSessionInfo[]; // members, newest-first (already the manager.list order)
|
||||
}
|
||||
// launch options the FE passes to TabApp.launchFanout (FE-internal, but typed shared)
|
||||
export interface FanoutLaunchOpts {
|
||||
prompt: string;
|
||||
lanes: number; // 2..maxFanoutLanes
|
||||
branchBase: string; // slug; lanes get `${branchBase}-lane-${i}`
|
||||
mode?: PermissionMode; // reuse types.ts:445
|
||||
}
|
||||
```
|
||||
Add `maxFanoutLanes?: number` to **`UiConfig`** (`types.ts:656`) so the FE stepper max is server-controlled.
|
||||
|
||||
### `src/config.ts` env var (additive, optional)
|
||||
|
||||
| Env | Field (add to `Config` `types.ts:26` block) | Default | Parser |
|
||||
|---|---|---|---|
|
||||
| `MAX_FANOUT_LANES` | `maxFanoutLanes: number` | `6` (matches `grid-6` capacity, `grid-layout.ts:30`) | `parseNonNegativeInt` (`config.ts:83`, as `maxSessions` `:297`) |
|
||||
|
||||
Effective N is `min(opts.lanes, cfg.maxFanoutLanes, 6, maxSessions − liveCount)` — bounded by grid capacity and the DoS cap (`assertUnderSessionCap`, `manager.ts:106`).
|
||||
|
||||
### Client-side contract
|
||||
|
||||
- **No new `ClientMessage`/`ServerMessage`.** The stream stays a byte-shuttle. Launch = N existing `POST /projects/worktree` + N existing WS attaches (via `openProject`). Discard = existing `DELETE /projects/worktree`.
|
||||
- **New `ProjectsHooks` method** (`projects.ts:39`): `onFanout: (repoPath: string, repoName: string, opts: FanoutLaunchOpts) => void` — mirrors the existing `onOpenProject` hook exactly; wired in `tabs.ts` constructor (`:180`) to `this.launchFanout(...)`.
|
||||
- **New pure launch-cmd builder** `buildFanoutCmd(prompt, mode, allowAutoMode): string` → `claude [--permission-mode <m>] '<shell-quoted, newline-collapsed prompt>'\r`. Single-quote wrap with `'` → `'\''` escaping; collapse `\r?\n`→space; cap length (`FANOUT_PROMPT_MAX = 4000`). Reuses `resolveMode`/`buildClaudeCmd` shape (`tabs.ts:659,664`).
|
||||
|
||||
---
|
||||
|
||||
## Files to change
|
||||
|
||||
| Path | Concrete change |
|
||||
|---|---|
|
||||
| `src/types.ts` | **Coordination edit.** Add `SessionGroup`, `FanoutLaunchOpts`; add `maxFanoutLanes?: number` to `UiConfig` (`:656`); add `maxFanoutLanes: number` to `Config` (`:26` block). |
|
||||
| `src/config.ts` | Parse `MAX_FANOUT_LANES` in `loadConfig` (helper exists, `parseNonNegativeInt` `:83`); include in returned `Config`. |
|
||||
| `src/http/session-groups.ts` **(new)** | Pure `deriveRepoRoot(cwd: string \| null): string \| null` (strip a trailing `/<name>` when the parent basename ends with `-worktrees`, else return cwd) + `groupSessionsByRepo(sessions: LiveSessionInfo[]): SessionGroup[]`. No I/O — fully node-unit-testable. High-cohesion small file (per coding-style). |
|
||||
| `src/server.ts` | Register `GET /live-sessions/grouped` → `res.json(groupSessionsByRepo(manager.list()))` beside `:332`; import `groupSessionsByRepo`; add `maxFanoutLanes` to the `UiConfig` object at `/config/ui` (`:1099`). |
|
||||
| `public/fanout.ts` **(new)** | `buildFanoutCmd(prompt, mode, allowAutoMode)`, `shellSingleQuote(s)`, `sanitizePrompt(s)` (collapse newlines, trim, cap), `laneBranch(base, i)`, `slugify(prompt)` — all pure/exported for jsdom unit tests. Plus `createWorktreeReq(repoPath, branch)` thin POST helper if not reusing the inline one in `projects.ts:725` (extract it to reuse — see below). |
|
||||
| `public/projects.ts` | Add `onFanout` to `ProjectsHooks` (`:39`); add `renderFanoutForm(detail, hooks)` (sibling of `renderNewWorktreeForm` `:692`: prompt `<textarea>`, N `<input type=number min=2 max=maxFanoutLanes>`, branch-base `<input>` prefilled `slugify(prompt)`, permission-mode `<select>`, "⑃ Fan out N lanes" submit) rendered in `renderProjectDetail` (`:822`) just after the New Worktree form (`:930`); **extract** the inline `POST /projects/worktree` body (`:725-737`) into an exported `createWorktreeReq(repoPath, branch)` so fan-out and the form share it (DRY). |
|
||||
| `public/tabs.ts` | Add `launchFanout(repoPath, repoName, opts)` + `keepFanoutWinner(groupId, winnerSessionId)`; a `fanoutGroups: Map<string, FanoutGroup>` field (persisted via a new `FANOUT_KEY`); extend `TabEntry` with `fanoutGroupId?: string`, `worktreePath?: string`, `branch?: string`; append a per-cell **🏆 Keep** button in `addEntry` (`:776-801`) shown only when `entry.fanoutGroupId` is set (toggled in `renderCell` `:1328`); wire `onFanout` in the constructor (`:180`). Reuse `setGridLayout` (`:1020`), `addEntry` (`:704`), `countOpenWithTitlePrefix` (`:849`), `removeWorktreeReq` (`projects.ts:299`). |
|
||||
| `public/styles.css` (or the FE CSS entry) | `.proj-fanout-form`, `.cell-keep` (mirror `.cell-max` `tabs.ts:779`), `.fanout-banner`. No new layout — reuses `lay-grid-4/6` + `term-cell` chrome. |
|
||||
|
||||
`launchFanout` algorithm (FE orchestration, **sequential** — `git worktree add` takes a repo lock, so parallel adds race):
|
||||
1. Clamp `N = min(opts.lanes, maxFanoutLanes, 6)`; validate `branchBase` via `validateBranchNameClient` (`projects.ts:672`); build `cmd = buildFanoutCmd(prompt, mode, allowAutoMode)` once.
|
||||
2. `for i in 1..N`: `const r = await createWorktreeReq(repoPath, laneBranch(base,i))`; on `r.ok` push `{branch, worktreePath:r.path}` to the group; on failure record the error, **continue** (partial-success, surfaced in the banner — no auto-rollback in v1).
|
||||
3. If ≥1 lane created: create a `FanoutGroup {id, repoPath, repoName, prompt, lanes[]}`; for each created lane `addEntry(null, `${repoName}·${branch}`, worktreePath, cmd)` (reuse `openProject`'s exact addEntry call shape `:842`), tag the returned `TabEntry` with `fanoutGroupId/worktreePath/branch`; persist.
|
||||
4. `setGridLayout(N <= 4 ? 'grid-4' : 'grid-6')` and `activate` the first lane. The board's existing per-quadrant approve/maximize/monitor + gauges now cover every lane for free.
|
||||
|
||||
`keepFanoutWinner(groupId, winnerSessionId)`:
|
||||
1. Resolve the group; single confirm: `"Keep <winner branch> and discard the other N−1 lanes (delete their worktrees)?"`.
|
||||
2. For each losing lane: `closeTab(idx)` (detach — PTY reaped by IDLE_TTL) then `removeWorktreeReq(repoPath, lane.worktreePath, false)`; on `409` dirty → `removeWorktreeReq(..., true)` (already inside the batch confirm — no per-lane prompt). Collect failures into the banner.
|
||||
3. Winner tab **stays**; drop the group from `fanoutGroups`; if all losers gone, `setGridLayout('single')` and activate the winner.
|
||||
|
||||
---
|
||||
|
||||
## TDD steps (ordered — RED→GREEN, matching repo style)
|
||||
|
||||
**Backend pure — `test/http/session-groups.test.ts`** (new, node env, no I/O; mirror the pure-helper style of `test/http/worktrees*.test.ts`):
|
||||
1. `deriveRepoRoot('/a/proj-worktrees/lane-1')` → `/a/proj` (parent basename ends `-worktrees`). `deriveRepoRoot('/a/proj')` → `/a/proj`. `deriveRepoRoot(null)` → `null`. → implement.
|
||||
2. `groupSessionsByRepo([...])`: two sessions with cwds `/a/proj-worktrees/lane-1` and `/lane-2` → **one** `SessionGroup` (`repoRoot:/a/proj`, 2 members); an unrelated `/b/other` cwd → its own group; `cwd:null` → skipped or an "ungrouped" bucket (decide + assert). Members preserve `manager.list` newest-first order.
|
||||
|
||||
**Backend config — `test/config.test.ts`** (extend): assert `maxFanoutLanes` default `6`, `MAX_FANOUT_LANES=3` override parses, `-1` throws (fail-fast, like the `MAX_SESSIONS` test). Update **every** all-fields `CFG` fixture (`grep -rl "maxSessions" test/`) to add `maxFanoutLanes` (compile gate).
|
||||
|
||||
**Backend route — `test/integration/*.test.ts`** (extend an existing live-sessions integration file; reuse `startServer` + `fetch`, `itPty` where a real session is needed):
|
||||
3. `GET /live-sessions/grouped` on an empty manager → `200 []`. No Origin header required (read-only) — assert it does **not** 403.
|
||||
4. `itPty`: attach two sessions with cwds under one `*-worktrees` dir → `grouped` returns one group with both ids. `GET /config/ui` includes `maxFanoutLanes`.
|
||||
|
||||
**FE pure — `test/fanout.test.ts`** (new, jsdom or node; pure functions):
|
||||
5. `shellSingleQuote("it's ok")` → `'it'\''s ok'`. `sanitizePrompt("a\nb\r\nc")` → `"a b c"`; over-length → truncated to `FANOUT_PROMPT_MAX`.
|
||||
6. `buildFanoutCmd("fix bug","plan",true)` → `claude --permission-mode plan 'fix bug'\r`; `mode:'default'` → `claude 'fix bug'\r`; `mode:'auto', allowAutoMode:false` → downgraded to no `--permission-mode` (SEC-M5 parity with `resolveMode` `tabs.ts:659`).
|
||||
7. `laneBranch('feat-x',3)` → `feat-x-lane-3`; result passes `validateBranchNameClient` (`projects.ts:672`).
|
||||
|
||||
**FE component — `test/projects.test.ts`** (extend; reuse `makeDetail`/`makeHooks`, stubbed `fetch`):
|
||||
8. `renderFanoutForm` renders a prompt textarea, N stepper (max = `maxFanoutLanes`), branch-base input, mode select, submit. Empty prompt → submit disabled / inline error via `textContent` (SEC-L3/H6, like `renderNewWorktreeForm` `:708`).
|
||||
9. Submitting calls `hooks.onFanout(detail.path, detail.name, {prompt, lanes, branchBase, mode})`.
|
||||
|
||||
**FE orchestration — `test/tabs.test.ts`** (extend; the file already stubs `TerminalSession`/`fetch`):
|
||||
10. `launchFanout(repo,'repo',{lanes:3,...})` with `fetch` stubbed to return `{ok:true,path:'/wt/lane-i'}` → **3** `addEntry` calls (assert 3 tabs), each with the **same** `initialInput` (`buildFanoutCmd` output) and its lane cwd; grid becomes `grid-4`; worktree POSTs happen **sequentially** (assert call order / that the (k+1)th starts after the kth resolves).
|
||||
11. Partial failure: 2nd `createWorktreeReq` rejects → 2 lanes created, banner shows the failure, no throw (never-throw discipline, `projects.ts:299`).
|
||||
12. `keepFanoutWinner(id, winnerId)`: `confirm=()=>true`, `fetch` `{ok:true}` → loser tabs closed (assert `tabs.length` drops to 1), `DELETE /projects/worktree` called once per loser with the loser's `worktreePath`; winner tab remains; layout → `single`.
|
||||
13. Dirty loser: `DELETE` first `{ok:false,status:409}` then `{ok:true}` → second call body has `force:true`; **no** second `window.confirm` (batch already confirmed).
|
||||
14. `confirm=()=>false` → no `closeTab`, no `DELETE` (no accidental deletion).
|
||||
|
||||
Run `npm test`; the ~80% gate holds — grouping/config/route are deterministic node tests, and every FE branch (build-cmd modes, sequential launch, partial failure, keep-winner happy/dirty/cancel) is jsdom-exercised. The only PTY-real assertion (grouped over live sessions) is `itPty`-gated (auto-skips in sandbox, runs in CI).
|
||||
|
||||
---
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **Prompt with quotes / `$` / backticks / `;`** — single-quote wrapping + `'\''` escaping means the shell passes it verbatim to `claude` as one argv element; no command injection into the shell (see Security).
|
||||
- **Multi-line prompt** — a raw newline typed into the PTY submits early; `sanitizePrompt` collapses `\r?\n`→space (single-line task descriptions are the use case, like an issue title). Documented in the form's helper text.
|
||||
- **N exceeds caps** — clamped to `min(maxFanoutLanes, 6, maxSessions−liveCount)`; if the DoS cap is hit mid-launch, `addEntry`→attach throws server-side via `assertUnderSessionCap` (`manager.ts:106`, the M4 path) and that lane shows an `exit(-1)`; the banner reports "started K of N".
|
||||
- **Branch already checked out / dir exists** — `createWorktree` returns `409` (`worktrees.ts:200-208`); that lane is skipped with a banner note; other lanes proceed (each branch is `-lane-i`, so collisions only on a re-run — suggest a fresh `branchBase`).
|
||||
- **Sequential lock contention** — awaiting each `createWorktree` before the next avoids git's `worktree add` lock race; total launch is O(N) git adds (bounded, N≤6).
|
||||
- **Keep-winner on a dirty loser** — `DELETE` 409 → auto-retry with `force:true` *inside the single batch confirm* (the user already accepted "discard the other lanes"); still never force-removes the **main** worktree (server rejects `isMain` 400, `worktrees.ts:327`) — losers are never main.
|
||||
- **Winner == current worktree you're viewing** — fine; only losers are removed. Removing a loser whose tab is elsewhere just detaches then deletes its dir.
|
||||
- **Reload mid-race** — `fanoutGroups` persisted to `FANOUT_KEY` (like `TABS_KEY` `tabs.ts:57`, written in `onSessionId` once ids resolve); on boot, reconstruct the board from persisted groups. Fallback discovery: `GET /live-sessions/grouped` re-clusters live sessions by repo even if localStorage was cleared.
|
||||
- **Session exits (Claude finished) before you pick a winner** — the exited lane keeps its last screen (L1 replay, `manager.ts:149`) and its gauge greys stale (`STATUSLINE_TTL_MS`, `tabs.ts:64`); still selectable as winner (its worktree persists until you keep someone).
|
||||
- **`worktreeEnabled=0`** — create/remove routes 403 (`server.ts:910,939`); `launchFanout` surfaces the 403 in the banner and creates nothing (the form can hide itself when `/config/ui` signals disabled — optional).
|
||||
|
||||
---
|
||||
|
||||
## Security
|
||||
|
||||
- **No new trust boundary.** Launch reuses `POST /projects/worktree` and each lane's WS attach; discard reuses `DELETE /projects/worktree` — all already `requireAllowedOrigin`-guarded (`server.ts:480,909,938`). `GET /live-sessions/grouped` is read-only (session ids, cwds, statuses — same data already in `/live-sessions`), so no Origin guard, consistent with `/live-sessions` (`:332`) and `/digest` (`:340`).
|
||||
- **Prompt → shell injection (the one new risk).** The prompt is typed as raw bytes into the lane's shell before `claude` parses it. `shellSingleQuote` wraps it in single quotes with `'` → `'\''` escaping so the shell treats it as a single literal argv element — no `$(...)`, backtick, `;`, `&&`, or redirection can execute. `sanitizePrompt` additionally strips newlines (which would submit a partial line) and caps length. **This is the load-bearing quoting invariant — unit-test it (steps 5-6) and never `String`-concat the prompt into the command unquoted.** (The threat is limited anyway: the caller already has full shell access via the terminal — this just avoids a *surprising* early-execution when a prompt contains shell metacharacters.)
|
||||
- **Branch names** — validated client-side (`validateBranchNameClient` `:672`) **and** server-side (`validateBranchName` `worktrees.ts:95`, `-b <branch>` after `--`); flag-injection (`-`-leading) and traversal already rejected; the worktree dir is containment-checked (`computeWorktreeDir` M2, `:146`).
|
||||
- **Destructive keep-winner** — a single explicit confirm before deleting N−1 worktrees; `removeWorktree`'s realpath-must-match-a-registered-worktree spine (`worktrees.ts:313-323`) means only genuine linked worktrees can be deleted, never an arbitrary path, and never `main`. Errors render via `textContent` (SEC-L3/H6), never `innerHTML`.
|
||||
- **DoS bounds** — N capped by `maxFanoutLanes`/grid-6/`maxSessions`; each lane is one bounded PTY; grouping endpoint does zero `git`/FS work (pure string grouping over the in-memory list).
|
||||
- **Audit** — each `createWorktree`/`removeWorktree` already logs via `sanitizeForLog` (`server.ts:923,952`); no new logging path, prompt bytes are never logged.
|
||||
|
||||
---
|
||||
|
||||
## Effort & dependencies
|
||||
|
||||
- **Rough effort: L (~3–4 dev-days).** Backend is genuinely thin: config field + one pure grouping module + one read-only route (~0.5 d incl. tests). FE carries the weight: `public/fanout.ts` pure helpers (~0.5 d), `renderFanoutForm` + `onFanout` wiring (~0.5 d), `launchFanout`/`keepFanoutWinner`/lane-group model + persistence + the 🏆 cell button (~1.5 d), tests (~0.75 d).
|
||||
- **Depends on (all shipped):** W2 `initialInput` (`terminal-session.ts:127,374`) + `openProject`/`addEntry` (`tabs.ts:836,704`); W4 `createWorktree` (`worktrees.ts:224`) + `removeWorktree` (`:301`) + `removeWorktreeReq`/`confirmAndRemoveWorktree` (`projects.ts:299,349`); the split-grid board (`grid-layout.ts`, `tabs.ts` `applyLayout`/`renderInlineApprove`/`renderCell`). **No** node-pty/protocol change, **no** new WS frame, **no** DB/migration, **no** new npm dep.
|
||||
- **Coordination:** the `src/types.ts` edit (`SessionGroup`, `FanoutLaunchOpts`, `UiConfig.maxFanoutLanes`, `Config.maxFanoutLanes`) is the only cross-cutting change — freeze it first and update every all-fields `CFG` test fixture in the same commit (it touches `Config`).
|
||||
- **Optional follow-ups (explicitly out of v1 scope):** a thin `merge(repoPath, branch, opts)` in `src/http/git-ops.ts` (sibling of `commit` `:218`/`push` `:312`, reusing `classifyGitError` `:81`) behind `POST /projects/git/merge` for one-click "land the winner"; server-side `git --git-common-dir` grouping for exact (non-heuristic) clustering; auto-rollback of partially-created worktrees on launch failure. Each is additive and can ship after the board proves out.
|
||||
Reference in New Issue
Block a user