Skip to content

(panel): read-only Changes view from git status, same for local and remote sessions - #280

Merged
devsuitup merged 2 commits into
mainfrom
feat/251-changes-view
Sep 12, 2026
Merged

devsuitup merged 2 commits into
mainfrom
feat/251-changes-view

Conversation

@devsuitup

Copy link
Copy Markdown
Owner

Closes #251. Context note: .ai/contexts/changes-view.md; user doc: docs/changes-view.md.

What

  • git-changes.js (pure, dual-loadable): parseStatusPorcelainV2, parseNumstat (binary → null, both rename spellings), mergeChanges → panel model with per-file staged/unstaged/untracked/renamed flags, counters and totals.
  • git-changes-runner.js: createGitChangesRunner({ kind, cwd, alias?, exec }) with .status() (status --porcelain=v2 --branch, diff --numstat, diff --cached --numstat) and .diff(path, { staged }) (bounded 512 KB, truncated flag). Local: execFile('git', args, { cwd }), no shell. Remote: git -C '<cwd>' '<arg>'… over buildRemoteCommandArgs — every token POSIX single-quoted, NUL/newline refused, .. refused in paths, no backtick emitted for any input (tested with adversarial cwd/path). Timeouts 10 s / 20 s.
  • git-changes-target.js: cwd resolution — live local session's own cwd, else the same resolveSessionRealCwd scan resume/fork uses; remote from the host's descriptor list, no PTY needed.
  • main.js / preload.js: git-changes-status(sessionId), git-changes-diff(sessionId, path, staged); { ok:false, error } on any failure.
  • public/file-panel.js: a changes tab type in the existing right-hand panel — header "N files changed +a −b", branch with ahead/behind, one row per file (state, path, counters), click → read-only diff rendered line by line (no diff language bundled in CodeMirror), Refresh button; entry from the terminal header.
  • public/session-activity.js: onSessionIdle(cb) fired once per setActivity(id, false, …); the panel refreshes on open, on the button and on idle of its session — no timer anywhere.

Proof

  • Parser mutations: collapsing staged/unstaged → red; dropping rename/origPath → red; binary - as 0 → red. Runner mutations: dropping the .. guard → red; breaking the quote escape → red.
  • Zero-polling test: no gitChangesStatus call while busy, on duplicate idle, or without a Changes tab.
  • task check: 1376 + 120 pass, 0 fail, pre-existing skips.

Not exercised: the real ssh transport (fakes only, by rule); the panel in the running app.

…emote sessions

Closes #251. A pure parser for porcelain v2 + numstat, one runner interface
with a local execFile and a remote ssh implementation (every token
single-quoted, no backtick), two IPCs resolving the session's real cwd
(recorded cwd locally, descriptor cwd remotely), and a Changes mode in the
right-hand panel: files changed with counters, branch and ahead/behind, click
for a read-only diff. Refresh on open, on the button and when the session
goes idle — no polling.
…out cap, id validation

Review follow-up. Every git invocation runs with --literal-pathspecs and a
leading ':' is refused, so a file name cannot carry pathspec magic; status
and numstat are read with -z, so a non-ASCII or spaced name round-trips to
git diff unescaped; the ssh transport caps stdout in bytes and kills the
child on overflow; sessionId is shape-checked before any lookup; the idle
refresh fires on a busy-to-idle edge only; the diff cap is measured in bytes
on a line boundary; the local runner drops inherited GIT_DIR-style variables
so only the session's cwd decides the repository. A real-git test pins that
an absolute pathspec outside the repo yields nothing.
@devsuitup

Copy link
Copy Markdown
Owner Author

Adversarial review (reproduced against a real git) → second commit:

  1. CRITICAL — remote stdout unbounded in defaultRunRemoteCommand: now maxStdoutBytes (default 8 MB; 2 MB for status, diff cap + 64 KB for diff), counted in bytes, child killed on overflow, error in the usual shape. Test with a fake child.
  2. MAJOR — pathspec magic (:(exclude)x after --): leading : refused and --literal-pathspecs on every invocation, local argv and remote string, pinned exactly.
  3. MAJOR — core.quotePath (café.txt"caf\303\251.txt", empty diff): status and numstat now read with -z, parsers rewritten on NUL records (renames as separate fields), fixtures with non-ASCII, spaces and renames checked against real git output.
  4. MAJOR — sessionId unvalidated: shape check before any lookup ([A-Za-z0-9._-]+ minus ./.., or pid:<n>); "../../x" never reaches resolveSessionRealCwd.
  5. MINOR — idle notification fires on a busy→idle edge only (two armReady:false idles → one refresh).
  6. MINOR — diff cap measured in UTF-8 bytes on a line boundary.
  7. Absolute / ~ pathspec: real-git test — git refuses a pathspec outside the repo, ~ never expands; no code change.

Found while committing: the real-git test, run by the pre-commit hook, inherited GIT_DIR/GIT_INDEX_FILE and wrote its scratch file into this repository's index and its user.email into the local config (both reverted). The test helper now strips GIT_*/HUSKY* and disables hooks for the scratch repo, and defaultLocalExec drops the repo-location variables too, so only the session's cwd decides which repository a Changes command reads. Documented in changes-view.md.

task check: green, 0 errors.

@devsuitup
devsuitup merged commit 93d2e42 into main Sep 12, 2026
10 checks passed
@devsuitup
devsuitup deleted the feat/251-changes-view branch September 12, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(panel): read-only Changes view from git status, same for local and remote sessions

1 participant