[kriszyp]

A database close during a pending source fill must settle waiters and tolerate late lock release.

⊙ Problem

A worker restart can close the database before a source fill settles. Late unlock throws, and a waiter reading inside the native callback can hang.

🤔 Requirement

❓ Your call: Preserve both fixes. Native close already releases the owning handle's locks.

💡 Solution

The native callback wakes the request. The read runs after await, so closed-store errors reject normally. The timer also wakes it. releaseRecordLock attempts unlock and suppresses closed-store failures; it logs open-store failures.

Rebase after #3417

Rebased onto main at 79e6f6533. #3417's sharing guards, store-first lookup, map registration and cleanup remain intact. Both commit outcomes call releaseFill, which deletes its map entry and uses releaseRecordLock. The merge changes none of #3490's shutdown behavior. The existing 10 s timeout policy is preserved.

⚖️ Alternatives

• A native unlock-after-close no-op needs two engine releases and still leaves the hung native callback.
• Draining pending sources would make shutdown await arbitrary origin calls; aborting them needs a cancellation protocol.
• Keeping either waiter block wholesale would discard one of the requested fixes.

Framing-Verdict: chosen-approach-sound

🔧 Changes

• resources/Table.ts: the wake-only callback and timeout resume the store-first waiter logic. Look hardest at releaseFill: it keeps Make concurrent sourcedFrom misses share one source call when nothing gets cached #3417's map cleanup and uses releaseRecordLock, including the root-store status check. Each commit outcome releases through releaseFill once.
• resources/DESIGN.md and its root index entry record the close/wake invariant. An existing format paragraph is only reflowed to meet the line budget.

Native ownership and transaction evidence

RocksDB keeps record locks in DBDescriptor::locks, shared per database path across handles. DBHandle::close() calls lockReleaseByOwner(this) (db_handle.cpp:169, present since May 2026). LMDB's EnvWrap::closeEnv releases the environment's locks; Harper closes environments rather than dbis. These locks are separate from the transaction verification table.

The second RocksDB handle keeps the descriptor observable after close: the owned key becomes unlocked and open native transactions are zero. Cache writes pass through assertDatabaseCommitAllowed / commitTrackedRocksTransaction, which abort a transaction after close. This evidence covers the tested path; the real worker restart remains queued for QA.

🤔 Preserve the foreign-lock timeout

After this handle closes, a waiter on another handle's lock uses the existing 10 s timer. Its native notification remains queued until foreign unlock or worker teardown. The task preserves that fallback; immediate cancellation would require a native API or a fill registry. The inherited open-handle timeout can also break a live leader’s lock and allow overlapping fills beyond 10 s. That ownership-policy redesign is outside this rebase and is recorded for follow-up.

🤔 Keep all unhandled faults visible

The fixture temporarily replaces Mocha's process-fault listeners. Every captured fault still fails the test; filtering by fixture would hide errors Mocha normally reports. Its polling now has a bounded diagnostic wait, and teardown tolerates failed setup.

✅ Verification

• Cache-fill close regressions and coalescing/caching: RocksDB 66 pass; LMDB 23 pass / 1 skipped. Base Table.ts fails all 4 applicable close cases on RocksDB and 3 on LMDB. Owned locks release on close; the fixture finishes with 0 open native transactions.
• Full LMDB resources: 4,383 pass / 1,215 skipped. Build, typecheck, lint, format and design-doc checks pass.
• Existing HTTP coalescing integration: 2/2 pass against this worktree's built distribution, covering missing records and Cache-Control:no-store.
• Full integration: 3,013 pass / 4 failures / 16 skipped / 2 todo. All failures are unchanged Ollama generation tests: the requested model alias returns 404 on this host; a family variant is installed.
• Old coverage job failed the PR body's framing-verdict gate. It did not report a numeric coverage drop or the audit-log flake. The required verdict line is now present; both current-head coverage runs pass.

Broader local gates that are not clean

Full RocksDB resources: 5,788 pass / 235 skipped / 1 failure, updateAttributesLock.test.js:258, expected one global warning and got two. That unchanged schema-lock file passes 9/9 on branch and base; its helper is identical on both.

The combined core unit command timed out at 30 minutes after reporting eight component/filesystem failures. All eight cases pass as a targeted set on branch and base; the full-process failures remain unclassified. No clean full-core gate is claimed.

Current CI: runLinter fails because the merged resources/DESIGN.md is 1,001 lines. Main at 92aa9f908 fails the same job with the same line-budget error; this worktree has 996 lines. Other jobs remain in progress.

PR QA: Do cache-fill callers settle across a real HTTP-worker restart? — queued @ 6c40f07, qa-pr-harper-3490-6c40f07

Refs #3470

Docs: none needed; no public API or configuration change.

_{🤖 Generated by OpenAI Codex; posted via @kriszyp.}

Release target: main — the owner decides a v5.3 backport
Related PRs: #3417 overlaps (merged; shared waiter block); #3610 overlaps (merged; source-fill stamp ordering already in base); 14 others independent
Complexity: complicated

Origin — the dispatch brief this PR was written from

Fix a record-lock lifecycle bug in Harper core on main: the cache-fill (source resolution) path in resources/Table.ts releases its record lock in the commit settle callbacks without checking that the store is still open, roughly: when(commitPromise, () => { primaryStore.unlock(id); }, (error) => { primaryStore.unlock(id); if (resolved && !(disposed && error?.code === 'ERR_COLUMN_FAMILY_DROPPED')) logger.error?.('Error committing cache update', error); });. When a worker restart (restart_service for http) closes the thread's databases before that commit settles, rocksdb-js Store.unlock throws 'Database not open' (rocksdb-js src/store.ts, via PrimaryRocksDatabase.unlock in src/database.ts). The throw is an unhandled rejection on the job/worker thread, and the lock taken for that id may never be released. Find the root cause and fix the close/unlock lifecycle so that closing a database during a pending cache-fill commit neither leaks the record lock nor produces an unhandled rejection. First establish what actually happens to a native key lock when its database handle closes: rocksdb-js keeps locks in a process-global slot array (the verification table), so a lock may outlive the handle that took it and block other threads or databases. The fix belongs where that invariant is owned: if close already releases a handle's locks, an unlock after close is a safe no-op and should be treated as one; if it does not, the lock must be released (or close must release it) rather than the error merely being swallowed.

Acceptance

A regression test reproduces the close-before-commit-settles case and fails on unmodified main (unhandled rejection and/or held lock), and passes with the fix; it asserts the lock is released and no unhandled rejection occurs. The PR states what happens to a native key lock when its database closes, with evidence, and whether this path can leave a transaction handle open. All other primaryStore.unlock sites with the same pattern are fixed or explained. test:unit:resources passes apart from known pre-existing failures, which the PR names. Draft PR only; if the right fix is in rocksdb-js, open that PR too and link them.

_{Dispatch: task harper-cache-fill-unlock-after-close · queued by kris-session · ran by claude/opus/high · worker kzyp-xps-1}

Late Grok findings

3 surviving (highest major) — Grok reviewed 27fc064, ruled against 6c40f07

• major — resources/Table.ts:11891 — With a source pending beyond 10 seconds, a waiter unlocks the leader’s lock and starts another fill; the original fill can then unlock its replacement.
• major — resources/Table.ts:11890 — Closing a waiter’s handle behind a foreign lock leaves rejection waiting for the 10-second timer, with no explicit callback cancellation.
• minor — unitTests/resources/cacheFillClose.test.js:119 — Unrelated process errors fail the test, while commit errors arriving after its 50 ms observation window escape the assertion.

Not standing: 2 resolved. Grok ran 22m (22m from launch to report); the fold adjudication (cursor-grok@27fc06418c1b-b4b50179, codex) took 2m.

Deferred findings

• df753f00d9e6c5ca major concurrency — resources/Table.ts — Timeout waiter unlocks a live source-fill owner — NOT FILED
• bdab0665fc7e6f25 major reliability — resources/Table.ts — Foreign-lock close delay and retained callback — NOT FILED

_{Review-Coverage: authored=codex; ran=cursor-composer,gemini,cursor-grok; late=cursor-grok:adjudicated; adjudicated=domain; declined=muse,claude; rounds=5; full=2 @ 6c40f07}

_{Review-Attention: deep ~25m (critical: Table.ts; decisions: preserve-timeout-policy, native-close-ownership, global-fatal-faults; raised: degraded review) @ 6c40f07}

[gemini-code-assist]

Code Review

This pull request addresses an issue where a database close during an active cache fill could cause unhandled exceptions or leave requests unsettled. It introduces a safer lock release mechanism ('releaseRecordLock') that gracefully handles closed stores, refactors the waiting logic using a promise-based wake callback, and documents these behaviors in 'DESIGN.md'. Additionally, a comprehensive test suite is added in 'cacheFillClose.test.js' to verify these scenarios across database closures. No review comments were provided, and the implementation aligns well with the repository's guidelines, so there is no further feedback to provide.