From 43afc1b935cd688850a405280b4a67e94ee61aaa Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 02:15:08 -0700 Subject: [PATCH 1/6] test(backfill): the two cutoff guards pin reachability, not text order (#1149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both tests describe control-flow properties of backfill-thread-root-id.ts and assert them with SCRIPT.indexOf position comparisons. @sprint-review mutated each at c0da084f and both stayed green on a script carrying the regression the test names, because a text-position comparison cannot see a `return`. - "the UPDATE and the ledger INSERT are one transaction": a bare `return` between the UPDATE and the INSERT keeps all four anchors in place and makes the ledger write unreachable. Now asserts the slice between them contains no return/throw/process.exit. - "a run that finds the ledger row reports it and never re-measures": the property is that it STOPS, which rests on one `return`. Deleting that line leaves both indices unchanged and the script re-measures — the exact failure #1148 removed. Now asserts the reported-and-stopped block contains a return. Negative control, each mutant applied in isolation and reverted: A reddens exactly the transaction test, B reddens exactly the ledger test. 34/34 green unmutated. #1170 did not retire these — it was additive (+111 -0) and covers CUTOFF_SQL's semantics, which is orthogonal to statement reachability. Co-Authored-By: Claude Opus 5 --- .../unit/models/threadingCutoffRecord.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/backend/__tests__/unit/models/threadingCutoffRecord.test.js b/backend/__tests__/unit/models/threadingCutoffRecord.test.js index 333216991..fd114daf5 100644 --- a/backend/__tests__/unit/models/threadingCutoffRecord.test.js +++ b/backend/__tests__/unit/models/threadingCutoffRecord.test.js @@ -90,6 +90,15 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { const populationRead = SCRIPT.indexOf('count(*)::int AS needs_root'); expect(ledgerRead).toBeGreaterThan(-1); expect(ledgerRead).toBeLessThan(populationRead); + // #1149: reading the ledger first is not the property — STOPPING is, and a + // position comparison cannot see a `return`. Deleting the return at :209 + // leaves both indices unchanged and the script re-measures anyway, which is + // the exact failure #1148 removed. Pin the thing that stops it. + const reportedAndStopped = SCRIPT.slice( + SCRIPT.indexOf('already recorded at'), + populationRead, + ); + expect(reportedAndStopped).toMatch(/\breturn\b/); expect(SCRIPT).toMatch(/already recorded at \$\{ledger\.applied_at\}/); expect(SCRIPT).toMatch(/the boundary is not re-measured/); }); @@ -106,6 +115,11 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { expect(begin).toBeLessThan(update); expect(update).toBeLessThan(insert); expect(insert).toBeLessThan(commit); + // #1149: order is not reachability. A bare `return` between the UPDATE and + // the INSERT keeps all four anchors in place and makes the ledger write + // unreachable — the mutant that stayed green here. Assert no control-flow + // break separates them. + expect(SCRIPT.slice(update, insert)).not.toMatch(/\breturn\b|\bthrow\b|process\.exit/); expect(SCRIPT).toMatch(/await client\.query\('ROLLBACK'\)/); expect(SCRIPT).toMatch(/client\.release\(\)/); }); From 4a9c1f164ff9ce4fbf004f20886370627ebfbdb4 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 03:28:01 -0700 Subject: [PATCH 2/6] test(backfill): execute the ledger-first guard instead of scanning for a token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review re-checked this PR's own guards rather than taking the corroboration, and found both are still source scans: :89-92 and :101-107 are pure indexOf/lastIndexOf over SCRIPT. The token slices this PR added do catch the two named mutants, but they are a proxy for reachability, not reachability — blind to any mutant that removes a path without inserting a keyword, and false-red-prone if anyone extracts a helper between the anchors. Exports main(injectedPool?) so the ledger-first half can be run. The require.main === module gate is untouched, so importing still executes nothing — the property the comment at the bottom of that file protects. An injected pool also owns its own lifecycle: no env check, no Pool construction, no pool.end(). The instrument is a counting pool, because "stopped" is not observable from the database: ON CONFLICT DO NOTHING means a re-measuring run leaves the ledger row byte-identical. Which queries issue is what differs, so that is what the test records. Four cases including a CONTROL with no ledger row, without which "issues one query" is equally consistent with a main() that returns under every condition. Two mutations, both restored: delete the early return -> executing 2 red, text guard 1 red if (ledger && never) -> executing 3 red, text guard ALL GREEN The second is the justification. Every string stays in place, so no scan over the source can see it. Residue recorded in place: the transaction half stays a text guard, because reaching the APPLY path needs the argv flags and a seeded messages population. Named rather than left to be rediscovered. 38 tests green across both suites. Co-Authored-By: Claude Opus 5 --- .../models/threadingCutoffLedgerFirst.test.js | 99 +++++++++++++++++++ .../unit/models/threadingCutoffRecord.test.js | 9 ++ backend/scripts/backfill-thread-root-id.ts | 21 +++- 3 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 backend/__tests__/unit/models/threadingCutoffLedgerFirst.test.js diff --git a/backend/__tests__/unit/models/threadingCutoffLedgerFirst.test.js b/backend/__tests__/unit/models/threadingCutoffLedgerFirst.test.js new file mode 100644 index 000000000..b1d23999f --- /dev/null +++ b/backend/__tests__/unit/models/threadingCutoffLedgerFirst.test.js @@ -0,0 +1,99 @@ +/** + * A second run reports the recorded cutoff and STOPS — executed, not read. + * + * @sprint-review 57397 established the ordering; #1149 pinned it; and + * @sprint-review then re-checked the guards themselves and found both were + * pure `indexOf`/`lastIndexOf` over the script SOURCE, so the mutants they + * were written against ship green. A position comparison cannot see a + * `return`: delete the early return and `ledgerRead < populationRead` still + * holds, while the script re-measures a boundary that is no longer + * measurable — the exact #1148 failure. + * + * Text guards remain in threadingCutoffRecord.test.js and are useful for the + * structural half (rule 18). This file covers the behavioural half the only + * way it can be covered: by running the thing. + * + * The instrument is a counting pool. "Stopped" is not observable from the + * database — ON CONFLICT DO NOTHING means a re-measuring run leaves the ledger + * row byte-identical, so asserting on stored state cannot distinguish the two. + * What distinguishes them is WHICH QUERIES ISSUE, so that is what we record. + */ +const { newDb } = require('pg-mem'); +const { applyTable } = require('../../utils/schemaTable'); + +const mockDb = newDb(); +const basePool = new (mockDb.adapters.createPg().Pool)(); + +const issued = []; +const countingPool = { + query: (...args) => { + issued.push(String(args[0]?.text || args[0])); + return basePool.query(...args); + }, +}; + +const { main, MIGRATION_NAME } = require('../../../scripts/backfill-thread-root-id'); + +const CUTOFF = '2026-08-01T00:00:00.000Z'; + +beforeAll(async () => { + await applyTable(basePool, 'migration_records'); +}); + +beforeEach(() => { + issued.length = 0; + jest.spyOn(console, 'log').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => jest.restoreAllMocks()); + +describe('the ledger row short-circuits the run', () => { + beforeAll(async () => { + await basePool.query( + `INSERT INTO migration_records (name, details) + VALUES ($1, $2::jsonb) ON CONFLICT (name) DO NOTHING`, + [MIGRATION_NAME, JSON.stringify({ threadingCutoff: CUTOFF, rowsUpdated: 7 })], + ); + }); + + it('issues the ledger read and NOTHING else', async () => { + await main(countingPool); + expect(issued).toHaveLength(1); + expect(issued[0]).toMatch(/FROM migration_records WHERE name = \$1/); + }); + + it('never counts the population, which is the query that would lie', async () => { + // After a backfill every pre-threading reply carries a root, so the + // population read and the MIN below it return a plausible wrong answer + // rather than an obviously broken one. Not reaching them is the property. + await main(countingPool); + expect(issued.join('\n')).not.toMatch(/needs_root/); + expect(issued.join('\n')).not.toMatch(/UPDATE messages/); + }); + + it('reports the recorded value rather than a recomputed one', async () => { + await main(countingPool); + const said = console.log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(said).toContain(MIGRATION_NAME); + expect(said).toContain(CUTOFF); + expect(said).toContain('the boundary is not re-measured'); + }); + + it('CONTROL: with no ledger row the run does NOT stop there', async () => { + // Without this the assertions above are equally consistent with a main() + // that issues one query and returns under every condition — including a + // stubbed-out one. Proves the short-circuit is the ledger row's doing. + await basePool.query('DELETE FROM migration_records WHERE name = $1', [MIGRATION_NAME]); + issued.length = 0; + await main(countingPool).catch(() => {}); + expect(issued.length).toBeGreaterThan(1); + expect(issued.join('\n')).toMatch(/needs_root/); + // Restore for any later describe. + await basePool.query( + `INSERT INTO migration_records (name, details) + VALUES ($1, $2::jsonb) ON CONFLICT (name) DO NOTHING`, + [MIGRATION_NAME, JSON.stringify({ threadingCutoff: CUTOFF, rowsUpdated: 7 })], + ); + }); +}); diff --git a/backend/__tests__/unit/models/threadingCutoffRecord.test.js b/backend/__tests__/unit/models/threadingCutoffRecord.test.js index fd114daf5..f3aacdc41 100644 --- a/backend/__tests__/unit/models/threadingCutoffRecord.test.js +++ b/backend/__tests__/unit/models/threadingCutoffRecord.test.js @@ -120,6 +120,15 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { // unreachable — the mutant that stayed green here. Assert no control-flow // break separates them. expect(SCRIPT.slice(update, insert)).not.toMatch(/\breturn\b|\bthrow\b|process\.exit/); + // Residue, stated so this is not read as reachability: a token scan over a + // source slice catches the mutants that INSERT a control-flow keyword and + // is blind to every mutant that removes reachability without one — an + // `if (false)` around the block, a condition edited to never hold. It is + // also false-red-prone: extract a helper between these two anchors and its + // `return` fails this. The ledger-first half is now executed instead + // (threadingCutoffLedgerFirst.test.js); this transaction half is not, + // because reaching the APPLY path needs the argv flags and a seeded + // messages population. Keep the text guard until that exists. expect(SCRIPT).toMatch(/await client\.query\('ROLLBACK'\)/); expect(SCRIPT).toMatch(/client\.release\(\)/); }); diff --git a/backend/scripts/backfill-thread-root-id.ts b/backend/scripts/backfill-thread-root-id.ts index ad37f47e6..ca16cc6d0 100644 --- a/backend/scripts/backfill-thread-root-id.ts +++ b/backend/scripts/backfill-thread-root-id.ts @@ -161,15 +161,27 @@ export const CUTOFF_SQL = ` FROM messages WHERE reply_to_message_id IS NOT NULL AND thread_root_id IS NULL) unrooted`; -async function main(): Promise { - if (!process.env.PG_HOST) { +/** + * The migration body. Exported ONLY so a test can execute it — the guards on + * this script pinned text order (`indexOf` < `indexOf`) and a position + * comparison cannot see a `return`, so every mutant that changed reachability + * without moving a string shipped green. Exporting is what makes reachability + * assertable at all; see __tests__/unit/models/threadingCutoffLedgerFirst.test.js. + * + * `injectedPool` also skips the env check, the Pool construction and the + * `pool.end()` — a caller supplying a pool owns its lifecycle. The + * `require.main === module` gate at the bottom is unchanged, so importing this + * file still runs nothing. + */ +export async function main(injectedPool?: Partial): Promise { + if (!injectedPool && !process.env.PG_HOST) { console.error('PG_HOST is required'); process.exit(2); } // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const fs = require('fs'); const caPath = process.env.PG_SSL_CA_PATH; - const pool = new Pool({ + const pool = (injectedPool as Pool) || new Pool({ host: process.env.PG_HOST, port: Number(process.env.PG_PORT) || 5432, user: process.env.PG_USER, @@ -440,7 +452,8 @@ async function main(): Promise { console.error('backfill failed:', (error as Error).message); process.exitCode = 1; } finally { - await pool.end(); + // Only a pool this function built is a pool this function may close. + if (!injectedPool) await pool.end(); } } From e1872c2fc460dcda2232049dad8a4a07c29bb431 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:15:15 -0700 Subject: [PATCH 3/6] =?UTF-8?q?docs:=20cite=20by=20symbol=20=E2=80=94=20th?= =?UTF-8?q?ree=20line=20pointers,=20and=20one=20rotted=20inside=20its=20ow?= =?UTF-8?q?n=20PR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review caught that the early return this PR's guard is written about is at :221, not the :209 the comment claims. The export of main() in this same branch is what moved it: a pointer that rotted inside the branch that wrote it, in the commit whose whole subject is that pointer. Swept the siblings rather than fixing the one (rule 15's third rider) and found two more, both real targets, both adrift: nativeRuntimeService.ts:512 cites ":697" for a quoted comment that lives at :740 — 43 lines off. I nearly filed it as a phantom quote; grepping the quoted TEXT found it immediately, which is the argument for the fix. agentEventService.lifecycle cites ":650" for the requeue (it is :646, .test.js:289 inside the call) and ":698" for the pending delete (it is :715; :698 is a comment line). All three now cite the symbol or the quoted text. A line number in a comment is a claim about a file's LENGTH, which is the one property every commit is entitled to change — and it fails silently, because a wrong pointer lands on plausible code and nobody follows it far enough to notice. 56 tests green across the three affected suites. Co-Authored-By: Claude Opus 5 --- .../unit/models/threadingCutoffRecord.test.js | 13 ++++++++++--- .../services/agentEventService.lifecycle.test.js | 3 ++- backend/services/nativeRuntimeService.ts | 11 ++++++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/backend/__tests__/unit/models/threadingCutoffRecord.test.js b/backend/__tests__/unit/models/threadingCutoffRecord.test.js index f3aacdc41..b86c5868e 100644 --- a/backend/__tests__/unit/models/threadingCutoffRecord.test.js +++ b/backend/__tests__/unit/models/threadingCutoffRecord.test.js @@ -91,9 +91,16 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { expect(ledgerRead).toBeGreaterThan(-1); expect(ledgerRead).toBeLessThan(populationRead); // #1149: reading the ledger first is not the property — STOPPING is, and a - // position comparison cannot see a `return`. Deleting the return at :209 - // leaves both indices unchanged and the script re-measures anyway, which is - // the exact failure #1148 removed. Pin the thing that stops it. + // position comparison cannot see a `return`. Deleting the early return in + // the script's `if (ledger)` branch leaves both indices unchanged and the + // script re-measures anyway, which is the exact failure #1148 removed. Pin + // the thing that stops it. + // + // Cited by symbol, not by line. This comment said `:209` and the export of + // `main` in this same PR moved it to `:221` — a pointer that rotted inside + // the branch that wrote it, caught by @sprint-review reproducing the two + // mutation rows. A line number in a comment is a claim about a file's + // length, which is the one property every commit is entitled to change. const reportedAndStopped = SCRIPT.slice( SCRIPT.indexOf('already recorded at'), populationRead, diff --git a/backend/__tests__/unit/services/agentEventService.lifecycle.test.js b/backend/__tests__/unit/services/agentEventService.lifecycle.test.js index be359ba87..b5644cd30 100644 --- a/backend/__tests__/unit/services/agentEventService.lifecycle.test.js +++ b/backend/__tests__/unit/services/agentEventService.lifecycle.test.js @@ -286,7 +286,8 @@ describe('garbageCollect: the requeue predicate and its cap', () => { expect(expire.filter.attempts).toEqual({ $gte: 3 }); }); - // #993: the requeue at :650 and the pending delete at :698 ran in the same + // #993: the requeue (`requeueResult = await AgentEvent.updateMany`) and the + // pending delete (`deleteMany({ status: 'pending' ... })`) ran in the same // Promise.all against different fields — the requeue sets `status` but not // `createdAt`, so an event it had just rescued walked into a `createdAt`-keyed // delete carrying its original age. 38 events were destroyed in one measured diff --git a/backend/services/nativeRuntimeService.ts b/backend/services/nativeRuntimeService.ts index 7f0c37f1d..a8798ce0b 100644 --- a/backend/services/nativeRuntimeService.ts +++ b/backend/services/nativeRuntimeService.ts @@ -508,9 +508,14 @@ export function buildUserMessage( // // WHAT WAS BROKEN. The branches below compare `trigger.type` against // 'mention' / 'chat.message', but the caller passes the RAW event type — - // agentEventService:994 forwards `type` verbatim, and the claimable-set gate - // at :697 in this same file says so out loud ("raw-type gate mirrors the - // wrapper's claimable set"). So `chat.mention` never matched 'mention', and + // `agentEventService`'s native dispatch forwards `type` verbatim into + // `runAgent`, and the claimable-set gate further down THIS file says so out + // loud — grep `Raw-type gate mirrors the wrapper's claimable set`. Cited by + // its text rather than its line: that pointer read `:697` and the comment is + // at `:740`, forty-three lines adrift, which is a citation that survives + // being wrong because nobody follows it. + // + // So `chat.mention` never matched 'mention', and // every message-shaped wake fell through to the generic "Trigger: X, use // commonly_read_context" below — which discards the cue entirely and tells // the agent to go look around instead. From b7777a689f1550b399d3bd550c96f29fcd337616 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:19:14 -0700 Subject: [PATCH 4/6] test(backfill): execute the atomicity property at Tier 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review measured the residue I had only predicted: gating the ledger INSERT on a never-true condition leaves BEGIN/UPDATE/INSERT/COMMIT/ROLLBACK all present and in order, both unit suites green at 38/38, and the run leaves every chain rooted with no cutoff recorded — verbatim the harm the transaction guard's own comment names. The guard was blind to the one failure it was written for. Not fixable at Tier 0: pg-mem rejects WITH RECURSIVE, which threading.derivation.test.js already records. So the atomicity property needs a real server too. Two directions, because "one transaction" is two claims. FORWARD: a successful run leaves both the rooted rows and the ledger row. BACKWARD: a run whose INSERT throws leaves NEITHER. Forward alone passes against a script with no transaction at all. BACKWARD asserts process.exitCode, not a rejection — main()'s outer catch logs and sets the exit code, so a failed backfill surfaces as non-zero exit, never a thrown promise. That is the contract an operator observes. One fixture bug found and fixed in the writing, worth the comment it got: patching client.query on a POOLED pg client outlives the test, because the pool hands the same object out again. It broke the next two cases as a missing ledger row — a plausible product bug, not a broken fixture. The patch is now undone on release. Mutation-checked across both tiers: the gated INSERT reddens 3/3 at Tier 1 and 0/38 at Tier 0. Co-Authored-By: Claude Opus 5 --- .../threading.backfillTransaction.test.js | 181 ++++++++++++++++++ .../unit/models/threadingCutoffRecord.test.js | 25 ++- 2 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 backend/__tests__/service/threading.backfillTransaction.test.js diff --git a/backend/__tests__/service/threading.backfillTransaction.test.js b/backend/__tests__/service/threading.backfillTransaction.test.js new file mode 100644 index 000000000..d92674d4e --- /dev/null +++ b/backend/__tests__/service/threading.backfillTransaction.test.js @@ -0,0 +1,181 @@ +/** + * The backfill's UPDATE and its ledger INSERT are ONE transaction — executed. + * + * Tier 1 on purpose. The guard for this property in + * `__tests__/unit/models/threadingCutoffRecord.test.js` is a token scan over a + * slice of the script source, and @sprint-review measured what that misses: + * gate the ledger INSERT on a never-true condition and every anchor stays put + * (BEGIN, the UPDATE, the INSERT, COMMIT, ROLLBACK all still present, still in + * order), both unit suites report 38/38 green, and the run leaves every chain + * rooted with no cutoff recorded. That is verbatim the harm the transaction + * guard's own comment names — so the guard is blind to the one failure it was + * written for. + * + * It cannot be fixed at Tier 0: pg-mem rejects `WITH RECURSIVE` outright, which + * `threading.derivation.test.js` already records. The backfill's CTE needs a + * real server, so the atomicity property does too. + * + * Two directions, because "one transaction" is two claims: + * forward — a successful run leaves BOTH the rooted rows and the ledger row + * backward — a run whose INSERT throws leaves NEITHER + * A test of only the forward half passes against a script with no transaction + * at all. + */ +const { Pool } = require('pg'); + +const RUN = process.env.INTEGRATION_TEST === 'true'; +const d = RUN ? describe : describe.skip; + +// `--apply` is read at module load (`const APPLY = process.argv.includes(...)`), +// so it has to be in place before the require below, not before the call. +if (!process.argv.includes('--apply')) process.argv.push('--apply'); +// eslint-disable-next-line global-require +const { main, MIGRATION_NAME } = require('../../scripts/backfill-thread-root-id'); + +const POD = 'aaaaaaaaaaaaaaaaaaaaaa02'; +const USER = 'bbbbbbbbbbbbbbbbbbbbbb02'; + +let pool; + +const connect = () => new Pool({ + host: process.env.PG_HOST, + port: Number(process.env.PG_PORT || 5432), + database: process.env.PG_DATABASE, + user: process.env.PG_USER, + password: process.env.PG_PASSWORD, + ssl: false, +}); + +const reset = async () => { + await pool.query('DELETE FROM migration_records WHERE name = $1', [MIGRATION_NAME]); + await pool.query('DELETE FROM messages WHERE pod_id = $1', [POD]); + await pool.query( + `INSERT INTO users (_id, username) VALUES ($1,'tester') ON CONFLICT (_id) DO NOTHING`, [USER], + ); + await pool.query( + `INSERT INTO pods (id, name, type, created_by) VALUES ($1,'P','chat',$2) + ON CONFLICT (id) DO NOTHING`, [POD, USER], + ); + const ins = async (replyTo, rootId, minutesAgo) => { + const { rows } = await pool.query( + `INSERT INTO messages (pod_id, user_id, content, reply_to_message_id, thread_root_id, created_at) + VALUES ($1,$2,'m',$3,$4, NOW() - ($5 || ' minutes')::interval) RETURNING id`, + [POD, USER, replyTo, rootId, String(minutesAgo)], + ); + return rows[0].id; + }; + // One already-rooted reply so CUTOFF_SQL takes the from_rooted branch — + // without it the script needs --derivation-live and refuses to record. + const liveRoot = await ins(null, null, 50); + await ins(liveRoot, liveRoot, 40); + // And an un-rooted chain for the backfill to actually repair. + const oldRoot = await ins(null, null, 30); + const a = await ins(oldRoot, null, 20); + await ins(a, null, 10); + return { oldRoot }; +}; + +const unrootedCount = async () => { + const { rows } = await pool.query( + `SELECT count(*)::int AS n FROM messages + WHERE pod_id = $1 AND reply_to_message_id IS NOT NULL AND thread_root_id IS NULL`, [POD], + ); + return rows[0].n; +}; + +const ledgerRow = async () => { + const { rows } = await pool.query( + 'SELECT details FROM migration_records WHERE name = $1', [MIGRATION_NAME], + ); + return rows[0] || null; +}; + +d('the UPDATE and the ledger INSERT commit together or not at all', () => { + beforeAll(() => { pool = connect(); }); + afterAll(async () => { + if (!pool) return; + await pool.query('DELETE FROM messages WHERE pod_id = $1', [POD]); + await pool.query('DELETE FROM migration_records WHERE name = $1', [MIGRATION_NAME]); + await pool.end(); + }); + + beforeEach(async () => { + await reset(); + jest.spyOn(console, 'log').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + afterEach(() => jest.restoreAllMocks()); + + it('FORWARD: a successful run leaves both the rooted rows and the ledger row', async () => { + expect(await unrootedCount()).toBeGreaterThan(0); // CONTROL: there was work + await main(pool); + expect(await unrootedCount()).toBe(0); + const row = await ledgerRow(); + expect(row).not.toBeNull(); + // The mutant @sprint-review measured gates exactly this INSERT off. Every + // text anchor survives it; this assertion does not. + expect(row.details.rowsUpdated).toBeGreaterThan(0); + expect(row.details).toHaveProperty('threadingCutoff'); + }); + + it('BACKWARD: an INSERT that throws rolls the UPDATE back', async () => { + const before = await unrootedCount(); + expect(before).toBeGreaterThan(0); + + // Wrap the pool so the ledger INSERT — and only it — fails. Everything + // else, including BEGIN/ROLLBACK, goes to the real server, so this + // exercises the script's own transaction rather than a simulated one. + // + // The patch is UNDONE on release. A pg client is pooled: assigning to + // `client.query` mutates the object the pool hands out again, so without + // this the poison outlives the test and every later checkout rejects its + // ledger INSERT. That cost the next two cases in this file before it was + // found, and it failed as a plausible product bug (no ledger row) rather + // than as a broken fixture. + const failingPool = { + query: (...args) => pool.query(...args), + connect: async () => { + const client = await pool.connect(); + const realQuery = client.query.bind(client); + const realRelease = client.release.bind(client); + client.query = (...args) => { + const sql = String(args[0]?.text || args[0]); + if (/INSERT INTO migration_records/i.test(sql)) { + return Promise.reject(new Error('simulated ledger write failure')); + } + return realQuery(...args); + }; + client.release = (...args) => { + delete client.query; + delete client.release; + return realRelease(...args); + }; + return client; + }, + }; + + // NOT a rejection: main()'s outer catch logs and sets `process.exitCode`, + // so a failed backfill surfaces as a non-zero exit, never as a thrown + // promise. Asserting on the exit code is asserting the contract an + // operator (and any CI wrapper) actually observes. + process.exitCode = 0; + await main(failingPool); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + + // Neither half landed: the rows are still un-rooted and there is no record. + expect(await unrootedCount()).toBe(before); + expect(await ledgerRow()).toBeNull(); + }); + + it('and a second run reports the recorded cutoff without re-measuring', async () => { + await main(pool); + const first = await ledgerRow(); + console.log.mockClear(); + await main(pool); + const second = await ledgerRow(); + expect(second.details).toEqual(first.details); + expect(console.log.mock.calls.map((c) => String(c[0])).join('\n')) + .toContain('the boundary is not re-measured'); + }); +}); diff --git a/backend/__tests__/unit/models/threadingCutoffRecord.test.js b/backend/__tests__/unit/models/threadingCutoffRecord.test.js index b86c5868e..8a1ce4cce 100644 --- a/backend/__tests__/unit/models/threadingCutoffRecord.test.js +++ b/backend/__tests__/unit/models/threadingCutoffRecord.test.js @@ -127,15 +127,22 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { // unreachable — the mutant that stayed green here. Assert no control-flow // break separates them. expect(SCRIPT.slice(update, insert)).not.toMatch(/\breturn\b|\bthrow\b|process\.exit/); - // Residue, stated so this is not read as reachability: a token scan over a - // source slice catches the mutants that INSERT a control-flow keyword and - // is blind to every mutant that removes reachability without one — an - // `if (false)` around the block, a condition edited to never hold. It is - // also false-red-prone: extract a helper between these two anchors and its - // `return` fails this. The ledger-first half is now executed instead - // (threadingCutoffLedgerFirst.test.js); this transaction half is not, - // because reaching the APPLY path needs the argv flags and a seeded - // messages population. Keep the text guard until that exists. + // Residue, and @sprint-review measured it rather than leaving it predicted: + // gating the ledger INSERT on a never-true condition leaves every anchor + // above in place and this whole file green at 38/38, while the run leaves + // every chain rooted and no cutoff recorded — verbatim the harm the + // comment at the top of this test names. A token scan catches mutants that + // INSERT a control-flow keyword and is blind to every mutant that removes + // reachability without one. It is also false-red-prone in the other + // direction: extract a helper between these two anchors and its `return` + // fails this. + // + // Both halves are now executed elsewhere, and this stays as the cheap + // structural check only: + // ledger-first -> threadingCutoffLedgerFirst.test.js (Tier 0, pg-mem) + // atomicity -> service/threading.backfillTransaction.test.js (Tier 1) + // Atomicity had to go to Tier 1 because pg-mem cannot parse the backfill's + // `WITH RECURSIVE`, which threading.derivation.test.js already records. expect(SCRIPT).toMatch(/await client\.query\('ROLLBACK'\)/); expect(SCRIPT).toMatch(/client\.release\(\)/); }); From ac5a7f9d51ede98c1732be1c25d3723744f4563e Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:20:33 -0700 Subject: [PATCH 5/6] test(backfill): the residue leads with the measurement, not the class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review: "blind to a class" reads softer than the truth. The measured statement is that this test is blind to THE defect it was written for — gate the ledger INSERT on a never-true condition and every anchor holds, the file reports 38/38, and the run leaves every chain rooted with no cutoff recorded, word for word the harm named four lines above. The generalisation is the weaker sentence and now goes second. Also records that the anchors are sound rather than leaving `lastIndexOf` looking like a smell: `UPDATE messages m` is unique, and lastIndexOf is the correct pick for the INSERT because the zero-eligible-edges branch carries a second `INSERT INTO migration_records` that indexOf would grab. The limit is the instrument, not the anchors — which is the reason the property moved to a tier that runs the code, not a reason to distrust this guard. Guard stays as-is otherwise, per review. 38/38. Co-Authored-By: Claude Opus 5 --- .../unit/models/threadingCutoffRecord.test.js | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/backend/__tests__/unit/models/threadingCutoffRecord.test.js b/backend/__tests__/unit/models/threadingCutoffRecord.test.js index 8a1ce4cce..efb30ead8 100644 --- a/backend/__tests__/unit/models/threadingCutoffRecord.test.js +++ b/backend/__tests__/unit/models/threadingCutoffRecord.test.js @@ -127,15 +127,26 @@ describe('the cutoff is measured before it becomes unmeasurable', () => { // unreachable — the mutant that stayed green here. Assert no control-flow // break separates them. expect(SCRIPT.slice(update, insert)).not.toMatch(/\breturn\b|\bthrow\b|process\.exit/); - // Residue, and @sprint-review measured it rather than leaving it predicted: - // gating the ledger INSERT on a never-true condition leaves every anchor - // above in place and this whole file green at 38/38, while the run leaves - // every chain rooted and no cutoff recorded — verbatim the harm the - // comment at the top of this test names. A token scan catches mutants that - // INSERT a control-flow keyword and is blind to every mutant that removes - // reachability without one. It is also false-red-prone in the other - // direction: extract a helper between these two anchors and its `return` - // fails this. + // MEASURED, not predicted (@sprint-review, 2026-08-25): this test is blind + // to THE defect it was written for. Gate the ledger INSERT on a never-true + // condition and every anchor above stays in place, this file reports 38/38 + // green, and the run leaves every chain rooted with no cutoff recorded — + // which is word for word the harm named four lines up. Not "blind to a + // class of mutants": blind to the one. + // + // The class is the generalisation, and it is the weaker sentence, so it + // goes second. A token scan catches mutants that INSERT a control-flow + // keyword and misses every mutant that removes reachability without one. + // It is false-red-prone in the other direction too: extract a helper + // between these two anchors and its `return` fails this. + // + // The anchors themselves are sound and were checked — `UPDATE messages m` + // is unique, and `lastIndexOf` is the correct pick for the INSERT because + // the zero-eligible-edges branch above contains a second + // `INSERT INTO migration_records` that `indexOf` would grab instead. So + // this guard is as good as a source scan can be here; the limit is the + // instrument, not the anchors, which is exactly why the property moved to + // a tier that runs the code. // // Both halves are now executed elsewhere, and this stays as the cheap // structural check only: From f85b4cdc3e8e902cc7094404268c0ab972cfd050 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:45:22 -0700 Subject: [PATCH 6/6] test(threading): record the mutant that separates BACKWARD from FORWARD Co-Authored-By: Claude Opus 5 --- .../threading.backfillTransaction.test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/backend/__tests__/service/threading.backfillTransaction.test.js b/backend/__tests__/service/threading.backfillTransaction.test.js index d92674d4e..8193440d9 100644 --- a/backend/__tests__/service/threading.backfillTransaction.test.js +++ b/backend/__tests__/service/threading.backfillTransaction.test.js @@ -119,6 +119,23 @@ d('the UPDATE and the ledger INSERT commit together or not at all', () => { }); it('BACKWARD: an INSERT that throws rolls the UPDATE back', async () => { + // Why this case is not redundant with FORWARD, measured rather than argued. + // @sprint-review reproduced the gated-INSERT mutant and found all three arms + // redden, reading that as BACKWARD earning its place. It doesn't: three arms + // failing on ONE mutant is co-sensitivity, not discrimination — FORWARD alone + // would have caught it. The question is whether any mutant reddens BACKWARD + // and leaves the rest green. Two do, on real pg16: + // + // remove `BEGIN` → FORWARD ✓, BACKWARD ✕ | Tier 0 ✕ (textually) + // UPDATE via `pool.query` → FORWARD ✓, BACKWARD ✕ | Tier 0 38/38 GREEN + // + // The second is the one that matters, and it is the likelier refactor of the + // two: the transaction still opens, the ledger INSERT still sits inside it, + // BEGIN still precedes UPDATE precedes INSERT precedes COMMIT in the source — + // so every ordering assertion in threadingCutoffRecord.test.js holds while the + // UPDATE autocommits on a second connection and survives the ROLLBACK. A + // source scan can see that a transaction is written; only this case can see + // that the write is inside it. const before = await unrootedCount(); expect(before).toBeGreaterThan(0);