From e530849b0f63ea940c7145601f3c6b2e7e589fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 20 Apr 2026 12:33:45 -0400 Subject: [PATCH] Review iteration: warn on gitSha read failure, WAL invariant comment, C6.2 wave-3 coordination gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - backup.ts: log.warn when `git rev-parse HEAD` fails so sandbox/CI runs with no git show diagnosably instead of silently emitting gitSha:null (Karpathy review ask). - backup.ts: expand comment on VACUUM INTO to pin the WAL-mode invariant — future readers must not "helpfully" add a wal_checkpoint call (Karpathy review ask). - C6.2 proposal: add hard coordination gate before wave 3 because commit 0db6e42 already demonstrated the race hazard when two agents touch scripts/ / connector-definitions/ in the same window (Linus review ask). --- docs/proposals/C6.2-error-taxonomy-phase2.md | 17 +++++++++++++++++ src/main/database/backup.ts | 12 +++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/proposals/C6.2-error-taxonomy-phase2.md b/docs/proposals/C6.2-error-taxonomy-phase2.md index 19d25003..81cf1cbc 100644 --- a/docs/proposals/C6.2-error-taxonomy-phase2.md +++ b/docs/proposals/C6.2-error-taxonomy-phase2.md @@ -93,6 +93,23 @@ Depends on Wave 1 only insofar as `registerHandle` wrapper lives in ### Wave 3 — database + credentials + browser + leftovers (≈58 sites) +**⚠️ Coordination gate (hard)**: Wave 3 touches `src/main/database/**` +(Linus's domain — backup hook, migrations, state-machine work) and +`src/main/connector-definitions/**` (which was rehomed into a subdirectory +during C6.1). Before starting Wave 3 the executor MUST: +1. Check `git log --oneline -20` for any pending database / backup / + migration commits from other streams. +2. Confirm B2 (tasks state-machine refactor) is either landed or + pre-gated — migrating database catches while B2 is mid-flight + produces diff storms that lose both sides' intent. +3. Open a short coordination thread (issue or chat) with the database + owner before the first file edit — this wave is the highest-risk of + the three for scope conflicts. + +Skipping this gate previously caused commit 093718d in the C6.1 wave to +accidentally carry an M4 scripts/ reorg because two agents touched +`scripts/` in the same window. Do not repeat. + Buckets: | Module | Approx sites | Dominant kind | Notes | diff --git a/src/main/database/backup.ts b/src/main/database/backup.ts index 78b491a0..2fabad83 100644 --- a/src/main/database/backup.ts +++ b/src/main/database/backup.ts @@ -21,7 +21,11 @@ function readGitSha(): string | null { }); const sha = out.toString("utf8").trim(); return sha.length === 40 ? sha : null; - } catch { + } catch (e) { + log.warn( + "[database/backup] git rev-parse HEAD failed; manifest.gitSha will be null", + e instanceof Error ? e.message : String(e), + ); return null; } } @@ -89,6 +93,12 @@ export function buildBackupMethods( // while the connection is held open — no WAL checkpoint dance, no // torn writes, works on sqlite3 ≥ 3.27. The destination must not // exist; `mkdirSync` above only created the parent directory. + // + // Safety invariant under WAL mode: VACUUM INTO opens an implicit + // read transaction that sees every committed frame including those + // still in profile.sqlite-wal. DO NOT add PRAGMA wal_checkpoint + // here — a TRUNCATE checkpoint would fight concurrent writers and + // buys nothing for the snapshot; a PASSIVE checkpoint is redundant. const dbTarget = join(targetDir, "profile.sqlite").replace(/'/g, "''"); db.exec(`VACUUM INTO '${dbTarget}'`);