chore(yun-tianming): share template poll reader, hoist cron refs lookup

Merged
FE
4d ago
+15-12

Summary

  • src/commands/template.ts: the deployment poll closure passed to watchDeployment was written out three times (deploy, upgrade, rollback), each building the same GET /template-deployments/:id request with the project id attached, and two of the three carried the same comment explaining why the project id must ride along. The three copies now share one module-local readDeployment(api, projectId) and the constraint lives in one comment above it. The upgrade path's remaining comment now describes only the preview GET it still sits above.
  • src/commands/cron.ts: jobShowLines called refsOf(j.request) once for the rows and again inside the headerNames.filter callback, per header. The refs are now read once into a local and used for both.

Safety

  • template.ts — duplicate collapse / pure refactor inside one module. The three closures were byte-identical apart from the captured api and p.projectId, which the helper takes as parameters; no exported symbol changed name, signature or behaviour. test/template.test.ts asserts the exact poll path and that the request carries projectId (lines 720 and 753), and test/template-upgrade.test.ts exercises the upgrade and rollback poll paths; all pass before and after.
  • cron.ts — redundant work removed. refsOf is r.secretRefs ?? {}, so every call returns the same object by construction; hoisting it changes nothing observable. test/cron.test.ts describe('jobShowLines') covers literal-only, mixed, and secret-only header rows; passes before and after.

Verification

npm run typecheck      # baseline: exit 0 (~4s)      final: exit 0
npm test               # baseline: 106 files, 2109 passed, 135 skipped (~35s)
                       # final:    106 files, 2109 passed, 135 skipped

Both gates run on this machine; the suite uses dependency-injected fakes and needs no database, Docker, or network. There is no lint script in this repository. No test failed or flaked in either run.

Deferred findings

  • src/deploy-archive.ts:96 defines a local sleep while the module already imports setTimeout as delay from node:timers/promises. Replacing sleep with delay is not behaviour-preserving under test: vitest's fake timers do not intercept the promise-based timer, so test/deploy-archive-logs.test.ts:48 (polls expected at 0/3000/6000 ms) fails with delay as the default wait. Tried, reverted with git restore. Collapsing it would need the test to inject wait, which is a test-design decision for the owners.
  • src/commands/template.ts:462,617,672 repeat res.body.deploymentId ?? (res.body.deployment ?? res.body).id. Left in place: a shared helper needs a declared parameter type, and the expression currently relies on the any-typed response body to be assignable to watchDeployment's string parameter. Not a defect.
  • src/commands/deploy.ts:19-29 deployRequestBody re-runs parseEndpoint on a value deploy() already normalized at line 189. Redundant but harmless; the function is exported and unit-tested standalone on raw option strings, so the re-parse is part of its contract. Left.
  • Comments narrating pre-fix behaviour remain at src/commands/compute.ts:2061-2068 ("used to be rewritten only by --setup …") and src/deploy-archive.ts:52-55 ("Keying on the tar digest instead bought …"). Each also states what breaks without the current code, and my earlier comment-shrinking PR (#286) went three rounds without approval, so these are left for a human to judge.
  • Unexported-elsewhere symbols: a scan of every export in src/ found none unreferenced; every candidate is used inside its own file. No dead code to remove.

Filed, not fixed here

Opened by Yun Tianming: nightly sweep 2026-10-17-1000, model claude-fable-5-1, 55 turns, 2 files, 27 changed lines.
Verification: npm run typecheck (exit 0) and npm test (106 files, 2109 passed, 135 skipped) both pass on the final tree, identical to baseline.

🤖 Generated with Claude Code

Summary by cubic

Deduplicates the deployment poll closure in template.ts into a single readDeployment helper, and hoists the refsOf call in cron.ts so it runs once instead of per header. No behavior changes; all tests pass before and after.