refactor(match): one canonical writer for transaction match state (IMP-CODE-03)

match_status, matched_bill_id and ignored must move together, but they were
updated by copy-pasted inline UPDATEs across six routes/services — exactly how
they drift apart (QA-B5-04 left match_status='matched' with a NULL bill).

Add services/transactionMatchState.js (markMatched / markUnmatched / markIgnored,
each ownership-scoped, returning rows changed) and route the six single-
transaction transitions through it: matchTransactionToBill, unmatchTransaction,
ignoreTransaction, unignoreTransaction (transactionMatchService), the match/
unmatch handlers (routes/matches), and unmatch-on-payment-delete (routes/
transactions, routes/payments).

Guarded bulk auto-match sweeps (subscription tracking, merchant-rule matching,
historical import) and the retention purge intentionally keep their own queries
— their WHERE clauses carry idempotency guards (AND match_status='unmatched')
the simple helper must not silently drop.

Test: tests/transactionMatchState.test.js (transitions + ownership scoping).
transactionMatchService/subscriptionService regression suites still pass;
server 122 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

routes/matches.js

@@ -7,6 +7,7 @@ const {
   rejectMatchSuggestion,
 } = require('../services/matchSuggestionService');
 const { learnMerchantRuleFromMatch } = require('../services/billMerchantRuleService');
+const { markMatched, markUnmatched } = require('../services/transactionMatchState');
 const { serializePayment } = require('../services/paymentValidation');
 const { todayLocal } = require('../utils/dates');
 
@@ -69,11 +70,7 @@ router.post('/confirm', (req, res) => {
     const paymentForAccounting = db.prepare('SELECT * FROM payments WHERE id = ?').get(payResult.lastInsertRowid);
     applyBankPaymentAsSourceOfTruth(db, bill, paymentForAccounting);
 
-    db.prepare(`
-      UPDATE transactions
-      SET matched_bill_id = ?, match_status = 'matched', updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(billId, txId, req.user.id);
+    markMatched(db, req.user.id, txId, billId);
 
     // Learn a merchant→bill rule from this explicit confirmation so future
     // synced transactions from the same merchant auto-match. Best-effort.
@@ -123,11 +120,7 @@ router.post('/:transactionId/unmatch', (req, res) => {
       UPDATE payments SET deleted_at = datetime('now'), updated_at = datetime('now')
       WHERE transaction_id = ? AND payment_source = 'transaction_match' AND deleted_at IS NULL
     `).run(txId);
-    db.prepare(`
-      UPDATE transactions
-      SET matched_bill_id = NULL, match_status = 'unmatched', updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(txId, req.user.id);
+    markUnmatched(db, req.user.id, txId);
     db.exec('COMMIT');
     res.json({ ok: true });
   } catch (err) {

routes/payments.js

@@ -5,6 +5,7 @@ const { getDb } = require('../db/database');
 const { computeBalanceDelta, applyBalanceDelta } = require('../services/billsService');
 const { validatePaymentInput, serializePayment } = require('../services/paymentValidation');
 const { getCycleRange, resolveDueDate } = require('../services/statusService');
+const { markUnmatched } = require('../services/transactionMatchState');
 const {
   markProvisionalManualPaymentsOverridden,
   reactivatePaymentsOverriddenBy,
@@ -177,11 +178,7 @@ router.post('/:id/undo-auto', (req, res) => {
       }
       reactivatePaymentsOverriddenBy(db, payment.id);
       db.prepare("UPDATE payments SET deleted_at = datetime('now') WHERE id = ?").run(payment.id);
-      db.prepare(`
-        UPDATE transactions
-        SET match_status = 'unmatched', matched_bill_id = NULL, updated_at = datetime('now')
-        WHERE id = ? AND user_id = ?
-      `).run(payment.transaction_id, req.user.id);
+      markUnmatched(db, req.user.id, payment.transaction_id);
     })();
     res.json({ success: true });
   } catch (err) {

routes/transactions.js

@@ -8,6 +8,7 @@ const {
 } = require('../services/transactionService');
 const { checkTransaction: advisoryCheck } = require('../services/advisoryFilterService');
 const { getBankSyncConfig } = require('../services/bankSyncConfigService');
+const { markUnmatched } = require('../services/transactionMatchState');
 const {
   ignoreTransaction,
   matchTransactionToBill,
@@ -794,11 +795,7 @@ router.post('/unmatch-bulk', (req, res) => {
             reactivatePaymentsOverriddenBy(db, payment.id);
             db.prepare("UPDATE payments SET deleted_at = datetime('now') WHERE id = ?").run(payment.id);
           }
-          db.prepare(`
-            UPDATE transactions
-            SET match_status = 'unmatched', matched_bill_id = NULL, updated_at = datetime('now')
-            WHERE id = ? AND user_id = ?
-          `).run(txId, userId);
+          markUnmatched(db, userId, txId);
           results.push({ transaction_id: txId, ok: true });
         } else {
           // Standard service unmatch (restores balance for transaction_match payments)

services/transactionMatchService.js

@@ -11,6 +11,7 @@ const {
   getTransactionForUser,
 } = require('./transactionService');
 const { serializePayment } = require('./paymentValidation');
+const { markMatched, markUnmatched, markIgnored } = require('./transactionMatchState');
 
 const MATCH_PAYMENT_SOURCE = 'transaction_match';
 const MATCH_PAYMENT_METHOD = 'transaction_match';
@@ -255,14 +256,7 @@ function matchTransactionToBill(userId, transactionId, billId, opts = {}) {
     const bill = getOwnedBill(db, userId, billId);
     const paymentId = createOrUpdateMatchPayment(db, userId, transaction, bill);
 
-    db.prepare(`
-      UPDATE transactions
-      SET matched_bill_id = ?,
-          match_status = 'matched',
-          ignored = 0,
-          updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(bill.id, transaction.id, userId);
+    markMatched(db, userId, transaction.id, bill.id, { resetIgnored: true });
 
     if (opts.learnMerchant) {
       const { learnMerchantRuleFromMatch } = require('./billMerchantRuleService');
@@ -281,14 +275,7 @@ function unmatchTransaction(userId, transactionId) {
     const transaction = getOwnedTransaction(db, userId, transactionId);
     const removedPayment = unlinkPaymentForTransaction(db, userId, transaction.id);
 
-    db.prepare(`
-      UPDATE transactions
-      SET matched_bill_id = NULL,
-          match_status = 'unmatched',
-          ignored = 0,
-          updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(transaction.id, userId);
+    markUnmatched(db, userId, transaction.id, { resetIgnored: true });
 
     return responseForTransaction(db, userId, transaction.id, null, { removed_payment: removedPayment });
   });
@@ -302,14 +289,7 @@ function ignoreTransaction(userId, transactionId) {
     const transaction = getOwnedTransaction(db, userId, transactionId);
     const removedPayment = unlinkPaymentForTransaction(db, userId, transaction.id);
 
-    db.prepare(`
-      UPDATE transactions
-      SET ignored = 1,
-          match_status = 'ignored',
-          matched_bill_id = NULL,
-          updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(transaction.id, userId);
+    markIgnored(db, userId, transaction.id);
 
     return responseForTransaction(db, userId, transaction.id, null, { removed_payment: removedPayment });
   });
@@ -322,14 +302,7 @@ function unignoreTransaction(userId, transactionId) {
   const tx = db.transaction(() => {
     const transaction = getOwnedTransaction(db, userId, transactionId);
 
-    db.prepare(`
-      UPDATE transactions
-      SET ignored = 0,
-          match_status = 'unmatched',
-          matched_bill_id = NULL,
-          updated_at = datetime('now')
-      WHERE id = ? AND user_id = ?
-    `).run(transaction.id, userId);
+    markUnmatched(db, userId, transaction.id, { resetIgnored: true });
 
     return responseForTransaction(db, userId, transaction.id);
   });

services/transactionMatchState.js

@@ -0,0 +1,52 @@
+'use strict';
+
+// Canonical writers for a transaction's match state.
+//
+// A transaction's match state lives in three columns that must move together:
+//   match_status   'unmatched' | 'matched' | 'ignored'
+//   matched_bill_id  the bill when matched, else NULL
+//   ignored          1 when explicitly ignored, else 0
+//
+// These were previously updated by copy-pasted inline UPDATEs across routes and
+// services, which is exactly how they drift out of sync — e.g. QA-B5-04, where a
+// bill purge left match_status='matched' with a NULL bill. Routing every
+// single-transaction transition through these helpers keeps the columns
+// consistent by construction. All are ownership-scoped (user_id) and return the
+// number of rows changed. Bulk transitions (auto-match sweep, retention purge)
+// stay in their own services where the WHERE clause is fundamentally different.
+
+/**
+ * Mark a transaction matched to a bill.
+ * @param {boolean} [opts.resetIgnored] also clear the ignored flag (used when
+ *   matching a transaction directly, which implicitly un-ignores it).
+ */
+function markMatched(db, userId, transactionId, billId, { resetIgnored = false } = {}) {
+  return db.prepare(`
+    UPDATE transactions
+    SET matched_bill_id = ?, match_status = 'matched'${resetIgnored ? ', ignored = 0' : ''}, updated_at = datetime('now')
+    WHERE id = ? AND user_id = ?
+  `).run(billId, transactionId, userId).changes;
+}
+
+/**
+ * Clear a transaction's match — back to 'unmatched' with no bill.
+ * @param {boolean} [opts.resetIgnored] also clear the ignored flag (un-ignore).
+ */
+function markUnmatched(db, userId, transactionId, { resetIgnored = false } = {}) {
+  return db.prepare(`
+    UPDATE transactions
+    SET matched_bill_id = NULL, match_status = 'unmatched'${resetIgnored ? ', ignored = 0' : ''}, updated_at = datetime('now')
+    WHERE id = ? AND user_id = ?
+  `).run(transactionId, userId).changes;
+}
+
+/** Mark a transaction ignored — dropped from matching, no bill. */
+function markIgnored(db, userId, transactionId) {
+  return db.prepare(`
+    UPDATE transactions
+    SET ignored = 1, match_status = 'ignored', matched_bill_id = NULL, updated_at = datetime('now')
+    WHERE id = ? AND user_id = ?
+  `).run(transactionId, userId).changes;
+}
+
+module.exports = { markMatched, markUnmatched, markIgnored };

tests/transactionMatchState.test.js

@@ -0,0 +1,80 @@
+'use strict';
+
+// IMP-CODE-03: the canonical match-state writers keep match_status,
+// matched_bill_id and ignored moving together, so no path can leave the columns
+// inconsistent (the QA-B5-04 class of bug).
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const os = require('node:os');
+const path = require('node:path');
+const fs = require('node:fs');
+
+const dbPath = path.join(os.tmpdir(), `bill-tracker-match-state-${process.pid}.sqlite`);
+process.env.DB_PATH = dbPath;
+
+const { getDb, closeDb } = require('../db/database');
+const { markMatched, markUnmatched, markIgnored } = require('../services/transactionMatchState');
+
+let db, userId, otherId, billId;
+
+function newTxn(uid, { status = 'unmatched', bill = null, ignored = 0 } = {}) {
+  return db.prepare(
+    'INSERT INTO transactions (user_id, source_type, provider_transaction_id, amount, match_status, matched_bill_id, ignored) VALUES (?, ?, ?, -1000, ?, ?, ?)',
+  ).run(uid, 'manual', `ms-${Math.random().toString(36).slice(2)}`, status, bill, ignored).lastInsertRowid;
+}
+const row = (id) => db.prepare('SELECT match_status, matched_bill_id, ignored FROM transactions WHERE id = ?').get(id);
+
+test.before(() => {
+  db = getDb();
+  userId = db.prepare("INSERT INTO users (username, password_hash, role, active) VALUES ('ms-user','x','user',1)").run().lastInsertRowid;
+  otherId = db.prepare("INSERT INTO users (username, password_hash, role, active) VALUES ('ms-other','x','user',1)").run().lastInsertRowid;
+  billId = db.prepare('INSERT INTO bills (user_id, name, due_day, expected_amount, active) VALUES (?, ?, 1, 1000, 1)').run(userId, 'Bill').lastInsertRowid;
+});
+
+test.after(() => {
+  closeDb();
+  for (const suffix of ['', '-wal', '-shm']) { try { fs.unlinkSync(dbPath + suffix); } catch {} }
+});
+
+test('markMatched sets status + bill together', () => {
+  const id = newTxn(userId);
+  const changes = markMatched(db, userId, id, billId);
+  assert.equal(changes, 1);
+  assert.deepEqual(row(id), { match_status: 'matched', matched_bill_id: billId, ignored: 0 });
+});
+
+test('markMatched leaves the ignored flag unless resetIgnored is set', () => {
+  const kept = newTxn(userId, { ignored: 1, status: 'ignored' });
+  markMatched(db, userId, kept, billId);
+  assert.equal(row(kept).ignored, 1, 'ignored preserved by default');
+
+  const cleared = newTxn(userId, { ignored: 1, status: 'ignored' });
+  markMatched(db, userId, cleared, billId, { resetIgnored: true });
+  assert.equal(row(cleared).ignored, 0, 'resetIgnored clears it');
+});
+
+test('markUnmatched clears the bill and status (never leaves matched+NULL)', () => {
+  const id = newTxn(userId, { status: 'matched', bill: billId });
+  markUnmatched(db, userId, id);
+  assert.deepEqual(row(id), { match_status: 'unmatched', matched_bill_id: null, ignored: 0 });
+});
+
+test('markUnmatched with resetIgnored un-ignores', () => {
+  const id = newTxn(userId, { status: 'ignored', ignored: 1 });
+  markUnmatched(db, userId, id, { resetIgnored: true });
+  assert.deepEqual(row(id), { match_status: 'unmatched', matched_bill_id: null, ignored: 0 });
+});
+
+test('markIgnored sets ignored + status, clears bill', () => {
+  const id = newTxn(userId, { status: 'matched', bill: billId });
+  markIgnored(db, userId, id);
+  assert.deepEqual(row(id), { match_status: 'ignored', matched_bill_id: null, ignored: 1 });
+});
+
+test('all writers are ownership-scoped — never touch another user\'s transaction', () => {
+  const foreign = newTxn(otherId, { status: 'matched', bill: null });
+  assert.equal(markMatched(db, userId, foreign, billId), 0);
+  assert.equal(markUnmatched(db, userId, foreign), 0);
+  assert.equal(markIgnored(db, userId, foreign), 0);
+  assert.equal(row(foreign).match_status, 'matched', 'foreign row untouched');
+});