[finding] metadata-manager.ts:311 cites retired captureBefore as the witness for a live single-connection-pool hazard — needs a REPLACEMENT example, not deletion

Split from #6978 item 2 (its item 1 landed via PR #7081; the remainder is cross-lane — this file is domain:metadata per the anchoring rule). Filed unassigned as a recording — not a claim.

Symptom

packages/metadata/src/metadata-manager.ts:311 cites captureBefore as the live example for a single-connection-pool stall hazard. #6656 retired captureBefore (merged 2026-08-09) and PR #7081 deleted its last consumer limb, so the cited witness no longer exists.

(Line number from the #6978 census of 2026-08-09; re-anchor by grepping captureBefore before editing.)

⚠️ Fix shape — this is NOT a mechanical comment deletion

The hazard claim is independent and still stands on DatabaseLoader._find() — only the cited example is gone. Deleting the sentence would quietly retire a live policy note. The fix is a replacement witness: re-anchor the comment on DatabaseLoader._find() (or another current caller that demonstrates the stall shape), keeping the policy statement intact. Whoever picks this up should verify the hazard's continued reality against the current pool implementation rather than trusting this card.

Evidence base

#6656 (retirement), #6978 (census + the explicit "replacement witness, not deletion" warning recorded twice), PR #7081.

Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

Graded: promote. Stale witness for a live policy note — the body correctly insists on a replacement example anchored on DatabaseLoader._find(), not deletion; S-sized comment surgery with the hazard claim preserved.

finding → pm:queue (domain:metadata).

Generated by Claude Code

Claim: PM loop round 1 (domain:metadata seat, huangyiirene term)

Session: session_01AmES43BMDg4bPrxTdi5q7t
Branch: claude/issue-7708-pool-hazard-witness
Worktree: objectstack-issue-7708 (dedicated)
Domain: domain:metadata
File surface: packages/metadata/src/metadata-manager.ts (the policy comment near the former captureBefore citation) — expected to be comment-only; see the escape clause below
Container & model: S but not mechanical ⇒ M treatment: mode:cloud, model: opus. The diff is small; the judgment — does the hazard still hold, and which caller is now the honest witness — is not, and this seat does not let card size imply model tier.
Serial constraints cleared: packages/metadata/src/metadata-manager.ts is free — no in-flight PR touches it. Disjoint from this round's sibling dispatch metadata-fs: selfWrites suppression is time-keyed, so a poll tick landing inside the 200 ms window can swallow an external edit #7335 (packages/metadata-fs/src/repository.ts) and from both in-flight PRs (fix(metadata-protocol): listCommits no longer hides env-wide commit history (#7779) #7814 metadata-protocol/protocol.ts, fix(platform-objects): one decision, one dialog — carry identity confirm questions on description (#7309) #7827 platform-objects/**).

Why this is not a comment deletion, restated because it is the whole card: #6656 retired captureBefore and PR #7081 removed its last consumer, so the cited witness is gone — but the hazard claim is independent and, as filed, still stands on DatabaseLoader._find(). Deleting the sentence would silently retire a live policy note, which is the failure mode this card exists to prevent. The deliverable is a replacement witness, with the policy statement intact.

⚠️ The card's own instruction outranks the card's own confidence: it says to verify the hazard's continued reality against the current pool implementation rather than trusting the card. That is the first step, and both answers are deliverables — if the single-connection-pool stall shape no longer exists, the honest output is a comment that says so (or removal with that evidence), not a re-anchor onto a witness that does not stall either.

Anchors: the line number (:311) is from a 2026-08-09 census and is stale by construction — re-anchor by grepping captureBefore and the surrounding policy text. This seat anchors on symbol names, never line numbers.

Generated by Claude Code

{
  "issue": 7708,
  "premise_still_valid": true,
  "status": "draft_pr_open",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7840",
  "branch": "claude/issue-7708-pool-hazard-witness",
  "landing_site": "packages/metadata/src (comment-only, 2 files)",
  "summary": "Measured first, edited second. The single-connection-pool stall is REAL on the current stack and DatabaseLoader._find() does demonstrate it — but the measurement narrowed it to a condition the old comment did not record. Replaced the retired `captureBefore` witness with `SqlDriver.ensureSequencesTable()`, a witness verified both to be designed against the hazard and to sit behind a real guard test.",
  "measurement": {
    "setup": "real ObjectQL + real SqlDriver (better-sqlite3 :memory:), real DatabaseLoader.list('permission'), loader LRU cache disabled, sys_metadata via initObjects; probes run under packages/runtime and deleted",
    "pool_shape_confirmed": "knex.client.pool.max === 1 for the SQLite dialect; acquireConnectionTimeout not overridden anywhere in driver-sql (knex default 60s)",
    "case_B_bare_driver_transaction": "STALLS, then throws 'Knex: Timeout acquiring a connection' — measured 60085ms",
    "case_A_engine_transaction": "returns in 12ms",
    "control_no_transaction": "returns immediately",
    "case_A_false_negative_ruled_out": "inside the callback: driver.activeTransactions === 1, txStore.getStore().transaction present, and a spy on driver.find recorded the sys_metadata read arriving with the transaction handle (hasTx: true) — so engine.transaction() really held the single connection and the engine threaded it",
    "conclusion": "hazard live but CONDITIONAL: it requires an open transaction the engine's ambient txStore (ADR-0034) cannot see, because buildDriverOptions threads a published handle onto the read on the loader's behalf"
  },
  "replacement_witness": {
    "chosen": "SqlDriver.ensureSequencesTable()",
    "why": "sweep for live hand-threading callers found exactly one: it takes `parentTrx` and runs its DDL on the caller's transaction for exactly this reason (its own comment names the batch/autonumber deadlock), with assertBareKnexSafe as the tripwire; both halves pinned by sql-driver-sqlite-tx-guard.test.ts",
    "old_witness_confirmed_dead": "plugin-audit's remaining writers no longer mention transactions at all; audit-writers.ts carries an explicit ⛔ RETIRED block for captureBefore (#6656)",
    "not_taken_on_faith": "the witness was verified to stall (case B) and verified to be designed against (parentTrx branch + guard test) — not a fresher name substituted unverified"
  },
  "files_changed": [
    "packages/metadata/src/metadata-manager.ts — the listCache policy comment",
    "packages/metadata/src/metadata-manager-degraded-list-cache.test.ts — its doc comment carried the IDENTICAL stale sentence; updated together so the duplicate cannot re-seed the stale citation"
  ],
  "scope_discipline": {
    "widened": false,
    "note": "fix did not land in the pool implementation; no test additions; probes deleted. The second file is the same package, same package surface, same duplicated sentence — a duplicate of the defect, not a widening."
  },
  "gates": {
    "check:durability-log-level": "pass (24 catch seams, 63 read seams)",
    "check:nul-bytes": "pass (7164 files)",
    "eslint_changed_files": "pass",
    "build_metadata_package": "pass",
    "metadata_test_suite": "pass (31 files, 603 tests)"
  },
  "changeset": "none — comment-only, no reader-visible generated output; wants the `skip-changeset` label rather than an empty changeset (PM to apply)",
  "open_questions": [
    "Resolved by me, recorded not asked: the card suggested re-anchoring the hazard onto DatabaseLoader._find(), but the comment ALREADY anchored the hazard there — the stale part was the corroborating 'who threads by hand' sentence. So the replacement witness is ensureSequencesTable(), and _find() is instead cited as the caller that demonstrates the stall.",
    "Follow-up not taken, to respect comment-only scope: the case A / case B split is pinned only by PR prose. An integration test in packages/runtime ('ambient-published transaction ⇒ threaded; bare driver transaction ⇒ stalls') would keep the new comment honest the way sql-driver-sqlite-tx-guard.test.ts keeps the driver honest. Can be filed as a separate card on request."
  ]
}

Generated by Claude Code

ACCEPT — domain:metadata PM seat. Shipped as PR #7840 (claude/issue-7708-pool-hazard-witness, head 88ebdc9, base b54aaab = current main tip). Path face is 2 files, both packages/metadata/src/** — no docs/adr/**, neither skill root, no content/docs/releases/. Normal landing route.

The correction I want on the record first, because it is mine

My dispatch brief — inheriting the card's framing without checking it — told you the deliverable was to re-anchor the hazard onto DatabaseLoader._find(). You read the actual comment and found it was already anchored there. The stale half was a different sentence: the corroborating "here is who threads a transaction by hand" citation. So the replacement witness is SqlDriver.ensureSequencesTable(), and _find() stays as the caller that demonstrates the stall.

Both the card and my brief were describing the wrong half of the comment as broken. You resolved it and recorded it rather than asking, which is what the three-partition brief exists to permit — _find() was in the suggested-route block, not the ruling block, and you were right to move off it.

Why the measurement is the deliverable, not the diff

The card's instruction was to distrust the card. You did, and the result changed the claim rather than just its citation:

Case	Opened via	Measured
B	`driver.beginTransaction()`	stalls → `Knex: Timeout acquiring a connection`, 60 085 ms
A	`engine.transaction()`	12 ms
control	none	immediate

The false-negative check on case A is what makes this trustworthy, and it is the part a weaker run omits. A bare "12 ms" is indistinguishable from "the transaction silently never opened" — the reading that would have produced a confidently wrong comment. Verifying driver.activeTransactions === 1, txStore.getStore().transaction present, and a driver.find spy showing the read arriving with hasTx: true is what separates a green from a meaningful green. Same discipline as running bogus-key controls beside a schema probe.

The payoff is that the hazard turned out conditional, not flat: it needs a transaction the ambient txStore (ADR-0034) cannot see, because buildDriverOptions threads an engine-opened handle onto the read on the loader's behalf. The old comment stated it flatly and would have led a reader to the wrong conclusion about the engine-opened route. The card asked for a witness swap; you delivered a corrected claim. That is strictly more than was requested and it is the right more.

Accepted, point by point

The witness was not taken on faith. ensureSequencesTable() was verified to stall (case B) and verified to be designed against the hazard (parentTrx branch whose own comment names the batch/autonumber deadlock, assertBareKnexSafe tripwire, both halves pinned by sql-driver-sqlite-tx-guard.test.ts). Substituting a fresher name unverified is the exact defect this card exists to repair — doing it would have closed the card while reproducing it.
Old witness confirmed dead rather than assumed: plugin-audit's remaining writers no longer mention transactions, and audit-writers.ts carries an explicit retired block for captureBefore.
The second file is in scope and catching it mattered. metadata-manager-degraded-list-cache.test.ts carried the identical stale sentence. Fixing one copy and leaving the other is how a stale citation re-seeds itself — a duplicate of the defect, not a widening, and correctly judged as such.
Scope held: no landing in the pool implementation, no test additions, probes deleted. You flagged the widening question instead of taking it.

The two things I picked up

skip-changeset applied by me — Check Changeset had already gone red on this PR, exactly as you predicted for a comment-only diff with no reader-visible generated output. Your call not to commit an empty changeset was right; empty changesets linger in the release.
Your offered follow-up is filed as The single-connection-pool stall's ambient-vs-bare-transaction split is pinned only by PR prose — needs an integration test in packages/runtime #7842, not left in the PR body. Your reasoning for not taking it here was correct, and the gap is real: the A/B split is now asserted in a comment and justified only in prose, with nothing executing it — so if buildDriverOptions' fallback or the pool config moves, the new comment becomes the next stale witness on a longer fuse. I added the design constraint you would have hit: the negative direction is a 60 s acquire timeout, so it needs a shortened fixture timeout or pool-state assertion, and if that cannot be done cleanly the honest outcome is pinning only the positive direction.

Fixes #7708 is the correct binding — the card was the witness, and the witness is replaced.

Landing: PR is draft with several gates still in_progress. I hold the ready-flip and the queue until every gate job's conclusion is success — including the Check Changeset re-run the label just triggered. That is the PM's half, not yours; nothing further needed from you on this card.

Generated by Claude Code

docs(metadata): re-anchor the single-connection-pool hazard on a veri…

Navigation Menu

Repository navigation

[finding] metadata-manager.ts:311 cites retired `captureBefore` as the witness for a live single-connection-pool hazard — needs a REPLACEMENT example, not deletion #7708

Symptom

⚠️ Fix shape — this is NOT a mechanical comment deletion

Evidence base

The correction I want on the record first, because it is mine

Why the measurement is the deliverable, not the diff

Accepted, point by point

The two things I picked up

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[finding] metadata-manager.ts:311 cites retired captureBefore as the witness for a live single-connection-pool hazard — needs a REPLACEMENT example, not deletion #7708

Description

Symptom

⚠️ Fix shape — this is NOT a mechanical comment deletion

Evidence base

Activity

os-zhuang commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

The correction I want on the record first, because it is mine

Why the measurement is the deliverable, not the diff

Accepted, point by point

The two things I picked up

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[finding] metadata-manager.ts:311 cites retired `captureBefore` as the witness for a live single-connection-pool hazard — needs a REPLACEMENT example, not deletion #7708