refactor: named seams — one download path, cli-core atomic writes and lock, program.ts split

[leemour]

Step 7 of the architecture re-review (2026-10-14), zoom's items. Each claim was checked against origin/main at 9062f17 before acting; the review is three days older than that.

1. Store opening — claim false as stated, one small duplicate fixed

• Found: the store is opened in one place only (createStoreConnector in src/store-connection.ts). There were no eight copies. What was written twice was the step before it: program.ts and bin/main.ts (the MCP path) each checked the selected profile, built the { profile, externalId, intent } request and phrased the same error.
• Changed: connectSelectedProfile in store-connection.ts does that once. StoreConnection and StoreConnectRequest move from program.ts to store-connection.ts (the contract they describe); index.ts still exports both. The MCP path still reads the profile once; program.ts still wraps non-CliError failures.

2. Hand-written atomic writes and lock — claim true

• Found: six temp-file-then-rename/link writes (archive manifest, verified-size cache, schedule job file, systemd unit, sync checkpoint, export file) and one hand-written lock (the checkpoint's .lock/owner.json with a .lock.recovery directory).
• Changed: five writes call cli-core 0.307.0 writeFileAtomic, each keeping mode 0600, its overwrite rule (overwrite: false for units, !!existing for schedule jobs) and its last-moment check in beforePublish (the schedule file's inode/size/mtime check, the checkpoint lease check, cancellation). signal is never passed, so cancellation still raises zoom's own timeout/cancelled errors.
• Checkpoint lock → acquireLock at a new path <checkpoint>.locks, so an old .lock/owner.json is not read as an unknown holder. Contention still answers configuration_error with the same words; release twice still refuses.
• Left on purpose: src/output-file.ts. It reports a failure when cleanup after publishing fails (its tests pin that); writeFileAtomic swallows that cleanup error.

Strictly safer:

• the verified cache is now fsynced, and its folder too (it was a plain write + rename);
• a checkpoint temp file is now removed when the lease check fails (it leaked);
• the lock folder <checkpoint>.locks stays 0700 like the old .lock (created private before acquireLock, mode set again once held; tested);
• the checkpoint folder sync refuses a non-ordinary folder. A symlinked archive folder still works: the save resolves the folder first (new test).

Not strictly the same — please review:

• two processes asking for the checkpoint lock at the same instant can both be refused (never both granted); a retry succeeds. The archive writer lock already behaves this way. The test now asserts "at most one winner, then a retry wins".
• an empty lock folder no longer blocks; an unreadable marker still does.
• an older zm that still uses .lock does not exclude the new .locks (no users yet).

3. Download paths — claim true in substance, wrong place named

• Found: ZoomRemote itself had no download; the host check and the manual-redirect loop were written twice, in api.ts (transcript) and recording-archive.ts (recording).
• Changed: one ZoomRemote.download(url, { accessToken, signal, what, headers, unreachable }): checks the host on every hop, follows at most 5 redirects, cancels redirect bodies, returns the final response. Status and body checks stay with each caller. The transcript path keeps provider_error and no cancellation check on its private 30 s deadline; the recording path keeps network_error and its cancellation check — through the unreachable callback.

4. Large files — split

File	main	now
src/program.ts	977	476
src/recording-archive.ts	560	314
src/recording-manifest.ts (new)	—	214
src/watch-service.ts (new)	—	172
src/store-commands.ts (new)	—	162
src/setup-commands.ts (new)	—	158
src/recording-schedule-setup.ts (new)	—	93
src/meeting-source.ts (new)	—	67
src/answer-policy.ts	10	45

• program.ts keeps the command tree, output and per-run state; per-run state a parse resets stays there or behind a reset() the parse calls. Command registration order is unchanged.
• Layout: zoom's flat *-commands.ts files, not drive's src/commands/ — moving every existing group would be a much larger diff for no seam gained.
• src/mcp-server.ts (476) stays: it is one job (the MCP transport and its tools) and under 500.
• biome.json: per-file cap 1,000 → 500. The installed-package acceptance scripts (scripts/installed-*.mjs, the largest 710 lines) join the tests that are exempt.
• The watch service and the lock/manifest code the review named: the watch cycle is watch-service.ts; the manifest code is recording-manifest.ts; the lock is now cli-core's.

Contract

• commands --json: captured from main and from this branch with ~/.cache/step7/capture.mjs; diff without run_id/duration_ms is empty.
• MCP tools/list of zm serve --mcp (plain, and with --unified-search --meeting-context --meeting-preparation) in a throwaway config: identical (8 / 14 tools). A --semantic run listed the same 8 as plain, so it did not exercise a semantic tool.

Tests changed

• recording-sync-store.test.ts: lock tests plant cli-core markers (<pid>-<nonce>.json in .locks) instead of .lock/owner.json; the .lock.recovery case becomes "a live holder refuses"; the simultaneous-acquire test asserts at most one winner, then a retry wins; new test for a symlinked archive folder (it fails with ENOTDIR without the realpath).
• recording-schedule-store-race.test.ts: the injected race now matches the helper's temp name (.tmp suffix) instead of .zoom-schedule-; it still asserts the race happened.

Not done: effects beside each command

command-effects.ts still decides effects centrally: some commands are mounted by cli-core, and the list wins over per-command annotations. It already disagrees with them (watch is FETCH in the list, LOCAL_WRITE in watch-commands.ts; same for recordings schedule run), so moving effects would change what commands --json reports. Left as debt for its own change.

Checks (local)

The PR runs only changelog / guard and standards; "Full validation" (ci.yml) is workflow_call and did not run here. Run locally:

typecheck, lint, docs:lint, docs:check, standards:check, contracts:check, contracts:upgrade, security:no-leak, generate:check, lint:commands, full tests with coverage, test:matrix:check.

🤖 Generated with Claude Code