fix(moshscript): keep code in the result under --dry-run - #139
Merged
ralyodio merged 1 commit intoJul 31, 2026
Merged
Conversation
runMoshcode() is documented "Returns { ok, code } — always" and shell() as
"Returns { ok, code } so scripts can branch on the exit status", but both
dropped `code` on the --dry-run path. A script written to the documented
convention read `undefined` and took the failure branch on a dry run where
nothing was spawned and nothing failed.
Add `code: 0` to both dry-run returns. Real runs are untouched and still
report the true exit status.
Adds test/dryrun-result-code.test.mjs (11 tests).
Merged
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.
The bug
runMoshcode()insrc/cli.mjsis documented:and the
shell()verb insrc/commands.mjscarries its own version of the same promise:Under
--dry-runneither actually returnedcode. Both returned{ ok: true, dryRun: true }, so a script written to the documented convention readundefinedand took the failure branch on a run where nothing was spawned and nothing failed.PRD 0004 R8 [P0] requires "a documented convention ... MUST be specified so scripts can branch on outcomes", and R7 [P0] requires
--dry-runto narrate instead of acting. Narrating is correct; changing the result shape while narrating is what broke the branch.Reproducing it
On unmodified
main(v0.13.2,1d2f1e1), end-to-end through the real CLI:After the fix, same command:
The asymmetry against real runs, which is the whole defect:
{"ok":true,"code":0}{"ok":false,"code":1,"signal":null}{"ok":true,"dryRun":true}The fix
code: 0added to both dry-run returns. 5 insertions / 2 deletions of real change, the rest comment. Real runs are untouched and still report the true exit status. ThedryRun: trueflag stays, so anything keying off it keeps working.Tests
New
test/dryrun-result-code.test.mjs, 11 tests.5 are the bug:
runMoshcodecarries a numericcode: 0in dry-run; the documented.code !== 0branch no longer misfires;shell()the same; a sweep over all 13 CLI verbs from the vocabulary; and one end-to-end throughrunScriptasserting a dry run reports SUCCESS and never prints FAILURE.6 are controls that pass both before and after, and deliberately assert the opposite direction so the fix cannot buy a passing
.codeby pretending everything succeeded or by quietly executing for real: a real success still reportscode: 0; a real failure still reports a true non-zero code;shell("exit 3")in a real run still returnscode: 3; dry-run still narrates and never spawns (both verbs); anddryRun: trueis still set.Fail-before verified by reverting
src/cli.mjsandsrc/commands.mjsonly: 5 fail / 6 pass unpatched, 11/11 patched.Full suite 489 → 500, 0 failing. Baseline measured by moving the new test file out, not assumed.
The existing
test/cli.test.mjsassertion "all CLI verbs return{ ok: true, dryRun: true }in dry-run mode" still passes unchanged — it only ever checkedokanddryRun, nevercode, which is why this slipped through.Scoped out
notify()returns bare{ dryRun: true }under dry-run (nook, nocode), but its documented contract is{ id, url }rather than the{ ok, code }branch convention, so it is a different question and I left it alone. Say the word if you want it aligned too.