chore(yun-tianming): shrink change-narration comments to their constraints

Closed
FE
1mo ago
+54-76

Summary

  • Comment narration shrunk to the surviving constraint in ten files. Each edited comment told the story of a past bug or a past code shape (“used to”, “previously working”, “the fix”, “was the actual gap”, “Neon is no longer used”, “since 0.0.38”, “moved in the command re-organization”) around a rule the code still obeys. The rule stays; the story goes.
    • src/commands/postgres.ts — dbStats header: dropped the retired-Neon narrative, kept “never dialed” and “nothing here polls”.
    • src/commands/compute.ts — computeSSH: two comments (check-before-mint ordering; config block re-rendered on every store write) reduced to the ordering and re-render constraints.
    • src/commands/cron.ts — cronEditWarnings: removed the “Silence here was the actual gap” sentence; the body-loss rule stays.
    • src/commands/auth.ts — logout doc: kept why it must use the persisted config; removed “made this actively unsafe … The revoke now always goes”.
    • src/config.ts — pickApiUrl guard comment and findProjectRoot stop-at-home comment rephrased as present-tense constraints.
    • src/commands/deploy.ts — discoverLane validation comment and the --port parser comment reduced to the rule.
    • src/commands/ssh-config.ts — dropped “it also drops the : the old token carried”.
    • src/commands/upgrade.ts — skipsUpdateCheck body comment reduced to the rule.
    • src/commands/env.ts — removed the “since 0.0.38” version reference.
  • Misplaced comments moved to the function they describe (documentation drift).
    • src/commands/upgrade.ts — the “Called once at CLI start-up. Never blocks…” comment sat above skipsUpdateCheck (inserted later, per git blame); it describes maybeUpdate and now sits there.
    • src/commands/setup.ts — two paragraphs describing defaultAsk and openPromptInput sat above makePromptSource; each now precedes the function it describes.

Safety

  • Every hunk is a comment edit. git diff | grep '^[+-]' | grep -v '^[+-]\s*//\|^[+-]\s*\*\|^[+-]\s*/\*\*' (excluding file headers) prints nothing: no code line changed. No exported symbol, signature, string literal, or behaviour is touched.
  • The three comment moves in upgrade.ts and setup.ts are documentation drift fixes: git blame shows the functions were inserted between a comment and its subject after the comment was written.
  • Typecheck and the full vitest suite pass with the same result as the untouched baseline.

Verification

Baseline on the untouched tree, then again on the final tree:

npx tsc -p tsconfig.json --noEmit        # exit 0 both times (~4s)
npx vitest run                           # 85 files passed, 1673 tests passed, 121 skipped, both times (~25s)

Not run here: the 103 SSH-certificate tests in test/ssh-orchestration.test.ts and the ssh -G effective-config tests in test/ssh-config.test.ts skip because this machine has no ssh/ssh-keygen (they skip identically on the baseline). The remaining Windows-only skips are platform-gated. No lint script exists in package.json; CI runs only typecheck + vitest.

Deferred findings

  • src/util.ts promptPassword: the echo-mute sets _writeToOutput on process.stdout, but readline’s Interface uses its own (symbol-keyed) method, so the override is a no-op and the password echoes. Reported as a security finding; the fix changes user-visible behaviour.
  • src/commands/postgres.ts: the branch/group URLSearchParams block is repeated verbatim in dbAlwaysOn, dbLimits, dbStats, dbVolume. Not refactored: those handlers are not exercised by tests (only their pure seams are — limits.test.ts, volume.test.ts, db-stats.test.ts).
  • src/commands/metrics.ts and src/commands/storage.ts: identical private qs() helpers. Collapsing means one module importing from another (or a new util export), which is a cross-module move; left for the owners.
  • src/commands/compute.ts (2082 lines): lines ~800–2060 are SSH client infrastructure (key paths, alias store, cert staging, locks, config block) unrelated to compute lifecycle/domain commands, while ssh-config.ts already exists. A file doing two jobs; moving code is out of scope.
  • src/telemetry.ts detectAgent vs src/agent.ts detectAgent: two same-named detectors with drifted rules (truthy CLAUDECODE + CURSOR_TRACE_ID vs CLAUDECODE === '1' + CURSOR_AGENT === '1' + Codex). Which is right is a product decision.
  • src/commands/secrets.ts ensureIgnored vs src/gitignore.ts ensureGitignore: two gitignore writers with different rules (.git presence check, header line). Similar, not identical, so not collapsed.
  • src/commands/secrets.ts: secrets, secretsBind, secretsUnbind, secretsBindings, secretsSources interpolate branch into output and query strings without a fallback; when neither --branch nor a linked branch is set they print or send the literal undefined. Behaviour change to fix.
  • src/commands/project.ts resolveProjectName: splits cwd on / only, so on Windows a backslash path is slugified whole (e.g. c-users-me-projects-app) instead of using the directory basename. Defect; fix is a behaviour change.
  • src/commands/setup.ts local cliVersion duplicates src/version.ts cliVersion with a different fallback ('latest' vs '0.0.0') and without the INSTA_CLI_VERSION check. Not identical, so not collapsed.
  • src/commands/govern.ts: approvalsList and events build query strings without encodeURIComponent on --status and --limit, unlike the rest of the repo. Cosmetic inconsistency; user-controlled input only.
  • Export hygiene: a background scan found 134 exported symbols referenced only within their own file and 205 exported solely for tests; none is unreferenced. The repo’s “pure seam, exported for tests” convention is deliberate, so nothing was removed.

Security findings

Opened by Yun Tianming: nightly sweep 2026-09-23-1000, model claude-fable-5-1, 75 turns, 10 files, 98 changed lines.
Verification: Passed: npx tsc -p tsconfig.json --noEmit (exit 0) and npx vitest run (85 files, 1673 passed, 121 skipped — identical to baseline); SSH-certificate suites skip here for lack of ssh/ssh-keygen.

🤖 Generated with Claude Code

Summary by cubic

Rewrites code comments across ten files so they describe the rules the code enforces, not the history of past bugs or refactors that led to those rules.

  • Shrinks change-narration comments in auth.ts, compute.ts, cron.ts, deploy.ts, env.ts, postgres.ts, ssh-config.ts, upgrade.ts, and config.ts to the surviving constraint (e.g., the "Neon no longer used" history in postgres.ts, the "since 0.0.38" version reference in env.ts, the "was the actual gap" story in cron.ts).
  • Round 2 trimmed the residual narration in computeSSH, restored the dropped "carries no :" fact in ssh-config.ts, and unified mixed tense in auth.ts.
  • Moves three comments in upgrade.ts and setup.ts to sit above the functions they describe (maybeUpdate, openPromptInput, defaultAsk); two fix drift from later inserts, the third was misplaced from the start.

Comment-only change: no code lines, symbols, or behavior are touched, and typecheck plus the full vitest suite pass identically to the untouched baseline.