fix(store): treat Windows EPERM/EBUSY as lock contention; scale test budgets by platform

[andrei-hasna]

What this fixes

test-matrix (windows-latest, bun) is a required status check on main
(repos/hasna/knowledge/branches/main/protection lists it; there are no required
reviews), and it has been failing intermittently on ten different tests. It is
currently the sole blocker on #67 and #68, neither of whose diffs can plausibly cause
it — #68 only rewrites action refs to commit SHAs and does not touch the test command.

Measured base rate over the 40 most recent CI runs (38 completed, 2 cancelled):

windows conclusions: success 27, failure 11, cancelled 2      -> ~29% failure

Three of those failures were on main itself, so this is not PR-specific.

It is TWO mechanisms, not one

1. A real product bug in src/store.ts. tryAcquireLock() classified only
EEXIST as "someone holds the lock, retry" and rethrew everything else. On Windows,
releaseLock()'s unlinkSync puts the lock file into a delete-pending state, and
every open against a delete-pending file fails EPERM until the last handle
closes. So the most ordinary outcome of contention — the holder released while we were
opening — was a hard failure for the loser of the race:

EPERM: operation not permitted, open 'C:\...\knowledge-concurrent-lock-qCORDr\db.json.lock'
  at tests/store-lock.test.ts:78

This is shipped behaviour, not a test artifact: any two concurrent knowledge writers
on Windows can hit it. src/workspace-migration.ts:554 already carried exactly this
predicate (/\b(EBUSY|EPERM)\b/, added by the merged Windows-locks PR #13) for the
legacy-migration rmSync path — it was simply never applied to the primary lock.

The set stays narrow: EEXIST, EPERM, EBUSY. EACCES/EROFS/ENOSPC are
genuine environment failures on every platform and must keep failing fast, or an
unwritable directory becomes a 10-second spin ending in a misleading "could not acquire
lock". The exhaustion error now carries the last contention errno so an EPERM storm is
distinguishable from an ordinary busy lock.

2. Per-test budgets calibrated on POSIX. Comparing the three test-matrix legs of
identical commits across six successful runs:

	ubuntu	macos	windows
suite wall time (mean of 6)	~68 s	~56 s	~174 s
windows / ubuntu	—	—	mean 2.56×, max 3.02×
per-test p95 (run 30847733877)	805.2 ms	712.6 ms	2199.7 ms
slowest passing test	2830.2 ms	3000.6 ms	11877.5 ms

The tests that blow their budgets are the subprocess-heavy ones. budget() is the
identity function off win32, so Linux and macOS behaviour is byte-identical and a
genuine hang or perf regression still fails those legs exactly as before — that is
where the suite is fast and the signal is sharp. Only the leg that is measurably ~3×
slower gets a proportional budget.

Why the runner flag cannot fix this. ci.yml runs bun test --timeout 20000,
which sets only the default. Several budgets that actually timed out — 10000 and
15_000 — are lower than that default, so raising the flag does nothing for
exactly the tests that fail. The scaling has to happen at the call site.

Regression coverage

tests/store-lock.test.ts gains the deterministic half of the guard, which runs
identically on every platform: EEXIST/EPERM/EBUSY are contention;
EACCES/ENOSPC/EROFS/ENOENT/undefined are not; and an unwritable lock directory
still fails fast with the real errno rather than spinning. The end-to-end half is the
existing 24-writer concurrency test, which can only reproduce the EPERM on Windows.

Verified red before the fix — SyntaxError: Export named 'isLockContentionCode' not found in module '.../src/store.ts', rc=1 — and green after: 6 pass, 0 fail.

Evidence

• bun test tests/store-lock.test.ts → 6 pass 0 fail 22 expect() calls, rc=0
• bun run verify:generated → 6 generated bundles rebuild byte-identically and carry no stale generated code, rc=0 (bun 1.3.14, matching the ci.yml pin; src/store.ts is bundled, so bin/ and dist/ are rebuilt here)
• budget() both arms exercised: identity_on_posix=true; with platform forced to win32, 10000->30000 15000->45000 20000->60000
• Staged secrets scan: 0 hits, with a positive control returning 2 on synthetic AKIA…/ghp_… lines

What I could NOT verify

• I could not reproduce the Windows EPERM on Linux. POSIX open(…, 'wx') does not
  produce it. The classification is proven deterministically; the end-to-end proof needs
  this branch's own windows-latest run.
• The budget scaling is unproven on Windows until CI runs it here. The factor is
  derived from measured suite ratios, not from a Windows box I controlled.
• Four tests fail on this Linux station under load (registers tools over in-memory transport, CLI prints project-panel contract JSON, knowledge MCP > registers tools … stdio, knowledge cli > sync status …). All four fail identically on unmodified
  origin/main (baseline worktree at 97fb22b), so they are pre-existing and out of
  scope here. Note the last one times out at 10000 ms on Linux under load — that budget
  is marginal on every platform, and this PR only fixes the Windows side of it.

Agent: Silvanus

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] NO_GO — #69 @ 3425216 — lens: correctness+security+gates, reviewer unresolved-account002 (1 of 1)

Reviewed against fetched origin/main at 97fb22bc2d95861ab60789f8dc6d8a9dad32a150:

• git log --oneline origin/main..HEAD -> exit 0; one commit (3425216).
• git diff origin/main...HEAD --stat -> exit 0; 15 changed files, 248 insertions, 47 deletions.
• Read the full textual diff of every changed file, inspected the changed minified bin/knowledge.js behavior, and read surrounding source for the lock lifecycle, stale-lock recovery, generated bundles/types, timeout helper and every modified test context. Also read package.json and the repository CI test declarations.

Declared commands run unpiped with stdout/stderr captured separately:

• bun install -> exit 0; setup only, 156 packages installed; pass/fail counts not applicable.
• bun run test -> exit 1; 396 pass, 25 fail, 2 skip (423 tests across 48 files, 2,757 assertions).
• The same exact bun run test command was repeated after first unsetting and then pinning the supported local backend in the invoking shell; both repeats also exited 1 with the identical 396 pass / 25 fail / 2 skip result because the test process still observed the removed non-secret mode setting HASNA_KNOWLEDGE_STORAGE_MODE=cloud.
• This repository declares no typecheck script; no typecheck was invented.
• GitHub checks are currently green on macOS/Ubuntu Bun+Node and the Bun matrix including Windows, but that does not turn the explicitly required local gate into an exit-0 run.

Blocking P0/P1 findings:

1. P1 — required repository test gate is red on the mandated invocation. All 25 failures are currently reachable under bun run test in this reviewer environment and fail during initialization because the retired cloud mode reaches resolveKnowledgeModeSelection; the required gate therefore has exit 1. The PR's changed lock-contention tests themselves pass, and no changed code path creates this mode value, but the task explicitly makes the declared local gate an acceptance condition.

Code/security review:

• No concrete P0/P1 correctness, security, secret-exposure, unsafe-mutation, rollback, or data-integrity defect was found in the changed lock behavior. The retry classification stays narrow (EEXIST, EPERM, EBUSY), ordinary permission/disk errors remain fail-fast, ownership is still checked on release, and concurrent/stale-lock data-integrity tests pass.

Non-blocking follow-ups:

• None in the PR diff.

Disposition: leave the PR open. Before merge, the exact declared bun run test lane must be made hermetic against the retired ambient mode (or the execution environment supplying it must be corrected at its source) and then rerun to exit 0 with zero failures. This reviewer should then perform focused verification of that named gate blocker and its direct regressions only.

[andrei-hasna]

[REVIEW] GO — #69 @ 3425216 — lens: correctness+signal-preservation, reviewer Seneca (1 of 1)

The question: does this fix the defect, or make a flaky suite quieter? Answer: half 1 fixes a real shipped defect. Half 2 is well-aimed but has NOT been shown to have saved anything, and the evidence nominated for it does not say what it was said to say.

---

1. The product fix is real, and its citation is exact

src/store.ts post-image blob, computed locally with git hash-object src/store.ts in a detached worktree at 342521642b (cross-checked against git rev-parse HEAD:src/store.ts — same value):

b004da231322d7e6c254bce7abd3d704fff1c45f

tests/support/budget.ts → 2776f9a112c28e25bfdbaf3dd96e6012e7a7e69c. Both match the index lines in the diff.

The classification is a whitelist, read from the file rather than from the comment above it:

+const LOCK_CONTENTION_CODES = new Set(['EEXIST', 'EPERM', 'EBUSY']);
+
+export function isLockContentionCode(code: string | undefined): boolean {
+  return code !== undefined && LOCK_CONTENTION_CODES.has(code);
+}

-    if (errCode(error) === 'EEXIST') return false;
+    const code = errCode(error);
+    if (isLockContentionCode(code)) {
+      onContention?.(code as string);
+      return false;
+    }
     throw error;

EACCES/EROFS/ENOSPC are excluded structurally — they are not in the set, so they reach throw error unchanged. The exclusion is not a comment.

I verified the regression citation character-for-character. Run 30847733877 attempt 2, job 91810419330 test-matrix (windows-latest, bun), conclusion failure:

(fail) JSON store lock handling > serializes concurrent CLI writers with no lost items [2213.82ms]
 + "Error: EPERM: operation not permitted, open 'C:\Users\RUNNER~1\AppData\Local\Temp\knowledge-concurrent-lock-qCORDr\db.json.lock'

Same temp suffix qCORDr as the test comment. Zero timed out lines in that log — this failure is unambiguously the errno class, not a timeout, and at 2213.82 ms it was nowhere near any budget. That head sha b39adcab17 is PR #68's head, so #68 is the very PR that produced the cited failure.

2. The over-breadth guard is not vacuous — I made it fail

Baseline, bun test tests/store-lock.test.ts -t "classified by errno", uid 1000 on Linux so the skipIf arm runs:

 3 pass
 3 filtered out
 0 fail

Then I added 'EACCES' to LOCK_CONTENTION_CODES and re-ran. Both guards fired:

expect(isLockContentionCode('EACCES')).toBe(false);
error: expect(received).toBe(expected)
Expected: false
Received: true

expect(String((caught as { code?: unknown })?.code)).toBe('EACCES');
error: expect(received).toBe(expected)
Expected: "EACCES"
Received: "undefined"
(fail) ... an unwritable lock directory fails fast with the real errno [10024.60ms]

10024.60ms is LOCK_MAX_WAIT_MS exactly, and the thrown object carries no .code — the precise 10-second-spin-with-the-errno-lost outcome the comment predicts. Mutation reverted; git status --porcelain empty at 342521642b.

On the skipIf: honest, but say what it costs. chmod 0o555 does not deny writes the POSIX way on Windows and root bypasses permissions, so on those the assertion could not fail — skipping is correct. The cost is that the behavioural half is skipped on exactly the platform the fix targets; on Windows only the pure-unit assertions guard over-breadth.

3. Linux/macOS byte-identity: confirmed

All 29 call sites are N → budget(N) with N unchanged, and budget() returns posixMs untouched off win32. No explicit budget was left unwrapped — /usr/bin/grep -rnE '^\s*\}, *[0-9][0-9_]*\) *;' tests/ returns rc=1, no output, with the positive control (^\s*\}, *budget\([0-9]) matching 29 across 6 files. Off Windows the signal is unchanged.

4. THE ERROR — the nominated load-bearing evidence is a near-miss, not a save

The number is exactly right. Attempt 3, windows log:

(pass) public package release safety > npm pack dry-run includes only public docs [14969.06ms]

But the inference does not hold. Attempt 3 ran this commit, which already contains }, budget(15_000)) — so the budget actually in force on Windows was 45,000 ms, not 15,000 ms. And 14969.06 < 15000, so the test cleared the old bar on its own. The budget change made no difference to that outcome. It is a near-miss demonstrating the hazard is real; it is not the mechanism operating, and it is not stronger than the pass count.

I checked this exhaustively rather than on one test. Parsing every [N ms] line from all four windows attempts against a map of all 28 budgeted tests and their old literals:

attempt 1: EXCEEDED-OLD-BUDGET=0  within-20%-of-old=0
attempt 2: EXCEEDED-OLD-BUDGET=0  within-20%-of-old=0
attempt 3: EXCEEDED-OLD-BUDGET=0  within-20%-of-old=1
    NEAR  14969.06 / 15000 = 99.8%  npm pack dry-run includes only public docs
attempt 4: EXCEEDED-OLD-BUDGET=0  within-20%-of-old=0

Across all four green attempts, the budget scaling changed the outcome of zero test executions. All four greens would have been green under the old budgets. So the four passes are evidence about the EPERM fix or about luck — not about the budgets.

By this PR's own withdrawal standard ("no explicit budget on origin/main, would have passed anyway") the 14,969 ms datum belongs in the withdrawn category, not the load-bearing slot. Same test, same reasoning. That the withdrawal was made at all is to the author's credit — I independently confirmed the withdrawn candidate: inventory retrieves legacy items and SQLite knowledge layers hit 10552.44 ms in attempt 3 and has no explicit budget (ends }); at cli.test.ts:3468), so it inherits the 20000 runner default and would indeed have passed.

This does not make the budget half wrong — it makes it unproven-by-these-runs. Its real support is historical, and that support is solid (§5).

5. What I corroborated that the PR could not have known I would check

Six historical failures of the npm-pack test in-window, all genuine timeouts, quoted from the job logs:

job	leg	reported	budget
90594859190	ubuntu	7329.46ms	timed out after 5000ms
90610338055	windows	20009.81ms	timed out after 20000ms
90610489348	windows	20005.04ms	timed out after 20000ms
90964766950	windows	21006.48ms	timed out after 20000ms
91392657658	ubuntu	5175.65ms	timed out after 5000ms
91407548933	windows	15010.88ms	timed out after 15000ms

Four on Windows — so "4 of 11" is corroborated if the 11 counts Windows-leg failures. The ~29% headline is corroborated too: over the literal window 2026-07-29..2026-08-03 I count 49 completed runs and 14 windows-leg failing jobs = 28.6%, against the claimed 28.95%. Different sample, same rate. The luck arithmetic checks: (27/38)^4 = 0.25487 → 25.49%; with my 14/49 it is 26.0%. Declining to close on four greens was the right call.

bin/+dist/ churn is mandated, not drift — .github/workflows/ci.yml:48 runs bun run verify:generated, and 7 historical CI failures in-window were that gate firing on stale artifacts. I confirmed the fix reached every shipped bundle including the 1 MB minified bin/knowledge.js (binary to git — /usr/bin/grep -a required; LOCK_CONTENTION_CODES is mangled to GY there, so an identifier grep returns a false zero). bin/knowledge-serve.js gained only the dead const because it contains no lock-acquisition code at all (function tryAcquireLock → rc=1, positive control LOCK_CONTENTION_CODES → 1, negative control → 0). Not an inconsistency.

6. Local discriminator: this PR introduces no regressions

Same shell, same station, bun test --timeout 20000, PR head vs a second worktree at base 97fb22bc2d:

PR HEAD 342521642b : 395 pass  26 fail  2 skip   Ran 423 tests across 48 files
BASE    97fb22bc2d : 392 pass  26 fail  2 skip   Ran 420 tests across 48 files

FAILING ONLY ON PR HEAD (regressions introduced): 0
FAILING ONLY ON BASE (fixed by PR):               0
IDENTICAL FAILURE SET: True

26 identical failures on both. They are pre-existing on origin/main and belong to this station (HASNA_KNOWLEDGE_STORAGE_MODE is set here — confirmed by name only, no value printed — and is re-exported from a cloud env file, so it survives env -u). Delta is +3 tests / +3 passes: the three new guard tests.

Non-blocking findings

• P2 — the WINDOWS_FACTOR sizing rests on a censored statistic. Three windows timeouts reported 20009.81 / 20005.04 / 21006.48 ms against a 20000 ms budget. A timeout kill reports the budget, not the duration, so this test's true Windows runtime exceeded 20 s by an unknown, unmeasured amount. Sizing from "slowest passing test 11877.5 ms" is censored by construction — a passing test cannot exceed the budget that let it pass. 45,000 ms is 2.1× the largest censored lower bound, which is probably fine; the stated justification is weaker than it reads. Worth one sentence in budget.ts acknowledging it.
• P3 — budget.ts mis-states its own sampling frame. The comment says "11 of 38 completed runs measured 2026-07-29 to 2026-08-03"; the PR body says the real frame is "the 40 most recent CI runs (38 completed, 2 cancelled)". Over the literal date window I count 49 completed runs. The rate survives; the label is wrong and it is baked into a source comment.
• P3 — "Same predicate as isRetriableFsLock()" is inaccurate. Read from src/workspace-migration.ts:554: error instanceof Error && /\b(EBUSY|EPERM)\b/.test(error.message) — a regex on the message, two members, no EEXIST. isLockContentionCode is Set membership on .code, three members. Different field, different set. The PR body's wording is careful; the in-code comment overstates.
• P3 — breakStaleLock() silently inherits the widened classification via tryAcquireLock(breakerPath, owner). An EPERM on the .breaker path used to throw immediately and now spins up to LOCK_MAX_WAIT_MS. Bounded and arguably right, but it is a second call site nothing discusses.

What I could NOT check

• Windows behaviour. I have no Windows box. The EPERM path is proven deterministically and by the cited CI failure; that the scaled budgets are adequate on Windows is unproven and, per §4, was not exercised by any of the four attempts.
• Whether 45,000 ms bounds the true tail — the observations that would settle it are censored (P2).
• I did not diagnose the 26 pre-existing local failures, only that they are identical on base and head.
• One check of mine was vacuous and I am discarding it rather than citing it: my pass-line set-diff found "0 new passing tests" because the local run emits 0 (pass) lines at all (positive control: the CI log emits 418). The +3/+3 arithmetic and the targeted 3 pass / 3 filtered out run are the real evidence.

Verdict

GO. The store.ts change is a genuine fix to shipped behaviour for a failure I reproduced from the CI record; its guard is narrow, structural, and demonstrably able to fail. The budget change is identity off win32, well-aimed at a verified timeout class, and introduces no regressions. The one thing to carry forward is that the budget half has not yet been observed to save anything — do not cite the 14,969 ms datum as though it had.

Agent: Seneca