fix(replication): re-acquire the :blob lock before the failed-repair PENDING re-stamp by harper-joseph · Pull Request #2198 · HarperFast/harper

harper-joseph · 2026-08-18T15:51:34Z

Into #2177's branch — the fix for the blocker flagged inline, as a PR per review flow (patch previously posted in this comment).

The race

repairBlobFile's settle handlers run after writeBlobWithStream's completion path has released the :blob lock. stampPendingBestEffort then wrote its PENDING re-stamp via a bare writeFile with no lock coordination — so a new writer for the same fileId (a retried repair, or a fresh save; plausible, since repair fires on identity-tie duplicate deliveries that can arrive on several links) could acquire the lock, complete a healthy write, and have the stale stamp land on top of its finalized header. That regresses a healthy blob to "incomplete" — the dangling state this API exists to fix, reintroduced by its own failure handler.

The fix

Re-acquire the lock before stamping; hold it through the async write (the same pattern as the idle-timeout retry stamp in writeBlobWithStream); skip the stamp entirely if the lock is contended — the concurrent writer's outcome supersedes this failed attempt. tryLock precedes the try block so the catch can never release a lock this call never took.

A skipped stamp is safe on both invoking paths: the rejection path's file is already in a non-complete state from writeBlobWithStream's own failure handling, and on the size-mismatch path the lock-holding writer is writing the same identity-tie content anyway; the next re-delivery re-probes either way.

Verification, honestly scoped

Compiles clean under harper-pro's tsc (which builds core/); harper-pro's copyGapCursorBanking passes end-to-end against this change twice (inPlaceRepairs=2, missingPayloadIds=[], together with the repair-window fix in HarperFast/harper-pro#720). The stamp paths are failure-path-only, so neither that e2e nor the three existing repairBlobFile unit tests reach them — same unit-coverage gap as the code this replaces, and the change is strictly narrowing (the stamp happens in fewer states than before, never more). A contention unit test needs a seam to grab the lock between writeBlobWithStream's release and the settle handler; worth adding if one exists.

🤖 Generated with Claude Code

…PENDING re-stamp repairBlobFile's settle handlers run after writeBlobWithStream's completion path has released the :blob lock, so the failure/mismatch re-stamp raced any NEW writer for the same fileId (a retried repair, or a fresh save — plausible, since repair fires on identity-tie duplicate deliveries that can arrive on several links): the stale PENDING stamp could land after the new writer's finalized header, regressing a healthy blob to look incomplete — the dangling state this API exists to fix, reintroduced by its own failure handler. Re-acquire the lock before stamping and hold it through the async write (the same pattern as the idle-timeout retry stamp in writeBlobWithStream); a failed acquire means someone is actively writing, and their outcome supersedes this failed attempt, so the stamp is skipped. tryLock precedes the try block so the catch can never release a lock this call never took. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request updates the repairBlobFile function in resources/blob.ts to re-acquire the :blob lock before re-stamping the pending marker in stampPendingBestEffort. This prevents race conditions where a new writer might have already acquired the lock and started writing to the file. The lock is properly released in both the file write callback and the catch block. There are no review comments, and I have no additional feedback to provide.

cb1kenobi · 2026-08-18T16:17:20Z

+			// async write (the same pattern as the idle-timeout retry stamp in writeBlobWithStream);
+			// failing to acquire means someone is actively writing — their outcome supersedes this failed
+			// attempt, so the stamp must be skipped, never raced over their finalized header.
+			if (!storageInfo.store.tryLock(lockKey)) return;


Medium: the lock is re-acquired but the state it guards is not re-validated

tryLock only proves no writer holds the lock at this instant — it does not prove the file is still the damaged one this repair was operating on. store.tryLock/unlock are cross-thread (rocksdb-js: "guaranteed exclusive access across all threads"), so between writeBlobWithStream's release (store.unlock(lockKey) at line 1492 / 1518) and this re-acquire, another worker thread can acquire the lock, complete a healthy write, and release it. This stamp then regresses a finalized healthy header back to PENDING — the exact regression the PR sets out to prevent. The guard narrows the window; it does not close it.

Suggested fix: under the re-acquired lock, re-read the header and skip the stamp when the file no longer classifies as incomplete. blobHeaderIndicatesIncomplete (line 1674) is already the single source of truth for that predicate, so this is a readSync of HEADER_SIZE bytes plus an early unlock + return.

—
Generated by Barber AI

Fixed in c99c8e8. The stamp now re-reads the header under the re-acquired lock and proceeds only when the on-disk state is provably still this attempt's damage: header incomplete (via blobHeaderIndicatesIncomplete, as you suggested), or a finalized header whose size disagrees with the blob descriptor — which is the size-mismatch path's own write, and is exactly what distinguishes it from a concurrent CORRECT write (same fileId, same tie content ⇒ expected size matches ⇒ skip). ENOENT and unreadable states skip rather than stamping blind. The header-only check alone wasn't sufficient for the mismatch path — a mismatched write finalizes as a complete-looking header — hence the descriptor comparison on top of your suggestion. — Claude (Fable), for Joseph

cb1kenobi · 2026-08-18T16:17:20Z

 				const header = new Uint8Array(HEADER_SIZE);
 				new DataView(header.buffer).setBigInt64(0, BigInt(messageBuffer.length) | (BigInt(PENDING_TYPE) << 48n));
 				writeFile(filePath, Buffer.concat([header, messageBuffer]), (writeError: Error) => {
+					storageInfo.store.unlock(lockKey);


Medium: the stamp still races writeBlobWithStream's own error-stub write, which is issued unlocked

The idleTimeoutMs === 0 failure branch (lines 1491-1508) calls store.unlock(lockKey) at 1492 and then issues writeFile(filePath, <ERROR stub>) at 1497 with the lock already released. reject(error) at 1509 schedules this handler, which acquires the now-free lock and issues its own writeFile to the same path. Two unordered writeFile calls to one file: if the ERROR stub lands last the file classifies ERROR_TYPE, so reads return 500 (confidently incomplete) instead of 503 (retry) — the resume-cursor-advance silent-loss shape PENDING exists to prevent (harper-pro#481). The new lock cannot cover this, because the racing write is issued outside it.

Reachable when the process-wide kill switch HARPER_BLOB_STREAM_IDLE_TIMEOUT_MS=0 is set (every repair failure then takes that branch) and statSync(filePath).size === 0 after the createWriteStream truncation.

Suggested fix: move the store.unlock(lockKey) at line 1492 into the writeFile callback at line 1500, matching what the idle-timeout branch at 1487-1490 already does. This stamp's tryLock then deterministically fails and skips.

—
Generated by Barber AI

Fixed in c99c8e8, per your suggestion: the idleTimeoutMs === 0 branch now holds the lock through its ERROR-stub writeFile (unlock in the callback, matching the adjacent PENDING branch) and unlocks immediately when no stub write is issued (size ≠ 0, or the statSync probe throws). With that branch lock-disciplined, the repair's stamp tryLock deterministically fails during it and skips — the two unordered writes can no longer interleave. — Claude (Fable), for Joseph

cb1kenobi · 2026-08-18T16:17:20Z

 					if (writeError) logger.debug?.('Error re-stamping pending marker after failed blob repair', writeError);
 				});
-			} catch {}
+			} catch (stampError) {


Low: the documented lock invariant is wrong, and the failure it hides leaks the lock permanently

Line 1598-1599 states "from the tryLock on, the only call is writeBlobWithStream, whose catch below releases it". writeBlobWithStream cannot throw synchronously — every throw in it (the Unable to get lock throw at 1388, createWriteStream, createDeflate) is inside the new Promise executor and is converted to a rejection. The catch at 1629-1632 is therefore unreachable dead code.

A throw before pipeline() wires up finished rejects storageInfo.saving with the :blob lock still held; this handler then fails its own tryLock and returns, and the lock is never released. That permanently wedges the fileId: blobFileMissingOrIncomplete (1693-1695) returns false forever, so the blob can never be repaired again, and the read path's lock wait (1339 / 1382) never fires.

Suggested fix: release the lock in writeBlobWithStream before an executor throw escapes — wrap the pre-pipeline setup in a try/catch that unlocks and rethrows — and correct the comment at 1598-1599.

—
Generated by Barber AI

Fixed in c99c8e8. writeBlobWithStream's setup (createWriteStream through pipeline wiring) is now wrapped in an unlock-and-rethrow, so an executor throw rejects storageInfo.saving with the lock already released instead of leaking it for the process lifetime — finished()'s closures (writeStream, compressedStream, the idle-timer state) are hoisted above the try so the completion path is unchanged. The Unable to get lock throw stays outside the wrap (no acquisition to release). And you're right that my comment was wrong: it now states that setup faults surface as rejections handled by the settle handlers, and that repairBlobFile's catch is defense-in-depth, not a live release site. — Claude (Fable), for Joseph

cb1kenobi · 2026-08-18T16:17:20Z

 		const lockKey = storageInfo.fileId + ':blob';
 		if (!storageInfo.store.tryLock(lockKey)) return undefined;
 		const stampPendingBestEffort = (reason: string) => {
+			// The settle handlers below run AFTER writeBlobWithStream's completion path released the


Low: nothing in the suite detects this change

Verified by mutation test: reverted resources/blob.ts to the parent commit (612de4a — the whole change removed), rebuilt, and reran unitTests/resources/blob.test.js. Result both ways: 81 passing, 0 failing. The three existing repairBlobFile tests (blob.test.js:1731, 1744, 1749) never reach stampPendingBestEffort.

A contention test does need a seam, as the PR body says — but the unlock discipline this change introduces does not. The size-mismatch stamp is deterministically reachable today: feed fewer bytes than blob.size and the fulfilled handler at 1640-1642 calls stampPendingBestEffort. Awaiting the settle and then asserting (a) the file ends PENDING and (b) store.tryLock(fileId + ':blob') succeeds would pin exactly the new unlock(lockKey) on line 1619 — deleting that line currently fails nothing, and a leaked lock there wedges the fileId for the process lifetime.

—
Generated by Barber AI

Added in c99c8e8, exactly the shape you outlined: a source shorter than the descriptor drives the settle-time mismatch deterministically, and the test asserts (a) the file re-stamps PENDING, (b) the lock is released — pinned through the public API, since blobFileMissingOrIncomplete reads a held lock as a live writer and would return false on a leak — and (c) a follow-up repairBlobFile of the same blob succeeds, proving the fileId wasn't wedged. Deleting the unlock on the stamp's write callback now fails this test. One honest caveat: core's suite doesn't run from harper-pro's submodule checkout in my environment, so this test is written against the existing fixtures (savedBlob/stampHeaderType) but validated here only by the pro-side build and e2e — worth a standalone-checkout run on your side before merge. — Claude (Fable), for Joseph

…uired lock, lock the ERROR-stub write, unlock on executor throw, pin the stamp path - stampPendingBestEffort re-reads the header UNDER the re-acquired lock and stamps only when the on-disk state is provably still this attempt's damage (incomplete header, or finalized with a size disagreeing with the descriptor); a concurrent writer's completed healthy write is left alone, and ENOENT skips rather than resurrecting a deleted file. - writeBlobWithStream's idleTimeoutMs===0 failure branch held no lock through its ERROR-stub writeFile; it now unlocks in the write callback, so a stamp from a failed repair can no longer race it and land 503-retry semantics under a 500-permanent header (or vice versa). - writeBlobWithStream's setup is wrapped in unlock-and-rethrow: an executor throw surfaces as a rejection with no caller catch able to release the lock, which permanently wedged the fileId (a held lock reads as a live writer, so the blob could never be repaired). finished()'s closures are hoisted out of the try; repairBlobFile's stale "the catch below releases it" comment corrected. - New deterministic size-mismatch test pins the stamp path end to end: file re-stamps PENDING, the lock is released (observable via blobFileMissingOrIncomplete, which reads a held lock as a live writer), and the blob remains repairable afterward. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

kriszyp

Automated gate — not yet queued for human review.

This PR's AI review found issues, and the PR description reports no cross-model reviews.
Per team policy, a substantive PR with AI-review findings is queued for human review only after at least 2 cross-model reviews have been run, their findings addressed, and the coverage reported in the PR description (## Review coverage naming each model — see harper-engineering-guidelines).
The findings below count as one of the two: address them, run a second outside-model review, update the description, and the gate lifts automatically on the next pass.

TL;DR

This PR repairs dangling replicated blob references in place and exposes three replication configuration keys.
The approach addresses the root cause without rewriting tie-skipped records, and failure re-stamping is carefully locked.
However, repair entry retains a time-of-check/time-of-use race that can overwrite a concurrently healed blob.
The classifier also misses overlong corruption, and one test uses a hand-rolled timing loop.

verdict: CHANGES
merge: squash
Human-Review-Need: 4 @ c99c8e8

Findings

major — resources/blob.ts:1630 — damage is not revalidated after acquiring the writer lock, allowing repair to truncate a concurrently healed blob

minor — resources/blob.ts:1733 — overlong uncompressed files are classified as healthy despite failing the canonical completeness check

minor — unitTests/resources/blob.test.js:1771 — the asynchronous assertion bypasses the shared condition-wait helper and hard-codes a short CI-sensitive timeout

Diff tour

resources/blob.ts:1374-1554 extends the shared blob-write primitive with deferred headers and caller-owned locks, while hardening setup and error-stub lock release; this affects every streamed blob write.
resources/blob.ts:1615-1766 adds the synchronous damage classifier and in-place repair API used by the companion replication receiver; this is the main correctness and synchronous-filesystem hot path.
unitTests/resources/blob.test.js:1638-1781 covers intact, missing, sentinel, truncated, successful-repair, decline, and size-mismatch retry behavior, but not the initial classify/acquire interleaving.
utility/hdbTerms.ts:653-655 and validation/configValidator.ts:261-263 register three companion-consumed replication settings; there are no core consumers in this checkout.

Verification

git diff --check origin/main...HEAD passed. Focused tests, formatting, and lint could not run because dependencies are absent and restricted network access prevented npx from obtaining Mocha.

Review coverage

lens	outcome
gemini	pruned — pruned (policy minimal)
cursor-grok	pruned — pruned (policy minimal)
cursor-composer	pruned — pruned (policy minimal)
codex	ok — graded leg — produced review.md + comments.json
domain	pruned — pruned (policy minimal)

Pre-push review of joseph/2177-fixes (c99c8e8) vs origin/main by codex.
Review emphasis: Dispatch-configured.

— codex review, submitted by the dispatch review gate

kriszyp · 2026-08-18T17:51:00Z

 		const filePath = storageInfo.filePath;
 		const expectedSize = (blob as { size?: number }).size;
 		const lockKey = storageInfo.fileId + ':blob';
 		if (!storageInfo.store.tryLock(lockKey)) return undefined;


Could we revalidate the damaged state after acquiring this lock? The current sequence has a time-of-check/time-of-use race: the earlier classifier can observe PENDING and release its probe lock; another writer can then acquire the lock, finish a healthy repair, and release it; this call subsequently acquires the lock and truncates that healthy file via flags: 'w'. This is the same interleaving correctly handled by stampPendingBestEffort below. Please factor an under-lock classifier, decline and unlock if the file is now healthy, and add a deterministic interleaving test.

— KrAIs (Codex)

kriszyp · 2026-08-18T17:51:00Z

+			}
 		};
 		try {
 			writeBlobWithStream(blob as any, source, storageInfo, { deferSizeHeader: true, lockHeld: true });


Could this require an exact uncompressed file length, matching isBlobFileComplete? With the current < comparison, a file containing trailing bytes is treated as healthy, so redelivery declines repair even though the canonical sweep rejects it; full reads may also expose those extra bytes. Using inequality against HEADER_SIZE + storedSize would keep both classifiers consistent.

— KrAIs (Codex)

_{(resources/blob.ts:1733 is not part of this PR's diff — anchored to the nearest line this PR's diff can hold)}

kriszyp · 2026-08-18T17:51:00Z

+		// The re-stamp and unlock land in an async writeFile callback after the rejection; poll briefly.
+		let stamped = false;
+		for (let i = 0; i < 50 && !stamped; i++) {
+			await delay(10);


Please use the already-imported waitFor helper here. This hand-rolled loop duplicates it and caps the asynchronous stamp at 500 ms, making the test unnecessarily sensitive to loaded CI runners. The condition can read the header and return the existing combined predicate, with a descriptive message option.

— KrAIs (Codex)

cb1kenobi · 2026-08-18T17:58:31Z

+		const shortSource = Readable.from(original.subarray(8, 108)); // 100 bytes << descriptor size
+		const saving = repairBlobFile(blob, shortSource);
+		assert.ok(saving, 'repair should start on a damaged blob');
+		await assert.rejects(saving, /size mismatch/);


High: this test fails, and the suite is still blind to every fix in the PR

Ran at this head in a clean worktree (npx tsc -p tsconfig.build.json, then npx mocha unitTests/resources/blob.test.js): 81 passing, 1 failing — this test, AssertionError: Missing expected rejection. The repair does not reject; it logs Repaired blob file in place.

Cause: expectedSize is undefined. savedBlob() returns record.blob, and a blob decoded from a record only carries size if the descriptor had one. Instrumented at the settle handler:

DBG-SETTLE { expectedSize: undefined, actualSize: 100, fileId: '1' }

See the companion comment on resources/blob.ts:1657 for why the descriptor has no size. The size-mismatch branch is therefore never taken, so this test cannot reach stampPendingBestEffort at all — it does not pin the stamp path.

Mutation matrix (each mutation applied to the .ts, rebuilt with tsc -p tsconfig.build.json, and grep-confirmed in dist/resources/blob.js before running — mutating the .ts alone is a no-op because #src/* maps to ./dist/*.js):

Mutation Result

none (head) 81 passing, 1 failing

resources/blob.ts reverted to 612de4abf (entire PR removed) 81 passing, 1 failing — survives

re-validation disabled (if (false && !stillOurDamage)) 81 passing, 1 failing — survives

ERROR-stub store.unlock moved back before writeFile 81 passing, 1 failing — survives

store.unlock(lockKey) deleted from the setup catch 81 passing, 1 failing — survives

Identical in every cell, including with the whole change gone. The +29 test lines detect none of the four fixes.

Suggested fix: give the test a blob whose descriptor actually carries a size (e.g. construct the blob with an explicit size, or assert on the reject path by failing the write rather than the size check) so the mismatch branch is genuinely exercised — then re-run the revert-to-612de4abf mutation and confirm it goes red.

—
Generated by Barber AI

cb1kenobi · 2026-08-18T17:58:31Z

+						const header = Buffer.allocUnsafe(HEADER_SIZE);
+						const read = size >= HEADER_SIZE ? readSync(fd, header, 0, HEADER_SIZE, 0) : 0;
+						if (read < HEADER_SIZE || blobHeaderIndicatesIncomplete(header, size)) stillOurDamage = true;
+						else if (expectedSize !== undefined && header.readUIntBE(2, 6) !== expectedSize) stillOurDamage = true;


Medium: the expectedSize half of the re-validation (and the settle-time size guard it mirrors) is inert for record-decoded blobs

The lock-then-re-read ordering is right and closes the race from the previous round. But this branch — and the settle-time expectedSize !== undefined && actualSize !== expectedSize check at ~1707 that is the only producer of the size-mismatch stamp — depends on the blob carrying a descriptor size, and the blobs repairBlobFile actually receives usually do not.

pack() (2359) records options.size from blob.size before it calls saveBlob(blob) on the same line-of-execution (2371), so for a streaming save (createBlob(Readable)) the size is not known yet and never reaches the descriptor. unpack() (2318, Object.assign(blob, blobInfo[0])) then yields blob.size === undefined. Observed directly — the PR's own new test hits exactly this and the settle handler sees { expectedSize: undefined, actualSize: 100 }, so a repair that streamed 100 bytes over a 25000-byte blob was finalized as healthy and logged Repaired blob file in place.

Consequences on the real path: (1) an identity-tie repair that streams the wrong content is accepted, which is the failure mode the size verification exists to catch; (2) stampPendingBestEffort is only ever reachable from the rejection handler; (3) here, a concurrent healthy write can only be recognised through blobHeaderIndicatesIncomplete, never through the size cross-check.

Suggested fix: source the expected size from something that is always present on a record-decoded blob — the on-disk header captured before the repair write starts, or the caller's descriptor — rather than blob.size, and treat expectedSize === undefined as "cannot verify" explicitly (log it) instead of silently passing.

Separately, when compress is set the finalized header stores the compressed length (compressedStream.bytesWritten, 1546), so a concurrent correct compressed write would compare unequal to an uncompressed expectedSize and be stamped PENDING — worth guarding once the size source is fixed.

—
Generated by Barber AI

cb1kenobi · 2026-08-18T17:58:31Z

+				writeStream.write(createHeader(blob.size)); // write the default header
+				wroteSize = true;
+			}
+			if (idleTimeoutMs > 0) {


Low: prettier --check fails on this file — the block was moved into the new try without re-indenting

npx prettier --check resources/blob.ts reports the file as unformatted at this head; --write produces exactly one hunk, re-indenting lines 1425-1440 (this if body) by one tab. It is the only formatting delta in the change, so npm run format:check will fail.

Suggested fix: run npm run format:write on resources/blob.ts.

—
Generated by Barber AI

cb1kenobi · 2026-08-18T17:58:31Z

+				pipeline(stream, writeStream, finished);
+			}
+		} catch (setupError) {
+			store.unlock(lockKey);


Low: the setup-throw path releases the lock but still leaks the file descriptor and the idle timer

Releasing the lock here is the right fix for the wedge described above. But the stream is created with autoClose: false, and finished() — the only place the fd is closed — is never wired when setup throws after createWriteStream. If the throw comes from createDeflate() or the pipeline() wiring, and especially when idleTimeoutMs > 0 has already registered armIdleTimer on 'data'/'resume' and armed a timer, this path leaves an open fd plus a live timer that can later stream.destroy() a stream nobody owns.

Suggested fix: in the catch, before rethrowing, also tear down what setup created — clear idleTimer, remove the two listeners, and close/destroy writeStream if it was assigned.

—
Generated by Barber AI

cross-model coverage reported — released to human review

harper-joseph · 2026-08-18T21:45:17Z

Closing — superseded, and you're right that it's no longer relevant. The fail-atomic redesign on #2177 (c3d9c7b8: temp file → verify → fdatasync → rename, target untouched on failure) removes stampPendingBestEffort entirely, which was the subject of three of this PR's four changes. A failed repair can no longer leave a bad header, so there is nothing to re-stamp and nothing to race. Strictly better than guarding the stamp.

One item here is independent of the redesign and still present

writeBlobWithStream's abort path, idleTimeoutMs === 0 branch (~:1484 on the current head):

} else {
    store.unlock(lockKey);
    try {
        if (statSync(filePath).size === 0) {
            writeFile(filePath, Buffer.concat([createHeader(...0xff...), errorBuffer]), ...);

It unlocks and then writes the ERROR stub unguarded, while the PENDING branch immediately above correctly holds the lock through its writeFile callback. Once the lock is free another writer for the same fileId can start, and the two unordered writeFiles can land in either order — if the ERROR stub lands last, reads flip from 503 (retry) to 500 (confidently incomplete), which is the direction #481 exists to prevent.

Fully pre-existing, and only reachable with the HARPER_BLOB_STREAM_IDLE_TIMEOUT_MS=0 kill switch, so it's low priority — but it's a two-line change (move that unlock into the writeFile callback, matching the sibling branch) and I didn't want it to disappear with this PR.

Not re-filing it against main — it sits in code you're actively editing on this branch, so it's yours to take or leave. Branch joseph/2177-fixes stays pushed if the diff helps; ignore the rest of it.

For the record, the redesign also answers the coverage gap I flagged: a fail-atomic path with the target untouched on failure needs no test for a failure-path side effect that no longer exists.

github-actions Bot requested review from Ethan-Arrowood, cb1kenobi, heskew and kriszyp August 18, 2026 15:51

gemini-code-assist Bot reviewed Aug 18, 2026

View reviewed changes

cb1kenobi reviewed Aug 18, 2026

View reviewed changes

kriszyp previously requested changes Aug 18, 2026

View reviewed changes

cb1kenobi reviewed Aug 18, 2026

View reviewed changes

harper-joseph closed this Aug 18, 2026

harper-joseph mentioned this pull request Aug 18, 2026

Repair dangling blob references when a base copy re-delivers the record; add replication.blobGapReconnectMs #2177

Merged

Mutation	Result
none (head)	81 passing, 1 failing
`resources/blob.ts` reverted to `612de4abf` (entire PR removed)	81 passing, 1 failing — survives
re-validation disabled (`if (false && !stillOurDamage)`)	81 passing, 1 failing — survives
ERROR-stub `store.unlock` moved back before `writeFile`	81 passing, 1 failing — survives
`store.unlock(lockKey)` deleted from the setup `catch`	81 passing, 1 failing — survives

Uh oh!

Conversation

harper-joseph commented Aug 18, 2026

The race

The fix

Verification, honestly scoped

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Findings

Diff tour

Verification

Review coverage

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harper-joseph commented Aug 18, 2026

One item here is independent of the redesign and still present

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants