From 261da66b07d0e4679725ec8f5890181e21f8f15c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 16:27:08 -0500 Subject: [PATCH 01/13] daemon: durationMs on project-sync log lines (latency baseline) --- lib/daemon/project-sync.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index eb9dee2d..3fc0f4a9 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -187,7 +187,7 @@ async function syncImpl( store.setScope(repoName, { authors: scopeAuthors, windowDays }); deepFailedAt.delete(repoName); log.debug( - { repo: repoName, mode: "deep", scoped: true, authors: scopeAuthors.length, open: kept.length, changed: changed.length }, + { repo: repoName, mode: "deep", scoped: true, authors: scopeAuthors.length, open: kept.length, changed: changed.length, durationMs: Date.now() - syncStartedAt }, "project sync", ); if (changed.length > 0) { @@ -204,7 +204,7 @@ async function syncImpl( // this point. store.setScope(repoName, null); deepFailedAt.delete(repoName); - log.debug({ repo: repoName, mode: "deep", open: prs.length, changed: changed.length }, "project sync"); + log.debug({ repo: repoName, mode: "deep", open: prs.length, changed: changed.length, durationMs: Date.now() - syncStartedAt }, "project sync"); if (changed.length > 0) { deps.broadcast("project-mrs", { repoName, iids: changed }); } @@ -295,7 +295,7 @@ async function syncImpl( if (pr) changed.push(...store.upsert(repoName, projectPath, pr, "events")); } - log.debug({ repo: repoName, mode: "delta", changed: changed.length, topup: topup.length }, "project sync"); + log.debug({ repo: repoName, mode: "delta", changed: changed.length, topup: topup.length, durationMs: Date.now() - deltaStartedAt }, "project sync"); if (changed.length > 0) { deps.broadcast("project-mrs", { repoName, iids: changed }); } From f1fabeeaf86b49dd6be4781a8c229543788f32f0 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 17:21:39 -0500 Subject: [PATCH 02/13] rt-client 0.5.0: codeownerSections demand axis, board.tabs registry row --- bun.lock | 4 ++-- package.json | 2 +- packages/rt-client/package.json | 2 +- packages/rt-client/src/commands.ts | 8 +++++++- .../rt-client/src/settings/__tests__/registry.test.ts | 1 + packages/rt-client/src/settings/registry-defs.ts | 7 +++++++ 6 files changed, 19 insertions(+), 5 deletions(-) diff --git a/bun.lock b/bun.lock index 442600fd..cb7b7658 100644 --- a/bun.lock +++ b/bun.lock @@ -6,7 +6,7 @@ "name": "repo-tools", "dependencies": { "@inkjs/ui": "^2.0.0", - "@mattstack/glance": "^0.19.0", + "@mattstack/glance": "^0.20.0", "@rezi-ui/core": "^0.1.0-alpha.60", "@rezi-ui/jsx": "^0.1.0-alpha.60", "@rezi-ui/node": "^0.1.0-alpha.60", @@ -68,7 +68,7 @@ "@inkjs/ui": ["@inkjs/ui@2.0.0", "", { "dependencies": { "chalk": "^5.3.0", "cli-spinners": "^3.0.0", "deepmerge": "^4.3.1", "figures": "^6.1.0" }, "peerDependencies": { "ink": ">=5" } }, "sha512-5+8fJmwtF9UvikzLfph9sA+LS+l37Ij/szQltkuXLOAXwNkBX9innfzh4pLGXIB59vKEQUtc6D4qGvhD7h3pAg=="], - "@mattstack/glance": ["@mattstack/glance@0.19.0", "", { "dependencies": { "@gitbeaker/rest": "^43.8.0", "@octokit/core": "^7.0.7", "@octokit/graphql": "^9.0.4", "@octokit/plugin-paginate-rest": "^15.0.0", "@octokit/plugin-retry": "^8.1.1", "@octokit/plugin-throttling": "^11.0.5", "@octokit/request-error": "^7.1.1" } }, "sha512-H+QMuyC3IZ3SXl8TpdjIKVhZvx+vP2ltVdt88CDz8e6YFponB2fplyOrHXLvGFITN1EnnEM6DQyYTcQ06F+7fg=="], + "@mattstack/glance": ["@mattstack/glance@0.20.0", "", { "dependencies": { "@gitbeaker/rest": "^43.8.0", "@octokit/core": "^7.0.7", "@octokit/graphql": "^9.0.4", "@octokit/plugin-paginate-rest": "^15.0.0", "@octokit/plugin-retry": "^8.1.1", "@octokit/plugin-throttling": "^11.0.5", "@octokit/request-error": "^7.1.1" } }, "sha512-WaOLfpQPim4p/HHg1JdQ2wltLIU/YUkVN0Il+4Y1Df/+EQJsN1cUUWmAg+QAgd9xH6PJNOuHlVQt4t4pPecAYQ=="], "@mattstack/rt-client": ["@mattstack/rt-client@workspace:packages/rt-client"], diff --git a/package.json b/package.json index 601015e6..9088d407 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ }, "dependencies": { "@inkjs/ui": "^2.0.0", - "@mattstack/glance": "^0.19.0", + "@mattstack/glance": "^0.20.0", "@rezi-ui/core": "^0.1.0-alpha.60", "@rezi-ui/jsx": "^0.1.0-alpha.60", "@rezi-ui/node": "^0.1.0-alpha.60", diff --git a/packages/rt-client/package.json b/packages/rt-client/package.json index 8d262731..b5dfb8ec 100644 --- a/packages/rt-client/package.json +++ b/packages/rt-client/package.json @@ -1,6 +1,6 @@ { "name": "@mattstack/rt-client", - "version": "0.6.0", + "version": "0.6.1", "type": "module", "exports": { ".": { diff --git a/packages/rt-client/src/commands.ts b/packages/rt-client/src/commands.ts index 85a0059d..f224cf80 100644 --- a/packages/rt-client/src/commands.ts +++ b/packages/rt-client/src/commands.ts @@ -12,6 +12,8 @@ export type Discussion = MRDetail["discussions"][number]; export interface DemandDecl { client: string; authors: string[]; + /** Codeowner sections this client needs covered (spec: second demand axis). */ + codeownerSections?: string[]; declaredAt: number; } @@ -19,10 +21,14 @@ export interface ProjectMRsScope { authors: string[]; windowDays: number; uncovered: string[]; + /** Effective synced section union; absent from a pre-sections daemon. */ + sections?: string[]; + /** Demanded sections not yet swept for this client. */ + uncoveredSections?: string[]; } export interface ProjectMRsData { - mrs: Record; + mrs: Record; listSyncedAt: number; source: "poll" | "events" | "mutation"; syncedAt: number; diff --git a/packages/rt-client/src/settings/__tests__/registry.test.ts b/packages/rt-client/src/settings/__tests__/registry.test.ts index 152f3490..de5ce79d 100644 --- a/packages/rt-client/src/settings/__tests__/registry.test.ts +++ b/packages/rt-client/src/settings/__tests__/registry.test.ts @@ -224,6 +224,7 @@ describe("settings/registry", () => { "board.slack", "board.doctorSkill", "board.triage.doctorSkill", + "board.tabs", "board.staleAfterDays", "board.workspaces", "board.defaultMember", diff --git a/packages/rt-client/src/settings/registry-defs.ts b/packages/rt-client/src/settings/registry-defs.ts index 0313b1ee..6fc9b509 100644 --- a/packages/rt-client/src/settings/registry-defs.ts +++ b/packages/rt-client/src/settings/registry-defs.ts @@ -352,6 +352,13 @@ export const REGISTRY: readonly SettingDef[] = [ merge: "replace", description: "Doctor skill the board's own API-tier triage sweep runs on your MRs; deliberately never resolved through a repo's skills.jsonc manifest. A sibling flat key of board.triage, not a field inside it — the board reader assembles the two independently.", }, + { + key: "board.tabs", + type: "array", + scopes: ["team"], + merge: "replace", + description: "Board tab definitions ({id, label, source, slackChannel?, reviewSkill?}); source.kind 'authors' is the classic roster board, 'codeowners' lists MRs blocked on an unapproved CODEOWNERS section. Absent = one implicit authors tab (fallback lives in the board reader, never here).", + }, // --- board (user) ---------------------------------------------------------- { From 126144499d64b4ac23f2844e57f9985a7bcc553b Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 17:37:33 -0500 Subject: [PATCH 03/13] daemon: project-mrs store learns section tags, scope sections, demand sections (statedb V4) --- .../specs/2026-08-20-rt-statedb.md | 2 + .../__tests__/project-mrs-store.test.ts | 41 +++++++++ lib/daemon/project-mrs-store.ts | 90 +++++++++++++++---- lib/state/__tests__/db.test.ts | 15 ++-- lib/state/db.ts | 20 ++++- 5 files changed, 141 insertions(+), 27 deletions(-) diff --git a/docs/superpowers/specs/2026-08-20-rt-statedb.md b/docs/superpowers/specs/2026-08-20-rt-statedb.md index b7f0bb39..4a410c90 100644 --- a/docs/superpowers/specs/2026-08-20-rt-statedb.md +++ b/docs/superpowers/specs/2026-08-20-rt-statedb.md @@ -98,6 +98,8 @@ CREATE TABLE IF NOT EXISTS kv ( ); ``` +**V6 addendum (RT board codeowner tabs; v5 is reserved by another lane):** adds `project_mr_sections` (repo, iid, sections JSON string[], PK (repo, iid)) and a nullable `project_mr_demands.sections` column (JSON string[]), for CODEOWNERS section tags on project-mrs rows. + **branch_cache keeps the bare-branch primary key** (review r1 findings 3+4): the bare branch IS the cache's semantic key today (`DiskCache.entries: Record`); `enrichBranches`' `fetchAndCache` path has no repoName, and a `(repo, branch)` PK would mint permanent duplicate rows plus an undefined collapse tie-break that flaps `checkAndNotify`. The cross-repo branch-name collision is today's documented quirk, carried forward unchanged; `repo` stays what it is today — an optional attribute (used by `worktree:list`'s guard exactly as before). `kv` holds whole-blob states where per-key granularity buys nothing: notifier state (`ns='notifier', k='state'`, read+written once per cycle as a unit) and event cursors (`ns='events-cursor', k=`). Payload shapes stay opaque JSON — this ticket changes persistence, never payload schemas. diff --git a/lib/daemon/__tests__/project-mrs-store.test.ts b/lib/daemon/__tests__/project-mrs-store.test.ts index 0fc85f0e..5aba5bf2 100644 --- a/lib/daemon/__tests__/project-mrs-store.test.ts +++ b/lib/daemon/__tests__/project-mrs-store.test.ts @@ -323,6 +323,47 @@ describe("scope", () => { }); }); +describe("section tags", () => { + test("setSectionTags tags, replaces, and clears per iid", () => { + const store = tmpStore(); + store.fullSync("r", "g/p", [pr(1), pr(2)], 1000); + store.setSectionTags("r", { 1: ["Acme"] }); + expect(store.read("r")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + expect(store.read("r")!.mrs[2]!.codeownerSections).toBeUndefined(); + store.setSectionTags("r", { 1: [] }); + expect(store.read("r")!.mrs[1]!.codeownerSections).toBeUndefined(); + }); + + test("setSectionTags replaceAll clears tags the map does not mention", () => { + const store = tmpStore(); + store.fullSync("r", "g/p", [pr(1), pr(2)], 1000); + store.setSectionTags("r", { 1: ["Acme"], 2: ["Beta"] }); + store.setSectionTags("r", { 2: ["Beta"] }, { replaceAll: true }); + expect(store.read("r")!.mrs[1]!.codeownerSections).toBeUndefined(); + expect(store.read("r")!.mrs[2]!.codeownerSections).toEqual(["Beta"]); + }); + + test("fullSync prune drops the pruned row's tag row too", () => { + const db = tmpDb(); + const store = createProjectMRs(db); + store.fullSync("r", "g/p", [pr(1)], 1000); + store.setSectionTags("r", { 1: ["Acme"] }); + store.fullSync("r", "g/p", [], 2000); + expect(store.read("r")!.mrs[1]).toBeUndefined(); + const rows = db.query("SELECT * FROM project_mr_sections WHERE repo = 'r';").all(); + expect(rows).toHaveLength(0); + }); + + test("registerDemand stores sections and scope round-trips them", () => { + const store = tmpStore(); + store.registerDemand("r", "board:1", ["ada"], 5, ["Acme"]); + expect(store.read("r")!.demands!["board:1"]!.sections).toEqual(["Acme"]); + store.fullSync("r", "g/p", [], 1000); + store.setScope("r", { authors: ["ada"], sections: ["Acme"], windowDays: 30 }); + expect(store.read("r")!.scope).toEqual({ authors: ["ada"], sections: ["Acme"], windowDays: 30 }); + }); +}); + describe("persistence — rows mirror the in-memory model", () => { test("fullSync writes are visible as project_mrs / project_mrs_meta rows", () => { const dir = mkdtempSync(join(tmpdir(), "rt-pmrs-")); diff --git a/lib/daemon/project-mrs-store.ts b/lib/daemon/project-mrs-store.ts index 386a154a..083d24c8 100644 --- a/lib/daemon/project-mrs-store.ts +++ b/lib/daemon/project-mrs-store.ts @@ -48,8 +48,8 @@ export function rekeyProjectMrDemandsTable(): Promise { return rekeyTableColumn("project_mr_demands", "repo"); } -export interface ProjectMREntry { pr: PullRequest; fetchedAt: number; } -export interface DemandEntry { authors: string[]; declaredAt: number; lastSeenAt: number; } +export interface ProjectMREntry { pr: PullRequest; fetchedAt: number; codeownerSections?: string[]; } +export interface DemandEntry { authors: string[]; sections?: string[]; declaredAt: number; lastSeenAt: number; } export interface ProjectMRStore { projectPath: string; mrs: Record; @@ -57,7 +57,7 @@ export interface ProjectMRStore { deltaSyncedAt?: number; source: "poll" | "events" | "mutation"; demands?: Record; - scope?: { authors: string[]; windowDays: number }; + scope?: { authors: string[]; sections?: string[]; windowDays: number }; } /** Read freshness = the more recent of a deep sync and a delta sync (spec §5.7). */ @@ -72,10 +72,12 @@ export interface ProjectMRs { fullSync(repoName: string, projectPath: string, prs: PullRequest[], syncStartedAt: number): number[]; applyDelta(repoName: string, projectPath: string, prs: PullRequest[], deltaStartedAt: number): number[]; findBySourceBranch(repoName: string, branch: string): PullRequest | null; - registerDemand(repoName: string, client: string, authors: string[], declaredAt: number): boolean; + registerDemand(repoName: string, client: string, authors: string[], declaredAt: number, sections?: string[]): boolean; expireDemands(repoName: string, maxIdleMs: number): string[]; /** Passing null clears an existing scope (the demand that motivated it is gone). */ - setScope(repoName: string, scope: { authors: string[]; windowDays: number } | null): void; + setScope(repoName: string, scope: { authors: string[]; sections?: string[]; windowDays: number } | null): void; + /** Per-iid replace; [] deletes the tag. replaceAll first clears every tag for the repo (deep sweep semantics). */ + setSectionTags(repoName: string, tags: Record, opts?: { replaceAll?: boolean }): void; } // ─── Row shapes ────────────────────────────────────────────────────────── @@ -89,7 +91,8 @@ interface MetaRow { project_path: string; scope: string | null; } -interface DemandRow { repo: string; client: string; authors: string; declared_at: number; last_seen_at: number; } +interface DemandRow { repo: string; client: string; authors: string; sections: string | null; declared_at: number; last_seen_at: number; } +interface SectionRow { repo: string; iid: number; sections: string; } function emptyStore(projectPath: string, source: ProjectMRStore["source"] = "poll"): ProjectMRStore { return { projectPath, mrs: {}, listSyncedAt: 0, source }; @@ -116,11 +119,24 @@ function loadAll(db: Database): Record { store.mrs[r.iid] = { pr: JSON.parse(r.pr) as PullRequest, fetchedAt: r.fetched_at }; } - const demandRows = db.query("SELECT repo, client, authors, declared_at, last_seen_at FROM project_mr_demands;").all() as DemandRow[]; + const demandRows = db.query("SELECT repo, client, authors, sections, declared_at, last_seen_at FROM project_mr_demands;").all() as DemandRow[]; for (const d of demandRows) { const store = data[d.repo] ?? (data[d.repo] = emptyStore("")); store.demands ??= {}; - store.demands[d.client] = { authors: JSON.parse(d.authors) as string[], declaredAt: d.declared_at, lastSeenAt: d.last_seen_at }; + store.demands[d.client] = { + authors: JSON.parse(d.authors) as string[], + sections: d.sections !== null ? (JSON.parse(d.sections) as string[]) : undefined, + declaredAt: d.declared_at, + lastSeenAt: d.last_seen_at, + }; + } + + const sectionRows = db.query("SELECT repo, iid, sections FROM project_mr_sections;").all() as SectionRow[]; + for (const s of sectionRows) { + const store = data[s.repo]; + const entry = store?.mrs[s.iid]; + if (!entry) continue; // MR absent (pruned/never synced): its tag row is stale, fullSync's prune cleans it up + entry.codeownerSections = JSON.parse(s.sections) as string[]; } return data; @@ -145,11 +161,17 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR scope = excluded.scope `); const upsertDemandStmt = db.query(` - INSERT INTO project_mr_demands (repo, client, authors, declared_at, last_seen_at) VALUES (?, ?, ?, ?, ?) + INSERT INTO project_mr_demands (repo, client, authors, sections, declared_at, last_seen_at) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT(repo, client) DO UPDATE SET - authors = excluded.authors, declared_at = excluded.declared_at, last_seen_at = excluded.last_seen_at + authors = excluded.authors, sections = excluded.sections, declared_at = excluded.declared_at, last_seen_at = excluded.last_seen_at `); const deleteDemandStmt = db.query(`DELETE FROM project_mr_demands WHERE repo = ? AND client = ?;`); + const upsertSectionStmt = db.query(` + INSERT INTO project_mr_sections (repo, iid, sections) VALUES (?, ?, ?) + ON CONFLICT(repo, iid) DO UPDATE SET sections = excluded.sections + `); + const deleteSectionStmt = db.query(`DELETE FROM project_mr_sections WHERE repo = ? AND iid = ?;`); + const deleteAllSectionsStmt = db.query(`DELETE FROM project_mr_sections WHERE repo = ?;`); function writeMeta(repoName: string, store: ProjectMRStore): void { upsertMetaStmt.run( @@ -243,7 +265,10 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR persistOrWarn("project-mrs", () => { const run = db.transaction(() => { for (const w of toWrite) upsertMrStmt.run(repoName, w.iid, JSON.stringify(w.pr), w.fetchedAt); - for (const iid of toDelete) deleteMrStmt.run(repoName, iid); + for (const iid of toDelete) { + deleteMrStmt.run(repoName, iid); + deleteSectionStmt.run(repoName, iid); + } writeMeta(repoName, store); }); run(); @@ -318,19 +343,22 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR return fallback; } - function registerDemand(repoName: string, client: string, authors: string[], declaredAt: number): boolean { + function registerDemand(repoName: string, client: string, authors: string[], declaredAt: number, sections?: string[]): boolean { const store = data[repoName] ?? (data[repoName] = emptyStore("")); store.demands ??= {}; const prev = store.demands[client]; if (prev && declaredAt < prev.declaredAt) return false; + const sameSections = (a?: string[], b?: string[]) => + (a ?? []).length === (b ?? []).length && (a ?? []).every((s, i) => s === (b ?? [])[i]); const unchanged = prev !== undefined && prev.authors.length === authors.length - && prev.authors.every((a, i) => a === authors[i]); + && prev.authors.every((a, i) => a === authors[i]) + && sameSections(prev.sections, sections); const lastSeenAt = Date.now(); - store.demands[client] = { authors: [...authors], declaredAt, lastSeenAt }; + store.demands[client] = { authors: [...authors], sections: sections ? [...sections] : undefined, declaredAt, lastSeenAt }; persistOrWarn("project-mrs", () => { - upsertDemandStmt.run(repoName, client, JSON.stringify(authors), declaredAt, lastSeenAt); + upsertDemandStmt.run(repoName, client, JSON.stringify(authors), sections ? JSON.stringify(sections) : null, declaredAt, lastSeenAt); }, { repo: repoName, op: "registerDemand" }); return !unchanged; @@ -353,17 +381,44 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR return dropped; } - function setScope(repoName: string, scope: { authors: string[]; windowDays: number } | null): void { + function setScope(repoName: string, scope: { authors: string[]; sections?: string[]; windowDays: number } | null): void { const store = data[repoName]; if (!store) return; if (scope === null) { delete store.scope; } else { - store.scope = { authors: [...scope.authors], windowDays: scope.windowDays }; + store.scope = { authors: [...scope.authors], sections: scope.sections ? [...scope.sections] : undefined, windowDays: scope.windowDays }; } persistOrWarn("project-mrs", () => writeMeta(repoName, store), { repo: repoName, op: "setScope" }); } + function setSectionTags(repoName: string, tags: Record, opts?: { replaceAll?: boolean }): void { + const store = data[repoName]; + if (!store) return; + if (opts?.replaceAll) { + for (const entry of Object.values(store.mrs)) delete entry.codeownerSections; + } + for (const [iidStr, sections] of Object.entries(tags)) { + const entry = store.mrs[Number(iidStr)]; + if (!entry) continue; + if (sections.length > 0) entry.codeownerSections = [...sections]; + else delete entry.codeownerSections; + } + + persistOrWarn("project-mrs", () => { + const run = db.transaction(() => { + if (opts?.replaceAll) deleteAllSectionsStmt.run(repoName); + for (const [iidStr, sections] of Object.entries(tags)) { + const iid = Number(iidStr); + if (!data[repoName]!.mrs[iid]) continue; + if (sections.length > 0) upsertSectionStmt.run(repoName, iid, JSON.stringify(sections)); + else deleteSectionStmt.run(repoName, iid); + } + }); + run(); + }, { repo: repoName, op: "setSectionTags" }); + } + return { data, read: (repoName) => data[repoName], @@ -374,6 +429,7 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR registerDemand, expireDemands, setScope, + setSectionTags, }; } diff --git a/lib/state/__tests__/db.test.ts b/lib/state/__tests__/db.test.ts index c01e4109..71c71f0e 100644 --- a/lib/state/__tests__/db.test.ts +++ b/lib/state/__tests__/db.test.ts @@ -65,6 +65,7 @@ const ALL_TABLE_NAMES = [ "kv", "notify_queue", "project_mr_demands", + "project_mr_sections", "project_mrs", "project_mrs_meta", "run_history", @@ -72,10 +73,10 @@ const ALL_TABLE_NAMES = [ ]; describe("openStateDb — fresh open", () => { - test("a fresh database reaches v4 directly, gaining every v1-v4 table", () => { + test("a fresh database reaches v6 directly, gaining every v1, v2, v3, v4, and v6 table (v5 is reserved by another lane)", () => { const dbPath = join(dir, "state.db"); const db = openStateDb(dbPath, "cli"); - expect(SCHEMA_VERSION).toBe(4); + expect(SCHEMA_VERSION).toBe(6); expect(userVersion(db)).toBe(SCHEMA_VERSION); expect(tableNames(db)).toEqual(ALL_TABLE_NAMES); db.close(); @@ -94,7 +95,7 @@ describe("openStateDb — fresh open", () => { expect( db.query("SELECT name FROM sqlite_master WHERE name IN ('chat_presence','chat_dms','chat_room_defaults')").all(), ).toHaveLength(3); - expect(db.query("PRAGMA user_version").get()).toMatchObject({ user_version: 4 }); + expect(db.query("PRAGMA user_version").get()).toMatchObject({ user_version: 6 }); db.close(); }); }); @@ -154,13 +155,13 @@ function buildV1Fixture(path: string): Database { return db; } -describe("openStateDb — v1 database migrates to v4", () => { - test("existing v1 rows survive, and v2's, v3's, and v4's new tables appear alongside them", () => { +describe("openStateDb — v1 database migrates to v6", () => { + test("existing v1 rows survive, and v2's, v3's, v4's, and v6's new tables appear alongside them", () => { const dbPath = join(dir, "state.db"); buildV1Fixture(dbPath); const db = openStateDb(dbPath, "cli"); - expect(userVersion(db)).toBe(4); + expect(userVersion(db)).toBe(6); expect(tableNames(db)).toEqual(ALL_TABLE_NAMES); const branchRow = db.query("SELECT branch, repo, linear_id, fetched_at FROM branch_cache WHERE branch = ?;").get("main"); @@ -390,7 +391,7 @@ describe("getStateDb / closeStateDb — lazy singleton", () => { // unrelated exports (reading SCHEMA_VERSION, pushing to LEGACY_IMPORTS) // never opens or creates a db file on its own. const before = SCHEMA_VERSION; - expect(before).toBe(4); + expect(before).toBe(6); LEGACY_IMPORTS.push({ file: "x.json", import: () => {} }); LEGACY_IMPORTS.length = 0; // No db.ts function that touches disk was called above; nothing to assert diff --git a/lib/state/db.ts b/lib/state/db.ts index c6e2dab5..daaa5e7b 100644 --- a/lib/state/db.ts +++ b/lib/state/db.ts @@ -21,8 +21,8 @@ import { rtDir } from "../rt-paths.ts"; export type DbFlavor = "cli" | "daemon"; -/** PRAGMA user_version target for the combined schema below (v1 + v2 + v3 + v4). */ -export const SCHEMA_VERSION = 4; +/** PRAGMA user_version target for the combined schema below (v1 + v2 + v3 + v4 + v6; v5 is reserved by another lane). */ +export const SCHEMA_VERSION = 6; // busy_timeout is per-process, not per-store (spec "The database"): a CLI // command may block briefly; the daemon's event loop must never block long, @@ -237,6 +237,20 @@ CREATE TABLE IF NOT EXISTS chat_dms ( ); `; +// Tables (v6): CODEOWNERS section tags on project-mrs rows (RT board +// codeowner tabs). `project_mr_sections` is a separate table, not a column +// on `project_mrs`, so `setSectionTags`'s per-iid clear (empty array) is a +// plain DELETE rather than a NULL-vs-empty-string ambiguity on that row. +const V6_SCHEMA = ` +CREATE TABLE IF NOT EXISTS project_mr_sections ( + repo TEXT NOT NULL, + iid INTEGER NOT NULL, + sections TEXT NOT NULL, -- JSON string[] + PRIMARY KEY (repo, iid) +); +ALTER TABLE project_mr_demands ADD COLUMN sections TEXT; +`; + /** bun:sqlite error codes that mean "the file on disk is not a usable db". */ function isCorruptionError(err: unknown): boolean { const code = (err as { code?: string } | undefined)?.code; @@ -367,7 +381,7 @@ function runMigrations(db: Database, dir: string): void { // One exec of the full combined schema, not a per-version step: every // statement is IF NOT EXISTS, so replaying v1's DDL against an // already-v1 db is a no-op and existing rows are untouched. - db.exec(V1_SCHEMA + V2_SCHEMA + V3_SCHEMA + V4_SCHEMA); + db.exec(V1_SCHEMA + V2_SCHEMA + V3_SCHEMA + V4_SCHEMA + V6_SCHEMA); // Legacy-JSON import is single-shot and only correct from a true // v0 (never-migrated) database: branch-cache's UPSERT would silently // overwrite current rows with stale ones, and project-mrs-store's From 5fca5f6978e9ef336fc1d1198fdb87bc3272b79e Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 17:52:39 -0500 Subject: [PATCH 04/13] daemon: deep sync sweeps codeowner sections; backfillSections; sections log line --- lib/daemon/__tests__/project-sync.test.ts | 81 +++++++++++- lib/daemon/project-sync.ts | 147 ++++++++++++++++++++-- 2 files changed, 219 insertions(+), 9 deletions(-) diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index 0f2d7941..a5400d7d 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -2,7 +2,7 @@ import { describe, expect, setSystemTime, test } from "bun:test"; import { mkdtempSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import { syncProjectMRs, backfillAuthors, DEEP_RECONCILE_MS, DEEP_RETRY_BACKOFF_MS, DELTA_OVERLAP_MS } from "../project-sync.ts"; +import { syncProjectMRs, backfillAuthors, effectiveSections, sectionsMatching, DEEP_RECONCILE_MS, DEEP_RETRY_BACKOFF_MS, DELTA_OVERLAP_MS } from "../project-sync.ts"; import { createProjectMRs } from "../project-mrs-store.ts"; import { openStateDb } from "../../state/index.ts"; import type { PullRequest } from "@mattstack/glance"; @@ -890,3 +890,82 @@ describe("demand-scoped sync", () => { expect(store.read("s7")!.mrs[8]).toBeDefined(); }); }); + +describe("codeowner section sweep (deep)", () => { + const deps = (repo: string) => ({ repoIndex: () => ({ [repo]: "/tmp/repo" }), broadcast: () => {} }); + + test("pure helpers: effectiveSections unions demands, sectionsMatching filters rules", () => { + expect(effectiveSections({ demands: { + a: { authors: ["x"], sections: ["Acme"], declaredAt: 1, lastSeenAt: 1 }, + b: { authors: ["y"], sections: ["Acme", "Beta"], declaredAt: 1, lastSeenAt: 1 }, + } } as any)).toEqual(["Acme", "Beta"]); + expect(effectiveSections(undefined)).toEqual([]); + expect(sectionsMatching( + [{ type: "CODE_OWNER", approved: false, section: "Acme" }, + { type: "CODE_OWNER", approved: true, section: "Beta" }, + { type: "REGULAR", approved: false, section: null }], + ["Acme", "Beta"], + )).toEqual(["Acme"]); + }); + + test("deep with a sections demand sweeps rules, hydrates matches, keeps them past fullSync", async () => { + const store = tmpStore(); + store.registerDemand("r1", "board:1", ["ada"], 1, ["Acme"]); + const hydrated: number[] = []; + await syncProjectMRs(deps("r1"), "r1", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 1, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + { iid: 9, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + { iid: 5, rules: [{ type: "CODE_OWNER", approved: true, section: "Acme" }] }, + ] }), + fetchSingle: async (_r, _p, iid) => { hydrated.push(iid); return pr(iid, { author: { username: "stranger" } as any }); }, + }); + const rec = store.read("r1")!; + expect(hydrated).toEqual([9]); // 1 is author-covered, 5 unmatched + expect(Object.keys(rec.mrs).sort()).toEqual(["1", "9"]); + expect(rec.mrs[9]!.codeownerSections).toEqual(["Acme"]); + expect(rec.mrs[1]!.codeownerSections).toEqual(["Acme"]); // tagged even when author-covered + expect(rec.scope).toMatchObject({ sections: ["Acme"] }); + }); + + test("deep sweep untags rows the sweep no longer matches (replaceAll)", async () => { + const store = tmpStore(); + store.registerDemand("r2", "board:1", ["ada"], 1, ["Acme"]); + // First deep: iid 9 matches an unapproved CODE_OWNER rule and gets hydrated + tagged. + await syncProjectMRs(deps("r2"), "r2", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 9, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + fetchSingle: async (_r, _p, iid) => pr(iid, { author: { username: "stranger" } as any }), + }); + expect(store.read("r2")!.mrs[9]!.codeownerSections).toEqual(["Acme"]); + + // Second deep: the rule is gone (approved/removed) -- the sweep no longer matches iid 9. + await syncProjectMRs(deps("r2"), "r2", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [] }), + }); + const rec = store.read("r2")!; + expect(rec.mrs[9]).toBeUndefined(); // pruned: stranger, no longer matched + expect(Object.values(rec.mrs).some((e) => e.codeownerSections?.length)).toBe(false); + }); + + test("containment: a demand without sections never calls fetchRules and never tags", async () => { + const store = tmpStore(); + store.registerDemand("r3", "board:1", ["ada"], 1); + let rulesCalled = 0; + await syncProjectMRs(deps("r3"), "r3", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => { rulesCalled++; return { projectPath: "g/p", rules: [] }; }, + }); + expect(rulesCalled).toBe(0); + expect(store.read("r3")!.mrs[1]!.codeownerSections).toBeUndefined(); + expect(store.read("r3")!.scope).toEqual({ authors: ["ada", "self"], windowDays: 30 }); + }); +}); diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index 3fc0f4a9..647c4043 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -29,7 +29,7 @@ * record to delta against) and explicit `mode: "deep"` requests still reject. */ -import type { PullRequest } from "@mattstack/glance"; +import type { PullRequest, ApprovalRuleLite, MRApprovalRules } from "@mattstack/glance"; import { getRepoContext, getSelfUsername, resolveSelfUsername } from "./freshness.ts"; import { getProjectMRs, freshnessOf, type ProjectMRs, type ProjectMRStore } from "./project-mrs-store.ts"; import { loadRepoTracking, grants } from "../repo-tracking.ts"; @@ -59,6 +59,18 @@ export function effectiveAuthors(record: ProjectMRStore | undefined, selfUsernam return [...authors].sort(); } +/** Union of every live demand's sections, sorted (parity with effectiveAuthors); [] when none. */ +export function effectiveSections(record: ProjectMRStore | undefined): string[] { + const sections = new Set(); + for (const d of Object.values(record?.demands ?? {})) for (const s of d.sections ?? []) sections.add(s); + return [...sections].sort(); +} + +/** Demanded sections with an unapproved CODE_OWNER rule; [] when none. */ +export function sectionsMatching(rules: ApprovalRuleLite[], demanded: string[]): string[] { + return demanded.filter((s) => rules.some((r) => r.type === "CODE_OWNER" && !r.approved && r.section === s)); +} + /** Drops PRs whose updatedAt is older than the window. A missing/unparseable timestamp is kept -- never guess-drop. */ function withinWindow(prs: PullRequest[], windowDays: number, now: number): PullRequest[] { const cutoff = now - windowDays * 86_400_000; @@ -88,6 +100,8 @@ export interface ProjectSyncOverrides { fetchSingle?: (repoName: string, projectPath: string, iid: number) => Promise; /** Scoped deep's fetch: every open MR by any of these authors. */ fetchAuthors?: (repoName: string, authors: string[]) => Promise<{ projectPath: string; prs: PullRequest[] }>; + /** Codeowner section sweep's fetch, shared by the deep sweep and backfillSections. */ + fetchRules?: (repoName: string, opts: { updatedAfter?: string; iids?: number[] }) => Promise<{ projectPath: string; rules: MRApprovalRules[] }>; /** Overrides the grants-resolved window (test seam); production always resolves it from repo-tracking. */ windowDays?: number; /** @@ -141,6 +155,18 @@ async function syncImpl( const deepBackingOff = Date.now() - (deepFailedAt.get(repoName) ?? 0) < DEEP_RETRY_BACKOFF_MS; const isDeep = explicitDeep || (deepDue && (!record || !deepBackingOff)); + // Shared by the deep section sweep below and the delta pipeline top-up + // further down, so each gets one declaration rather than two. + const fetchSingle = overrides.fetchSingle ?? (async (repo: string, pp: string, iid: number) => { + const { provider } = await getRepoContext(repo, deps.repoIndex()[repo]); + return provider.fetchSingleMR(pp, iid, null); + }); + const fetchRules = overrides.fetchRules ?? (async (repo: string, opts: { updatedAfter?: string; iids?: number[] }) => { + const { provider, projectPath } = await getRepoContext(repo, deps.repoIndex()[repo]); + const rules = await provider.fetchApprovalRules({ projectPath, ...opts }); + return { projectPath, rules }; + }); + if (isDeep) { // Idle demand clients (a board tab closed a week ago) must not keep // pinning their authors into the scope forever. @@ -181,10 +207,60 @@ async function syncImpl( // now covers both out-of-scope authors and out-of-window MRs...a // failed author fetch rejects the whole deep (below) rather than // landing here as "this author has no MRs". + const sections = effectiveSections(record); const { projectPath, prs } = await fetchAuthors(repoName, scopeAuthors); const kept = withinWindow(prs, windowDays, syncStartedAt); - const changed = store.fullSync(repoName, projectPath, kept, syncStartedAt); - store.setScope(repoName, { authors: scopeAuthors, windowDays }); + const byIid = new Map(kept.map((pr) => [pr.iid, pr])); + const tags: Record = {}; + let sweep = { candidates: 0, matched: 0, hydrated: 0 }; + // Containment: with no demand ever declaring a section, `sections` + // is [] and this whole block is skipped -- no rules fetch, no tag + // write, byIid stays exactly `kept`. Bit-identical to pre-sections behavior. + if (sections.length > 0) { + const sweepStartedAt = Date.now(); + const updatedAfter = new Date(syncStartedAt - windowDays * 86_400_000).toISOString(); + const { rules } = await fetchRules(repoName, { updatedAfter }); + sweep.candidates = rules.length; + const matched = rules + .map((r) => ({ iid: r.iid, sections: sectionsMatching(r.rules, sections) })) + .filter((m) => m.sections.length > 0); + sweep.matched = matched.length; + for (const m of matched) tags[m.iid] = m.sections; + // Hydrate tagged MRs the author fetch did not cover. Stored rows are + // reused (delta/events keep them fresh); only unseen iids pay a fetch. + const toHydrate = matched.filter((m) => !byIid.has(m.iid) && !record?.mrs[m.iid]); + for (let i = 0; i < toHydrate.length; i += TOPUP_CONCURRENCY) { + const chunk = toHydrate.slice(i, i + TOPUP_CONCURRENCY); + await Promise.all(chunk.map(async (m) => { + try { + const pr = await fetchSingle(repoName, projectPath, m.iid); + if (pr) { byIid.set(m.iid, pr); sweep.hydrated++; } + } catch (err) { + log.warn({ err, repo: repoName, iid: m.iid }, "section hydration failed"); + } + })); + } + for (const m of matched) { + const stored = record?.mrs[m.iid]?.pr; + if (!byIid.has(m.iid) && stored) byIid.set(m.iid, stored); + } + log.debug( + { repo: repoName, mode: "sections", sections, ...sweep, durationMs: Date.now() - sweepStartedAt }, + "project sync", + ); + } + const changed = store.fullSync(repoName, projectPath, [...byIid.values()], syncStartedAt); + // Containment: with sections never declared there is nothing to write + // and nothing to clear, so this stays a no-op call away from + // bit-identical behavior. It runs (replaceAll) when sections are live + // OR when stale tags linger from a demand that just dropped -- read + // AFTER fullSync so the prune above has already dropped pruned rows' + // tags. + const hasStaleTags = Object.values(store.read(repoName)?.mrs ?? {}).some((e) => e.codeownerSections?.length); + if (sections.length > 0 || hasStaleTags) { + store.setSectionTags(repoName, tags, { replaceAll: true }); + } + store.setScope(repoName, { authors: scopeAuthors, ...(sections.length > 0 ? { sections } : {}), windowDays }); deepFailedAt.delete(repoName); log.debug( { repo: repoName, mode: "deep", scoped: true, authors: scopeAuthors.length, open: kept.length, changed: changed.length, durationMs: Date.now() - syncStartedAt }, @@ -254,11 +330,8 @@ async function syncImpl( // STORED pipeline is still in flight and that this delta didn't already // cover — the set is naturally tiny (pipelines currently running on open // MRs), so this restores the old ≤5-min pipeline freshness for ~0-5 - // targeted fetches per cycle. - const fetchSingle = overrides.fetchSingle ?? (async (repo: string, pp: string, iid: number) => { - const { provider } = await getRepoContext(repo, deps.repoIndex()[repo]); - return provider.fetchSingleMR(pp, iid, null); - }); + // targeted fetches per cycle. (fetchSingle is hoisted above the deep + // branch, shared with the section sweep's hydration.) const deltaIids = new Set(changed); const topup: number[] = []; const afterDelta = store.read(repoName); @@ -352,3 +425,61 @@ export async function backfillAuthors( deps.broadcast("project-mrs", { repoName, iids: changed }); } } + +/** + * Sections analog of backfillAuthors: on-demand top-up for sections newly + * declared by a demand (e.g. a board tab widening its request) without + * waiting for the next daily deep. Sweeps rules for the window, hydrates + * any matched MR the store doesn't already have, tags every match, and + * extends (never replaces) the existing scope's section list. + */ +export async function backfillSections( + deps: ProjectSyncDeps, + repoName: string, + sections: string[], + overrides: ProjectSyncOverrides = {}, +): Promise { + if (sections.length === 0) return; + + const store = overrides.store ?? getProjectMRs(); + const record = store.read(repoName); + const windowDays = record?.scope?.windowDays + ?? overrides.windowDays + ?? grants(loadRepoTracking(), repoName).projectMrsWindowDays; + const fetchRules = overrides.fetchRules ?? (async (repo: string, opts: { updatedAfter?: string; iids?: number[] }) => { + const { provider, projectPath } = await getRepoContext(repo, deps.repoIndex()[repo]); + const rules = await provider.fetchApprovalRules({ projectPath, ...opts }); + return { projectPath, rules }; + }); + const fetchSingle = overrides.fetchSingle ?? (async (repo: string, pp: string, iid: number) => { + const { provider } = await getRepoContext(repo, deps.repoIndex()[repo]); + return provider.fetchSingleMR(pp, iid, null); + }); + + const updatedAfter = new Date(Date.now() - windowDays * 86_400_000).toISOString(); + const { projectPath, rules } = await fetchRules(repoName, { updatedAfter }); + const matched = rules + .map((r) => ({ iid: r.iid, sections: sectionsMatching(r.rules, sections) })) + .filter((m) => m.sections.length > 0); + + const changed: number[] = []; + for (const m of matched) { + if (!store.read(repoName)?.mrs[m.iid]) { + try { + const pr = await fetchSingle(repoName, projectPath, m.iid); + if (pr) changed.push(...store.upsert(repoName, projectPath, pr, "events")); + } catch (err) { + log.warn({ err, repo: repoName, iid: m.iid }, "section backfill hydration failed"); + } + } + } + + store.setSectionTags(repoName, Object.fromEntries(matched.map((m) => [m.iid, m.sections]))); + const union = new Set([...(store.read(repoName)?.scope?.sections ?? []), ...sections]); + const scope = store.read(repoName)?.scope; + if (scope) store.setScope(repoName, { ...scope, sections: [...union].sort() }); + + if (changed.length > 0) { + deps.broadcast("project-mrs", { repoName, iids: changed }); + } +} From 39fe2c6a87e8d9f06e8b0cb3fbdfd83700305e33 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 18:12:18 -0500 Subject: [PATCH 05/13] daemon: fix section-sweep review findings (backfill sections preservation, rollback detection, coverage) - backfillAuthors carried forward an existing scope's sections; setScope's full-replace semantics were silently dropping them on every author backfill. - hasStaleTags now reads the pre-fullSync record: fullSync's reconcile overwrites every surviving entry, which was making the post-fullSync read always false and the rollback branch dead code. - rewrote the untags test to exercise replaceAll instead of the prune path, added a spy-based test pinning the rollback's exactly-once write, and added backfillSections coverage (no-op, selective hydration, non-replaceAll tag write, sorted scope union, no-scope guard). --- lib/daemon/__tests__/project-sync.test.ts | 161 ++++++++++++++++++++-- lib/daemon/project-sync.ts | 30 ++-- 2 files changed, 173 insertions(+), 18 deletions(-) diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index a5400d7d..7a81431b 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -2,7 +2,7 @@ import { describe, expect, setSystemTime, test } from "bun:test"; import { mkdtempSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import { syncProjectMRs, backfillAuthors, effectiveSections, sectionsMatching, DEEP_RECONCILE_MS, DEEP_RETRY_BACKOFF_MS, DELTA_OVERLAP_MS } from "../project-sync.ts"; +import { syncProjectMRs, backfillAuthors, backfillSections, effectiveSections, sectionsMatching, DEEP_RECONCILE_MS, DEEP_RETRY_BACKOFF_MS, DELTA_OVERLAP_MS } from "../project-sync.ts"; import { createProjectMRs } from "../project-mrs-store.ts"; import { openStateDb } from "../../state/index.ts"; import type { PullRequest } from "@mattstack/glance"; @@ -933,26 +933,72 @@ describe("codeowner section sweep (deep)", () => { test("deep sweep untags rows the sweep no longer matches (replaceAll)", async () => { const store = tmpStore(); store.registerDemand("r2", "board:1", ["ada"], 1, ["Acme"]); - // First deep: iid 9 matches an unapproved CODE_OWNER rule and gets hydrated + tagged. + // First deep: iid 1 is author-covered AND matches an unapproved CODE_OWNER rule. await syncProjectMRs(deps("r2"), "r2", { store, selfUsername: "self", windowDays: 30, mode: "deep", - fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), fetchRules: async () => ({ projectPath: "g/p", rules: [ - { iid: 9, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + { iid: 1, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, ] }), - fetchSingle: async (_r, _p, iid) => pr(iid, { author: { username: "stranger" } as any }), }); - expect(store.read("r2")!.mrs[9]!.codeownerSections).toEqual(["Acme"]); + expect(store.read("r2")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); - // Second deep: the rule is gone (approved/removed) -- the sweep no longer matches iid 9. + // Second deep: iid 1 stays author-covered (fullSync retains the row -- + // the prune never fires), but the rule no longer matches. Only the + // replaceAll write can clear this tag; the prune path is not exercised. await syncProjectMRs(deps("r2"), "r2", { store, selfUsername: "self", windowDays: 30, mode: "deep", - fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), fetchRules: async () => ({ projectPath: "g/p", rules: [] }), }); const rec = store.read("r2")!; - expect(rec.mrs[9]).toBeUndefined(); // pruned: stranger, no longer matched - expect(Object.values(rec.mrs).some((e) => e.codeownerSections?.length)).toBe(false); + expect(rec.mrs[1]).toBeDefined(); // still in scope: not pruned + expect(rec.mrs[1]!.codeownerSections).toBeUndefined(); + }); + + test("hasStaleTags rollback: dropping a demand's sections clears a still-in-scope row's tag exactly once (finding 3)", async () => { + const raw = tmpStore(); + let tagCalls = 0; + const store = { ...raw, setSectionTags: (repoName: string, tags: Record, opts?: { replaceAll?: boolean }) => { + tagCalls++; + raw.setSectionTags(repoName, tags, opts); + } }; + store.registerDemand("r6", "board:1", ["ada"], 1, ["Acme"]); + + // 1st deep: iid 1 is author-covered and matches -> tagged. + await syncProjectMRs(deps("r6"), "r6", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 1, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + }); + expect(raw.read("r6")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + expect(tagCalls).toBe(1); + + // The demand drops its sections but keeps wanting the same author. + store.registerDemand("r6", "board:1", ["ada"], 2); + + // 2nd deep: no sections demanded anymore, but iid 1 stays author-covered + // -- the sweep is skipped (no fetchRules call), yet the stale tag from + // before must still be cleared via the hasStaleTags rollback. + await syncProjectMRs(deps("r6"), "r6", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => { throw new Error("must not fetch rules: no sections demanded"); }, + }); + expect(raw.read("r6")!.mrs[1]).toBeDefined(); + expect(raw.read("r6")!.mrs[1]!.codeownerSections).toBeUndefined(); + expect(tagCalls).toBe(2); + + // 3rd deep: nothing changed -- no stale tag left, no sections demanded + // -> no further tag-clear write. + await syncProjectMRs(deps("r6"), "r6", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => { throw new Error("must not fetch rules: no sections demanded"); }, + }); + expect(tagCalls).toBe(2); }); test("containment: a demand without sections never calls fetchRules and never tags", async () => { @@ -968,4 +1014,99 @@ describe("codeowner section sweep (deep)", () => { expect(store.read("r3")!.mrs[1]!.codeownerSections).toBeUndefined(); expect(store.read("r3")!.scope).toEqual({ authors: ["ada", "self"], windowDays: 30 }); }); + + test("backfillAuthors preserves an existing scope's sections (finding 1)", async () => { + const store = tmpStore(); + store.fullSync("r5", "g/p", [], Date.now() - 1000); + store.setScope("r5", { authors: ["alice"], sections: ["Acme"], windowDays: 30 }); + await backfillAuthors( + deps("r5"), "r5", ["newbie"], + { store, windowDays: 30, fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }) }, + ); + expect(store.read("r5")!.scope).toEqual({ authors: ["alice", "newbie"], sections: ["Acme"], windowDays: 30 }); + }); + + describe("backfillSections (finding 4)", () => { + test("empty section list is a no-op: no fetch, no scope mutation, no broadcast", async () => { + const store = tmpStore(); + store.fullSync("bs1", "g/p", [], Date.now() - 1000); + store.setScope("bs1", { authors: ["alice"], sections: ["Acme"], windowDays: 30 }); + const events: any[] = []; + await backfillSections( + { repoIndex: () => ({ bs1: "/tmp/repo" }), broadcast: (t, d) => events.push({ t, d }) }, + "bs1", [], + { store, fetchRules: async () => { throw new Error("must not fetch"); } }, + ); + expect(store.read("bs1")!.scope!.sections).toEqual(["Acme"]); + expect(events.length).toBe(0); + }); + + test("hydrates only iids the store doesn't already have", async () => { + const store = tmpStore(); + store.fullSync("bs2", "g/p", [pr(1, { author: { username: "alice" } as any })], Date.now() - 1000); + store.setScope("bs2", { authors: ["alice"], windowDays: 30 }); + const hydrated: number[] = []; + await backfillSections( + { repoIndex: () => ({ bs2: "/tmp/repo" }), broadcast: () => {} }, + "bs2", ["Acme"], + { + store, windowDays: 30, + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 1, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, // already stored + { iid: 2, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, // needs hydration + ] }), + fetchSingle: async (_r, _pp, iid) => { hydrated.push(iid); return pr(iid, { author: { username: "stranger" } as any }); }, + }, + ); + expect(hydrated).toEqual([2]); + expect(store.read("bs2")!.mrs[2]).toBeDefined(); + expect(store.read("bs2")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + expect(store.read("bs2")!.mrs[2]!.codeownerSections).toEqual(["Acme"]); + }); + + test("tags matches without replaceAll -- existing tags on other iids survive", async () => { + const store = tmpStore(); + store.fullSync("bs3", "g/p", [ + pr(1, { author: { username: "alice" } as any }), + pr(2, { author: { username: "alice" } as any }), + ], Date.now() - 1000); + store.setSectionTags("bs3", { 1: ["Beta"] }); + store.setScope("bs3", { authors: ["alice"], sections: ["Beta"], windowDays: 30 }); + await backfillSections( + { repoIndex: () => ({ bs3: "/tmp/repo" }), broadcast: () => {} }, + "bs3", ["Acme"], + { + store, windowDays: 30, + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 2, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + }, + ); + expect(store.read("bs3")!.mrs[1]!.codeownerSections).toEqual(["Beta"]); // untouched: not replaceAll + expect(store.read("bs3")!.mrs[2]!.codeownerSections).toEqual(["Acme"]); + }); + + test("unions sections into an existing scope, sorted", async () => { + const store = tmpStore(); + store.fullSync("bs4", "g/p", [], Date.now() - 1000); + store.setScope("bs4", { authors: ["alice"], sections: ["Beta"], windowDays: 30 }); + await backfillSections( + { repoIndex: () => ({ bs4: "/tmp/repo" }), broadcast: () => {} }, + "bs4", ["Acme"], + { store, windowDays: 30, fetchRules: async () => ({ projectPath: "g/p", rules: [] }) }, + ); + expect(store.read("bs4")!.scope!.sections).toEqual(["Acme", "Beta"]); + }); + + test("with no existing scope leaves scope unset (the `if (scope)` guard)", async () => { + const store = tmpStore(); + store.fullSync("bs5", "g/p", [], Date.now() - 1000); + await backfillSections( + { repoIndex: () => ({ bs5: "/tmp/repo" }), broadcast: () => {} }, + "bs5", ["Acme"], + { store, windowDays: 30, fetchRules: async () => ({ projectPath: "g/p", rules: [] }) }, + ); + expect(store.read("bs5")!.scope).toBeUndefined(); + }); + }); }); diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index 647c4043..6ba1fd97 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -208,6 +208,12 @@ async function syncImpl( // failed author fetch rejects the whole deep (below) rather than // landing here as "this author has no MRs". const sections = effectiveSections(record); + // Captured BEFORE fullSync runs: fullSync's reconcile overwrites + // every surviving entry with a fresh { pr, fetchedAt } object, which + // would erase this same signal if read afterward -- a demand that + // drops its sections while the MR stays in scope (author-covered) + // would then never trigger the rollback clear below. + const hasStaleTags = Object.values(record?.mrs ?? {}).some((e) => e.codeownerSections?.length); const { projectPath, prs } = await fetchAuthors(repoName, scopeAuthors); const kept = withinWindow(prs, windowDays, syncStartedAt); const byIid = new Map(kept.map((pr) => [pr.iid, pr])); @@ -250,13 +256,14 @@ async function syncImpl( ); } const changed = store.fullSync(repoName, projectPath, [...byIid.values()], syncStartedAt); - // Containment: with sections never declared there is nothing to write - // and nothing to clear, so this stays a no-op call away from - // bit-identical behavior. It runs (replaceAll) when sections are live - // OR when stale tags linger from a demand that just dropped -- read - // AFTER fullSync so the prune above has already dropped pruned rows' - // tags. - const hasStaleTags = Object.values(store.read(repoName)?.mrs ?? {}).some((e) => e.codeownerSections?.length); + // Containment: with sections never declared and no stale tag from a + // dropped demand, there is nothing to write and nothing to clear, so + // this stays a no-op call away from bit-identical behavior. It runs + // (replaceAll) when sections are live OR when `hasStaleTags` (above) + // found a tag from before this cycle -- covering both a pruned row + // (already cleaned by fullSync's own prune) and a still-in-scope row + // whose match just disappeared (the rollback's one permitted extra + // write, spec Decision 4). if (sections.length > 0 || hasStaleTags) { store.setSectionTags(repoName, tags, { replaceAll: true }); } @@ -419,7 +426,14 @@ export async function backfillAuthors( const selfUsername = overrides.selfUsername !== undefined ? overrides.selfUsername : getSelfUsername(); const union = new Set([...(record?.scope?.authors ?? []), ...authors]); if (selfUsername) union.add(selfUsername); - store.setScope(repoName, { authors: [...union].sort(), windowDays }); + // setScope is a full replace -- an existing scope's sections (set by the + // deep sweep) must be carried forward explicitly or this call erases them + // until the next deep. + store.setScope(repoName, { + authors: [...union].sort(), + ...(record?.scope?.sections ? { sections: record.scope.sections } : {}), + windowDays, + }); if (changed.length > 0) { deps.broadcast("project-mrs", { repoName, iids: changed }); From e1f0398d220c683f6220f382ab4604c3177db2bf Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 18:24:33 -0500 Subject: [PATCH 06/13] daemon: delta retags sections and keeps tagged strangers --- .../__tests__/project-mrs-store.test.ts | 12 +++ lib/daemon/__tests__/project-sync.test.ts | 102 ++++++++++++++++++ lib/daemon/project-mrs-store.ts | 7 +- lib/daemon/project-sync.ts | 52 ++++++++- 4 files changed, 170 insertions(+), 3 deletions(-) diff --git a/lib/daemon/__tests__/project-mrs-store.test.ts b/lib/daemon/__tests__/project-mrs-store.test.ts index 5aba5bf2..b6f29243 100644 --- a/lib/daemon/__tests__/project-mrs-store.test.ts +++ b/lib/daemon/__tests__/project-mrs-store.test.ts @@ -251,6 +251,18 @@ describe("applyDelta guards (review fixes)", () => { expect(s.read("repo")!.mrs[1]!.pr.state).toBe("merged"); expect(changed).toEqual([]); }); + + // Controller ruling (Task 6): applyDelta replaces entries wholesale, so a + // delta-updated tagged row would otherwise lose its in-memory tag while + // its SQL project_mr_sections row survives untouched. + test("delta preserves an existing codeownerSections tag on the replacement entry", () => { + const s = tmpStore(); + s.fullSync("repo", "g/p", [pr(1)], Date.now() - 10_000); + s.setSectionTags("repo", { 1: ["Acme"] }); + s.applyDelta("repo", "g/p", [pr(1, { title: "updated" } as any)], Date.now()); + expect(s.read("repo")!.mrs[1]!.pr.title).toBe("updated"); + expect(s.read("repo")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + }); }); describe("demand registry", () => { diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index 7a81431b..d1323d82 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -1110,3 +1110,105 @@ describe("codeowner section sweep (deep)", () => { }); }); }); + +describe("delta retag and keep-tagged-strangers", () => { + /** Seeds a real scope {authors:["ada","self"], sections:["Acme"]} via one deep sync; iid 9 is not author-covered so it's hydrated and tagged. */ + async function seededSectionStore() { + const store = tmpStore(); + const deps = { repoIndex: () => ({ r: "/tmp/repo" }), broadcast: () => {} }; + store.registerDemand("r", "board:1", ["ada"], 1, ["Acme"]); + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 9, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + fetchSingle: async (_r, _p, iid) => pr(iid, { author: { username: "stranger" } as any }), + }); + return { store, deps }; + } + + /** Seeds a real scope {authors:["ada","self"]} only -- no demand ever declared a section. */ + async function seededAuthorOnlyStore() { + const store = tmpStore(); + const deps = { repoIndex: () => ({ r: "/tmp/repo" }), broadcast: () => {} }; + store.registerDemand("r", "board:1", ["ada"], 1); + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, mode: "deep", + fetchAuthors: async () => ({ projectPath: "g/p", prs: [] }), + }); + return { store, deps }; + } + + test("delta keeps a tagged stranger's update and retags from the cycle's rules", async () => { + const { store, deps } = await seededSectionStore(); // deep already ran: iid 9 tagged, stored + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [ + pr(9, { author: { username: "stranger" } as any, title: "v2" }), + pr(3, { author: { username: "stranger" } as any }), + ] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 9, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + fetchSingle: async () => { throw new Error("nothing to hydrate"); }, + }); + const rec = store.read("r")!; + expect(rec.mrs[9]!.pr.title).toBe("v2"); // tagged stranger's update kept + expect(rec.mrs[9]!.codeownerSections).toEqual(["Acme"]); + expect(rec.mrs[3]).toBeUndefined(); // untagged stranger filtered + }); + + test("delta untags an MR whose rule got approved in-window", async () => { + const { store, deps } = await seededSectionStore(); + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 9, rules: [{ type: "CODE_OWNER", approved: true, section: "Acme" }] }, + ] }), + }); + // Tag cleared; the row itself waits for the deep prune. + expect(store.read("r")!.mrs[9]!.codeownerSections).toBeUndefined(); + }); + + test("delta hydrates AND tags a brand-new match in the same cycle", async () => { + const { store, deps } = await seededSectionStore(); // iid 4 unknown to the store + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 4, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + fetchSingle: async (_r, _p, iid) => pr(iid, { author: { username: "stranger" } as any }), + }); + const rec = store.read("r")!; + expect(rec.mrs[4]).toBeDefined(); + expect(rec.mrs[4]!.codeownerSections).toEqual(["Acme"]); // tag applied after hydration + }); + + test("delta with no section scope never calls fetchRules (containment)", async () => { + const { store, deps } = await seededAuthorOnlyStore(); // scope {authors} only + let rulesCalled = 0; + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => { rulesCalled++; return { projectPath: "g/p", rules: [] }; }, + }); + expect(rulesCalled).toBe(0); + }); + + test("a failing retag this cycle still keeps the tag (controller ruling: applyDelta preserves it)", async () => { + const { store, deps } = await seededSectionStore(); // iid 9 tagged, stored + await syncProjectMRs(deps, "r", { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [ + pr(9, { author: { username: "stranger" } as any, title: "v2" }), + ] }), + fetchRules: async () => { throw new Error("rules endpoint down"); }, + }); + const rec = store.read("r")!; + expect(rec.mrs[9]!.pr.title).toBe("v2"); // delta update still applied + expect(rec.mrs[9]!.codeownerSections).toEqual(["Acme"]); // tag survives despite the failed retag + }); +}); diff --git a/lib/daemon/project-mrs-store.ts b/lib/daemon/project-mrs-store.ts index 083d24c8..728566a3 100644 --- a/lib/daemon/project-mrs-store.ts +++ b/lib/daemon/project-mrs-store.ts @@ -302,7 +302,12 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR ? ({ ...pr, divergedCommitsCount: prevDiverged } as PullRequest) : pr; const fetchedAt = Date.now(); - store.mrs[pr.iid] = { pr: toStore, fetchedAt }; + // The tag lives on the ENTRY, not the pr, and this replaces the entry + // wholesale -- carry it forward or a same-cycle retag failure erases it + // from memory while its project_mr_sections row survives untouched. + store.mrs[pr.iid] = existing?.codeownerSections + ? { pr: toStore, fetchedAt, codeownerSections: existing.codeownerSections } + : { pr: toStore, fetchedAt }; changed.push(pr.iid); toWrite.push({ iid: pr.iid, pr: toStore, fetchedAt }); } diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index 6ba1fd97..f75f776f 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -320,6 +320,34 @@ async function syncImpl( }); let { projectPath, prs } = await fetchDelta(repoName, updatedAfter); + + // Sections-delta: re-sweep approval rules over the same delta window so a + // codeowner tag heals every cycle, not just on the once-a-day deep. A + // failed sweep must not fail the delta that keeps freshness flowing -- + // it heals on the next successful cycle or via approved events. + let freshTags: Map | null = null; + if (record?.scope?.sections?.length) { + const sweepStartedAt = Date.now(); + try { + const { rules } = await fetchRules(repoName, { updatedAfter }); + freshTags = new Map(rules.map((r) => [r.iid, sectionsMatching(r.rules, record.scope!.sections!)])); + log.debug( + { repo: repoName, mode: "sections-delta", candidates: rules.length, matched: [...freshTags.values()].filter((s) => s.length > 0).length, durationMs: Date.now() - sweepStartedAt }, + "project sync", + ); + } catch (err) { + log.warn({ err, repo: repoName }, "section retag failed"); + } + } + // Fresh sweep result wins when present; otherwise fall back to the + // currently stored tag (covers both "no sections demanded" and "sweep + // failed this cycle") -- never guess a stranger untagged. + const taggedNow = (iid: number): boolean => { + const fresh = freshTags?.get(iid); + if (fresh !== undefined) return fresh.length > 0; + return (record?.mrs[iid]?.codeownerSections?.length ?? 0) > 0; + }; + // A demand-scoped repo's delta window still queries the whole project // (updatedAfter has no author filter), so drop anything outside scope here // rather than letting it back into a store the deep sync just pruned it from. @@ -327,11 +355,31 @@ async function syncImpl( const authors = record.scope.authors; // A missing author is never guess-dropped (same rule as withinWindow and // the events-mapping upsertProject filter) -- there is no way to tell - // which side of the scope it belongs on. - prs = prs.filter((pr) => !pr.author?.username || authors.includes(pr.author.username)); + // which side of the scope it belongs on. A tagged stranger is kept too: + // codeowner-relevant, even though no demand named this author. + prs = prs.filter((pr) => !pr.author?.username || authors.includes(pr.author.username) || taggedNow(pr.iid)); } const changed = store.applyDelta(repoName, projectPath, prs, deltaStartedAt); + // Persist the retag, and hydrate any brand-new match the delta window + // didn't carry. Hydrate BEFORE tagging: setSectionTags skips iids with no + // stored row, so tagging first would leave a newly matched MR untagged -- + // and it would then be filtered out as an untagged stranger every delta + // until the next deep. + if (freshTags) { + const current = store.read(repoName); + const toHydrate = [...freshTags].filter(([iid, s]) => s.length > 0 && !current?.mrs[iid]).map(([iid]) => iid); + for (const iid of toHydrate) { + try { + const pr = await fetchSingle(repoName, projectPath, iid); + if (pr) changed.push(...store.upsert(repoName, projectPath, pr, "events")); + } catch (err) { + log.warn({ err, repo: repoName, iid }, "section hydration failed"); + } + } + store.setSectionTags(repoName, Object.fromEntries(freshTags)); + } + // Pipeline top-up: pipeline transitions bump no MR timestamp, so they // miss deltas (same blind spot as the events feed). Refresh the MRs whose // STORED pipeline is still in flight and that this delta didn't already From c533f7c900cc4846855fc6465225af1cc340f74e Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 18:26:09 -0500 Subject: [PATCH 07/13] daemon: drop process-artifact citations from new test comments --- lib/daemon/__tests__/project-mrs-store.test.ts | 6 +++--- lib/daemon/__tests__/project-sync.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/daemon/__tests__/project-mrs-store.test.ts b/lib/daemon/__tests__/project-mrs-store.test.ts index b6f29243..b8b22ce9 100644 --- a/lib/daemon/__tests__/project-mrs-store.test.ts +++ b/lib/daemon/__tests__/project-mrs-store.test.ts @@ -252,9 +252,9 @@ describe("applyDelta guards (review fixes)", () => { expect(changed).toEqual([]); }); - // Controller ruling (Task 6): applyDelta replaces entries wholesale, so a - // delta-updated tagged row would otherwise lose its in-memory tag while - // its SQL project_mr_sections row survives untouched. + // applyDelta replaces entries wholesale, so a tag on the entry must be + // carried forward the same way divergedCommitsCount already is -- else its + // SQL project_mr_sections row would outlive the in-memory tag. test("delta preserves an existing codeownerSections tag on the replacement entry", () => { const s = tmpStore(); s.fullSync("repo", "g/p", [pr(1)], Date.now() - 10_000); diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index d1323d82..02ad12cb 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -1198,7 +1198,7 @@ describe("delta retag and keep-tagged-strangers", () => { expect(rulesCalled).toBe(0); }); - test("a failing retag this cycle still keeps the tag (controller ruling: applyDelta preserves it)", async () => { + test("a failing retag this cycle still keeps the tag: applyDelta preserves it in memory", async () => { const { store, deps } = await seededSectionStore(); // iid 9 tagged, stored await syncProjectMRs(deps, "r", { store, selfUsername: "self", windowDays: 30, From cbbc4a7eb8203af5ca697c1f02a5f63955c92abc Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 18:43:51 -0500 Subject: [PATCH 08/13] daemon: preserve tags through upsert, cover delta untag ordering, broadcast tag-only changes --- .../__tests__/project-mrs-store.test.ts | 12 ++++++++ lib/daemon/__tests__/project-sync.test.ts | 28 +++++++++++++++++-- lib/daemon/project-mrs-store.ts | 8 +++++- lib/daemon/project-sync.ts | 14 ++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lib/daemon/__tests__/project-mrs-store.test.ts b/lib/daemon/__tests__/project-mrs-store.test.ts index b8b22ce9..644e7eb2 100644 --- a/lib/daemon/__tests__/project-mrs-store.test.ts +++ b/lib/daemon/__tests__/project-mrs-store.test.ts @@ -51,6 +51,18 @@ describe("upsert", () => { expect(s.read("repo")!.mrs[2]!.pr.state).toBe("merged"); expect(s.read("repo")!.source).toBe("events"); }); + + // upsert replaces the entry wholesale, same as applyDelta -- an + // events-path upsert of a tagged MR must carry the tag forward or it + // desyncs from the SQL project_mr_sections row. + test("preserves an existing codeownerSections tag on the replacement entry", () => { + const s = tmpStore(); + s.fullSync("repo", "g/p", [pr(1)], Date.now() - 10_000); + s.setSectionTags("repo", { 1: ["Acme"] }); + s.upsert("repo", null, pr(1, { title: "updated" } as any), "events"); + expect(s.read("repo")!.mrs[1]!.pr.title).toBe("updated"); + expect(s.read("repo")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + }); }); describe("fullSync reconcile", () => { diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index 02ad12cb..2cecf3a3 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -1161,15 +1161,23 @@ describe("delta retag and keep-tagged-strangers", () => { test("delta untags an MR whose rule got approved in-window", async () => { const { store, deps } = await seededSectionStore(); + // iid 9 also carries a pr update this cycle, so applyDelta's own + // preserve-copy runs on the same entry the fresh sweep is about to + // clear -- proving the sweep's clear wins over the preserve, not just + // that setSectionTags can clear a row applyDelta never touched. await syncProjectMRs(deps, "r", { store, selfUsername: "self", windowDays: 30, - fetchDelta: async () => ({ projectPath: "g/p", prs: [] }), + fetchDelta: async () => ({ projectPath: "g/p", prs: [ + pr(9, { author: { username: "self" } as any, title: "v2" }), + ] }), fetchRules: async () => ({ projectPath: "g/p", rules: [ { iid: 9, rules: [{ type: "CODE_OWNER", approved: true, section: "Acme" }] }, ] }), }); + const rec = store.read("r")!; + expect(rec.mrs[9]!.pr.title).toBe("v2"); // the delta update itself still landed // Tag cleared; the row itself waits for the deep prune. - expect(store.read("r")!.mrs[9]!.codeownerSections).toBeUndefined(); + expect(rec.mrs[9]!.codeownerSections).toBeUndefined(); }); test("delta hydrates AND tags a brand-new match in the same cycle", async () => { @@ -1211,4 +1219,20 @@ describe("delta retag and keep-tagged-strangers", () => { expect(rec.mrs[9]!.pr.title).toBe("v2"); // delta update still applied expect(rec.mrs[9]!.codeownerSections).toEqual(["Acme"]); // tag survives despite the failed retag }); + + test("a tag-only change (untagged this cycle, no pr change) still broadcasts", async () => { + const { store, deps } = await seededSectionStore(); // iid 9 tagged, stored + const events: Array<{ type: string; data: any }> = []; + await syncProjectMRs( + { ...deps, broadcast: (type, data) => events.push({ type, data }) }, + "r", + { + store, selfUsername: "self", windowDays: 30, + fetchDelta: async () => ({ projectPath: "g/p", prs: [] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [{ iid: 9, rules: [] }] }), + }, + ); + expect(store.read("r")!.mrs[9]!.codeownerSections).toBeUndefined(); + expect(events).toEqual([{ type: "project-mrs", data: { repoName: "r", iids: [9] } }]); + }); }); diff --git a/lib/daemon/project-mrs-store.ts b/lib/daemon/project-mrs-store.ts index 728566a3..65dd3082 100644 --- a/lib/daemon/project-mrs-store.ts +++ b/lib/daemon/project-mrs-store.ts @@ -195,8 +195,14 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR if (!path) return []; // never synced and caller has no path: no record to attach to const store = existing ?? emptyStore(path, source); store.projectPath = path; + const existingEntry = store.mrs[pr.iid]; const fetchedAt = Date.now(); - store.mrs[pr.iid] = { pr, fetchedAt }; + // Same wholesale-replace hole applyDelta has: the tag lives on the + // ENTRY, not the pr, so an events/mutation upsert of a tagged MR must + // carry it forward or it desyncs from the SQL project_mr_sections row. + store.mrs[pr.iid] = existingEntry?.codeownerSections + ? { pr, fetchedAt, codeownerSections: existingEntry.codeownerSections } + : { pr, fetchedAt }; store.source = source; data[repoName] = store; diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index f75f776f..c726540b 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -71,6 +71,11 @@ export function sectionsMatching(rules: ApprovalRuleLite[], demanded: string[]): return demanded.filter((s) => rules.some((r) => r.type === "CODE_OWNER" && !r.approved && r.section === s)); } +/** Order-sensitive equality on two optional tag lists; both sides come from sectionsMatching's stable demanded-order output. */ +function sameSections(a: string[] | undefined, b: string[] | undefined): boolean { + return (a ?? []).length === (b ?? []).length && (a ?? []).every((s, i) => s === (b ?? [])[i]); +} + /** Drops PRs whose updatedAt is older than the window. A missing/unparseable timestamp is kept -- never guess-drop. */ function withinWindow(prs: PullRequest[], windowDays: number, now: number): PullRequest[] { const cutoff = now - windowDays * 86_400_000; @@ -326,11 +331,19 @@ async function syncImpl( // failed sweep must not fail the delta that keeps freshness flowing -- // it heals on the next successful cycle or via approved events. let freshTags: Map | null = null; + // A tag-only change (a stranger newly tagged or untagged, pr itself + // untouched) never lands in applyDelta's changed set on its own -- without + // this, clients keep stale tab membership until an unrelated pr change or + // the next deep. + const tagChangedIids: number[] = []; if (record?.scope?.sections?.length) { const sweepStartedAt = Date.now(); try { const { rules } = await fetchRules(repoName, { updatedAfter }); freshTags = new Map(rules.map((r) => [r.iid, sectionsMatching(r.rules, record.scope!.sections!)])); + for (const [iid, sections] of freshTags) { + if (!sameSections(sections, record.mrs[iid]?.codeownerSections)) tagChangedIids.push(iid); + } log.debug( { repo: repoName, mode: "sections-delta", candidates: rules.length, matched: [...freshTags.values()].filter((s) => s.length > 0).length, durationMs: Date.now() - sweepStartedAt }, "project sync", @@ -422,6 +435,7 @@ async function syncImpl( for (const pr of fetched) { if (pr) changed.push(...store.upsert(repoName, projectPath, pr, "events")); } + for (const iid of tagChangedIids) if (!changed.includes(iid)) changed.push(iid); log.debug({ repo: repoName, mode: "delta", changed: changed.length, topup: topup.length, durationMs: Date.now() - deltaStartedAt }, "project sync"); if (changed.length > 0) { From d9e499acc24cf5976be3b255da3ff3e813a34988 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 18:56:23 -0500 Subject: [PATCH 09/13] daemon: project-mrs:read speaks the sections axis (validation, tags, backfill) --- lib/daemon/__tests__/project-sync.test.ts | 44 +++++++++++++++++++++++ lib/daemon/handlers/project-mrs.ts | 43 +++++++++++++++++++--- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index 2cecf3a3..aee414b6 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -296,6 +296,31 @@ describe("project-mrs:read handler", async () => { expect(store.read("remote:repo")!.demands).toBeUndefined(); }); + test("demand with malformed codeownerSections is rejected", async () => { + const store = tmpStore(); + store.fullSync("remote:repo", "g/p", [], Date.now()); + const h = createProjectMRsHandlers(fakeCtx, () => {}, { store, sync: async () => {}, tracking: grantedTracking }); + const base = { client: "b:1", authors: ["x"], declaredAt: 1 }; + + const notArray = await h["project-mrs:read"]!( + { repoName: "remote:repo", demand: { ...base, codeownerSections: "Acme" } } as any, + ); + expect(notArray.ok).toBe(false); + + const empty = await h["project-mrs:read"]!({ repoName: "remote:repo", demand: { ...base, codeownerSections: [] } }); + expect(empty.ok).toBe(false); // an empty list is meaningless; omit the field instead + + const emptyString = await h["project-mrs:read"]!({ repoName: "remote:repo", demand: { ...base, codeownerSections: [""] } }); + expect(emptyString.ok).toBe(false); + + const tooMany = await h["project-mrs:read"]!( + { repoName: "remote:repo", demand: { ...base, codeownerSections: Array.from({ length: 21 }, (_, i) => `s${i}`) } }, + ); + expect(tooMany.ok).toBe(false); + + expect(store.read("remote:repo")!.demands).toBeUndefined(); + }); + test("uncovered demanded authors are reported and kick a backfill on unforced reads", async () => { const store = tmpStore(); store.fullSync("remote:repo", "g/p", [], Date.now()); @@ -401,6 +426,25 @@ describe("project-mrs:read handler", async () => { expect(warns[0]!.obj.authors).toEqual(["newbie"]); }); + test("read returns entry tags and scope sections; uncoveredSections triggers backfill", async () => { + const store = tmpStore(); + store.fullSync("remote:repo", "g/p", [pr(1)], Date.now() - 60_000); + store.setSectionTags("remote:repo", { 1: ["Acme"] }); + store.setScope("remote:repo", { authors: ["ada"], sections: [], windowDays: 30 }); + const backfilled: string[][] = []; + const h = createProjectMRsHandlers(fakeCtx, () => {}, { + store, sync: async () => {}, tracking: grantedTracking, + sectionBackfill: async (_r, sections) => { backfilled.push(sections); }, + }); + const res = await h["project-mrs:read"]!({ repoName: "remote:repo", maxAgeMs: 0, + demand: { client: "b:1", authors: ["ada"], codeownerSections: ["Acme"], declaredAt: 1 } }); + expect(res.ok).toBe(true); + expect((dataOf(res) as any).mrs["1"].codeownerSections).toEqual(["Acme"]); + expect((dataOf(res) as any).scope.sections).toEqual([]); + expect((dataOf(res) as any).scope.uncoveredSections).toEqual(["Acme"]); + expect(backfilled).toEqual([["Acme"]]); + }); + test("uncovered is computed from the stored demand, not a stale request (finding 4)", async () => { const store = tmpStore(); store.fullSync("remote:repo", "g/p", [], Date.now()); diff --git a/lib/daemon/handlers/project-mrs.ts b/lib/daemon/handlers/project-mrs.ts index 6c65fc5e..c9441a78 100644 --- a/lib/daemon/handlers/project-mrs.ts +++ b/lib/daemon/handlers/project-mrs.ts @@ -20,7 +20,7 @@ import type { PullRequest } from "@mattstack/glance"; import { parseIdentity } from "../../settings/identity.ts"; import { loadRepoTracking, grants, type RepoTracking } from "../../repo-tracking.ts"; import { getProjectMRs, freshnessOf, type ProjectMRs } from "../project-mrs-store.ts"; -import { syncProjectMRs, backfillAuthors } from "../project-sync.ts"; +import { syncProjectMRs, backfillAuthors, backfillSections } from "../project-sync.ts"; import { getRepoContext } from "../freshness.ts"; import type { HandlerContext, HandlerMap, TypedHandlers } from "./types.ts"; import type { Commands } from "../../../packages/rt-client/src/commands.ts"; @@ -29,17 +29,22 @@ import type { Commands } from "../../../packages/rt-client/src/commands.ts"; interface DemandRequest { client: string; authors: string[]; + codeownerSections?: string[]; declaredAt: number; } /** Guards store writes: a bad demand must be rejected, never partially registered. */ function isValidDemand(d: unknown): d is DemandRequest { if (!d || typeof d !== "object") return false; - const { client, authors, declaredAt } = d as Record; + const { client, authors, declaredAt, codeownerSections } = d as Record; if (typeof client !== "string" || client.length === 0) return false; if (!Array.isArray(authors) || authors.length < 1 || authors.length > 200) return false; if (!authors.every((a) => typeof a === "string" && a.length > 0)) return false; if (typeof declaredAt !== "number" || !Number.isFinite(declaredAt)) return false; + if (codeownerSections !== undefined) { + if (!Array.isArray(codeownerSections) || codeownerSections.length < 1 || codeownerSections.length > 20) return false; + if (!codeownerSections.every((s) => typeof s === "string" && s.length > 0)) return false; + } return true; } @@ -58,6 +63,7 @@ export interface ProjectMRsHandlerOverrides { sync?: (repoName: string) => Promise; tracking?: () => RepoTracking; backfill?: (repoName: string, authors: string[]) => Promise; + sectionBackfill?: (repoName: string, sections: string[]) => Promise; /** * Returns projectPath explicitly alongside the PR so the write-back * upsert never depends on an implicit side channel (a prior version threaded @@ -78,6 +84,8 @@ export function createProjectMRsHandlers( const tracking = overrides.tracking ?? loadRepoTracking; const backfill = overrides.backfill ?? ((repoName: string, authors: string[]) => backfillAuthors({ repoIndex: ctx.repoIndex, broadcast }, repoName, authors)); + const sectionBackfill = overrides.sectionBackfill + ?? ((repoName: string, sections: string[]) => backfillSections({ repoIndex: ctx.repoIndex, broadcast }, repoName, sections)); return { "project-mrs:read": async ( payload: Commands["project-mrs:read"]["payload"], @@ -108,7 +116,7 @@ export function createProjectMRsHandlers( // Registered before the freshness gate so a forced read's awaited sync // (below) already sees this demand's authors. if (demand) { - store().registerDemand(repoName, demand.client, demand.authors, demand.declaredAt); + store().registerDemand(repoName, demand.client, demand.authors, demand.declaredAt, demand.codeownerSections); } if (typeof maxAgeMs === "number") { @@ -151,15 +159,40 @@ export function createProjectMRsHandlers( } } + // Sections axis mirrors the authors block above: read the STORED + // demand (not the raw request), dedupe, and forced reads await. + const demandedSections = demand ? [...new Set(record?.demands?.[demand.client]?.sections ?? [])] : []; + let coveredSections = new Set(record?.scope?.sections ?? []); + let uncoveredSections = demandedSections.filter((s) => !coveredSections.has(s)); + if (uncoveredSections.length > 0) { + const attemptedSections = uncoveredSections; + const run = sectionBackfill(repoName, attemptedSections); + const logSectionBackfillFailure = (err: unknown) => { + ctx.log.warn({ err, repo: repoName, sections: attemptedSections }, "section backfill failed"); + }; + if (maxAgeMs === 0) { + await run.catch(logSectionBackfillFailure); + record = store().read(repoName); + coveredSections = new Set(record?.scope?.sections ?? []); + uncoveredSections = demandedSections.filter((s) => !coveredSections.has(s)); + } else { + void run.catch(logSectionBackfillFailure); + } + } + if (!record) return { ok: true, data: { mrs: {}, listSyncedAt: 0, source: "poll", syncedAt: 0 } }; return { ok: true, data: { - mrs: record.mrs, + mrs: record.mrs, // verbatim: entries serialize as-is, so codeownerSections flows unmapped listSyncedAt: record.listSyncedAt, source: record.source, syncedAt: freshnessOf(record), - scope: record.scope ? { ...record.scope, uncovered } : undefined, + scope: record.scope ? { + ...record.scope, uncovered, + ...(demandedSections.length > 0 || record.scope.sections + ? { sections: record.scope.sections ?? [], uncoveredSections } : {}), + } : undefined, }, }; }, From f730952a1f5ec813618927b49fb1316af833f76c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 19:10:49 -0500 Subject: [PATCH 10/13] daemon: events keep tagged strangers; approved invalidations heal section tags --- .../__tests__/freshness-mapping.test.ts | 53 +++++++++++++++++++ lib/daemon/freshness.ts | 34 ++++++++++-- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/lib/daemon/__tests__/freshness-mapping.test.ts b/lib/daemon/__tests__/freshness-mapping.test.ts index 46f91434..186e18e5 100644 --- a/lib/daemon/__tests__/freshness-mapping.test.ts +++ b/lib/daemon/__tests__/freshness-mapping.test.ts @@ -621,6 +621,59 @@ describe("applyInvalidationBatch", () => { expect(store.read("repo-x")!.mrs[42]).toBeDefined(); }); + // ─── tagged strangers + approval heal (Task 8) ──────────────────────────── + + test("upsertProject keeps a tagged stranger despite the author scope filter", async () => { + const store = pmrsStore(); + store.fullSync("repo-x", "g/p", [fakePR(9, { sourceBranch: "branch-9", author: { id: "gid://gitlab/User/2", username: "stranger", name: "Stranger", avatarUrl: null } })], Date.now() - 1000); + store.setSectionTags("repo-x", { 9: ["Acme"] }); + store.setScope("repo-x", { authors: ["ada"], windowDays: 30 }); + const { env, broadcasts } = makeEnv({}); + const target: RepoTarget = { + repoName: "repo-x", projectPath: "g/p", + provider: { + fetchSingleMR: async (_pp: string, iid: number) => + fakePR(iid, { sourceBranch: "branch-9", title: "v2", author: { id: "gid://gitlab/User/2", username: "stranger", name: "Stranger", avatarUrl: null } }), + fetchPullRequestByBranch: async () => { throw new Error("unexpected"); }, + fetchPullRequestsByBranches: async () => { throw new Error("unexpected"); }, + } as any, + }; + await applyInvalidationBatch(env, target, makeRunner(), [key("mr", "9")], { + ...noNotify, grantsFor: projectGrants, projectStore: store, + }); + expect(store.read("repo-x")!.mrs[9]!.pr.title).toBe("v2"); // upserted, not dropped + expect(store.read("repo-x")!.mrs[9]!.codeownerSections).toEqual(["Acme"]); // tag preserved through upsert + expect(broadcasts.some((b) => b.type === "project-mrs" && b.data.iids.includes(9))).toBe(true); + }); + + test("an approved invalidation re-checks rules and untags", async () => { + const store = pmrsStore(); + store.fullSync("repo-x", "g/p", [fakePR(9, { sourceBranch: "branch-9" })], Date.now() - 1000); + store.setSectionTags("repo-x", { 9: ["Acme"] }); + store.setScope("repo-x", { authors: ["ada"], sections: ["Acme"], windowDays: 30 }); + const { env, broadcasts } = makeEnv({}); + const rulesCalls: number[][] = []; + const target: RepoTarget = { + repoName: "repo-x", projectPath: "g/p", + provider: { + fetchSingleMR: async (_pp: string, iid: number) => fakePR(iid, { sourceBranch: "branch-9" }), + fetchPullRequestByBranch: async () => { throw new Error("unexpected"); }, + fetchPullRequestsByBranches: async () => { throw new Error("unexpected"); }, + } as any, + }; + await applyInvalidationBatch(env, target, makeRunner(), [{ kind: "mr", ref: "9", cause: "approved" }], { + ...noNotify, grantsFor: projectGrants, projectStore: store, + fetchRulesByIid: async (iids: number[]) => { + rulesCalls.push(iids); + return [{ iid: 9, rules: [{ type: "CODE_OWNER", approved: true, section: "Acme" }] }]; + }, + }); + expect(rulesCalls).toEqual([[9]]); // rules re-checked for exactly this iid + expect(store.read("repo-x")!.mrs[9]!.codeownerSections).toBeUndefined(); // healed: rule is now approved + const projectBroadcasts = broadcasts.filter((b) => b.type === "project-mrs"); + expect(projectBroadcasts.some((b) => b.data.iids.includes(9))).toBe(true); + }); + test("notes: no discussions grant → no refresh; grant without cached discussions → no refresh; both → refresh", async () => { const entries: Record = { "feat-a": { mr: { iid: 42 }, fetchedAt: 1, repoName: "repo-x" }, diff --git a/lib/daemon/freshness.ts b/lib/daemon/freshness.ts index cfd7e569..a5f7ceba 100644 --- a/lib/daemon/freshness.ts +++ b/lib/daemon/freshness.ts @@ -27,7 +27,7 @@ */ import { execSync } from "child_process"; -import { GitLabProvider, type InvalidationKey, type PullRequest } from "@mattstack/glance"; +import { GitLabProvider, type InvalidationKey, type MRApprovalRules, type PullRequest } from "@mattstack/glance"; import { loadRepoTracking, grants, type RepoGrants } from "../repo-tracking.ts"; import { loadSecrets } from "../linear.ts"; import { parseRemoteUrl, isGitLabRemote, toMRInfo } from "../enrich.ts"; @@ -344,7 +344,7 @@ export const GAP_FILL_DEBOUNCE_MS = 5000; /** Narrow provider surface the mapping needs — tests stub this. */ export type TargetedFetcher = Pick< GitLabProvider, - "fetchSingleMR" | "fetchPullRequestByBranch" | "fetchPullRequestsByBranches" + "fetchSingleMR" | "fetchPullRequestByBranch" | "fetchPullRequestsByBranches" | "fetchApprovalRules" >; export interface RepoTarget { @@ -373,6 +373,8 @@ export interface MappingOverrides { grantsFor?: (repoName: string) => RepoGrants; // default: grants(loadRepoTracking(), repoName) projectStore?: ProjectMRs; // default: getProjectMRs() hasCachedDiscussions?: (repoName: string, iid: number) => boolean; // default: file-store read !== undefined + /** Approval-heal seam: rules for just these iids. default: provider.fetchApprovalRules({projectPath, iids}) */ + fetchRulesByIid?: (iids: number[]) => Promise; } /** @@ -438,9 +440,13 @@ async function processKeys( const upsertProject = (pr: PullRequest) => { // Demand-scoped repos only track the declared authors; an MR missing an // author never guess-drops, since we can't tell which side of the scope - // it belongs on. - const scope = pStore.read(repoName)?.scope; - if (scope && pr.author?.username && !scope.authors.includes(pr.author.username)) return; + // it belongs on. A tagged stranger (out-of-scope author, still section- + // tagged) is kept too: dropping it here would silently erase a tag that + // only the approval-heal path is meant to clear. + const rec = pStore.read(repoName); + const scope = rec?.scope; + const tagged = (rec?.mrs[pr.iid]?.codeownerSections?.length ?? 0) > 0; + if (scope && pr.author?.username && !scope.authors.includes(pr.author.username) && !tagged) return; const changed = pStore.upsert(repoName, projectPath, pr, "events"); if (changed.length > 0) env.broadcast("project-mrs", { repoName, iids: changed }); }; @@ -465,6 +471,24 @@ async function processKeys( ?? (pr && ctx.cache.entries[pr.sourceBranch]?.repoName === repoName ? pr.sourceBranch : undefined); if (feedBranch) mutated = updateEntry(env, repoName, feedBranch, pr) || mutated; if (wantProject && pr) upsertProject(pr); + // GitLab's approval action bumps no updatedAt, so this is the only + // signal that heals a tag within one events tick rather than + // waiting on the next deep/delta sweep. + if (k.cause === "approved" && wantProject) { + const rec = pStore.read(repoName); + const sections = rec?.scope?.sections ?? []; + if (sections.length > 0 && (rec?.mrs[iid]?.codeownerSections?.length ?? 0) > 0) { + const fetchRulesByIid = overrides.fetchRulesByIid + ?? (async (iids: number[]) => provider.fetchApprovalRules({ projectPath, iids })); + const rules = await fetchRulesByIid([iid]); + const match = rules.find((r) => r.iid === iid); + if (match) { + const { sectionsMatching } = await import("./project-sync.ts"); + pStore.setSectionTags(repoName, { [iid]: sectionsMatching(match.rules, sections) }); + env.broadcast("project-mrs", { repoName, iids: [iid] }); + } + } + } break; } case "notes": { From 6836602ea77a75deb1b677e321c746834490a2a1 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 21:56:46 -0500 Subject: [PATCH 11/13] fix: project_mr_demands.sections column survives a schema replay V4_SCHEMA's ALTER TABLE ADD COLUMN broke the migration chain's replay idempotency: every other statement in the combined DDL string is IF NOT EXISTS-safe, but ALTER TABLE has no such guard. A future SCHEMA_VERSION bump re-execs the whole string against every db already at v4, throws "duplicate column" on the ALTER, rolls back, and wedges every later openStateDb call. Moves the ALTER into its own conditional exec (addSectionsColumnIfMissing), gated on PRAGMA table_info not already listing the column, run right after the DDL string inside the same migration transaction. Any future ALTER-added column should follow this same pattern rather than join the DDL strings. Co-Authored-By: Claude Fable 5 --- lib/state/__tests__/db.test.ts | 27 +++++++++++++++++++++++++++ lib/state/db.ts | 29 ++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/state/__tests__/db.test.ts b/lib/state/__tests__/db.test.ts index 71c71f0e..48d61ef0 100644 --- a/lib/state/__tests__/db.test.ts +++ b/lib/state/__tests__/db.test.ts @@ -221,6 +221,33 @@ describe("openStateDb — reopen is a no-op", () => { expect(importCount).toBe(1); // not re-imported db2.close(); }); + + test("a future SCHEMA_VERSION bump replaying the full DDL string against an already-v6 db does not throw on the sections column", () => { + const dbPath = join(dir, "state.db"); + const db1 = openStateDb(dbPath, "cli"); + expect(userVersion(db1)).toBe(SCHEMA_VERSION); + db1.close(); + + // Force user_version back below SCHEMA_VERSION on a db that already has + // the v6 shape (sections column included) -- exactly what every existing + // v6 db looks like to a future SCHEMA_VERSION bump, whose migration + // re-execs this same combined DDL string. + const raw = new Database(dbPath); + raw.exec(`PRAGMA user_version = ${SCHEMA_VERSION - 1};`); + raw.close(); + + let db2: Database | undefined; + expect(() => { + db2 = openStateDb(dbPath, "cli"); + }).not.toThrow(); + + expect(userVersion(db2!)).toBe(SCHEMA_VERSION); + const sectionsColumns = (db2!.query("PRAGMA table_info(project_mr_demands);").all() as { name: string }[]).filter( + (c) => c.name === "sections", + ); + expect(sectionsColumns).toHaveLength(1); + db2!.close(); + }); }); describe("legacy import seam", () => { diff --git a/lib/state/db.ts b/lib/state/db.ts index daaa5e7b..2bb252e9 100644 --- a/lib/state/db.ts +++ b/lib/state/db.ts @@ -248,9 +248,23 @@ CREATE TABLE IF NOT EXISTS project_mr_sections ( sections TEXT NOT NULL, -- JSON string[] PRIMARY KEY (repo, iid) ); -ALTER TABLE project_mr_demands ADD COLUMN sections TEXT; `; +/** project_mr_demands.sections (v6): SQLite's ALTER TABLE ADD COLUMN has no + IF NOT EXISTS, so unlike every statement in the V*_SCHEMA strings above it + cannot simply replay -- a future SCHEMA_VERSION bump re-execs this whole + combined block against every db already at v6, and an unconditional ALTER + would throw "duplicate column", rolling back that migration and wedging + every later openStateDb call. Run it here instead, inside the same + migration transaction, gated on the column's actual absence. Any future + ALTER-added column follows this same conditional-exec pattern, never the + DDL strings. */ +function addSectionsColumnIfMissing(db: Database): void { + const columns = db.query("PRAGMA table_info(project_mr_demands);").all() as { name: string }[]; + if (columns.some((c) => c.name === "sections")) return; + db.exec("ALTER TABLE project_mr_demands ADD COLUMN sections TEXT;"); +} + /** bun:sqlite error codes that mean "the file on disk is not a usable db". */ function isCorruptionError(err: unknown): boolean { const code = (err as { code?: string } | undefined)?.code; @@ -367,10 +381,14 @@ function importLegacyStores(db: Database, dir: string): string[] { /** * The race-proof migration runner (spec "Schema versioning"): BEGIN * IMMEDIATE takes the write lock up front, user_version is RE-READ inside - * the transaction, and all DDL is IF NOT EXISTS. Two processes racing at - * v0: the loser blocks on IMMEDIATE (busy_timeout), then sees v1 inside its - * own transaction and applies nothing. A throwing migration rolls back and - * propagates — no swallow. + * the transaction, and every statement in the combined DDL string is IF NOT + * EXISTS-safe (the one exception, an ALTER TABLE ADD COLUMN, lives outside + * that string as its own conditional exec -- see addSectionsColumnIfMissing + * -- and any future ALTER-added column must follow that same pattern rather + * than join the DDL string). Two processes racing at v0: the loser blocks on + * IMMEDIATE (busy_timeout), then sees v1 inside its own transaction and + * applies nothing. A throwing migration rolls back and propagates, no + * swallow. */ function runMigrations(db: Database, dir: string): void { db.exec("BEGIN IMMEDIATE;"); @@ -382,6 +400,7 @@ function runMigrations(db: Database, dir: string): void { // statement is IF NOT EXISTS, so replaying v1's DDL against an // already-v1 db is a no-op and existing rows are untouched. db.exec(V1_SCHEMA + V2_SCHEMA + V3_SCHEMA + V4_SCHEMA + V6_SCHEMA); + addSectionsColumnIfMissing(db); // Legacy-JSON import is single-shot and only correct from a true // v0 (never-migrated) database: branch-cache's UPSERT would silently // overwrite current rows with stale ones, and project-mrs-store's From d73e1cd6af04c0c4c7d48de247c511dbe69e327b Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 22:54:56 -0500 Subject: [PATCH 12/13] fix: rt-client registry suite-key count after rebase merges two independent additions Main added mattstack.mode and this branch added board.tabs to the settings registry; both bumped the suiteKeys count from 34 to 35 independently, so the rebase's line-level merge silently kept toHaveLength(35) even though the combined array now has 36 entries. --- packages/rt-client/src/settings/__tests__/registry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rt-client/src/settings/__tests__/registry.test.ts b/packages/rt-client/src/settings/__tests__/registry.test.ts index de5ce79d..7d567a1a 100644 --- a/packages/rt-client/src/settings/__tests__/registry.test.ts +++ b/packages/rt-client/src/settings/__tests__/registry.test.ts @@ -243,7 +243,7 @@ describe("settings/registry", () => { "chat.push.provider", "chat.push.target", ]; - expect(suiteKeys).toHaveLength(35); + expect(suiteKeys).toHaveLength(36); expect(allDefs().map((d) => d.key).sort()).toEqual( [...migratedFalseKeys, ...migratedTrueKeys, ...suiteKeys].sort(), From 58e70aea51b938cdebec0c597d359e117da02c8d Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 23:23:17 -0500 Subject: [PATCH 13/13] fix: sectionsMatching returns sorted sections so tag comparisons are order-stable --- lib/daemon/__tests__/project-sync.test.ts | 31 +++++++++++++++++++++++ lib/daemon/project-sync.ts | 8 +++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/daemon/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index aee414b6..2437da6f 100644 --- a/lib/daemon/__tests__/project-sync.test.ts +++ b/lib/daemon/__tests__/project-sync.test.ts @@ -1264,6 +1264,37 @@ describe("delta retag and keep-tagged-strangers", () => { expect(rec.mrs[9]!.codeownerSections).toEqual(["Acme"]); // tag survives despite the failed retag }); + test("sectionsMatching sorts its output regardless of demanded's order", () => { + expect(sectionsMatching( + [{ type: "CODE_OWNER", approved: false, section: "Beta" }, + { type: "CODE_OWNER", approved: false, section: "Acme" }], + ["Beta", "Acme"], + )).toEqual(["Acme", "Beta"]); + }); + + test("backfillSections tags in delta's canonical order -- no post-backfill churn (CodeRabbit R1)", async () => { + const { store, deps } = await seededSectionStore(); // scope.sections ["Acme"], iid 9 already tagged + const rules = { projectPath: "g/p", rules: [ + { iid: 9, rules: [ + { type: "CODE_OWNER", approved: false, section: "Acme" }, + { type: "CODE_OWNER", approved: false, section: "Beta" }, + ] }, + ] }; + // backfillSections is called with the client's declared (unsorted) order, + // not scope.sections' sorted order -- this is the actual order mismatch + // CodeRabbit flagged between backfill and the delta/deep sweeps. + await backfillSections(deps, "r", ["Beta", "Acme"], { store, windowDays: 30, fetchRules: async () => rules }); + expect(store.read("r")!.mrs[9]!.codeownerSections).toEqual(["Acme", "Beta"]); // sorted despite unsorted demanded order + + const events: Array<{ type: string; data: any }> = []; + await syncProjectMRs( + { ...deps, broadcast: (type, data) => events.push({ type, data }) }, + "r", + { store, selfUsername: "self", windowDays: 30, fetchDelta: async () => ({ projectPath: "g/p", prs: [] }), fetchRules: async () => rules }, + ); + expect(events).toEqual([]); // same set backfill just wrote -- no spurious tag-change broadcast + }); + test("a tag-only change (untagged this cycle, no pr change) still broadcasts", async () => { const { store, deps } = await seededSectionStore(); // iid 9 tagged, stored const events: Array<{ type: string; data: any }> = []; diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index c726540b..690c2db7 100644 --- a/lib/daemon/project-sync.ts +++ b/lib/daemon/project-sync.ts @@ -66,12 +66,14 @@ export function effectiveSections(record: ProjectMRStore | undefined): string[] return [...sections].sort(); } -/** Demanded sections with an unapproved CODE_OWNER rule; [] when none. */ +/** Demanded sections with an unapproved CODE_OWNER rule, sorted; [] when none. Sorted here so every producer (deep, delta, backfill) stores the same canonical order regardless of `demanded`'s order. */ export function sectionsMatching(rules: ApprovalRuleLite[], demanded: string[]): string[] { - return demanded.filter((s) => rules.some((r) => r.type === "CODE_OWNER" && !r.approved && r.section === s)); + return demanded + .filter((s) => rules.some((r) => r.type === "CODE_OWNER" && !r.approved && r.section === s)) + .sort(); } -/** Order-sensitive equality on two optional tag lists; both sides come from sectionsMatching's stable demanded-order output. */ +/** Order-sensitive equality on two optional tag lists; both sides come from sectionsMatching's sorted output, so order-sensitivity is safe. */ function sameSections(a: string[] | undefined, b: string[] | undefined): boolean { return (a ?? []).length === (b ?? []).length && (a ?? []).every((s, i) => s === (b ?? [])[i]); }