Fix/1149 timeline prune active session

[MayurK-cmd]

Description

Closes #1149

Adds a per-process lockfile at .nanocoder/timeline/<sessionId>/.lock so pruneStaleSessions no longer wipes a session directory that another in-flight process is still writing into.

A long-running session mid-tool-call used to be vulnerable: its session directory's mtimeMs lagged behind now - MAX_TIMELINE_SESSION_AGE_MS whenever no new entry was being captured, and pruneStaleSessions could fs.rm the entire directory, breaking the active tool call.

Pruning is now gated on a liveness probe of the lockfile:

• Live PIDs (lock present, holder process is alive, purpose tag matches) → skip the prune.
• Dead PIDs (process gone) → reap the lock, then remove the directory.
• Malformed or wrong-purpose locks → reap the lock, then remove the directory.
• No lockfile at all → treat as abandoned, remove the directory (preserves the existing "abandoned sessions" behaviour).

The session directory's mtime is also refreshed on every ensureDir via fs.utimes so an active session naturally bubbles to the top of the count cap and out of the age-based cutoff.

How it works

New module: source/services/timeline-lock.ts

Self-contained, mirrors daemon/lockfile.ts:

• acquireTimelineLock(sessionDir, {pid, startedAt}) — creates a temporary lockfile using exclusive creation (O_EXCL), then atomically renames it into place. Returns false (does not throw) on contention so the chat never blocks.
• releaseTimelineLock(sessionDir) — idempotent unlink; ENOENT is ignored.
• isTimelineLockLive(sessionDir) — reads the lock, validates the purpose tag, probes the holder's PID with process.kill(pid, 0), and reaps stale locks as a side effect.
• isProcessAlive(pid) is duplicated from daemon/lockfile.ts for now; a follow-up can lift it into a shared util.

The lock payload is {pid, startedAt, purpose: 'session-active'}. The purpose tag allows isTimelineLockLive to distinguish a real timeline lock from a random JSON file another tool may have dropped in the session directory.

TimelineManager changes

• Acquires the lock on the first ensureDir call (best-effort: failure is logged, never thrown).
• Idempotent: a second ensureDir does not flip lockHeld back to false by racing its own on-disk lockfile.
• Refreshes the session directory's mtime on every ensureDir via fs.utimes so an active session bubbles to the top of the count cap and out of the age-based cutoff.
• Exposes async dispose() that releases the lock and clears the lockHeld flag. Idempotent and safe to call on managers that never acquired.

pruneStaleSessions changes

Probes the lockfile for every stale entry. If isTimelineLockLive reports the lock as held by a live process, the entry is skipped. A dead, malformed, or wrong-purpose lock is reaped before fs.rm runs, so the directory leaves the timeline root in a clean state.

Why no lifecycle wiring?

The lock remains useful without an explicit dispose() call:

• A process exiting does not automatically remove the lockfile from the filesystem.
• isTimelineLockLive() detects lockfiles whose recorded PID is no longer alive and reaps them during pruning.
• This means an abandoned lockfile cannot permanently prevent cleanup.
• The regression in [Bug] timeline-manager.ts mtime-based prune can delete a session that's mid-tool-call #1149 is fixed as long as active sessions hold the lock for the duration of the chat.
• dispose() provides prompt cleanup when lifecycle wiring is available, but it is not required for correctness.

A follow-up can plumb dispose() into the App / AcpSession teardown so production code releases the lock promptly. This is out of scope for this fix.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

.changeset/fix-1149-timeline-prune-lock.md is a patch changeset naming @nanocollective/nanocoder and explaining the lockfile and live-skip behaviour.

Validated locally with node scripts/validate-changesets.js (73 changesets checked, all resolve).

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

• Regression coverage added for the stale-session pruning bug
• Tests cover both success and error scenarios
• All existing tests pass (pnpm test:all completes successfully)
• Type checks pass
• Lint passes

Verified locally:

• pnpm test:ava source/services/timeline-lock.spec.ts → 10/10 passed
• pnpm test:ava source/services/timeline-manager.spec.ts → 21/21 passed (17 existing + 4 new)
• pnpm test:types → clean
• pnpm test:lint → 517 files, 0 fixes

New tests cover:

In timeline-lock.spec.ts:

• live lock acquire + live probe
• double-acquire returns false
• idempotent release
• stale-PID reap (dead PID)
• malformed-payload reap
• wrong-purpose-tag reap
• no-file path
• isProcessAlive for current process and invalid PIDs
• strict acquire path on a stale lock

In timeline-manager.spec.ts:

• TimelineManager prunes abandoned sessions whose lockfile points to a dead process — proves the lock reaper and fs.rm cooperate.
• TimelineManager skips pruning a stale-but-live session (issue #1149) — proves the regression this PR fixes.
• TimelineManager.dispose releases the session lock — proves the lifecycle.
• TimelineManager.dispose does not throw when the lock was never acquired — proves the idempotency.

Success and error scenarios are both covered: the live-skip and dead-reap tests exercise the two paths in pruneStaleSessions, while the malformed, wrong-purpose, and missing-lock tests cover the stale outcomes for isTimelineLockLive.

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

This change is filesystem-only and provider-agnostic. The lockfile lives under .nanocoder/timeline/<sessionId>/.lock and is read/written via node:fs/promises with no LLM or network calls.

Manual end-to-end testing against a real provider was not done as part of this PR; the change was validated via the unit/integration tests listed above.

To manually reproduce the original bug and verify the fix, the steps from the issue are: start a long-running session, then in a second shell run a flood of nanocoder invocations that each create a new session so the count cap is hit, and observe that the long-running session's directory is no longer removed.

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md#logging)

No documentation update included in this PR — the lockfile is an internal implementation detail of TimelineManager; the user-visible behaviour (pruning) is unchanged for the normal case. Happy to add a docs page if reviewers want one.

No breaking changes: the public surface of TimelineManager only grew by the new dispose() method. The constructor signature and all existing method signatures remain unchanged, and the file layout under .nanocoder/timeline/<sessionId>/ is unchanged. The tryAcquireSessionLock, touchSessionDir, and lock-related fields remain private.

Logging: the lock acquisition and release paths use the existing logWarning helper from source/utils/message-queue.ts with structured context (sessionId, error), matching the project's logging conventions. No additional getLogger().info calls were added because the happy path is silent, matching the rest of the timeline subsystem; only the failure path logs.

[will-lamerton]

Thanks for this. The #1149 diagnosis is right and the test that reproduces the regression is good. A few things need addressing before merge.

Scope

This PR contains two unrelated features:

• 5e42926 inline ?key=value slash-command overrides (closes #1151)
• 5eb1535 timeline prune lockfile (closes #1149)

The title, description and Closes line only cover #1149. Please split these into two PRs so the dispatcher rewrite in source/app/utils/app-util.ts gets reviewed on its own merits.

Correctness

1. acquireTimelineLock is not atomic, despite the docs saying it is. The docstring and PR body both say it uses exclusive creation (O_EXCL), but the code is existsSync then writeFile(tmp) then rename(tmp, lockPath). There is no O_EXCL, and POSIX rename atomically replaces the destination rather than failing, so two racers both pass the existsSync check and both return true. The if (code === 'EEXIST') return false branch is unreachable. Use open(lockPath, 'wx') if you want the guarantee the comment promises, or fix the comment.

2. PID reuse can pin a session forever. Since dispose() is never wired up, every real session leaves a .lock behind. If the recorded PID is later reused by any unrelated process, that session looks permanently live: it is never pruned and it permanently occupies a slot in MAX_TIMELINE_SESSIONS. startedAt is written to the payload but never read, so the cheap guard is right there.

3. applyOnceOverrides clears rather than restores. Every restoration pushes setAutoCompactThreshold(null) / setAutoCompactEnabled(null) / resetSessionContextLimit(). If a user has already set a session override with /compact --threshold 70, a single /compact ?threshold=80 silently wipes it back to the config default instead of putting 70 back. The prior values are readable via the autoCompactSessionOverrides proxy and getSessionContextLimit(), so capture-then-restore is a few lines. The current specs only assert restore() does not throw, so nothing catches this.

4. The changeset advertises syntax that does not exist. .changeset/inline-once-overrides.md uses /context-max 128k ?once as an example, but there is no once case in applyOnceOverrides and no once key in LEGACY_FLAG_NAMES, so it is parsed and silently dropped. That text ships to the public changelog.

5. expandOverrideArgs mishandles valued booleans. ?preview=yes expands to ['--preview', 'yes'], injecting a stray positional token into the handler's argv. Every entry in LEGACY_FLAG_NAMES is a boolean flag, so string values should go through the existing toBoolean helper.

Smaller

• UTF-8 BOM on four new files: .changeset/inline-once-overrides.md, .changeset/fix-1149-timeline-prune-lock.md, source/utils/inline-overrides.ts, source/services/timeline-lock.spec.ts. No other file in the repo has one, please strip them.
• The .pnpm-store/ .gitignore change is an unrelated drive-by.
• source/app/utils/app-util.spec.ts adds imports at line ~1087, after 1084 lines of tests. Move them to the top.
• In pruneStaleSessions, isTimelineLockLive unlinks the stale lock and the next line fs.rms the whole directory anyway, so the "leaves the timeline root in a clean state" rationale does not apply on that path.
• The parseInlineOverrides(['? bad']) case cannot occur, since args come from a split(/\s+/).

Question on approach

touchSessionDir() on its own fixes the age-based prune, with no lockfile at all. The lock is only load-bearing for the count-cap branch (position >= MAX_TIMELINE_SESSIONS). Could you justify the ~175 lines of locking machinery against that, or scope it down to just the count-cap path?

CI

The failing Unused Dependencies (knip) check is pre-existing on main, not caused by this PR. Nothing to do there.

[will-lamerton]

Thanks for splitting the scope, that's much easier to review. The real O_EXCL and the startedAt age guard both address my earlier points. Verified locally on the current head: 32/32 timeline tests pass, test:types clean, test:format and test:lint clean.

Three things still need fixing.

Blocking

1. knip fails, and this PR causes it. TIMELINE_LOCK_FILENAME (source/services/timeline-lock.ts:23) is exported but never imported. I ran knip on main (exit 0) and on this branch (exit 1, Unused exports (1)). My last review said the knip failure was pre-existing; that was true on the 6th, but main has since been cleaned up, so this is new. Drop the export.

2. The lockfile is readable as empty, and a prune landing in that window deletes the live session. open(lockPath, 'wx') creates a zero-byte file and the payload is written on the next line. A concurrent pruneStaleSessions probing in between reads '', JSON.parse throws, readLockPayload returns null, and isTimelineLockLive treats it as malformed: it unlinks the lock and reports live: false. The pruner then fs.rms the directory of a session that is actively writing, which is #1149.

Reproduced by interleaving a probe between the two steps of acquire:

probe during acquire -> { live: false, payload: null }
lockfile still on disk after acquire completed? false
probe after acquire -> { live: false, payload: null }

Note the second line: the acquiring process still returns true and sets lockHeld = true, but its lock is gone from disk. tryAcquireSessionLock early-returns on lockHeld, so it never re-acquires and that session stays unprotected for the rest of its life, with dispose() silently no-opping through ENOENT.

O_EXCL gives mutual exclusion between writers, but nothing for a reader arriving mid-write, so the module header comment ("writes use O_EXCL so two processes cannot both observe an empty file") claims the one property it does not buy. Write the payload to a unique temp file first, then fs.link(tmp, lockPath) (atomic, fails EEXIST if taken) and unlink the temp. That gets both.

3. MAX_LOCK_AGE_MS reaps live locks after 24h. startedAt is stamped once in the constructor and never rewritten, and the age check runs after the liveness probe has already passed, so a session still running after 24h has its lock deleted despite its PID being alive. timeline-lock.spec.ts encodes this directly: the reaps a lock that is older than MAX_LOCK_AGE_MS test uses pid: process.pid. Combined with the lockHeld early-return, nothing re-acquires, so a long-lived TUI or ACP session loses protection permanently and is then prunable mid-tool-call. touchSessionDir already runs on every ensureDir; re-stamping the lockfile there (or checking the lock's mtime rather than startedAt) closes it.

Smaller

• The UTF-8 BOM is still on .changeset/fix-1149-timeline-prune-lock.md and source/services/timeline-lock.spec.ts. It was only stripped from timeline-lock.ts.
• acquireTimelineLock's catch is if (code === 'EEXIST') return false; then return false;. Beyond the dead branch, it means acquire never throws, so the logWarning('Could not acquire timeline session lock') handler in tryAcquireSessionLock is unreachable and a real EACCES/ENOSPC is indistinguishable from contention and logs nothing.
• readFile is imported and unused in timeline-lock.spec.ts. biome.json excludes **/*.spec.ts, so CI won't catch it.
• The test acquireTimelineLock survives a stale lock (reaps and acquires) asserts t.false(acquired), which is the opposite of its title.
• My earlier question about justifying the lock against touchSessionDir alone wasn't answered, but I'm satisfied either way: the lock is load-bearing for the count-cap branch, which touchSessionDir doesn't cover. No change needed.

CI

Pull Request Automated Checks is sitting in action_required on both recent pushes, so the full suite has never run against the current head. I'll approve the run once the above is in, but knip will fail there too until (1) is fixed.

[MayurK-cmd]

Rebased onto current main (truncateAfter + atomicWriteFile(saveIndex) merged cleanly) and fixed the three blockers:

1. knip clean (exit 0).
2. Empty-read race closed for real: publish is now temp-file + link(tmp, lockPath) in the same directory, so readers never see a zero-byte lock and two racers can't both win. Verified no .lock.tmp-* files leak.
3. 24h reap fixed: expiry now checks lockfile mtime, and tryAcquireSessionLock calls new refreshTimelineLock() on every ensureDir, so a live session past 24h keeps its lock. The spec that encoded the old behavior now backdates mtime instead of startedAt.

Also fixed the small items: no BOMs, acquireTimelineLock re-throws non-EEXIST (so the acquire-failed warning is reachable), unused open import removed. Tests: timeline-lock.spec 13/13 (incl. new refresh + no-temp tests), timeline-manager.spec 23/23 (incl. live-skip, dead-reap, dispose lifecycle).

[will-lamerton]

Thanks, the scope split is clean and all three blockers from my last review are genuinely fixed. Verified on 6cca9aba: knip exit 0, link(tmp, lockPath) closes the empty-read race properly, and mtime-based expiry plus refreshTimelineLock on every ensureDir fixes the 24h reap. 38/38 timeline tests pass locally.

Two new holes reopen #1149 on reachable paths, though. Both reproduced.

Blocking

1. clear() permanently drops the lock. clear() removes the session directory including .lock, but leaves lockHeld = true. The next ensureDir hits the if (this.lockHeld) return early-exit in tryAcquireSessionLock, falls through to refreshTimelineLock, which reads no payload and no-ops. The lock is never re-created, so the session runs unprotected for the rest of the process.

lock after 1st capture: true
  pruner sees live: true
after clear, dir exists: false
after post-clear capture, dir exists: true
  lock exists: false
  pruner sees live: false

Reachable from acp-agent.ts:270, the /clear command, where the session continues afterwards. So in ACP one /clear puts the session straight back into the #1149 state. Setting this.lockHeld = false in clear() is enough.

2. A resumed session never reclaims a dead predecessor's lock. acquireTimelineLock refuses on any present lockfile without probing liveness. timeline-lock.spec.ts:165 documents this as intentional and says "Callers that want self-healing should run isTimelineLockLive first to reap, then call acquire", but tryAcquireSessionLock, the module's only caller, does not do that. So the module states a caller contract nothing satisfies.

Concretely: a process crashes leaving .lock with its PID, the session is resumed under the same id, acquire returns false, lockHeld stays false forever. Nothing reaps the dead lock either, because pruneStaleSessions only probes entries already in stale and touchSessionDir keeps the mtime fresh. Then a long tool call lets the mtime go stale, the pruner probes, sees a dead PID, reaps the lock and removes the directory of a live session.

lock file present: true
lock pid on disk: 2000000000 (this process is 87895)
live session dir survived prune: false

Calling isTimelineLockLive before acquire in tryAcquireSessionLock closes it, and matches the contract already written in that spec comment.

Smaller

• The UTF-8 BOM is still on .changeset/fix-1149-timeline-prune-lock.md and source/services/timeline-lock.spec.ts. Both start ef bb bf per xxd. It was only stripped from timeline-lock.ts. Third time raised, please get both this round.
• The changeset says "TimelineManager.dispose() releases the lock promptly", but dispose() has no production caller anywhere, only specs. That sentence ships to the public changelog and is inaccurate as merged. Either wire dispose() up or drop the clause.
• refreshTimelineLock does const {rename} = await import('node:fs/promises') inline while the module already imports statically from that same module on line 21. Make it a top-level import.
• refreshTimelineLock has no "leaves no temp files behind" test, unlike acquireTimelineLock. The finally looks right, so just a coverage gap.

CI

Only changeset-check and label have run. Pull Request Automated Checks has still never run against this head. I'll approve the run alongside the next push.

[MayurK-cmd]

All review points addressed and pushed (a461809, rebased onto current main — this supersedes the 6cca9ab UI merge, branch is linear again).

Blocking 1 — clear() drops the lock: clear() now resets this.lockHeld = false after removing the directory. Reproduced your snippet: post-clear capture reports lock live: true (was false). Reachable-via-/clear path closed.

Blocking 2 — resume never reclaims a dead lock: tryAcquireSessionLock now probes isTimelineLockLive() before acquireTimelineLock, matching the caller contract in timeline-lock.spec.ts. A live lock is left alone (acquire still returns false); a dead one is reaped and replaced. Verified: resumed session's lock PID becomes the current process and probes live.

Smaller items: BOMs stripped from the changeset and timeline-lock.spec.ts (byte-verified clean); changeset no longer claims a dispose() production caller — it now documents the clear()-reset and resume-reclaim behavior instead; rename is a top-level import; added a refreshTimelineLock leaves no temp files behind test.

Verification on this head: tsc, tsc -p plugins/vscode, biome check, biome lint, knip (exit 0) all clean; timeline-lock.spec 14/14, timeline-manager.spec 25/25. Full-suite note: pnpm test:ava shows ~100 failures on this Windows box, but I ran the identical suite on clean upstream/main (107 failed there) and diffed — zero failures unique to this branch, all pre-existing/environmental.