Name the long-transaction monitor's abort log with database/table/txn identity

[kriszyp]

⊙ Problem

When the long-transaction monitor aborts a write-bearing transaction, its error line named only the visited link's bare table (from table: <table> path: <url>). It gave no database, no native transaction id to join against the long-lived-holder sweep, and on LMDB no started from resource. For a transaction spanning several databases, operators had no way to tell which store and which native handle the abort belonged to. The newer stuck-commit log already solves this with describeCommitIdentity().

❓ Your call: Is the requirement warranted? As specified, yes: this is the line an operator sees when a write is rejected for running too long, and the stuck-commit lines already use this format. The one sibling touching the same line, #372, wraps it in harperLogger.status() without changing its text, so the two compose.

💡 Solution

Both engines' abort lines now go through the shared describeCommitIdentity() export. The line becomes:

Transaction was open too long and has been aborted after exceeding the open-transaction limit, from table: <db>.<table> (transaction <native id>)[, started from <Resource>.<method>] path: <url>

The request-path suffix both lines already carried is kept. No write, poison or abort ordering changes: both engines still log synchronously and then call abortDueToTimeout() on the chain head.

❓ Your call: Only the two abort error lines are converted. The monitor's sibling warn lines (commit-phase grace, released read snapshot) keep the old from table: <name> path: shape, so a transaction that is first spared and later aborted is logged in two formats. Converting them is a cheap follow-up; this PR stays on the lines the task named.

❓ Your call: The line names the link the monitor visited (txn.db, txn.transaction), while the abort is applied to commitChainHead. In a two-database chain the line can name database B while the rejection surfaces at head A. I kept the visited link because it is the one whose idle limit expired, and its native id is what the long-lived-holder sweep reports. Naming the head instead is a one-argument change.

🔧 Changes

• resources/DatabaseTransaction.ts: exports describeCommitIdentity(). It catches a throwing .id read, since rocksdb-js throws "Transaction has already been closed" on a reset handle. A throw while building the line would skip the abort and the rest of that monitor tick. The RocksDB abort line now uses the helper; started from moves from a separate logger argument into the message.

  ❓ Your call: The guard lives in the diagnostic rather than in rocksdb-js's Transaction.id (which could return undefined on a closed handle). It is local, cheap and removable once the binding changes.

• resources/LMDBTransaction.ts: imports the helper and uses it for its own copy of the abort line. LMDB gains the started from context; it has no native id to print.

  ❓ Your call: The log shape changes: a database prefix, an inline started from, and (transaction N). An outside alert keyed on from table: <table> path: or on the separate "was started from" argument stops matching, with no error. In-repo matchers key only on the Transaction was open too long / has been aborted prefix, apart from the integration test updated below.

✅ Verification

• integrationTests/database/delete-index-atomicity-rocksdb.test.ts: Arm A's matcher accepts the new db.table (transaction N)[, started from R.m] shape and still pins the table and path: /SlowMixedHold/. The old regex fails against the new line. npm run test:integration -- integrationTests/database/delete-index-atomicity-rocksdb.test.ts passed 9/9 before the rebase. It has not been re-run on the rebased head; CI's integration workflow covers it.
• unitTests/resources/longLivedTransactions.test.js, "monitor abort attribution": drives a real two-database, non-source-apply RocksDB transaction through the monitor's abort branch, with the second link reachable only through the head's chain. It asserts the poisoned commit still rejects (the harper#2062 invariant), then that the line names the head's db.table and native id. LMDB is skipped: LMDB has no native id to assert.
  • The first version was red on every Linux CI leg. It expected database test, but the unit config aliases data/dev/test/test2 onto one RocksDB root that takes the name of whichever alias opens it first. Inside the full suite, an earlier file had opened it as data. The head table now has its own database. The rejection check now rethrows anything other than the poison error, so a failed setup assertion reports under its own message.
  • Reproduction: a table declared in data before the original test produces CI's exact failure; the fixed test passes under the same ordering.
  • Mutation: making abortDueToTimeout() a no-op fails the test with Missing expected rejection: the poisoned commit must still surface as a rejection….
• npx mocha unitTests/resources/longLivedTransactions.test.js: 52 passing.
• npm run test:unit:resources:core on the rebased head: 3414 passing, 0 failing. Before the test fix: 3413 passing, 1 failing (this test).
• npx prettier --check and npx oxlint --quiet on the changed files: clean.
• CI at 4b0cbc134:
  • Unit Test Node 22, Node 24 and Windows: green. The fixed test passes on 22, 24 and 26.
  • Unit Test Node 26: red only in test:unit:main (nodeAdapterMiddleware, npmUtilities dry_run). The same 7 failures occur on main 26a914501 under the Node v26.11.1 runner.
  • Integration: all shards green.
  • Next.js adapter: one use cache test failed on each of Node 22 and Node 24, a different test on each leg. No abort-log line appears in any of that run's server logs, and a re-run passed.

❓ Your call: The new unit test covers RocksDB only. The LMDB call site is reached by no test. Its reads are optional-chained and the line has no native id, so the crash risk is low; an LMDB variant can be added later.

— Claude Opus 5.5

🤖 Generated with Claude Code

https://claude.ai/code/session_01V5mxNqhsJGMSr9SYiDUsTt

Related PRs: #372 overlaps (wraps the same RocksDB abort line in harperLogger.status(); whichever lands second keeps both the wrapper and the new message)

Complexity: easy

_{Review-Coverage: authored=claude; ran=cursor-muse,gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi; rounds=10; full=2 @ b58cba8}

_{Review-Attention: study ~13m (critical: DatabaseTransaction.ts, LMDBTransaction.ts; decisions: abort-log-attributes-visited-link, convert-only-the-abort-line, error-line-format-change) @ b58cba8}

[gemini-code-assist]

Code Review

This pull request improves transaction abort logging by standardizing the output format using describeCommitIdentity across database transactions (including RocksDB and LMDB). It also safely wraps native transaction ID access in a try-catch block to prevent crashes on already-closed handles. A new integration test suite is added to verify the attribution of aborted transactions. The review feedback suggests increasing the transaction expiration timeout in the new test from 20ms to 200ms to prevent flakiness in resource-constrained CI environments.

[kriszyp]

Why "monitor abort attribution" was red on Unit Test Node 22/24/26

The production change was fine. The test expected the wrong database name, and its assertion structure hid that.

• The test declared its head table in database test and expected from table: test.MonitorAbortPrimaryTable (transaction N).
• The unit config (unitTests/mocha.init.js, unitTests/testUtils.js) points data, dev, test and test2 at one RocksDB root. initStores in resources/databases.ts names that root with rootStore.databaseName ??= …, so the first alias to open it wins.
• Run alone, the root is first opened as test and the test passes. In npm run test:unit:resources:core, an earlier file opens it as data. The line then reads from table: data.MonitorAbortPrimaryTable/@… (transaction 81252) (captured locally from the full suite), and the assert.match fails.
• That assert.match ran inside the transaction callback. Its AssertionError became the transaction's rejection, the /exceeding the maximum open-transaction time/ validator didn't match it, and Node reported the custom message "the poisoned commit must still surface as a rejection". The poisoned commit had in fact rejected correctly.

Fix (test only, 4b0cbc134, branch rebased onto current main with no conflicts):

1. The head table gets its own database (monitor-abort-primary), so the logged name is deterministic.
2. The assert.rejects validator is a function that rethrows anything other than the open-transaction error, so a failed setup assertion inside the callback reports under its own message.
3. The attribution assert.match runs after the rejection check.

Evidence

• Repro: declaring one table in data before the original test produces CI's exact failure in about 3 s. The fixed test passes under the same ordering.
• Poison invariant still guarded (harper#2062): with abortDueToTimeout() made a no-op in dist/, the fixed test fails Missing expected rejection: the poisoned commit must still surface as a rejection once the callback returns.
• npm run test:unit:resources:core: 3414 passing, 0 failing. Before the fix: 3413 passing, 1 failing (this test).

The ??= naming also applies in production if two configured databases share one path: describeCommitIdentity() then prints whichever alias opened the root first. That predates this PR and also affects the stuck-commit log; I've noted it as a possible follow-up rather than changing it here.

— Claude Opus 5.5