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/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__/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/__tests__/project-mrs-store.test.ts b/lib/daemon/__tests__/project-mrs-store.test.ts index 0fc85f0e..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", () => { @@ -251,6 +263,18 @@ describe("applyDelta guards (review fixes)", () => { expect(s.read("repo")!.mrs[1]!.pr.state).toBe("merged"); expect(changed).toEqual([]); }); + + // 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); + 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", () => { @@ -323,6 +347,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/__tests__/project-sync.test.ts b/lib/daemon/__tests__/project-sync.test.ts index 0f2d7941..2437da6f 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, 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"; @@ -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()); @@ -890,3 +934,380 @@ 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 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: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [ + { iid: 1, rules: [{ type: "CODE_OWNER", approved: false, section: "Acme" }] }, + ] }), + }); + expect(store.read("r2")!.mrs[1]!.codeownerSections).toEqual(["Acme"]); + + // 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: [pr(1, { author: { username: "ada" } as any })] }), + fetchRules: async () => ({ projectPath: "g/p", rules: [] }), + }); + const rec = store.read("r2")!; + 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 () => { + 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 }); + }); + + 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(); + }); + }); +}); + +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(); + // 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: [ + 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(rec.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: applyDelta preserves it in memory", 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 + }); + + 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 }> = []; + 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/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": { 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, }, }; }, diff --git a/lib/daemon/project-mrs-store.ts b/lib/daemon/project-mrs-store.ts index 386a154a..65dd3082 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( @@ -173,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; @@ -243,7 +271,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(); @@ -277,7 +308,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 }); } @@ -318,19 +354,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 +392,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 +440,7 @@ export function createProjectMRs(db: Database = getStateDb("daemon")): ProjectMR registerDemand, expireDemands, setScope, + setSectionTags, }; } diff --git a/lib/daemon/project-sync.ts b/lib/daemon/project-sync.ts index eb9dee2d..690c2db7 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,25 @@ 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, 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)) + .sort(); +} + +/** 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]); +} + /** 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 +107,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 +162,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,13 +214,70 @@ 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); + // 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 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 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 }); + } + 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 }, + { 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 +294,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 }); } @@ -237,6 +327,42 @@ 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; + // 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", + ); + } 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. @@ -244,21 +370,38 @@ 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 // 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); @@ -294,8 +437,9 @@ 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 }, "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 }); } @@ -346,7 +490,72 @@ 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 }); + } +} + +/** + * 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 }); diff --git a/lib/state/__tests__/db.test.ts b/lib/state/__tests__/db.test.ts index c01e4109..48d61ef0 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"); @@ -220,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", () => { @@ -390,7 +418,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..2bb252e9 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,34 @@ 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) +); +`; + +/** 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; @@ -353,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;"); @@ -367,7 +399,8 @@ 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); + 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 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..7d567a1a 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", @@ -242,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(), 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) ---------------------------------------------------------- {