Review remediation: make the flock the sole critical section (concurrency)

[andreiRS]

From the three-lens deep review (review-remediation). Theme: make the flock the sole critical section. Conforms to ADR-0013; no ADR is overturned.

Problem

Mutating work escapes the lock. ADR-0013 says the flock makes id-allocation, validation, writes, and the commit "a single critical section" and that the dirty-tree guard "runs after the lock." It doesn't:

• createTask is the only mutation that omits requireClean, so new's dirty check (src/commands/new.ts:70-73) runs before the flock. A racing new sees the first process's uncommitted file as dirty and aborts with STORE_DIRTY, silently dropping the task (10 concurrent → 6-9 created).
• The preamble's ensureStore + dirty check (src/cli/preflight.ts:38-44) run their own git add/commit/status outside the lock, racing another process on .git/index.lock. The loser gets GIT_ERROR and leaves an orphaned, uncommitted backlog/*.md, breaking "every mutation is a commit."

Slices

1. Decide store-creation vs in-lock guard composition (attended — architectural decision)

The lock lives at <store>/.tasks-lock; acquiring it before the store dir and git repo exist is the open question. Two candidate shapes: (a) flock create-and-locks a fresh path, store init under the lock; or (b) idempotent "create store" runs pre-lock, only "mutate store" runs in-lock. Pick one with a one-paragraph rationale (record in an ADR via domain-docs) so the other slices have no ambiguity.

• Recorded decision on whether store init runs inside or outside the lock, and why.
• Decision specifies lock-acquisition behavior when .tasks-lock / the git repo does not yet exist.

2. tasks new is concurrency-safe (finding #2)

Make createTask run under the in-lock dirty-tree guard (requireClean); remove new's racy pre-lock dirty check.

• Firing N concurrent tasks new (e.g. 10) produces exactly N tasks with zero spurious STORE_DIRTY.
• createTask still rejects a genuinely dirty tree with STORE_DIRTY (now from inside the lock).
• The pre-lock dirty check no longer aborts a racing new.
• Existing concurrency and new tests stay green.

3. All mutating commands guard inside the lock (findings #3, #19)

Move ensureStore and the dirty-tree guard inside the flock in the shared mutating preamble; have new adopt that preamble instead of its inlined copy.

• A burst of concurrent mutating commands never fails with GIT_ERROR from a .git/index.lock collision and never leaves an orphaned untracked task file.
• The store tree is clean after the burst; every mutation is exactly one commit.
• new goes through the shared mutating preamble (no inlined flock / ensureStore / dirty-guard copy).

Open question

Moving ensureStore inside the lock interacts with first-time store creation (the lock path may not exist yet). Resolved by slice 1.

Method

TDD outside-in at the CLI boundary (spawn the binary against a TASKS_HOME tempdir; assert stdout/stderr, exit codes, on-disk state). One commit per green. Order: 1 → 2 → 3.