feat(create): accept --build-timeout with a unit suffix

[piscisaureus]

--build-timeout now takes 10m or 600s as well as a bare number,
matching the duration syntax of deno.json deploy.buildTimeout.

• a value without a unit keeps meaning minutes and converts as the
  flag's former numeric type did (10, 10.0), so existing scripts are
  unaffected; unlike deno.json, where a bare number is seconds, because
  changing it here would turn --build-timeout 10 into ten seconds
• the value must still come out as one of the available steps (5 to 30
  minutes); anything else is rejected with the list of valid values

[avocet-bot]

Review: #147 — feat(create): accept --build-timeout with a unit suffix

Reviewed head SHA: ad2222951d0acdf199f8a62accdc86cfc6f2b83f
Verdict: Approve — no blocking issues.

Review skill evidence

This review was produced via the Claude Code pr-review-toolkit:code-reviewer subagent (invoked through the Task tool), which independently reasoned over the diff and verified the floating-point and regression concerns below. Its conclusions matched my own independent analysis (including a direct numerical check of the parser against every valid seconds spelling). No manual-only fallback was needed.

What this PR does (background for a new reader)

deploy-cli is the command-line tool for creating and deploying Deno Deploy apps. Its create command accepts a --build-timeout flag that controls how long a build is allowed to run before being killed. The platform only permits a fixed set of discrete values — 5, 10, 15, 20, 25, or 30 minutes (AVAILABLE_BUILD_TIMEOUTS) — so the flag's job is to parse user input and reject anything that is not exactly one of those steps.

Historically the flag was typed as a number of minutes: --build-timeout <minutes:number>. The companion deno.json configuration key deploy.buildTimeout was recently taught to accept duration strings with unit suffixes ("10m", "600s"). This PR brings the CLI flag closer to that syntax so users can write --build-timeout 10m or --build-timeout 600s, while preserving the flag's long-standing behavior that a bare number means minutes.

Key control/data flow of the change:

• A new pure function parseBuildTimeoutFlag(value: string): number | null is added in deploy/create/flow.ts. It returns the timeout in minutes if the input resolves to one of the available steps, otherwise null.
• The Cliffy option definition in deploy/create/mod.ts changes from <minutes:number> to <duration:string>. Its value() callback now calls parseBuildTimeoutFlag, returns the parsed minutes on success, and throws a ValidationError (with the list of valid values) on failure.
• Because value() still returns a number of minutes, every downstream consumer is unaffected — the external contract of the option is unchanged.
• AGENTS.md help text and tests/build_timeout.test.ts are updated accordingly.

Intended parsing rules

• Input with a unit suffix must be a positive integer followed by s, m, or h: regex ^([1-9][0-9]*)([smh])$. It is converted to minutes via DURATION_UNIT_MINUTES (s: 1/60, m: 1, h: 60).
• Input without a unit is converted exactly as the former numeric type did, via Number(value), so legacy spellings like 10 and 10.0 keep working.
• In both cases, the result is accepted only if AVAILABLE_BUILD_TIMEOUTS.includes(minutes) (strict ===/SameValueZero membership).

Findings

No blocking issues, no correctness bugs, no regressions. I examined the areas most likely to hide a defect:

1. Floating-point exactness (verified SAFE — the highest-risk item)

Converting seconds to minutes multiplies by 1/60, which is not exactly representable in IEEE-754 double precision. Because acceptance uses Array.prototype.includes (SameValueZero, i.e. ===), the product must equal the integer step bit-for-bit, not merely approximately. I computed the actual double result for every seconds spelling that should map into [5,10,15,20,25,30]:

• 300s → 5, 600s → 10, 900s → 15, 1200s → 20, 1500s → 25, 1800s → 30

Every product rounds back to the exact integer double, so includes succeeds for all of them. There is no silent membership miss. (This is fragile in principle — if the step list or unit factors ever change, exactness should be re-verified — but it is correct as written.)

2. :number → :string migration (not a regression)

Odd inputs that lack a unit (" 10", "1e1", "0x1e", "Infinity", "") take the match === null branch and go through Number(value) followed by the same includes gate. That is exactly what Cliffy's :number type did before (it also used Number()/finiteness and then the callback's includes check). So nothing that previously parsed stops parsing, and nothing previously rejected becomes accepted. The new path is equal-or-stricter, never looser. No behavioral regression.

3. Downstream type preserved (SAFE)

value() returns minutes as a number on every accepted branch, matching the option's prior numeric return. The interactive (prompt-based) code path is untouched and continues to use AVAILABLE_BUILD_TIMEOUTS directly. Consumers still receive a number of minutes.

4. Test coverage (adequate for the risk surface)

The added test locks in the boundary seconds value (600s), the max step (30m), the legacy-number quirks (10.0 → 10, 05 → 5), and the important rejections (90s, 1h, 0, 10min, 1.5m, "", -5, 05m). This covers the real risk well. The only untested corner is the hex/exponent passthrough (0x1e, 1e1), which is pre-existing behavior inherited from the old :number type and not introduced by this PR.

Nits (no action required)

• Dead h unit. h can never yield a value in [5,30] (minimum 1h = 60), so any h input is always rejected. The help text correctly omits h from its examples. Harmless; could be dropped from DURATION_UNIT_MINUTES for tidiness.
• Error-message example uses AVAILABLE_BUILD_TIMEOUTS[1] twice → "e.g. 10 or 10m". This reads as an intentional illustration of the two forms (bare number vs. unit suffix) of the same value rather than a copy-paste error; using [0]/[1] ("5 or 10m") would be marginally clearer but is not worth a round-trip.
• Leading-zero asymmetry: "05" → 5 is accepted (no-unit Number() path) while "05m" → null is rejected (regex requires a leading [1-9]). This is intentional — the no-unit path deliberately preserves the former numeric behavior — and is explicitly pinned by the tests.

Conclusion

The change is small, backward-compatible, and well-tested. The one genuinely fragile aspect (float exactness of the seconds→minutes conversion) is correct for all reachable inputs. Approving.