Merge origin/main (v1.64.1.0) into garrytan/time-attack-fork-review

The code-smell wave refactored the tunnel start into the staged
startTunnel helper; the merge adopts that structure and threads this
branch's truthful consent strings through its consent parameter (the
receipt now names the isPairAgentEnabled gate at both call sites). The
eng-review PTY warmup takes both improvements: main's resolveClaudeBinary
fallback and this branch's resolveEvalModel kind. Resolver imports
union; carve budgets take the larger of both waves' measured values
(parity suite green on the merged tree); conflicted generated SKILL.md
files regenerated from resolved sources. VERSION stays 1.65.0.0 over
main's 1.64.1.0; CHANGELOG stacks 1.65.0.0 > 1.64.1.0 > 1.64.0.0.

lib/fs-atomic.ts

@@ -0,0 +1,76 @@
+/**
+ * Atomic file writes — the ONE implementation of tmp-write-then-rename.
+ *
+ * Before this module, the pattern was reimplemented ~20 times across lib/,
+ * bin/, and browse/src with three different tmp-suffix conventions — one of
+ * which (a bare `.tmp`) carries a real collision race that browse's
+ * server.ts documented after hitting it in production: two writers (batch
+ * subcommands, /tunnel/start handlers, or any combination) collide on the
+ * rename when the tmp filename is deterministic. The suffix here includes
+ * pid AND a random component so concurrent writers in the SAME process
+ * (async interleavings) can't collide either.
+ *
+ * Contract:
+ * - atomicWriteSync ALWAYS throws on failure, after best-effort tmp cleanup.
+ *   Callers own the error. Use it everywhere except shutdown paths.
+ * - atomicWriteQuiet swallows everything (returns false on failure). ONLY
+ *   for shutdown/emergency-cleanup paths where a throw would abort the rest
+ *   of cleanup — same philosophy as browse's safeUnlinkQuiet.
+ * - `mode` applies to the tmp file at creation (0600 for sensitive state),
+ *   so the final file never exists with looser permissions.
+ * - The tmp file is created in the target's directory (same filesystem, so
+ *   rename stays atomic). Parent dirs are NOT created — callers that need
+ *   mkdir own that decision (and its mode).
+ */
+import * as fs from 'fs';
+import * as crypto from 'crypto';
+
+export interface AtomicWriteOpts {
+  /** File mode for the tmp file at creation (e.g. 0o600). Default: umask. */
+  mode?: number;
+}
+
+function tmpPathFor(target: string): string {
+  return `${target}.tmp.${process.pid}.${crypto.randomBytes(4).toString('hex')}`;
+}
+
+/** Atomic write. Throws on failure (after best-effort tmp cleanup). */
+export function atomicWriteSync(
+  target: string,
+  data: string | NodeJS.ArrayBufferView,
+  opts: AtomicWriteOpts = {},
+): void {
+  const tmp = tmpPathFor(target);
+  try {
+    if (opts.mode !== undefined) {
+      fs.writeFileSync(tmp, data, { mode: opts.mode });
+    } else {
+      fs.writeFileSync(tmp, data);
+    }
+    fs.renameSync(tmp, target);
+  } catch (err) {
+    try {
+      fs.unlinkSync(tmp);
+    } catch {
+      // Best-effort cleanup; the original error is the one that matters.
+    }
+    throw err;
+  }
+}
+
+/**
+ * Atomic write that swallows all errors. Returns true on success.
+ * ONLY for shutdown/emergency paths — a throw there aborts remaining cleanup.
+ */
+export function atomicWriteQuiet(
+  target: string,
+  data: string | NodeJS.ArrayBufferView,
+  opts: AtomicWriteOpts = {},
+): boolean {
+  try {
+    atomicWriteSync(target, data, opts);
+    return true;
+  } catch {
+    return false;
+  }
+}

lib/gbrain-local-status.ts

@@ -41,10 +41,9 @@ import {
   existsSync,
   mkdirSync,
   readFileSync,
-  renameSync,
   statSync,
-  writeFileSync,
 } from "fs";
+import { atomicWriteSync } from "./fs-atomic";
 import { homedir } from "os";
 import { dirname, join } from "path";
 import { buildGbrainEnv, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec";
@@ -254,9 +253,7 @@ function writeCache(status: LocalEngineStatus, key: CacheEntry["key"]): void {
   };
   try {
     mkdirSync(dirname(cacheFilePath()), { recursive: true });
-    const tmp = cacheFilePath() + ".tmp." + process.pid;
-    writeFileSync(tmp, JSON.stringify(entry, null, 2), "utf-8");
-    renameSync(tmp, cacheFilePath());
+    atomicWriteSync(cacheFilePath(), JSON.stringify(entry, null, 2));
   } catch {
     // Cache write failure is non-fatal — we re-probe next call.
   }

lib/gstack-decision.ts

@@ -16,7 +16,8 @@
 import { join } from "path";
 import { homedir } from "os";
 import { randomUUID } from "crypto";
-import { writeFileSync, renameSync, existsSync, readFileSync, appendFileSync, statSync, openSync, closeSync, unlinkSync } from "fs";
+import { existsSync, readFileSync, appendFileSync, statSync, openSync, closeSync, unlinkSync } from "fs";
+import { atomicWriteSync } from "./fs-atomic";
 import { appendJsonl, readJsonl, hasInjection } from "./jsonl-store";
 import { scan } from "./redact-engine";
 
@@ -224,9 +225,7 @@ export function readEvents(paths: DecisionPaths): DecisionEvent[] {
  * O(active), not O(history).
  */
 export function writeSnapshot(paths: DecisionPaths, active: ActiveDecision[]): void {
-  const tmp = `${paths.snapshot}.tmp.${process.pid}`;
-  writeFileSync(tmp, JSON.stringify(active), "utf-8");
-  renameSync(tmp, paths.snapshot);
+  atomicWriteSync(paths.snapshot, JSON.stringify(active));
 }
 
 /** Read the bounded active snapshot. Returns [] if missing/corrupt (caller may rebuild). */
@@ -308,9 +307,7 @@ export function compact(paths: DecisionPaths): CompactResult {
       appendFileSync(paths.archive, superseded.map((e) => JSON.stringify(e)).join("\n") + "\n", "utf-8");
     }
 
-    const tmp = `${paths.log}.tmp.${process.pid}`;
-    writeFileSync(tmp, active.map((d) => JSON.stringify(d)).join("\n") + (active.length ? "\n" : ""), "utf-8");
-    renameSync(tmp, paths.log);
+    atomicWriteSync(paths.log, active.map((d) => JSON.stringify(d)).join("\n") + (active.length ? "\n" : ""));
     writeSnapshot(paths, active);
 
     return { activeCount: active.length, archivedCount: superseded.length, expungedCount: redactedIds.size };

lib/gstack-memory-helpers.ts

@@ -17,7 +17,8 @@
  * helper warns once and returns an empty findings list — fail-safe defaults.
  */
 
-import { existsSync, readFileSync, writeFileSync, mkdirSync, statSync, appendFileSync } from "fs";
+import { existsSync, readFileSync, writeFileSync, mkdirSync, statSync } from "fs";
+import { appendJsonl } from "./jsonl-store";
 import { dirname, join } from "path";
 import { execFileSync } from "child_process";
 import { homedir } from "os";
@@ -268,11 +269,7 @@ function logGbrainError(kind: string, detail: string): void {
   try {
     const path = errorLogPath();
     mkdirSync(dirname(path), { recursive: true });
-    appendFileSync(
-      path,
-      JSON.stringify({ ts: new Date().toISOString(), kind, detail: detail.slice(0, 500) }) + "\n",
-      "utf-8"
-    );
+    appendJsonl(path, { ts: new Date().toISOString(), kind, detail: detail.slice(0, 500) });
   } catch { /* logging is best-effort */ }
 }
 
@@ -505,7 +502,7 @@ function logErrorContext(entry: ErrorContextEntry): void {
   try {
     const path = errorLogPath();
     mkdirSync(dirname(path), { recursive: true });
-    appendFileSync(path, JSON.stringify(entry) + "\n", "utf-8");
+    appendJsonl(path, entry);
   } catch {
     // Logging failure is non-fatal — never block the op.
   }

lib/jsonl-store.ts

@@ -1,17 +1,22 @@
 /**
- * jsonl-store — shared, audited plumbing for gstack's append-only JSONL stores.
+ * jsonl-store — shared plumbing for gstack's append-only JSONL stores in
+ * lib/ and bin/. (browse/src keeps its own appenders by design — the
+ * compiled-binary surface has different logging semantics and its own
+ * secure-append helper.)
  *
- * Single source of truth for the three things every JSONL store must get right:
- *   1. Injection sanitization (the prompt-injection patterns that must NOT survive
- *      into agent context when a record is later resurfaced).
+ * The three things a JSONL store must get right:
+ *   1. Injection screening — SEE THE CONTRACT BELOW: appendJsonl does NOT
+ *      screen; callers that store free text MUST pre-check with
+ *      hasInjection()/firstInjectionMatch() and reject. Enforcing callers
+ *      today: bin/gstack-learnings-log, bin/gstack-decision-log (via
+ *      lib/gstack-decision.ts), bin/gstack-question-log.
  *   2. Atomic single-line append (concurrent agents must not corrupt the file).
- *   3. Tolerant read (a partially-written tail or one corrupt line must not take
- *      down the whole read).
+ *   3. Tolerant read (a partially-written tail or one corrupt line must not
+ *      take down the whole read).
  *
- * Extracted from `bin/gstack-learnings-log` (D2A) so `gstack-learnings-*` and the
- * new `gstack-decision-*` bins share ONE audited path — a new injection pattern or
- * a write-atomicity fix lands in both at once, never drifts. Per the
- * `squash-with-regen` / DRY discipline + the eng-review D2A decision.
+ * Extracted from `bin/gstack-learnings-log` (D2A) so the learnings/decision/
+ * question stores share ONE audited path — a new injection pattern or a
+ * write-atomicity fix lands in all at once.
  */
 
 import { appendFileSync, readFileSync, existsSync } from "fs";
@@ -60,12 +65,19 @@ export function firstInjectionMatch(text: string): RegExp | null {
  * Caveat: a record larger than PIPE_BUF loses the cross-process atomicity guarantee.
  * Keep records line-bounded; very large free-text should be truncated by the caller.
  */
-export function appendJsonl(path: string, obj: unknown): void {
+export function appendJsonl(path: string, obj: unknown, opts: { mode?: number } = {}): void {
   const line = JSON.stringify(obj);
   if (line.includes("\n")) {
     throw new Error("jsonl-store: record serialized to multiple lines (embedded newline)");
   }
-  appendFileSync(path, line + "\n", { encoding: "utf-8" });
+  // `mode` applies only when the append CREATES the file (POSIX open(2)
+  // semantics) — pass 0o600 for stores holding sensitive content so the
+  // file never exists world-readable.
+  if (opts.mode !== undefined) {
+    appendFileSync(path, line + "\n", { encoding: "utf-8", mode: opts.mode });
+  } else {
+    appendFileSync(path, line + "\n", { encoding: "utf-8" });
+  }
 }
 
 /**

lib/redact-audit-log.ts

@@ -18,6 +18,7 @@ import * as fs from "fs";
 import * as os from "os";
 import * as path from "path";
 import { createHash } from "crypto";
+import { appendJsonl } from "./jsonl-store";
 
 export interface SemanticReviewEntry {
   ts: string;
@@ -43,7 +44,9 @@ export function appendSemanticReview(entry: SemanticReviewEntry): void {
     const dir = securityDir();
     fs.mkdirSync(dir, { recursive: true });
     const file = path.join(dir, "semantic-reviews.jsonl");
-    fs.appendFileSync(file, JSON.stringify(entry) + "\n");
+    // 0600 at create via appendJsonl's mode opt; the chmod backstop covers
+    // files created looser by pre-mode versions.
+    appendJsonl(file, entry, { mode: 0o600 });
     try {
       fs.chmodSync(file, 0o600);
     } catch {

lib/worktree.ts

@@ -13,6 +13,7 @@ import { spawnSync } from 'child_process';
 import * as crypto from 'crypto';
 import * as fs from 'fs';
 import * as path from 'path';
+import { atomicWriteSync } from './fs-atomic';
 import * as os from 'os';
 
 // --- Interfaces ---
@@ -84,9 +85,9 @@ function loadDedupIndex(): DedupIndex {
 function saveDedupIndex(index: DedupIndex): void {
   const dir = path.dirname(getDedupPath());
   fs.mkdirSync(dir, { recursive: true });
-  const tmp = getDedupPath() + '.tmp';
-  fs.writeFileSync(tmp, JSON.stringify(index, null, 2));
-  fs.renameSync(tmp, getDedupPath());
+  // Was a bare '.tmp' suffix — the deterministic-tmp collision race the
+  // shared helper exists to prevent.
+  atomicWriteSync(getDedupPath(), JSON.stringify(index, null, 2));
 }
 
 // --- WorktreeManager ---