diff --git a/.ai/contexts/changes-view.md b/.ai/contexts/changes-view.md index 2f70cd22..259386b9 100644 --- a/.ai/contexts/changes-view.md +++ b/.ai/contexts/changes-view.md @@ -306,20 +306,20 @@ counts. ## 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. +A session's working directory need not be inside a git work tree. The Changes +control is offered for every session regardless, and the panel reports whichever +of four answers is true: the file list when there is a work tree with changes, +`No changes` when that tree is clean, `This directory is not a git repository.` +when there is no work tree, and git's own bounded message when there **is** a +repository and git refuses to open it. + +**A control that vanishes explains nothing.** Its absence is indistinguishable +from a bug and offers no way to ask why, so the affordance is unconditional and +every answer is text in the panel it opens. That puts the whole weight on +telling the last two answers apart: "there is no repository here" and "git will +not open this repository" are indistinguishable from git's exit status, and they +must not read alike in the panel — the second is a repository the user has, +usually with a one-line fix in git's own message. **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 @@ -343,15 +343,15 @@ 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 +message — exactly the case the plain "this is not a repository" line must never +be shown for. `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 +worktree outside a repository would otherwise be reported as "not a git +repository". `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 @@ -367,8 +367,8 @@ 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 +reached the root seeing nothing) is "there is no repository here"; 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. @@ -378,85 +378,32 @@ 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. +repository shows git's own bounded message rather than the plain-language line, +which is the safe side of the trade — the reported message is always true, and +only its wording is coarser than a local session's. + +**Who asks.** `status()` is the only production caller: it returns +`{ok: false, reason: 'not-a-repo'}` when a command failed **and** +`isWorkTree()` then establishes there is no work tree, and every other failure +carries its bounded message and no reason. The renderer turns that one reason +into the plain-language line and everything else into git's own words, so the +distinction the probe establishes is the distinction the user reads. `tab.notARepo` +carries it to both surfaces that can show it: the list renders a neutral note +rather than an error, and the notice line above an open editor states the same +fact in the same neutral colour instead of reporting a refresh that failed — +a directory with no repository did not fail at anything. `isWorkTree()` stays on the +runner's interface for the tests that pin the table above. + +**The probe is a diagnosis, not a precondition.** A session in a repository pays +three commands per refresh, pinned by `calls.length === 3`; the fourth runs only on a status that already failed. 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. + +A repository that disappears under an open tab is reported into that tab on the +next refresh, and one that appears under a session that had none is listed on +the next refresh too. Nothing polls and nothing is memoised, because the answer +arrives with the status the tab was going to fetch anyway. ## Bounded error messages @@ -472,7 +419,7 @@ 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. +message rather than to the "not a git repository" line. ## cwd resolution (`git-changes-target.js`) diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index aa6e164d..afd80957 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -89,8 +89,7 @@ design (parser, runner, quoting, cwd resolution, refresh triggers, editing): | IPC | Args | Returns | Notes | |---|---|---|---| -| `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-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'` — the panel's plain-language "not a git repository" line; every other failure carries a bounded message and no reason, including a repository git refuses to open. Telling the two apart is `git rev-parse --is-inside-work-tree` plus a filesystem walk, never a message match — see `.ai/contexts/changes-view.md` ("Not a repository"). | | `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/docs/changes-view.md b/docs/changes-view.md index 4912ba41..1744a32b 100644 --- a/docs/changes-view.md +++ b/docs/changes-view.md @@ -6,22 +6,24 @@ 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. +Every session has the button. What the panel shows when you open it is whichever +of these is true: + +- the list of changed files, when there are any; +- **No changes**, when the working tree is clean; +- **This directory is not a git repository.**, when the session was started + somewhere that is not one — a scratch directory, a notes folder. Run + `git init` there and the files are listed from the next refresh; +- git's own message, when there *is* a repository and git will not open it: it + is owned by another user, its permissions are wrong, or its format is one this + git does not support. That message usually names the fix. + +The last two are deliberately not worded alike: the first is a directory you +never meant to be a repository, the second is a repository you have and cannot +read yet. + +Delete the repository under a running session and the panel says so on its next +refresh. 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. @@ -113,10 +115,9 @@ 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. +A remote working directory that is not a git repository shows git's own message +rather than the plain "not a git repository" line. Telling those two cases apart +needs a 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/main.js b/main.js index 772da514..f48d6a52 100644 --- a/main.js +++ b/main.js @@ -1757,17 +1757,6 @@ 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 1ff0d97a..0c16d842 100644 --- a/preload.js +++ b/preload.js @@ -34,7 +34,6 @@ 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/file-panel.js b/public/file-panel.js index b14b4707..05c3ef30 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -57,11 +57,9 @@ 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") +// 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 NOT_A_REPO_TEXT = 'This directory is not a git repository.'; const PANEL_WIDTH_KEY = 'filePanelWidth'; const DEFAULT_PANEL_WIDTH = parseInt(localStorage.getItem(PANEL_WIDTH_KEY), 10) || 450; @@ -356,7 +354,6 @@ function getSessionState(sessionId) { panelVisible: false, panelWidth: DEFAULT_PANEL_WIDTH, mcpActive: false, - changesAvailable: null, }); } return filePanelState.get(sessionId); @@ -557,7 +554,6 @@ function hidePanel() { function switchPanel(sessionId) { currentPanelSessionId = sessionId; updateMcpIndicator(); - refreshChangesAvailability(sessionId); if (typeof syncPanelTerminal === 'function') syncPanelTerminal(sessionId); if (!sessionId) { @@ -714,54 +710,6 @@ 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') { @@ -806,6 +754,7 @@ function openChangesTab(sessionId) { saveError: null, saving: false, externalChange: false, + notARepo: false, }; state.panelVisible = true; restoreChangesEdits(sessionId, state, state.currentTab); @@ -833,14 +782,15 @@ async function refreshChanges(sessionId) { tab.loading = false; if (result && result.reason === NOT_A_REPO_REASON) { - noteChangesUnavailable(sessionId); - if (!stillState.currentTab) return; - tab.error = result.error || 'not a git repository'; + tab.notARepo = true; + tab.error = NOT_A_REPO_TEXT; tab.data = null; } else if (!result || result.ok === false) { + tab.notARepo = false; tab.error = (result && result.error) || 'failed to load changes'; tab.data = null; } else { + tab.notARepo = false; tab.error = null; tab.data = result; tab.remote = result.kind === 'remote'; @@ -1085,7 +1035,7 @@ function renderChangesList(sessionId, tab) { changesSummaryEl.textContent = ''; changesListEl.innerHTML = ''; const err = document.createElement('div'); - err.className = 'changes-error'; + err.className = tab.notARepo ? 'changes-note' : 'changes-error'; err.textContent = tab.error; changesListEl.appendChild(err); if (branchInfoEl) branchInfoEl.textContent = ''; @@ -1313,14 +1263,16 @@ function updateChangesSaveButton(sessionId, tab) { function renderChangesNotice(tab) { const notes = []; - const alarming = !!(tab.saveError || tab.fileError || tab.error || tab.externalChange || tab.restoredEdits); + const listFailed = !!tab.error && !tab.notARepo; + const alarming = !!(tab.saveError || tab.fileError || listFailed || tab.externalChange || tab.restoredEdits); if (tab.remote) notes.push('Remote session — read-only.'); if (tab.fallbackReason) notes.push(`${tab.fallbackReason} — showing the diff read-only.`); if (tab.diffTruncated) notes.push('Diff truncated at 512 KB.'); if (tab.restoredEdits) notes.push('Unsaved edits kept from when the session opened something else in this panel have been restored.'); if (tab.externalChange) notes.push('This file changed on disk since you opened it — reload before saving, or your edits will not be accepted.'); if (tab.fileError) notes.push(`This file can no longer be read: ${tab.fileError}`); - if (tab.error) notes.push(`The file list could not be refreshed: ${tab.error}`); + if (tab.notARepo) notes.push(NOT_A_REPO_TEXT); + else if (tab.error) notes.push(`The file list could not be refreshed: ${tab.error}`); if (tab.saveError) notes.push(`Save failed: ${tab.saveError}`); changesDiffNoticeEl.textContent = notes.join(' '); diff --git a/public/style.css b/public/style.css index 128c1599..2c8504ab 100644 --- a/public/style.css +++ b/public/style.css @@ -4635,6 +4635,12 @@ body { display: flex; flex-direction: column; } font-size: 12px; } +.changes-note { + padding: 12px; + color: var(--text-muted); + font-size: 12px; +} + .changes-degraded-note { margin-top: 4px; font-size: 11px; diff --git a/test/dom-file-panel-changes.test.js b/test/dom-file-panel-changes.test.js index 63ed9634..33516abf 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, availableImpl } = {}) { +function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmImpl, locateImpl } = {}) { 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: [], available: [] }; + const calls = { status: [], diff: [], file: [], save: [], watch: [], unwatch: [], confirm: [], locate: [], readFile: [] }; const editors = []; const fileChangedListeners = []; @@ -95,10 +95,6 @@ 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)); @@ -1914,94 +1910,63 @@ 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(); +// --- What the panel says, for each answer git can give --------------------- +// The Changes control is unconditional; the panel reports. See +// .ai/contexts/changes-view.md ("Not a repository"). - 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(); } -}); +function noRepoStatus() { + return { ok: false, reason: 'not-a-repo', error: 'fatal: not a git repository (or any of the parent directories): .git' }; +} -test('the Changes button stays visible for a session that is in a work tree', async () => { +test('a work tree with changes lists its files under a count', async () => { const ctx = setupFilePanelDom(); try { ctx.window.switchPanel('s1'); + ctx.window.openChangesTab('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'); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2); + assert.match(ctx.document.getElementById('changes-summary').textContent, /2 files changed/); } 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; +test('a clean work tree keeps the Changes control and says there is nothing to show', async () => { const ctx = setupFilePanelDom({ - availableImpl: () => ({ ok: true, isRepo }), - statusImpl: () => (isRepo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + statusImpl: () => makeStatusResult({ files: [], totals: { files: 0, added: 0, deleted: 0 } }), }); try { ctx.window.switchPanel('s1'); - ctx.window.openChangesTab('s1'); - await flush(); - assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none'); - isRepo = false; - ctx.setActivity('s1', true); - ctx.setActivity('s1', false); + ctx.window.openChangesTab('s1'); 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'); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 0); + assert.match(ctx.document.getElementById('changes-summary').textContent, /No changes/); } 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' }), - }); +test('a directory that is not a repository keeps the Changes control and says so in plain words', async () => { + const ctx = setupFilePanelDom({ statusImpl: noRepoStatus }); try { ctx.window.switchPanel('s1'); + assert.notEqual(ctx.document.getElementById('changes-toggle-btn').style.display, 'none', + 'a control that vanishes explains nothing; the panel explains instead'); + 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/); + const note = ctx.document.querySelector('.changes-note'); + assert.ok(note, 'the answer reaches the panel as a note, not as a git failure'); + assert.match(note.textContent, /not a git repository/); + assert.equal(ctx.document.querySelectorAll('.changes-error').length, 0); + assert.doesNotMatch(note.textContent, /fatal:/, "git's own wording is not what this state says"); } finally { ctx.destroy(); } }); -test('a genuine git failure is still reported in the tab, and the button stays', async () => { +test("a repository git refuses shows git's own message, not the no-repository text (mutation target: the reason check)", async () => { const ctx = setupFilePanelDom({ - statusImpl: () => ({ ok: false, error: 'could not read directory: Permission denied' }), + statusImpl: () => ({ ok: false, error: 'fatal: detected dubious ownership in repository at /srv/repo' }), }); try { ctx.window.switchPanel('s1'); @@ -2009,454 +1974,135 @@ test('a genuine git failure is still reported in the tab, and the button stays', 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.ok(err, 'a repository git will not open is a failure to report, with its own message'); + assert.match(err.textContent, /dubious ownership/); + assert.equal(ctx.document.querySelectorAll('.changes-note').length, 0); + assert.doesNotMatch(ctx.document.body.textContent, /This directory is not a git repository/, + 'telling the two apart is the whole point of the filesystem corroboration behind the reason'); 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' }), - }); +test('a repository that disappears under an open tab reports into it instead of closing it', async () => { + let repo = true; + const ctx = setupFilePanelDom({ statusImpl: () => (repo ? makeStatusResult() : noRepoStatus()) }); 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); }; - - isRepo = false; + repo = 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(); } -}); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), true, + 'the tab stays open and says what happened'); + assert.match(ctx.document.querySelector('.changes-note').textContent, /not a git repository/); -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'); + repo = true; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); await flush(); - assert.equal(ctx.calls.available.filter((id) => id === 's1').length, before); + assert.equal(ctx.document.querySelectorAll('.changes-file-row').length, 2, + 'and a git init is picked up by the next refresh, with no probe to re-ask'); } 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' }), - }); +test('a git failure after a no-repository answer is shown as a failure (mutation target: the reset)', async () => { + let answer = noRepoStatus; + const ctx = setupFilePanelDom({ statusImpl: () => answer() }); 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(); + assert.ok(ctx.document.querySelector('.changes-note')); - const writes = countDisplayWrites(ctx); - writes.reset(); - - d.settle('s1', { ok: true, isRepo: true }); + answer = () => ({ ok: false, error: 'fatal: detected dubious ownership in repository at /srv/repo' }); + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); 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'); + assert.equal(ctx.document.querySelectorAll('.changes-note').length, 0, + 'the previous answer must not colour the next one'); + assert.match(ctx.document.querySelector('.changes-error').textContent, /dubious ownership/); } 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"). +// The three ways the tree under an open editor can stop answering: the +// repository removed, the directory itself gone, and a repository git refuses. +const TREE_LOST_PAYLOADS = [ + ['the repository removed', noRepoStatus], + ['the working directory gone', () => ({ ok: false, error: 'the working directory no longer exists: /gone' })], + ['a repository git refuses', () => ({ ok: false, error: 'fatal: detected dubious ownership in repository at /srv/repo' })], +]; -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); +for (const [label, lostStatus] of TREE_LOST_PAYLOADS) { + test(`unsaved edits survive ${label}, and nothing is asked`, async () => { + let repo = true; + const ctx = setupFilePanelDom({ + confirmImpl: () => true, + statusImpl: () => (repo ? makeStatusResult() : lostStatus()), + }); + try { + await openFile(ctx, 's1', 'src/a.js'); + ctx.editors[0].box.text = 'my unsaved edit\n'; - // 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(); } -}); + repo = false; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); -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 }), + assert.equal(ctx.calls.confirm.length, 0, 'nothing is being discarded, so nothing is asked'); + assert.equal(ctx.editors[0].box.destroyed, false); + assert.equal(ctx.editors[0].box.text, 'my unsaved edit\n'); + assert.equal(ctx.document.getElementById('file-panel').classList.contains('open'), true); + } finally { ctx.destroy(); } }); - 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 () => { +test('the notice bar states the no-repository fact without borrowing the failure colour or its wording', async () => { let repo = true; - const ctx = setupFilePanelDom({ - confirmImpl: () => false, - statusImpl: () => (repo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), - }); + const ctx = setupFilePanelDom({ statusImpl: () => (repo ? makeStatusResult() : noRepoStatus()) }); 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(); } -}); + await openFile(ctx, 's1', 'src/a.js'); -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; }); + repo = false; + ctx.setActivity('s1', true); + ctx.setActivity('s1', false); + await flush(); - 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'); + const notice = ctx.document.getElementById('changes-diff-notice'); + assert.match(notice.textContent, /This directory is not a git repository\./); + assert.doesNotMatch(notice.textContent, /could not be refreshed/, + 'a directory that has no repository did not fail to refresh'); + assert.equal(notice.classList.contains('changes-error'), false, + 'the list below says this in a neutral note; the bar must not contradict it'); + assert.equal(notice.classList.contains('changes-diff-truncated'), true); } finally { ctx.destroy(); } }); -test('a withdrawal with nothing unsaved never asks, it just withdraws', async () => { +test('a genuine refresh failure still reaches the notice bar in the failure colour', async () => { let repo = true; const ctx = setupFilePanelDom({ - confirmImpl: () => false, - statusImpl: () => (repo ? makeStatusResult() : { ok: false, reason: 'not-a-repo', error: 'not a git repository' }), + statusImpl: () => (repo ? makeStatusResult() : { ok: false, error: 'could not read directory: Permission denied' }), }); 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'); + const notice = ctx.document.getElementById('changes-diff-notice'); + assert.match(notice.textContent, /The file list could not be refreshed: could not read directory/); + assert.equal(notice.classList.contains('changes-error'), true); } 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 @@ -2491,7 +2137,7 @@ test('the panel X closes a Changes tab on its own path, not through toggleChange } finally { ctx.destroy(); } }); -test('the panel X leaves the Changes button, so the tab it closed can be reopened', async () => { +test('the tab the panel X closed is reopened by the Changes button', async () => { const ctx = setupFilePanelDom(); try { ctx.window.switchPanel('s1'); @@ -2499,8 +2145,6 @@ test('the panel X leaves the Changes button, so the tab it closed can be reopene 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(); diff --git a/test/panel-terminal.test.js b/test/panel-terminal.test.js index d44be4fd..bb814bf4 100644 --- a/test/panel-terminal.test.js +++ b/test/panel-terminal.test.js @@ -817,7 +817,6 @@ const STATUS_OK = { function setupPanelWithTab(extra = {}) { return setupPanel({ api: { - gitChangesAvailable: () => Promise.resolve({ ok: true, isRepo: true }), gitChangesStatus: () => Promise.resolve(STATUS_OK), }, ...extra,