Distillation

<observations> Date: July 21, 2026

🟡 (10:14) Test run: cloudflare.test.ts — 25/25 tests passed (1.62s duration).
🟡 (10:15) Verification: tsc --noEmit exit 0 (clean). Lint: 7 problems, 0 errors, 7 warnings, including 228:12 warning '_err' is defined but never used @typescript-eslint/no-unused-vars.
🟡 (10:17) Full test suite run: 58 test files passed, 1050 tests passed, 1 skipped (1051 total), 34.67s duration.
🟡 (10:17) Docs build succeeded: pagefind search index built (293ms), sitemap-index.xml created, 28 pages built in 8.83s, exit 0.
🟡 (10:27) Adversarial subagent review (task id ses_07bd1397cffe6sAzPv1i7n5z2G) completed on PR #843 follow-up branch. Verification repeated in review: tsc clean, cloudflare.test.ts 25/25, full suite 1050 passed/1 skipped, no regressions. Confirmed Node baseline volta.node = 24.18.0 → global fetch available, no polyfill/new dependency, raw fetch used (no CF SDK).
🟡 (10:27) Review section A: traced publish() (cloudflare.ts:343) — dry-run check at line 378 returns before resolveProductionBranch() (lines 393-396, the only fetch() caller) and before spawnProcess (line 418). Confirmed: dry-run makes zero network calls (critical invariant holds). Noted minor non-bug: dry-run preview omits --branch when branch is unconfigured/inferred, so dry-run output isn't byte-exact vs. real run when inference is in play.
🟡 (10:27) Review finding MINOR-1: the config-time ${VAR} expansion guard (cloudflare.ts:175-186, regex /^\$\{.*\}$/) protects config-sourced projectName/productionBranch/workingDir from being expanded against env (which includes CLOUDFLARE_API_TOKEN) by spawnProcess, but does NOT cover the API-inferred production branch pushed into argv at line 262. Recommended fix: apply same check to resolved branch in resolveProductionBranch(), treating a match as unresolved (return undefined).
🟡 (10:27) Review finding MINOR-2: token-leak surface in catch block (cloudflare.ts:327-332) reviewed — verdict "no token leak": !response.ok path throws HTTP ${status} ${statusText} (no token); network/DNS errors via undici don't include headers; token only ever in Authorization header, never in argv or logs.
🟡 (10:27) Review finding MINOR-3: failed branch inference (cloudflare.ts:310-334) silently degrades to omitting --branch (wrangler defaults to production) on non-2xx/missing-field/network error. Recommendation: distinguish 404 (project-not-found, real misconfig → should hard-fail via reportError) from transient 5xx/network blips (keep soft-fail warn+continue).
🟡 (10:27) Review finding MINOR-4: no real defect — test coverage assessed as strong, covering token-in-header, correct URL, API-failure fallback (403), no-account-id fallback, worker-never-calls-API, account-id-forwarded-only-when-set, config-branch-skips-API, dry-run-no-fetch.
🟡 (10:27) Review B/C/D/E findings: encodeURIComponent correctly applied to accountId and projectName (lines 303-304) — no URL injection. response.json() parse errors caught in try (line 327). Default flip to worker — grep clean, no other code assumes old pages default. projectName still required only for pages deployType (lines 165-169); bare {name:'cloudflare'} = worker needs no projectName. accountId and token both read at construct-time (lines 201, 212) — consistent, tests correctly isolate via beforeEach/afterEach env reset; || undefined treats empty-string env as unset. ${VAR} guard's typeof value === 'string' check (line 180) skips undefined safely. Quoted: "accountId not guarded against ${VAR} — correct by construction: it comes from process.env, never from config, so it can't be a config-injected expansion" and "it's forwarded via env (line 412), not argv, so replaceEnvVariable never touches it." Worker-never-calls-API confirmed: resolveProductionBranch() gated on deployType === 'pages' at lines 393-396; quoted: "Test worker deploy never calls the Pages API proves it." Docs reviewed as accurate (worker default note lines 10-12, optional account id line 29, 3-step branch resolution lines 37-41, no-OIDC note line 32, Account → Cloudflare Pages → Edit scope).
🟡 (10:27) Review verdict: "Solid" — no CRITICAL/MAJOR issues; dry-run invariant preserved; no token leakage; no new dependency; tests meaningful. MINOR-1 recommended as worth fixing before merge (cheap defense-in-depth); MINOR-3 valid but non-blocking.
🟡 (10:27) Assistant decided to apply both MINOR-1 and MINOR-3 fixes: extract ${VAR} regex check into reusable constant (ENV_EXPANSION_REGEX), apply to API-inferred branch, and distinguish a 404 (project-not-found) as a hard error via reportError while keeping soft-fail (warn+continue) for transient errors.
🟡 (10:29) Assistant noted (uncertain) that reportError throws in test context based on prior "no artifacts found" test expecting rejects.toThrow; planned that a 404 hard-fail via reportError inside publish() would propagate as a rejection.
🟡 (10:29) Added test for 404 hard-fail: "hard-fails when the Pages project is not found (404)" expecting rejection matching /Pages project "missing-project" not found/. Initial test run: 1 failed, 26 passed (27 total) — AssertionError: promise resolved "undefined" instead of rejecting.
🟡 (10:30) Debugging: initial hypothesis was that reportError doesn't throw in this test context. Actual root cause found: the reportError call for the 404 case was placed INSIDE the method's own try block, so the error it threw was caught by that same method's catch and swallowed into a warning instead of propagating. Fix: restructured resolveProductionBranch() so the try only wraps the network fetch + JSON parse (returning a result), and

Distillation

Observations