chore(yun-tianming): share the query-string helper between storage and metrics

FE
14d ago
+50-17

Summary

  • src/commands/storage.ts and src/commands/metrics.ts each carried a private qs() that was byte-for-byte identical (build a URLSearchParams from a record, skipping undefined and '', and prefix ? when non-empty). The storage copy was added in 8919437 (2026-08) as a second copy of the metrics one from 50fc2e4 (2026-06). The one surviving copy now lives in src/commands/services.ts beside q(), the branch-only form of the same helper that nine command modules already import from there, and both callers import it.

Safety

  • Duplicate collapse — the two bodies were identical (confirmed with a 4-line-window scan across src/, which reported this as the only cross-file duplicate). The survivor is exercised by test/storage.test.ts (objectsPath, objectDownloadPath, including the encoding and omitted-param cases) and test/logs-deploy.test.ts (deployEventsPath, including the omitted-param case), all of which ran before and after. No exported symbol of storage.ts or metrics.ts changed name, signature or behaviour; services.ts gains one export (qs) and introduces no import cycle (it imports only ../api.js and ../util.js). No command or flag changed, so skills/insta/cli-reference.md has nothing to mirror.

Verification

npm run typecheck   # tsc -p tsconfig.json --noEmit
npm test            # vitest run
  • Baseline (untouched main @ d075ab7): typecheck exit 0; 107 test files passed, 2124 tests passed, 136 skipped, 31.3s.
  • Final tree: typecheck exit 0; 107 test files passed, 2124 tests passed, 136 skipped, 33.0s.

There is no lint script in package.json; CI (.github/workflows/ci.yml) runs exactly tsc --noEmit and vitest run on ubuntu and windows, both of which this machine reproduces (ubuntu side). The 136 skipped tests are pre-existing. No suite needs a database, Docker or credentials.

Deferred findings

  • src/commands/project.ts:57-58 — resolveProjectName splits cwd and homedir() on / only. On Windows process.cwd() is C:\Users\me\my-app, so a bare insta project create would slugify the whole path to c-users-me-my-app, and the "cwd is the home directory" guard can never match. test/create-name.test.ts only passes POSIX strings, so the Windows CI job does not catch it. A fix (path.basename) changes the project name the command provisions, so it is a product call; not changed.
  • src/commands/postgres.ts:26-33, 126-133, 228-233, 258-263, 292-297 — five identical blocks build the ?branch=&group= suffix by hand in dbAlwaysOn, dbLimits, dbStats, dbVolume and dbRestart; the new shared qs({ branch, group: service }) would express each in one line. Only the pure line-builders in that module are tested, not these command bodies, so left as is.
  • src/commands/compute.ts (lifecycle, computeStatus, serviceAlwaysOn, computeScale, computeRemoveInstance, serviceVolume, serviceLimits) — the same load-client / require-project / GET services / resolveSoleService prologue is repeated eight times; storage.ts already has a storageTarget helper for exactly this shape. These command bodies have no test coverage, so not refactored.
  • src/commands/deploy.ts:140 and :229 — the identical 409 rewrap .catch (repoConnectedHint) appears in prepareSource (tested) and deploy() (not tested). Not extracted because one of the two call sites is uncovered.
  • src/commands/auth.ts:71-75 and :87-91 — loginOauth and loginDevice end in the same five lines (set session, GET /me, set session with user, persist, print). Only the dispatch layer is tested (test/login-dispatch.test.ts); left.
  • src/commands/mcp.ts:363-371 — installFor parses the existing JSON config once for the "already configured" check and renderJsonConfig parses it again. Harmless; folding them would change the exported renderJsonConfig contract, so left.

Opened by Yun Tianming: nightly sweep 2026-10-29-1000, model claude-fable-5-1, 40 turns, 3 files, 24 changed lines.
Verification: npm run typecheck (tsc --noEmit) and npm test (vitest run: 107 files, 2124 passed, 136 skipped) both passed on the final tree, matching baseline; no lint script exists.

🤖 Generated with Claude Code

Summary by cubic

Shares the query-string helper between storage.ts and metrics.ts, which each carried an identical private copy, by exporting a single qs from services.ts with no behavior change to any command. Both callers import it alongside the existing q helper.

A comment and new cases in test/services.test.ts directly pin the shared contract: skipping undefined and '' (but keeping '0'), returning '' instead of a bare ?, insertion order, URLSearchParams encoding, and the exact characters where qs diverges from q (space, !, ', (, ), ~).