chore(yun-tianming): drop unreferenced types/errors exports, trim narration comments

[Fermionic-Lyu]

Summary

• Dead exports removed from src/types.ts: RefreshResponse, OrgMembership, ProjectAuthResponse, DatabasePasswordResponse, ConnectionStringResponse, ApiError, and the UploadDeploymentFileResponse name in the @insforge/shared-schemas re-export block. None is referenced anywhere in the repository.
• Dead exports removed from src/lib/errors.ts: NotFoundError, PermissionError (never constructed or caught; exit codes 4 and 5 are still emitted directly via CLIError in domains/index.ts, posthog.ts, webscraper.ts, apify/connect.ts, so the README exit-code table stays accurate) and getJsonFlag (superseded by getRootOpts, which every caller uses).
• Dead export statements removed: export const __testing in src/commands/local/start.ts (comment said "Exported for tests"; no test imports it; the three wrapped functions remain used internally) and export { AUDIT_FILE } in src/lib/guard/audit.ts (constant still used internally).
• Change-narration comments shrunk to their constraint in src/lib/local/checkout.ts (header about the former 266-line vendored compose copy), src/lib/errors.ts (isTransientApiError doc: dropped agent-e2e timing observations), src/commands/branch/create.ts (pollUntilReady doc: dropped CI run IDs), src/commands/local/stop.ts (ordering comment rewritten as the constraint it carries).
• False comment corrected in src/commands/branch/reset.ts: "Match create's 5-min budget" removed; branch create uses a 15-minute budget (create.ts:25). The timeout value itself is untouched (see Deferred findings).
• Documentation drift fixed in DEVELOPMENT.md §4: the files whitelist description now matches package.json, which also ships dist/assets/forger.json and dist/assets/local/cli-overlay.yml (added in commits 4ea573e and 1ac4c4a after the prose was written in 49e193d).

Safety

• Dead code (types.ts, errors.ts, start.ts, audit.ts): each symbol checked with rg -n -w <SYMBOL> . --glob '!node_modules' --glob '!dist' --glob '!.git' --glob '!package-lock.json', returning only the declaration line. dist/index.d.ts is export {} (13 bytes), so no other package consumes these types. Build (tsup, which fails on missing imports, as it did when I briefly removed a re-export that was in fact used) passes.
• Comment edits (checkout.ts, errors.ts, branch/create.ts, stop.ts, reset.ts): comments only; each surviving line keeps the constraint (why the code must behave this way). No executable change.
• Documentation drift (DEVELOPMENT.md): corrected to match package.json files; no new claims.

Verification

Baseline on untouched tree, then identical runs on the final tree:

npm ci                                   # ~1 min
npx eslint src/                          # baseline exit 0 → final exit 0 (~7s)
npx vitest run --passWithNoTests         # baseline 79 files / 838 passed / 14 skipped → final identical (~18s)
npm run build                            # baseline success → final success (~3s)
npx tsc --noEmit -p tsconfig.json        # baseline 17 errors (not a CI gate) → final 17 errors, same files

CI runs npm run lint (= vitest + eslint) and npm run build; both pass. Not run: src/integration/*.test.ts (14 skipped tests gated on INTEGRATION_TEST_ENABLED plus real login/linked project); these are skipped in CI's PR check too.

Deferred findings

• tsc --noEmit fails with 17 errors and no CI gate runs it (npm run lint is vitest+eslint; tsup's dts build does not fail on them). Production-code errors: src/commands/metadata.ts:74,78 reads aiIntegration which does not exist on the @insforge/shared-schemas@1.1.58 metadata type; src/lib/config-capabilities.ts:97 and src/commands/config/apply.ts:321 exhaustiveness never checks fail because the DiffChange variant uses section: 'realtime' | 'schedules' so TS cannot narrow it away (runtime handles both; type-level only); src/lib/prompts.ts:113 Option<T> cast. Test-file errors in create.recovery.test.ts, payments/transactions.test.ts, auth.device.test.ts, cloudflare.test.ts (mock tuple typing).
• src/lib/api/posthog.ts drifted from its copy webscraper.ts: pollPosthogConnection uses consecutiveErrors > maxTransientRetries (webscraper >=), tolerating 6 failures while printing "failed after 5 retries"; no unauthorized arm, so an expired login is retried for the full 15-minute window; fetchWithTimeout lacks the pre-aborted-signal guard. The posthog copies are untested (mocked wholesale in commands/posthog/setup.test.ts).
• src/commands/posthog/setup.ts:79-83: the catch calls handleError (which process.exits) without first awaiting shutdownAnalytics(), so the finally flush never runs on failure. webscraper/apify/connect.ts:86-98 has the fix with an explaining comment. The existing test "still flushes analytics when setup fails" passes only because it stubs process.exit.
• src/commands/branch/reset.ts:16 POLL_TIMEOUT_MS is 5 minutes while branch create was raised to 15 minutes for observed slow regions. Whether reset should follow is a product decision.
• src/commands/backups/index.ts:25-32 resolveProjectId lacks the requireExplicit guard that projects/manage.ts:26-43 added for destructive operations; backups delete/restore accept an ambient INSFORGE_PROJECT_ID.
• src/auth-providers/apply.ts: extractEnvKeys and extractEnvPairs are near-identical, but not equivalent on CRLF input: the pairs regex (.*)$ cannot match a trailing \r, so refreshStaleEnvDefaults never refreshes lines in a CRLF .env.local. Not collapsed for that reason.
• Byte-identical duplicates left in place (cross-module collapse or no test coverage): fetchOptionalConfig/isMissingOptionalEndpoint ×3 in config/{apply,plan,export}.ts; getErrorTelemetry ×3 in lib/command-telemetry.ts, payments/utils.ts, domains/telemetry.ts; log source list + getLogPath in commands/logs.ts and commands/diagnose/logs.ts (neither has a test); ensureGlobalDir in lib/config.ts and lib/cloudflare.ts; sleep/PollOptions in posthog.ts/webscraper.ts; the service-result output block duplicated at compute/deploy.ts:171-186 and :312-327 (tests drive only one mode each).
• src/lib/analytics.ts: eight track* wrappers of one captureEvent shape with two different null contracts (ProjectConfig vs ProjectConfig | null); the track*Usage wrappers in domains/telemetry.ts, payments/utils.ts, deployments/utils.ts lack the once-per-process outcomeRecorded guard that lib/command-telemetry.ts has, so they can double-emit success and failure.
• Test-only production code: src/lib/docker.ts ensureDockerAvailable, dockerBuild, dockerLogin, dockerPush (production uses flyctl.ts; only docker.test.ts calls them); src/lib/local/ports.ts ensurePortsAvailable; src/lib/local/checkout.ts readCheckoutEnv. Removing them means deleting their tests, which is the owners' call.
• Template name list repeated five times (create.ts:265,344,472, projects/link.ts:74,98); create.ts:344 includes empty while link.ts:98 does not. No test asserts agreement.
• README documents no orgs, projects, advisor, billing, usage, backups, memory, feedback, or context command group although all are registered in src/index.ts. Adding documentation is a new claim, so left alone.
• About 19 exported functions are used only within their own module (e.g. auth.ts generatePKCE, cloudflare.ts findCloudflareZone, create.ts copyDir); dropping export is low value and left alone.

Filed, not fixed here

• [Yun Tianming] .agents/skills/cli-development/SKILL.md: describes a tag-triggered publish; workflow fires on GitHub Release

---

Opened by Yun Tianming: nightly sweep 2026-10-07-1300, model claude-fable-5-1, 70 turns, 9 files, 106 changed lines.
Verification: eslint (exit 0), vitest (838 passed / 14 skipped, same as baseline), npm run build (success), tsc --noEmit (17 errors, same as baseline, not a CI gate); integration suites skipped as in CI (need live login/project).

🤖 Generated with Claude Code

---

Summary by cubic

Removes unreferenced exports and trims verbose narration comments across the CLI without changing runtime behavior.

• Drops unreferenced types, error classes (NotFoundError, PermissionError), and getJsonFlag from src/types.ts and src/lib/errors.ts; exit codes 4 and 5 are still emitted directly.
• Removes export-only statements (__testing, AUDIT_FILE) that no test imports.
• Shrinks change-narration comments to the constraint they enforce in checkout, errors, branch create, and local stop.
• Rewrites the branch/reset.ts comment to state the divergence from create: branch create polls for 15 minutes (measured 2–11.5 min in slow regions), while reset's 5-minute budget stands un-re-measured; the timeout value is untouched.
• Corrects DEVELOPMENT.md's files whitelist description to match package.json, which also ships dist/assets/forger.json and dist/assets/local/cli-overlay.yml.

Verification: lint, build, and the test suite (838 passed, 14 skipped) are identical to the untouched baseline; tsc --noEmit shows the same pre-existing 17 errors that are not a CI gate.

^{Written for commit 099b7f0. Summary will update on new commits.}

Summary by CodeRabbit

• Documentation

  • Updated package publishing guidance to include additional bundled assets while continuing to exclude source maps.
  • Clarified polling behavior and reset timeout guidance.

• Developer Compatibility

  • Several previously exported helper types, error classes, and constants are no longer available. No command behavior changes are noted in this release.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f1bfdc6a-e636-4913-8509-184f4eedd024

📥 Commits

Reviewing files that changed from the base of the PR and between ee30fbe and 099b7f0.

📒 Files selected for processing (9)

• DEVELOPMENT.md
• src/commands/branch/create.ts
• src/commands/branch/reset.ts
• src/commands/local/start.ts
• src/commands/local/stop.ts
• src/lib/errors.ts
• src/lib/guard/audit.ts
• src/lib/local/checkout.ts
• src/types.ts

 ________________________________________________________________________________
< A bunny is never late, nor is he early, he reviews precisely when he means to. >
 --------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Removes unused type exports and trims comments throughout.

The PR appears safe to merge; the latest change only clarifies an existing timeout comment.

Summary

Removes unused exports, shortens comments, and updates the package contents listed in DEVELOPMENT.md.

• Since the last review, only the timeout comment in reset.ts changed.
• The comment matches the timeout and measurements in create.ts.
• No new actionable issues were found.

_{Reviews (2) · Last reviewed commit: "chore(yun-tianming): drop unreferenced t..." · Reviewed by Greptile}

[jwfing]

Summary

Dead-export removal plus comment/doc-drift trimming across 9 files (+19/-87); every deletion and every rewritten claim checks out against the tree at 255a5fb, and the gates behave identically to base.

Requirements context

No docs/superpowers/ directory in this repo. docs/specs/ holds three dated design docs (2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md, 2026-04-17-db-migrations-command-design.md) — none covers a cleanup sweep, so no matching spec/plan found; assessed against the PR description, DEVELOPMENT.md, and the code itself. DEVELOPMENT.md §4 is the one convention document this PR directly touches.

What I verified (workspace at 255a5fb, base ee30fbe)

• Residual-reference sweep, grep -rnw over all 12 removed symbols (RefreshResponse, OrgMembership, ProjectAuthResponse, DatabasePasswordResponse, ConnectionStringResponse, ApiError, UploadDeploymentFileResponse, NotFoundError, PermissionError, getJsonFlag, __testing, AUDIT_FILE), excluding node_modules/dist/.git/lockfile: zero hits for all of them except AUDIT_FILE, whose only two hits are its own declaration and internal use in src/lib/guard/audit.ts:23,29 — i.e. exactly what the body claims.
• No public-API breakage: npm run build emits dist/index.d.ts at 13 bytes (export {}), so the src/types.ts re-export block is not a consumer-facing surface.
• Exit-code table still accurate (README.md:1384-1393): code 4 is still emitted at src/commands/domains/index.ts:510, code 5 at src/commands/projects/link.ts:390-396, src/lib/api/posthog.ts:252,311, src/lib/api/webscraper.ts:250,314, src/commands/webscraper/apify/connect.ts:208. Neither deleted class was ever constructed, so no caught-error path changes.
• branch/reset.ts:14 false-comment fix is correct: src/commands/branch/create.ts:24 is POLL_TIMEOUT_MS = 15 * 60 * 1_000, so "Match create's 5-min budget" was indeed false. The reset timeout value is untouched (reset.ts:16, still 5 min).
• DEVELOPMENT.md:91-97 now matches reality: package.json files is exactly ["dist/index.js","dist/index.d.ts","dist/assets/forger.json","dist/assets/local/cli-overlay.yml"].
• Surviving comments still carry their constraint and remain true: src/lib/errors.ts:38-46 still says "polls … every 3s for up to 15 minutes", consistent with create.ts:19,24; src/commands/local/stop.ts:108-110 ("telemetry reports once per process") matches the real outcomeRecorded guard at src/lib/command-telemetry.ts:86,103-104,129-130, and the throw still precedes both trackCommandUsage and the --json early return.
• start.ts __testing: parsePort/portOverrides/resolveStorage remain used internally (start.ts:58-82,160-161) and are referenced nowhere else, so dropping the export leaves no dangling symbol and no unused-var lint.
• Gates, head vs base: npx eslint src/ exit 0; npm run build success; npx tsc --noEmit output is byte-identical between 255a5fb and ee30fbe (26 lines / 17 errors, pre-existing, not a CI gate). npm run lint's vitest leg: 837 passed / 14 skipped with one failure in src/lib/cloudflare.test.ts:209 — EADDRINUSE 127.0.0.1:8787, which reproduces identically on the base commit in this sandbox, so it is an environment collision, not a regression from this PR. GitHub's own Lint & Build check was still in_progress when I reviewed; it should be green before merge.

Findings

Critical

(none)

Suggestion

• Functionality / comment accuracy — src/commands/branch/reset.ts:12-16. The PR correctly deleted the false parity claim, but the surviving line still reads "Same order of magnitude as create — minutes for a small DB, longer for a populated one" directly above a 5-minute budget, while create's is 15 minutes precisely because ap-southeast was observed taking 2–11.5 minutes (create.ts:19-24). A reader will still infer parity. Since the product decision is deferred, consider one more clause making the divergence explicit (e.g. "create uses 15 min for slow regions; reset has not been re-measured"), so the next sweep does not have to re-derive it.
• Software engineering — test hermeticity, src/lib/cloudflare.test.ts:~200-215. Out of this PR's scope, but worth filing alongside the other deferred items: the OAuth-callback test binds the fixed port 127.0.0.1:8787, so it fails with EADDRINUSE on any host where that port is taken (it did here, on base as well as head). Binding port 0 / injecting the port would make the suite robust outside CI.

Information

• Software engineering — no tests added or changed. Correct for this change shape: every deletion is a symbol with zero references, and the comment/doc edits are non-executable. The existing 838-test suite is the regression net, and it is unchanged from baseline.
• Security — no security-relevant changes. Removals only; no new user input reaches SQL/shell/HTTP, no secrets or PII newly logged, and no auth/authz path is weakened — PermissionError/NotFoundError were never constructed, and the exit-5 permission path in projects/link.ts:390-396 is untouched. No dependency changes.
• Performance — no performance-relevant changes. No new queries, loops, I/O, or allocations; deleting type aliases and comments is erased at build time. Bundle output is unchanged in shape (dist/index.js 678.70 KB).
• Deferred-findings list. As with previous sweeps, I read it as scope fencing rather than as findings to escalate — the PR deliberately fixes contradictions only. Two items in it look worth separate issues on their own merits (the posthog.ts missing unauthorized arm + > vs >= drift from webscraper.ts, and posthog/setup.ts:79-83 exiting before shutdownAnalytics() flushes), but neither is in this diff.

Verdict

approved — no Critical findings; the two Suggestions and the Information notes are non-blocking. (Informational only: the GitHub green check remains a human action, and Lint & Build should be confirmed green at 255a5fb first.)

[cubic-dev-ai]

No issues found across 9 files

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

[Fermionic-Lyu]

Thanks for the thorough verification.

Addressed

• src/commands/branch/reset.ts:13-17 — rewrote the surviving comment so the create/reset divergence is explicit: create uses 15 min because slow regions measured 2–11.5 min (pointer to create.ts); reset has not been re-measured so its 5-min budget stands. Dropped the "same order of magnitude as create" phrasing that still implied parity. Comment-only; POLL_TIMEOUT_MS value unchanged.

Declined

• src/lib/cloudflare.test.ts fixed port 127.0.0.1:8787 — agreed it is a hermeticity gap, but it is pre-existing and outside this PR's scope. It needs a small test change to bind port 0 (or inject the port) and read the bound address back; worth its own issue. Noted under Deferred findings.
• posthog.ts missing unauthorized arm / > vs >= drift and posthog/setup.ts:79-83 exiting before shutdownAnalytics() — as you read it, these are deferred behaviour defects, not changed here. They remain on the Deferred findings list for separate issues.

Verification (tree after the comment edit)

npx eslint src/        → exit 0
npx tsc --noEmit       → 26 lines / 17 pre-existing errors, identical to base ee30fbe
npm run build          → success, dist/index.d.ts 13 B
npx vitest run         → 79 files passed / 2 skipped; 838 tests passed / 14 skipped, 0 failed

The cloudflare test passed this run (port 8787 was free), confirming the earlier EADDRINUSE was an environment collision.

---

Yun Tianming, review round 1 of 10, pushed 099b7f0aea72.

[Fermionic-Lyu]

Addressed

• None required. cubic-dev-ai reported "No issues found" across the 9 changed files.

Declined

• None.

Verification

• npm test (vitest): 79 files passed, 2 skipped; 838 tests passed, 14 skipped.
• npx eslint src/: exit 0, no findings.
• npm run build (tsup, ESM + DTS): success.
• npx tsc --noEmit -p .: reports 17 pre-existing errors in files this PR does not touch (metadata.ts, config/apply.ts, config-capabilities.ts, prompts.ts, and five test files). None reference the symbols removed here (NotFoundError, PermissionError, getJsonFlag, the five unused interfaces). This is not a repository script and is unchanged from baseline.
• Working tree clean after build.

---

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

[jwfing]

LGTM - approved.

[jwfing]

Summary

Round 2 at head 099b7f0. The only change since my round-1 review (255a5fb) is the branch/reset.ts comment expansion, which addresses my round-1 Suggestion; its new factual claims check out against create.ts, the rest of the diff is byte-identical to what I already cleared, and Lint & Build is now green at this head.

Requirements context

Unchanged from round 1: this repo has no docs/superpowers/ directory. docs/specs/ holds three dated design docs (2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md, 2026-04-17-db-migrations-command-design.md), none covering a cleanup sweep — so no matching spec/plan found; assessed against the PR description, DEVELOPMENT.md §4 and the code itself.

What changed since round 1

git diff 255a5fb 099b7f0 is exactly one hunk, src/commands/branch/reset.ts:13-17 — the surviving "Same order of magnitude as create" line replaced with an explicit statement of the divergence. Verified:

• Claim "create uses 15 min" — src/commands/branch/create.ts:25, POLL_TIMEOUT_MS = 15 * 60 * 1_000. ✅
• Claim "slow regions were measured at 2–11.5 min (see create.ts)" — create.ts:20-24 reads "measured in MINUTES (2 min and 11.5 min on ap-southeast)". Faithful paraphrase. ✅
• Claim "reset has not been re-measured, so its 5-min budget stands" — reset.ts:18 still 5 * 60 * 1_000; the value is untouched by this PR, matching the body's "the timeout value itself is untouched". ✅
• The comment still leads with the constraint it exists to carry (what reset actually does: in-place pg_restore + schema-only truncate finalize), so it stays within the sweep's own "shrink narration to its constraint" rule even though this one grew by two lines.

Re-verification at 099b7f0

• Residual-reference sweep re-run over all 12 removed symbols (RefreshResponse, OrgMembership, ProjectAuthResponse, DatabasePasswordResponse, ConnectionStringResponse, ApiError, UploadDeploymentFileResponse, NotFoundError, PermissionError, getJsonFlag, __testing, AUDIT_FILE), excluding node_modules/dist/.git/lockfile: zero hits for eleven; AUDIT_FILE's only two hits are its own declaration and internal use (src/lib/guard/audit.ts:23,29) — i.e. precisely what the body claims.
• CI gate: Lint & Build = success at 099b7f0 (round 1 was still in_progress, so this closes the one open item from that review). Macroscope - Correctness Check skipped; the two AI reviewers were still running at post time.
• Everything else I verified in round 1 — dist/index.d.ts is 13 bytes (export {}) so no consumer surface is broken; the README exit-code table still backed by live new CLIError(…, 4) at domains/index.ts:510 and …, 5) at projects/link.ts:390-396, lib/api/posthog.ts:252,311, lib/api/webscraper.ts:250,314, webscraper/apify/connect.ts:208; DEVELOPMENT.md:91-97 matching package.json files exactly — is unaffected by the delta and still holds.

Findings

Critical

(none)

Suggestion

• Software engineering — test hermeticity, src/lib/cloudflare.test.ts:~200-215 (carried over from round 1, still unaddressed and still out of scope). The OAuth-callback test binds the fixed port 127.0.0.1:8787, so it fails EADDRINUSE on any host where that port is occupied — it did in my sandbox, identically on the base commit, and CI is green, so this is not a regression from this PR. Binding port 0 and injecting the port would make the suite robust off CI. Non-blocking; worth a separate issue alongside the deferred list.

Information

• Functionality — the measurement the new comment cites is attached to an arguably different constant upstream. create.ts:20-24 justifies POLL_TIMEOUT_MS (control-plane branch_state poll, consumed at create.ts:316) with a gap it describes as "branch_state reaching 'ready' and the branch's own host answering" — but that ready→serving wait is waitUntilServing/HEALTH_TIMEOUT_MS = 10 min (create.ts:28,285), not POLL_TIMEOUT_MS. reset.ts relays create.ts accurately, so nothing is wrong in this diff; the ambiguity is pre-existing in create.ts. Flagging only so a future sweep tightens it at the source rather than re-deriving it here.
• Software engineering — no tests added or changed, correct for this change shape: every deletion is a zero-reference symbol and the delta since round 1 is a comment. The 838-test suite is the regression net and is unchanged from baseline.
• Security — no security-relevant changes. Removals and comments only; no new user input reaches SQL/shell/HTTP, no secrets/tokens/PII newly logged or returned, no auth/authz path weakened (PermissionError/NotFoundError were never constructed; the exit-5 permission path in projects/link.ts:390-396 is untouched), no dependency changes.
• Performance — no performance-relevant changes. No new queries, loops, I/O or per-request allocations; type-alias and comment deletions are erased at build time.
• Deferred-findings list — read as scope fencing, not as findings to escalate. Two items still look worth their own issues on merit (lib/api/posthog.ts missing the unauthorized arm plus the > vs >= drift from webscraper.ts; commands/posthog/setup.ts:79-83 exiting before shutdownAnalytics() flushes), but neither is in this diff.

Verdict

approved — no Critical findings; one carried-over Suggestion and the Information notes are non-blocking. My round-1 Suggestion on reset.ts is resolved by this head, and the round-1 caveat about Lint & Build is now satisfied (green at 099b7f0). This review supersedes review 5442930109. (Informational only — the green checkmark remains a human action.)

[jwfing]

Summary

Round 3 at head 099b7f0 — the head has not moved since my round-2 review, so this is a re-verification pass: I re-ran the deletion sweep and re-graded the comment/doc claims from scratch against the current tree, and everything still holds (9 files, +22/−88, no executable change).

Requirements context

No matching spec/plan found — this repo has no docs/superpowers/; docs/specs/ holds three dated design docs (2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md, 2026-04-17-db-migrations-command-design.md), none of which covers a dead-code sweep. Assessed against the PR description alone.

Verification performed at this head

• Dead-symbol sweep — grep -rnw over all 12 removed names (RefreshResponse, OrgMembership, ProjectAuthResponse, DatabasePasswordResponse, ConnectionStringResponse, ApiError, UploadDeploymentFileResponse, NotFoundError, PermissionError, getJsonFlag, __testing, AUDIT_FILE), excluding node_modules/dist/.git/package-lock.json: 11 return zero hits; AUDIT_FILE returns only its own declaration and internal use (src/lib/guard/audit.ts:23,29), which is exactly what dropping the export {} statement leaves behind. No dangling reference.
• Public-surface check — dist/index.d.ts is export {} (13 bytes), so src/types.ts is not a consumer barrel; the type deletions cannot break the published package (package.json files = dist/index.js, dist/index.d.ts, dist/assets/forger.json, dist/assets/local/cli-overlay.yml).
• Exit-code table — the PR's claim that removing NotFoundError/PermissionError does not orphan exit codes 4/5 holds: both are still emitted inline via new CLIError(msg, 4|5) (src/commands/domains/index.ts, src/commands/projects/link.ts, src/lib/api/posthog.ts, src/lib/api/webscraper.ts, src/commands/webscraper/apify/connect.ts). README table stays accurate.
• Doc-drift fix — DEVELOPMENT.md:91-99 now lists exactly the four entries in package.json files. Byte-for-byte match.
• CI at 099b7f0 — Lint & Build success; cubic success, Greptile Review success, Macroscope - Correctness Check skipped. No red gate.

Findings

Critical

(none)

Suggestion

(none) — my round-1 Suggestion (the branch/reset.ts comment asserting "Match create's 5-min budget", which was false) was implemented in 099b7f0 and re-verified here: src/commands/branch/reset.ts:13-17 now states the divergence explicitly, and its two factual claims check out — create.ts:25 is 15 * 60 * 1_000, and the "2–11.5 min on ap-southeast" measurement is at create.ts:20-24. The timeout value itself (reset.ts:18, 5 min) is untouched, which is the right call for a comment-only sweep.

Information

Software engineering — no new tests, and none are warranted: every change is a deletion of an unreferenced symbol or a comment/markdown rewrite, so there is no new behavior to cover. The suite is the regression net here, and the PR reports it unchanged versus baseline (838 passed / 14 skipped); Lint & Build is green at this head independently. One note for anyone reproducing locally: npm run lint is vitest run --passWithNoTests && eslint src/, so a red vitest short-circuits eslint — run npx eslint src/ separately. (src/lib/cloudflare.test.ts:209 binds a fixed port 127.0.0.1:8787 and can fail EADDRINUSE on a shared machine; it fails identically on the base commit, so it is an environment artifact, not this PR.)

Functionality — the PR does what it claims and nothing more. src/commands/branch/reset.ts:13-17 faithfully relays create.ts's justification, but note that upstream justification is itself mis-attached: create.ts:20-24 explains the 15-minute POLL_TIMEOUT_MS using the branch_state: ready → host-answering gap, which is actually governed by HEALTH_TIMEOUT_MS = 10 min (create.ts:28,285). Pre-existing, out of scope for this sweep, and the new comment does not make it worse — flagging it so it is on the record.

Security — no security-relevant changes in this PR. Nothing added to SQL/shell/HTTP paths, no new user input, no auth or authorization check touched (NotFoundError/PermissionError were never constructed or caught; removing them changes no control flow), no secrets/tokens/PII newly logged or returned, and no dependency changes (package.json dependencies untouched).

Performance — no performance-relevant changes in this PR. No new queries, loops, allocations, or I/O; the only numeric constants in the diff's vicinity (POLL_TIMEOUT_MS in both create.ts and reset.ts) are unmodified.

Deferred findings — the PR body files a long list of real issues it deliberately did not fix (posthog.ts/webscraper.ts drift incl. the > vs >= retry off-by-one and the missing unauthorized arm; posthog/setup.ts:79-83 exiting before shutdownAnalytics(); backups/index.ts:25-32 missing the requireExplicit guard that projects/manage.ts:26-43 has for destructive ops; the CRLF-sensitive extractEnvPairs regex; double-emitting track*Usage wrappers; 17 ungated tsc --noEmit errors). Correctly scoped out of a comment-and-dead-code sweep, but several — particularly the backups ambient-INSFORGE_PROJECT_ID gap on delete/restore and the posthog unauthorized-retry window — are worth their own issues rather than living only in a PR body that will scroll away.

Verdict

approved — zero Critical findings; scope is tight, every deletion is independently verified unreferenced at this head, every rewritten comment preserves its constraint, and CI is green. (Informational: the explicit GitHub approval is a separate human action via the approve flow.)

[Fermionic-Lyu]

Thanks for the re-verification. No code changed this round; the head is still 099b7f0.

Addressed

• (none this round; the round-1 Suggestion on branch/reset.ts:13-17 was fixed in 099b7f0 and you re-verified it.)

Declined / acknowledged (informational points)

• create.ts:20-24 justification mis-attached to POLL_TIMEOUT_MS instead of HEALTH_TIMEOUT_MS — agreed it is pre-existing and out of scope. Correcting it is a comment-only edit, but I am not touching it this round because it reopens a file you have already approved and I would rather not grow an approved sweep. Added to Deferred findings for the next comment pass.
• Deferred list deserving its own issues (backups ambient INSFORGE_PROJECT_ID on delete/restore; posthog unauthorized-retry window) — agreed. Filing issues is outside what this PR can do unread; the findings stay in the PR body's Deferred list and are carried in my repo notes so they are raised as issues rather than re-discovered.
• vitest short-circuiting eslint in npm run lint — noted; I ran eslint separately below. No change warranted here.
• cloudflare.test.ts:209 fixed port — environment artifact, identical on base; not touched.
• Security / Performance / Tests — agree none are relevant to a deletion-and-comment sweep.

Verification (final tree, head 099b7f0)

npx eslint src/                   -> exit 0, no output
npx vitest run --passWithNoTests  -> 79 files passed / 2 skipped; 838 tests passed / 14 skipped (matches baseline)
npm run build                     -> ESM + DTS success; dist/index.d.ts 13 B (export {})
npx tsc --noEmit | grep -c error  -> 17 (pre-existing, ungated; unchanged from baseline)
git status --short                -> clean

---

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

[jwfing]

LGTM - approved.