AGENTS.md

1# Crit — Development Guide
2 
3Single-binary Go CLI that opens a browser-based UI for reviewing code changes and markdown files with GitHub PR-style inline commenting. Multi-file review with git diff rendering and structured review file output for AI coding agents.
4 
5## Project map
6 
7```
8crit/
9├── cmd/crit/            # package main — thin CLI (main.go, cli_*.go, wire.go)
10├── internal/            # Core logic packages (daemon, server, session, github, share, vcs, …)
11├── web/                 # Embedded frontend assets (Go package webassets; embed.go)
12│   ├── index.html       # HTML shell — code-review OR live-mode script fork
13│   ├── app.js           # Code-review mode JS
14│   ├── live-mode*.js    # Live-mode modules
15│   ├── crit-agent.js / agent-*.js / agent-marker.css  # Injected iframe scripts
16│   ├── crit-*.js        # Shared renderer, SSE, draft, comment UI modules
17│   ├── style.css / style-live.css / theme.css
18│   ├── __tests__/       # Node.js unit tests (node --test)
19│   └── *.min.js         # Vendored markdown-it, highlight.js, mermaid
20├── integrations/        # Drop-in config files for AI coding tools (claude-code, cursor, aider, …)
21├── test/                # Test harnesses, E2E, roundtrip docs, shared fixtures
22├── Makefile
23├── package.json
24└── copy-deps.js         # Copies npm deps into web/ for embedding
25```
26 
27## Key architecture decisions
28 
291. **All frontend assets embedded** via Go's `embed.FS` — produces a true single binary
302. **No frontend build step** — vanilla JS, no npm/webpack/framework. npm is only for fetching vendor libs.
313. **Two modes**: "git" mode (auto-detect from git) and "files" mode (explicit file arguments)
324. **markdown-it for parsing** — chosen because it provides `token.map` (source line mappings per block)
335. **Block-level splitting** — lists, code blocks, tables, blockquotes split into per-item/per-line/per-row blocks so each source line is independently commentable
346. **Diff hunk rendering** — code files show git diffs with dual gutters (old/new line numbers)
357. **Comments reference source line numbers** — stored in `~/.crit/reviews/<key>.json` with per-file sections
368. **Real-time output** — review file written on every comment change (200ms debounce)
379. **File watching** — git mode polls `git status --porcelain`; files mode polls mtimes; reloads via SSE
3810. **Localhost by default** — server binds to `127.0.0.1` (no CORS headers needed). Non-loopback `--host` / `CRIT_HOST` / global `host`, or any `public_url`, require `--allow-unauthenticated-network` / `CRIT_ALLOW_UNAUTHENTICATED_NETWORK=1` (Crit has no network auth).
3911. **Two-level config** — `~/.crit.config.json` (global) merged with `.crit.config.json` (project), CLI flags override both. `agent_cmd`, `auth_token`, `share_url`, and `plan_approve_mode` are global-only (prevents malicious repos from hijacking agent commands, redirecting share requests, or weakening Claude Code permissions)
4012. **Headless CLI comment** — `crit comment` writes directly to the review file without starting the server; SSE notifies any running server
4113. **Comment threading** — comments support nested replies and a `resolved` boolean. Review file schema nests replies inside each comment's `replies` array.
4214. **Centralized review storage** — `~/.crit/reviews/<key>.json` keyed by cwd + branch (git mode) or cwd + args (file mode)
4315. **VCS abstraction** — `vcs.go` defines a backend interface; `git_vcs.go`, `sapling.go`, and `jj.go` are the implementations. Auto-detected, overridable via `--vcs` flag or `vcs` config key. Subcommands not yet threaded through (see TODO at `main.go:1826`).
4416. **Focus mode** — sub-views over the file list: file focus, range focus (`--range A..B`), stacked focus (range layer in a stacked PR). Lives in `focus_*.go` and `/api/focus`.
45 
46<important if="you need to build, test, lint, or run crit">
47 
48```bash
49go build -o crit ./cmd/crit                           # Build
50go test ./...                                         # Run all tests
51gofmt -l .                                            # Check formatting (should be clean)
52golangci-lint run ./...                               # Lint (should be clean)
53make build-all                                        # Cross-compile to dist/
54./crit                                                # Git mode (auto-detect changed files)
55./crit test-plan.md                                   # Review specific file(s)
56./crit --no-open --port 3000 test-plan.md             # Headless on fixed port
57```
58</important>
59 
60<important if="you need to know what crit subcommands do or are adding/modifying a CLI subcommand">
61 
62Subcommands are dispatched via `commandDispatch` in `main.go`. Anything not in the table falls through to `runReview`.
63 
64```
65crit                          # Review git changes (starts daemon, blocks for feedback)
66crit <file|dir> [...]         # Review specific files or directories (falls through to runReview)
67crit review [...]             # Explicit review invocation (same as default)
68crit live <url>               # Review a running web app in live mode (also: crit <url>)
69crit preview <file.html>      # Review a local HTML file in preview mode (also: crit <file.html>)
70crit stop [--all]             # Stop daemon for current directory; --all stops every daemon
71crit status [--json]          # Show review file path, daemon status, comment stats
72crit cleanup [--days N] [--force]  # Delete stale review files from ~/.crit/reviews/
73crit pull [pr-number]         # Fetch GitHub PR comments into the review file
74crit push [--dry-run] [--event <type>] [-m <msg>] [pr]  # Post review comments as a GitHub PR review
75crit pr <num|url>             # Thin shim — forwards to `crit review --pr <n>`
76crit fetch ...                # Fetch remote artefacts (see runFetch)
77crit comment <path>:<line[-end]> <body>         # Add a comment (no server needed)
78crit comment --reply-to <id> [--resolve] <body> # Reply to a comment
79crit comment --json [--file <path>] [--author <name>]  # Bulk add comments from JSON (stdin or --file; - = stdin)
80crit share <file> [file...]   # Share files to crit-web, print URL
81crit unpublish                # Remove shared review from crit-web
82crit config [--generate]      # Print resolved config (or starter template)
83crit install <agent>          # Install integration config for an AI tool
84crit auth ...                 # Auth flow for hosted crit-web (login/logout)
85crit plan [...]               # Plan-file workflow
86crit plan-hook [--mode claude|codex]  # Internal hook used by agent plan flows
87crit check                    # Self-check (env, git, gh availability)
88crit _serve                   # Internal: foreground server (used by daemon spawn)
89crit --version | -v           # Version
90crit help | --help | -h       # Show help
91```
92</important>
93 
94<important if="you are working with config files (~/.crit.config.json or .crit.config.json) or adding a config key">
95 
96Two-level JSON config files, merged (project overrides global):
97 
98- **Global**: `~/.crit.config.json` — user-wide defaults
99- **Project**: `.crit.config.json` in repo root — per-project overrides
100 
101Config keys: `port`, `host`, `no_open`, `share_url`, `quiet`, `output`, `author`, `base_branch`, `ignore_patterns`, `auto_viewed_patterns`, `agent_cmd`, `auth_token`, `auth_user_name`, `auth_user_email`, `auth_user_id`, `plan_approve_mode`, `cleanup_on_approve`, `notify_on_round_ready`, `disable_stats`, `no_update_check`, `no_integration_check`, `vcs`, `proxy_auth`, `live_cookie`, `live_cookie_file`, `live_cdp_url`, `close_on_approve_after_ms`.
102 
103- `base_branch` overrides auto-detected default branch (used as diff base in git mode, and by `crit pull`/`crit push`/`crit comment`)
104- `author` falls back to the configured VCS user name if not set
105- `agent_cmd`, `auth_token`, `share_url`, `proxy_auth`, `plan_approve_mode`, and `close_on_approve_after_ms` are **global config only**; project-level config cannot override (security — prevents malicious repos from hijacking the agent command, redirecting share requests to an attacker-controlled host, weakening Claude Code permissions, or forcing a reviewer's tab to auto-close)
106- `close_on_approve_after_ms` (default: unset/disabled) — auto-close the review tab N ms after Approve with no unresolved comments; negative values are treated as unset. Not included in `crit config --generate` scaffolding.
107- `proxy_auth` (default: `false`) — when `true`, terminal `crit share` / `crit fetch` / `crit unpublish` are blocked (SSO proxy); the browser UI uses a popup relay instead. Global-only for security. See proxy-auth transport rules.
108- `cleanup_on_approve` (default: `true`) — auto-delete review file when reviewer approves with no unresolved comments
109- `notify_on_round_ready` (default: `false`) — opt in to a desktop notification when a review round becomes ready for the human
110- `disable_stats` (default: `false`) — disable session stats recording to `~/.crit/stats.json`
111- `ignore_patterns` are unioned (global + project both apply); types: `*.ext`, `dir/`, `exact.file`, `path/*.ext`
112- `auto_viewed_patterns` are unioned (global + project both apply); matched client-side against file paths and applied once per launch to auto-mark matching files viewed (collapsed). No runtime default (empty). Plumbed through `/api/config` only — Go does no glob matching.
113- `vcs` selects backend: `"git"` (default), `"sl"` (sapling), or `"jj"` (Jujutsu)
114- `auth_*` keys hold cached hosted-crit-web credentials (set by `crit auth`); treat as secrets
115- `live_cookie` / `live_cookie_file` forward session cookies to the upstream app in live mode (global or project; prefer gitignored `live_cookie_file` e.g. `.crit/live-cookies.txt`). CLI: `crit live --cookie`, `--cookie-file`
116- `live_cdp_url` reuses cookies from a local Chrome DevTools endpoint (global or project). CLI: `crit live --cdp-url`. Explicit `--cookie` values override CDP cookies with the same name.
117- CLI flags override config file values
118</important>
119 
120<important if="you are adding or modifying a CLI subcommand that HTTP-calls crit-web (share, fetch, unpublish, or any new crit-web API interaction)">
121 
122Self-hosted crit-web behind an SSO reverse proxy cannot be reached from the terminal. When `proxy_auth: true` in global config:
123 
1241. **Terminal subcommands** must call `checkProxyAuthCLIAllowed("crit <cmd>")` at the top of the `Run*` entrypoint (`internal/share/cli.go` pattern). Fail fast with the shared message — do not HTTP-call crit-web and get an HTML login page.
1252. **Browser UI** must implement both transports: direct Go HTTP when `proxy_auth` is false; popup relay via `web/crit-share.js` + crit-web `assets/js/share_receiver/handlers.js` when true. See `.claude/rules/proxy-auth-transport.md`.
1263. **New crit-web endpoints**: add a popup handler in crit-web `share_receiver/handlers.js` (same-origin fetch proxy to the existing `/api/...` endpoint — no relay-specific API). Add the relay branch in `web/crit-share.js` / `web/app.js`.
1274. **Integration tests** that exec the `crit` binary must use an isolated temp `HOME` (`runCritCmd` in `share_integration_test.go`) so a developer's `proxy_auth` setting doesn't skew results.
128 
129Full rules: `.cursor/rules/proxy-auth-transport.mdc` / `.claude/rules/proxy-auth-transport.md`.
130</important>
131 
132<important if="you are working with crit pull, crit push, or GitHub PR sync">
133 
134Requires `gh` CLI installed and authenticated.
135 
136- `crit pull` fetches PR review comments (RIGHT-side only) and merges them into the review file, deduplicating by author+lines+body
137- `crit push` reads the review file and posts unresolved comments as a GitHub PR review
138- `crit push --dry-run` shows what would be posted without creating the review
139- `crit push --event approve` submits an approval; `--event request-changes` requests changes (default: `comment`)
140- `crit push -m 'message'` adds a review-level body message
141- PR number auto-detected from current branch, or pass explicitly: `crit pull 42`
142- Any code path that imports comments from an external source (GitHub PR, crit-web) into the local review file MUST dedup against local state first: `buildLocalIDSet` + `buildLocalFingerprintIndex` + `dropDuplicateWebComment`. This applies to direct HTTP paths AND browser relay paths. Calling `mergeWebComments` without pre-filtering causes duplicate comments on repeated pull.
143</important>
144 
145<important if="you are writing, running, or modifying Playwright E2E tests in test/e2e/">
146 
147The `test/e2e/` directory contains Playwright tests against a real compiled `crit` binary — no mocking.
148 
149### Running
150 
151```bash
152make e2e                                              # Full suite
153cd test/e2e && npx playwright test tests/comments.spec.ts  # One file
154cd test/e2e && npx playwright test --headed                # Visible browser
155E2E_DEBUG=1 make e2e                                  # Enable video + trace capture on failure
156make e2e-report                                       # View HTML report with screenshots
157```
158 
159### Projects
160 
161Nine Playwright projects. Test naming convention determines which project runs which file:
162 
163| Project | Port | Fixture | Test glob |
164| --- | --- | --- | --- |
165| `git-mode` | 3123 | `setup-fixtures.sh` (git repo + feature branch) | `*.spec.ts` (excludes other suffixes) |
166| `mobile` | 3123 | `setup-fixtures.sh` (reuses git-mode fixture) at 375x812, `hasTouch: true` | `*.mobile.spec.ts` |
167| `file-mode` | 3124 | `setup-fixtures-filemode.sh` (plain files, no git) | `*.filemode.spec.ts` |
168| `single-file-mode` | 3125 | `setup-fixtures-singlefile.sh` (one markdown file) | `*.singlefile.spec.ts` |
169| `no-git-mode` | 3126 | `setup-fixtures-nogit.sh` (file mode without git) | `*.nogit.spec.ts` |
170| `multi-file-mode` | 3127 | `setup-fixtures-multifile.sh` (code + markdown files) | `*.multifile.spec.ts` |
171| `range-mode` | 3128 | `setup-fixtures-range-mode.sh` (`--range A..B` stacked git) | `*.rangemode.spec.ts` |
172| `live-mode` | 3129 | `setup-fixtures-livemode.sh` (Go upstream + crit live) | `*.livemode.spec.ts` |
173| `share-transport` | 3132 (stub crit-web on 3133) | `setup-fixtures-sharetransport.sh` (file mode + stub crit-web) | `*.sharetransport.spec.ts` |
174 
175The `mobile` project shares the git-mode fixture port. In `run.sh` it runs strictly after `git-mode` finishes so the two don't race on shared comment state (both projects `DELETE /api/comments` in `beforeEach`).
176 
177CI runs E2E on PRs via `.github/workflows/test.yml`; a separate `coverage.yml` uploads unit coverage on push to `main`. Failed test artifacts are uploaded.
178 
179### Best practices
180 
181- **Never `waitForTimeout` / `setTimeout`** for state. Use auto-retrying assertions (`toPass()`, `toHaveClass()`, `toBeVisible()`). Sleep is OK only inside polling loops where you're already retrying.
182- **Never `.count()` followed by `expect(count).toBe(N)`** — that's a snapshot. Use `await expect(locator).toHaveCount(N)` or wrap in `toPass()`.
183- **Always import shared helpers from `./helpers`** (the file is `test/e2e/tests/helpers.ts`, plus `range-helpers.ts` for range-mode tests): `clearAllComments`, `loadPage`, `mdSection`, `goSection`, `jsSection`, `switchToDocumentView`, `dragBetween`, `clearFocus`, `addComment`, `getMdPath` (and `rangeFixture`, `ensureRangeFocus`, `ensureStackedFocus` from `range-helpers`). Don't redefine locally. Use `Page` types, not `any`.
184- **Always call `clearAllComments(request)` in `beforeEach`** — server persists comments across tests. This calls `DELETE /api/comments` (bulk endpoint).
185- **Markdown defaults**: git mode → diff view (call `switchToDocumentView()`); file mode → document view (no toggle).
186- **Parallel execution**: projects run in parallel via shell. Within a project, tests run sequentially (`workers: 1`) — don't change this; they share server state.
187- **Scroll before interact**: in file-mode (multiple files below the fold), call `scrollIntoViewIfNeeded()` before hover/click/drag.
188- **CSS selectors**: check existing tests for class names (e.g. `.tree-comment-badge`, not `.tree-file-comments`).
189</important>
190 
191<important if="you are running or modifying share integration tests (build tag: integration)">
192 
193`share_integration_test.go` exercises the crit ↔ crit-web share flow. When modifying share logic, the share payload, comment sync, or any crit-web interaction:
194 
1951. Run: `make e2e-share` (or `./scripts/e2e-share.sh`)
1962. Add new test cases for new share functionality — name them `TestShareSync*`
1973. Inspect on web: `./scripts/e2e-share.sh --serve` starts crit-web and logs review URLs
198 
199Requires a local crit-web checkout at `../crit-web` and PostgreSQL. See `scripts/AGENTS.md` for full details.
200</important>
201 
202<important if="you are modifying crit pull, crit push, GitHub PR comment sync, the review-file ↔ GitHub roundtrip, or anything in `github.go` / `pr_cache.go` / `pr_fetch_test.go` / `push_buckets.go` / `comment_cli.go` reply handling">
203 
204`roundtrip_integration_test.go` (build tag `e2e_github`) exercises the crit ↔ GitHub PR roundtrip against a real sandbox PR. When modifying pull/push, GitHub-comment-bucket logic, reply posting, or `mergeGHComments*` dedup:
205 
2061. Run: `make e2e-roundtrip` (or `./scripts/e2e-roundtrip.sh -run <TestName> -v` for one scenario)
2072. Add new `TestRoundtrip_<Name>` scenarios for new state transitions — see `test/roundtrip/README.md` for authoring notes
2083. If a scenario is currently `t.Skip`'d against an issue and your change fixes the underlying bug, REMOVE the skip and run the scenario
209 
210Requires `gh` authenticated and `CRIT_ROUNDTRIP_REPO=<owner>/crit-roundtrip-sandbox` exported. Each scenario opens-then-closes a real PR (~10-25s each, ~100s suite). Tests are local-only (build tag keeps them out of CI / default `go test ./...`).
211</important>
212 
213<important if="you are adding or modifying HTTP API endpoints in server.go">
214 
215All routes wrapped with `s.withReady` return 503 until session init completes — except `/api/health` and `/api/qr`.
216 
217Session-scoped:
218 
219- `GET  /api/health` — liveness probe (no readiness gate; used for daemon health checks)
220- `GET  /api/qr` — QR code for current shared URL
221- `GET  /api/session` — session metadata
222- `GET  /api/config` — `{share_url, hosted_url, delete_token, version, latest_version, ...}`
223- `GET  /api/review-cycle` — review-cycle metadata (round number, edits-since-last)
224- `POST /api/share` — perform a share (POST to crit-web `/api/reviews`); returns URL+delete_token
225- `POST /api/share-url` / `DELETE /api/share-url` — persist or unpublish shared URL
226- `POST /api/finish` — write review file, return prompt for agent
227- `GET  /api/events` — SSE stream (file-changed, edit-detected, server-shutdown)
228- `GET  /api/wait-for-event` — long-poll until finish (used by `crit` daemon mode)
229- `POST /api/round-complete` — agent signals all edits done; triggers new round
230- `…/api/focus` — set/clear focus (file or range scope)
231- `…/api/picker` — file-picker UI backend
232- `POST /api/agent/request` — send comment to configured `agent_cmd`
233- `GET  /api/branches` — list local branches (for base-branch picker)
234- `GET|POST /api/base-branch` — read/update active base branch
235- `GET  /api/commits` — list commits between base ref and HEAD (git mode only)
236- `GET  /api/files/list` — list session files (lighter than `/api/session`)
237- `GET|POST /api/comments` — list/add review-level comments
238- `PUT|DELETE /api/review-comment/{id}` (and `/replies[/{rid}]`, `/resolve`) — review-comment CRUD
239 
240File-scoped (require `?path=X`):
241 
242- `GET  /api/file?path=X` — file content + metadata
243- `GET  /api/file/diff?path=X` — diff hunks (git diff for code; inter-round diff for markdown)
244- `GET|POST /api/file/comments?path=X` — list/add comments (10MB body limit on POST)
245- `PUT|DELETE /api/comment/{id}?path=X` — update or delete (10MB body limit on PUT)
246- `POST|PUT|DELETE /api/comment/{id}/replies[/{rid}]?path=X` — reply CRUD
247- `PUT /api/comment/{id}/resolve?path=X` — set resolved state
248 
249Static: `GET /files/<path>` — serve files from repo root (path traversal protected). `GET /` — embedded frontend assets.
250</important>
251 
252<important if="you are modifying server security, request handling, or path-validation logic">
253 
254- Server binds to `127.0.0.1` by default. Non-loopback listen or any `public_url` refuses to start unless `--allow-unauthenticated-network` / `CRIT_ALLOW_UNAUTHENTICATED_NETWORK=1` is set (CLI/env only — never project config). Prefer SSH `-L`, Tailscale Serve to loopback, or Docker `-p 127.0.0.1:port:port`.
255- State-changing requests (POST/PUT/PATCH/DELETE) with a `Sec-Fetch-Site` header must be `same-origin`. Missing header is allowed (CLI/curl/agent). `cross-site` is rejected — CSRF defense against malicious pages posting to loopback. Complements `checkHost` (DNS-rebinding); does not authenticate network clients.
256- `/files/` validates paths, blocks `..` traversal, verifies resolved path stays within repo root
257- Body size: 10MB for comments, 1MB for share-url via `http.MaxBytesReader`
258- HTTP server: `ReadTimeout: 15s`, `IdleTimeout: 60s` (no `WriteTimeout` — SSE needs open connections)
259- Comment renderer uses `html: false` (XSS prevention in user comments)
260- Document renderer uses `html: true` intentionally (reviewing local files)
261</important>
262 
263<important if="you are modifying web/ — app.js, style.css, theme.css, or index.html">
264 
265Frontend split: `index.html` (HTML shell), `app.js` (all logic), `style.css` (layout/components), `theme.css` (theme variables).
266 
267### Multi-file state model
268 
269Three top-level globals in `app.js`: `session` (mode, branch, base_ref, review_round, files), `files` (per-file render state with comments, lineBlocks), `activeForms` (multiple comment forms can be open simultaneously). See top of `app.js` for shapes.
270 
271### Source line mapping (markdown)
272 
2731. Parse with `markdown-it` to get tokens with `token.map` (source line ranges)
2742. `buildLineBlocks()` dispatches to per-token-type handlers: `handleFenceToken`, `handleListToken`, `handleTableToken`, `handleBlockquoteToken`
2753. Container tokens (lists, tables, blockquotes) are drilled into — each item/row/child becomes its own block
2764. Code blocks (`fence` tokens) split into per-line blocks with syntax highlighting preserved via `splitHighlightedCode()`
2775. Each block gets a gutter entry with its source line number(s)
2786. Comments are keyed by `end_line` and displayed after their referenced block
279 
280### Diff hunk rendering (code files)
281 
282Hunk headers (`@@ -27,6 +31,23 @@`), dual gutters, colored backgrounds for additions/deletions, spacers between hunks, inline comment via gutter `+` buttons.
283 
284### Known complexities
285 
286- `markdown-it` token.map quirks: last list item often claims a trailing blank line — code trims trailing blank lines from item ranges.
287- Table separators (`|---|---|`): not in tokens, appear as gap lines. Detected via regex and hidden with CSS.
288- Per-row tables: each row in its own `<table>` with `table-layout: fixed` + `<colgroup>` for column alignment.
289- `splitHighlightedCode()` tracks open `<span>` tags across lines to properly close/reopen them.
290</important>
291 
292<important if="you are changing any agent-*.js, crit-agent.js, or agent-marker.css in web/">
293 
294These files are the scripts crit injects into live/preview iframes — the canonical set + order is `agentScriptFiles` in `server.go`, plus `agent-marker.css` (served at `/agent-marker.css`). **crit-web vendors them verbatim** into `crit-web/priv/static/preview-agent/` so DOM anchoring stays byte-identical across both renderers.
295 
296When you change any of these files here:
297 
2981. Re-sync into crit-web: run `crit-web/scripts/sync-preview-agent.sh` (copies the 8 files from `../crit/web/`).
2992. Commit the change in **both** repos.
300 
301crit-web's drift-guard test `test/crit_web/preview_agent_sync_test.exs` fails loudly if the vendored copies diverge (and skips when the sibling `crit/` checkout is absent, e.g. CI). Don't hand-edit `crit-web/priv/static/preview-agent/*` — always re-sync from here.
302</important>
303 
304<important if="you are adding CSS variables or modifying theme.css">
305 
306Header has a 3-button theme pill (System / Light / Dark):
307 
308- No `data-theme` attribute → system preference via `prefers-color-scheme`
309- `data-theme="light"` / `data-theme="dark"` → explicit override
310- CSS vars are set in `:root` (dark fallback), `@media (prefers-color-scheme: light) html:not([data-theme])`, `[data-theme="dark"]`, and `[data-theme="light"]` blocks. **Define every new variable in all four blocks.**
311- Theme choice persisted via `crit-settings` cookie (`theme` key, `"system"` | `"light"` | `"dark"`).
312- Use CSS custom properties from `theme.css` for all colors. Never hardcode hex values.
313</important>
314 
315<important if="you are modifying share, unpublish, or share-button UI in crit/">
316 
317Sharing is opt-in. When `--share-url` (or `CRIT_SHARE_URL` env var, or `share_url` in config file) is set:
318 
319- Share button appears in the header
320- Click POSTs document + comments to `{share_url}/api/reviews` (crit-web API)
321- Response `{url, delete_token}` persisted to review file via `POST /api/share-url`
322- Share-notice banner shows the URL with Copy / Unpublish actions
323- Unpublish calls `DELETE {share_url}/api/reviews?delete_token=...` then clears local state
324</important>
325 
326<important if="you are modifying multi-round logic, round-complete, or finish handling">
327 
328When the agent runs `crit` again (or calls `POST /api/round-complete`):
329 
330- **Markdown files**: snapshot content, carry forward unresolved comments, re-read from disk
331- **Code files**: re-run git diff against base ref to get updated hunks
332- **File list**: re-run `ChangedFiles()` to detect new/removed files
333- Waiting modal shows live count of file edits while the agent works
334- Diff toggle for markdown shows inter-round changes
335</important>
336 
337<important if="you are modifying daemon spawning, session lookup, or ~/.crit/sessions/">
338 
339`crit` manages a background daemon for seamless multi-round reviews:
340 
3411. **First `crit`**: starts background daemon (`crit _serve`), opens browser, blocks for feedback
3422. **Subsequent `crit`**: connects to existing daemon (same cwd + args), signals round-complete, blocks
3433. **`crit plan.md`**: looks up daemon by hash(cwd + "plan.md") — reuses if alive, starts new if dead
3444. **Ctrl+C**: kills the daemon the client started
3455. **`crit stop`**: kills daemon for current cwd; `crit stop --all` kills every daemon
3466. **Lifetime**: daemon runs until killed (Ctrl+C, `crit stop`, or SIGINT/SIGTERM/SIGHUP). No idle timeout — walking away from a review session is fine.
347 
348### Deferred initialization & readiness
349 
350The daemon signals readiness (via OS pipe) as soon as the HTTP port is bound, but session init (git, file reads) continues in the background. Until `SetSession()` is called, most endpoints return **503 Service Unavailable**.
351 
352**Any client connecting to a daemon must poll `/api/session` until it stops returning 503 before calling other endpoints.** See `runReviewClient` and `runReviewClientRaw` for the canonical readiness loop. Skipping this poll causes races where endpoints return 503, and error-fallback paths may silently allow/approve when they shouldn't.
353 
354### Session registry
355 
356Daemon state in `~/.crit/sessions/`, one file per session.
357- Git mode (no args): `sha256(cwd + "\0" + branch)[:12]`
358- File mode (args present): `sha256(cwd + "\0" + args...)[:12]` (branch excluded — file reviews aren't branch-dependent)
359 
360Session file: `{"pid", "port", "cwd", "args", "branch", "review_path", "started_at"}`. Review data lives at `~/.crit/reviews/<key>.json` (same key).
361 
362`crit _serve` runs the server in foreground (used by daemon spawning, not user-facing).
363</important>
364 
365<important if="you are reviewing code or evaluating audit findings for this project">
366 
367Calibrate against the tool's actual scale before flagging issues. False-positive filters:
368 
369- **"Real problem at this scale?"** Localhost-only, single-user CLI. Patterns that matter for cloud services (context propagation, map-based lookups, connection pooling) often don't apply. Typical sessions: 5–50 files, <50 comments.
370- **"Does the execution model make this possible?"** JavaScript is single-threaded — there are no race conditions between synchronous scope assignments and async fetches. Verify the threading model before claiming races.
371- **"Realistic inputs?"** Markdown files can be 10,000+ lines (AI-generated plans) — perf concerns for large markdown are legitimate. Perf concerns for file lists or comment lists are not.
372- **"Simpler than the duplication?"** For a single-file vanilla JS app and a flat Go CLI, inline code is often clearer than extracted helpers. Don't abstract for fewer than 3 call sites.
373 
374Project-specific calibration:
375 
376- **Unexport what isn't needed.** This is `package main` (a binary, not a library). If a function/type is only used within the package, it should be unexported.
377- **Don't add `context.Context` to local git operations.** All git commands here are read-only local ops (diff, status, log, rev-parse). They complete in milliseconds and don't touch the network. The one path that benefits from context (`fileDiffUnifiedCtx` for lazy loading) already has it.
378- **O(n) scans over file lists are fine.** A linear scan of `fileByPathLocked` is nanoseconds. Don't add map indices unless profiling shows a real bottleneck.
379- **Mechanical duplication can be OK.** Comment CRUD (review vs file-scoped) is structurally identical but stable. Don't abstract stable boilerplate unless adding new operations would grow the duplication.
380- **Don't fight browser built-ins.** `EventSource` auto-reconnects natively. `<details>`/`<summary>` handles keyboard natively. Don't reimplement.
381</important>
382 
383<important if="you are about to claim work is complete — pre-completion checklist">
384 
385These issues recur in AI-generated code for this project. Only items NOT caught by automated tooling (golangci-lint, ESLint, Stylelint, axe-core) are listed.
386 
387### Go backend
3881. Forgetting to clear review-level comments when clearing file comments
3892. Missing fields in struct construction (silent data loss)
3903. Creating wrapper functions that just delegate
3914. Inline reimplementation of existing helper functions
3925. Mixing `git status --porcelain` and `git diff --name-status` inconsistently
3936. Not validating daemon health check response body
394 
395### Frontend JS
3961. Missing `aria-label` on icon-only buttons (axe-core catches in E2E, but prevent at source)
3972. Not checking `response.ok` after `fetch()`
3983. Not resetting navigation state on context changes
3994. SSE handlers re-fetching more data than changed
4005. Async operations without dedup guards when triggerable from multiple sources
4016. `.remove()` on elements with CSS exit animations (use animationend)
402 
403### Frontend CSS
4041. Not defining new CSS variables in all 4 theme blocks
4052. Leaving dead selectors after renaming CSS classes or DOM IDs
4063. Referencing undefined CSS variables (check-css-vars.sh catches in pre-commit)
407</important>
408 
409<important if="you are compacting context or summarizing this session">
410 
411Always preserve:
412- The list of files modified in this session
413- Any unresolved review comments or crit feedback
414- The current phase of any multi-phase workflow (review, ship, audit)
415- Which worktree you're working in
416</important>
417