Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2uprnlv3zi is past its budget: trevato/forge: "the releases page > a writer gets a button on a kept release, not on the live or a pruned one" failed o… #4684
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4681-sentinel-shared-01m23vygyffx5hg2f5vm8r74 → main 2 commits fork 2 threads · 2 unresolved
3 files
changed +47
−9
src/release/gate.ts +2 −2
| @@ -15,7 +15,7 @@ import { lacks } from "../core/roles.ts"; | ||
| 15 | 15 | import type { User } from "../core/users.ts"; |
| 16 | 16 | import { releaseDir } from "./checkout.ts"; |
| 17 | 17 | import type { Release } from "./record.ts"; |
| 18 | import { liveSchema, releaseSchema, rollback } from "./rollback.ts"; | |
| 18 | import { releaseSchema, rollback, schemaCount } from "./rollback.ts"; | |
| 19 | 19 | import { beginSwap, endSwap, handover } from "./swap.ts"; |
| 20 | 20 | |
| 21 | 21 | /** When this process began serving: "healthy since" for the release it runs. */ |
| @@ -50,7 +50,7 @@ export async function canRollBackTo(app: App, r: Release): Promise<boolean> { | ||
| 50 | 50 | const dir = releaseDir(dataDir, r.sha); |
| 51 | 51 | if (!r.ok || !/^[0-9a-f]{40}$/.test(r.sha) || !existsSync(dir)) return false; |
| 52 | 52 | const known = await releaseSchema(dir); |
| 53 | return known !== undefined && known >= liveSchema(dataDir); | |
| 53 | return known !== undefined && known >= schemaCount(app.db); | |
| 54 | 54 | } |
| 55 | 55 | |
| 56 | 56 | export type RolledBack = { ok: true } | { ok: false; line: string }; |
src/release/rollback.ts +10 −6
| @@ -24,17 +24,21 @@ export function liveSchema(dataDir: string): number { | ||
| 24 | 24 | if (!existsSync(file)) return 0; |
| 25 | 25 | const db = new Database(file, { readonly: true }); |
| 26 | 26 | try { |
| 27 | const table = db | |
| 28 | .query<{ n: number }, []>("SELECT COUNT(*) AS n FROM sqlite_master WHERE type='table' AND name='schema_migration'") | |
| 29 | .get(); | |
| 30 | if (table?.n === 1) | |
| 31 | return db.query<{ n: number }, []>("SELECT COUNT(*) AS n FROM schema_migration").get()?.n ?? 0; | |
| 32 | return db.query<{ user_version: number }, []>("PRAGMA user_version").get()?.user_version ?? 0; | |
| 27 | return schemaCount(db); | |
| 33 | 28 | } finally { |
| 34 | 29 | db.close(); |
| 35 | 30 | } |
| 36 | 31 | } |
| 37 | 32 | |
| 33 | /** The same count from a connection already open: no second reader to meet a busy WAL. */ | |
| 34 | export function schemaCount(db: Database): number { | |
| 35 | const table = db | |
New exported schemaCount has no direct test; cover both the schema_migration table path and the user_version fallback. |
||
| 36 | .query<{ n: number }, []>("SELECT COUNT(*) AS n FROM sqlite_master WHERE type='table' AND name='schema_migration'") | |
| 37 | .get(); | |
| 38 | if (table?.n === 1) return db.query<{ n: number }, []>("SELECT COUNT(*) AS n FROM schema_migration").get()?.n ?? 0; | |
| 39 | return db.query<{ user_version: number }, []>("PRAGMA user_version").get()?.user_version ?? 0; | |
| 40 | } | |
| 41 | ||
| 38 | 42 | /** How many migrations the kept release knows, read from its own schema module. */ |
| 39 | 43 | export async function releaseSchema(dir: string): Promise<number | undefined> { |
| 40 | 44 | const file = join(dir, "src", "core", "schema.ts"); |
test/release-rollback.test.ts +35 −1
| @@ -11,7 +11,10 @@ import { mkdtemp, rm } from "node:fs/promises"; | ||
| 11 | 11 | import { tmpdir } from "node:os"; |
| 12 | 12 | import { join } from "node:path"; |
| 13 | 13 | import { releaseDir } from "../src/release/checkout.ts"; |
| 14 | import { liveSchema, rollback } from "../src/release/rollback.ts"; | |
| 14 | import type { App } from "../src/app.ts"; | |
| 15 | import { canRollBackTo } from "../src/release/gate.ts"; | |
| 16 | import type { Release } from "../src/release/record.ts"; | |
| 17 | import { liveSchema, rollback, schemaCount } from "../src/release/rollback.ts"; | |
| 15 | 18 | |
| 16 | 19 | const KEPT = "a".repeat(40); |
| 17 | 20 | const OLDER = "b".repeat(40); |
| @@ -153,6 +156,37 @@ describe("liveSchema", () => { | ||
| 153 | 156 | }); |
| 154 | 157 | }); |
| 155 | 158 | |
| 159 | describe("schemaCount", () => { | |
| 160 | test("counts schema_migration rows, and falls back to user_version without the table", () => { | |
| 161 | const db = new Database(":memory:"); | |
| 162 | db.exec("PRAGMA user_version = 4"); | |
| 163 | expect(schemaCount(db)).toBe(4); | |
| 164 | db.exec("CREATE TABLE schema_migration (id TEXT PRIMARY KEY, position INTEGER, applied_at TEXT)"); | |
| 165 | expect(schemaCount(db)).toBe(0); | |
| 166 | db.exec("INSERT INTO schema_migration VALUES ('a', 1, 'now'), ('b', 2, 'now')"); | |
| 167 | expect(schemaCount(db)).toBe(2); | |
| 168 | db.close(); | |
| 169 | }); | |
| 170 | }); | |
| 171 | ||
| 172 | describe("canRollBackTo", () => { | |
| 173 | // The open connection is the live database; dataDir/forge.db is left absent, so the old | |
| 174 | // on-disk read (schema 0) would allow every kept release. | |
| 175 | test("compares a kept release with the open app.db, not the file in the data dir", async () => { | |
| 176 | const data = await emptyDir(); | |
| 177 | const db = new Database(":memory:"); | |
| 178 | db.exec("CREATE TABLE schema_migration (id TEXT PRIMARY KEY, position INTEGER, applied_at TEXT)"); | |
| 179 | for (let i = 0; i < 3; i++) db.exec(`INSERT INTO schema_migration VALUES ('m${i}', ${i + 1}, 'now')`); | |
| 180 | const app = { db, config: { dataDir: data } } as unknown as App; | |
| 181 | const rel = (sha: string): Release => ({ sha, ok: true }) as unknown as Release; | |
| 182 | keep(data, OLDER, 2); | |
| 183 | keep(data, KEPT, 3); | |
| 184 | expect(await canRollBackTo(app, rel(OLDER))).toBe(false); | |
| 185 | expect(await canRollBackTo(app, rel(KEPT))).toBe(true); | |
| 186 | db.close(); | |
| 187 | }); | |
| 188 | }); | |
| 189 | ||
| 156 | 190 | describe("--rollback command line", () => { |
| 157 | 191 | const run = async (args: string[], env: Record<string, string>) => { |
| 158 | 192 | const proc = Bun.spawn([process.execPath, join(import.meta.dir, "..", "src", "main.ts"), ...args], { |
new above
offline
Behaviour changes here (open connection instead of the file) with no test that would fail on the old code. Add one where dataDir/forge.db differs from app.db.