diff --git a/.github/scripts/issue-triage-autoclose.cjs b/.github/scripts/issue-triage-autoclose.cjs index 99ef2d0cec..0d7c2f14e6 100644 --- a/.github/scripts/issue-triage-autoclose.cjs +++ b/.github/scripts/issue-triage-autoclose.cjs @@ -66,7 +66,11 @@ function normalizeSignatureLine(raw) { // case-bearing discriminators; the "exact signature" promise is only true // if the comparison keeps them distinct. return String(raw || "") - .replace(//g, " ") + // An UNCLOSED comment hides everything after it through EOF, exactly as the + // quality parser treats it. Matching only `` let hidden text + // become the "exact shared signature" that closes an issue: two reports + // whose visible bodies differ could match on text GitHub never renders. + .replace(/|$)/g, " ") .replace(/^\s*(?:>|[-*+]\s+|\d+[.)]\s+)/, "") .replace(/^[`~]{3,}[^\n]*$/, "") // Backticks are unambiguous Markdown code delimiters. Preserve `_`, `~`, diff --git a/scripts/build-release-changelog.ts b/scripts/build-release-changelog.ts index ee4e24ef6d..c7868db572 100644 --- a/scripts/build-release-changelog.ts +++ b/scripts/build-release-changelog.ts @@ -118,14 +118,25 @@ export function categoryForPull(pr: AssociatedPullRequest): string { * Prerelease: newest prior release of either channel, so previews stay incremental. * Stable: newest prior stable only, so the final changelog always reconstructs * the complete stable-to-stable range and cannot lose prerelease changes. + * + * `isAncestor` filters to tags actually reachable from the target. Without it a + * preview could select the newest STABLE tag, which lives on `main` and is not + * reachable from `preview` — the ancestry guard then threw and no preview could + * be released at all once the stable lineage advanced. Selecting the newest + * REACHABLE release keeps previews incremental and keeps the range honest. */ -export function selectReleaseBaseline(version: string, tags: string[]): string | null { +export function selectReleaseBaseline( + version: string, + tags: string[], + isAncestor?: (tag: string) => boolean, +): string | null { const releaseTag = version.startsWith("v") ? version : `v${version}`; const targetIsPrerelease = isPrereleaseVersion(version); const candidates = tags .map(tag => tag.trim()) .filter(tag => /^v\d/.test(tag) && compareReleaseTags(tag, releaseTag) < 0) .filter(tag => targetIsPrerelease || !isPrereleaseVersion(tag)) + .filter(tag => isAncestor === undefined || isAncestor(tag)) .sort(compareReleaseTags); return candidates.length > 0 ? candidates[candidates.length - 1]! : null; } @@ -530,16 +541,23 @@ async function main(argv: string[]): Promise { const tags = (await commandText(["git", "tag", "--list", "v[0-9]*"])) .split(/\r?\n/) .filter(Boolean); - const baseline = selectReleaseBaseline(version, tags); + // Resolve reachability once per tag, then let selection skip anything not in + // this target's history. Previously selection ignored ancestry and the guard + // below threw, so a preview became unreleasable the moment the stable lineage + // moved ahead of it. + const ancestryCache = new Map(); + for (const tag of tags) { + const probe = await runCommand(["git", "merge-base", "--is-ancestor", tag, target]); + ancestryCache.set(tag, probe.exitCode === 0); + } + const baseline = selectReleaseBaseline(version, tags, tag => ancestryCache.get(tag) === true); const releaseTag = version.startsWith("v") ? version : `v${version}`; - // Ancestry is required for BOTH channels. It used to be checked only for - // stable releases, which let a preview pick the newest tag from a diverged - // lineage: `git log baseline..target` then emitted the handful of commits - // unique to that unrelated branch and reported "commits=0" coverage, so a - // preview shipped notes that omitted its own history and named someone - // else's. Fail closed instead — a non-ancestral baseline cannot describe a - // range at all, whichever channel asked for it. + // Selection above already filters to reachable tags, so this is now a + // belt-and-braces assertion rather than the primary gate. It stays because a + // non-ancestral baseline cannot describe a range at all, and a future caller + // that skips the filter must still fail closed rather than emit notes drawn + // from an unrelated lineage. if (baseline) { const ancestry = await runCommand(["git", "merge-base", "--is-ancestor", baseline, target]); if (ancestry.exitCode !== 0) { diff --git a/src/codex/log-guard/inspect.ts b/src/codex/log-guard/inspect.ts index f7696c6d83..d2c1719236 100644 --- a/src/codex/log-guard/inspect.ts +++ b/src/codex/log-guard/inspect.ts @@ -261,6 +261,23 @@ function sameColumns(columns: ColumnRow[]): boolean { }); } +/** + * The authoritative compatibility predicate: exact table SQL, exact column + * metadata, and every canonical index. + * + * Exported because the mutation paths must apply the SAME test inside their + * write transaction. They used to check column NAMES only, which is strictly + * weaker than what the inspector reports, so a schema change landing between + * the outer inspection and the locked write let Protect install a row-dropping + * trigger and let Reclaim vacuum pages on a database the inspector classifies + * as monitor-only. The lock serializes OpenCodex against itself; it does not + * stop Codex or another SQLite writer, so that TOCTOU window is real. + */ +export function hasCurrentLogsSchema(db: Database): boolean { + const columns = db.query("PRAGMA table_info(logs)").all(); + return hasCurrentLogsTable(db, columns); +} + function hasCurrentLogsTable(db: Database, columns: ColumnRow[]): boolean { const table = db.query( "SELECT name, type, sql FROM sqlite_schema WHERE name = 'logs' LIMIT 1", diff --git a/src/codex/log-guard/maintenance.ts b/src/codex/log-guard/maintenance.ts index d488d41540..81b3a465b5 100644 --- a/src/codex/log-guard/maintenance.ts +++ b/src/codex/log-guard/maintenance.ts @@ -3,7 +3,7 @@ import { Database, constants as sqliteConstants } from "bun:sqlite"; import { getCodexHome, resolveCodexLogsDbPath } from "../paths"; import { samePathIdentity } from "../user-identity"; -import { inspectCodexLogs } from "./inspect"; +import { hasCurrentLogsSchema, inspectCodexLogs } from "./inspect"; import { withCodexLogGuardLock, type CodexLogGuardLockOutcome } from "./lock"; import { sameLogGuardPathIdentity } from "./path-safety"; import { isSqliteBusy } from "./sqlite-errors"; @@ -145,10 +145,10 @@ function databasePathStillMatches( } function exactCurrentSchema(db: Database): boolean { - const columns = db.query("PRAGMA table_info(logs)").all().map(row => row.name).sort(); - const expected = [...CURRENT_LOG_COLUMNS].sort(); - return columns.length === expected.length - && columns.every((value, index) => value === expected[index]); + // Same reasoning as protection.ts: the pre-mutation recheck must match the + // inspector's compatibility contract exactly, or Reclaim can vacuum a + // database the inspector classifies as monitor-only. + return hasCurrentLogsSchema(db); } function pragmaNumber(db: Database, sql: string): number { diff --git a/src/codex/log-guard/protection.ts b/src/codex/log-guard/protection.ts index 5955000180..613d2677ce 100644 --- a/src/codex/log-guard/protection.ts +++ b/src/codex/log-guard/protection.ts @@ -3,7 +3,7 @@ import { pathToFileURL } from "node:url"; import { Database, constants as sqliteConstants } from "bun:sqlite"; import { getCodexHome, resolveCodexLogsDbPath } from "../paths"; -import { inspectCodexLogs, type CodexLogGuardInspection } from "./inspect"; +import { hasCurrentLogsSchema, inspectCodexLogs, type CodexLogGuardInspection } from "./inspect"; import { withCodexLogGuardLock, type CodexLogGuardLockOutcome } from "./lock"; import { sameLogGuardPathIdentity } from "./path-safety"; import { isSqliteBusy } from "./sqlite-errors"; @@ -193,9 +193,11 @@ function observeTriggers(db: Database): CodexLogGuardObservedMode { } function exactCurrentSchema(db: Database): boolean { - const columns = db.query("PRAGMA table_info(logs)").all().map(row => row.name).sort(); - const expected = [...CURRENT_LOG_COLUMNS].sort(); - return columns.length === expected.length && columns.every((value, index) => value === expected[index]); + // Delegates to the inspector's predicate so the locked recheck is exactly as + // strict as the compatibility report. Column names alone let a schema change + // between inspection and the locked write slip a mutation onto a database the + // inspector calls monitor-only. + return hasCurrentLogsSchema(db); } /** diff --git a/tests/build-release-changelog.test.ts b/tests/build-release-changelog.test.ts index 226afc416f..7baf966d91 100644 --- a/tests/build-release-changelog.test.ts +++ b/tests/build-release-changelog.test.ts @@ -38,6 +38,29 @@ const generatedBugFix = [ ].join("\n"); describe("selectReleaseBaseline", () => { + test("skips a newer release that is not reachable from the target", () => { + // A preview lives on its own lineage. Selecting the newest tag regardless of + // reachability picked a stable tag on main, the ancestry guard then threw, + // and no preview could be released once the stable lineage moved ahead. + // Reproduced against the real repository: without the filter the preview + // range selected v2.21.0, which is an ancestor of neither preview nor dev. + const tags = ["v2.17.0", "v2.19.0", "v2.21.0"]; + const reachable = new Set(["v2.17.0", "v2.19.0"]); + + expect(selectReleaseBaseline("2.22.0-preview.1", tags)).toBe("v2.21.0"); + expect(selectReleaseBaseline("2.22.0-preview.1", tags, tag => reachable.has(tag))).toBe("v2.19.0"); + }); + + test("the reachability filter applies to stable releases too", () => { + const tags = ["v2.17.0", "v2.19.0", "v2.21.0"]; + const reachable = new Set(["v2.17.0"]); + expect(selectReleaseBaseline("2.22.0", tags, tag => reachable.has(tag))).toBe("v2.17.0"); + }); + + test("an absent filter preserves the previous selection behavior", () => { + const tags = ["v2.17.0", "v2.19.0"]; + expect(selectReleaseBaseline("2.20.0", tags)).toBe("v2.19.0"); + }); test("preview releases are incremental from the previous release", () => { expect(selectReleaseBaseline("2.0.0-preview.2", [ "v1.0.0", diff --git a/tests/codex-log-guard-protection.test.ts b/tests/codex-log-guard-protection.test.ts index 9e49d8dab8..a2e0a7be8f 100644 --- a/tests/codex-log-guard-protection.test.ts +++ b/tests/codex-log-guard-protection.test.ts @@ -327,4 +327,22 @@ describe("Codex Log Guard protection", () => { expect(unprotectCodexLogs(deps).ok).toBe(true); expect(triggers(databasePath)).toEqual([]); }); + test("a schema change between inspection and the locked write is refused", () => { + // TOCTOU: the lock serializes OpenCodex against itself, not against Codex or + // another SQLite writer. The locked recheck used to compare column NAMES + // only - strictly weaker than the inspector's contract - so a migration + // landing in that window let Protect install a row-dropping trigger on a + // database the inspector classifies as monitor-only. + const { codexHome, databasePath } = fixture(); + const deps = testDeps(codexHome); + + // Drop a canonical index the inspector requires but a column-name check + // cannot see. + const db = new Database(databasePath); + db.exec("DROP INDEX idx_logs_ts"); + db.close(); + + expect(protectCodexLogs("quiet", deps)).toEqual({ ok: false, error: "unsupported_schema" }); + expect(triggers(databasePath)).toEqual([]); + }); });