Review two environments at once: concurrent nit instances (frontend + admin UI)

[kevmtt]

Use case

A feature often spans two surfaces: the storefront and the admin UI behind it. You add a field in the backend admin, and the storefront has to render it. Reviewing that means flipping between http://localhost:4200 and http://localhost:3000 constantly — change something in admin, check how the storefront reacts, nit both sides as you go.

Today you can only review them one after the other. The second nit review in another terminal does not start at all.

What already works

The multi-base workspace from #13 already models this exactly: one nit-review/ holding a self-contained review per base URL, one MCP server serving both with qualified ids (admin:a1), and --base narrowing every command to one environment.

nit-review/
  bases.json
  localhost-4200/     the storefront
  localhost-3000/     the admin ui

So the data model needs no change. What is missing is only running two sessions at the same time.

What blocks it today

1. The Chromium profile singleton (hard blocker). src/browser/launch.ts:42 pins every session to ~/.nit/chrome-profile, and profileDir is a test-only hook that is not exposed on the CLI. Two headed persistent contexts on one user-data-dir cannot coexist — Chromium's process singleton hands off to the running instance and the second process dies:

first headed context OK
SECOND HEADED FAILED: browserType.launchPersistentContext:
  Target page, context or browser has been closed

(Headless does not hit this, which is why it has gone unnoticed — nit runs headed.)

2. bases.json write race. writeIndex (src/store/workspace-write.ts:261) writes a fixed bases.json.tmp and renames it. Two processes racing: one renameSync hits ENOENT and throws straight out of ensureBase, killing session startup. The read side self-heals from folder names, so no data is lost — but the crash is real.

3. Flat → multi migration race. A workspace that is still flat (a single nit-review/annotations.json) and gets two instances at once has both processes see layout: 'flat' and both call migrateFlat. One throws <slug>/ already exists, or worse, the two interleave inside the renameSync loop mid-move.

4. Two instances on the same base would lose annotations. All three browser commands write — view flushes too, through the context-close handler at src/browser/session.ts:191 — and mergeExternalStatuses only reconciles status changes of annotations it already knows. An annotation created by a second instance on the same base is silently dropped on the next flush. This one has to be refused, not merged.

Annotation storage across different bases is already safe: each base has its own folder and its own atomic store flush.

Proposed feature

Two independent processes, one per environment, two terminals:

terminal 1 $ nit review http://localhost:4200
terminal 2 $ nit review http://localhost:3000 --window right

Profile registry

~/.nit/profiles.json maps base slug → profile directory, resolved relative to ~/.nit/:

{ "version": 1,
  "profiles": { "localhost-4200": "chrome-profile",
                "localhost-3000": "chrome-profile-localhost-3000" } }

Resolution order: an explicit --profile <name>, then a registry hit, then a miss — which adopts the existing ~/.nit/chrome-profile in place if it exists and no other slug has claimed it, else mints chrome-profile-<slug>. The choice is recorded, so it never moves again.

Nothing is copied. The existing profile is ~476 MB on a normal dev machine, so copying it per base is not an option; adopting it in place means the environment you already use keeps every login, and only genuinely new environments start cold. nit doctor reports the mapping.

The file is hand-editable, so values are validated as safe path segments before reaching path.join — the same defence isSafeSlug already gives bases.json.

Locks

Lock files live in ~/.nit/locks/<sha1(abs base dir)>.json, not inside the review folder — that folder is committed to git, packed by nit export, and shared with teammates, and a lock has no business travelling with it.

Acquisition is fs.openSync(path, 'wx') (atomic create). On EEXIST the holder is read and probed with process.kill(pid, 0): a dead or malformed holder is taken over silently, a live one produces a refusal naming the command, pid, url and start time. Release on session.done and on process.on('exit'); a hard kill leaves a stale lock that the next run's liveness probe clears.

Two kinds:

• Base lock — held for the session's lifetime by review, view and verify alike, which is what makes blocker 4 an explicit refusal instead of silent data loss.
• Workspace mutex — short-lived, around ensureBase, so migrateFlat and writeIndex cannot interleave (blockers 2 and 3). Bounded retry, then a clear error rather than an indefinite hang.

nit doctor reports live locks and prunes dead ones.

--window

--window left|right|<x>,<y>[,<w>,<h>] on review, view and verify, so the second instance does not open stacked on the first.

Applied after launch via CDP (Browser.getWindowForTarget + Browser.setWindowBounds) rather than through Chromium launch args, because left/right need the screen size and that is only available from the page (screen.availWidth). It must run before openPanel — the panel docks itself off the site window's live screenX/outerWidth (src/browser/panel.ts:33) and would otherwise dock against the pre-move position. A CDP failure logs one line and continues rather than aborting the session.

No automatic tiling: window managers already do this well, and guessing wrong is more annoying than passing a flag.

Non-goals

• No cross-base awareness in the panel — each window shows its own environment only.
• No merged view of both environments.
• No annotation schema change, and no MCP change. The workspace and MCP layers already handle N bases; this only makes N of them safely live at once.
• No cap on instance count. Two is the motivating case, not a limit.

Footprint

New: src/util/profiles.ts, src/util/lock.ts, src/browser/window.ts.

Changed: src/browser/launch.ts, src/browser/session.ts, src/cli/index.ts, src/cli/doctor.ts, src/store/workspace-write.ts, src/types.ts, plus README.md and docs/wiki/{commands,reviewing,how-it-works}.md, src/README.md.

Untouched: all of overlay/, panel/, anchor/, capture/, store/store.ts, mcp/ — the same separation that made #13 possible.

Tests

• unit-profiles.test.js — adopt-legacy, already-claimed legacy, fresh mint, --profile override, unsafe value rejected, no legacy present.
• unit-lock.test.js — live holder refused, dead pid taken over, malformed lock taken over, release on exit.
• unit-window.test.js — the --window parse table.
• cli-concurrent.test.js — two headless sessions on two bases into one workspace: both sets of annotations survive; a second session on the same base exits 1 naming the holder.
• cli-migrate.test.js (extended) — concurrent ensureBase against a flat folder produces exactly one migration and both bases.

Risks

Risk	Mitigation
Existing users lose logins	The first base to run adopts the existing profile in place; only new environments start cold. nit doctor shows the mapping.
A stale lock blocks a legitimate run	Liveness-probed on every acquire; nit doctor prunes.
Two instances on one base corrupt a review	Refused up front — the one case where the loss would be silent.
CDP window placement fails on some setup	Non-fatal; logs one line and continues.