BREAKING: canonical --timeout everywhere; remove prepare --force alias#66
Open
NicolasSchuler wants to merge 1 commit into
Open
BREAKING: canonical --timeout everywhere; remove prepare --force alias#66NicolasSchuler wants to merge 1 commit into
NicolasSchuler wants to merge 1 commit into
Conversation
Clean-break CLI-contract convergence (approved; no deprecation aliases): - Every per-command "give up after this long" flag is now `--timeout` taking a DURATION string (30s, 5m, 1h, or bare seconds), with each command's default unchanged: test (was --wait-timeout + hidden --timeout alias; 180s), notebook (was --ready-timeout; 10m), germinate (was --pending-timeout; 30m), and the doctor mpi-smoke/fabric-smoke/readiness subcommands plus the hidden legacy doctor flag interface (were --timeout-seconds taking raw u64 seconds; default 300 -> "5m", same duration). when and sweep observe already used --timeout. Old spellings are removed outright; error messages updated to the new names. - Remove the hidden deprecated `prepare --force` alias for --force-rebuild (definition, dispatch branch, stderr warning, test fixture). `prepare --force` is now an unknown-flag error (clap suggests --force-rebuild; pinned by a new test), so --force means only "overwrite the output file" (new/evolve) across the whole CLI. - CHANGELOG: BREAKING entries under [Unreleased] (### Changed with a before->after table, ### Removed for the alias). Docs updated (cli-reference.md, notebook.md) and the six affected man pages regenerated; gen-manpages --check passes after regen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes intentional, breaking CLI contract changes by standardizing all “give up after this long” flags to a single canonical --timeout <DURATION> spelling and fully removing the deprecated prepare --force alias, then updates tests, docs, changelog, and man pages accordingly.
Changes:
- Canonicalize per-command timeout flags to
--timeout <DURATION>(including migrating doctor subcommands from--timeout-secondsto--timeout). - Remove the deprecated hidden
prepare --forcealias so--forceonly means “overwrite output file” fornew/evolve. - Regenerate/update user-facing docs (mdbook + manpages) and adjust integration tests to pin the new CLI behavior.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cli_runtime.rs | Updates notebook timeout tests to use --timeout and expect the new flag name in errors. |
| tests/cli_init.rs | Migrates doctor CLI integration tests from --timeout-seconds N to --timeout Ns. |
| tests/cli_canary_rendezvous.rs | Switches germinate tests from --pending-timeout to --timeout and updates expected error strings. |
| tests/cli_cache.rs | Adds a regression test asserting prepare --force is now rejected and suggests --force-rebuild. |
| src/spec/mod.rs | Updates parse_short_duration docs to reference notebook --timeout. |
| src/output/tests.rs | Removes now-deleted force_deprecated field from a prepare-related test setup. |
| src/commands/runtime/mod.rs | Renames test command’s wait-timeout parameter to timeout and updates parse error context. |
| src/commands/runtime/germinate.rs | Renames germinate’s pending-timeout parameter to timeout and updates parse error context. |
| src/commands/mod.rs | Wires renamed CLI fields through dispatch; introduces parse_doctor_timeout and updates notebook timeout parsing context. |
| src/commands/doctor.rs | Updates doctor validation error messages to reference --timeout and 1s semantics. |
| src/cli/commands.rs | Removes prepare --force alias, converts doctor timeouts to --timeout <DURATION>, and standardizes test/notebook/germinate to --timeout. |
| man/man1/hpc-compose-test.1 | Updates manpage flag from --wait-timeout to --timeout. |
| man/man1/hpc-compose-notebook.1 | Updates manpage flag from --ready-timeout to --timeout. |
| man/man1/hpc-compose-germinate.1 | Updates manpage flag from --pending-timeout to --timeout. |
| man/man1/hpc-compose-doctor-readiness.1 | Updates manpage flag from --timeout-seconds to --timeout <DURATION>. |
| man/man1/hpc-compose-doctor-mpi-smoke.1 | Updates manpage flag and default from --timeout-seconds 300 to --timeout 5m. |
| man/man1/hpc-compose-doctor-fabric-smoke.1 | Updates manpage flag and default from --timeout-seconds 300 to --timeout 5m. |
| docs/src/notebook.md | Updates notebook documentation to reference --timeout instead of --ready-timeout. |
| docs/src/cli-reference.md | Updates CLI reference examples and option docs to the canonical --timeout spelling. |
| CHANGELOG.md | Adds BREAKING “Changed/Removed” entries documenting timeout canonicalization and removal of prepare --force. |
Comment on lines
+1636
to
+1637
| let ready_timeout_seconds = parse_short_duration(&timeout) | ||
| .with_context(|| format!("--timeout '{timeout}' must be like 30s, 10m, or 1h"))?; |
Comment on lines
+1924
to
+1927
| fn parse_doctor_timeout(raw: &str) -> Result<u64> { | ||
| hpc_compose::spec::parse_short_duration(raw) | ||
| .with_context(|| format!("doctor --timeout '{raw}' must be like 30s or 5m")) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGES
This PR intentionally breaks CLI backward compatibility (approved): the old
timeout flag spellings and the deprecated
prepare --forcealias are removedoutright, with no hidden aliases and no deprecation period. Stacked on #62
(
feat/cli-contract-nits).1. Timeout flags: one canonical
--timeout <DURATION>Every per-command "give up after this long" flag is now spelled
--timeoutandtakes a DURATION string (
30s,5m,1h, or a bare number of seconds). Eachcommand's effective default is unchanged.
test--wait-timeout <DURATION>(hidden alias--timeout)--timeout <DURATION>180snotebook--ready-timeout <DURATION>--timeout <DURATION>10mgerminate--pending-timeout <DURATION>--timeout <DURATION>30mwhen--timeout <DURATION>sweep observe--timeout <DURATION>doctor mpi-smoke--timeout-seconds <SECONDS>(raw u64)--timeout <DURATION>5m(= previous300)doctor fabric-smoke--timeout-seconds <SECONDS>(raw u64)--timeout <DURATION>5m(= previous300)doctor readiness--timeout-seconds <SECONDS>(raw u64, optional)--timeout <DURATION>(optional)doctor(hidden legacy flag interface)--timeout-seconds(hidden)--timeout(hidden, DURATION)5mMigration one-liners
2.
prepare --forceremovedThe hidden deprecated alias for
prepare --force-rebuildis gone;prepare --forceis now an unknown-flag error (with clap's"a similar argument exists: '--force-rebuild'" tip, pinned by a new test).
--forcenow means only "overwrite the output file" (new/evolve)everywhere in the CLI.
hpc-compose prepare --force -> hpc-compose prepare --force-rebuildAlso in this PR
## [Unreleased](### Changed+### Removed,both marked BREAKING, with the before/after table). The pending 0.1.53
section from Cut v0.1.53 release #63 is untouched.
docs/src/cli-reference.md,docs/src/notebook.md) and manpages regenerated for the six affected pages;
gen-manpages --checkpassesafter regen (help-text changes here are deliberate).
test --timeout is invalid,germinate --timeout is invalid,doctor --timeout '...' must be like 30s or 5m,doctor ... --timeout must be at least 1s when --submit is used).Gates
cargo test --locked(full): passcargo clippy --all-targets --locked -- -D warnings: pass, no issuescargo fmt --all -- --check: passgen-manpages -- --check: passmdbook buildpass,typospass,markdownlint-cli2no new findings (repo baseline unchanged),lycheealllinks OK except a sandbox DNS block on a pre-existing external URL
Verified end-to-end against the built binary: all six
--helpoutputs show--timeout <DURATION>with the preserved defaults, invalid durations producethe new error messages, and
prepare --forcefails with the--force-rebuildtip.🤖 Generated with Claude Code