Skip to content

fix(cli): recognize --help/-h in subcommand position - #57

Open
vernonstinebaker wants to merge 2 commits into
mainfrom
fix/subcommand-help-flag
Open

vernonstinebaker wants to merge 2 commits into
mainfrom
fix/subcommand-help-flag

Conversation

@vernonstinebaker

Copy link
Copy Markdown
Contributor

Fixes #56. nullboiler server --help previously ignored the flag and started the real server (DB opened, port bound) — surprising interactively, harmful in scripts. --help/-h now print usage and exit 0 from any argument position, while top-level behavior is unchanged. Includes verification on v2026.5.29 (released binary shows the bug; this build prints usage).

'server --help' and 'serve --help' previously fell through to server
startup: the flag was silently ignored, the DB opened, and the port
bound — surprising interactively and harmful in scripts. Help is now
honored in any argument position.

Fixes #56

@DonPrus DonPrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original server --help / serve -h cases now work, and I also verified top-level help and validate-workflows --help. The PR merges cleanly with main.

Blocking: perform help detection before the early command handlers (src/main.zig:40-66). The new scan is below --from-json and --export-manifest, so it does not actually cover every command path. On 9ae80e9 I reproduced:

nullboiler --from-json --help
# exits 1: error: invalid JSON

nullboiler --from-json '{"home":"/tmp/isolated-help-check"}' --help
# exits 0: {"status":"ok"}; creates config.json instead of printing help

The second case can overwrite an existing configuration even though help was explicitly requested. This is the same class of unexpected side effect that #56 is intended to eliminate. It would be great to move the help-flag scan ahead of all command dispatch, retain the standalone help alias, and add CLI regression coverage for both flag spellings across the server, validation, manifest and JSON-bootstrap paths. In particular, assert that help requests neither create nor alter config/database files. The current PR has no regression test for this behavior.

I built the branch and ran the cases above against an isolated temporary home. Withholding approval until this remaining side-effecting path is covered.

…ests

The help scan sat below the --export-manifest/--from-json handlers, so
'--from-json --help' still ran bootstrap: exit 1 with 'invalid JSON' and no
usage, and '--from-json <payload> --help' created config.json while reporting
success (DonPrus review of #57).

Move help detection above every early exit and cover both flag spellings
across the server, validation, manifest and JSON-bootstrap paths. Each case
asserts exit 0, usage output, and that the working directory stays empty.
Tests spawn the installed binary via NULLBOILER_TEST_EXE (build.zig now makes
the test Run step depend on install).

Refs #56
@vernonstinebaker

Copy link
Copy Markdown
Contributor Author

Blocking finding addressed in f3ecb63.

1. Help detection moved above the early command handlers. The scan now runs before --export-manifest and --from-json — previously it sat below them, so those handlers consumed argv first. Both of your reproductions now behave correctly:

command before after
nullboiler --from-json --help exit 1, error: invalid JSON, no usage exit 0, usage
nullboiler --from-json '{"home":...}' --help exit 0, {"status":"ok"}, created config.json exit 0, usage, 0 files created

The standalone help alias is retained, and the loop now scans all argv (not argv[1..]) so position no longer matters.

2. Regression coverage added — two tests spawning the installed binary, covering both flag spellings across all four paths:

  • cli help: --help/-h print usage at every position with no side effects — iterates --help, -h, server --help, serve -h, validate-workflows --help, --export-manifest --help, --from-json --help. Each asserts exit 0, Usage: in output, and that the isolated working directory is empty afterwards (so a config or database write fails the test).
  • cli help: --from-json with a payload does not create config when help is requested — asserts the bootstrap payload case exits 0 with usage and writes nothing into the supplied home.

Tests locate the binary via NULLBOILER_TEST_EXE, set by build.zig; the test Run step now depends on install so the binary always exists. They skip cleanly rather than reporting a false pass if it does not.

Red/green verified: against the previous head (9ae80e9) with these tests applied, both fail — [manifest-long] printed the manifest JSON instead of usage, and the payload case printed {"status":"ok"}. With the fix, they pass.

Validation — merge result 2e43169c (base = current main c03b5f54, includes #54): zig build test --summary all 372/372, zig fmt --check src/ OK, run in a Linux container. Manual spot-check confirms all seven help paths exit 0 with usage, isolated home left with 0 files, and non-help paths (--version, validate-workflows) unchanged. All 4 CI checks pass, including Windows.

Refs #56. Ready for re-review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI: subcommand-level --help is silently ignored — the server starts instead of printing help

2 participants