chore(yun-tianming): correct stale doc comments, drop filename headers

Merged
FE
2d ago
+41-52

Summary

  • src/lib/errors.ts:54-56: the isTransientApiError doc claimed ossFetch does NOT wrap raw fetch rejections and that "the deployment poller" handles them. Since fbffb74, ossFetch tags transport failures as CLIError(NETWORK_ERROR_CODE) (src/lib/api/oss.ts:248-256), exactly like platformFetch (platform.ts:106,117). Comment now says both wrap.
  • src/lib/api/platform.ts:255-256: streamDiagnosticAnalysis doc said "Returns the raw Response so the caller can handle errors before streaming"; the signature is Promise<void> and the only caller (commands/diagnose/index.ts) uses onEvent alone. Sentence dropped.
  • src/commands/branch/create.ts:20-24: the POLL_TIMEOUT_MS comment attributed the 11.5-minute measurement to the ready→host-answering gap, which is governed by HEALTH_TIMEOUT_MS (10 min, line 25/282), and narrated the change from 5 to 15 minutes. Replaced with the one surviving constraint: must exceed the longest observed provisioning time (11.5 min on ap-southeast, InsForge/InsForge#1790, per commit 7fa4829's message), and what breaks below it.
  • src/auth-providers/apply.ts:124: "Exported for unit testing." sat above the non-exported MASKED_PASSWORD_PATTERN; moved to sit above refreshStaleEnvDefaults, which is the export apply.test.ts imports. Lines 367-373: a seven-line "Two passes" comment that restated refreshStaleEnvDefaults' own doc and narrated "after the CLI started fetching DATABASE_URL" shrunk to two lines.
  • src/commands/deployments/deploy.ts:321 and src/lib/api/platform.ts:88: dropped "now" change narration ("ossFetch now tags", "it is now the long-lived uak_").
  • Filename header comments (// CLI/src/...) removed from the eight config-family files: lib/config-{metadata,secrets,capabilities,schema}.ts, commands/config/{apply,export,plan,index}.ts. Nothing else in src carries one.
  • IMPORTANT: prose labels dropped from lib/config-capabilities.ts:14, lib/command-telemetry.ts:24, lib/flyctl.ts:104; the instruction text under each is unchanged.

Safety

  • All 15 files: comment edits only. git diff contains no non-comment line. Every surviving factual claim was checked against the code it describes (citations above).
  • The one declaration adjacent to an edit (MASKED_PASSWORD_PATTERN) did not move; only the comment line above it moved one line down.
  • No exported symbol, signature, string, default or import changed; dist/ untouched.

Verification

Baseline on untouched HEAD (e68f9c1), then final tree:

npx tsc --noEmit            # 17 errors on baseline (pre-existing, ungated) → 17 on final
npx eslint src/             # clean → clean
npx vitest run --passWithNoTests
                            # baseline: 79 files passed, 2 skipped; 838 tests passed, 14 skipped
                            # final:    79 files passed, 2 skipped; 838 tests passed, 14 skipped

npm run lint is vitest run && eslint src/; both halves were run separately so a red vitest could not mask eslint. Not run: test:integration:real (needs real-project cloud secrets, CI-only on main push), and npm run build (the diff is comments only; tsc covers type validity).

Deferred findings

Already filed in PR #308 body (not restated): backups/index.ts missing requireExplicit guard for delete/restore; api/posthog.ts/webscraper.ts retry > vs >= and missing unauthorized arm; posthog/setup.ts:79-83 exits before shutdownAnalytics; extractEnvPairs CRLF-sensitive; track*Usage double-emit.

New tonight, all behaviour changes or refactors without direct coverage, left alone:

  • src/commands/projects/link.ts:357-359: link --project-id X without --org-id skips the org picker (:332), leaves orgId undefined and writes it to config.default_org_id; saveGlobalConfig JSON-stringifies, so the user's saved default org is silently deleted.
  • src/commands/projects/link.ts:104-106: --auth validation throws outside the try, bypassing handleError; index.ts has no unhandledRejection handler, so a bad --auth prints a raw stack instead of Error: ... with the CLIError exit code. The sibling --template check at :109 is inside the try.
  • src/commands/functions/delete.ts:40-44 and functions/deploy.ts:87-89: a result.success === false response is printed through outputSuccess (✓ Failed to delete function), telemetry records success, exit code 0. deploy.ts:87 also prints the non-word "updation".
  • src/commands/orgs/manage.ts:276: orgs members remove resolves the org from ambient INSFORGE_ORG_ID/linked project, while leave (:120) and delete (:154) use requireExplicitOrgId. With --yes a stray env var points the eviction at the wrong org. Same shape as the backups finding in #308.
  • src/commands/secrets/update.ts:24-25, src/commands/schedules/update.ts:32: opts.active === 'true' turns --active yes/TRUE/1 into false with no error; link.ts:86-89 normalises the same kind of flag.
  • src/commands/docs.ts:31: reportCliUsage('cli.docs', true) runs before any work and the catch never reports false; every sibling reports after output and false on failure.
  • src/commands/records/{list,delete,update}.ts: opts.filter.split('=') computed twice per call, triplicated; a filter with no = becomes ?id= and is sent as a DELETE/PATCH predicate. No records tests exist, so the redundant split was not collapsed.
  • src/lib/api/platform.ts:110-112: when getAccessToken() was empty, line 73 already minted a token; a 401 then calls refreshAccessToken again, spending a second round trip and rotation on a seconds-old token.
  • src/lib/api/platform.ts:119-127 vs :134-141: identical error-body parsing written twice inside platformFetch, already drifted in naming. No platform.test.ts exists, so not collapsed.
  • src/lib/api/platform.ts:274-293: SSE currentEvent is never reset after a dispatch, so a bare data: frame after event: error is delivered as error.
  • src/lib/api/posthog.ts:9-24 vs webscraper.ts:9-27: copy-pasted fetchWithTimeout drifted; only webscraper has the pre-abort guard, so pollPosthogConnection issues one more request after the user aborts. sleep, PollOptions, both pollers and both cli-start functions are byte-identical duplicates across the two files. Cross-module; not collapsed.
  • src/lib/api/posthog.ts:306, webscraper.ts:309: body.error ?? res.statusText ?? \HTTP ${status}`—statusText is always a string (''` on HTTP/2), so the last arm is unreachable and an HTTP/2 error yields an empty message.
  • src/lib/api/posthog.ts:91-110: comment says null "strictly" means no connection, but any CLIError whose message contains the substring 404 is swallowed to null, and a real connection lacking apiKey also returns null.
  • src/lib/api/oss.ts:341: res.clone().json() where res is never otherwise read; res.json() is equivalent. probeBackendHealth is only ever mocked in tests, so left as is.
  • src/commands/deployments/deploy.ts:331: the formatFetchError branch is unreachable because isTransientApiError returns false for every non-CLIError; removing it needs a cast or a type-predicate signature change, so left.
  • src/commands/projects/link.ts:250-256 vs :311-317: byte-identical "agent connected" block in both direct-link branches; :552-558 awaits shutdownAnalytics() in both catch and finally (the finally copy is unreachable after handleError's process.exit).
  • src/commands/branch/create.ts commit 7fa4829's message is internally inconsistent about which phase the 11.5-minute measurement belongs to (point 1 says branch_state read ready throughout; point 2 says it was reported "still creating"). The new comment cites the issue rather than the phase.

Security findings

Opened by Yun Tianming: nightly sweep 2026-10-22-1300, model claude-fable-5-1, 58 turns, 15 files, 48 changed lines.
Verification: tsc 17 errors (= baseline, pre-existing, ungated); eslint src/ clean; vitest 838 passed / 14 skipped (= baseline); integration:real not run (needs cloud secrets).

🤖 Generated with Claude Code

Summary by cubic

Fixes stale, misleading, or redundant documentation comments across the CLI and removes filename header comments from the eight config-family files. Comment-only; no code behavior changes.

  • Corrects the doc claiming ossFetch does not wrap fetch rejections (it tags transport failures as CLIError, like platformFetch) and removes the false claim that streamDiagnosticAnalysis returns a raw Response (its signature is Promise<void>).
  • Moves the "Exported for unit testing." comment above the export it refers to, and trims a seven-line comment that restated the function's own doc.
  • Adds a note that the formatFetchError arm in the deployment poller is unreachable since ossFetch wraps raw fetch rejections.
  • Drops change narration ("now") and IMPORTANT: labels from comments; instruction text is unchanged.
  • Restores the original POLL_TIMEOUT_MS comment in src/commands/branch/create.ts after review, and adds a one-line pointer on HEALTH_TIMEOUT_MS to the sourcing issue.

No exported symbol, signature, string, default, or import changed. npm run lint, npm run build, and targeted vitest runs pass; tsc --noEmit reports only 17 pre-existing, ungated errors in untouched files.

Summary by CodeRabbit

  • Documentation
    • Updated internal code comments to clarify authentication, provisioning timeouts, deployment polling, error handling, and credential redaction.
    • Removed file-path headers and revised emphasis in several technical notes.
    • No user-facing behavior changed.