web-terminal/docs/plans/w4-commit-push.md

# Stage / commit / push from the diff viewer

`id: w4-commit-push` — the highest-risk git **write** set. Scope is bounded to the MVP the task defines: **per-file stage/unstage toggle + commit + push-current-branch**. Discard / checkout / restore-working-tree are explicitly **deferred** (they are the only destructive-to-working-tree ops; nothing in this feature ever touches file contents on disk).

This mirrors the existing read-only diff channel (`src/http/diff.ts`) and the one existing git-write channel (`src/http/worktrees.ts` + the `POST /projects/worktree` route at `src/server.ts:699–725`). The server stays a byte-shuttle; these are out-of-band side-channel routes.

---

## Contract

### New routes (all in `src/server.ts`, all `express.json`, all `requireAllowedOrigin` + `isValidGitDir`)

Every route: `requireAllowedOrigin(req, res)` (CSRF, `src/server.ts:352`) → `gitOpsEnabled` gate (403 if off) → per-IP rate limit → validate `path` with the in-file `isValidGitDir` (three-prong: absolute + dir + `.git`, `src/server.ts:123`) → delegate to `src/http/git-ops.ts`. The delegate re-validates and **realpath-contains** every path (defense in depth — the route’s `isValidGitDir` does not follow symlinks).

| Method + path | Body | Success | Notes |
|---|---|---|---|
| `POST /projects/git/stage` | `{ path: string, files: string[], stage?: boolean }` | `200 {ok:true, staged:boolean, count:number}` | `stage` default `true` → `git add`; `false` → unstage (`git restore --staged`). `stage` is an **addition** to the task’s bare `{path,files[]}` so one endpoint serves the toggle in both directions. |
| `POST /projects/git/commit` | `{ path: string, message: string }` | `200 {ok:true, commit:string}` (short SHA) | Commits **staged** changes only. `message` capped at `commitMsgMaxLen`. |
| `POST /projects/git/push` | `{ path: string }` | `200 {ok:true, branch:string, remote:string}` | Pushes the **current branch** to its existing upstream, else `-u <sole-remote> <branch>`. Remote/branch are **derived server-side**, never taken from the client. |

Error shape (mirrors the worktree route at `src/server.ts:724`): `res.status(result.status).json({ error: result.error })` where `error` is a **safe** message (never raw git stderr, SEC-M10 — see `classifyWorktreeError`, `src/http/worktrees.ts:193`).

Status codes: `400` invalid input / detached HEAD / no remote / identity unset; `403` disabled or bad Origin; `404` not a git dir; `409` nothing staged / non-fast-forward / index.lock held; `401` push auth required; `429` rate-limited; `500` unclassified; `200` success.

### `src/types.ts` (coordination edit — frozen shared-contract source)

Add, next to `CreateWorktreeResult` (`src/types.ts:485`):

```
export interface GitOpResult {
  ok: boolean;
  status?: number;      // HTTP status on failure
  error?: string;       // SAFE message only (never raw git stderr)
  // success payloads (route-specific, all optional):
  staged?: boolean;     // stage: direction applied
  count?: number;       // stage: files affected
  commit?: string;      // commit: short SHA
  branch?: string;      // push: branch pushed
  remote?: string;      // push: remote pushed to
}
```

One interface keeps the coordination churn minimal (like `CreateWorktreeResult`). The FE narrows with an `isGitOpResult` guard mirroring `isWorktreeResult` (`public/projects.ts:545`).

### `src/config.ts` env vars (4 new, mirroring the B3 worktree group at `src/config.ts:371–378`)

| Env | Field | Default | Purpose |
|---|---|---|---|
| `GIT_OPS_ENABLED` | `gitOpsEnabled: boolean` | `true` | Master kill-switch (mirrors `worktreeEnabled`, `src/config.ts:372`). Off → all three routes 403. |
| `GIT_OPS_TIMEOUT_MS` | `gitOpsTimeoutMs: number` | `10_000` | stage/commit exec timeout (mirrors `DEFAULT_WORKTREE_TIMEOUT_MS`, `src/config.ts:65`). |
| `GIT_PUSH_TIMEOUT_MS` | `gitPushTimeoutMs: number` | `120_000` | push is network-bound → longer bound. |
| `COMMIT_MSG_MAX_LEN` | `commitMsgMaxLen: number` | `5_000` | commit-message length cap. |

Parsed with the existing `parseBool` / `parseNonNegativeInt` helpers and appended to the frozen object (`src/config.ts:413–438`). File-count cap for `stage` reuses the existing `diffMaxFiles` (`src/config.ts:358`, default 300) — no new var. `src/types.ts` `Config` interface gains the 4 fields (same coordination note the file already carries at `src/config.ts:15`).

---

## Files to change

| Path | Change |
|---|---|
| **`src/http/git-ops.ts`** (new) | The whole write engine. Mirrors `diff.ts`/`worktrees.ts` execFile pattern: no shell, `timeout`+`maxBuffer`, `--` terminator, `env:{...process.env, GIT_TERMINAL_PROMPT:'0'}`. Exports (all never-throw, return `GitOpResult`): `stageFiles(repoPath, files, stage, opts)`, `commit(repoPath, message, opts)`, `push(repoPath, opts)`, plus pure `classifyGitError(stderr): {status,error}` and `validateRepoFiles(repoRealPath, files): string[] \| null`. |
| **`src/http/git-path.ts`** (new, small) | Extracts the containment helper so it isn’t duplicated: `resolveRealPath(target)` (copy of `worktrees.ts:118`) + `isContained(realBase, realCandidate): boolean`. Used by `git-ops.ts`. (Optional follow-up: refactor `worktrees.ts:141` `computeWorktreeDir` onto it — **deferred**, out of this task’s lane.) |
| **`src/types.ts`** | Coordination edit: add `GitOpResult` (above) + 4 `Config` fields. |
| **`src/config.ts`** | Parse the 4 new env vars; append to the frozen config object. |
| **`src/server.ts`** | Register the 3 routes after the worktree route (`:725`). Add two module-level rate constants + two limiters via `createRateLimiter` (`:109`). Reuse in-file `isValidGitDir`, `requireAllowedOrigin`, `sanitizeForLog` (`:162`) for the commit/push audit log. Import from `./http/git-ops.js`. |
| **`public/diff.ts`** | `mountDiffViewer` (`:253`) gains: per-file **Stage/Unstage** toggle button in each file row, and a bottom **commit/push bar** (message `<textarea>` + Commit + Push). Add optional `onToggleStage` to the render path; add POST helpers `postStage/postCommit/postPush`. All text via `textContent`/`el()` (SEC-H4, `:139` note). Re-`loadDiff()` after any successful write. |
| **`public/projects.ts`** | No logic change required — `buildDiffSection` (`:612`) already mounts `mountDiffViewer` and gets the new UI for free. Only add CSS-class hooks if styling inline. |
| **`public/style.css`** (or wherever `df-*` classes live) | Styles for `df-file-stage`, `df-commitbar`, `df-commit-msg`, `df-commit-btn`, `df-push-btn`, `df-op-error`, `df-op-busy`. |

---

## TDD steps (ordered)

Backend uses **node env + real throwaway git repos** in `os.tmpdir()` exactly like `test/http/diff.test.ts:277` and `test/http/worktrees-create.test.ts:42`. FE uses **jsdom + mocked `fetch`** like `test/diff.test.ts`.

**Pure classifier first (fast, deterministic):**
1. `test/http/git-ops.test.ts` → `describe('classifyGitError')`: feed canned stderr strings, assert `{status,error}` and that `error` never contains `fatal:` (SEC-M10, mirror `worktrees-create.test.ts:193`). Cases: `nothing to commit`→409; `Please tell me who you are`→400; `! [rejected]`/`non-fast-forward`/`fetch first`→409; `could not read from Username`/`Authentication failed`/`terminal prompts disabled`/`Permission denied (publickey)`→401; `index.lock`→409; unknown→500. → then implement `classifyGitError`.
2. `describe('validateRepoFiles')`: rejects `[]`, absolute paths, leading-`-`, `../escape`, > `diffMaxFiles` entries; accepts in-repo relative paths (incl. a **deleted** file whose path no longer exists on disk — `resolveRealPath` resolves the existing prefix). Symlink-escape: pre-plant a symlink inside the repo pointing outside, assert rejection (mirror `worktrees-create.test.ts:137`). → implement `validateRepoFiles` on `git-path.ts`.

**Integration against real repos:**
3. `describe('stageFiles')`: init repo + commit; modify a tracked file + add an untracked file; `stageFiles(repo,[file],true)` → assert `git diff --cached --name-only` lists them; `stage:false` → assert unstaged. Assert non-git path → `{ok:false,status:404}`. → implement `stageFiles`.
4. `describe('commit')`: stage a change → `commit(repo,'msg')` → `{ok:true, commit}` and `git log -1 --format=%H` starts with it. Empty index → `{ok:false,status:409}`. Repo with `user.name` unset → 400. Message length > cap → 400 (route-level; test the length guard where it lives). → implement `commit`.
5. `describe('push')`: create a **bare** repo as `origin` (`git init --bare`), `git remote add origin`, first push sets upstream via `-u`; assert bare repo received the ref (`git --git-dir=<bare> rev-parse <branch>`). Detached HEAD → 400. Zero remotes → 400. Two remotes + no upstream → 409. Second push (upstream now set) → plain `git push`. → implement `push`.

**Config + route wiring:**
6. `test/config.test.ts`: the 4 new vars default correctly and parse/validate (reuse existing patterns).
7. `test/integration/server.test.ts` (extend, using the `startServer` + `fetch` + `Origin` pattern at `:783`): for each of the 3 routes — foreign Origin → 403, **no** Origin → 403 (default-deny, mirror `:778`), non-git path → 404, missing field → 400, `gitOpsEnabled:false` → 403, and a happy-path 200 against a temp repo (push against a temp bare remote). Confirm `error` bodies carry no `fatal:`.

**Frontend (jsdom):**
8. `test/diff.test.ts` (extend): stub `global.fetch`. Assert each file row renders a Stage button on the Working view and Unstage on the Staged view; clicking POSTs to `/projects/git/stage` with the right `{path,files,stage}` body and re-fetches. Assert the commit bar disables Commit when the message is empty, POSTs `/projects/git/commit`, and shows a failure via `textContent` (SEC-H4 — assert zero `innerHTML`, mirror the file’s existing security assertions). Assert Push POSTs `/projects/git/push` and reflects busy/disabled state. → implement the `mountDiffViewer` changes to pass.

Coverage: the pure `classifyGitError`/`validateRepoFiles` + real-repo integration cover `git-ops.ts` branches; steps 7–8 cover the new server + `diff.ts` lines. Keeps the 80% gate.

---

## Edge cases & failure modes

- **Nothing staged → commit**: git exits non-zero (`nothing to commit`) → 409 "Nothing staged to commit." (not a 500).
- **Author identity unset**: `commit` fails (`Please tell me who you are`) → 400 with a safe hint. Do **not** auto-inject a fake identity.
- **Unborn HEAD (no commits yet)**: unstage via `git restore --staged` errors → classify to a safe 409/400; commit still works (creates the first commit). Documented limitation.
- **Detached HEAD on push**: `git rev-parse --abbrev-ref HEAD` → `HEAD` → refuse 400 "Cannot push a detached HEAD."
- **No upstream, one remote**: `git push -u <remote> <branch>`. **No upstream, ≥2 remotes**: 409 "Set an upstream first." **Zero remotes**: 400.
- **Non-fast-forward / remote ahead**: `! [rejected]` → 409 "Push rejected — pull/rebase first." (never force-push).
- **Credential prompt hang**: `GIT_TERMINAL_PROMPT=0` (+ optional `GIT_SSH_COMMAND='ssh -o BatchMode=yes'`) makes auth fail fast → 401 instead of hanging; the exec `timeout` is the backstop.
- **`index.lock` held (concurrent op)**: → 409 "Another git operation is in progress." (Optional hardening: a per-realpath in-memory promise-chain mutex in `server.ts` to serialize writes; git’s own lock is the correctness backstop, so this is MEDIUM, not required for MVP.)
- **Deleted file staged**: path absent on disk → `resolveRealPath` resolves the existing prefix and re-appends the basename; `git add -- <path>` records the deletion. **Renamed file**: FE sends both `oldPath` and `newPath` so the deletion+addition both stage.
- **Huge `files[]`**: capped at `diffMaxFiles` → 400.
- **maxBuffer overflow / timeout**: caught, returned as a safe 500 (never a thrown rejection — house style, `diff.ts:302`).
- **Commit message with newlines / leading `-` / emoji**: safe — passed as the single value of `-m` via argv (no shell), length-capped, never a pathspec.
- **Write succeeds but re-fetch fails**: FE shows the diff error state but the write already happened; surface a non-blocking notice.

---

## Security

- **Origin/CSRF**: all 3 routes call `requireAllowedOrigin` (`src/server.ts:352`) — this is a **write** channel, unlike read-only `/projects/diff` which has no guard. Tested for foreign-Origin **and** missing-Origin (default-deny).
- **No shell, ever**: `execFile('git', argv, …)` only — mirrors `diff.ts:296` / `worktrees.ts:242`. Untrusted strings (message, file paths) are argv elements, never interpolated. `--` terminates options before any pathspec so a path can’t become a flag; file paths additionally rejected if they start with `-`.
- **Path containment (highest-risk)**: `isValidGitDir` at the route (absolute+dir+`.git`) **plus** `realpath`-based containment of the repo and of **every** `files[]` entry inside `git-ops.ts` (`resolveRealPath` + `startsWith(realBase+sep)`, the M2 pattern from `worktrees.ts:141`). Defeats `../` traversal and pre-planted-symlink escapes even though the diff route only does the lighter three-prong check.
- **Server-derived remote/branch**: push never accepts a remote or refspec from the client — both are read back from the repo — eliminating arg-injection and push-to-arbitrary-URL risk.
- **Safe error classification**: `classifyGitError` maps stderr substrings to safe messages; raw git stderr is never returned (SEC-M10). Tested that responses contain no `fatal:`.
- **Rate limiting**: reuse `createRateLimiter` (`src/server.ts:109`, 60 s window) — `gitWriteLimiter` (stage+commit) ≈ 30/min/IP, `gitPushLimiter` ≈ 6/min/IP; over-limit → 429. Fixed policy constants beside the existing ones (`src/server.ts:74–76`).
- **Audit log**: commit/push log actor+path via `sanitizeForLog` (`src/server.ts:162`, control-char strip + truncate), like the worktree audit at `:714`.
- **Kill-switch**: `gitOpsEnabled=false` disables all three (mirrors `worktreeEnabled`, `src/server.ts:701`) for locked-down deployments.
- **Body size**: `express.json({ limit: '4kb' })` for commit/push, `'64kb'` for stage (large `files[]`) — matching the existing routes’ caps.
- **FE injection**: all rendered git output/errors via `textContent`/`el()` — zero `innerHTML` (SEC-H4, asserted in `test/diff.test.ts`).

---

## Effort & dependencies

- **Estimate**: ~3–4 days. Backend `git-ops.ts` + classifier + containment ≈ 1.5 d; route wiring + config + integration tests ≈ 0.75 d; FE toggles + commit/push bar + jsdom tests ≈ 1 d; polish/edge-cases ≈ 0.5 d.
- **Depends on**: the existing B1 diff channel (`src/http/diff.ts`, `public/diff.ts:253` `mountDiffViewer`) and B3 worktree channel (`src/http/worktrees.ts`) — both shipped; this reuses their execFile + realpath-containment + error-classification patterns directly. No new dependency on other roadmap items.
- **Shares infra with** `w4-worktree-remove` (task #12): both are git-write routes under `requireAllowedOrigin` + `classifyGitError`; landing the shared `git-path.ts` + `classifyGitError` here de-risks that one. Extractable-later: a generic `runGitWrite()` wrapper could later back `worktrees.ts` too (deferred — out of lane).
- **Unlocks**: an in-browser commit loop for the walk-away workflow (stage → commit → push from a phone), and a natural home for a future "recent commits" chip (task #11).