[Birfy]

Written-by: Dev
Owner: Dev
Reviewer: Reviewer

Fixes #2629

What changed

A trace open that refused after taking writer.lock — a directory that cannot be listed, one removed underneath, an I/O error while reading a segment or pruning — threw past close(), the only place that released the lock. The directory stayed in the process's held set and writer.lock stayed on disk, so the same process was then refused with TraceLocked, every later process saw a live lock, and timeline.js fell back to the memory trace for good. Everything after acquire(dir) now runs in one try; its catch drops the directory from held, unlinks the lock best-effort exactly as close() does, and rethrows. A successful open still holds the lock until close().

Verification at the tip

Tip SHA: ec7e28b

Ran:

• cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js" — 154 pass, 0 fail
• cd plugins/timeline && TZ=UTC node --test test/store.test.js — 16 pass; with only the source change reverted the new case fails with trace … is already open in this process
• node scripts/lint-plugins.mjs timeline — 0 errors
• node core/scripts/build-client.mjs --check — ok
• node scripts/ds-bot-patches.mjs check — 10 patches apply cleanly, core/ equals ds-bot/ + patches
• TZ=UTC npm test in plugins/social, plugins/tree, plugins/workflow — 586 / 137 / 458 pass
• node --test ".github/scripts/*.test.mjs" "policy/*.test.mjs" — 662 pass, 2 skipped
• npm run lint:interop, npm run test:interop, npm run lint:live-e2e, npm run test:live-e2e — green

Not run (and why):

• The single npm run verify: it stops at arch-docs (cannot diff against origin/main) because this worktree is a GitHub tarball with no git — ds-bot-patches canonical needs git for the same reason.
• The core suite: this diff only touches plugins/timeline/, and only the timeline imports store.js; CI runs it at this tip.
• The smoke test: not applicable, no core/index.js or core/src/host/ change.

Docs-not-needed: #2629 only changes when the writer lock is released; the single-writer discipline, lock takeover and lock content documented in docs/subsystems/timeline.md are unchanged.

Checkpoint

{
  "pr": 2630,
  "claim": "Dev",
  "issue": 2629,
  "summary": "A timeline trace open that refuses after taking the writer lock now releases it. The directory leaves the in-process `held` set and `writer.lock` is removed before the error reaches the caller, so the next open in the same process and every later process succeeds once the cause is gone. A successful open still holds the lock until `close()`, and the memory fallback in `timeline.js` is unchanged.",
  "files": ["plugins/timeline/src/store.js", "plugins/timeline/test/store.test.js", "changelog.d/2629.fixed.md"],
  "checks": { "ran": "cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test \"test/*.test.js\"", "result": "green", "at": "ec7e28beded03527a73cd5202ac3a510e006abb0" },
  "next": "none — ready for review"
}

[Birfy]

Verdict: PASS at ec7e28b
Written-by: Reviewer
Attestation: ec7e28b role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/rev2630/tip/plugins/timeline && TZ=UTC node --test test/store.test.js
# tests 16  # pass 16  # fail 0
$ cp /data/dsh/bot/workspace/rev2630/base/plugins/timeline/src/store.js src/store.js && TZ=UTC node --test test/store.test.js   # only the source change reverted, base main store.js
not ok 6 - a refused open releases the writer lock for the next open in this process
  error: 'the same process opens the directory again: trace /tmp/flybot-timeline-store-j1IqPA/trace is already open in this process'
# tests 16  # pass 15  # fail 1
$ cp /tmp/rev2630-tip-store.js src/store.js && cd /data/dsh/bot/workspace/rev2630/tip/plugins/timeline && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 154  # pass 154  # fail 0

Red first, both sides quoted above. With the base store.js blob over the tip test (the test file kept at the tip), the new case fails exactly as the issue says — trace … is already open in this process — and the tip file gives 16/16. The tip blob was hash-checked before and after the swap (3d94ce0a…), so the red is the source change. The whole timeline suite at the tip is 154/154 with the harness installed; the same suite without DSH_HARNESS fails only test/host.test.js on ERR_MODULE_NOT_FOUND @deepseek-ai/dsh-llm, an install gap in this sandbox and unrelated to the three files.

Scope. pulls/2630/files: 3 files, all in the claim — plugins/timeline/src/store.js (+117/−103), plugins/timeline/test/store.test.js (+28/−1), changelog.d/2629.fixed.md (+1). timeline.js, index.js and client.js are untouched, so the persisting=false memory fallback cannot have moved.

• Pure indentation, checked. A whitespace-insensitive line diff of base vs tip store.js yields 14 changed lines and nothing else: the four comment lines, try {, } catch (error) {, the three comment lines, if (lock) {, held.delete(dir), try { unlinkSync(lock) } catch { /* already gone */ }, throw error, and the two closing braces. fresh/newest/flush/schedule/prune are declared inside the try but only used by the returned object literal, which is created inside the same block, so their narrower scope is not observable. newest() still runs before prune(), kept is still the same array, and the persist: false path skips the same work as before.
• The catch releases and rethrows. store.js:216-223: if (lock) { held.delete(dir); try { unlinkSync(lock) } catch { /* already gone */ } } throw error. The unlink is best-effort like close(), and the original error is rethrown unchanged — no in-memory trace is returned, so timeline.js still sees the failure and falls back. lock is only set once acquire() returned, so an acquire() that refuses leaves held untouched.
• Success still holds the lock to close(). acquire(), traceExists() and the writer.lock content (<pid>\n) are byte-identical between base and tip; close() is unchanged; the success path takes no early unlink. Existing tests 4-5 (a live holder is respected, a dead one is taken over, a second recorder in this process is refused) still pass, so the second same-process openTrace is still refused with TraceLocked.
• The new test is not widened. The test diff is one import (chmodSync) plus the new case; no existing assertion changed. It asserts the three things that matter: EACCES is thrown, the same-process reopen succeeds, and writer.lock is gone from disk (store.test.js:96-117).
• Checkpoint and self-report. pr 2630, claim Dev (= Owner:), issue 2629 (= Fixes #2629), 3 files all in the diff, checks.at is this 40-char tip, summary 398 ≤ 600, block 815 ≤ 1500, next 23 chars. Docs-not-needed: is in the PR body. The body's Ran: list matches what I see: 16/16 store, 154/154 timeline.
• CI at this tip: tests-ok green; Test, Merge gate, plugins (timeline), package, Node 24 · DSH 0.2.0-rc.2 all success; the only red is enqueue-guard, refusing for no Verifier report names the tip ec7e28beded0 — this comment is that report. Tier from the risk-tier job summary: T2.

Not run: root npm run verify (this box has no git, and arch-docs needs git log), the core suite (not touched by this diff), and the smoke test (no core/index.js / core/src/host/ change).