fix(cmd/morphic): print help on stdout and exit 0

[fuad-daoud]

Summary

Asking for help was treated as an error. morphic --help, -h and help were rejected as unknown commands. morphic compile --help printed the flag package's table and then the CLI's own usage block — two overlapping texts. All of it went to stderr at exit 2, and nothing reached stdout.

Every help form now renders one text to stdout and exits 0:

Invocation	Output
morphic	root help
morphic -h / --help / -help	root help
morphic help	root help
morphic help compile	compile help
morphic -h compile	compile help
morphic compile -h / --help	compile help

All command-help forms render through one function, so they are byte-identical. Misuse prints one reason line and one short usage pointer to stderr at exit 2 — unknown command, help accepts at most one command, a bad flag, the wrong positional count, an invalid --fail-on.

Three details worth a reviewer's attention:

• A leading help flag is the help command spelled differently, so it routes through runHelp rather than shortcutting to root help. That is what makes morphic -h compile print compile's help instead of silently dropping the name, and morphic -h bogus report the bad name instead of masking it behind root help.
• Help flags are stripped from help's own arguments before the command lookup, so morphic help bogus --help still reports the bad name. help takes only a bare command name, so no help token there can be a legitimate value.
• compile detects help through errors.Is(err, flag.ErrHelp), never by scanning argv. That is what keeps morphic compile -o --help spec.yaml treating --help as -o's value. TestRun_HelpFlagAsFlagValue exists to stop a future refactor from replacing this with a pre-scan — such a change would keep full statement coverage while silently breaking it.

The command table becomes a function rather than a package-level var. As a var it sits in the package's initialization graph, so the first time a command's own code reaches back into the table — writeRootHelp already does — the result is initialization cycle for commands, a compile error rather than a test failure. Verified: with the table as a function, pointing compile's help path at writeRootHelp builds fine and TestRun_HelpFormsAgree goes red on it, which is the failure you want.

Bare morphic now prints help and exits 0 rather than printing usage and exiting 2.

Closes #62

Test plan

• TestRun_HelpForms covers every help invocation: exit 0, text on stdout, nothing on stderr.
• TestRun_HelpFormsAgree byte-compares all five command-help spellings, including morphic -h compile.
• TestRun_UsageErrors covers -h bogus and --help compile extra.
• TestRun_HelpFlagAsFlagValue proves -o --help writes a file named --help and prints no help.
• TestRun_CompileHelpListsEveryFlag asserts the rendered flag form (\n -<name>), not the bare name. Verified by making command help render root help: the rendered form catches all four flags including -o, where the bare-name form silently passed -o because "o" is a substring of nearly any text.
• compileUsageError takes a finished string rather than a format. A printf-style wrapper here is invisible to go vet, so a reason carrying a literal % was mangled into the output with nothing to catch it; verified that a reason containing 100% now renders literally.
• gofmt -l, go vet ./..., golangci-lint run (0 issues), go build ./..., ./scripts/check-coverage.sh — 4232/4232 statements.

Breaking

Bare morphic changes from exit 2 to exit 0. A script invoking morphic with no arguments and relying on failure would now see success. The README is updated to match in #207, which stacks on this PR.

[OmarAlJarrah]

Second pass on this branch. The help design itself is good: one renderer for all three command-help forms, errors.Is(err, flag.ErrHelp) rather than an argv pre-scan, and TestRun_HelpFlagAsFlagValue pinning that choice is exactly the right instinct. flagSet finally earns its place now that writeCommandHelp reads it.

Gate is clean on the merged state: build, vet, golangci-lint 0 issues, coverage 4234/4234.

One thing outside the diff: the Breaking section says "the README documents the new contract", but README isn't in this PR. Its exit-code line still reads 2 a usage or I/O error with no mention of help, the synopsis block doesn't show morphic help, -h or --help, and the flag table still omits --explain even though compile help now documents it. Either update it here, since this is the PR that makes help the source of truth, or drop the claim.

[OmarAlJarrah]

Make this a function too. Turning compileCommand into newCompileCommand moved the cycle one hop instead of removing it, because commands is still a package-level var and still sits in the chain.

Proof: point compile's help path at writeRootHelp, which is the obvious next thing an error path wants, and the package stops compiling.

initialization cycle for commands
	commands refers to newCompileCommand
	newCompileCommand refers to runCompile
	runCompile refers to writeRootHelp

Suggested change

	var commands = []command{newCompileCommand()}
	func commands() []command { return []command{newCompileCommand()} }

Three call sites move to commands(): the loop in writeRootHelp, and the require.NotEmpty plus the range in TestRootHelp_ListsEveryCommand. I ran it both ways: with this in place the same mutation builds fine and TestRun_HelpFormsAgree goes red on it, which is what you want. Right now the compile error hides the fact that the test would have caught it.

[fuad-daoud]

Done — commands() is a function, with the three call sites moved.

Confirmed both directions. Before: pointing compile's help path at writeRootHelp gives exactly the cycle you printed (commands → newCompileCommand → runCompile → writeRootHelp → commands). After: the same mutation builds, and TestRun_HelpFormsAgree goes red with help text differs for [help compile]. The compile error really was hiding the test.

[OmarAlJarrah]

The doc comment on this one is worth tightening while you're at it. It says a var "would be a Go initialization cycle, while two package-level functions may freely refer to each other", which reads as though the problem is gone. It isn't, because commands is still a var; see the note there.

[fuad-daoud]

Tightened. It now points at commands for the real reason instead of implying the cycle is gone.

[OmarAlJarrah]

This is a printf wrapper that vet doesn't recognise, so a reason string with a literal % in it garbles silently. I dropped (100% of the time) into the spec-file message:

morphic: compile requires exactly one spec file (100%!o(MISSING)f the time)

go vet said nothing and TestRun_UsageErrors stayed green, because the assertion only checks the substring before the mangled part.

Take a finished string instead, so a stray % can't matter:

Suggested change

	// compileUsageError reports a misuse of compile: one reason line, one short
	// usage pointer, exit 2. It never touches stdout.
	func compileUsageError(stderr io.Writer, format string, args ...any) int {
	emitf(stderr, "morphic: "+format+"\n", args...)
	writeCommandUsage(stderr, newCompileCommand())
	// compileUsageError reports a misuse of compile: one reason line, one short
	// usage pointer, exit 2. It never touches stdout.
	func compileUsageError(stderr io.Writer, reason string) int {
	emitf(stderr, "morphic: %s\n", reason)
	writeCommandUsage(stderr, newCompileCommand())
	return 2
	}

Call sites become compileUsageError(stderr, err.Error()) and compileUsageError(stderr, fmt.Sprintf("invalid --fail-on %q (want error or warning)", opts.failOn)).

[fuad-daoud]

Fixed — it takes a finished string now, with the reason recorded in the doc comment.

Reproduced your mutation first (100% in the spec-file message → 100%!o(MISSING)f, vet silent, tests green), then confirmed the same reason renders literally after the change.

[OmarAlJarrah]

f.Name is "o" for -o, so this assertion can't fail for that flag. Any text with the letter o in it passes, root help included (I checked: it does).

Assert the rendered form:

Suggested change

	fs.VisitAll(func(f *flag.Flag) {
	assert.Contains(t, got, f.Name, "compile help must document -%s", f.Name)
	})
	fs.VisitAll(func(f *flag.Flag) {
	assert.Contains(t, got, "\n -"+f.Name, "compile help must document -%s", f.Name)
	})

[fuad-daoud]

Fixed to the rendered form.

Isolated the -o case to be sure the new assertion is what's doing the work: with writeCommandHelp mutated to render root help — documenting no flags at all — the rendered form catches all four flags including -o, while the bare-name form catches only three and passes -o silently.

[OmarAlJarrah]

morphic -h compile takes this branch and prints root help, dropping compile without a word. That's the opposite of the care taken in runHelp, where help bogus --help deliberately reports the bad name rather than letting the help flag mask it. Worth making the two agree, or saying why root is different.

[fuad-daoud]

Fixed, and made the two agree rather than documenting a difference. A leading help flag now routes through runHelp instead of shortcutting to root help:

if args[0] == "help" || isHelpFlag(args[0]) {
    return runHelp(args[1:], stdout, stderr)
}

So morphic -h compile prints compile help, morphic -h bogus reports the bad name, and morphic --help compile extra gives the at-most-one-command error — same as the help spellings, because it is now literally the same path. TestRun_HelpForms and TestRun_HelpFormsAgree gain the -h compile rows, TestRun_UsageErrors gains -h bogus and --help compile extra.

[fuad-daoud]

Force-pushed. The README claim in the Breaking section is gone — it now says the README is updated in #207, which is where the file actually is.

Gate on the merged state: golangci-lint 0 issues, coverage 4232/4232.