[Fermionic-Lyu]

Summary

• src/commands/domain.ts: the three-way copy of "which project signs this org mutation in agent mode" (domainDelegate, zoneDelegate, zoneRelease) is now one local helper, signingScope, carrying the constraint comment once. The doc comment describing zoneDelegate was attached to the orgArgOf const declared between them; the const now sits above the comment so the doc lands on the function it describes. The inline --org suffix in domainDelegate's watch hint now uses the existing orgArgOf.
• src/commands/github.ts: the byte-identical 429-tolerant poll catch in authorizeTerminal and findCallerRepo is one local helper, backOff, that rethrows a non-429 ApiError and returns the next interval.
• src/commands/upgrade.ts: the read-merge-write cache expression repeated in upgrade (twice) and backgroundCheck is one local helper, recordLatest, returning the written cache so backgroundCheck keeps using it.
• src/commands/postgres.ts: resolveDbUrl reuses the module's existing resolvePostgres instead of repeating its two lines; resolvePostgres takes the same structural { request } type resolveDbUrl already accepts.
• src/commands/ssh-config.ts: planCertAuthority ran the pure isSupersededAnchor predicate over every line twice to build the removed set and then its complement; the kept lines are now collected in the same pass.
• src/agent.ts: agentHeaders called the pure canonicalTarget(path) twice; the single result is hoisted above the first use.

Safety

• domain.ts signingScope — duplicate collapse: the three copies were textually identical (same deps?.project ?? readProject() fallback, same org check, same projectId ? { projectId } : undefined shape); readProject is still awaited before rawRequest, as before. test/domain.test.ts and test/zone.test.ts assert the exact scope ({ projectId: 'p1' } or undefined) for matched and mismatched --org.
• domain.ts doc-comment / orgArgOf reorder — pure refactor, reorder declarations of a non-exported const; it is only called at runtime after module evaluation, so the TDZ is unaffected. orgArgOf(opts) is the same expression the hint inlined; test/domain.test.ts checks the hint carries --org org9.
• github.ts backOff — duplicate collapse: both catch blocks were byte-identical apart from a comment; the helper preserves "non-429 ApiError rethrows, anything else backs off by 5s". test/github-connect.test.ts exercises both authorizeTerminal and findCallerRepo.
• upgrade.ts recordLatest — duplicate collapse: identical modulo the variable name; the same object is written and returned. test/upgrade-resolver.test.ts and test/upgrade.test.ts run upgrade and backgroundCheck against a temp cache file and assert its contents.
• postgres.ts resolveDbUrl → resolvePostgres — duplicate collapse: both statements were identical (GET /services${q(branch)} then resolveSoleService(..., 'postgres', service)); the widened parameter type is on a non-exported function. test/db-url.test.ts, test/db-query.test.ts (resolveDbUrl) and test/postgres-access.test.ts (resolvePostgres via dbPublicAccess/dbPrivateAccess) cover both callers.
• ssh-config.ts planCertAuthority — redundant work removed: isSupersededAnchor is a pure string predicate, so the complement of the removed set is the kept set by construction; order of kept is unchanged. The known_hosts trust anchor, an anchor rotation can be taken back and matched field by field suites in test/ssh-config.test.ts (26 tests) ran, not skipped.
• agent.ts canonicalTarget hoist — redundant work removed: canonicalTarget is pure (new URL with a fixed base); it was already evaluated before the /agent/sessions check in every path, so a throw happens at the same point. test/agent.test.ts (10 tests) covers bootstrap, project-route and scoped paths.

Verification

Baseline on the untouched tree:

npm run typecheck          # exit 0
npm test                   # 106 files, 2110 passed | 135 skipped, ~31s

Final tree:

npm run typecheck          # exit 0
npm test                   # 106 files, 2110 passed | 135 skipped, ~30s

Targeted runs after each edit: npx vitest run test/domain.test.ts test/zone.test.ts test/agent.test.ts test/github-connect.test.ts test/upgrade-resolver.test.ts test/upgrade.test.ts test/ssh-config.test.ts test/ssh-known-hosts-injection.test.ts test/db-url.test.ts test/postgres-access.test.ts test/db-query.test.ts → 392 passed, 18 skipped (the 18 are the ssh -G block, skipped because ssh is not installed here).

The 135 skips are pre-existing: ssh-keygen and ssh are absent on this machine, so the keygen-gated describe blocks in test/ssh-orchestration.test.ts and the ssh -G block in test/ssh-config.test.ts do not run. There is no lint script. No suite needs a database or Docker.

Deferred findings

• src/commands/project.ts resolveProjectName (defect): splits cwd and homedir() on / only. On Windows (C:\Users\alice\myapp) the whole path becomes one segment, so the project is created as c-users-alice-myapp and the generic-directory and home-directory guards never trigger. test/create-name.test.ts only uses POSIX paths. A fix (path.basename) changes the created name, so it is the owners' call.
• src/commands/postgres.ts: the branch/group query-string preamble is repeated in dbAlwaysOn, dbLimits, dbStats, dbVolume, dbRestart (and settingsSuffix is a sixth variant), and the "no manageable instance" guard appears four times with slightly different wording. Those command bodies call ApiClient.load()/requireProject() directly and are not exercised by any test (only their pure helpers are), so left alone.
• src/commands/compute.ts setDomain/checkDomain: the 409 → domainConflictMessage mapping line is byte-identical in both, and removeDomain repeats the preamble. Covered by the domain-flow tests, but the gain is one line in a 2200-line file; left for a sweep that touches that area.
• src/commands/auth.ts loginOauth/loginDevice: identical five-line tail (double setSession, /me, persist, same printed line). loginOauth's body has no direct test; not refactored.
• src/commands/services.ts servicesRemove/servicesRename and src/commands/storage.ts storageGet/storageDelete/storageSetAccess: identical preambles, command bodies not covered by tests; not refactored.
• src/commands/setup.ts openPromptInput vs canPromptViaTty: two helpers encode the same stdin-TTY / win32 / /dev/tty decision in the same order and can drift apart; canPromptViaTty has no direct test.
• src/ensure-skills.ts imports resolveSpawnable through the commands/setup.ts re-export rather than from src/spawn.ts; a cross-module import change, so not touched.
• src/config.ts persistAutoLink walks findProjectRoot once in isHomeLinkTarget and again inside writeProject; the second walk is across a function with side effects between, so left.
• 40 exported symbols are only referenced inside their own file (full list recorded in the sweep notes); dropping export is an API-surface change and several are intentional test seams, so not changed.
• The domainStatusLines verdict ladder in compute.ts is repeated three times with differing messages; similar, not identical, so not collapsed.

Filed, not fixed here

• [Yun Tianming] .claude/skills/developing-insta-cli/SKILL.md: commands/ table omits five command files (already open)

---

Opened by Yun Tianming: nightly sweep 2026-10-23-1000, model claude-fable-5-1, 75 turns, 6 files, 85 changed lines.
Verification: npm run typecheck (exit 0) and npm test (106 files, 2110 passed, 135 skipped, identical to baseline) on the final tree; ssh-keygen/ssh-gated suites skip on this machine.

🤖 Generated with Claude Code

---

Summary by cubic

Collapses duplicated signing, retry, and cache logic across six command files without changing behavior. The 429-tolerant poll backoff in github.ts, the project-signing scope in domain.ts, and the cache write in upgrade.ts now live in one local helper each; the other changes remove redundant work or reuse existing helpers.

• planCertAuthority in ssh-config.ts collects kept lines in the same pass that builds the removed set instead of filtering twice.
• canonicalTarget in agent.ts is now evaluated once instead of twice.
• A doc comment in domain.ts was moved so it lands on the function it describes, and the inline --org suffix reuses the existing orgArgOf helper.
• resolveDbUrl in postgres.ts now calls the module's resolvePostgres instead of repeating its two lines.
• Typecheck passes and all 2110 tests pass, same as baseline; a pre-existing Windows path bug in resolveProjectName was noted but left for the owners.

^{Written for commit 952bdc6. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 6 files

_{View guided diff | Turn on auto-fix | Re-trigger cubic}

[Fermionic-Lyu]

Addressed

• None. cubic-dev-ai reported "No issues found" across the 6 changed files, so there was nothing to act on.

Declined

• None.

Verification

Re-ran the pre-commit gate on the tree at 952bdc6 (no changes this round):

npm run typecheck   # tsc -p tsconfig.json --noEmit → exit 0
npm test            # vitest run → 106 files, 2110 passed, 135 skipped, exit 0

The repository has no lint script in package.json, so typecheck + tests are the full gate.

---

Yun Tianming, review round 1 of 10: nothing left to change.

[jwfing]

LGTM - approved.

[jwfing]

Summary

A six-file duplicate-collapse sweep that is behaviour-preserving everywhere I could pin it: typecheck exit 0, full suite green, and 11 of 12 negative-control mutations on the collapsed code go red.

Requirements context

No matching spec/plan found — this repo has no docs/ tree at all (ls at 952bdc6 shows only .claude/, .github/, AGENTS.md, CLAUDE.md, CONTRIBUTING.md, README.md, scripts/, src/, test/). Assessed against the PR body plus .claude/skills/developing-insta-cli/SKILL.md (the command-architecture rules there are not engaged: no command, flag or help-surface change, so the cli-reference.md mirror rule and the MCP-parity rule in §6 do not apply).

Verification I ran at this head

Clean npm ci, node v20.20.2, in a throwaway workspace at 952bdc6:

• npm run typecheck → exit 0
• npx vitest run → 106 files, 2244 passed, 1 skipped (~73 s). Your body reports 2110 passed / 135 skipped; the difference is environmental — ssh/ssh-keygen are present on this box, so the gated describe blocks ran rather than skipped. Not a discrepancy in the change.
• CI at this head: test, test-windows, cubic all success. Sibling reviews at 952bdc6d: cubic "No issues found", jwfing APPROVED.

Negative controls (each mutation applied alone, tree restored after):

mutation	result
domainDelegate call site → hard-coded { projectId: 'WRONG' } (domain.ts:183)	2 red
zoneDelegate call site → same (domain.ts:396)	2 red
zoneRelease call site → same (domain.ts:439)	1 red
signingScope drops the link.orgId === orgId check (domain.ts:32)	2 red
backOff 429 → 500 (github.ts:101)	2 red
backOff stops growing the interval (github.ts:102)	2 red
backOff rethrow deleted (github.ts:101)	red (suite hangs → error after 368 s, i.e. the infinite poll it guards against)
recordLatest deleted at upgrade.ts:288	1 red
recordLatest('9.9.9', now) at upgrade.ts:329	5 red
recordLatest stops merging the prior cache (upgrade.ts:176)	1 red
planCertAuthority drops the else keptLines.push(l) (ssh-config.ts:682)	11 red
backgroundCheck uses { checkedAt: now, latest } instead of the returned cache (upgrade.ts:361)	GREEN — see Suggestion 1

Per the sweep-dedup lesson from #380, I ran the domain NCs per call site, not just once inside the shared helper; all three sites are independently pinned here.

Findings

Critical

(none)

Suggestion

1 — test coverage: backgroundCheck's use of the returned cache is unpinned (src/commands/upgrade.ts:358-366, :204)

Your Safety section says recordLatest returns "the written cache so backgroundCheck keeps using it" — that return feeds decideAction, whose auto-throttle reads cache.lastAutoAt (upgrade.ts:204). Replacing line 361 with recordLatest(latest, now); const cache = { checkedAt: now, latest } — i.e. dropping lastAutoAt from the value handed to decideAction, which would let an auto-upgrade re-fire inside the throttle window — leaves test/upgrade.test.ts + test/upgrade-resolver.test.ts at 38/38 green. What is pinned is the on-disk preservation (test/upgrade-resolver.test.ts:414-421, via upgrade, not backgroundCheck). This is a pre-existing gap that your refactor does not widen, but since the return value is the one novel bit of the collapse, a backgroundCheck test seeding lastAutoAt recently and asserting 'nudge' rather than an auto-run would lock it down.

2 — the deferred resolveProjectName Windows defect is real; file it (src/commands/project.ts:55-61)

Confirmed by reading the code: cwd.split('/') and homedir().split('/') make C:\Users\alice\myapp a single segment, so the slug is c-users-alice-myapp and both the GENERIC_DIRS and the base !== home guards are dead on win32. This repo runs test-windows in CI, so it is a platform the project supports, and the symptom (a project provisioned under a junk name) is user-visible. You correctly left the fix out of a refactor sweep, but unlike the SKILL.md gap you filed as #379 this one exists only in a PR body that disappears on squash — open an issue for it.

3 — the merged comment under-enumerates its own callers (src/commands/domain.ts:23-29)

The helper now serves three verbs, but the comment names only domain.delegate and zone.delegate. zoneRelease (domain.ts:439) is the third caller and previously carried no comment at all, so a reader checking whether zone.release really is read at the session project gets no answer. Either name the third scope or phrase it as "these org mutations" so the list cannot go stale as callers are added.

Information

• Security — the one authorization-adjacent change is behaviour-preserving. signingScope concentrates the "only sign with this directory's project when the link belongs to the org being mutated" rule. The three originals were textually identical in the diff, and the collapsed check still fails closed: removing link.orgId === orgId reds 2 tests, and all three sites assert both the matched ({ projectId: 'p1' }) and, for two of them, the mismatched (scope undefined) shapes (test/domain.test.ts:350,390,392; test/zone.test.ts:39,52,124). readProject() is still awaited before rawRequest — the await signingScope(...) argument is evaluated before the call. No new user input reaches SQL/shell/HTTP, no secrets newly logged, no new dependencies.
• zoneRelease has no org-mismatch case of its own — test/zone.test.ts:124 asserts only the positive scope. Pre-existing, and the shared helper now means the branch is exercised by the other two verbs; noting it only so a future split of the helper does not silently un-cover it.
• Performance — strictly neutral-to-better, no regressions. planCertAuthority halves the isSupersededAnchor evaluations and preserves kept order (filter order == forEach push order) and the removedAt indices. agentHeaders drops one duplicate new URL parse. resolveDbUrl still issues exactly the same two requests — resolvePostgres performs the identical GET /projects/:id/services${q(branch)} the inlined copy did, so no extra round-trip and no N+1 introduced. No new loops, allocations or blocking I/O.
• resolvePostgres's widened parameter (postgres.ts:475) is not new looseness: it is a verbatim copy of the structural { request: (m: string, p: string) => Promise<any> } field resolveDbUrl has declared since before this PR (postgres.ts:313-316), on a non-exported function, and is what makes the reuse typecheck. Fine as-is.
• orgArgOf TDZ — the const at domain.ts:382 is referenced from domainDelegate at :195, textually earlier. Safe: module-scope const, and nothing calls domainDelegate during module evaluation. Typecheck and the full suite agree.
• No new tests, and that is the right call here — the repo's TDD convention applies to behaviour changes; a duplicate collapse is graded by the existing suite staying green and by the collapsed code remaining pinned, which the NC table above establishes. Scope is clean: 6 files, +45/-40, no behaviour, no command surface, deferred items genuinely deferred.
• backOff's mutant that deletes the rethrow does not fail an assertion — it hangs the poll loop until vitest errors out. That is the correct red, but it is the slow kind; worth knowing if anyone re-runs it.

Verdict

approved — zero Critical findings; 3 Suggestions and a set of Information notes, none blocking. (Informational: the GitHub green check remains a separate human action via the approve flow — and jwfing has already approved at this head.)