Skip to content

Commit 474b7cc

Browse files
committed
fix: second adversarial pass (git, client shell, server limits, regressions)
- git: never look inside submodules (their own filters ran via diff); fail closed when the driver listing is cut; re-list drivers per operation (included configs); rename limits, UTF-8 log output and --root forced; a timed-out diff is truncated, not clean. - server: a shell that cannot start no longer exits the server, at most 64 shells; index deletes are not quadratic; git status refresh is throttled, not starved; search reports symlinked matches under the real path and falls back to the index in an ignored folder. - client: open() line races, events-socket resync, Changes refresh on edits, Cmd+B/J in the terminal, image blob URLs revoked, IME Enter, stale gutter base, tooltips escaped; settings saved as merged PATCHes of changed keys, tabs: roots pruned, layout flushed on close. - terminal: a capped REP leaves the cursor where the full count would. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Px3ERqxxjWWCw6ty21M5S
1 parent ac2f8e5 commit 474b7cc

22 files changed

Lines changed: 424 additions & 61 deletions

‎DESIGN.md‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -995,5 +995,16 @@ each finding below was reproduced before it was fixed.
995995
| If the new install could not be moved in and the old one not moved back, the staging dir (holding the only copy) was deleted. | It is kept, and the error names it. |
996996
| Relaunch after an update dropped `--host` and `--shell`. | Forwarded (`--port` is not: the old process still holds it). |
997997
| Release signing ran in a job whose third-party actions were pinned by mutable tags, and checkout left the job token in `.git/config` for the build. | Actions pinned to the commits their tags named (checkout v5.1.0, setup-bun v2.2.0, attest-build-provenance v4.2.2); `persist-credentials: false`. |
998-
| Not changed: signing still shares the build job (moving it to its own job is a workflow redesign); `codesign --verify` without a requirement only proves a consistent signature (releases are ad hoc signed); a Linux update drops a setuid `chrome-sandbox`; redirects of signed downloads are not host-checked (the signature covers them). | — |
998+
| Security (critical). `git diff` looked inside submodules (it runs `git status` in each), and a submodule's own config is never read by the overrides: its filter ran from the Changes view. | `diff.ignoreSubmodules=all` in `HARDEN` and `--ignore-submodules=all` on the diff. |
999+
| Security (critical). The driver listing was capped at 1 MiB and parsed even when cut: a config padded past the cap left the drivers after it armed. | 16 MiB, and a cut or failed listing fails closed: no git call runs. |
1000+
| Security. The overrides were cached until `.git/config` changed, but a config can include files the watcher does not see (a tracked file via `include.path`, `~/.gitconfig`): a driver added there later ran. | The drivers are listed again at the start of each operation (status, changes, blame, history, base, file list). |
1001+
| `diff.renameLimit=0` made status and Changes run into the 30 s timeout on large renames and come back as a clean tree; `i18n.logOutputEncoding=UTF-16` broke history, blame and the HEAD line; `log.showRoot=false` dropped the commit that added a file. | Rename limits of 1000, UTF-8 log output and `--root` forced; a timed-out diff is reported as truncated. |
1002+
| Search found nothing in a viewed folder that is itself git-ignored (git lists no files there). | An empty git list falls back to the file index. |
1003+
| A shell that could not start (deleted folder, bad `--shell`, out of fds) threw in the socket handler and exited the server; sessions were unlimited. | That terminal gets an exit notice and is closed; at most 64 live shells. |
1004+
| Deleting files rescanned the whole index per path: a 1 500-file `rm` froze the server for 20 s on a 200 000-file tree. | Only a path that was a directory is scanned for children. |
1005+
| A file written more often than every 200 ms (a log) held git status back for as long as it was written: every poke restarted the wait. | Throttled: a pending refresh keeps its time. |
1006+
| Regressions from the first pass: a capped REP left the cursor elsewhere; heavy-dir events restarted the index build; search reported a match under a symlink instead of its target; tooltips still showed raw names; two doc comments ended up on the wrong code. | REP runs the cap plus the remainder of one row's period; heavy dirs don't invalidate; a link whose target is also listed is skipped; tooltips escaped per line; comments moved. |
1007+
| Client: a second jump into a file still loading was undone by the first; reconnecting the events socket resynced nothing; Changes counts went stale on a second edit of a modified file; ⌘B/⌘J did nothing from the terminal on macOS; image tabs never revoked their blob: URLs; Enter confirming an IME composition ran the palette's top item; the change gutter kept an old HEAD base after a discarded fetch. | Each fixed; the reproductions were rerun in Chromium against the build. |
1008+
| Settings: each window PUT its whole snapshot, so two windows (or two folders) erased each other's keys, the snapshot grew forever (`tabs:<root>`), and over 64 KB the save on close was dropped (keepalive limit); a layout change in the last 200 ms was lost. | `PATCH /api/settings` with only the changed keys, merged on the server in the write queue; the 100 most recent `tabs:` roots are kept; layout flushes before the save on close. |
1009+
| Not changed: signing still shares the build job; the index caps file count, not path bytes or directories; the recursive watcher can use the account's whole inotify budget on huge trees; `/api/list` on a 1M-entry directory takes ~10 s (sort); a repository path containing a newline gets no git integration; OSC 8 links stop for good after 65 534 distinct targets (moving it to its own job is a workflow redesign); `codesign --verify` without a requirement only proves a consistent signature (releases are ad hoc signed); a Linux update drops a setuid `chrome-sandbox`; redirects of signed downloads are not host-checked (the signature covers them). | — |
9991010

‎src/client/app.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ class App {
104104

105105
async start() {
106106
await store.init();
107+
// A layout change in the last 200 ms before closing would miss the save on close otherwise.
108+
store.beforeSave(() => store.set("layout", { ...this.L, v: LAYOUT_VERSION }));
107109
const saved = store.get<Partial<Layout> & { v?: number }>("layout", {});
108110
// Older layouts carried pre-panel font sizes; let the new defaults apply.
109111
if (saved.v !== LAYOUT_VERSION) {
@@ -688,7 +690,7 @@ class App {
688690
this.lastStatus = s;
689691
// The text sits in its own span so a long path gets an ellipsis (flex items don't ellipsize bare text).
690692
const item = (text: string, title = "") => h("span", { class: "st-item", title }, h("span", { class: "st-text" }, text));
691-
const left: HTMLElement[] = [h("span", { class: "st-item st-root", title: this.info.root }, h("span", { class: "st-dot live" }), showHidden(this.info.name))];
693+
const left: HTMLElement[] = [h("span", { class: "st-item st-root", title: showHidden(this.info.root) }, h("span", { class: "st-dot live" }), showHidden(this.info.name))];
692694
const g = this.git.status;
693695
if (g.repo) {
694696
const name = g.branch ?? (g.oid ? g.oid.slice(0, 7) : "no commits");
@@ -698,7 +700,7 @@ class App {
698700
g.head ? `last commit: ${g.head.subject} (${g.head.author}, ${relativeTime(g.head.time)})` : "",
699701
].filter(Boolean).join("\n");
700702
const dirty = this.git.counts.changed + this.git.counts.untracked + this.git.counts.conflicts > 0;
701-
const branch = h("button", { class: "st-item st-git", title: tip, onclick: () => this.showChanges() });
703+
const branch = h("button", { class: "st-item st-git", title: tip.split("\n").map(showHidden).join("\n"), onclick: () => this.showChanges() });
702704
branch.innerHTML = `${icons.branch}<span>${esc(showHidden(name))}${dirty ? "*" : ""}</span>`;
703705
if (g.behind || g.ahead) {
704706
branch.insertAdjacentHTML("beforeend", `<span class="st-ab">${g.behind ? `↓${g.behind}` : ""}${g.ahead ? `↑${g.ahead}` : ""}</span>`);
@@ -782,7 +784,7 @@ class App {
782784
else if (mod && e.shiftKey && k === "e") {
783785
if (!this.L.sidebar) this.toggleSidebar(true);
784786
this.tree.focus();
785-
} else if (mod && k === "b" && !inTerm) this.toggleSidebar();
787+
} else if (mod && k === "b" && appKey) this.toggleSidebar();
786788
else if (mod && !e.shiftKey && k === "f" && !inTerm) this.editor.openFind();
787789
else if (e.ctrlKey && k === "g" && !inTerm) this.palette.open(":");
788790
else if (mod && k === "w" && !inTerm) this.editor.close();
@@ -807,7 +809,7 @@ class App {
807809
else if (mod && (k === "=" || k === "+") && appKey) this.zoom(1);
808810
else if (mod && k === "-" && appKey) this.zoom(-1);
809811
else if (mod && k === "0" && appKey) this.zoom(0);
810-
else if (mod && k === "j" && !inTerm) this.togglePanel();
812+
else if (mod && k === "j" && appKey) this.togglePanel();
811813
else if (helpKey(e, { isMac: IS_MAC }) && appKey) this.help.open();
812814
else if (e.key === "Escape" && this.help.isOpen) this.help.close();
813815
else if (e.key === "Escape" && this.palette.isOpen) this.palette.close();
@@ -827,7 +829,16 @@ class App {
827829
const ws = new WebSocket(wsUrl("/ws/events"));
828830
ws.onopen = () => {
829831
retry = 500;
830-
if (document.body.classList.contains("offline")) this.notify("Reconnected to codeviewer");
832+
if (document.body.classList.contains("offline")) {
833+
this.notify("Reconnected to codeviewer");
834+
// Events sent while disconnected are gone: refresh everything they would have touched.
835+
void api.gitStatus().then((s) => this.git.set(s), () => {});
836+
this.palette.invalidate();
837+
this.search.markStale([], true);
838+
void this.tree.reloadAll();
839+
this.editor.onChanged(this.editor.tabs.map((t) => t.path));
840+
this.changes.onGitChanged();
841+
}
831842
document.body.classList.remove("offline");
832843
};
833844
ws.onmessage = (ev) => {
@@ -837,6 +848,9 @@ class App {
837848
if (msg.t === "changed" && msg.paths) {
838849
if (msg.structural) this.palette.invalidate();
839850
this.search.markStale(msg.paths, msg.many);
851+
// A second edit to an already modified file changes no status, so no git event follows:
852+
// the Changes counts would stay stale (it debounces its own refresh).
853+
this.changes.onGitChanged();
840854
if (msg.many) {
841855
void this.tree.reloadAll();
842856
this.editor.onChanged(this.editor.tabs.map((t) => t.path));

‎src/client/codeview.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ export function markRanges(n: number, ranges: readonly MarkRange[]): Uint8Array
3636
return out;
3737
}
3838

39-
/** New scrollLeft that brings 0-based column `col` into view, or `scrollLeft` when it already is. */
4039
/**
4140
* Trojan Source: each bidi override/isolate is wrapped in a zero-width inline-block (`.bidi`, drawn
4241
* as a bar, the code point in its title). An atomic inline reorders nothing around it, so the line
@@ -47,6 +46,7 @@ export function markBidi(html: string): string {
4746
return html.replace(/[\u202a-\u202e\u2066-\u2069]/g, (c) => `<span class="bidi" title="U+${c.charCodeAt(0).toString(16).toUpperCase()}">${c}</span>`);
4847
}
4948

49+
/** New scrollLeft that brings 0-based column `col` into view, or `scrollLeft` when it already is. */
5050
export function revealCol(scrollLeft: number, clientWidth: number, gutterW: number, charW: number, col: number): number {
5151
const x = gutterW + Math.min(col, MAX_RENDER_COLS) * charW;
5252
if (x < scrollLeft + gutterW || x > scrollLeft + clientWidth - 40) return Math.max(0, x - clientWidth / 2);
@@ -149,6 +149,7 @@ export class CodeView {
149149
new ResizeObserver(() => this.schedule()).observe(this.scroller);
150150
this.findInput.addEventListener("input", () => this.runFind());
151151
this.findInput.addEventListener("keydown", (e) => {
152+
if (e.isComposing || e.keyCode === 229) return; // the IME's own Enter / Escape
152153
if (e.key === "Enter") {
153154
e.preventDefault();
154155
this.nextMatch(e.shiftKey ? -1 : 1);

‎src/client/editor.ts‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,21 @@ export class Editor {
133133
const i = this.active ? this.tabs.indexOf(this.active) + 1 : this.tabs.length;
134134
this.tabs.splice(i, 0, tab);
135135
}
136+
// load() applies the line. Set before the await: a second open() of the same file while this
137+
// one loads (another search result) replaces it, and must not be undone when this resumes.
138+
if (opts.line) tab.goto = { line: opts.line, col: opts.col, len: opts.len };
136139
this.activate(tab);
137140
await this.load(tab);
138141
} else {
139142
if (opts.pin) tab.pinned = true;
140143
this.activate(tab);
141144
if (opts.line && tab.md) await this.toSource(tab);
145+
if (opts.line && tab.code) {
146+
if (opts.col !== undefined && opts.len) tab.code.showRange(opts.line - 1, opts.col, opts.col + opts.len);
147+
else tab.code.revealLine(opts.line - 1);
148+
} else if (opts.line && !tab.md && !tab.image) tab.goto = { line: opts.line, col: opts.col, len: opts.len }; // still loading
142149
}
143-
if (opts.line && tab.code) {
144-
if (opts.col !== undefined && opts.len) tab.code.showRange(opts.line - 1, opts.col, opts.col + opts.len);
145-
else tab.code.revealLine(opts.line - 1);
146-
} else if (opts.line && !tab.md && !tab.image) tab.goto = { line: opts.line, col: opts.col, len: opts.len }; // still loading
147-
if (opts.focus !== false) tab.code?.focus();
150+
if (opts.focus !== false && this.active === tab) tab.code?.focus();
148151
this.render();
149152
}
150153

@@ -352,6 +355,7 @@ export class Editor {
352355
tab.md = undefined;
353356
}
354357
if (!plan.code) this.dropCode(tab);
358+
if (f.kind !== "image") this.dropImage(tab); // an image reload keeps its view (and zoom)
355359
if (f.kind === "text") {
356360
tab.eol = /\r\n/.test(f.text.slice(0, 10000)) ? "CRLF" : "LF";
357361
if (plan.view === "md") {
@@ -400,6 +404,7 @@ export class Editor {
400404
if (tab.image && reload) tab.image.reload(tab.mtime);
401405
else {
402406
this.dropCode(tab);
407+
this.dropImage(tab);
403408
tab.image = new ImageView(tab.path, tab.mtime, f.size, () => this.status());
404409
tab.el.replaceChildren(tab.image.el);
405410
if (tab.path.toLowerCase().endsWith(".svg")) tab.el.prepend(this.viewToggle(tab, "View source", icons.code));
@@ -447,7 +452,7 @@ export class Editor {
447452
tab.forcePreview = next.forcePreview; // an explicit request overrides the size gate
448453
} else tab.asSource = !tab.asSource; // SVG: image or source
449454
this.dropCode(tab);
450-
tab.image = undefined;
455+
this.dropImage(tab);
451456
tab.md?.dispose();
452457
tab.md = undefined;
453458
void this.load(tab);
@@ -523,9 +528,16 @@ export class Editor {
523528
tab.code = undefined;
524529
}
525530

531+
/** Releases an image view (its blob: URL holds the file's bytes until revoked). */
532+
private dropImage(tab: Tab) {
533+
tab.image?.dispose();
534+
tab.image = undefined;
535+
}
536+
526537
private disposeTab(tab: Tab) {
527538
tab.gen++; // a load still in flight must not build a view for a closed tab
528539
tab.code?.dispose();
540+
tab.image?.dispose();
529541
tab.diff?.dispose();
530542
tab.md?.dispose();
531543
tab.el.remove();
@@ -570,7 +582,12 @@ export class Editor {
570582
else if (g?.xy[0] === "A") code.setGit("", false); // staged new file: HEAD has nothing
571583
else {
572584
const base = await api.gitBase(tab.path);
573-
if (gen !== tab.gen || tab.code !== code) return;
585+
if (gen !== tab.gen || tab.code !== code) {
586+
// Discarded (a reload overtook it): forget the key, or the reload's own refresh skips
587+
// and the gutter keeps the old HEAD base.
588+
if (tab.gitKey === key) tab.gitKey = "";
589+
return;
590+
}
574591
code.setGit(base, false);
575592
}
576593
if (tab === this.active) this.status();
@@ -708,9 +725,9 @@ export class Editor {
708725
const g = t.isDiff && t.file ? { letter: letterOf(t.file.status), label: labelOf(t.file) } : this.git?.repo ? this.git.file(t.path) : null;
709726
if (g) cls.push(`git-${g.letter}`);
710727
t.tabEl.className = cls.join(" ");
711-
t.tabEl.title = t.commit
728+
t.tabEl.title = showHidden(t.commit
712729
? `${t.path} · ${t.commit.oid.slice(0, 7)} ${t.commit.subject} (${t.commit.author}, ${relativeTime(t.commit.time)})`
713-
: g ? `${t.path} · ${g.label}${t.isDiff ? ` · diff ${this.diffContext.vs}` : ""}` : t.path;
730+
: g ? `${t.path} · ${g.label}${t.isDiff ? ` · diff ${this.diffContext.vs}` : ""}` : t.path);
714731
t.tabEl.setAttribute("aria-selected", String(t === this.active));
715732
t.tabEl.innerHTML = `${fileIcon(t.path)}<span class="tab-name">${esc(showHidden(basename(t.path)))}</span>${hint(t.path) ? `<span class="tab-dir">${esc(showHidden(hint(t.path)))}</span>` : ""}${t.commit ? `<span class="tab-kind oid">${t.commit.oid.slice(0, 7)}</span>` : t.isDiff ? '<span class="tab-kind">diff</span>' : ""}<button class="tab-close" title="Close (${MOD}W)">${icons.close}</button>`;
716733
t.tabEl.querySelector(".tab-close")!.addEventListener("click", (e) => {
@@ -786,10 +803,22 @@ class ImageView {
786803
},
787804
{ passive: false },
788805
);
789-
new ResizeObserver(() => this.mode === "fit" && this.apply()).observe(this.stage);
806+
this.ro = new ResizeObserver(() => this.mode === "fit" && this.apply());
807+
this.ro.observe(this.stage);
790808
this.reload(mtime);
791809
}
792810

811+
private ro: ResizeObserver;
812+
private disposed = false;
813+
814+
/** Releases the image's bytes (the blob: URL holds them until revoked) and the observer. */
815+
dispose() {
816+
this.disposed = true;
817+
this.ro.disconnect();
818+
if (this.blobUrl) URL.revokeObjectURL(this.blobUrl);
819+
this.blobUrl = "";
820+
}
821+
793822
private blobUrl = "";
794823
private failed = false;
795824

@@ -804,6 +833,8 @@ class ImageView {
804833
reload(_mtime: number) {
805834
void api.rawBlobUrl(this.path).then(
806835
(u) => {
836+
// Closed while it loaded: nothing will show it.
837+
if (this.disposed) return URL.revokeObjectURL(u);
807838
if (this.blobUrl) URL.revokeObjectURL(this.blobUrl);
808839
this.blobUrl = u;
809840
this.img.src = u;

‎src/client/palette.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ export class Palette {
153153
});
154154
this.input.addEventListener("input", () => this.update());
155155
this.input.addEventListener("keydown", (e) => {
156+
// Enter or Escape that confirms or cancels an IME composition belongs to the IME.
157+
if (e.isComposing || e.keyCode === 229) return;
156158
if (e.key === "ArrowDown" || (e.ctrlKey && e.key === "n")) this.move(1);
157159
else if (e.key === "ArrowUp" || (e.ctrlKey && e.key === "p")) this.move(-1);
158160
else if (e.key === "Enter") this.accept(this.sel);

‎src/client/term/terminal.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,16 @@ const enum S {
204204

205205
export const MAX_SCROLLBACK = 10_000;
206206

207-
/**
208-
* Headless VT/xterm emulator: parser + screen model. Rendering lives in renderer.ts.
209-
* Coordinates are 0-based internally.
210-
*/
211207
/** Most UTF-16 units one cell's cluster holds (the longest emoji ZWJ sequences are about 35). */
212208
const MAX_CLUSTER = 64;
213209
/** Longest OSC 8 target kept as a link. */
214210
const MAX_LINK_URI = 2048;
215211

212+
/**
213+
* Headless VT/xterm emulator: parser + screen model. Rendering lives in renderer.ts.
214+
* Coordinates are 0-based internally.
215+
*/
216+
216217
export class Terminal {
217218
cols: number;
218219
rows: number;
@@ -1158,7 +1159,12 @@ export class Terminal {
11581159
const cp = this.lastChar.codePointAt(0)!;
11591160
const ch = String.fromCodePoint(cp);
11601161
this.lastChar = ch;
1161-
for (let i = Math.min(this.p(0, 1), 65535, this.cols * this.rows); i > 0; i--) this.print(cp, ch);
1162+
// Past a screenful the output repeats with the period of one row (half a row of wide
1163+
// characters): the remainder keeps the cursor where the full count would leave it.
1164+
const n = Math.min(this.p(0, 1), 65535);
1165+
const cap = this.cols * this.rows;
1166+
const period = charWidth(cp) === 2 ? Math.max(1, Math.floor(this.cols / 2)) : this.cols;
1167+
for (let i = n > cap ? cap + ((n - cap) % period) : n; i > 0; i--) this.print(cp, ch);
11621168
return;
11631169
}
11641170
case "c":

‎src/client/terminals.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ export class TerminalPanel {
266266
for (const [i, s] of this.sessions.entries()) {
267267
s.tabEl.className = `term-tab${s === this.active ? " active" : ""}${s.exited ? " exited" : ""}`;
268268
s.tabEl.innerHTML = `<span class="term-dot"></span><span class="tab-name">${esc(showHidden(s.title || `Terminal ${i + 1}`))}</span><button class="tab-close" title="Kill terminal">${icons.close}</button>`;
269-
s.tabEl.title = s.title;
269+
s.tabEl.title = showHidden(s.title);
270270
s.tabEl.querySelector(".tab-close")!.addEventListener("click", (e) => {
271271
e.stopPropagation();
272272
this.close(s);

0 commit comments

Comments
 (0)