diff --git a/.ai/contexts/changes-view.md b/.ai/contexts/changes-view.md index d7667931..2f70cd22 100644 --- a/.ai/contexts/changes-view.md +++ b/.ai/contexts/changes-view.md @@ -34,7 +34,7 @@ integration: `.ai/contexts/viewer-panel.md` ("Changes mode"). ## Runner interface (`git-changes-runner.js`) -`createGitChangesRunner({kind, cwd, alias, exec, timeoutMs, fsOps})` → `{status(), diff(path, {staged, untracked})}`. `status()` runs three commands in parallel (`git status --porcelain=v2 --branch -uall -z`, `git diff --numstat -z`, `git diff --cached --numstat -z`), merges them, and reports `untrackedCollapsed` (see "Untracked files"). `diff()` runs `git diff [--cached] -- ` — or, with `untracked: true`, `git diff --no-index -- /dev/null ` (see "Untracked files") — capped at 512 KB (`MAX_DIFF_BYTES`) measured in UTF-8 bytes and cut on a line boundary, with a `truncated` flag. +`createGitChangesRunner({kind, cwd, alias, exec, timeoutMs, fsOps})` → `{status(), diff(path, {staged, untracked}), isWorkTree()}`. `status()` runs three commands in parallel (`git status --porcelain=v2 --branch -uall -z`, `git diff --numstat -z`, `git diff --cached --numstat -z`), merges them, and reports `untrackedCollapsed` (see "Untracked files"). `diff()` runs `git diff [--cached] -- ` — or, with `untracked: true`, `git diff --no-index -- /dev/null ` (see "Untracked files") — capped at 512 KB (`MAX_DIFF_BYTES`) measured in UTF-8 bytes and cut on a line boundary, with a `truncated` flag. - **Local** (`kind: 'local'`): `child_process.execFile('git', args, {cwd, timeout, maxBuffer})` — cwd is `execFile`'s own option, never a `-C` argument. No shell is invoked, so argument content cannot be interpreted as a command regardless of what it contains; timeout 10s. - **Remote** (`kind: 'remote'`): the same ssh transport `remote-attach.js` already uses for the tmux probe/restore calls (`buildRemoteCommandArgs`, `defaultRunRemoteCommand`) — `ssh -o BatchMode=yes -o ConnectTimeout=5 -n "git -C '' '--literal-pathspecs' 'diff' '--' '' ..."`. Timeout 20s. This command string DOES run through a shell on the far end. @@ -304,6 +304,176 @@ counts. `opts.spawnFn` is dependency injection for tests only (`test/remote-run-command-stdout-cap.test.js`, a fake `child_process`-shaped `EventEmitter` with `stdout`/`stderr`/`kill`) — production code never passes it, and the lazy `require('child_process')` stays the real default. +## Not a repository + +A session's working directory need not be inside a git work tree, and when it +is not, the Changes affordance is not offered: `#changes-toggle-btn` is +`display: none` for that session. The panel never renders a refusal for this +case, because there is nothing to refuse — the button that would produce it is +not there. + +**Withdrawing a control is only correct on positive evidence.** A missing +button says nothing and offers no way to ask why, so it is the wrong answer to +every failure except the one it describes. `isWorkTree()` in +`git-changes-runner.js` therefore reports `isRepo: false` only when something +actually established that there is no work tree, and returns `{ok: false, +error}` — *not an answer* — for everything else. The renderer's +`typeof result.isRepo !== 'boolean'` guard leaves the button alone on a +non-answer, and `status()` reports its bounded message into the tab. + +**No message is ever matched.** git translates every diagnostic (this project's +own host runs it in French), so the detection reads only `git rev-parse +--is-inside-work-tree`'s exit code and the two literal tokens it prints: + +| probe result | meaning | outcome | +|---|---|---| +| exit 0, stdout `true` | inside a work tree | `{ok: true, isRepo: true}` | +| exit 0, stdout `false` | no work tree here: a bare repository, or a cwd inside an ordinary repository's `.git/` | `{ok: true, isRepo: false}` | +| exit 0, anything else | git answered something this code does not understand | `{ok: false, error}` | +| exit 128, corroborated | see below | `{ok: true, isRepo: false}` | +| exit 128, not corroborated | a repository git refuses to open | `{ok: false, error}` | +| exit 128, cwd gone | the directory, not the repository, is what is missing | `{ok: false, error}` naming the directory | +| a spawn that never ran (`-1`) or a thrown exec | a local cwd that was deleted, is a file, or is a dangling symlink | `{ok: false, error}` naming the directory | +| any other non-zero | ssh's own `255`, a timeout | `{ok: false, error}` | + +**128 is git's generic fatal code, not a "no repository" code**, which is why it +needs corroborating. Real git exits 128 for a plain directory *and* for a +repository it will not open: dubious ownership (a repo owned by root, cloned by +another user, on a mounted or NFS filesystem), a `.git` whose permissions it +cannot read, an unsupported `core.repositoryformatversion`, a `.git` file whose +gitdir is gone, and a worktree whose main repository was deleted. Every one of +those is a repository the user has, usually with a one-line fix in git's own +message — exactly the case where silently removing the panel is worse than +printing the message. `test/git-changes-runner-real-git.test.js` builds those +fixtures against real git, asserts each really does exit 128, and pins that none +of them produces `reason: 'not-a-repo'`. + +**A cwd that is gone is ruled out before the corroboration is trusted.** The walk +below answers "no `.git` anywhere" for a path that does not exist, so a deleted +worktree outside a repository would otherwise withdraw the panel. `execFile` +happens to fail to spawn for such a cwd — code `-1`, not 128 — but the local and +remote transports differ here (`git -C ` exits 128), so the check is an +outcome of its own rather than something left to a code that happens not to +match. It also decides the wording: `spawn git ENOENT` reads as "git is not +installed", where the truth is that the directory is gone, and the message names +it. + +`fsOps` is dependency injection for tests only, and both functions that use it +require a complete seam: `missingCwdError` throws a `TypeError` rather than +treating a missing `stat` as "the cwd is fine", which would switch the check off +with nothing to show for it. + +The corroboration is `gitEntryAtOrAbove(cwd)`: an `fs.lstat` for a `.git` entry +at the cwd and at each ancestor up to the filesystem root. It needs no process +and no locale, and it answers the one question the exit code cannot — *is there +a repository here at all*. It returns three ways, and only `false` (a walk that +reached the root seeing nothing) withdraws the panel; an `EACCES` or any other +unexpected `lstat` error is `null`, undecidable, and reports. A `.git` that +exists but is broken counts as `true`: the repository is there, it is just +unreadable. + +**Local and remote diverge here, deliberately.** The probe itself goes through +the same `invoke()` and the same quoting on both transports, but the +corroboration is a local filesystem walk and there is no remote equivalent that +does not either re-read git's translated message or add ssh round-trips. So a +remote session that exits 128 is never corroborated and always reports. The +practical consequence: a remote working directory that is genuinely not a +repository keeps its Changes button and shows git's own bounded message when +clicked, instead of hiding the button. That is the pre-existing behaviour, and +it is the safe side of the trade — it also means a remote cwd that is merely +unmounted no longer loses the control. + +**Who asks, and when.** Two paths reach the same conclusion, and `status()` is +the cheaper of them: + +- `git-changes-available` runs the probe from `switchPanel()`. The answer is + cached on that session's `filePanelState` entry (`changesAvailable`) and + applied to the button before the round trip, so a known answer never flashes a + button that does not work. +- `status()` returns `{ok: false, reason: 'not-a-repo'}` when a command failed + **and** the probe then establishes there is no work tree. The renderer treats + that exactly like an `isRepo: false` availability answer. That covers the + window between a switch and the repository disappearing under a running + session, and it means a click landing before the availability answer arrives + is handled too. + +**The probe is bounded on both axes.** It is a *diagnosis, not a precondition*: +a session in a repository pays three commands per refresh, the same three as +before, pinned by `calls.length === 3`. A `-uall` run that overruns the stdout +cap does not ask either — that is a volume problem with its own fallback, and +the large repositories that hit it are the ones an extra spawn costs most. And +on the switch path: + +- a session already answered `true` is **never probed again**; +- a session git **could not answer for** is never probed again either. That + answer is memoised as its own state (`CHANGES_UNANSWERED`), because a + `{ok: false}` leaves the button visible and can never change it — asking again + buys nothing and costs an ssh with a 20 s kill timer. It is not a rare shape: + a remote cwd outside a repository, a local repository git refuses, and an + unreachable host all produce it, on every activation, forever; +- **a non-answer never displaces an answer.** `CHANGES_UNANSWERED` is written + only for a session nothing has been established for yet, so the two memo + writes cannot collide. "No repository" is the one answer deliberately + re-asked, which makes it the one a transient failure — an ssh blip, a sleeping + host — can land on; overwriting it would un-hide a button for a directory that + is definitely not a repository, and then never ask again. A session that keeps + its `false` stays re-askable, so the blip costs nothing beyond that one probe; +- a second probe for a session whose first is still in flight is **dropped** + (`changesAvailabilityInFlight`), so a burst of switches cannot put a burst of + ssh children on a remote host; +- an answer is **recorded against the session it is about**, then applied to the + button only while that session is still the one on screen — on every branch, + because "a stale reply never touches the DOM" is an invariant, not a + per-branch outcome. Discarding a + correct answer because the panel had moved on would cost that session another + probe on its next activation; painting from it would paint the wrong + session's state. + +Only a session that answered "no repository" is re-asked on a later switch. +That is the one answer worth re-checking — a `git init` turns it into a +repository — and it is what makes the button come back without polling +anything. The reverse transition is not tracked: a session that answered "in a +repository" keeps its button even if the repository is deleted under it, and +clicking Changes then closes the tab straight away through the `status()` path. +Re-probing every activation to catch that is exactly the cost this memo exists +to remove. + +**Withdrawal reuses the tab's own close control.** `noteChangesUnavailable` +calls `toggleChangesTab(sessionId)` rather than tearing the tab down itself, so +it takes the same path a user's click on the Changes toggle takes and inherits +`confirmDiscardChangesEdits` along with it. A repository that stops being one +under an open editor therefore asks before discarding the buffer, exactly as +the toggle does. + +A Changes tab has a second way out: the panel's own X +(`changesCloseBtn` → `handleClose`), which clears `currentTab` and hides the +panel directly without passing through `toggleChangesTab`, and carries its own +call to the same guard. What keeps a tab from being torn down without asking is +that **each exit is guarded**, not that they funnel into one — a new exit has to +be guarded on its own terms, and reusing an existing one is how the withdrawal +avoids being such an exit. + +If the close does not happen — the user refused the discard, so the tab is still +there afterwards — the button is left visible, because hiding the control while +its tab is still open would strand the edit it is holding with no way back to +it. + +## Bounded error messages + +`firstError()` puts every unexpected git failure through `boundErrorMessage()`: +at most `MAX_ERROR_LINES` (5) lines and `MAX_ERROR_CHARS` (500) characters, with +a trailing `…` when anything was dropped. Git's `diff --no-index` usage page is +over 150 lines and git prints it on a plain exit-129 misuse; unbounded, that +page is what the panel would display. The bound is the first lines rather than a +flat character cut because git's own diagnosis is on the first line and the +noise is below it. + +A genuine failure — a permission error, a corrupt repository, a transport +problem — is still reported, in git's own words and in whatever language git +chose. Only the volume is capped. That is what "Not a repository" above leans +on: every failure the probe cannot positively explain falls back to this +message rather than to a missing button. + ## cwd resolution (`git-changes-target.js`) `resolveGitChangesTarget(sessionId, deps)`, in order: diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index c0b69498..aa6e164d 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -89,7 +89,8 @@ design (parser, runner, quoting, cwd resolution, refresh triggers, editing): | IPC | Args | Returns | Notes | |---|---|---|---| -| `git-changes-status` | `(sessionId)` | `{ok, kind, branch, files, totals, untrackedCollapsed} \| {ok:false, error}` | `git status --porcelain=v2 --branch -uall` + `git diff --numstat` + `git diff --cached --numstat`, merged by `git-changes.js`'s `mergeChanges()`. A `-uall` run too large for the transport falls back to git's default untracked mode and reports `untrackedCollapsed: true`. `kind` is `'local'` or `'remote'` — the renderer decides from it whether the panel is editable. | +| `git-changes-available` | `(sessionId)` | `{ok:true, isRepo} \| {ok:false, error}` | `git rev-parse --is-inside-work-tree`, one invocation. Answers by exit code and the `true`/`false` token, never by message — see `.ai/contexts/changes-view.md` ("Not a repository"). `isRepo:false` is what withdraws the Changes button and is returned only on positive evidence; exit 128 is git's generic fatal code and needs a corroborating filesystem walk (local only). Everything else — a refused repository, a cwd that is gone, ssh's own 255, a thrown exec — is `{ok:false, error}`, which leaves the button alone and is memoised so the session is not asked again. | +| `git-changes-status` | `(sessionId)` | `{ok, kind, branch, files, totals, untrackedCollapsed} \| {ok:false, reason?, error}` | `git status --porcelain=v2 --branch -uall` + `git diff --numstat` + `git diff --cached --numstat`, merged by `git-changes.js`'s `mergeChanges()`. A `-uall` run too large for the transport falls back to git's default untracked mode and reports `untrackedCollapsed: true`. `kind` is `'local'` or `'remote'` — the renderer decides from it whether the panel is editable. A cwd with no work tree comes back `reason: 'not-a-repo'`; every other failure carries a bounded message and no reason. | | `git-changes-diff` | `(sessionId, filePath, staged, untracked)` | `{ok, content, truncated, added, deleted} \| {ok:false, error}` | `git diff [--cached] -- `, or `git diff --no-index -- /dev/null ` when `untracked`; capped at 512 KB. `added`/`deleted` are filled for an untracked file only — see `.ai/contexts/changes-view.md` ("Untracked files"). | | `git-changes-file` | `(sessionId, filePath, {staged})` | `{ok, original, current, version, binary, truncated} \| {ok:false, error, reason}` | The content pair behind the editable diff: `git cat-file blob :` (or `HEAD:` when `staged`) and the working-tree file, both LF-normalised and strictly UTF-8. `version` is an opaque token the renderer hands back on save. Local sessions only. `reason` is one of `invalid-path`, `repo`, `missing`, `outside`, `symlink`, `hardlink`, `git-dir`, `sensitive`, `not-a-file`, `binary`, `too-large`, `encoding`, `mixed-eol`, `git`, `remote`. | | `git-changes-save` | `(sessionId, filePath, content, version)` | `{ok:true, version} \| {ok:false, error, reason}` | Writes the working-tree file the guard resolved, re-applying its line endings, and returns the token for the next save. Refused with `reason:'stale'` when the file changed since `version` was issued, and with `invalid-version` when no token is passed. Local sessions only; never creates a file. | diff --git a/.ai/contexts/panel-terminal.md b/.ai/contexts/panel-terminal.md index dcb1e584..6207b204 100644 --- a/.ai/contexts/panel-terminal.md +++ b/.ai/contexts/panel-terminal.md @@ -50,13 +50,26 @@ everywhere a live PTY stands for a session the user can see or act on: | Place | Why it must skip a panel shell | |---|---| | `get-active-terminals` (main) | It restores plain-terminal *rows*; a panel shell has none. | +| `buildProjectsFromCache` (main) | It injects every live plain-terminal PTY into the `get-projects` payload so a terminal sorts among the session rows. That payload *is* the sidebar, and the renderer's `dedup()` copies it into `sessionMap` — so an unfiltered shell becomes a session row of its own, labelled `Terminal` from the hard-coded `summary` here rather than the `Shell` the renderer sets on its own object. It also lands in `cachedAllProjects`, which is what `renderDefaultStatus` sums for "N sessions". | +| `layoutGridCards` (renderer) | Only *implicitly*, and the row above is what does it. Grid mode has no panel-shell predicate: it walks the sidebar's DOM rows and wraps every id that is also in `openGridSessionIds()`, which **does** contain `panel:`. Given a row, it would move the shell's container out of `#panel-terminal-region` into a grid card and cost it the WebGL context the mount/unmount pair manages. The absent row is therefore load-bearing for a second subsystem, and `test/panel-terminal.test.js` pins both halves. | | the missing-project remap guard (main) | It refuses a remap while sessions append to the transcripts being rewritten, and tells the user to stop them first. A shell appends to nothing and offers nothing to stop. | | `scheduleActiveSessionsPoll` (renderer) | A non-empty `activePtyIds` pins the poll at 3 s. One open shell would cancel the 30 s idle back-off of the v0.0.33–41 perf campaign for the whole window. | | `renderDefaultStatus` (renderer) | "N running" would count a session with a shell twice. | -The renderer side goes through `countSessionsWithoutPanelShells(ids)`, which -keys on the `panel:` id prefix rather than on a lookup, because `activePtyIds` -holds ids and nothing else. +**The two sides of the exclusion use different keys, because they hold +different things.** In the main process the value is the session object, so +`isPanelShellSession(session)` (`panel-terminal-target.js`) reads `panelFor` — +the field that actually defines a panel shell. `panel-terminal-target.js` +requires nothing, so any main-process module can import it without a cycle. The +renderer holds ids and nothing else (`activePtyIds`), so +`countSessionsWithoutPanelShells(ids)` keys on the `panel:` id prefix instead. + +Every other `isPlainTerminal` reader either *skips* plain terminals already +(`cli-session-state.js`, `session-transitions.js`, and the `!isPlainTerminal` +branches in `main.js`) — which excludes a shell for free — or is per-PTY spawn +and wiring behaviour in `open-terminal` that a panel shell should get exactly +like any other shell. The readers that *select* plain terminals as user-facing +sessions are the two named in the table, and only those two need the predicate. ## The three lifted assumptions in terminal-manager.js @@ -107,10 +120,27 @@ height) and `.terminal-container.panel-terminal` overrides the inset to `0`. The region and its handle live at the end of `#file-panel-content`, after the viewer/diff/changes children, and are `display: none` until `.open`. -The region and its handle carry `margin-top: auto` so that with no tab open — -the state the `hidePanel` guard exists to support, where every flexible child -above is `display: none` — they sit at the bottom of the panel instead of -stacking at the top. +`#panel-terminal-handle` carries `margin-top: auto`, which is what pins the +handle and the region to the bottom of the panel while a tab is shown: the tab +above is the flexible child and the region is a fixed pixel height below it. + +**With no tab open, the region is the panel.** That is the state the `hidePanel` +guard exists to support — every flexible child above is `display: none` — and +`margin-top: auto` alone leaves the tab area as an empty dark block with the +shell squeezed into its stored height underneath. `renderTabContent()` therefore +calls `setPanelTerminalShellOnly(!tab)`, which puts `.shell-only` on +`#file-panel-content`, and two rules key off it: the handle is `display: none`, +and the region becomes `flex: 1 1 0; min-height: 0`. The handle is hidden rather +than left in place because with no tab above it there is nothing for a drag to +give space back to — it would shrink the shell and re-create the empty block the +class exists to remove. + +A `flex-basis` of `0` is what overrides the region's own `height` for layout, so +nothing writes to that height to make this work: `panelTerminalDesiredHeight`, +`localStorage.panelTerminalHeight` and the inline `style.height` all still hold +what the last drag asked for, and opening a tab again restores exactly that. The +class toggle refits the shell whenever it actually changes, since the region's +geometry changes with it. Height persists in `localStorage.panelTerminalHeight` (alongside `filePanelWidth`), floor 80 px, ceiling `#file-panel-content`'s height minus diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 53d28371..124cd344 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -29,7 +29,7 @@ From `session-cache.js`: - `init(ctx)` — wire main process → cache (mainWindow ref for IPC events) - `refreshFolder(folder, opts)` — opts `{files: Set}` for targeted refresh (watcher payload). Defaults to full folder walk. - `populateCacheFromFilesystem()` / `populateCacheViaWorker()` — initial scan / re-scan. The worker (`workers/scan-projects.js`) streams one `{type:'folder', result, current, total}` message per on-disk folder (plus a final `{type:'done'}`) instead of buffering the whole tree, so each folder is written to the DB and pushed to the renderer as soon as it's read — a large history no longer leaves the sidebar empty for the entire scan. -- `buildProjectsFromCache(showArchived)` — produces the sidebar payload (sorted, grouped by project, missing flag computed here) +- `buildProjectsFromCache(showArchived)` — produces the sidebar payload (sorted, grouped by project, missing flag computed here). It also injects every live plain-terminal PTY from `activeSessions` as a synthetic row, so a terminal sorts among the cached sessions even though it has no JSONL; the row's `summary` is the hard-coded string `Terminal`, which is what the sidebar displays — the renderer's own session object is never the one shown. A panel shell is a plain terminal too and is excluded here by `isPanelShellSession` (`panel-terminal-target.js`); see `.ai/contexts/panel-terminal.md` for the full list of places that have to skip one. - `notifyRendererProjectsChanged()` — throttled (~1.5s leading-edge) push to renderer - `sendIndexingProgress()` (internal) — emits the `indexing-progress` IPC event, gated on `coldStart` (captured once at the top of `populateCacheViaWorker()` via `!isInitialScanComplete()`) and throttled to ~4 events/s (the first event and every `done:true` always pass). Feeds the renderer's first-run banner; see `.ai/contexts/ipc-bridge.md`. A `done:true` payload carrying `error` keeps the banner visible with the failure message instead of hiding it. diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index ddf00ed6..2b7594cc 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -66,6 +66,46 @@ the terminal header's stop button, and the grid card's stop button all funnel through it) — it now asks `resolveSessionStop` which IPC to call instead of always calling `stopSession`. +### Reopening a plain terminal + +A session's `type` is what tells `open-terminal` which of its two branches to +take: `isPlainTerminal = sessionOptions?.type === 'terminal'` picks a login +shell, anything else runs `claude --resume `. The type lives on the +session object in the renderer, and every path that reopens one has to carry it +across the IPC boundary — `resolveDefaultSessionOptions` (`public/dialogs.js`) +resolves *Claude launch* options (permission mode, worktree, chrome, sandbox, +preLaunchCmd, addDirs, MCP emulation) and deliberately says nothing about the +session type, because it is also what a Claude resume uses. + +`openSession` therefore chooses the options in one chain, in this order: + +1. `customOptions`, when the caller supplied them. Only the + resume-with-config dialog does, and the sidebar never renders its button on + a terminal row (`session.type !== 'terminal'` gates the whole action group), + so an explicit choice always wins and never has to be reconciled with the + type. +2. `{type: 'terminal'}` for a session whose own `type` is `'terminal'`. A shell + has no permission mode, worktree or MCP emulation to resolve, so the + defaults call is skipped entirely. +3. `resolveDefaultSessionOptions()` otherwise. + +Without step 2, a terminal that is no longer in `openSessions` — its shell +exited and the entry was destroyed, or the renderer reloaded — reopens as a +Claude resume against an id minted by `launchTerminalSession` for a shell, +which has no transcript. `activeSessions` hides it whenever the PTY is still +live, because `open-terminal`'s reattach branch runs first; the failure needs +the PTY to be gone as well. + +**An exited terminal reopens under its own id.** Both branches of the +`openSessions` check now converge on the same reopen: a closed entry is +destroyed and the function falls through, exactly as it already did for a +Claude session. Minting a fresh id instead (`launchTerminalSession`) left the +row the user clicked behind, pointing at an id nothing could open correctly, +while the new shell arrived on a row they had not asked for. `main.js` needs +nothing for this: its plain-terminal branch ignores `isNew` and spawns a shell +either way, and the resume-cwd lookup is already guarded on +`sessionOptions?.type !== 'terminal'`. + ### Archive/delete are stop-then-archive/delete (issue #271) `public/sidebar.js`'s four archive/delete call sites (`.project-archive-btn`, diff --git a/docs/changes-view.md b/docs/changes-view.md index 7fd181a2..4912ba41 100644 --- a/docs/changes-view.md +++ b/docs/changes-view.md @@ -6,6 +6,23 @@ Click the **Changes** button in the terminal header, next to the stop button. Click it again to close. +The button is only there for a session whose working directory is inside a git +repository. A session started somewhere that is not one — a scratch directory, a +notes folder — has no Changes button at all. Run `git init` there and the button +appears the next time the panel follows that session. + +The reverse is looser: a session that was in a repository keeps its button for +the rest of the run even if you delete the repository under it. Clicking Changes +then closes the view again straight away. Checking for that on every click of +every session would cost a git command each time, which is not worth it for a +case that ends the moment you restart. + +A repository git *refuses to open* is a different case and keeps its button. If +git will not read the repository — it is owned by another user, its permissions +are wrong, or its format is one this git does not support — Changes shows you +git's own message, which usually names the fix. The button disappears only when +there is genuinely no repository there. + A file link in the terminal opens here too, when it points at one of this session's changed files: the panel opens on that row, ready to edit against its diff. A link to a file the session has not touched, or to one outside its repository, opens in the plain viewer as before. ## What it shows @@ -68,8 +85,10 @@ The session you are watching writes these files, so the panel assumes it is not Changes list, in the same directory the list is read from — so you can run a `git add`, a test, or anything else against exactly the tree you are looking at, then hit **Refresh**. Both stay visible; a horizontal handle between them -sets how much room each gets. Local sessions only — see -[Terminal](terminal.md) for the lifecycle and the remote limitation. +sets how much room each gets. With no list or file open above it, the shell +takes the whole panel and the handle is gone; whatever height you dragged to +comes back the moment you open something above it again. Local sessions only — +see [Terminal](terminal.md) for the lifecycle and the remote limitation. ## What it doesn't do @@ -94,4 +113,10 @@ The same parser and the same panel render both. Only the command runner differs: - **Local**: `git status`/`git diff` run directly against the session's real working directory (its worktree, if it has one — the same directory a `claude --resume` targets). - **Remote**: the same commands run over the existing ssh connection to the host, against the directory recorded in that session's descriptor. No attach, no tmux — this works even for a session you've never opened a terminal tab for. +A remote working directory that is not a git repository keeps its button and +shows git's message when you click, rather than hiding the button the way a +local one does. Telling "there is no repository here" apart from "git will not +open this repository" needs to look at the directory itself, which Switchboard +can only do on this machine. + Diffs are capped at 512 KB; a diff larger than that is truncated with a note at the bottom. diff --git a/docs/terminal.md b/docs/terminal.md index 8ae8912d..48f84508 100644 --- a/docs/terminal.md +++ b/docs/terminal.md @@ -6,6 +6,7 @@ Switchboard includes a full built-in terminal powered by xterm.js. You can launc - Click a session in the sidebar to open it in the terminal. If the session has an active process, you attach to its running PTY. If not, a new Claude CLI process is launched. - Click **New Session** (the `+` button next to a project) to start a fresh Claude Code session for that project. +- A **Terminal** row (the `+` button's Terminal entry) is a plain shell, not a Claude session. Clicking it after its shell has exited gives that same row a new shell — one row, the one you clicked, not a second one beside it. ## Panel shell @@ -14,6 +15,10 @@ under whatever the panel is showing — the [Changes](changes-view.md) list, a file, or a diff. Both stay visible; drag the horizontal handle between them to give one more room. The height is remembered. +With nothing open above it, the shell takes the whole panel and there is no +handle to drag. Open a list or a file again and the shell goes back to the +height you last dragged it to. + The shell starts in the session's own working directory, worktree included — the same directory the Changes panel reads and a `claude --resume` targets. You never pick the path, and it cannot drift from the one the session really runs diff --git a/eslint.config.js b/eslint.config.js index 45641c17..44f14174 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -239,6 +239,7 @@ const rendererCrossFileGlobals = { initPanelTerminal: 'readonly', syncPanelTerminal: 'readonly', panelTerminalIsOpen: 'readonly', + setPanelTerminalShellOnly: 'readonly', destroyPanelTerminalFor: 'readonly', isPanelTerminalSession: 'readonly', notePanelTerminalExit: 'readonly', diff --git a/git-changes-runner.js b/git-changes-runner.js index 4d9e7c6b..1474e6d2 100644 --- a/git-changes-runner.js +++ b/git-changes-runner.js @@ -16,6 +16,14 @@ const LOCAL_MAX_BUFFER = 20 * 1024 * 1024; const STATUS_MAX_STDOUT_BYTES = 2 * 1024 * 1024; const DIFF_STDOUT_SLACK_BYTES = 64 * 1024; const DIFF_MAX_STDOUT_BYTES = MAX_DIFF_BYTES + DIFF_STDOUT_SLACK_BYTES; +// An unexpected git failure is reported, bounded — see .ai/contexts/changes-view.md ("Bounded error messages") +const MAX_ERROR_LINES = 5; +const MAX_ERROR_CHARS = 500; +// git's generic fatal exit code, not a "no repository" code — see .ai/contexts/changes-view.md ("Not a repository") +const GIT_FATAL_EXIT_CODE = 128; +// defaultLocalExec's code for a spawn that never ran — see .ai/contexts/changes-view.md ("Not a repository") +const EXEC_FAILED_CODE = -1; +const NOT_A_REPO_REASON = 'not-a-repo'; // Denylist, not allowlist — see .ai/contexts/changes-view.md ("Quoting rule") function isSafeShellArg(s) { @@ -101,6 +109,42 @@ function resolveLocalNoIndexOperand(cwd, filePath, fsOps = DEFAULT_FS_OPS, pathO } } +// true/false/null (undecidable) — see .ai/contexts/changes-view.md ("Not a repository") +function gitEntryAtOrAbove(startDir, fsOps = DEFAULT_FS_OPS, pathOps = path) { + let dir; + try { + dir = pathOps.resolve(startDir); + } catch { + return null; + } + for (;;) { + try { + fsOps.lstat(pathOps.join(dir, '.git')); + return true; + } catch (err) { + const code = err && err.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') return null; + } + const parent = pathOps.dirname(dir); + if (parent === dir) return false; + dir = parent; + } +} + +// A local spawn fails on the cwd long before it fails on git — see .ai/contexts/changes-view.md ("Not a repository") +function missingCwdError(cwd, fsOps = DEFAULT_FS_OPS) { + if (!fsOps || typeof fsOps.stat !== 'function') { + throw new TypeError('missingCwdError requires fsOps.stat'); + } + try { + return fsOps.stat(cwd).isDirectory() ? null : `working directory is not a directory: ${cwd}`; + } catch (err) { + const code = err && err.code; + if (code === 'ENOENT' || code === 'ENOTDIR') return `working directory no longer exists: ${cwd}`; + return null; + } +} + // --literal-pathspecs on every invocation — see .ai/contexts/changes-view.md ("Quoting rule"). function buildGitArgs(args) { return ['--literal-pathspecs', ...args]; @@ -153,8 +197,22 @@ function defaultLocalExec(args, { cwd, timeoutMs }) { }); } +// Bounds what git wrote — see .ai/contexts/changes-view.md ("Bounded error messages") +function boundErrorMessage(text) { + const trimmed = String(text || '').trim(); + if (!trimmed) return ''; + const lines = trimmed.split('\n'); + let out = lines.slice(0, MAX_ERROR_LINES).join('\n'); + let dropped = lines.length > MAX_ERROR_LINES; + if (out.length > MAX_ERROR_CHARS) { + out = out.slice(0, MAX_ERROR_CHARS).trimEnd(); + dropped = true; + } + return dropped ? out + '…' : out; +} + function firstError(result) { - return (result.stderr || '').trim() || `git exited with code ${result.code}`; + return boundErrorMessage(result.stderr) || `git exited with code ${result.code}`; } // The two stdout-cap overruns: the remote transport's own, and execFile's maxBuffer — see .ai/contexts/changes-view.md ("Untracked files") @@ -190,6 +248,39 @@ function createGitChangesRunner({ kind, cwd, alias, exec, timeoutMs, fsOps } = { return kind === 'local' ? runExec(fullArgs) : runExec(buildRemoteGitCommand(cwd, fullArgs), remoteOpts); } + function cwdRefusal() { + return kind === 'local' ? missingCwdError(cwd, fsOps || DEFAULT_FS_OPS) : null; + } + + // Only positive evidence withdraws the panel — see .ai/contexts/changes-view.md ("Not a repository") + async function isWorkTree() { + let probe; + try { + probe = await invoke(['rev-parse', '--is-inside-work-tree']); + } catch (err) { + return { ok: false, error: cwdRefusal() || err.message }; + } + if (probe.code === 0) { + const answer = String(probe.stdout || '').trim(); + if (answer === 'true' || answer === 'false') return { ok: true, isRepo: answer === 'true' }; + return { ok: false, error: 'git rev-parse gave no answer' }; + } + if (probe.code === EXEC_FAILED_CODE) return { ok: false, error: cwdRefusal() || firstError(probe) }; + if (probe.code !== GIT_FATAL_EXIT_CODE) return { ok: false, error: firstError(probe) }; + const gone = cwdRefusal(); + if (gone) return { ok: false, error: gone }; + const corroborated = kind === 'local' + ? gitEntryAtOrAbove(cwd, fsOps || DEFAULT_FS_OPS) + : null; + if (corroborated === false) return { ok: true, isRepo: false }; + return { ok: false, error: firstError(probe) }; + } + + async function cwdHasNoWorkTree() { + const probe = await isWorkTree(); + return probe.ok === true && probe.isRepo === false; + } + async function status() { let results; try { @@ -203,6 +294,10 @@ function createGitChangesRunner({ kind, cwd, alias, exec, timeoutMs, fsOps } = { } let [st] = results; const [, unstagedNum, stagedNum] = results; + const failed = results.filter((r) => r.code !== 0); + if (failed.length > 0 && !failed.every(isStdoutCapFailure) && await cwdHasNoWorkTree()) { + return { ok: false, reason: NOT_A_REPO_REASON, error: 'not a git repository' }; + } if (unstagedNum.code !== 0) return { ok: false, error: firstError(unstagedNum) }; if (stagedNum.code !== 0) return { ok: false, error: firstError(stagedNum) }; @@ -287,7 +382,7 @@ function createGitChangesRunner({ kind, cwd, alias, exec, timeoutMs, fsOps } = { return { ok: true, content, truncated }; } - return { status, diff, kind, cwd, alias: alias || null }; + return { status, diff, isWorkTree, kind, cwd, alias: alias || null }; } module.exports = { @@ -296,12 +391,18 @@ module.exports = { buildGitArgs, localGitEnv, truncateDiffContent, + boundErrorMessage, shQuote, isSafeCwd, isSafeGitPath, isSafeNoIndexPath, resolveLocalNoIndexOperand, + gitEntryAtOrAbove, + missingCwdError, MAX_DIFF_BYTES, STATUS_MAX_STDOUT_BYTES, DIFF_MAX_STDOUT_BYTES, + MAX_ERROR_LINES, + MAX_ERROR_CHARS, + NOT_A_REPO_REASON, }; diff --git a/main.js b/main.js index f48d6a52..772da514 100644 --- a/main.js +++ b/main.js @@ -1757,6 +1757,17 @@ function gitChangesRunnerFor(target) { : createGitChangesRunner({ kind: 'local', cwd: target.cwd }); } +// Decides whether the Changes affordance is offered at all — see .ai/contexts/changes-view.md ("Not a repository") +ipcMain.handle('git-changes-available', async (_event, sessionId) => { + const target = resolveGitChangesTarget(sessionId); + if (!target.ok) return target; + try { + return await gitChangesRunnerFor(target).isWorkTree(); + } catch (err) { + return { ok: false, error: err.message }; + } +}); + ipcMain.handle('git-changes-status', async (_event, sessionId) => { const target = resolveGitChangesTarget(sessionId); if (!target.ok) return target; diff --git a/preload.js b/preload.js index 0c16d842..1ff0d97a 100644 --- a/preload.js +++ b/preload.js @@ -34,6 +34,7 @@ contextBridge.exposeInMainWorld('api', { startSubagentWatch: (parentSessionId, agentId) => ipcRenderer.invoke('start-subagent-watch', parentSessionId, agentId), stopSubagentWatch: (watchId) => ipcRenderer.invoke('stop-subagent-watch', watchId), // see .ai/contexts/changes-view.md + gitChangesAvailable: (sessionId) => ipcRenderer.invoke('git-changes-available', sessionId), gitChangesStatus: (sessionId) => ipcRenderer.invoke('git-changes-status', sessionId), gitChangesDiff: (sessionId, filePath, staged, untracked) => ipcRenderer.invoke('git-changes-diff', sessionId, filePath, staged, untracked), gitChangesFile: (sessionId, filePath, opts) => ipcRenderer.invoke('git-changes-file', sessionId, filePath, opts), diff --git a/public/app.js b/public/app.js index bbbc6e5a..06f29cd7 100644 --- a/public/app.js +++ b/public/app.js @@ -1095,10 +1095,6 @@ async function openSession(session, customOptions) { const entry = openSessions.get(sessionId); if (entry.closed) { destroySession(sessionId); - if (session.type === 'terminal') { - launchTerminalSession({ projectPath: session.projectPath }); - return; - } } else { showSession(sessionId); return; @@ -1108,8 +1104,9 @@ async function openSession(session, customOptions) { // Create new terminal entry (hidden until showSession) const entry = createTerminalEntry(session); - // Open terminal in main process - const resumeOptions = customOptions || await resolveDefaultSessionOptions({ projectPath }); + // Open terminal in main process — see .ai/contexts/session-state.md ("Reopening a plain terminal") + const resumeOptions = customOptions + || (session.type === 'terminal' ? { type: 'terminal' } : await resolveDefaultSessionOptions({ projectPath })); const result = await window.api.openTerminal(sessionId, projectPath, false, resumeOptions, entry.initialSize); if (!result.ok) { entry.terminal.write(`\r\nError: ${result.error}\r\n`); diff --git a/public/file-panel.js b/public/file-panel.js index e1a8e346..b14b4707 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -57,6 +57,12 @@ const MIN_CHANGES_LIST_HEIGHT = 96; const MIN_CHANGES_EDITOR_HEIGHT = 120; let changesListDesiredHeight = readStoredChangesListHeight(); +// No work tree, no Changes affordance — see .ai/contexts/changes-view.md ("Not a repository") +const NOT_A_REPO_REASON = 'not-a-repo'; +// see .ai/contexts/changes-view.md ("Who asks, and when") +const CHANGES_UNANSWERED = 'unanswered'; +const changesAvailabilityInFlight = new Set(); + const PANEL_WIDTH_KEY = 'filePanelWidth'; const DEFAULT_PANEL_WIDTH = parseInt(localStorage.getItem(PANEL_WIDTH_KEY), 10) || 450; const MIN_PANEL_WIDTH = 280; @@ -350,6 +356,7 @@ function getSessionState(sessionId) { panelVisible: false, panelWidth: DEFAULT_PANEL_WIDTH, mcpActive: false, + changesAvailable: null, }); } return filePanelState.get(sessionId); @@ -550,6 +557,7 @@ function hidePanel() { function switchPanel(sessionId) { currentPanelSessionId = sessionId; updateMcpIndicator(); + refreshChangesAvailability(sessionId); if (typeof syncPanelTerminal === 'function') syncPanelTerminal(sessionId); if (!sessionId) { @@ -591,6 +599,8 @@ function renderPanel(sessionId) { function renderTabContent(sessionId, tab) { const vpContainer = document.getElementById('file-panel-viewer'); const diffContainer = document.getElementById('file-panel-diff'); + // see .ai/contexts/panel-terminal.md ("Layout") + if (typeof setPanelTerminalShellOnly === 'function') setPanelTerminalShellOnly(!tab); if (!tab) { vpContainer.style.display = 'none'; @@ -704,6 +714,54 @@ function handleDiffAction(sessionId, tab, action) { // ── Changes Mode — see .ai/contexts/changes-view.md ────────────────── +// see .ai/contexts/changes-view.md ("Not a repository") +function updateChangesToggle() { + if (!changesToggleBtn) return; + const state = currentPanelSessionId ? filePanelState.get(currentPanelSessionId) : null; + changesToggleBtn.style.display = state && state.changesAvailable === false ? 'none' : ''; +} + +// see .ai/contexts/changes-view.md ("Who asks, and when") +async function refreshChangesAvailability(sessionId) { + updateChangesToggle(); + if (!sessionId || typeof window.api?.gitChangesAvailable !== 'function') return; + const memo = getSessionState(sessionId).changesAvailable; + if (memo === true || memo === CHANGES_UNANSWERED) return; + if (changesAvailabilityInFlight.has(sessionId)) return; + + changesAvailabilityInFlight.add(sessionId); + let result; + try { + result = await window.api.gitChangesAvailable(sessionId); + } finally { + changesAvailabilityInFlight.delete(sessionId); + } + + if (!result || typeof result.isRepo !== 'boolean') { + const state = getSessionState(sessionId); + if (state.changesAvailable === null) state.changesAvailable = CHANGES_UNANSWERED; + if (currentPanelSessionId === sessionId) updateChangesToggle(); + return; + } + if (result.isRepo) { + getSessionState(sessionId).changesAvailable = true; + if (currentPanelSessionId === sessionId) updateChangesToggle(); + return; + } + noteChangesUnavailable(sessionId); +} + +// see .ai/contexts/changes-view.md ("Not a repository") +function noteChangesUnavailable(sessionId) { + const state = getSessionState(sessionId); + if (state.currentTab && state.currentTab.type === 'changes') { + toggleChangesTab(sessionId); + if (state.currentTab) return; + } + state.changesAvailable = false; + if (currentPanelSessionId === sessionId) updateChangesToggle(); +} + function toggleChangesTab(sessionId) { const state = getSessionState(sessionId); if (state.currentTab && state.currentTab.type === 'changes') { @@ -774,7 +832,12 @@ async function refreshChanges(sessionId) { if (!stillState || stillState.currentTab !== tab) return; tab.loading = false; - if (!result || result.ok === false) { + if (result && result.reason === NOT_A_REPO_REASON) { + noteChangesUnavailable(sessionId); + if (!stillState.currentTab) return; + tab.error = result.error || 'not a git repository'; + tab.data = null; + } else if (!result || result.ok === false) { tab.error = (result && result.error) || 'failed to load changes'; tab.data = null; } else { diff --git a/public/panel-terminal.js b/public/panel-terminal.js index a919acf4..9921566e 100644 --- a/public/panel-terminal.js +++ b/public/panel-terminal.js @@ -7,6 +7,7 @@ const panelSpawnsInFlight = new Set(); // ownerSessionIds whose openTerminal has const panelReopenAfterSpawn = new Set(); // owners whose Shell was clicked again mid-close const PANEL_TERMINAL_ID_PREFIX = 'panel:'; +const PANEL_SHELL_ONLY_CLASS = 'shell-only'; const PANEL_TERMINAL_HEIGHT_KEY = 'panelTerminalHeight'; const DEFAULT_PANEL_TERMINAL_HEIGHT = 220; const MIN_PANEL_TERMINAL_HEIGHT = 80; @@ -154,6 +155,15 @@ function setupPanelTerminalSplitter() { }); } +// see .ai/contexts/panel-terminal.md ("Layout") +function setPanelTerminalShellOnly(shellOnly) { + if (!panelTerminalContentEl) return; + const want = !!shellOnly; + if (panelTerminalContentEl.classList.contains(PANEL_SHELL_ONLY_CLASS) === want) return; + panelTerminalContentEl.classList.toggle(PANEL_SHELL_ONLY_CLASS, want); + refitPanelTerminal(); +} + function refitPanelTerminal() { const state = panelTerminals.get(panelTerminalOwnerId); if (!state) return; diff --git a/public/style.css b/public/style.css index 9b137a5e..128c1599 100644 --- a/public/style.css +++ b/public/style.css @@ -4411,6 +4411,16 @@ body { display: flex; flex-direction: column; } display: block; } +/* No tab above it: the region is the panel — see .ai/contexts/panel-terminal.md ("Layout") */ +#file-panel-content.shell-only #panel-terminal-handle.open { + display: none; +} + +#file-panel-content.shell-only #panel-terminal-region.open { + flex: 1 1 0; + min-height: 0; +} + .terminal-container.panel-terminal { inset: 0; padding: 6px 0 6px 8px; diff --git a/session-cache.js b/session-cache.js index e4fbbae9..57ab8e0f 100644 --- a/session-cache.js +++ b/session-cache.js @@ -6,6 +6,7 @@ const { deriveProjectPath } = require('./derive-project-path'); const { readSessionFile, readSessionDisplayHeader, enumerateSessionFiles, resolveJsonlPath, mergeBridgeGroups } = require('./read-session-file'); const { encodeProjectPath, decodeProjectFolderBestEffort } = require('./encode-project-path'); const { parseFolderKey, joinFolderKey } = require('./remote-hosts'); +const { isPanelShellSession } = require('./panel-terminal-target'); /** * Session cache module. @@ -539,6 +540,8 @@ function buildProjectsFromCache(showArchived) { // Inject active plain terminal sessions so they participate in sorting for (const [sessionId, session] of activeSessions) { if (session.exited || !session.isPlainTerminal) continue; + // see .ai/contexts/panel-terminal.md ("A panel shell is not a session") + if (isPanelShellSession(session)) continue; if (!session.projectPath) continue; if (isProjectHidden(hiddenProjects, null, session.projectPath)) continue; const localKey = groupKey(null, session.projectPath); diff --git a/test/build-projects-panel-shell.test.js b/test/build-projects-panel-shell.test.js new file mode 100644 index 00000000..059d2fd4 --- /dev/null +++ b/test/build-projects-panel-shell.test.js @@ -0,0 +1,154 @@ +// buildProjectsFromCache injects live plain-terminal PTYs as sidebar rows so +// they participate in sorting. A panel shell is a plain terminal, so without an +// explicit exclusion it is injected too and reaches the sidebar as a session of +// its own — see .ai/contexts/panel-terminal.md ("A panel shell is not a +// session"). +// +// The jsdom renderer harnesses cannot catch this: they never run +// buildProjectsFromCache, and the row the sidebar draws comes from this +// payload, not from the renderer's own session object. + +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const sessionCache = require('../session-cache'); + +function initCache(projectsDir, activeSessions) { + sessionCache.init({ + PROJECTS_DIR: projectsDir, + activeSessions, + getMainWindow: () => null, + log: { info: () => {}, debug: () => {}, warn: () => {}, error: () => {} }, + db: { + isInitialScanComplete: () => true, + setInitialScanComplete: () => {}, + deleteCachedFolder: () => {}, getCachedByFolder: () => [], + upsertCachedSessions: () => {}, touchCachedModified: () => {}, + deleteCachedSession: () => {}, replaceSessionMetrics: () => {}, + deleteSearchFolder: () => {}, deleteSearchSession: () => {}, + upsertSearchEntries: () => {}, + setFolderMeta: () => {}, getFolderMeta: () => null, + getAllFolderMeta: () => new Map(), + getAllMeta: () => new Map(), + getAllCached: () => [], + getSetting: () => ({}), + getMeta: () => null, + setName: () => {}, + }, + }); +} + +const OWNER_ID = 'a31c9ebc-916c-4da5-abcc-42bc60d4dafd'; +const PANEL_ID = 'panel:' + OWNER_ID; +const PROJECT = '/tmp/switchboard-bpps/tools/switchboard'; + +function terminalSession(extra = {}) { + return { + exited: false, + isPlainTerminal: true, + projectPath: PROJECT, + _openedAt: Date.now(), + ...extra, + }; +} + +function allSessions(projects) { + return projects.flatMap((p) => p.sessions); +} + +function withCache(activeSessions, fn) { + const projectsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'switchboard-bpps-')); + try { + initCache(projectsDir, activeSessions); + return fn(); + } finally { + fs.rmSync(projectsDir, { recursive: true, force: true }); + } +} + +test('a panel shell is not injected as a session row (mutation target: dropping the isPanelShellSession filter)', () => { + const active = new Map([ + [OWNER_ID, terminalSession()], + [PANEL_ID, terminalSession({ panelFor: OWNER_ID })], + ]); + + const rows = withCache(active, () => allSessions(sessionCache.buildProjectsFromCache(false))); + + assert.deepEqual(rows.map((s) => s.sessionId), [OWNER_ID], + 'the shell the panel owns has no sidebar row of its own'); + assert.equal(rows.length, 1, + 'a second row here also inflates the status bar’s session count, which reads this payload'); +}); + +test('an ordinary terminal is still injected — the filter must not swallow the row it exists to add', () => { + const active = new Map([[OWNER_ID, terminalSession()]]); + + const rows = withCache(active, () => allSessions(sessionCache.buildProjectsFromCache(false))); + + assert.equal(rows.length, 1); + assert.equal(rows[0].sessionId, OWNER_ID); + assert.equal(rows[0].type, 'terminal'); + assert.equal(rows[0].projectPath, PROJECT); +}); + +test('a panel shell alone leaves the project it runs in out of the sidebar entirely', () => { + const active = new Map([[PANEL_ID, terminalSession({ panelFor: OWNER_ID })]]); + + const projects = withCache(active, () => sessionCache.buildProjectsFromCache(false)); + + assert.deepEqual(allSessions(projects).map((s) => s.sessionId), []); + assert.equal(projects.some((p) => p.projectPath === PROJECT), false, + 'a shell must not conjure a project group either — the injection creates one when missing'); +}); + +test('an exited panel shell is excluded by the exited check, whichever runs first', () => { + const active = new Map([[PANEL_ID, terminalSession({ panelFor: OWNER_ID, exited: true })]]); + + const rows = withCache(active, () => allSessions(sessionCache.buildProjectsFromCache(false))); + assert.deepEqual(rows, []); +}); + +// The predicate is shared with get-active-terminals rather than re-tested as a +// string prefix: keying on the id spelling here would pass while the real +// session object carries panelFor, and diverge the moment the id shape changes. +test('the exclusion keys on the session object, not on the "panel:" id spelling', () => { + const oddId = 'shell-for-' + OWNER_ID; + const active = new Map([[oddId, terminalSession({ panelFor: OWNER_ID })]]); + + const rows = withCache(active, () => allSessions(sessionCache.buildProjectsFromCache(false))); + assert.deepEqual(rows, [], + 'panelFor is what makes a PTY a panel shell — see panel-terminal-target.js'); +}); + +// --- The status bar reads this same payload --------------------------------- +// "N running" and "N sessions" come from two different sources, and only one of +// them was wrong. Pinning both here keeps the pair from drifting apart again. + +const APP_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); + +function renderDefaultStatusBody() { + const start = APP_SRC.indexOf('function renderDefaultStatus()'); + assert.notEqual(start, -1); + return APP_SRC.slice(start, APP_SRC.indexOf('\n}', start)); +} + +test('public/app.js: the session total counts the get-projects payload, which is what the injected row inflates', () => { + assert.match(renderDefaultStatusBody(), /cachedAllProjects\.reduce\(/, + 'totalSessions is summed over the cached projects, so a phantom row there is a phantom session in the status bar'); +}); + +test('public/app.js: "N running" goes through runningSessionCount, not activePtyIds.size (mutation target: counting the raw set)', () => { + const body = renderDefaultStatusBody(); + assert.match(body, /const running = runningSessionCount\(\);/, + 'a panel shell is in activePtyIds by design — the LRU needs it there — so the count must filter'); + assert.doesNotMatch(body, /activePtyIds\.size/); + + const start = APP_SRC.indexOf('function runningSessionCount()'); + assert.notEqual(start, -1); + assert.match(APP_SRC.slice(start, APP_SRC.indexOf('\n}', start)), /countSessionsWithoutPanelShells\(activePtyIds\)/); +}); diff --git a/test/dom-file-panel-changes.test.js b/test/dom-file-panel-changes.test.js index 5823c2f6..63ed9634 100644 --- a/test/dom-file-panel-changes.test.js +++ b/test/dom-file-panel-changes.test.js @@ -81,11 +81,11 @@ function makeEditorStub(window, mode, doc, created, onChange) { return view; } -function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmImpl, locateImpl } = {}) { +function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmImpl, locateImpl, availableImpl } = {}) { const dom = new JSDOM(INDEX_HTML, { url: 'http://localhost/', runScripts: 'outside-only', pretendToBeVisual: true }); const { window } = dom; - const calls = { status: [], diff: [], file: [], save: [], watch: [], unwatch: [], confirm: [], locate: [], readFile: [] }; + const calls = { status: [], diff: [], file: [], save: [], watch: [], unwatch: [], confirm: [], locate: [], readFile: [], available: [] }; const editors = []; const fileChangedListeners = []; @@ -95,6 +95,10 @@ function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmIm onMcpCloseAllDiffs: () => {}, onMcpCloseTab: () => {}, mcpDiffResponse: () => {}, + gitChangesAvailable: (sessionId) => { + calls.available.push(sessionId); + return Promise.resolve((availableImpl || (() => ({ ok: true, isRepo: true })))(sessionId)); + }, gitChangesStatus: (sessionId) => { calls.status.push(sessionId); return Promise.resolve((statusImpl || (() => makeStatusResult()))(sessionId)); @@ -1909,3 +1913,598 @@ test('Save follows the buffer in every mode, plain included (mutation target: th } } finally { ctx.destroy(); } }); + +// --- No git work tree — see .ai/contexts/changes-view.md ("Not a repository") --- + +test('a cwd with no work tree withdraws the Changes button instead of offering a tab that cannot fill', async () => { + const ctx = setupFilePanelDom({ availableImpl: () => ({ ok: true, isRepo: false }) }); + try { + ctx.window.switchPanel('s1'); + await flush(); + + const btn = ctx.document.getElementById('changes-toggle-btn'); + assert.equal(btn.style.display, 'none', 'no work tree, no Changes affordance'); + assert.equal(ctx.calls.status.length, 0, 'the availability answer costs no status call'); + } finally { ctx.destroy(); } +}); + +test('the Changes button stays visible for a session that is in a work tree', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + await flush(); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + } finally { ctx.destroy(); } +}); + +test('the button follows the session the panel shows, not the last answer that arrived', async () => { + const ctx = setupFilePanelDom({ availableImpl: (id) => ({ ok: true, isRepo: id !== 'norepo' }) }); + try { + ctx.window.switchPanel('norepo'); + await flush(); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + + ctx.window.switchPanel('s1'); + await flush(); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'a session in a repo must get its button back'); + + ctx.window.switchPanel('norepo'); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'a known answer applies before the round trip, with no flash of a button that does not work'); + } finally { ctx.destroy(); } +}); + +test('a Changes tab already open when the cwd turns out to have no work tree is withdrawn, not left half-rendered', async () => { + let isRepo = true; + const ctx = setupFilePanelDom({ + availableImpl: () => ({ ok: true, isRepo }), + statusImpl: () => (isRepo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2); + + isRepo = false; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); + + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), false, + 'the tab closes rather than reporting into itself'); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + + const statusCalls = ctx.calls.status.length; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); + assert.equal(ctx.calls.status.length, statusCalls, 'the tab is gone, so nothing refreshes it any more'); + } finally { ctx.destroy(); } +}); + +test('git stderr never reaches the panel as the message when the cwd has no work tree', async () => { + const ctx = setupFilePanelDom({ + statusImpl: () => ({ ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + + assert.equal(ctx.document.querySelectorAll('.changes-error').length, 0, + 'a missing work tree is not an error to report, it is an affordance to withdraw'); + assert.doesNotMatch(ctx.document.body.textContent, /dépôt git|not a git repository/); + } finally { ctx.destroy(); } +}); + +test('a genuine git failure is still reported in the tab, and the button stays', async () => { + const ctx = setupFilePanelDom({ + statusImpl: () => ({ ok: false, error: 'could not read directory: Permission denied' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + + const err = ctx.document.querySelector('.changes-error'); + assert.ok(err, 'an unexpected failure must still be visible'); + assert.match(err.textContent, /Permission denied/); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'only a missing work tree withdraws the button — a transient failure must not'); + } finally { ctx.destroy(); } +}); + +// --- The availability probe is memoised, deduped and ignored when stale ------ +// For a remote session each probe is an ssh with a 20 s kill timer, and +// switchPanel is reached from every panel-shell open, close and exit as well as +// from every sidebar click — see .ai/contexts/changes-view.md ("Not a repository"). + +function deferredAvailable() { + const gates = []; + return { + gates, + impl: (sessionId) => new Promise((resolve) => gates.push({ sessionId, resolve })), + settle: (sessionId, value) => { + for (const g of gates.filter((x) => x.sessionId === sessionId)) g.resolve(value); + }, + settleAll: (value) => { for (const g of gates.splice(0)) g.resolve(value); }, + }; +} + +test('a session already known to be in a repository is never probed again', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + await flush(); + assert.equal(ctx.calls.available.length, 1); + + for (let i = 0; i < 5; i++) { + ctx.window.switchPanel('s1'); + await flush(); + } + assert.equal(ctx.calls.available.length, 1, + 'five re-entries into the same session must cost one probe, not five'); + } finally { ctx.destroy(); } +}); + +test('re-entering a session while its probe is still out does not start a second one', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + for (let i = 0; i < 10; i++) ctx.window.switchPanel('s1'); + await flush(); + assert.equal(ctx.calls.available.length, 1, + 'ten rapid switches must not put ten concurrent ssh children on a remote host'); + + d.settleAll({ ok: true, isRepo: true }); + await flush(); + } finally { ctx.destroy(); } +}); + +test('a session with no repository is re-asked, so a git init is picked up', async () => { + let isRepo = false; + const ctx = setupFilePanelDom({ availableImpl: () => ({ ok: true, isRepo }) }); + try { + ctx.window.switchPanel('s1'); + await flush(); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + + isRepo = true; + ctx.window.switchPanel('s2'); + await flush(); + ctx.window.switchPanel('s1'); + await flush(); + + assert.equal(ctx.calls.available.filter((id) => id === 's1').length, 2, + 'only the sessions that answered "no repository" pay a second probe'); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + } finally { ctx.destroy(); } +}); + +test('a probe that answers after the panel has moved on changes nothing', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + ctx.window.switchPanel('s1'); + await flush(); + ctx.window.switchPanel('s2'); + await flush(); + + d.settle('s1', { ok: true, isRepo: false }); + d.settle('s2', { ok: true, isRepo: true }); + await flush(); + + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 's1’s answer must not withdraw s2’s button'); + } finally { ctx.destroy(); } +}); + +// --- Withdrawal goes through the tab's own close control -------------------- + +test('withdrawing the tab goes through toggleChangesTab, not a teardown of its own', async () => { + let isRepo = true; + const ctx = setupFilePanelDom({ + availableImpl: () => ({ ok: true, isRepo }), + statusImpl: () => (isRepo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + + const toggles = []; + const realToggle = ctx.window.toggleChangesTab; + ctx.window.toggleChangesTab = (id) => { toggles.push(id); return realToggle(id); }; + + isRepo = false; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); + + assert.deepEqual(toggles, ['s1'], + 'the one close path a future gate will guard must be the one the withdrawal uses'); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), false); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + } finally { ctx.destroy(); } +}); + +test('a close the tab refuses leaves the button, so the tab can still be reopened', async () => { + const ctx = setupFilePanelDom({ + statusImpl: () => ({ ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + // A gate that declines — what #302's confirmDiscardChangesEdits does on "cancel". + ctx.window.toggleChangesTab = () => {}; + await flush(); + + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'hiding the control while its tab is still open strands whatever the tab is holding'); + } finally { ctx.destroy(); } +}); + +// --- A non-answer is memoised too — see .ai/contexts/changes-view.md ("Who asks, and when") --- +// A remote 128 is always {ok:false} by design, and a {ok:false} can never change +// the button, so re-asking costs an ssh with a 20 s kill timer to learn nothing. + +function countRevisits(ctx, sessionId, times) { + const before = ctx.calls.available.filter((id) => id === sessionId).length; + const run = async () => { + for (let i = 0; i < times; i++) { + ctx.window.switchPanel('other'); + await flush(); + ctx.window.switchPanel(sessionId); + await flush(); + } + return ctx.calls.available.filter((id) => id === sessionId).length - before; + }; + return run(); +} + +test('a session git could not answer for is asked once, not on every activation', async () => { + const ctx = setupFilePanelDom({ + availableImpl: (id) => (id === 'remote' ? { ok: false, error: 'fatal: …' } : { ok: true, isRepo: true }), + }); + try { + ctx.window.switchPanel('remote'); + await flush(); + assert.equal(ctx.calls.available.filter((id) => id === 'remote').length, 1); + + assert.equal(await countRevisits(ctx, 'remote', 6), 0, + 'six revisits must add no probes — the answer cannot change the button, so asking again buys nothing'); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'and the button stays, because nothing established that there is no repository'); + } finally { ctx.destroy(); } +}); + +test('a session in a repository is still asked once and never again', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + await flush(); + assert.equal(await countRevisits(ctx, 's1', 6), 0); + } finally { ctx.destroy(); } +}); + +test('a session with no repository is still re-asked, so the two memos do not collapse into one', async () => { + const ctx = setupFilePanelDom({ + availableImpl: (id) => (id === 'norepo' ? { ok: true, isRepo: false } : { ok: true, isRepo: true }), + }); + try { + ctx.window.switchPanel('norepo'); + await flush(); + assert.equal(await countRevisits(ctx, 'norepo', 3), 3, + 'only the answer that hides the button is worth re-checking'); + } finally { ctx.destroy(); } +}); + +// --- The stale-reply guard covers the DOM, not the memo --------------------- + +test('an answer for a session the panel has left is still recorded against that session', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + ctx.window.switchPanel('s1'); + await flush(); + ctx.window.switchPanel('s2'); + await flush(); + + d.settle('s1', { ok: true, isRepo: true }); + d.settle('s2', { ok: true, isRepo: true }); + await flush(); + + const before = ctx.calls.available.filter((id) => id === 's1').length; + ctx.window.switchPanel('s1'); + await flush(); + assert.equal(ctx.calls.available.filter((id) => id === 's1').length, before, + 'a correct answer must not be thrown away just because the panel had moved on'); + } finally { ctx.destroy(); } +}); + +test('an answer for a session the panel has left never touches the current button', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + ctx.window.switchPanel('s1'); + await flush(); + ctx.window.switchPanel('s2'); + await flush(); + + d.settle('s1', { ok: false, error: 'fatal: …' }); + d.settle('s2', { ok: true, isRepo: true }); + await flush(); + + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + + // ...and the memo it wrote is s1's, proven by s1 not being probed again. + const before = ctx.calls.available.filter((id) => id === 's1').length; + ctx.window.switchPanel('s1'); + await flush(); + assert.equal(ctx.calls.available.filter((id) => id === 's1').length, before); + } finally { ctx.destroy(); } +}); + +// --- A refused withdrawal must not freeze the tab --------------------------- + +test('a close the tab refuses leaves a readable tab, not a permanent Loading', async () => { + const ctx = setupFilePanelDom({ + statusImpl: () => ({ ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + ctx.window.toggleChangesTab = () => {}; // a gate that declines + await flush(); + + const summary = ctx.document.getElementById('changes-summary'); + assert.doesNotMatch(summary.textContent, /Loading/, + 'the tab is staying open, so it has to say something other than the render it was stuck on'); + const err = ctx.document.querySelector('.changes-error'); + assert.ok(err, 'a tab that could not be withdrawn must explain itself'); + } finally { ctx.destroy(); } +}); + +// Counts writes to the button's display, which is the only trace a redundant +// repaint leaves: the value written is always the current session's. +function countDisplayWrites(ctx) { + const btn = ctx.document.getElementById('changes-toggle-btn'); + const style = btn.style; + let writes = 0; + const proto = Object.getPrototypeOf(style); + const descriptor = Object.getOwnPropertyDescriptor(proto, 'display'); + Object.defineProperty(style, 'display', { + configurable: true, + get() { return descriptor.get.call(style); }, + set(v) { writes++; descriptor.set.call(style, v); }, + }); + return { count: () => writes, reset: () => { writes = 0; } }; +} + +test('a reply for a session the panel has left repaints nothing (mutation target: dropping the guard around updateChangesToggle)', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + ctx.window.switchPanel('s1'); + await flush(); + ctx.window.switchPanel('s2'); + await flush(); + + const writes = countDisplayWrites(ctx); + writes.reset(); + + d.settle('s1', { ok: true, isRepo: true }); + await flush(); + assert.equal(writes.count(), 0, + 's1 is not the session on screen, so its answer has no button to paint'); + + d.settle('s2', { ok: true, isRepo: true }); + await flush(); + assert.equal(writes.count(), 1, 's2 is, so its answer does'); + } finally { ctx.destroy(); } +}); + +// --- A non-answer never displaces an answer --------------------------------- +// "no repository" is the one answer deliberately re-asked, so it is also the +// one that a transient failure can land on — an ssh blip, a sleeping host. See +// .ai/contexts/changes-view.md ("Who asks, and when"). + +test('a transient failure on the re-ask does not un-hide a button that was correctly hidden', async () => { + let answer = { ok: true, isRepo: false }; + const ctx = setupFilePanelDom({ availableImpl: (id) => (id === 'scratch' ? answer : { ok: true, isRepo: true }) }); + try { + const btn = ctx.document.getElementById('changes-toggle-btn'); + ctx.window.switchPanel('scratch'); + await flush(); + assert.equal(btn.style.display, 'none', 'a scratch directory is not a repository'); + + answer = { ok: false, error: 'ssh: connect to host h port 22: Connection refused' }; + ctx.window.switchPanel('other'); + await flush(); + ctx.window.switchPanel('scratch'); + await flush(); + + assert.equal(btn.style.display, 'none', + 'a probe that could not answer must not overwrite the answer that was already established'); + assert.equal(ctx.calls.available.filter((id) => id === 'scratch').length, 2); + + // And the session is still re-askable, so the blip costs nothing permanent. + answer = { ok: true, isRepo: true }; + ctx.window.switchPanel('other'); + await flush(); + ctx.window.switchPanel('scratch'); + await flush(); + assert.notEqual(btn.style.display, 'none', 'once git can answer again, the answer applies'); + } finally { ctx.destroy(); } +}); + +test('a session that has only ever been unanswerable is still asked exactly once', async () => { + const ctx = setupFilePanelDom({ + availableImpl: (id) => (id === 'remote' ? { ok: false, error: 'fatal: …' } : { ok: true, isRepo: true }), + }); + try { + ctx.window.switchPanel('remote'); + await flush(); + assert.equal(await countRevisits(ctx, 'remote', 4), 0, + 'the memo must still stop the forever-probe it was added for'); + } finally { ctx.destroy(); } +}); + +test('a transient failure before any answer is memoised, and a later one after an answer is not', async () => { + let answer = { ok: false, error: 'fatal: …' }; + const ctx = setupFilePanelDom({ availableImpl: (id) => (id === 't' ? answer : { ok: true, isRepo: true }) }); + try { + ctx.window.switchPanel('t'); + await flush(); + assert.equal(ctx.calls.available.filter((id) => id === 't').length, 1); + + // Nothing was established, so the non-answer sticks and stops the asking. + answer = { ok: true, isRepo: false }; + assert.equal(await countRevisits(ctx, 't', 3), 0); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + } finally { ctx.destroy(); } +}); + +test('a reply for a departed session repaints nothing on the unanswerable branch either', async () => { + const d = deferredAvailable(); + const ctx = setupFilePanelDom({ availableImpl: d.impl }); + try { + ctx.window.switchPanel('s1'); + await flush(); + ctx.window.switchPanel('s2'); + await flush(); + + const writes = countDisplayWrites(ctx); + writes.reset(); + + d.settle('s1', { ok: false, error: 'fatal: …' }); + await flush(); + assert.equal(writes.count(), 0, + 'the twin of the isRepo branch: a stale reply never touches the DOM, whichever way it failed'); + + d.settle('s2', { ok: false, error: 'fatal: …' }); + await flush(); + assert.equal(writes.count(), 1, 'the current session’s reply does'); + } finally { ctx.destroy(); } +}); + +// --- Withdrawal over a dirty editor buffer ---------------------------------- +// The one combination neither branch could have had a test for: the editor is +// #302's, the withdrawal is this branch's, and they meet at toggleChangesTab. +// See .ai/contexts/changes-view.md ("Withdrawal reuses the tab's own close +// control"). + +async function openDirtyFileThen(ctx, statusAfter) { + await openFile(ctx, 's1', 'src/a.js'); + ctx.editors[0].box.text = 'my unsaved edit\n'; + statusAfter(); + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); +} + +test('a withdrawal over unsaved edits asks first, and a refusal keeps both the editor and the button', async () => { + let repo = true; + const ctx = setupFilePanelDom({ + confirmImpl: () => false, + statusImpl: () => (repo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + await openDirtyFileThen(ctx, () => { repo = false; }); + + assert.equal(ctx.calls.confirm.length, 1, 'the withdrawal asks the same question every other exit asks'); + assert.equal(ctx.editors[0].box.destroyed, false, 'a refused discard must not destroy the buffer'); + assert.equal(ctx.editors[0].box.text, 'my unsaved edit\n'); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'hiding the control while its tab still holds the edit is what strands it'); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), true); + } finally { ctx.destroy(); } +}); + +test('a withdrawal over unsaved edits proceeds once the user confirms, and then withdraws the button', async () => { + let repo = true; + const ctx = setupFilePanelDom({ + confirmImpl: () => true, + statusImpl: () => (repo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + await openDirtyFileThen(ctx, () => { repo = false; }); + + assert.equal(ctx.calls.confirm.length, 1); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), false); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); + } finally { ctx.destroy(); } +}); + +test('a withdrawal with nothing unsaved never asks, it just withdraws', async () => { + let repo = true; + const ctx = setupFilePanelDom({ + confirmImpl: () => false, + statusImpl: () => (repo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + }); + try { + await openFile(ctx, 's1', 'src/a.js'); + repo = false; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); + + assert.equal(ctx.calls.confirm.length, 0, 'a clean buffer has nothing to discard'); + assert.equal(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'so the refusal path is never reached and the button goes'); + } finally { ctx.destroy(); } +}); + +// --- The panel's own X is a second close path ------------------------------- +// `handleClose` clears currentTab and hides the panel directly, without passing +// through toggleChangesTab — see .ai/contexts/changes-view.md ("Withdrawal +// reuses the tab's own close control"). + +test('the panel X closes a Changes tab on its own path, not through toggleChangesTab', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2); + + const toggles = []; + const realToggle = ctx.window.toggleChangesTab; + ctx.window.toggleChangesTab = (id) => { toggles.push(id); return realToggle(id); }; + + const closeBtn = ctx.document.querySelector('#file-panel-changes .fp-close-btn'); + assert.ok(closeBtn, 'the Changes toolbar carries its own close button'); + closeBtn.click(); + + assert.deepEqual(toggles, [], + 'this exit does not funnel through the toggle — a guard on one is not a guard on the other'); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), false); + + // The tab is gone, so nothing refreshes it any more. + const statusCalls = ctx.calls.status.length; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); + assert.equal(ctx.calls.status.length, statusCalls); + } finally { ctx.destroy(); } +}); + +test('the panel X leaves the Changes button, so the tab it closed can be reopened', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('s1'); + await flush(); + + ctx.document.querySelector('#file-panel-changes .fp-close-btn').click(); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'closing the view is not the same as the session having no repository'); + + ctx.document.getElementById('changes-toggle-btn').click(); + await flush(); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), true); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2); + } finally { ctx.destroy(); } +}); diff --git a/test/git-changes-runner-real-git.test.js b/test/git-changes-runner-real-git.test.js index a97cf85b..ea6d502a 100644 --- a/test/git-changes-runner-real-git.test.js +++ b/test/git-changes-runner-real-git.test.js @@ -24,7 +24,7 @@ const os = require('os'); const path = require('path'); const { execFileSync, spawnSync } = require('child_process'); -const { createGitChangesRunner, isSafeNoIndexPath } = require('../git-changes-runner'); +const { createGitChangesRunner, isSafeNoIndexPath, NOT_A_REPO_REASON, missingCwdError } = require('../git-changes-runner'); // git translates its diagnostics; the assertions below match its English text. // Set on this process so both the scratch-repo helper and the runner's own @@ -225,6 +225,9 @@ test('real git: a leaf symlink to a DIRECTORY leaks a file named "null" unless t const runner = createGitChangesRunner({ kind: 'local', cwd: repoDir }); const status = await runner.status(); + // Shape first: a status() that came back as an error would otherwise fail + // here as a TypeError naming nothing. + assert.ok(Array.isArray(status.files), `status() must return a file list, got ${JSON.stringify(status)}`); assert.ok(status.files.some((f) => f.path === 'dirlink'), 'git lists the symlink as a row of its own — this needs no crafted path, just a click'); // What raw git does with that row's own path, pinned. @@ -363,3 +366,242 @@ test('real git: a literal "~/..." pathspec is never shell-expanded (no shell is cleanup(tmp); } }); + +// --- Not a git work tree — see .ai/contexts/changes-view.md ("Not a repository") --- +// +// Real git, a real directory outside any repository. The suite pins LC_ALL=C +// for its message assertions; these cases run under a second locale on purpose, +// because the detection is an exit code and must not move when the message does. +// A host without fr_FR.UTF-8 installed falls back to English — the assertions +// hold either way, which is the point. + +const LOCALES = ['C', 'fr_FR.UTF-8']; +const ALT_LOCALE = 'fr_FR.UTF-8'; + +// Asserting that two locales agree proves nothing unless git actually speaks the +// second one. GitHub's runners ship no fr_FR.UTF-8, so without this probe the +// locale tests pass vacuously on every CI leg. +function gitSpeaks(locale) { + const message = (env) => { + const r = spawnSync('git', ['rev-parse', '--is-inside-work-tree'], { + cwd: os.tmpdir(), encoding: 'utf8', env: { ...scratchGitEnv(), ...env }, + }); + return String(r.stderr || ''); + }; + const english = message({ LC_ALL: 'C', LANGUAGE: 'C', LANG: 'C' }); + const other = message({ LC_ALL: locale, LANGUAGE: locale, LANG: locale }); + return !!english && !!other && english !== other; +} + +const ALT_LOCALE_AVAILABLE = gitSpeaks(ALT_LOCALE); +const SKIP_ALT_LOCALE = ALT_LOCALE_AVAILABLE ? false : `git does not translate its output under ${ALT_LOCALE} here`; + +// A temp directory with a repository somewhere above it would answer "true"; +// os.tmpdir() is not inside one on any supported platform, and the assertions +// below would fail loudly rather than silently if it ever were. +function withLocale(locale, fn) { + const saved = { LC_ALL: process.env.LC_ALL, LANGUAGE: process.env.LANGUAGE, LANG: process.env.LANG }; + process.env.LC_ALL = locale; + process.env.LANGUAGE = locale; + process.env.LANG = locale; + return Promise.resolve() + .then(fn) + .finally(() => { + for (const [k, v] of Object.entries(saved)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + }); +} + +test('real git: a directory outside any repository is reported as its own reason, in every locale', { skip: SKIP_ALT_LOCALE }, async () => { + const results = []; + for (const locale of LOCALES) { + const tmp = mkTmp(); + try { + await withLocale(locale, async () => { + const result = await createGitChangesRunner({ kind: 'local', cwd: tmp }).status(); + assert.equal(result.ok, false, `${locale}: no repository, no changes`); + assert.equal(result.reason, NOT_A_REPO_REASON, `${locale}: the outcome is machine-readable`); + assert.doesNotMatch(result.error, /fatal|dépôt|GIT_DISCOVERY|usage/, + `${locale}: git's own text must not become the panel's message`); + results.push(result); + }); + } finally { cleanup(tmp); } + } + assert.deepEqual(results[1], results[0], 'the two locales must produce byte-identical outcomes'); +}); + +test('real git: isWorkTree() answers by exit code, in every locale', { skip: SKIP_ALT_LOCALE }, async () => { + for (const locale of LOCALES) { + const tmp = mkTmp(); + try { + const repoDir = path.join(tmp, 'repo'); + initRepo(repoDir); + await withLocale(locale, async () => { + assert.deepEqual(await createGitChangesRunner({ kind: 'local', cwd: repoDir }).isWorkTree(), + { ok: true, isRepo: true }, `${locale}: a real repository`); + assert.deepEqual(await createGitChangesRunner({ kind: 'local', cwd: tmp }).isWorkTree(), + { ok: true, isRepo: false }, `${locale}: its parent, which is not one`); + }); + } finally { cleanup(tmp); } + } +}); + +test('real git: a subdirectory of a repository is still inside the work tree', async () => { + const tmp = mkTmp(); + try { + const repoDir = path.join(tmp, 'repo'); + initRepo(repoDir); + const sub = path.join(repoDir, 'nested', 'deeper'); + fs.mkdirSync(sub, { recursive: true }); + + assert.deepEqual(await createGitChangesRunner({ kind: 'local', cwd: sub }).isWorkTree(), { ok: true, isRepo: true }, + 'a session recorded in a subdirectory must keep its Changes panel'); + } finally { cleanup(tmp); } +}); + +// --- A repository git refuses is not a missing repository -------------------- +// Exit 128 is git's generic fatal code. Every fixture below is a REAL repository +// that real git refuses to open, and each one exits 128 exactly like a plain +// directory does — so the exit code alone cannot tell them apart, and the +// corroborating filesystem check is what does. See .ai/contexts/changes-view.md +// ("Not a repository"). + +function initRefusedRepo(dir, wreck) { + fs.mkdirSync(dir, { recursive: true }); + git(dir, ['init', '-q']); + git(dir, ['config', 'user.email', 'a@a.com']); + git(dir, ['config', 'user.name', 'a']); + wreck(dir); + return dir; +} + +const REFUSED_FIXTURES = [ + ['an unsupported core.repositoryformatversion', (d) => git(d, ['config', 'core.repositoryformatversion', '99'])], + ['a .git file pointing at a gitdir that is not there', (d) => { + fs.rmSync(path.join(d, '.git'), { recursive: true, force: true }); + fs.writeFileSync(path.join(d, '.git'), 'gitdir: /nonexistent/elsewhere\n'); + }], + ['a .git file that is not a gitdir line at all', (d) => { + fs.rmSync(path.join(d, '.git'), { recursive: true, force: true }); + fs.writeFileSync(path.join(d, '.git'), 'not a gitdir line\n'); + }], +]; + +for (const [label, wreck] of REFUSED_FIXTURES) { + test(`real git: ${label} is reported, not silently treated as "no repository"`, async () => { + const tmp = mkTmp(); + try { + const repoDir = initRefusedRepo(path.join(tmp, 'repo'), wreck); + + const raw = spawnSync('git', ['rev-parse', '--is-inside-work-tree'], { cwd: repoDir, encoding: 'utf8', env: scratchGitEnv() }); + assert.equal(raw.status, 128, 'the fixture must actually make git exit 128, or it proves nothing'); + + const runner = createGitChangesRunner({ kind: 'local', cwd: repoDir }); + const probe = await runner.isWorkTree(); + assert.equal(probe.ok, false, 'a repository git will not open is not an answer of "no repository"'); + assert.ok(probe.error, 'and the reason must survive to the caller'); + + const result = await runner.status(); + assert.equal(result.ok, false); + assert.equal(result.reason, undefined, + 'reason: not-a-repo withdraws the Changes button — a refused repository must never trigger it'); + } finally { cleanup(tmp); } + }); +} + +test('real git: a worktree whose main repository was deleted is reported, not withdrawn', async () => { + const tmp = mkTmp(); + try { + const mainRepo = path.join(tmp, 'main'); + initRepo(mainRepo); + git(mainRepo, ['add', '-A']); + git(mainRepo, ['commit', '-q', '-m', 'second']); + const wt = path.join(tmp, 'wt'); + git(mainRepo, ['worktree', 'add', '-q', wt, '-b', 'wt']); + fs.rmSync(mainRepo, { recursive: true, force: true }); + + const result = await createGitChangesRunner({ kind: 'local', cwd: wt }).status(); + assert.equal(result.ok, false); + assert.equal(result.reason, undefined, 'the .git file is still there, so the repository is not missing — it is broken'); + } finally { cleanup(tmp); } +}); + +test('real git: a plain directory is still the one case that withdraws the panel', async () => { + const tmp = mkTmp(); + try { + const plain = path.join(tmp, 'notes'); + fs.mkdirSync(plain, { recursive: true }); + + const probe = await createGitChangesRunner({ kind: 'local', cwd: plain }).isWorkTree(); + assert.deepEqual(probe, { ok: true, isRepo: false }); + + const result = await createGitChangesRunner({ kind: 'local', cwd: plain }).status(); + assert.equal(result.reason, NOT_A_REPO_REASON); + } finally { cleanup(tmp); } +}); + +test('real git: a subdirectory of a refused repository is reported too, not withdrawn', async () => { + const tmp = mkTmp(); + try { + const repoDir = initRefusedRepo(path.join(tmp, 'repo'), (d) => git(d, ['config', 'core.repositoryformatversion', '99'])); + const sub = path.join(repoDir, 'nested'); + fs.mkdirSync(sub, { recursive: true }); + + const result = await createGitChangesRunner({ kind: 'local', cwd: sub }).status(); + assert.equal(result.reason, undefined, 'the walk must climb to the repository root, not just look in the cwd'); + } finally { cleanup(tmp); } +}); + +// --- A working directory that is gone --------------------------------------- +// The walk answers "no .git anywhere" for a path that does not exist, so the +// cwd has to be ruled out before the corroboration is trusted — otherwise a +// deleted worktree outside a repository would withdraw the panel. See +// .ai/contexts/changes-view.md ("Not a repository"). + +const VANISHED_CWDS = [ + ['a directory that was deleted', (tmp) => { + const gone = path.join(tmp, 'gone'); + fs.mkdirSync(gone, { recursive: true }); + fs.rmSync(gone, { recursive: true, force: true }); + return gone; + }], + ['a path that is a file, not a directory', (tmp) => { + const file = path.join(tmp, 'notadir'); + fs.writeFileSync(file, 'x'); + return file; + }], + ['a symlink whose target is gone', (tmp) => { + const link = path.join(tmp, 'link'); + fs.symlinkSync(path.join(tmp, 'never-existed'), link); + return link; + }], +]; + +for (const [label, make] of VANISHED_CWDS) { + test(`real git: ${label} is reported as a missing directory, never as a missing repository`, async () => { + const tmp = mkTmp(); + try { + const cwd = make(tmp); + const runner = createGitChangesRunner({ kind: 'local', cwd }); + + const probe = await runner.isWorkTree(); + assert.equal(probe.ok, false, 'a directory that is not there cannot answer whether it is a repository'); + assert.doesNotMatch(probe.error, /spawn git/, + '"spawn git ENOENT" reads as "git is not installed"; the directory is what is missing'); + assert.match(probe.error, /working directory/); + assert.ok(probe.error.includes(cwd), 'and it must name the directory'); + + const result = await runner.status(); + assert.equal(result.reason, undefined, 'withdrawing the panel here would blame the repository for the cwd'); + } finally { cleanup(tmp); } + }); +} + +test('real git: missingCwdError says nothing about a directory that is simply there', () => { + const tmp = mkTmp(); + try { + assert.equal(missingCwdError(tmp), null); + } finally { cleanup(tmp); } +}); diff --git a/test/git-changes-runner.test.js b/test/git-changes-runner.test.js index 47cf07c8..d2ad7cee 100644 --- a/test/git-changes-runner.test.js +++ b/test/git-changes-runner.test.js @@ -13,14 +13,20 @@ const { buildRemoteGitCommand, buildGitArgs, truncateDiffContent, + boundErrorMessage, shQuote, isSafeCwd, isSafeGitPath, isSafeNoIndexPath, resolveLocalNoIndexOperand, + gitEntryAtOrAbove, + missingCwdError, MAX_DIFF_BYTES, STATUS_MAX_STDOUT_BYTES, DIFF_MAX_STDOUT_BYTES, + MAX_ERROR_LINES, + MAX_ERROR_CHARS, + NOT_A_REPO_REASON, } = require('../git-changes-runner'); // The guard resolves against the running platform's path rules, so a fixture @@ -895,3 +901,264 @@ test('remote runner: uses the real defaultRunRemoteCommand transport when no exe assert.equal(runner.kind, 'remote'); assert.equal(runner.alias, 'vps'); }); + +// --- Not a git work tree — see .ai/contexts/changes-view.md ("Not a repository") --- + +// What git actually wrote on this host, in French, when the Changes panel was +// pointed at a directory outside any repository. The detection must not depend +// on a single byte of it. +const FRENCH_FATAL = 'fatal: ni ceci ni aucun de ses répertoires parents (jusqu\'au point de montage /) n\'est un dépôt git\nArrêt à la limite du système de fichiers (GIT_DISCOVERY_ACROSS_FILESYSTEM n\'est pas défini).'; +const FRENCH_DIFF_USAGE = ['warning: Pas un dépôt git. Utilisez --no-index pour comparer deux chemins hors d\'un arbre de travail', 'usage : git diff --no-index [] [...]'] + .concat(Array.from({ length: 150 }, (_, i) => ` --some-option-${i} une description de l'option ${i}`)).join('\n'); + +// The fixture cwds are synthetic paths, so the fs seam describes them: a real +// directory with no .git at or above it. +const NO_REPO_FS = { + stat: () => ({ isDirectory: () => true }), + lstat: () => { const e = new Error('ENOENT'); e.code = 'ENOENT'; throw e; }, +}; + +// Every git command fails the way a non-repo cwd makes it fail, rev-parse included. +function nonRepoExec(calls) { + return (args) => { + calls.push(args); + if (args[1] === 'rev-parse') return Promise.resolve({ code: 128, stdout: '', stderr: FRENCH_FATAL }); + if (args[1] === 'status') return Promise.resolve({ code: 128, stdout: '', stderr: FRENCH_FATAL }); + return Promise.resolve({ code: 129, stdout: '', stderr: FRENCH_DIFF_USAGE }); + }; +} + +test('status(): a cwd outside any repository is its own machine-readable outcome, not a git message (mutation target: returning firstError)', async () => { + const calls = []; + const runner = createGitChangesRunner({ kind: 'local', cwd: REPO, exec: nonRepoExec(calls), fsOps: NO_REPO_FS }); + const result = await runner.status(); + + assert.equal(result.ok, false); + assert.equal(result.reason, NOT_A_REPO_REASON, 'the renderer must key on a reason, not parse a string'); + assert.equal(result.error, 'not a git repository'); + assert.equal(calls.filter((a) => a[1] === 'rev-parse').length, 1, 'exactly one probe, after the failure'); +}); + +test('status(): nothing git wrote reaches the caller when the cwd is outside a repository', async () => { + const runner = createGitChangesRunner({ kind: 'local', cwd: REPO, exec: nonRepoExec([]), fsOps: NO_REPO_FS }); + const result = await runner.status(); + + assert.doesNotMatch(result.error, /dépôt|fatal|usage|GIT_DISCOVERY/, + 'a localized fatal and a 150-line usage page are exactly what must not land in the panel'); + assert.ok(result.error.length < 60); +}); + +test('status(): the detection is the exit code, so a translated git says the same thing as an English one', async () => { + const ENGLISH_FATAL = 'fatal: not a git repository (or any parent up to mount point /)\nStopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).'; + const english = (args) => Promise.resolve(args[1] === 'diff' + ? { code: 129, stdout: '', stderr: 'usage: git diff --no-index' } + : { code: 128, stdout: '', stderr: ENGLISH_FATAL }); + + const fr = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec: nonRepoExec([]), fsOps: NO_REPO_FS }).status(); + const en = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec: english, fsOps: NO_REPO_FS }).status(); + assert.deepEqual(en, fr); +}); + +test('status(): a healthy repository never pays for the probe (mutation target: probing on every refresh)', async () => { + const calls = []; + const exec = (args) => { + calls.push(args); + return Promise.resolve({ code: 0, stdout: args[1] === 'status' ? '# branch.head main\x00' : '', stderr: '' }); + }; + const result = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec }).status(); + + assert.equal(result.ok, true); + assert.equal(calls.length, 3, 'three commands, not four — the probe is a diagnosis, not a precondition'); + assert.equal(calls.filter((a) => a[1] === 'rev-parse').length, 0); +}); + +test('status(): a failure inside a real repository keeps its own message and carries no reason', async () => { + const exec = (args) => { + if (args[1] === 'rev-parse') return Promise.resolve({ code: 0, stdout: 'true\n', stderr: '' }); + if (args[1] === 'status') return Promise.resolve({ code: 128, stdout: '', stderr: 'fatal: could not read directory: Permission denied' }); + return Promise.resolve({ code: 0, stdout: '', stderr: '' }); + }; + const result = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec }).status(); + + assert.equal(result.ok, false); + assert.equal(result.reason, undefined, 'a broken repository is not the same condition as no repository'); + assert.match(result.error, /Permission denied/); +}); + +test('status(): an ssh transport failure is never mistaken for "no repository"', async () => { + // ssh's own failure exit is 255, not git's 128; the probe cannot answer, so + // the original error stands. + const exec = () => Promise.resolve({ code: 255, stdout: '', stderr: 'ssh: connect to host build-01 port 22: Connection refused' }); + const result = await createGitChangesRunner({ kind: 'remote', cwd: '/srv/app', alias: 'build-01', exec }).status(); + + assert.equal(result.ok, false); + assert.equal(result.reason, undefined); + assert.match(result.error, /Connection refused/); +}); + +test('status(): a remote fatal is reported, bounded, because nothing can corroborate it', async () => { + const calls = []; + const exec = (command) => { + calls.push(command); + return Promise.resolve({ code: command.includes("'rev-parse'") || command.includes("'status'") ? 128 : 129, stdout: '', stderr: FRENCH_FATAL }); + }; + const result = await createGitChangesRunner({ kind: 'remote', cwd: '/srv/app', alias: 'build-01', exec }).status(); + + assert.equal(result.reason, undefined, + 'exit 128 is git\'s generic fatal code; with no filesystem to check, "no repository" is a guess, not a finding'); + assert.match(result.error, /dépôt git/, 'so the user gets git\'s own message instead of a silently missing control'); + assert.ok(result.error.length <= MAX_ERROR_CHARS + 1, 'still bounded'); + assert.ok(calls.some((c) => c === "git -C '/srv/app' '--literal-pathspecs' 'rev-parse' '--is-inside-work-tree'"), + 'the probe goes through the same quoted transport as every other command'); +}); + +// --- isWorkTree() ---------------------------------------------------------- + +test('isWorkTree(): the exit code and the printed answer, never the message', async () => { + const run = (response) => createGitChangesRunner({ kind: 'local', cwd: REPO, exec: () => Promise.resolve(response), fsOps: NO_REPO_FS }).isWorkTree(); + + assert.deepEqual(await run({ code: 0, stdout: 'true\n', stderr: '' }), { ok: true, isRepo: true }); + assert.deepEqual(await run({ code: 128, stdout: '', stderr: FRENCH_FATAL }), { ok: true, isRepo: false }); + assert.deepEqual(await run({ code: 0, stdout: 'false\n', stderr: '' }), { ok: true, isRepo: false }, + 'a bare repository has no work tree, so it has no changes to show either'); + + const broken = await run({ code: 255, stdout: '', stderr: 'ssh: Connection refused' }); + assert.equal(broken.ok, false, 'a transport failure is not an answer'); + assert.match(broken.error, /Connection refused/); + + const mute = await run({ code: 0, stdout: '', stderr: '' }); + assert.equal(mute.ok, false, 'no answer is not the same as "no"'); +}); + +test('isWorkTree(): a thrown exec is reported, not treated as a missing repository', async () => { + const runner = createGitChangesRunner({ kind: 'local', cwd: REPO, exec: () => { throw new Error('ENOENT'); }, fsOps: NO_REPO_FS }); + const result = await runner.isWorkTree(); + assert.equal(result.ok, false); + assert.match(result.error, /ENOENT/); +}); + +// --- Bounded error messages — see .ai/contexts/changes-view.md -------------- + +test('boundErrorMessage: git\'s 150-line usage page is cut to the stated bound (mutation target: dropping the cap)', () => { + const bounded = boundErrorMessage(FRENCH_DIFF_USAGE); + + assert.ok(bounded.split('\n').length <= MAX_ERROR_LINES, `at most ${MAX_ERROR_LINES} lines`); + assert.ok(bounded.length <= MAX_ERROR_CHARS + 1, `at most ${MAX_ERROR_CHARS} characters plus the ellipsis`); + assert.ok(bounded.endsWith('…'), 'the cut is signalled, not silent'); + assert.match(bounded, /Pas un dépôt git/, 'the first, informative line survives'); + assert.ok(FRENCH_DIFF_USAGE.length > 4000, 'the fixture has to be big enough for the bound to bite'); +}); + +test('boundErrorMessage: a short message passes through whole, with no ellipsis', () => { + assert.equal(boundErrorMessage('fatal: could not read directory: Permission denied'), + 'fatal: could not read directory: Permission denied'); + assert.equal(boundErrorMessage(' \n '), ''); +}); + +test('boundErrorMessage: one enormous line is cut by characters, not only by lines', () => { + const bounded = boundErrorMessage('x'.repeat(10_000)); + assert.ok(bounded.length <= MAX_ERROR_CHARS + 1); + assert.ok(bounded.endsWith('…')); +}); + +test('.diff(): an unexpected git failure reaches the caller bounded, never as the whole usage page', async () => { + const exec = () => Promise.resolve({ code: 129, stdout: '', stderr: FRENCH_DIFF_USAGE }); + const result = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec }).diff('foo.js'); + + assert.equal(result.ok, false); + assert.ok(result.error.split('\n').length <= MAX_ERROR_LINES); + assert.ok(result.error.length <= MAX_ERROR_CHARS + 1); +}); + +// --- gitEntryAtOrAbove: the corroboration, on both path flavours ------------- +// It answers three ways on purpose: true (a repository is there), false (there +// is definitively none), null (could not tell). Only `false` withdraws the +// panel — see .ai/contexts/changes-view.md ("Not a repository"). + +function fakeFs(entries, errorFor = {}) { + return { + lstat: (p) => { + if (errorFor[p]) { const e = new Error('nope'); e.code = errorFor[p]; throw e; } + if (entries.has(p)) return {}; + const e = new Error('ENOENT'); e.code = 'ENOENT'; throw e; + }, + }; +} + +for (const flavour of ['posix', 'win32']) { + const p = path[flavour]; + const root = flavour === 'posix' ? '/repo' : 'C:\\repo'; + const deep = p.join(root, 'nested', 'deeper'); + + test(`gitEntryAtOrAbove (${flavour}): a .git at the root is found from a deep subdirectory`, () => { + const fsOps = fakeFs(new Set([p.join(root, '.git')])); + assert.equal(gitEntryAtOrAbove(deep, fsOps, p), true); + }); + + test(`gitEntryAtOrAbove (${flavour}): no .git anywhere up to the filesystem root is a definite no`, () => { + assert.equal(gitEntryAtOrAbove(deep, fakeFs(new Set()), p), false, + 'the walk must terminate at the root instead of spinning on dirname'); + }); + + test(`gitEntryAtOrAbove (${flavour}): a .git that exists as a FILE counts — a broken gitdir is still a repository`, () => { + const fsOps = fakeFs(new Set([p.join(deep, '.git')])); + assert.equal(gitEntryAtOrAbove(deep, fsOps, p), true); + }); + + test(`gitEntryAtOrAbove (${flavour}): an unreadable ancestor is undecidable, never a definite no`, () => { + const fsOps = fakeFs(new Set(), { [p.join(deep, '.git')]: 'EACCES' }); + assert.equal(gitEntryAtOrAbove(deep, fsOps, p), null, + 'EACCES means the walk cannot see — guessing "no repository" here is what withdraws the panel wrongly'); + }); +} + +test('gitEntryAtOrAbove: a .git the walk cannot stat for an unexpected reason is undecidable', () => { + const fsOps = { lstat: () => { throw new Error('no code at all'); } }; + assert.equal(gitEntryAtOrAbove('/repo/x', fsOps, path.posix), null); +}); + +// --- The probe is not paid on the stdout-cap path --------------------------- + +test('status(): a -uall overrun does not add a probe — it is a volume problem, not a repository question', async () => { + const calls = []; + const exec = (args) => { + calls.push(args); + if (args[1] !== 'status') return Promise.resolve({ code: 0, stdout: '1\t2\tfoo.js\0', stderr: '' }); + if (args.includes('-uall')) return Promise.resolve({ code: -1, stdout: '', stderr: 'stdout exceeded 2097152 bytes' }); + return Promise.resolve({ code: 0, stdout: '# branch.head main\x00', stderr: '' }); + }; + const result = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec }).status(); + + assert.equal(result.ok, true); + assert.equal(result.untrackedCollapsed, true); + assert.equal(calls.filter((a) => a[1] === 'rev-parse').length, 0, + 'the large repositories that hit this cap are exactly the ones an extra spawn per refresh costs most'); + assert.equal(calls.length, 4, 'three commands plus the one collapsed-listing retry'); +}); + +test('status(): a cap overrun alongside a real failure still asks, so a non-repo is not missed', async () => { + const calls = []; + const exec = (args) => { + calls.push(args); + if (args[1] === 'rev-parse') return Promise.resolve({ code: 128, stdout: '', stderr: FRENCH_FATAL }); + if (args.includes('-uall')) return Promise.resolve({ code: -1, stdout: '', stderr: 'stdout maxBuffer length exceeded' }); + return Promise.resolve({ code: 128, stdout: '', stderr: FRENCH_FATAL }); + }; + const result = await createGitChangesRunner({ kind: 'local', cwd: REPO, exec, fsOps: NO_REPO_FS }).status(); + + assert.equal(result.reason, NOT_A_REPO_REASON); + assert.equal(calls.filter((a) => a[1] === 'rev-parse').length, 1); +}); + +test('missingCwdError refuses a partial fs seam instead of quietly switching the cwd check off', () => { + assert.throws(() => missingCwdError('/repo', { lstat: () => ({}) }), TypeError, + 'a seam with no stat used to be swallowed as "the cwd is fine", disabling the guard with no sign'); + assert.throws(() => missingCwdError('/repo', {}), TypeError); + assert.throws(() => missingCwdError('/repo', null), TypeError); +}); + +test('missingCwdError still treats an unexpected stat failure as "cannot tell", not as a missing directory', () => { + const fsOps = { stat: () => { const e = new Error('nope'); e.code = 'EACCES'; throw e; } }; + assert.equal(missingCwdError('/repo', fsOps), null, + 'only ENOENT/ENOTDIR name the directory; anything else leaves the 128 corroboration to decide'); +}); diff --git a/test/open-session-terminal.test.js b/test/open-session-terminal.test.js new file mode 100644 index 00000000..28998354 --- /dev/null +++ b/test/open-session-terminal.test.js @@ -0,0 +1,190 @@ +// Tests for app.js's openSession — reopening a plain terminal must reach +// open-terminal as a plain terminal. See .ai/contexts/session-state.md +// ("Reopening a plain terminal"). +// +// app.js cannot be eval-ed in jsdom (module-scope `new ViewerPanel(...)` etc. +// — see test/running-indicators.test.js's file header for the full reason). +// `makeOpenSession` below is therefore a HAND-MAINTAINED MIRROR of the real +// function, not the shipped code — it pins the *decision* logic in isolation. +// The source-level pins at the bottom catch the regressions that matter in the +// shipped file without needing a full eval — the same two-layer technique as +// test/confirm-and-stop-session.test.js. + +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const APP_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); + +// Mirrors public/app.js's openSession(session, customOptions), with every +// external dependency injected instead of read off globals. +function makeOpenSession(deps) { + return async function openSession(session, customOptions) { + const { sessionId, projectPath } = session; + + if (deps.openSessions.has(sessionId)) { + const entry = deps.openSessions.get(sessionId); + if (entry.closed) { + deps.destroySession(sessionId); + } else { + deps.showSession(sessionId); + return; + } + } + + const entry = deps.createTerminalEntry(session); + const resumeOptions = customOptions + || (session.type === 'terminal' ? { type: 'terminal' } : await deps.resolveDefaultSessionOptions({ projectPath })); + const result = await deps.api.openTerminal(sessionId, projectPath, false, resumeOptions, entry.initialSize); + if (!result.ok) { + deps.markFailed(sessionId, result.error); + return; + } + deps.showSession(sessionId); + }; +} + +function makeDeps(overrides = {}) { + const calls = { openTerminal: [], destroyed: [], shown: [], created: [], resolveDefaults: 0, launchTerminal: [] }; + const deps = { + calls, + openSessions: new Map(), + destroySession: (id) => calls.destroyed.push(id), + showSession: (id) => calls.shown.push(id), + createTerminalEntry: (session) => { calls.created.push(session.sessionId); return { initialSize: { cols: 80, rows: 24 } }; }, + resolveDefaultSessionOptions: async () => { calls.resolveDefaults++; return { permissionMode: 'plan' }; }, + // The mint-a-new-session path openSession must no longer take. + launchTerminalSession: (project) => calls.launchTerminal.push(project), + markFailed: () => {}, + api: { + openTerminal: async (sessionId, projectPath, isNew, sessionOptions, initialSize) => { + calls.openTerminal.push({ sessionId, projectPath, isNew, sessionOptions, initialSize }); + return { ok: true }; + }, + }, + ...overrides, + }; + return deps; +} + +const TERMINAL_SESSION = { sessionId: 'term-uuid', projectPath: '/proj', type: 'terminal' }; +const CLAUDE_SESSION = { sessionId: 'claude-uuid', projectPath: '/proj' }; + +// --- The intent that was being dropped ------------------------------------- + +test('a terminal session that is not open reaches open-terminal as a plain terminal (mutation target: the dropped type)', async () => { + const deps = makeDeps(); + await makeOpenSession(deps)(TERMINAL_SESSION); + + assert.equal(deps.calls.openTerminal.length, 1); + assert.deepEqual(deps.calls.openTerminal[0].sessionOptions, { type: 'terminal' }, + 'main.js reads sessionOptions.type === "terminal"; without it a shell id is handed to claude --resume'); + assert.equal(deps.calls.resolveDefaults, 0, + 'a shell has no permission mode, worktree or MCP emulation to resolve'); +}); + +test('a Claude session still resumes with the project\'s current defaults, and carries no terminal type', async () => { + const deps = makeDeps(); + await makeOpenSession(deps)(CLAUDE_SESSION); + + assert.equal(deps.calls.resolveDefaults, 1); + assert.deepEqual(deps.calls.openTerminal[0].sessionOptions, { permissionMode: 'plan' }); + assert.equal(deps.calls.openTerminal[0].sessionOptions.type, undefined); +}); + +test('an explicit customOptions still wins — the resume-with-config dialog is not overridden', async () => { + const deps = makeDeps(); + const chosen = { permissionMode: 'acceptEdits', chrome: true }; + await makeOpenSession(deps)(CLAUDE_SESSION, chosen); + + assert.equal(deps.calls.openTerminal[0].sessionOptions, chosen); + assert.equal(deps.calls.resolveDefaults, 0); +}); + +test('customOptions wins over the terminal default too, so the precedence has one rule, not two', async () => { + const deps = makeDeps(); + const chosen = { type: 'terminal', panelFor: 'owner' }; + await makeOpenSession(deps)(TERMINAL_SESSION, chosen); + + assert.equal(deps.calls.openTerminal[0].sessionOptions, chosen); +}); + +// --- The orphaned row ------------------------------------------------------ + +test('a terminal whose shell exited reopens under its own id instead of minting a second row', async () => { + const deps = makeDeps(); + deps.openSessions.set('term-uuid', { closed: true }); + await makeOpenSession(deps)(TERMINAL_SESSION); + + assert.deepEqual(deps.calls.destroyed, ['term-uuid'], 'the dead entry is torn down first'); + assert.deepEqual(deps.calls.launchTerminal, [], + 'minting a new id leaves the clicked row pointing at an id nothing can open'); + assert.equal(deps.calls.openTerminal.length, 1); + assert.equal(deps.calls.openTerminal[0].sessionId, 'term-uuid', 'the row you clicked is the row that comes back'); + assert.deepEqual(deps.calls.openTerminal[0].sessionOptions, { type: 'terminal' }); +}); + +test('an exited Claude session still reopens in place, the way it always did', async () => { + const deps = makeDeps(); + deps.openSessions.set('claude-uuid', { closed: true }); + await makeOpenSession(deps)(CLAUDE_SESSION); + + assert.deepEqual(deps.calls.destroyed, ['claude-uuid']); + assert.equal(deps.calls.openTerminal[0].sessionId, 'claude-uuid'); +}); + +test('a live entry is only shown — no second PTY for a terminal that is already running', async () => { + const deps = makeDeps(); + deps.openSessions.set('term-uuid', { closed: false }); + await makeOpenSession(deps)(TERMINAL_SESSION); + + assert.deepEqual(deps.calls.shown, ['term-uuid']); + assert.equal(deps.calls.openTerminal.length, 0); + assert.equal(deps.calls.created.length, 0); +}); + +// --------------------------------------------------------------------------- +// Source-level pins for the REAL public/app.js. +// --------------------------------------------------------------------------- + +function openSessionBody() { + const start = APP_SRC.indexOf('async function openSession(session, customOptions)'); + assert.notEqual(start, -1, 'openSession must still exist with the (session, customOptions) signature'); + const end = APP_SRC.indexOf('\n}', APP_SRC.indexOf('pollActiveSessions();', start)); + assert.ok(end > start); + return APP_SRC.slice(start, end); +} + +test('public/app.js: openSession passes { type: \'terminal\' } for a terminal session (mutation target: reverting to the bare defaults call)', () => { + const body = openSessionBody(); + assert.match(body, /session\.type === 'terminal'\s*\?\s*\{\s*type:\s*'terminal'\s*\}/, + 'a terminal session must supply its own type rather than the Claude launch defaults'); +}); + +test('public/app.js: customOptions is still the first term of the resumeOptions chain', () => { + const body = openSessionBody(); + const chain = body.slice(body.indexOf('const resumeOptions')); + const custom = chain.indexOf('customOptions'); + const terminal = chain.indexOf("session.type === 'terminal'"); + assert.notEqual(custom, -1); + assert.ok(custom < terminal, 'an explicit choice from the resume dialog must keep winning'); +}); + +test('public/app.js: openSession no longer mints a new session for an exited terminal', () => { + const body = openSessionBody(); + assert.doesNotMatch(body, /launchTerminalSession\(/, + 'minting a second id leaves the original sidebar row pointing at an id nothing can open'); + assert.match(body, /window\.api\.openTerminal\(sessionId,/, + 'the reopen must target the session id that was clicked'); +}); + +test('public/dialogs.js: resolveDefaultSessionOptions still returns Claude launch options only', () => { + const src = fs.readFileSync(path.join(__dirname, '..', 'public', 'dialogs.js'), 'utf8'); + const start = src.indexOf('async function resolveDefaultSessionOptions(project)'); + assert.notEqual(start, -1); + const body = src.slice(start, src.indexOf('\n}', start)); + assert.doesNotMatch(body, /type:\s*'terminal'/, + 'the terminal intent belongs to the caller that knows the session type, not to the Claude defaults'); +}); diff --git a/test/panel-terminal.test.js b/test/panel-terminal.test.js index 5b3c3151..d44be4fd 100644 --- a/test/panel-terminal.test.js +++ b/test/panel-terminal.test.js @@ -803,3 +803,197 @@ test('panel shell ids are excluded from the running-session count', () => { 'a lone panel shell must not pin the poll to its fast cadence'); } finally { ctx.destroy(); } }); + +// --- 8. No tab above the region — see .ai/contexts/panel-terminal.md ("Layout") --- + +const STATUS_OK = { + ok: true, + branch: { head: 'main', upstream: null, ahead: 0, behind: 0 }, + files: [], + totals: { files: 0, added: 0, deleted: 0 }, +}; + +// A panel that can open a real Changes tab on top of the shell region. +function setupPanelWithTab(extra = {}) { + return setupPanel({ + api: { + gitChangesAvailable: () => Promise.resolve({ ok: true, isRepo: true }), + gitChangesStatus: () => Promise.resolve(STATUS_OK), + }, + ...extra, + }); +} + +test('with no tab open the region is the panel, and there is nothing left to drag against', async () => { + const ctx = setupPanelWithTab(); + try { + const { window, document } = ctx; + await window.togglePanelTerminal('owner'); + + const content = document.getElementById('file-panel-content'); + assert.ok(content.classList.contains('shell-only'), + 'a shell with nothing above it must fill the panel instead of leaving the tab area as an empty block'); + assert.ok(document.getElementById('panel-terminal-region').classList.contains('open')); + } finally { ctx.destroy(); } +}); + +test('opening a tab gives the region its stored height back, and closing it fills the panel again', async () => { + const ctx = setupPanelWithTab(); + try { + const { window, document } = ctx; + await window.togglePanelTerminal('owner'); + const content = document.getElementById('file-panel-content'); + + await window.openChangesTab('owner'); + await microtasks(); + assert.equal(content.classList.contains('shell-only'), false, + 'a tab is back above the region, so the region goes back to its own height'); + + window.toggleChangesTab('owner'); + assert.ok(content.classList.contains('shell-only')); + } finally { ctx.destroy(); } +}); + +test('a height the user dragged to survives a full-height episode', async () => { + const ctx = setupPanelWithTab(); + try { + const { window, document } = ctx; + await window.togglePanelTerminal('owner'); + const region = document.getElementById('panel-terminal-region'); + const handle = document.getElementById('panel-terminal-handle'); + + setPanelHeight(document, 800); + await window.openChangesTab('owner'); + await microtasks(); + handle.dispatchEvent(mouse(window, 'mousedown', 400)); + document.dispatchEvent(mouse(window, 'mousemove', 220)); // drag up to 400px + document.dispatchEvent(mouse(window, 'mouseup', 220)); + assert.equal(region.style.height, '400px'); + + window.toggleChangesTab('owner'); // no tab: the region fills the panel + assert.ok(document.getElementById('file-panel-content').classList.contains('shell-only')); + + await window.openChangesTab('owner'); + await microtasks(); + assert.equal(region.style.height, '400px', + 'the stored height is what the drag asked for, never a value read back out of the full-height state'); + assert.equal(window.localStorage.getItem('panelTerminalHeight'), '400'); + } finally { ctx.destroy(); } +}); + +test('the shell is refitted when the region stops or starts filling the panel', async () => { + const ctx = setupPanelWithTab(); + try { + const { window, document, spies } = ctx; + await window.togglePanelTerminal('owner'); + assert.ok(document.getElementById('file-panel-content').classList.contains('shell-only')); + + const before = spies.resize.length; + await window.openChangesTab('owner'); + await microtasks(); + assert.ok(spies.resize.length > before, + 'the region just changed size — an unfitted terminal renders at the wrong geometry'); + + const afterOpen = spies.resize.length; + window.toggleChangesTab('owner'); + assert.ok(spies.resize.length > afterOpen, 'and again when it takes the whole panel back'); + } finally { ctx.destroy(); } +}); + +test('a tab with no shell open leaves the panel untouched', async () => { + const ctx = setupPanelWithTab(); + try { + const { window, document } = ctx; + window.switchPanel('owner'); + await window.openChangesTab('owner'); + await microtasks(); + + assert.equal(document.getElementById('file-panel-content').classList.contains('shell-only'), false); + assert.equal(document.getElementById('panel-terminal-region').classList.contains('open'), false); + } finally { ctx.destroy(); } +}); + +// --- 9. The layout rules those states depend on ---------------------- +// Same source-grep shape as test/session-meta-layout-css.test.js — no real CSS +// parser, just enough to isolate a rule's declarations. + +const CSS = require('node:fs').readFileSync(require('node:path').join(__dirname, '..', 'public', 'style.css'), 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, ''); + +function cssRuleFor(selectorPattern) { + const blocks = CSS.match(/[^{}]+\{[^{}]*\}/g) || []; + return blocks.find((block) => { + const lines = block.split('{')[0].split('\n'); + return selectorPattern.test(lines[lines.length - 1].trim()); + }); +} + +test('style.css: the handle bottom-anchors the region while a tab is shown', () => { + const rule = cssRuleFor(/^#panel-terminal-handle$/); + assert.ok(rule, 'expected a #panel-terminal-handle rule'); + assert.match(rule, /margin-top:\s*auto/, + 'with flexible content above it, the handle is what pins the region to the bottom of the panel'); +}); + +test('style.css: with no tab, the handle is gone and the region takes the free space', () => { + const handleRule = cssRuleFor(/^#file-panel-content\.shell-only #panel-terminal-handle\.open$/); + assert.ok(handleRule, 'expected the shell-only override for the handle'); + assert.match(handleRule, /display:\s*none/, + 'a drag handle with nothing above it to give space back to is a control that does nothing'); + + const regionRule = cssRuleFor(/^#file-panel-content\.shell-only #panel-terminal-region\.open$/); + assert.ok(regionRule, 'expected the shell-only override for the region'); + assert.match(regionRule, /flex:\s*1/, + 'a flex-basis of 0 is what lets the region fill the panel while style.height still records the drag'); + assert.match(regionRule, /min-height:\s*0/); +}); + +// --- 10. Grid mode's exclusion rides on the sidebar payload ------------------ +// layoutGridCards iterates SIDEBAR ROWS and wraps every id that is also in +// openGridSessionIds(), which does contain `panel:`. Nothing in grid-view +// knows about panel shells: the shell keeps its container only because +// buildProjectsFromCache gives it no row. That coupling is load-bearing and +// belongs in a test — see .ai/contexts/panel-terminal.md. + +function addSidebarRow(ctx, sessionId) { + const item = ctx.document.createElement('div'); + item.className = 'session-item'; + item.dataset.sessionId = sessionId; + ctx.window.sidebarContent.appendChild(item); + return item; +} + +test('a panel shell gets no grid card, because the sidebar payload gives it no row', async () => { + const ctx = setupPanel(); + try { + const { window } = ctx; + window.createTerminalEntry({ sessionId: 'owner' }); + await window.togglePanelTerminal('owner'); + addSidebarRow(ctx, 'owner'); // the only row main sends for this project + + // layoutGridCards builds its array inside the vm realm — copy before comparing. + const laid = Array.from(window.layoutGridCards(window.openGridSessionIds())); + + assert.deepEqual(laid, ['owner'], 'the shell is in openGridSessionIds but has no row to be laid out from'); + assert.equal(window.openSessions.get('panel:owner').element.parentElement.id, 'panel-terminal-region', + 'a grid card would move the shell out of its region and cost it its WebGL context'); + } finally { ctx.destroy(); } +}); + +test('the shell IS in the grid-eligible set — only the missing row keeps it out (mutation target: re-adding the row)', async () => { + const ctx = setupPanel(); + try { + const { window } = ctx; + window.createTerminalEntry({ sessionId: 'owner' }); + await window.togglePanelTerminal('owner'); + + assert.ok(window.openGridSessionIds().has('panel:owner'), + 'grid-view has no panel-shell predicate of its own, so this set includes it'); + + // What buildProjectsFromCache used to emit: a row for the shell. + addSidebarRow(ctx, 'owner'); + addSidebarRow(ctx, 'panel:owner'); + assert.deepEqual(Array.from(window.layoutGridCards(window.openGridSessionIds())), ['owner', 'panel:owner'], + 'given a row, grid mode lays the shell out like any session — which is why the row must not exist'); + } finally { ctx.destroy(); } +});