* feat: introduce v4 folder-format review storage with migration shim

Replaces the flat <key>.json + <key>.snapshots.json layout with a
per-review folder containing review.json and snapshots.json. The folder
identity remains the existing path returned by resolveReviewPath, so all
session/daemon registry consumers stay backward compatible.

- New types: RoundSnapshot, SnapshotsFile, reviewPaths
- New helper: reviewPathsFor(identity) — derives folder/Review/Snapshots
- New helpers: loadSnapshotsFile, saveSnapshotsFile (atomic via atomicWriteFile)
- ensureReviewFolder migrates a v3 flat file (and any sibling sidecar) into
  the folder layout via a tmp-folder + rename. Idempotent and crash-tolerant.
  All migration code is tagged MIGRATION-REMOVAL for the future cleanup pass
  (see plan §"Follow-up issue draft").
- loadCritJSON now triggers ensureReviewFolder on read and reads
  <identity>/review.json.
- saveCritJSON writes <identity>/review.json (atomicWriteFile MkdirAlls).
- clearCritJSON / clearReviewFolder os.RemoveAll the entire folder.

Tests cover folder-path derivation, sidecar round-trip, missing-sidecar
benign empty-map, migration of flat files, idempotency on existing folder,
no-op when nothing exists, orphan-sidecar tolerance, folder-wins on crash
recovery, and migration-on-load via loadCritJSON.

Bypasses pre-commit hook (project memory: hook leaks GIT_DIR and corrupts
the worktree). gofmt -l . and golangci-lint run ./... both clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: route all review-identity reads/writes through reviewPathsFor

After the v4 folder layout, every os.ReadFile / os.WriteFile / os.Stat
that targeted a review identity must operate on <identity>/review.json
instead of the identity path directly (which is now a folder).

Production sites converted: share.go (7), main.go (4), github.go (2),
session.go / session_write.go (5), watch.go (2). Session.WriteFiles
empty-removal branch now removes review.json only, not the folder
(B1 fix from plan v4): snapshots are server-only state and may still
be valid for the timeline; full-folder cleanup is reserved for the
explicit cleanup paths.

Session.loadCritJSON now triggers ensureReviewFolder up front and
documents its pre-SetSession lock contract. Session.ClearAllComments
switches to os.RemoveAll on the folder identity.

Test sites converted via mechanical perl rewrite covering the patterns
os.{Read,Write}File(critPath, ...), os.{Read,Write}File(s.critJSONPath()),
and os.{Read,Write}File(filepath.Join(dir, ".crit.json")). Added a small
mustMkdirAll test helper that ensures the parent folder exists before a
direct WriteFile seed inside the layout.

go test -race ./... clean. golangci-lint run ./... clean.
gofmt -l . clean.

Bypasses pre-commit hook per project memory (hook leaks GIT_DIR and
corrupts the worktree). Verified manually via go test -race ./....

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: add Session.RoundSnapshots, capture, R1 baseline, and sidecar restore

Wires per-round file content snapshots into the session lifecycle.

- Session.RoundSnapshots map[path]map[round]RoundSnapshot, lock-discipline
  documented (read/write under s.mu post-SetSession; constructor-time use is
  single-goroutine and lock-free).
- Session.sessionStarted atomic flag; Server.SetSession flips it. Session.
  loadCritJSON enforces the pre-SetSession-only contract via this guard
  with a log + no-op (chosen over panic for end-user safety; documented in
  plan v4 §Lock discipline).
- captureRoundSnapshot(round): files-mode only, skips lazy/deleted, idempotent
  per (path, round) — does not overwrite an existing capture.
- cloneRoundSnapshots: deep-copy helper so callers can release s.mu before
  serialising the sidecar.
- NewSessionFromFiles: extracted captureBaselineAndPersist helper which
  captures R1 and writes <identity>/snapshots.json — gated on having a real
  identity (ReviewFilePath or OutputDir set) so tests that fall back to the
  ambient RepoRoot don't poke the working tree's .crit.json.
- handleRoundCompleteFiles: captures R(N+1) BEFORE rereadFileContents and
  BEFORE incrementing ReviewRound, then persists the sidecar after lock
  release.
- ClearAllComments: resets s.RoundSnapshots = nil inside the same critical
  section as ReviewRound = 1 (in-lock, before unlock + RemoveAll).
- Session.loadCritJSON: ensures folder layout, restores RoundSnapshots from
  the sidecar via loadSnapshotsFromSidecar.

go test -race ./... clean. golangci-lint run ./... clean.
gofmt -l . clean.

Bypasses pre-commit hook per project memory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: extend file/comments handlers with ?round and add /api/rounds

Cross-stage HTTP contract for the per-round timeline (files mode).

- GET /api/rounds — returns {current_round, rounds: [{n, comment_count,
  captured_at}]}. Files-mode only; git/range mode returns the same shape
  with an empty rounds list so the frontend doesn't need mode branching.
- GET /api/file?round=N — returns {path, round, content, previous_content,
  status} from the recorded snapshot. Returns 400 invalid round, 404
  file_not_in_round when the snapshot is absent. Git/range mode ignores
  the round param.
- GET /api/file/comments?round=N — server-side filter to comments where
  review_round <= N. Files mode only.
- GET /api/comments?round=N — same scoping for review-level comments.

New helpers in round_snapshots.go: roundSnapshotForFile,
commentsAtOrBeforeRound, availableRounds. The server's serveFileAtRound
helper centralizes the lookup + 400/404/200 wire-format response so
handleFile stays small.

go test ./... clean. golangci-lint run ./... clean.
gofmt -l . clean.

Bypasses pre-commit hook per project memory. -race shows the same
pre-existing flaky TestReviewCommentsSurviveRound / TestEnsureLoaded
race that exists on origin/main; verified by stashing this work and
reproducing on a clean tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: persist R1 baseline sidecar after constructor sets ReviewFilePath

NewSessionFromFiles captures R1 in-memory but the canonical daemon path
(cli_serve) assigns ReviewFilePath AFTER NewSessionFromFiles returns and
then calls Session.loadCritJSON. Without this fix, the sidecar was never
written for fresh sessions because captureBaselineAndPersist's
ReviewFilePath check fired too early.

Re-runs captureBaselineAndPersist at the end of loadCritJSON so the
in-memory baseline gets written once the identity is known. Idempotent:
if the sidecar already had R1 it stays put.

Verified manually with the smoke harness:
  HOME=/tmp/crit-smoke-home crit --no-open --port N plan.md
  curl /api/rounds → R1 with captured_at present
  ls ~/.crit/reviews/<key>/ → review.json + snapshots.json
And migration:
  Pre-seed <key>.json + <key>.snapshots.json flat
  Restart crit → folder layout, review.json + snapshots.json inside
  /api/rounds reports comment_count from migrated review

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: extend ?round=N to file/diff and session, add line stats to /api/rounds

Completes Task 9 of round-timeline stage 1 v4:

- /api/file/diff?round=N returns the diff between round N and round
  N-1 snapshots (R1 baseline returns empty hunks). 400 on bad param,
  404 when the file lacks a snapshot at that round. Git/range mode
  ignores the param.
- /api/session?round=N filters the file list to files that had a
  snapshot at round N. Files-mode only.
- /api/rounds now reports real additions/deletions per round,
  computed via ComputeLineDiff between adjacent round snapshots.

* feat: cleanup wiring + review-file scans walk v4 folder layout

Completes Task 10 of the round-timeline stage 1 v4 plan.

- findStaleReviews now enumerates folder-form reviews (with the
  v4-native checkStaleReviewFolder helper that reads
  <folder>/review.json and falls back to folder mtime for
  orphan-snapshots folders) plus, behind a MIGRATION-REMOVAL marker,
  legacy v3 flat *.json files.
- deleteStaleReviews / deleteStaleReviewsSilent / cleanupOnApproval
  share a removeStaleReviewPath helper that uses os.RemoveAll for
  folder-form identities and os.Remove (+ sibling sidecar) for the
  v3 flat-file fallback.
- findReviewFileByCommentID and findReviewFileByBranch now scan
  subdirectories first via a shared walkReviewIdentities helper,
  reading <dir>/review.json. Orphan folders (snapshots-only or
  unreadable review.json) are skipped with a stderr warning.
  Legacy flat files are still discovered through the
  MIGRATION-REMOVAL fallback inside the same walker.

* test: add round-snapshot regression guards

Three regression tests for Task 11 of the round-timeline stage 1 v4
plan:

- TestReviewFile_DoesNotContainRoundSnapshots — review.json must
  never carry per-round content; agents only see comment metadata.
- TestSharePayload_DoesNotIncludeRoundSnapshots — share payload
  inspection guards against history leaking to share recipients.
- TestWriteFiles_EmptyDoesNotDeleteSidecar — fix B1 from v3 review:
  emptying the review file via WriteFiles must remove only review.json,
  leaving snapshots.json (and the folder) intact so a mid-session
  comment-clear does not silently lose every captured round.

* test: end-to-end integration test for round snapshots

Task 12 of round-timeline stage 1 v4. Drives the full files-mode
timeline:

- NewSessionFromFiles captures R1 baseline and persists snapshots.json
- Simulated agent edit + handleRoundCompleteFiles produces R2 with
  capture-before-reread ordering preserved
- /api/rounds returns both rounds with non-zero R2 line stats
- /api/file?round=1 serves R1 content
- /api/file/diff?round=2 returns the R1 -> R2 diff with previous_content
  populated
- folder layout sanity: review.json and snapshots.json live inside the
  identity folder; no flat sibling .json file leaks outside it

Test runs in the default test suite (no //go:build integration tag) so
CI catches regressions without an opt-in flag.

* test: cover round-timeline v4 edge cases (migration, cleanup, API, concurrency)

Surgical additions to fill coverage gaps in the v4 folder-format / round-
snapshots stack. No production code changes.

Categories covered:
- Migration: leftover .crit-migrate.tmp dir from a crashed prior run is
  wiped before re-attempting; malformed flat-file JSON still migrates
  (rename is structural, not parsing); in-repo .crit.json identity
  migrates correctly.
- Folder layout: saveCritJSON and the WriteFiles empty-removal branch both
  preserve unknown sibling files inside the review folder (forward-compat
  with future attachments).
- Cleanup: orphan-folder mtime boundary respected; empty folder ignored
  (not stale); mixed v3 flat + v4 folder both swept; partial-failure in
  deleteStaleReviews continues past missing path; cleanupOnApproval
  removes flat-file + .snapshots.json sibling via MIGRATION-REMOVAL path.
- walkReviewIdentities: orphan folder (snapshots-only) silently skipped;
  malformed review.json in a sibling folder does not abort branch scan;
  comment present in BOTH folder and flat review surfaces ambiguity error.
- API: ?round=0 -> 400; ?round=N where N==current returns recorded
  snapshot; empty ?round= falls through to working-tree; duplicate
  ?round=&round= obeys Go's first-wins semantics; /api/rounds in git mode
  returns empty list; /api/rounds rejects non-GET; ?round= filters
  /api/file/comments by ReviewRound.
- commentsAtOrBeforeRound: round<=0 returns nil; does not mutate input
  slice (the [:0:0] reslice contract).
- lineStatsForRound: file added at R(N) with no R(N-1) snapshot counts
  every line as addition; R1 always 0/0.
- sessionStarted guard: post-SetSession loadCritJSON logs BUG and no-ops
  rather than racing on RoundSnapshots / reviewComments.
- Race exercise: concurrent capture/availableRounds/roundSnapshotForFile
  under -race.
- Sidecar restore: orphan paths (file deleted from session since R1)
  are kept; unknown JSON fields tolerated for forward compat.

Pre-commit hook bypassed via core.hooksPath=/dev/null per
feedback_crit_precommit_hook_corruption — go test, go test -race,
gofmt -l, and golangci-lint were all run manually and pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: read .crit.json/review.json in share integration test (review B1)

Post-v4 the review identity is a folder, not a flat file. os.ReadFile on the
identity returns EISDIR. Read the canonical payload at
.crit.json/review.json instead so the share-comment-pull assertion runs.

Pre-commit hook bypassed: pre-commit go test leaks GIT_DIR into the worktree
(see project memory feedback_crit_precommit_hook_corruption). Lint, vet, full
test suite, and -race were run manually before this commit.

* fix: drop unused any return from serveFileAtRound (review I2)

The any payload was discarded at the only call site (handleFile) with an
explicit _ = snap. Match serveFileDiffAtRound's cleaner bool-only signature
and remove the dead return value.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* fix: span handleRounds reads under one RLock (review I6)

Previously GetReviewRound() acquired and released the RLock, then a
separate mu.RLock() guarded the rounds slice. A round-complete landing
between the two reads could yield an internally inconsistent response
(current=N while rounds ended at N-1). Take the RLock once for the whole
handler and read both values directly.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* fix: surface loadCritJSON lock-violation panic under CRIT_DEBUG (review I3)

The post-SetSession guard previously logged BUG: ... to stderr and silently
no-op'd. On a long-running daemon, that stderr line is invisible and a
real regression would never get noticed. Keep the no-op fallback for
production (preserves liveness), but panic when CRIT_DEBUG is set so
dev/CI runs fail loudly. Extracted into reportLoadCritJSONLockViolation
to keep loadCritJSON's cyclomatic complexity within the gocyclo budget.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* docs: assert capture-before-reread invariant in handleRoundComplete (review I4)

Add an INVARIANT comment immediately above captureRoundSnapshot(nextRound)
so future maintainers don't reorder it after rereadFileContents(true).
Reordering would snapshot the new on-disk content as the previous round
and silently corrupt the timeline.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* fix: normalize to folder layout on saveCritJSON (review I7)

Defense-in-depth against a future flat-file regression. ensureReviewFolder
is a no-op when <identity> is already a directory, so steady-state v4
costs only one extra os.Stat per save. If anything ever drops a flat file
back at the identity path (external tool, v3 downgrade, errant test),
this guard migrates it before writing instead of silently producing a
corrupt mixed layout.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* fix: warn on malformed review JSON during cross-file lookups (review I5)

findReviewFileByBranch and findReviewFileByCommentID both scan every
review file in ~/.crit/reviews/ and quietly continue past corrupt
JSON. Silent skip is correct behavior — one bad file shouldn't abort the
whole scan — but zero observability means a real corruption regression
could mask itself indefinitely. Print a stderr warning per malformed
file, matching walkReviewIdentities's existing warn style.

Inline the JSON parse + ID lookup in findReviewFileByCommentID directly
so the warn site is at the call point, and drop the now-unused
reviewFileContainsComment helper.

Pre-commit hook bypassed: see project memory feedback_crit_precommit_hook_corruption.
Lint/vet/test/race were run manually before this commit.

* fix: drop .json from review folder name

The v4 folder-format review storage shipped with `.json` accidentally kept on
the folder name (`~/.crit/reviews/<key>.json/` and in-repo `.crit.json/`).
Folders should not have file extensions: it produced absurd output like
`cat ~/.crit/reviews/<key>.json` returning EISDIR, broke tools that
pattern-match `*.json` as files, and the finish-JSON `prompt`/`review_file`
fields that crit emits to agents pointed at the folder rather than the
cat-able review.json inside.

Centralized layout is now `~/.crit/reviews/<key>/review.json` (folder is
`<key>`, no extension). In-repo layout is `.crit/review.json`. The files
inside the folder are unchanged: review.json + snapshots.json.

Migration shim handles three pre-fix shapes — all idempotent and
crash-tolerant, all marked MIGRATION-REMOVAL:

  1. Steady-state v4 folder at <identity>: no-op.
  2. Early-v4 mid-state folder at <identity>.json: rename to <identity>.
  3. v3 flat file at <identity>.json (or directly at <identity>): move into
     <identity>/review.json plus any sibling .snapshots.json sidecar.

Server-side handleFinish + handleReviewCycle now report
`reviewPathsFor(identity).Review` in the JSON `review_file` field and embed
that path in the agent-facing prompt, so `cat $review_file` works again.
The `crit status` and `crit fetch` user-facing prints surface the file path
similarly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: scope reply visibility to its own review round

Replies inherited their parent comment's visibility window, so a reply
authored in R2 against an R1 parent rendered when scrubbing back to R1.
Round-faithful playback was broken for threaded conversations.

Add Reply.ReviewRound, stamp it on every authoring path (HTTP file +
review-comment replies, headless `crit comment --reply-to`), and filter
replies in commentsAtOrBeforeRound. Legacy replies with no field set
inherit the parent's round so existing data continues to behave the same.

Tests cover the unit helper (chain across rounds, legacy fallback, no
mutation of input), the JSON wire format (round-trip + omitempty), the
HTTP path (reply hidden at round=1, visible at round=2), and both reply
authoring paths (live session + on-disk CritJSON).

* feat: stamp resolved_round on Comment + Reply

Add ResolvedRound field to Comment and Reply, set it from the current
review round on a false -> true transition, and clear it back to 0 when
a comment is unresolved (or re-opened by a new reply).

Wired through the file-comment and review-comment HTTP resolve handlers,
the headless `crit comment --reply-to ... --resolve` CLI path (reads
from CritJSON.ReviewRound), and the disk -> in-memory merge so resolves
made via CLI sync into a running daemon.

Stage 1 of the per-round timeline: the field is exposed via existing
GET /api/file/comments and /api/comments responses (omitempty hides it
for legacy data) and is intentionally not yet used to filter visibility.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: track Position on RoundSnapshot

Add Position to RoundSnapshot, populated from the file's index in
Session.Files at capture time. Surface it on the per-round file API
(GET /api/file?path=...&round=N) so the timeline can render rounds in
their original display order even if the session-level file list later
reorders.

Snapshots persisted before this field landed read back as 0; no
re-population of historical snapshots required — Position lights up
going forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: SetFocus comment refresh + SetSession ordering (review B1, B2)

B1: extract loadCritJSONLocked variant that skips the pre-SetSession guard, used by runtime callers (SetFocus) that already hold s.mu. The public loadCritJSON still enforces the guard for constructor-time callers. Fixes silent comment-wipe on focus changes after SetSession.

B2: store sessionStarted before publishing the session pointer, not after via defer. Otherwise withReady can observe the session pointer with sessionStarted still 0 and a code path that reaches loadCritJSON would falsely believe it is pre-SetSession.

* fix: CLI appendReply unresolves when adding non-resolving reply (review W1)

appendReply (CLI path) now matches AddReply / AddReviewCommentReply (HTTP path) by clearing Resolved and ResolvedRound when a reply is added without --resolve. Without this, a CLI reply on a resolved comment leaves it resolved and the new reply gets hidden by the resolution filter, with two writers producing inconsistent data semantics for the same operation.

* fix: validate ?round consistently across all round-aware endpoints (review W2)

Previously /api/file and /api/file/diff returned 400 on a malformed round value, while /api/session, /api/file/comments, and /api/comments silently ignored the same input. Centralize the parse-and-validate step in parseRoundParam so all five endpoints share one contract: empty value is accepted (back-compat), well-formed integer >= 1 is accepted, anything else is 400.

* perf: skip redundant sidecar rewrite on resumed sessions (review W3, W4, W5)

W3: document that commentsAtOrBeforeRound returns the *current* Resolved/ResolvedRound values, not the state as of the requested round. Stage 1 exposes both fields on the wire so the frontend can compute round-faithful resolution itself; pin the contract with a regression test.

W4: document the inline assumption that sidecar writes from concurrent round-completes are serialized by the upstream debounce. Move-or-no-move guidance lives next to the unlock+write so the constraint travels with the code.

W5: skip the captureBaselineAndPersist disk write when ReviewFilePath / OutputDir was set on entry AND the sidecar already carried snapshots. Resumed sessions used to do an O(N*M) clone+marshal+rename that produced bit-identical bytes on every cold boot. The capture itself stays (it's idempotent and keeps R1 well-defined in memory).

Refactor: split loadCritJSONLocked share/comments restore into helpers to keep cyclomatic complexity under the project gate.

* cleanup: notes from review (W6, N1, N2, N3, N4)

W6: explain why migrateFlatToFolder doesn't route through atomicWriteFile (file-write vs folder-rename are structurally different — reusing the helper would force a redundant read+write of the review JSON for no atomicity gain).

N1: collapse the dead errors.Is branch in findReviewFileByBranch — both branches returned walkErr verbatim.

N2: pin no-mutate intent on commentsAtOrBeforeRound's comments[:0:0] slice header trick so it doesn't read like a typo.

N3: pick the earliest CapturedAt across all files at a round in handleRounds. Map iteration order is randomized, so the previous 'first match' was non-deterministic across requests.

N4: regression test for the capture-before-reread invariant in handleRoundCompleteFiles. Simulates an agent that edited in-memory only; if rereadFileContents ran first the captured R2 content would equal the on-disk R1 content and the timeline would silently lose what changed.

* test: post-rebase fixups for v4 folder layout and SetFocus race

- concurrent_save_test.go: read via reviewPathsFor(...).Review since the
  identity path is now a folder, not a flat file.
- focus_session_test.go: drain debounced WriteFiles in
  TestSetFocus_PostSetSession_PreservesComments before reading on-disk
  state — the debounce goroutine and the test reader were racing on
  s.Files. Existing flushWrites helper handles it.

Both surfaced when rebasing onto origin/main (which added concurrent
save test #445 expecting flat-file layout). Pre-commit bypassed
(GIT_DIR leak); ran gofmt/golangci-lint/go test -race manually.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>