diff --git a/.changeset/git-status-indicator.md b/.changeset/git-status-indicator.md index 25eff89b..c190d1e8 100644 --- a/.changeset/git-status-indicator.md +++ b/.changeset/git-status-indicator.md @@ -2,4 +2,4 @@ "aicodeman": minor --- -Git status in the bottom bar. Agents leave work uncommitted and unpushed; turn on Settings → Header & Panels → Bottom bar → "Git status" (per-device, off by default) and a small indicator at the right of the bottom bar shows the active session's repository at a glance (`● 3` uncommitted files, `↑ 2` commits not pushed, `✓` when everything is committed and pushed). Click it for a draggable window, like the Files window, listing exactly which files are uncommitted (staged, not staged, untracked, merge conflicts; click one to see its diff; files sit under collapsed folders unless you turn off "group files by folder") and which commits are not pushed. When a session's folder holds several projects rather than being a repository itself, every repository found up to two levels down gets its own collapsible section (collapsed by default) and the indicator adds them up; an unrelated repository above the workspace (such as a dotfiles repo in your home folder) is ignored. Read-only and offline: Codeman never fetches or changes the repository, so "behind" is as of your last fetch. Not shown for Docker or remote sessions. New `GET /api/sessions/:id/git-status`. +Git status in the bottom bar. Agents leave work uncommitted and unpushed; turn on Settings → Header & Panels → Bottom bar → "Git status" (per-device, off by default) and a small indicator at the right of the bottom bar shows the active session's repository at a glance (`● 3` uncommitted files, `↑ 2` commits not pushed, `✓` when everything is committed and pushed). Click it for a draggable window, like the Files window, listing exactly which files are uncommitted (staged, not staged, untracked, merge conflicts; click one to see its diff; files sit under collapsed folders unless you turn off "group files by folder") and which commits are not pushed. When a session's folder holds several projects rather than being a repository itself, every repository found up to two levels down gets its own collapsible section (collapsed by default) and the indicator adds them up; an unrelated repository above the workspace (such as a dotfiles repo in your home folder) is ignored. Read-only and offline: Codeman never fetches or changes the repository, so "behind" is as of your last fetch. Not shown for Docker or remote sessions. New `GET /api/sessions/:id/git-status` and `GET /api/sessions/:id/git-diff` (both read-only; the diff route only serves files the status lists). diff --git a/CLAUDE.md b/CLAUDE.md index 1ab43603..32e3afe4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -322,7 +322,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph ### Frontend -Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `tab-layout-browser.js`(5.9) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. `mobile-ime-preview.js` (iOS WebKit only) paints the text an IME is composing: an iOS IME commit is routed into the local-echo overlay through the ordinary printable/paste branch and then `_transferMobileImeCommitToLocalEcho`, and without local echo the preview clears only on output parsed AFTER the commit (or its 2 s fallback). ⚠️ It watches keydown in the capture phase on `terminal.element`, never on the textarea, because xterm finalizes the composition and emits the commit in its own capture listener on the textarea. +Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `tab-layout-browser.js`(5.9) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `git-status-ui.js`(12.57) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. `mobile-ime-preview.js` (iOS WebKit only) paints the text an IME is composing: an iOS IME commit is routed into the local-echo overlay through the ordinary printable/paste branch and then `_transferMobileImeCommitToLocalEcho`, and without local echo the preview clears only on output parsed AFTER the commit (or its 2 s fallback). ⚠️ It watches keydown in the capture phase on `terminal.element`, never on the textarea, because xterm finalizes the composition and emits the commit in its own capture listener on the textarea. **Entrance animations** (`entrance-animations.js`, all OFF by default): opt-in animations for tabs, terminal, windows and connection lines, chosen via `data-tab-anim` / `data-term-anim` / `data-win-anim` / `data-line-anim` on ``; the default `legacy` theme short-circuits every hook. ⚠️ Tabs and lines are destroyed mid-animation on re-render, so re-apply to the fresh element by id with a negative `animation-delay` (resume, never restart). ⚠️ Terminal-pane styles may animate only transform / opacity / clip-path (anything else resizes the PTY via FitAddon); `blur` is the ONE sanctioned `filter` exception, do not generalise it. ⚠️ Line glow lives in `--line-glow` so blur keyframes interpolate. Persisted per-device in `codeman:*Anim` localStorage keys, never in `SettingsUpdateSchema`; lab at `?animlab=1`. Test: `test/entrance-animations.test.ts`. → [architecture-invariants#entrance-animations](docs/architecture-invariants.md#entrance-animations) diff --git a/docs/api-reference.md b/docs/api-reference.md index 22132a09..0f032cde 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -702,9 +702,9 @@ normal `caseName`/`mode`/etc. body) ## Git status -`GET /api/sessions/:id/git-status` is what the bottom-bar Git indicator and its panel read (Settings → Header & Panels → Bottom bar, per-device, default off). It reports what the session's workspace has not committed or pushed. **Read-only and offline:** it never fetches, pulls, commits or writes (it runs `git status` with `--no-optional-locks`, so it does not even refresh the index), which is why `behind` is as of the last `git fetch`. The session is resolved like every session route (ownership via `findSessionOrFail`; another user's session is `404`). +`GET /api/sessions/:id/git-status` is what the bottom-bar Git indicator and its panel read (Settings → Header & Panels → Bottom bar, per-device, default off). It reports what the session's workspace has not committed or pushed. **Read-only and offline:** it never fetches, pulls, commits or writes (it runs `git status` with `--no-optional-locks`, so it does not even refresh the index), which is why `behind` is as of the last `git fetch`. The session is resolved like every session route (ownership via `findSessionOrFail`; another user's session is `404`). A repository whose root is, or is inside, a Docker case workspace is dropped (from the walk-up, the scan below a folder, and the diff route): a container can write there, and a repository's own clean filter or signature program would run on the host. When a branch's upstream was deleted and pruned on the remote, `upstreamGone` is `true` and the unpushed list falls back to commits on no remote-tracking ref at all. -`GET /api/sessions/:id/git-diff?repo=&path=&kind=staged|unstaged|untracked|conflicted` returns the unified diff of one file the panel lists (`{ diff, truncated, binary }`; staged is index vs HEAD, unstaged is working tree vs index, untracked is the whole file as additions). It is what opens when you click a file in the Git panel. `repo` and `path` are matched against the current status rather than trusted, so anything the status does not list is `404`. Read-only (`--no-ext-diff --no-textconv`, so repository config never runs a program), capped at 400 KB, and refused (`400`) for remote and Docker sessions. +`GET /api/sessions/:id/git-diff?repo=&path=&kind=staged|unstaged|untracked|conflicted` returns the unified diff of one file the panel lists (`{ diff, truncated, binary }`; staged is index vs HEAD, unstaged is working tree vs index, untracked is the whole file as additions). It is what opens when you click a file in the Git panel. `repo` and `path` are matched against the current status rather than trusted, so anything the status does not list is `404`. Read-only: it passes `--no-ext-diff --no-textconv` (no external diff or textconv driver runs), but a repository's clean filters still run, as they do for any `git diff`, which is why a repository a container can write to is never inspected (below). Capped at 400 KB, and refused (`400`) for remote and Docker sessions; a repository at or inside a Docker case workspace is not in the status, so it is `404` here. **Which repositories.** git finds a repository by walking *up* from the session's working directory, so: diff --git a/docs/wiki/Working-With-Files.md b/docs/wiki/Working-With-Files.md index a4b7646f..f79894e1 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -174,7 +174,9 @@ Click it for a draggable window, in the style of the File Viewer: - **Uncommitted changes**, grouped as staged, not staged, untracked and conflicted, each with a status letter (`M` modified, `A` added, `D` deleted, `R` renamed, `?` new, `U` conflict). -- **Not pushed**: the commits no remote has. A branch with no upstream says so. +- **Not pushed**: the commits no remote has. A branch with no upstream says so, and so does one whose + upstream was deleted on the remote ("Upstream is gone"), which counts every commit on no remote + rather than showing a green tick. - Files are grouped under their folders, collapsed until you click a folder (a chain of single-child folders is one row, and the folders you opened stay open when the list refreshes). Turn off **App Settings → Header & Panels → Bottom bar → Git status: group files by folder** for a flat @@ -190,7 +192,9 @@ Click it for a draggable window, in the style of the File Viewer: in your home folder) is ignored. It is read-only and offline: Codeman never fetches, commits or changes the repository, so -"behind" is as of your last fetch. It is not shown for Docker or remote (SSH) sessions. The +"behind" is as of your last fetch. It is not shown for Docker or remote (SSH) sessions, and a repository at or inside a Docker case +workspace is skipped even from a local session (a container can write there, and git would run +that repository's own configuration on the host). The data comes from `GET /api/sessions/:id/git-status` and `GET /api/sessions/:id/git-diff` (see the [API reference](https://github.com/Ark0N/Codeman/blob/master/docs/api-reference.md)). diff --git a/src/git-workspace-status.ts b/src/git-workspace-status.ts index c0b41f0f..9b3cd3a5 100644 --- a/src/git-workspace-status.ts +++ b/src/git-workspace-status.ts @@ -29,10 +29,14 @@ * shell: the working directory is the process `cwd`, and the only operand-like input is a fixed * revision range. * - Output is capped: the counts are exact, the lists are not (`filesTruncated`). - * - git can run helpers a repository configures (`core.fsmonitor`, clean filters). A LOCAL session - * already runs as this same OS user, so polling adds no privilege; `core.fsmonitor` is turned off - * anyway. Remote and Docker sessions are never inspected (the route answers `unsupported`): - * a Docker workspace is writable from inside a sandbox and git here would run on the host. + * - git can run helpers a repository configures: a clean filter (`filter..clean`) still runs + * during `git status` and `git diff`, as it does for any `git status`. A LOCAL session already + * runs as this same OS user, so polling adds no privilege there. What is turned off: the + * filesystem monitor (`core.fsmonitor`), external diff and textconv drivers, and the signature + * program (`log.showSignature`). A repository a container can write to is NOT inspected: a + * Docker session answers `unsupported`, and any repository whose root is, or is inside, a Docker + * case workspace is dropped from the walk-up, the scan below a folder, and the diff route, because + * the container could have planted that config and git here would run it on the host. * - Remote URLs and git's stderr can embed `user:token@host`; anything that reaches a client goes * through `redactGitCredentials`. * @@ -94,6 +98,8 @@ export interface GitWorkspaceStatus { branch: string | null; detached: boolean; upstream: string | null; + /** The configured upstream no longer exists on the remote (deleted and pruned): nothing is tracked. */ + upstreamGone: boolean; ahead: number; /** Behind the remote-tracking ref as of the LAST FETCH; this module never fetches. */ behind: number; @@ -120,6 +126,7 @@ const EMPTY: Omit = { branch: null, detached: false, upstream: null, + upstreamGone: false, ahead: 0, behind: 0, hasRemote: false, @@ -143,6 +150,8 @@ export interface ParsedStatus { branch: string | null; detached: boolean; upstream: string | null; + /** `# branch.upstream` was printed but `# branch.ab` was not: the remote branch is gone (deleted and pruned). */ + upstreamGone: boolean; ahead: number; behind: number; files: GitFileEntry[]; @@ -154,7 +163,16 @@ export interface ParsedStatus { * one more NUL-terminated token holding the original path. */ export function parsePorcelainV2(text: string): ParsedStatus { - const out: ParsedStatus = { branch: null, detached: false, upstream: null, ahead: 0, behind: 0, files: [] }; + const out: ParsedStatus = { + branch: null, + detached: false, + upstream: null, + upstreamGone: false, + ahead: 0, + behind: 0, + files: [], + }; + let sawAb = false; const tokens = text.split('\0'); for (let i = 0; i < tokens.length; i++) { const t = tokens[i]; @@ -168,6 +186,7 @@ export function parsePorcelainV2(text: string): ParsedStatus { } else if (key === 'branch.upstream') { out.upstream = value; } else if (key === 'branch.ab') { + sawAb = true; const m = /^\+(\d+) -(\d+)$/.exec(value); if (m) { out.ahead = Number(m[1]); @@ -203,6 +222,7 @@ export function parsePorcelainV2(text: string): ParsedStatus { } // '!' (ignored) is not requested; anything unknown is skipped rather than guessed at. } + out.upstreamGone = out.upstream !== null && !sawAb; return out; } @@ -241,8 +261,9 @@ export const runGit: GitRunner = async (cwd, args) => { const { stdout } = await execFileAsync( 'git', // --no-optional-locks: never touch the index just to look. core.fsmonitor=false: do not start or - // consult a filesystem monitor on behalf of a poll. - ['--no-optional-locks', '-c', 'core.fsmonitor=false', ...args], + // consult a filesystem monitor on behalf of a poll. log.showSignature=false: `git log` must not run + // a configured gpg.program to verify signatures. + ['--no-optional-locks', '-c', 'core.fsmonitor=false', '-c', 'log.showSignature=false', ...args], { cwd, timeout: GIT_TIMEOUT_MS, @@ -289,9 +310,11 @@ async function collect(cwd: string, git: GitRunner): Promise } }; - const hasUpstream = parsed.upstream !== null; - // With an upstream: what is ahead of it. Without one (a branch never pushed, or a detached HEAD): - // what is on HEAD but on no remote-tracking ref at all. + // A configured upstream whose remote branch is gone has no `branch.ab`, and `@{upstream}` no longer + // resolves: treat it as no usable upstream rather than letting the failed rev-list read as 0. + const hasUpstream = parsed.upstream !== null && !parsed.upstreamGone; + // With an upstream: what is ahead of it. Without one (a branch never pushed, a detached HEAD, or an + // upstream that is gone): what is on HEAD but on no remote-tracking ref at all. const range = hasUpstream ? ['@{upstream}..HEAD'] : ['HEAD', '--not', '--remotes']; const [root, remotes, stash, countText, logText] = await Promise.all([ safe(['rev-parse', '--show-toplevel']), @@ -321,6 +344,7 @@ async function collect(cwd: string, git: GitRunner): Promise branch: parsed.branch, detached: parsed.detached, upstream: parsed.upstream, + upstreamGone: parsed.upstreamGone, ahead: parsed.ahead, behind: parsed.behind, hasRemote, @@ -386,7 +410,7 @@ export async function getGitWorkspaceStatus( /** How far below the working directory to look for repositories (`cwd/a/b` is found, `cwd/a/b/c` is not). */ const DISCOVERY_MAX_DEPTH = 2; -/** Directory entries inspected per folder, so a folder with thousands of children costs a bounded readdir. */ +/** Directory entries inspected per folder (after sorting), so a folder with thousands of children stays cheap. */ const DISCOVERY_MAX_ENTRIES = 300; /** Repositories reported for one workspace. */ export const MAX_REPOS = 12; @@ -429,6 +453,21 @@ const realOr = async (p: string): Promise => { } }; +/** Real paths of `dirs` (a Docker case workspace may be reached through a symlink). */ +const realAll = (dirs: string[]): Promise => Promise.all(dirs.map(realOr)); + +const isWithin = (child: string, root: string): boolean => child === root || child.startsWith(root + sep); + +/** + * True when `path` is, or is inside, any of the (already real) `roots`. Used for Docker case + * workspaces: a container can write there, so git must not run on its behalf on the host. + */ +export async function isInsideAny(path: string, realRoots: string[]): Promise { + if (!realRoots.length) return false; + const real = await realOr(path); + return realRoots.some((r) => isWithin(real, r)); +} + /** * True when `repoRoot` is a repository that merely contains the workspace and is the home folder or * above it (`$HOME` managed as a dotfiles repo, `/`, `/home`): its changes are not the session's work. @@ -449,24 +488,51 @@ async function hasDotGit(dir: string): Promise { } } +/** Most directory entries READ from one folder before sorting and slicing, so the scan of a huge folder is bounded. */ +const DISCOVERY_MAX_SCAN = 5000; + +/** Up to `DISCOVERY_MAX_SCAN` entries of `dir` (null when unreadable). */ +async function readDirBounded(dir: string): Promise { + let handle; + try { + handle = await fs.opendir(dir); + } catch { + return null; + } + const out: import('node:fs').Dirent[] = []; + try { + for await (const e of handle) { + out.push(e); + if (out.length >= DISCOVERY_MAX_SCAN) break; + } + } catch { + /* a folder that fails mid-read: use what was read */ + } finally { + await handle.close().catch(() => {}); + } + return out; +} + /** Repositories up to `DISCOVERY_MAX_DEPTH` levels below `cwd`, nearest and alphabetical first. Never follows symlinks. */ -export async function discoverChildRepos(cwd: string): Promise<{ dirs: string[]; truncated: boolean }> { +export async function discoverChildRepos( + cwd: string, + excludeRealRoots: string[] = [] +): Promise<{ dirs: string[]; truncated: boolean }> { const found: string[] = []; let level = [cwd]; for (let depth = 1; depth <= DISCOVERY_MAX_DEPTH && level.length > 0; depth++) { const next: string[] = []; for (const dir of level) { - let entries; - try { - entries = (await fs.readdir(dir, { withFileTypes: true })).slice(0, DISCOVERY_MAX_ENTRIES); - } catch { - continue; - } + const entries = await readDirBounded(dir); + if (!entries) continue; entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + entries.length = Math.min(entries.length, DISCOVERY_MAX_ENTRIES); for (const e of entries) { // isDirectory() is false for a symlink, which is how a link to elsewhere is never followed. if (!e.isDirectory() || e.name.startsWith('.') || DISCOVERY_SKIP.has(e.name)) continue; const child = join(dir, e.name); + // A Docker case workspace (or anything inside one) is never inspected, nor descended into. + if (await isInsideAny(child, excludeRealRoots)) continue; if (await hasDotGit(child)) found.push(child); else next.push(child); } @@ -498,13 +564,27 @@ async function mapLimited(items: T[], limit: number, fn: (item: T) => Prom */ export async function getGitWorkspaceOverview( cwd: string, - opts: { git?: GitRunner; now?: () => number; fresh?: boolean; home?: string } = {} + opts: { + git?: GitRunner; + now?: () => number; + fresh?: boolean; + home?: string; + /** Docker case workspaces (host paths): repositories at or inside these are never inspected. */ + dockerWorkspaces?: string[]; + } = {} ): Promise { const now = opts.now ?? Date.now; + const dockerRoots = await realAll(opts.dockerWorkspaces ?? []); + // Checked BEFORE any git runs: git walks up from cwd, and a repository the container can write to + // could carry config (a clean filter) that runs on the host. + if (await isInsideAny(cwd, dockerRoots)) return emptyOverview('unsupported', { reason: 'docker' }); const primary = await getGitWorkspaceStatus(cwd, opts); if (primary.state === 'error') return emptyOverview('error', { error: primary.error }); const home = opts.home ?? homedir(); + if (primary.repoRoot && (await isInsideAny(primary.repoRoot, dockerRoots))) { + return emptyOverview('unsupported', { reason: 'docker' }); + } if (primary.state === 'ok' && !(primary.repoRoot && (await isUnrelatedAncestor(primary.repoRoot, cwd, home)))) { const root = primary.repoRoot ?? cwd; return { @@ -520,7 +600,7 @@ export async function getGitWorkspaceOverview( let found: { dirs: string[]; truncated: boolean }; if (!opts.fresh && hit && now() - hit.at < DISCOVERY_TTL_MS) found = hit.value; else { - found = await discoverChildRepos(cwd); + found = await discoverChildRepos(cwd, dockerRoots); discoveryCache.set(cwd, { at: now(), value: found }); if (discoveryCache.size > CACHE_MAX_ENTRIES) discoveryCache.delete(discoveryCache.keys().next().value as string); } @@ -546,17 +626,18 @@ export interface GitFileDiff { binary: boolean; } -/** A repo-relative path git reported, minus anything that could be read as an option or escape the repo. */ +/** A repo-relative path git reported, minus anything that could escape the repo. (A leading `-` is fine: every operand follows `--`.) */ export function isSafeRepoRelativePath(p: string): boolean { - if (!p || p.length > 4096 || p.includes('\0') || p.startsWith('-') || p.startsWith('/')) return false; + if (!p || p.length > 4096 || p.includes('\0') || p.startsWith('/')) return false; return !p.split('/').includes('..'); } /** * The diff of one changed file, as the panel's rows describe it: `staged` is index vs HEAD, * `unstaged`/`conflicted` is working tree vs index (a conflict shows git's combined diff), and - * `untracked` is the whole file as additions. Read-only, and `--no-ext-diff --no-textconv` keep a - * repository's own config from running programs on behalf of a click. + * `untracked` is the whole file as additions. Read-only. `--no-ext-diff --no-textconv` stop the external + * diff and textconv drivers a repository configures; a clean filter still runs, as it does for any + * `git diff`, which is why a container-writable repository never reaches this function. */ export async function getGitFileDiff( repoRoot: string, diff --git a/src/web/public/git-status-ui.js b/src/web/public/git-status-ui.js index 185e94b9..ff6d6309 100644 --- a/src/web/public/git-status-ui.js +++ b/src/web/public/git-status-ui.js @@ -75,6 +75,9 @@ Object.assign(CodemanApp.prototype, { if (!on) { this._gitStatus = null; this._gitStatusEpoch = (this._gitStatusEpoch || 0) + 1; // an in-flight read must not repaint + // That read's `finally` no longer owns the flag (its epoch is stale), so release it here: left set, + // turning the setting back on would skip every refresh for this session until a reload. + this._gitStatusInFlight = false; this.closeGitStatusPanel(); } this._renderGitStatusButton(); @@ -285,6 +288,10 @@ Object.assign(CodemanApp.prototype, { if (!body) return; const overview = this._currentGitStatus(); const el = (tag, cls, text) => this._gitEl(tag, cls, text); + // The 15 s poll replaces every row: put keyboard focus back on the same file afterwards. + const focusKey = body.contains(document.activeElement) + ? document.activeElement.closest?.('[data-git-key]')?.dataset.gitKey + : null; const view = this._gitDiffView; if (view && view.sessionId === this.activeSessionId) { // A file's diff is on screen: the 15 s poll re-renders the panel, and must not throw it away. @@ -316,7 +323,9 @@ Object.assign(CodemanApp.prototype, { overview.state === 'not-a-repo' ? 'No git repository here: this session’s folder is not one, and none was found inside it (up to two levels down).' : overview.state === 'unsupported' - ? `Git status is not available for ${overview.reason === 'docker' ? 'Docker' : 'remote (SSH)'} sessions.` + ? overview.reason === 'docker' + ? 'Git status is not available for Docker sessions, or for folders inside a Docker case workspace.' + : 'Git status is not available for remote (SSH) sessions.' : `Could not read the repository: ${overview.error || 'git failed'}`; body.append(el('div', 'git-status-empty', why)); clearChrome(); @@ -342,6 +351,10 @@ Object.assign(CodemanApp.prototype, { if (foot) { foot.textContent = `Checked ${new Date(overview.checkedAt).toLocaleTimeString()}. Read-only: Codeman never fetches or changes the repository, so “behind” is as of your last fetch.`; } + if (focusKey) { + const again = [...body.querySelectorAll('[data-git-key]')].find((n) => n.dataset.gitKey === focusKey); + again?.focus({ preventScroll: true }); + } }, /** @@ -384,7 +397,12 @@ Object.assign(CodemanApp.prototype, { // Branch / upstream line. const line = el('div', 'git-status-branchline'); - if (data.upstream) { + if (data.upstream && data.upstreamGone) { + line.append(el('span', 'git-status-chip', `${data.branch || 'HEAD'} → ${data.upstream}`)); + const gone = el('span', 'git-status-chip git-status-chip--warn', 'Upstream is gone'); + gone.title = 'The remote branch was deleted (and pruned), so the commits below are on no remote.'; + line.append(gone); + } else if (data.upstream) { line.append(el('span', 'git-status-chip', `${data.branch || 'HEAD'} → ${data.upstream}`)); if (data.ahead) line.append(el('span', 'git-status-chip git-status-chip--warn', `↑ ${data.ahead} ahead`)); if (data.behind) { @@ -449,9 +467,15 @@ Object.assign(CodemanApp.prototype, { ) ); } else { - if (!data.upstream) { + if (!data.upstream || data.upstreamGone) { pushSection.append( - el('div', 'git-status-note', 'This branch has no upstream, so these commits are on no remote yet.') + el( + 'div', + 'git-status-note', + data.upstreamGone + ? 'The upstream branch is gone from the remote, so these commits are on no remote.' + : 'This branch has no upstream, so these commits are on no remote yet.' + ) ); } for (const c of data.unpushed) pushSection.append(this._gitCommitRow(c)); @@ -523,6 +547,7 @@ Object.assign(CodemanApp.prototype, { f.kind === 'untracked' ? '?' : f.kind === 'conflicted' ? 'U' : f.kind === 'staged' ? f.index : f.worktree; const badge = el('span', `git-status-badge git-status-badge--${letter === '?' ? 'new' : letter}`, letter); badge.title = GIT_STATUS_BADGE_TITLE[letter] || letter; + row.dataset.gitKey = `${f.kind}|${f.path}`; row.append(badge); const name = el('span', 'git-status-path', displayName ?? f.path); if (displayName) name.title = f.path; diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 61b148a0..233d08b3 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -19316,16 +19316,18 @@ html .toolbar .btn-git-status.git-status--conflict { border-color: rgba(229, 83, 75, 0.5); } -.btn-git-status[aria-expanded='true'] { - background: rgba(255, 255, 255, 0.12); +html .toolbar .btn-git-status[aria-expanded='true'] { + background: color-mix(in srgb, currentColor 16%, transparent); } /* Same look as the Files window. Default spot is just left of it so both can be open. */ .git-status-panel { position: fixed; top: calc(var(--header-height) + 10px); - right: 320px; + /* Just left of the Files window; on a narrow viewport slide right so the left edge never leaves the screen. */ + right: clamp(8px, calc(100vw - 388px), 320px); width: 380px; + max-width: calc(100vw - 16px); height: calc(100vh - var(--header-height) - var(--toolbar-height) - 40px); height: calc(100dvh - var(--header-height) - var(--toolbar-height) - 40px); max-height: 600px; diff --git a/src/web/routes/git-status-routes.ts b/src/web/routes/git-status-routes.ts index 73200aab..5f64223b 100644 --- a/src/web/routes/git-status-routes.ts +++ b/src/web/routes/git-status-routes.ts @@ -5,13 +5,16 @@ * projects (see `getGitWorkspaceOverview` for exactly which). * * Read-only and offline: it never fetches and never runs a git write command. A remote (SSH) or - * Docker session is not inspected and answers `state: 'unsupported'`: a Docker workspace is writable - * from inside the sandbox, and git here would run on the host. Ownership goes through + * Docker session is not inspected and answers `state: 'unsupported'`, and neither is any repository at or + * inside a Docker case workspace (a container can write there, and git here would run on the host). Ownership goes through * `findSessionOrFail`, like every session-scoped route. */ import type { FastifyInstance } from 'fastify'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; +import { redactGitCredentials } from '../../git-clone.js'; +import { readDockerCases } from '../../docker-hosts.js'; +import { getDataDir } from '../../config/instance.js'; import { findSessionOrFail } from '../route-helpers.js'; import { emptyOverview, @@ -24,14 +27,30 @@ import { } from '../../git-workspace-status.js'; import type { SessionPort } from '../ports/index.js'; -export function registerGitStatusRoutes(app: FastifyInstance, ctx: SessionPort, git?: GitRunner): void { +/** Host paths of every Docker case workspace: repositories at or inside these are never inspected. */ +const defaultDockerWorkspaces = async (): Promise => + (await readDockerCases(getDataDir()).catch(() => [])).map((c) => c.hostWorkspacePath).filter(Boolean); + +export function registerGitStatusRoutes( + app: FastifyInstance, + ctx: SessionPort, + git?: GitRunner, + dockerWorkspaces: () => Promise = defaultDockerWorkspaces +): void { app.get('/api/sessions/:id/git-status', async (req): Promise> => { const { id } = req.params as { id: string }; const { fresh } = req.query as { fresh?: string }; const session = findSessionOrFail(ctx, id, req); if (session.remote) return { success: true, data: emptyOverview('unsupported', { reason: 'remote' }) }; if (session.docker) return { success: true, data: emptyOverview('unsupported', { reason: 'docker' }) }; - return { success: true, data: await getGitWorkspaceOverview(session.workingDir, { git, fresh: fresh === '1' }) }; + return { + success: true, + data: await getGitWorkspaceOverview(session.workingDir, { + git, + fresh: fresh === '1', + dockerWorkspaces: await dockerWorkspaces(), + }), + }; }); // The diff of one file the panel lists. `repo` and `path` are matched against the CURRENT status @@ -45,7 +64,11 @@ export function registerGitStatusRoutes(app: FastifyInstance, ctx: SessionPort, reply.code(400); return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Git is not available for remote or Docker sessions'); } - const overview = await getGitWorkspaceOverview(session.workingDir, { git, fresh: true }); + const overview = await getGitWorkspaceOverview(session.workingDir, { + git, + fresh: true, + dockerWorkspaces: await dockerWorkspaces(), + }); const status = overview.repos.find((r) => r.status.repoRoot === repo)?.status; const entry = status?.files.find((f) => f.path === path && f.kind === (kind as GitFileKind)); if (!status?.repoRoot || !entry) { @@ -56,7 +79,10 @@ export function registerGitStatusRoutes(app: FastifyInstance, ctx: SessionPort, return { success: true, data: await getGitFileDiff(status.repoRoot, entry, { git }) }; } catch (err) { reply.code(500); - return createErrorResponse(ApiErrorCode.INTERNAL_ERROR, `git diff failed: ${getErrorMessage(err)}`); + return createErrorResponse( + ApiErrorCode.INTERNAL_ERROR, + `git diff failed: ${redactGitCredentials(getErrorMessage(err))}` + ); } }); } diff --git a/test/git-status.browser.test.ts b/test/git-status.browser.test.ts index 615917cd..a63ac15e 100644 --- a/test/git-status.browser.test.ts +++ b/test/git-status.browser.test.ts @@ -203,6 +203,23 @@ describe('Git status indicator in a real browser', () => { await page.click('#gitStatusBody button:has-text("Back")'); }); + it('keeps keyboard focus on the same file row across the 15 s re-render', async () => { + await page.focus('.git-status-file:has-text("a.txt")'); + const key = () => page.evaluate(() => (document.activeElement as HTMLElement | null)?.dataset?.gitKey ?? null); + expect(await key()).toBe('unstaged|a.txt'); + await refresh(); + await page.waitForFunction(() => document.activeElement?.getAttribute('data-git-key') === 'unstaged|a.txt'); + expect(await key()).toBe('unstaged|a.txt'); + }); + + it('the panel never starts off-screen, even on a 650px-wide viewport', async () => { + const original = page.viewportSize()!; + await page.setViewportSize({ width: 650, height: original.height }); + const left = await page.evaluate(() => document.getElementById('gitStatusPanel')!.getBoundingClientRect().left); + await page.setViewportSize(original); + expect(left).toBeGreaterThanOrEqual(0); + }); + it('drags by the header', async () => { const before = await page.evaluate(() => document.getElementById('gitStatusPanel')!.getBoundingClientRect().left); const box = (await page.locator('.git-status-header').boundingBox())!; @@ -352,6 +369,33 @@ describe('Git status indicator in a real browser', () => { }); }); + it('turning the setting off while a read is in flight, then on again, does not leave polling dead', async () => { + let slow = true; + await page.route('**/api/sessions/*/git-status*', async (route) => { + if (slow) await new Promise((r) => setTimeout(r, 1500)); + await route.continue(); + }); + page.setDefaultTimeout(6000); + // A background poll may be mid-read: let it finish so OUR read is the one the slow route holds. + await page.waitForFunction(() => (window as any).app._gitStatusInFlight === false); + await page.evaluate(() => void (window as any).app.refreshGitStatus({ fresh: true })); + await page.waitForFunction(() => (window as any).app._gitStatusInFlight === true); + await setSetting(false); + expect(await page.evaluate(() => (window as any).app._gitStatusInFlight)).toBe(false); + slow = false; + await setSetting(true); + // Re-enabling starts its own read. With the flag stuck true that read is skipped for this session + // and the indicator never comes back. + await page.waitForFunction(() => !!(window as any).app._currentGitStatus()); + expect(await page.evaluate(() => (window as any).app._gitStatusInFlight)).toBe(false); + page.setDefaultTimeout(30000); + await page.unroute('**/api/sessions/*/git-status*'); + expect(await buttonVisible()).toBe(true); + // Turning the setting off closed the panel; reopen it for the tests that follow. + await page.click('#gitStatusBtn'); + await page.waitForSelector('#gitStatusPanel.visible'); + }, 30000); + it('closing the panel resets it; turning the setting off hides the button, closes the panel and stops polling', async () => { await page.click('.git-status-actions button[aria-label="Close git status"]'); expect(await page.isVisible('#gitStatusPanel')).toBe(false); @@ -364,5 +408,5 @@ describe('Git status indicator in a real browser', () => { const before = gitStatusRequests.length; await page.waitForTimeout(3000); expect(gitStatusRequests.length).toBe(before); - }); + }, 30000); }); diff --git a/test/git-workspace-status.test.ts b/test/git-workspace-status.test.ts index e9b6e617..02455cc0 100644 --- a/test/git-workspace-status.test.ts +++ b/test/git-workspace-status.test.ts @@ -33,6 +33,18 @@ describe('parsePorcelainV2', () => { }); }); + it('flags an upstream whose remote branch is gone: branch.upstream without branch.ab', () => { + expect( + parsePorcelainV2(['# branch.oid x', '# branch.head feature', '# branch.upstream origin/feature'].join(NUL) + NUL) + ).toMatchObject({ upstream: 'origin/feature', upstreamGone: true }); + expect( + parsePorcelainV2( + ['# branch.oid x', '# branch.head main', '# branch.upstream origin/main', '# branch.ab +0 -0'].join(NUL) + NUL + ) + ).toMatchObject({ upstreamGone: false }); + expect(parsePorcelainV2(['# branch.oid x', '# branch.head feature'].join(NUL) + NUL).upstreamGone).toBe(false); + }); + it('reads a detached HEAD and a branch with no upstream (no branch.ab line either)', () => { expect(parsePorcelainV2(['# branch.oid x', '# branch.head (detached)'].join(NUL) + NUL)).toMatchObject({ branch: null, @@ -314,6 +326,21 @@ describe('getGitWorkspaceStatus against a real repository', () => { expect(s.unpushed.map((c) => c.subject)).toEqual(['f2', 'f1']); }); + it('a branch whose upstream was deleted and pruned is NOT reported as everything pushed', async () => { + git(repo, 'checkout', '-q', '-b', 'feature'); + write('f1.txt'); + commit('f1'); + git(repo, 'push', '-q', '-u', 'origin', 'feature'); + git(repo, 'push', '-q', 'origin', '--delete', 'feature'); + git(repo, 'fetch', '-q', '--prune'); + write('f2.txt'); + commit('f2'); + const s = await getGitWorkspaceStatus(repo); + expect(s).toMatchObject({ branch: 'feature', upstream: 'origin/feature', upstreamGone: true }); + expect(s.unpushedCount).toBe(2); + expect(s.unpushed.map((c) => c.subject)).toEqual(['f2', 'f1']); + }); + it('a pushed branch is not reported as unpushed once it has an upstream', async () => { git(repo, 'checkout', '-q', '-b', 'feature'); write('f1.txt'); @@ -668,7 +695,8 @@ describe('isSafeRepoRelativePath', () => { ['a.txt', true], ['src/deep/x.ts', true], ['', false], - ['-rf', false], + ['-rf', true], // every operand follows `--`, so a leading dash is just a name + ['-', true], ['/etc/passwd', false], ['../x', false], ['a/../../x', false], @@ -725,3 +753,80 @@ describe('getGitFileDiff', () => { expect(cut.diff.endsWith('x')).toBe(true); }); }); + +describe('Docker case workspaces are never inspected', () => { + let top: string; + let home: string; + const repoAt = (p: string): string => { + mkdir(p, { recursive: true }); + git(p, 'init', '-q', '-b', 'main'); + writeFileSync(join(p, 'f.txt'), '1\n'); + git(p, 'add', '-A'); + git(p, 'commit', '-q', '-m', 'c'); + return p; + }; + /** A repository whose clean filter drops a marker file: proof that git ran on it. */ + const booby = (p: string): string => { + repoAt(p); + git(p, 'config', 'filter.mark.clean', 'touch RAN; cat'); + writeFileSync(join(p, '.gitattributes'), 'f.txt filter=mark\n'); + // Same size as the committed '1\n', so git must read the content (running the filter) to see the change. + writeFileSync(join(p, 'f.txt'), 'x\n'); + return p; + }; + + beforeEach(() => { + top = mkdtempSync(join(tmpdir(), 'git-docker-')); + home = join(top, 'home'); + mkdir(home, { recursive: true }); + clearGitStatusCache(); + }); + afterEach(() => rmSync(top, { recursive: true, force: true })); + + it('control: without the exclusion git does run the repository’s clean filter', async () => { + const ws = join(home, 'case'); + booby(join(ws, 'proj')); + await getGitWorkspaceOverview(ws, { home, git: undefined }); + expect(existsSync(join(ws, 'proj', 'RAN'))).toBe(true); + }); + + it('drops a Docker workspace found below the folder, and runs nothing in it', async () => { + const ws = join(home, 'case'); + booby(join(ws, 'sandbox')); + repoAt(join(ws, 'plain')); + const o = await getGitWorkspaceOverview(ws, { home, dockerWorkspaces: [join(ws, 'sandbox')] }); + expect(o.repos.map((r) => r.path)).toEqual(['plain']); + expect(existsSync(join(ws, 'sandbox', 'RAN'))).toBe(false); + }); + + it('answers unsupported/docker for a folder at or inside a Docker workspace, before any git runs', async () => { + const dock = booby(join(home, 'dock')); + mkdir(join(dock, 'sub')); + for (const cwd of [dock, join(dock, 'sub')]) { + clearGitStatusCache(); + const git = vi.fn(async () => ''); + const o = await getGitWorkspaceOverview(cwd, { home, git, dockerWorkspaces: [dock] }); + expect(o).toMatchObject({ state: 'unsupported', reason: 'docker', repos: [] }); + expect(git).not.toHaveBeenCalled(); + } + expect(existsSync(join(dock, 'RAN'))).toBe(false); + }); + + it('sees through a symlink to the workspace', async () => { + const dock = booby(join(home, 'dock')); + const ws = join(home, 'case'); + mkdir(ws, { recursive: true }); + symlink(dock, join(ws, 'link')); + const o = await getGitWorkspaceOverview(join(ws, 'link'), { home, dockerWorkspaces: [dock] }); + expect(o.state).toBe('unsupported'); + expect(existsSync(join(dock, 'RAN'))).toBe(false); + }); + + it('a folder next to the workspace, and one whose name merely starts the same, are not excluded', async () => { + const dock = join(home, 'dock'); + const sibling = repoAt(join(home, 'dock-two')); + mkdir(dock, { recursive: true }); + const o = await getGitWorkspaceOverview(sibling, { home, dockerWorkspaces: [dock] }); + expect(o.state).toBe('ok'); + }); +}); diff --git a/test/routes/git-status-routes.test.ts b/test/routes/git-status-routes.test.ts index dc6449f6..c5ee0fae 100644 --- a/test/routes/git-status-routes.test.ts +++ b/test/routes/git-status-routes.test.ts @@ -26,10 +26,15 @@ const git = (cwd: string, ...args: string[]) => execFileSync('git', args, { cwd, let dir: string; let session: Record; -async function setup(opts: { git?: GitRunner; authUser?: { username: string; role: 'admin' | 'user' } } = {}) { - const h = await createRouteTestHarness((app, ctx) => registerGitStatusRoutes(app, ctx, opts.git), { - authUser: opts.authUser, - }); +async function setup( + opts: { git?: GitRunner; authUser?: { username: string; role: 'admin' | 'user' }; dockerWorkspaces?: string[] } = {} +) { + const h = await createRouteTestHarness( + (app, ctx) => registerGitStatusRoutes(app, ctx, opts.git, async () => opts.dockerWorkspaces ?? []), + { + authUser: opts.authUser, + } + ); session = h.ctx._session as unknown as Record; session.workingDir = dir; return h; @@ -253,4 +258,15 @@ describe('GET /api/sessions/:id/git-diff', () => { expect(conflict.statusCode).toBe(200); expect(conflict.json().data.diff).toMatch(/<<<<<<<|\+\+<<<<<< { + const runner = vi.fn(async () => ''); + const { app } = await setup({ git: runner, dockerWorkspaces: [realpathSync(dir)] }); + const res = await app.inject({ + method: 'GET', + url: url({ repo: realpathSync(dir), path: 'a.txt', kind: 'unstaged' }), + }); + expect(res.statusCode).toBe(404); + expect(runner).not.toHaveBeenCalled(); + }); });