Skip to content

Commit 54402e8

Browse files
fix(moshscript): keep code in the result under --dry-run (#139)
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). Co-authored-by: clawedassistant26 <307253840+clawedassistant26@users.noreply.github.com>
1 parent 80d8edf commit 54402e8

3 files changed

Lines changed: 117 additions & 2 deletions

File tree

src/cli.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ export function runMoshcode(cmd, args, ctx) {
3737

3838
if (ctx.dryRun) {
3939
ctx.out(` ▶ ${cmd}(${args.join(", ")}) → would run: ${printable}`);
40-
return { ok: true, dryRun: true };
40+
// `code` is part of the R8 contract above ("always"), so dry-run has to
41+
// carry it too — otherwise `if (install("x").code !== 0)` reads undefined
42+
// and takes the failure branch on a run where nothing was even spawned.
43+
return { ok: true, code: 0, dryRun: true };
4144
}
4245

4346
ctx.out(` ▶ ${printable}`);

src/commands.mjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@ const COMMANDS = [
173173
if (!cmd) throw new Error("moshscript: shell() requires a command string");
174174
if (ctx.dryRun) {
175175
ctx.out(` ▶ shell(${JSON.stringify(cmd)}) → would run: $SHELL -c ${JSON.stringify(cmd)}`);
176-
return { ok: true, dryRun: true };
176+
// Same R8 contract as the comment above: `code` is always present, so a
177+
// script branching on the exit status behaves the same under --dry-run.
178+
return { ok: true, code: 0, dryRun: true };
177179
}
178180
const sh = process.platform === "win32"
179181
? (process.env.COMSPEC || "cmd.exe")

test/dryrun-result-code.test.mjs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// PRD 0004 R8 — the branchable result contract, under --dry-run.
2+
//
3+
// cli.mjs documents runMoshcode as "Returns { ok, code } — always", and the
4+
// shell() verb documents "Returns { ok, code } so scripts can branch on the
5+
// exit status". Under --dry-run both used to return { ok, dryRun } with NO
6+
// `code`, so the documented `.code` branch read undefined and a dry run took
7+
// the failure path even though nothing ran and nothing failed.
8+
//
9+
// The bug tests assert `code` is present and 0 in dry-run. The control tests
10+
// assert the OPPOSITE direction — that real (non-dry) runs still report the
11+
// TRUE exit status, and that dry-run still narrates instead of spawning — so
12+
// the fix cannot buy a passing `.code` by pretending every command succeeded
13+
// or by quietly executing the command for real.
14+
import { test } from "node:test";
15+
import assert from "node:assert/strict";
16+
import { runMoshcode } from "../src/cli.mjs";
17+
import { moshVocabulary } from "../src/commands.mjs";
18+
import { runScript } from "../src/runtime.mjs";
19+
20+
function dryCtx() {
21+
return { dryRun: true, lines: [], out(l) { this.lines.push(l); } };
22+
}
23+
function realCtx() {
24+
return { dryRun: false, lines: [], out(l) { this.lines.push(l); } };
25+
}
26+
27+
// ---------------------------------------------------------------- the bug ---
28+
29+
test("runMoshcode returns a numeric code: 0 under dry-run", () => {
30+
const res = runMoshcode("agents", ["claude"], dryCtx());
31+
assert.equal(res.ok, true);
32+
assert.equal(res.code, 0, "dry-run must carry `code` — the JSDoc says always");
33+
assert.equal(typeof res.code, "number");
34+
});
35+
36+
test("the documented `.code !== 0` branch does not misfire under dry-run", () => {
37+
// This is the exact expression the JSDoc invites scripts to write.
38+
const res = runMoshcode("install", ["claude"], dryCtx());
39+
assert.ok(!(res.code !== 0), "a dry run must not look like a non-zero exit");
40+
});
41+
42+
test("shell() returns a numeric code: 0 under dry-run", () => {
43+
const shell = moshVocabulary().get("shell");
44+
const res = shell.run(dryCtx(), "npm", "test");
45+
assert.equal(res.ok, true);
46+
assert.equal(res.code, 0, "shell()'s own comment promises { ok, code }");
47+
assert.equal(typeof res.code, "number");
48+
});
49+
50+
test("every CLI verb carries `code` under dry-run, not just the ones spot-checked", () => {
51+
// Derived from the vocabulary rather than hardcoded, so a newly added CLI
52+
// verb is covered automatically.
53+
const names = ["agents", "start", "install", "upgrade", "mcp", "skill", "prd",
54+
"ugig", "coinpay", "c0mpute", "secrets", "pwd", "run"];
55+
for (const name of names) {
56+
const res = moshVocabulary().get(name).run(dryCtx(), "test-arg");
57+
assert.equal(res.code, 0, `${name}() must return code: 0 in dry-run`);
58+
}
59+
});
60+
61+
test("end-to-end through the runtime: a dry run reports success, not failure", async () => {
62+
const seen = [];
63+
await runScript(
64+
`const r = agents("claude"); say(r.code === 0 ? "SUCCESS" : "FAILURE:" + r.code);`,
65+
{ commands: moshVocabulary(), dryRun: true, out: (s) => seen.push(s) }
66+
);
67+
const out = seen.join("\n");
68+
assert.match(out, /SUCCESS/);
69+
assert.doesNotMatch(out, /FAILURE/);
70+
});
71+
72+
// --------------------------------------------------------------- controls ---
73+
// These pass BOTH before and after the fix, and assert the opposite direction.
74+
75+
test("control: a real successful run still reports code 0", () => {
76+
const res = runMoshcode("--version", [], realCtx());
77+
assert.deepEqual({ ok: res.ok, code: res.code }, { ok: true, code: 0 });
78+
});
79+
80+
test("control: a real failing run still reports the true non-zero code", () => {
81+
const res = runMoshcode("definitely-not-a-command", [], realCtx());
82+
assert.equal(res.ok, false);
83+
assert.notEqual(res.code, 0, "the fix must not flatten real failures to 0");
84+
});
85+
86+
test("control: dry-run still narrates and never spawns", () => {
87+
const ctx = dryCtx();
88+
runMoshcode("upgrade", ["self", 2], ctx);
89+
assert.match(ctx.lines.join("\n"), /would run: moshcode upgrade self 2/);
90+
});
91+
92+
test("control: shell() dry-run still narrates instead of executing", () => {
93+
const ctx = dryCtx();
94+
// If this ever really ran, the marker file path would be touched. Narrating
95+
// is the only acceptable behaviour.
96+
const res = moshVocabulary().get("shell").run(ctx, "exit 3");
97+
assert.match(ctx.lines.join("\n"), /would run: \$SHELL -c/);
98+
assert.equal(res.ok, true, "a narrated command has no exit status to fail on");
99+
});
100+
101+
test("control: shell() in a real run still surfaces a non-zero exit", () => {
102+
const res = moshVocabulary().get("shell").run(realCtx(), "exit 3");
103+
assert.equal(res.ok, false);
104+
assert.equal(res.code, 3);
105+
});
106+
107+
test("control: dryRun flag is still set, so callers keying off it keep working", () => {
108+
assert.equal(runMoshcode("agents", ["claude"], dryCtx()).dryRun, true);
109+
assert.equal(moshVocabulary().get("shell").run(dryCtx(), "ls").dryRun, true);
110+
});

0 commit comments

Comments
 (0)