From 1bca95029fc76f7597fccf3658ed20b844318bb2 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 01:26:23 +0000 Subject: [PATCH] fix(clean): every change rechecks the verified folder and the measured file; full writes; EPIPE ends output quietly Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/capy-gsta226-e2-gstack-clean.md | 4 +- lib/repo-state-clean.ts | 59 ++++++++++++++++----- test/gstack-clean.test.ts | 11 ++++ 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/changelog.d/capy-gsta226-e2-gstack-clean.md b/changelog.d/capy-gsta226-e2-gstack-clean.md index 757d4d70c..edfa9f6f7 100644 --- a/changelog.d/capy-gsta226-e2-gstack-clean.md +++ b/changelog.d/capy-gsta226-e2-gstack-clean.md @@ -2,9 +2,9 @@ ### Added - **`bin/gstack-clean`** (rules in `lib/repo-state-clean.ts`): cleans `/.gstack` only. It removes top-level `tmp/codex-*` files older than 7 days, cuts `browse-console.log`, `browse-network.log`, `browse-dialog.log`, `browse-daemon.log` and `browse-startup-error.log` to their last 256 KB once they pass 1 MB (in place, starting after a line break, so a running writer keeps appending), and with `--audit-days N` drops `browse-audit.jsonl` entries older than N days while keeping every line without a readable timestamp. Everything else, including `browse.json`, `session-state.json`, `chromium-profile/`, `browser-skills/`, `browse-states/` and the other `tmp/` entries, is never touched. -- **Safety:** it refuses (exit 3) outside git, when `.gstack` is a symlink, when `BROWSE_STATE_FILE` names another folder, and when the folder is, holds or sits inside the gstack state root. It skips symlinks, logs with more than one hard link, files it cannot read and files changed in the last hour, skips browse files while the browse daemon runs and `tmp/` while codex runs, and skips a group when it cannot tell. `--json` prints the same result as one versioned object. +- **Safety:** it refuses (exit 3) outside git, when `.gstack` is a symlink, when `BROWSE_STATE_FILE` names another folder, and when the folder is, holds or sits inside the gstack state root. It skips symlinks, logs with more than one hard link, files it cannot read and files changed in the last hour, skips browse files while the browse daemon runs and `tmp/` while codex runs, and skips a group when it cannot tell. Right before each change it checks that the folder is still the one it verified and that the file still has the size, time and inode it measured, and it stops that change if either moved. `--json` prints the same result as one versioned object. - Result codes `GSTACK_CLEAN_USAGE`, `GSTACK_CLEAN_TARGET_REFUSED`, `GSTACK_CLEAN_PARTIAL` and `GSTACK_CLEAN_INTERNAL`, each with a docs/troubleshooting.md entry; BROWSER.md names the command where it lists the log files. ### Tests -- `test/gstack-clean.test.ts`: the dry run changes no byte or mtime, `--apply` removes exactly the allowlist, protected files survive at any age, every refusal touches nothing, links out of the folder, hard-linked and unreadable logs and a symlinked `tmp/` are left alone, a log with no line break in its tail and a malformed audit file are left alone, an unlistable `tmp/` skips only its group, `--json` reaches a pipe whole with hundreds of actions, live and unknown liveness skip their group, the cut keeps the inode and a line boundary, audit trimming keeps the right lines byte for byte, exit codes 0, 1, 2 and 3, and the `--json` shape. +- `test/gstack-clean.test.ts`: the dry run changes no byte or mtime, `--apply` removes exactly the allowlist, protected files survive at any age, every refusal touches nothing, links out of the folder, hard-linked and unreadable logs and a symlinked `tmp/` are left alone, a log with no line break in its tail and a malformed audit file are left alone, an unlistable `tmp/` skips only its group, `--json` reaches a pipe whole with hundreds of actions and a reader that closes early is not an error, live and unknown liveness skip their group, the cut keeps the inode and a line boundary, audit trimming keeps the right lines byte for byte, exit codes 0, 1, 2 and 3, and the `--json` shape. - `test/gstack-clean-producers.test.ts`: every tracked producer of a `.gstack/tmp/` entry is classified as scratch or kept, and any new `codex-*` name must be added to the scratch list on purpose. diff --git a/lib/repo-state-clean.ts b/lib/repo-state-clean.ts index e30336f75..dfc866413 100644 --- a/lib/repo-state-clean.ts +++ b/lib/repo-state-clean.ts @@ -71,7 +71,8 @@ type Group = 'tmp' | 'browse'; type Kind = 'remove' | 'cut' | 'rotate'; export interface CleanOptions { apply: boolean; json: boolean; auditDays: number | null } interface Candidate { file: string; group: Group; kind: Kind } -interface Action extends Candidate { bytes: number; reason: string; keep?: Buffer } +interface Seen { dev: number; ino: number; size: number; mtimeMs: number } +interface Action extends Candidate { bytes: number; reason: string; keep?: Buffer; seen: Seen } interface Skip { file: string; group: Group; reason: string } interface Done { action: Action; errno?: string } export interface CleanTarget { target: string; chosenBy: string } @@ -211,7 +212,8 @@ function evaluate(c: Candidate, dir: string, busy: string | null, opts: CleanOpt if (c.kind === 'cut' && st.size <= LOG_CUT_OVER_BYTES) return null; if (now - st.mtimeMs < RECENT_MS) return { ...c, reason: 'modified within the last hour' }; if (busy) return { ...c, reason: busy }; - if (c.kind === 'remove') return { ...c, bytes: st.size, reason: 'codex scratch older than 7 days' }; + const seen: Seen = { dev: st.dev, ino: st.ino, size: st.size, mtimeMs: st.mtimeMs }; + if (c.kind === 'remove') return { ...c, seen, bytes: st.size, reason: 'codex scratch older than 7 days' }; if (st.nlink !== 1) return { ...c, reason: `has ${st.nlink} hard links, so cutting it would change a file outside this folder; left alone` }; let content: Buffer; try { content = c.kind === 'cut' ? readTail(c.file, st.size) : fs.readFileSync(c.file); } catch (e: any) { @@ -222,12 +224,15 @@ function evaluate(c: Candidate, dir: string, busy: string | null, opts: CleanOpt const at = tail.indexOf(0x0a); if (at < 0) return { ...c, reason: 'no line break in its last 256 KB; left alone' }; const keep = tail.subarray(at + 1); - return { ...c, keep, bytes: st.size - keep.length, reason: 'over 1 MB; keeps the last 256 KB from a line start' }; + return { ...c, seen, keep, bytes: st.size - keep.length, reason: 'over 1 MB; keeps the last 256 KB from a line start' }; + } + let keep: Buffer | string; + try { keep = auditKeep(content, now - opts.auditDays! * DAY_MS); } catch (e: any) { + return { ...c, reason: `cannot read (${e?.code ?? e?.name ?? 'error'}); left alone` }; } - const keep = auditKeep(content, now - opts.auditDays! * DAY_MS); if (typeof keep === 'string') return { ...c, reason: keep }; if (keep.length === st.size) return null; - return { ...c, keep, bytes: st.size - keep.length, reason: `audit entries older than ${opts.auditDays} days` }; + return { ...c, seen, keep, bytes: st.size - keep.length, reason: `audit entries older than ${opts.auditDays} days` }; } function candidates(target: string, opts: CleanOptions): { list: Candidate[]; skips: Skip[] } { @@ -270,17 +275,35 @@ export function planClean(target: string, opts: CleanOptions, now = Date.now()): return { actions, skips }; } -function rewriteInPlace(file: string, keep: Buffer): void { - const before = fs.lstatSync(file); +const changed = () => Object.assign(new Error('the file changed after it was checked; rerunning is safe'), { code: 'ECHANGED' }); +const same = (st: fs.Stats, seen: Seen) => st.dev === seen.dev && st.ino === seen.ino && st.size === seen.size && st.mtimeMs === seen.mtimeMs; + +function writeFully(fd: number, buf: Buffer, position: number): void { + for (let off = 0; off < buf.length;) { + const n = fs.writeSync(fd, buf, off, buf.length - off, position + off); + if (n <= 0) throw Object.assign(new Error('short write'), { code: 'EIO' }); + off += n; + } +} + +/** The parent folder is still the verified real folder, so a swapped link cannot redirect the change. */ +function parentIntact(file: string, dir: string): boolean { + const parent = path.dirname(file); + try { return !fs.lstatSync(parent).isSymbolicLink() && fs.realpathSync(parent) === dir; } catch { return false; } +} + +/** A cut or trim keeps the inode, so O_APPEND writers go on; refused unless the file is exactly what evaluate saw. */ +function cutInPlace(file: string, keep: Buffer, seen: Seen): void { const fd = fs.openSync(file, fs.constants.O_RDWR | (fs.constants.O_NOFOLLOW ?? 0)); try { const st = fs.fstatSync(fd); - if (st.ino !== before.ino || st.dev !== before.dev || st.nlink !== 1) throw Object.assign(new Error('the file changed before it could be cut'), { code: 'ECHANGED' }); - fs.writeSync(fd, keep, 0, keep.length, 0); + if (st.nlink !== 1 || !same(st, seen)) throw changed(); + writeFully(fd, keep, 0); fs.ftruncateSync(fd, keep.length); } finally { fs.closeSync(fd); } } + /** Re-checks each planned action against its file and its group's liveness right before the change. */ export function applyClean(target: string, planned: Action[], opts: CleanOptions): { done: Done[]; skips: Skip[] } { const done: Done[] = []; @@ -298,8 +321,11 @@ export function applyClean(target: string, planned: Action[], opts: CleanOptions if (!fresh) continue; if (!('bytes' in fresh)) { skips.push(fresh); continue; } try { - if (fresh.kind === 'remove') fs.unlinkSync(fresh.file); - else rewriteInPlace(fresh.file, fresh.keep!); + if (!parentIntact(fresh.file, dirOf(fresh, target))) throw changed(); + if (fresh.kind === 'remove') { + if (!same(fs.lstatSync(fresh.file), fresh.seen)) throw changed(); + fs.unlinkSync(fresh.file); + } else cutInPlace(fresh.file, fresh.keep!, fresh.seen); done.push({ action: fresh }); } catch (e: any) { done.push({ action: fresh, errno: e?.code ?? 'UNKNOWN' }); @@ -367,7 +393,7 @@ function run(argv: string[], env: NodeJS.ProcessEnv, cwd: string, io: Io): numbe return 3; } if ('missing' in resolved) { - if (opts.json) io.out(JSON.stringify({ schema_version: CLEAN_SCHEMA_VERSION, tool: 'gstack-clean', mode: opts.apply ? 'apply' : 'dry-run', status: 'nothing_to_do', target: resolved.missing, actions: [], skips: [], total: { actions: 0, bytes: 0, failed: 0, skipped: 0 }, error: null, exit_code: 0 }, null, 2)); + if (opts.json) io.out(JSON.stringify({ schema_version: CLEAN_SCHEMA_VERSION, tool: 'gstack-clean', mode: opts.apply ? 'apply' : 'dry-run', status: 'nothing_to_do', target: resolved.missing, chosen_by: 'the .gstack folder at the git toplevel (missing)', audit_days: opts.auditDays, actions: [], skips: [], total: { actions: 0, bytes: 0, failed: 0, skipped: 0 }, error: null, exit_code: 0 }, null, 2)); else io.out(`nothing to clean: ${resolved.missing} does not exist`); return 0; } @@ -378,10 +404,15 @@ function run(argv: string[], env: NodeJS.ProcessEnv, cwd: string, io: Io): numbe } /** Synchronous, so a pipe gets every byte before the bin calls process.exit. */ +/** A reader that went away (EPIPE) ends the output quietly; the exit code still reports what was done. */ function writeAll(fd: number, text: string): void { const buf = Buffer.from(text, 'utf8'); for (let off = 0; off < buf.length;) { - try { off += fs.writeSync(fd, buf, off); } catch (e: any) { if (e?.code !== 'EAGAIN') throw e; } + try { off += fs.writeSync(fd, buf, off); } catch (e: any) { + if (e?.code === 'EPIPE') return; + if (e?.code !== 'EAGAIN') throw e; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10); + } } } @@ -392,7 +423,7 @@ export function main(argv: string[], env: NodeJS.ProcessEnv = process.env, cwd = return run(argv, env, cwd, io); } catch (e) { const err = e instanceof Error ? e : new Error(String(e)); - io.err(resultLine('gstack-clean', 'GSTACK_CLEAN_INTERNAL', `${err.name}: ${err.message}`.slice(0, 300))); + try { io.err(resultLine('gstack-clean', 'GSTACK_CLEAN_INTERNAL', `${err.name}: ${err.message}`.slice(0, 300))); } catch { /* stderr is gone too */ } return 1; } } diff --git a/test/gstack-clean.test.ts b/test/gstack-clean.test.ts index 73972d5b5..9907dfc51 100644 --- a/test/gstack-clean.test.ts +++ b/test/gstack-clean.test.ts @@ -453,6 +453,17 @@ describe('gstack-clean', () => { } }); + test('a reader that closes early does not turn a finished run into an internal error', () => { + const f = fixture(); + for (let i = 0; i < 300; i++) put(path.join(f.gs, 'tmp', 'codex-err-' + String(i).padStart(6, '0')), 'x', 30 * DAY); + const env: Record = { PATH: quietPsPath(f), HOME: f.home, GSTACK_HOME: f.state }; + const cmd = [process.execPath, ...bunIsolationArgs(), BIN, '--apply', '--json'].map(a => "'" + a.replace(/'/g, "'\\''") + "'").join(' ') + " 2>'" + path.join(f.root, 'err.txt') + "' | head -c 1 >/dev/null"; + const r = spawnSync('bash', ['-c', cmd], { cwd: f.repo, env, encoding: 'utf8', timeout: 30_000 }); + expect(r.status).toBe(0); + expect(fs.readFileSync(path.join(f.root, 'err.txt'), 'utf8')).not.toContain('GSTACK_CLEAN_INTERNAL'); + expect(fs.readdirSync(path.join(f.gs, 'tmp')).filter(n => n.startsWith('codex-'))).toEqual([]); + }); + test('--json reaches a pipe whole with hundreds of actions', () => { const f = fixture(); for (let i = 0; i < 300; i++) put(path.join(f.gs, 'tmp', 'codex-err-' + String(i).padStart(6, '0')), 'x', 30 * DAY);