kai/web-terminal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
5e427dcf988655b2507d4caddc11d64120030811
web-terminal/docs/plans/w3-pr-ci-chip.md

18 KiB
Raw Blame History

PR + CI/checks status chip via gh

A per-project chip in the project-detail view that shows, for the repo's current branch: PR state (open / draft / merged / closed / none), N checks passing (from statusCheckRollup), and mergeable (clean / conflicting). It is a read-only, out-of-band side-channel — exactly like getDiff (src/http/diff.ts): execFile('gh', …) (no shell), timeout + maxBuffer bound, parses gh's --json output, and capability-degrades (chip explains itself) when gh is missing, unauthenticated, or the branch has no PR. Cached at module scope with a short TTL (reuses cfg.projectScanTtlMs) so opening/refreshing a project detail doesn't hammer the GitHub API.

Grounding: buildProjectDetail (src/http/projects.ts:420) surfaces branch/dirty/worktrees but nothing PR. getDiff/runGit (src/http/diff.ts:289) is the runner+degrade pattern to mirror. isValidGitDir (src/server.ts:123) + the /projects/diff route (src/server.ts:661) are the exact route mirror. The FE detail header is renderProjectDetail (public/projects.ts:672, header at lines 696-706); buildDiffSection (public/projects.ts:612) and public/diff.ts fetchDiff/normalizeDiffResult (lines 110/38) are the FE fetch+degrade+textContent pattern.

Contract

New route

GET /projects/pr?path=<abs-repo-dir> — read-only, no Origin guard (same threat model as /projects and /projects/diff; see src/server.ts:660 comment).

  • 400 {error} — path missing/empty (mirror src/server.ts:662-666).
  • 404 {error:'project not found'} — !isValidGitDir(target) (mirror src/server.ts:667-670, SEC-H7 three-prong).
  • 200 PrStatus — always on a valid git dir, including all degrade cases (the availability lives in the body, not the HTTP status — so the FE renders one chip regardless). 500 {error} only on an unexpected throw (mirror src/server.ts:674-677).

New message type — src/types.ts (coordination edit, next to DiffResult at line 475)

/* ── W3 PR + CI status chip (gh) ── */

/** Why a PrStatus has (or lacks) PR data. Drives the FE chip's degraded text. */
export type PrAvailability =
  | 'ok'              // a PR exists for the current branch; fields below are populated
  | 'no-pr'           // gh works but the branch has no PR (or no remote/default repo)
  | 'not-installed'   // `gh` binary not found on PATH (ENOENT)
  | 'unauthenticated' // gh present but not logged in (needs `gh auth login`)
  | 'disabled'        // GH_ENABLED=0 — feature off, never spawns gh
  | 'error';          // gh spawned but failed for another reason (timeout, etc.)

/** Rolled-up CI check counts from gh's statusCheckRollup (CheckRun + StatusContext). */
export interface PrCheckSummary {
  total: number;
  passing: number;   // CheckRun conclusion SUCCESS/NEUTRAL/SKIPPED | StatusContext SUCCESS
  failing: number;   // FAILURE/TIMED_OUT/CANCELLED/ACTION_REQUIRED | ERROR/FAILURE
  pending: number;   // QUEUED/IN_PROGRESS/WAITING | PENDING/EXPECTED
}

/** GET /projects/pr result. Only present-when-'ok' fields are optional. */
export interface PrStatus {
  availability: PrAvailability;
  number?: number;
  title?: string;
  url?: string;
  state?: 'open' | 'closed' | 'merged'; // lower-cased from gh OPEN/CLOSED/MERGED
  isDraft?: boolean;
  mergeable?: 'mergeable' | 'conflicting' | 'unknown'; // lower-cased from gh
  headRefName?: string;
  baseRefName?: string;
  checks?: PrCheckSummary;
}

gh invocation (single spawn — KISS)

One command; statusCheckRollup already carries per-check state, so no second gh pr checks spawn:

gh pr view --json number,state,title,url,isDraft,mergeable,headRefName,baseRefName,statusCheckRollup

Run with cwd = repoPath. gh resolves the PR from the current branch. statusCheckRollup items are a mix of {__typename:'CheckRun', status, conclusion} and {__typename:'StatusContext', state} — the pure parser handles both.

New env vars — src/config.ts + Config in src/types.ts (coordination edit)

Env var Default Purpose
GH_ENABLED true (parseBool) Feature flag. false → route returns {availability:'disabled'}, never spawns gh.
GH_TIMEOUT_MS 8000 (parseNonNegativeInt) Hard-kill timeout for the gh spawn. Larger than diffTimeoutMs (2 s) because gh hits the network.

Cache TTL reuses cfg.projectScanTtlMs (src/config.ts:311, default 10 000 ms) — no new TTL var. Add the two fields to the assembled object in loadConfig (src/config.ts:390-438) and to the Config interface.

Files to change

Path Concrete change
src/types.ts Coordination edit. Add PrAvailability, PrCheckSummary, PrStatus near DiffResult (line 475). Add ghEnabled: boolean + ghTimeoutMs: number to the Config interface (find Config — used by loadConfig).
src/config.ts Add const DEFAULT_GH_TIMEOUT_MS = 8000 near line 65; parse GH_ENABLED via parseBool(env['GH_ENABLED'], true) and GH_TIMEOUT_MS via parseNonNegativeInt(...); add both to the frozen object (lines 413-438).
src/http/gh.ts New file (~180 lines), mirrors diff.ts structure: a runGh runner (execFileAsync('gh', args, {cwd, timeout, maxBuffer}) that captures stdout/stderr/code/spawn-ENOENT), a pure parsePrView(json): PrStatus-core + summarizeChecks(rollup): PrCheckSummary, a classifyGhFailure(exec): PrAvailability, module-scope short-TTL cache mirroring discoverCache (projects.ts:239-317) with in-flight dedupe, getPrStatus(repoPath, cfg): Promise<PrStatus>, and a _clearPrCache() test hook (mirror _clearProjectCache, projects.ts:314).
src/server.ts Add import { getPrStatus } from './http/gh.js' (next to line 41). Add GET /projects/pr route immediately after /projects/diff (after line 678), copying the path-missing → 400 and !isValidGitDir → 404 guards, then res.json(await getPrStatus(target, cfg)) in a try/catch → 500 (mirror lines 671-677).
public/gh-chip.ts New file (~120 lines), render-only, mirrors public/diff.ts: normalizePrStatus(raw): PrStatus | null, fetchPrStatus(repoPath): Promise<PrStatus | null> (mirror fetchDiff, diff.ts:110), chipText(status): {label, cls, title} (pure, unit-tested), renderPrChip(status): HTMLElement (all text via textContent, zero innerHTML — SEC-H4), mountPrChip(container, repoPath): {destroy()} that shows a loading placeholder then swaps in the resolved chip.
public/projects.ts Add import { mountPrChip } from './gh-chip.js' (near line 27). In renderProjectDetail, after the dirty indicator (line 704) inside the if (detail.isGit) guard, append the chip host and mount it; track the handle in a prRef and destroy() it in the back click handler (lines 683-688) alongside diffRef.h?.destroy().
public/styles.css (or the existing project-detail CSS file) Add .proj-pr-chip + state modifier classes (.proj-pr-open/.draft/.merged/.closed/.none/.unavailable, .proj-pr-checks-ok/.fail/.pending, .proj-pr-conflict). Reuse the existing .proj-branch chip look (line 699) as the base.
test/http/gh.test.ts New (node) — pure parsers + getPrStatus classification.
test/integration/pr-status.test.ts New (node) — real startServer, gh stubbed via a PATH shim.
test/gh-chip.test.ts New (jsdom) — normalizePrStatus/chipText/renderPrChip/mountPrChip.

TDD steps

Ordered; each "write test → run RED → implement → GREEN". Backend-pure first (cheap, deterministic), then route, then FE. Keeps the 80 % gate because the pure parser + classifier are the bulk of the logic and are fully covered without spawning gh.

Backend — test/http/gh.test.ts (node; mirror test/http/diff.test.ts:1-33). Import from ../../src/http/gh.js.

  1. summarizeChecks — canned statusCheckRollup arrays:
    • CheckRun {status:'COMPLETED',conclusion:'SUCCESS'} → passing++. Implement summarizeChecks.
    • CheckRun conclusion:'FAILURE' → failing++; TIMED_OUT/CANCELLED/ACTION_REQUIRED → failing.
    • CheckRun status:'IN_PROGRESS'/'QUEUED' (null conclusion) → pending.
    • StatusContext {state:'SUCCESS'} → passing; 'PENDING' → pending; 'FAILURE'/'ERROR' → failing.
    • NEUTRAL/SKIPPED → passing (don't block). Empty/undefined rollup → all-zero. Unknown shape → counted in total only, treated as pending. Assert total === passing+failing+pending.
  2. parsePrView — feed a full canned JSON string:
    • Valid PR JSON → availability:'ok', lower-cased state/mergeable, number/title/url/isDraft/headRefName/baseRefName mapped, checks from summarizeChecks. Implement parsePrView (never throws — try/JSON.parse; malformed → {availability:'error'}, mirroring diff.ts "never throws" house style).
    • isDraft:true still availability:'ok' (FE decides the "draft" label); mergeable:'UNKNOWN' → 'unknown'.
    • Malformed / non-object JSON → {availability:'error'}.
    • Security assert: a PR title containing <script>alert(1)</script> survives verbatim in PrStatus.title (proves no parsing-side mangling; FE renders it inert).
  3. classifyGhFailure — given synthetic exec results:
    • spawn ENOENT (code:'ENOENT') → 'not-installed'.
    • stderr containing gh auth login / not logged / authentication / HTTP 401 → 'unauthenticated'.
    • stderr containing no pull requests found / no default remote / no git remote → 'no-pr'.
    • other non-zero exit → 'error'. Implement classifyGhFailure (regex on lower-cased stderr).
  4. getPrStatus cache/dedupe — inject a fake runner (or spy) so no real gh spawns:
    • ghEnabled:false in cfg → resolves {availability:'disabled'} without invoking the runner.
    • Two rapid calls for the same path share one in-flight run (assert runner called once); after _clearPrCache(), it runs again. Mirror projects.ts:289-311. Implement the module cache + _clearPrCache.
    • Cache key includes the current branch (cheap read of .git/HEAD like readBranch, projects.ts:88) so a branch switch busts the cache before TTL. Test: same path, different HEAD branch → runner re-invoked.

To keep getPrStatus unit-testable without gh, factor the spawn into an injectable runGh (default real, overridable in tests) — same seam idea as getDiff's runGit. parsePrView/summarizeChecks/classifyGhFailure stay pure and exported.

Route — test/integration/pr-status.test.ts (node; mirror test/integration/projects-endpoint.test.ts:1-55). Use getFreePort + a temp dir with a fake .git repo (reuse makeFakeGitRepo shape). Stub gh with a PATH shim: write an executable script named gh into a temp bin/ that echoes canned JSON (or exits 1 with a canned stderr), then set process.env.PATH = binDir + ':' + process.env.PATH before startServer (execFile resolves gh via PATH). Restore PATH + _clearPrCache() in afterEach.

  1. Missing path → 400. Implement route guard 1.
  2. Non-git dir path → 404 (isValidGitDir fails). Implement guard 2.
  3. gh shim emits valid PR JSON → 200 with availability:'ok', correct checks. Wire res.json(await getPrStatus(...)).
  4. gh shim exits 1 with no pull requests found on stderr → 200 {availability:'no-pr'}.
  5. GH_ENABLED='0' env → 200 {availability:'disabled'}, and (assert via a shim that writes a marker file) gh is never spawned.

Frontend — test/gh-chip.test.ts (jsdom; mirror test/diff.test.ts:1-14, // @vitest-environment jsdom, dynamic await import('../public/gh-chip.js'), vi.stubGlobal('fetch', …)).

  1. normalizePrStatus — valid object round-trips; non-object / bad availability → null (mirror normalizeDiffResult, diff.ts:38). Implement.
  2. chipText(status) — pure map: ok+open → "PR #12 ✓ 5/5"; failing checks → "PR #12 ✕ 3/5"; mergeable:'conflicting' adds a ⚠ conflicts marker/class; no-pr → "No PR"; not-installed → "gh not installed" + title link to cli.github.com; unauthenticated → "gh auth login"; disabled → chip hidden (returns null/display:none). Implement.
  3. renderPrChip — SEC-H4 assert: a title of <img src=x onerror=...> appears as literal text (el.textContent contains it, el.querySelector('img') is null). Zero innerHTML.
  4. mountPrChip — fetch stubbed to resolve ok JSON → placeholder replaced by the chip; fetch rejects → degrades to an error chip (no throw); destroy() removes the node. Mirror mountDiffViewer (diff.ts:253-).

Frontend wiring — extend test/projects.test.ts (jsdom; renderProjectDetail is already exported/tested there).

  1. renderProjectDetail with detail.isGit:true → a .proj-pr-chip host is present in the header; with isGit:false → absent. (Mock gh-chip's mountPrChip via vi.mock so the DOM assertion doesn't depend on fetch.) Implement the renderProjectDetail edit + prRef.destroy() in the back handler.

Run npm test after each GREEN. The pure backend parser tests (steps 1-3) carry most of the coverage weight for gh.ts; FE steps 10-13 cover gh-chip.ts.

Edge cases & failure modes

  • gh not installed → spawn ENOENT → 'not-installed'. Chip shows "gh not installed" (never a 500, never a stack trace).
  • gh present, not authenticated → stderr auth pattern → 'unauthenticated' → "gh auth login".
  • No PR for branch / no remote / detached HEAD → 'no-pr' → "No PR". (Detached HEAD: .git/HEAD isn't ref: → cache-key branch is null; gh itself errors → no-pr.)
  • PR exists but checks not started / all pending → checks.total>0, passing=0, pending=total → "⧗ 0/N".
  • mergeable:'UNKNOWN' (GitHub computes mergeability async right after a push) → 'unknown' → neutral marker, not a red "conflict". Only 'conflicting' shows the ⚠.
  • Draft PR → isDraft:true → "Draft" styling; still availability:'ok'.
  • Merged/closed PR still on the branch → state:'merged'/'closed' badge (gh returns the most recent PR).
  • gh timeout (network hang) → execFileAsync kills at ghTimeoutMs → 'error' → generic "PR status unavailable". Bounded, never hangs the request.
  • Huge statusCheckRollup (100s of checks / monorepo) → bounded by maxBuffer (reuse diffMaxBytes); overflow → 'error' (don't try to parse a truncated JSON). Counts are aggregate so the chip stays tiny.
  • Malformed --json output (gh version drift) → parsePrView JSON.parse throws → caught → 'error'.
  • Branch switch within TTL → cache key includes HEAD branch, so it busts immediately rather than showing the previous branch's PR for up to 10 s.
  • FE fetch/network error → fetchPrStatus returns null → chip renders an 'error' state, never throws (mirror fetchDiff, diff.ts:117).

Security

  • No shell: execFile('gh', [fixed argv]) — identical guarantee to runGit (diff.ts:296, SEC-M9). The only user-influenced input reaching gh is cwd, which is the already-validated repoPath. No untrusted string is ever placed in argv (gh derives the PR from the branch; we never pass a branch/base/rev). This sidesteps the ?base= deferral rationale in diff.ts:18-20.
  • Path containment: route calls isValidGitDir(target) (server.ts:123) before spawning — absolute + is-dir + has .git (SEC-H7). Path traversal / arbitrary-cwd is blocked exactly as /projects/diff.
  • DoS bounds: timeout: ghTimeoutMs (hard kill) + maxBuffer: cfg.diffMaxBytes bound a slow/huge gh. Module-scope TTL cache + in-flight dedupe cap outbound GitHub-API calls to ≈1 per repo per projectScanTtlMs even under rapid detail-view refreshes (the panel auto-refreshes every 5 s, projects.ts:33).
  • Network egress note: unlike every other side-channel (all local), gh talks to GitHub's API using the host's existing gh/GH_TOKEN credential. The endpoint never accepts or forwards a token — it only triggers gh's own auth. Document this in the route comment and in TECH_DOC §7 (this is the first feature to make an outbound call on behalf of a LAN client; the GH_ENABLED=0 kill-switch lets a cautious operator disable it entirely).
  • No secret leakage: never log gh stdout (may contain private PR titles) or the token. If logging a failure, log only availability + sanitizeForLog(stderr.slice(0,200)) (reuse server.ts:162) — never raw stderr, mirroring the worktree audit line (server.ts:714) and SEC-M10 "never raw git stderr".
  • Origin/CSRF: GET is read-only and non-mutating → no Origin guard, consistent with /projects and /projects/diff (server.ts:660). It spawns a subprocess but performs no state change, so CSWH/CSRF risk is limited to triggering a cached, rate-bounded read.
  • XSS: PR title is attacker-controllable (anyone who can open a PR on a repo the host has access to). It is carried verbatim server-side and rendered only via textContent in gh-chip.ts (SEC-H4, same discipline as diff.ts line-rendering). Explicit jsdom test (step 12) asserts no element injection.
  • Rate-limit: no per-IP limiter needed (read-only, same as /projects/diff); the TTL cache is the effective throttle. If desired later, the createRateLimiter helper (server.ts:109) is available.

Effort & dependencies

  • Effort: ~1.5–2 days. Backend gh.ts + route ≈ 0.75 day (the runner/cache pattern is a near-copy of diff.ts/projects.ts; the pure parsePrView/summarizeChecks classifier is the real work). FE gh-chip.ts + wiring + CSS ≈ 0.5 day. Tests (3 files) ≈ 0.5 day, with the PATH-shim integration harness the only novel piece.
  • Depends on: nothing hard-blocking — Config/src/types.ts/config.ts coordination edits and isValidGitDir all exist today. Requires gh on the host for the live path, but the feature is designed to degrade cleanly without it (so it ships regardless).
  • Unlocks: the W3 "Quick wins" chip (task #11 — sync/ahead-behind chip, recent commits) can reuse the same gh.ts/gh-chip.ts side-channel + short-TTL-cache scaffold (e.g. gh pr status, git rev-list --count @{u}...HEAD). The PATH-shim gh-integration harness is reusable by any future gh-backed feature.
  • Interacts with (no conflict): W3 ?base= diff (task #9) touches diff.ts//projects/diff only; this feature is an independent file (gh.ts) and route (/projects/pr). Both add a chip/section to the same renderProjectDetail header — coordinate the header layout (place the PR chip after the branch chip, before or beside the diff toggle) but they do not edit the same functions.