Skip to content

Commit f489e12

Browse files
authored
Merge pull request #230 from kychee-com/claude/cranky-hamilton-d53ca6
fix: bug triage batch — 14 issues across CLI/core/SDK
2 parents f3cd143 + 1829bdc commit f489e12

36 files changed

Lines changed: 2858 additions & 194 deletions

cli-e2e.test.mjs

Lines changed: 1262 additions & 11 deletions
Large diffs are not rendered by default.

cli-env.test.mjs

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* cli-env.test.mjs — Subprocess tests for environment-variable validation.
3+
*
4+
* Bug GH-197: RUN402_API_BASE was previously read with `||` fallback, which
5+
* meant an empty string silently used the default and any non-URL/junk value
6+
* produced opaque "fetch failed" errors. The fix in `core/src/config.ts` adds
7+
* URL validation and a stderr warning for set-but-empty values.
8+
*
9+
* These tests spawn the CLI as a subprocess so each scenario gets its own
10+
* process with a freshly read env, which is what real users hit.
11+
*
12+
* Run: node --test cli-env.test.mjs
13+
*/
14+
15+
import { describe, it } from "node:test";
16+
import assert from "node:assert/strict";
17+
import { spawn } from "node:child_process";
18+
import { mkdtempSync, rmSync } from "node:fs";
19+
import { join, dirname } from "node:path";
20+
import { tmpdir } from "node:os";
21+
import { fileURLToPath } from "node:url";
22+
import { once } from "node:events";
23+
24+
const __dirname = dirname(fileURLToPath(import.meta.url));
25+
const CLI_PATH = join(__dirname, "cli", "cli.mjs");
26+
27+
async function runCli(argv, env) {
28+
const configDir = mkdtempSync(join(tmpdir(), "run402-env-"));
29+
const child = spawn(process.execPath, [CLI_PATH, ...argv], {
30+
env: {
31+
...process.env,
32+
RUN402_CONFIG_DIR: configDir,
33+
HOME: configDir,
34+
...env,
35+
},
36+
stdio: ["ignore", "pipe", "pipe"],
37+
});
38+
39+
let stdout = "";
40+
let stderr = "";
41+
child.stdout.on("data", (d) => { stdout += d.toString(); });
42+
child.stderr.on("data", (d) => { stderr += d.toString(); });
43+
44+
const killTimer = setTimeout(() => child.kill("SIGKILL"), 10_000);
45+
const [code] = await once(child, "exit");
46+
clearTimeout(killTimer);
47+
48+
rmSync(configDir, { recursive: true, force: true });
49+
return { code, stdout, stderr };
50+
}
51+
52+
describe("RUN402_API_BASE env-var validation", () => {
53+
it("invalid scheme (javascript:) produces clear error mentioning the env var", async () => {
54+
const r = await runCli(
55+
["allowance", "status"],
56+
{ RUN402_API_BASE: "javascript:alert(1)" },
57+
);
58+
assert.equal(r.code, 1, `expected exit 1, got ${r.code}\nstderr:\n${r.stderr}`);
59+
assert.match(r.stderr, /RUN402_API_BASE/,
60+
`stderr must mention the env var name; got: ${r.stderr}`);
61+
assert.match(r.stderr, /javascript:/,
62+
`stderr must mention the bad scheme; got: ${r.stderr}`);
63+
// Must NOT be the opaque "fetch failed" or generic Node stack trace.
64+
assert.doesNotMatch(r.stderr, /fetch failed/,
65+
`stderr must not be the opaque generic error; got: ${r.stderr}`);
66+
});
67+
68+
it("invalid scheme (file:) produces clear error mentioning the env var", async () => {
69+
const r = await runCli(
70+
["allowance", "status"],
71+
{ RUN402_API_BASE: "file:///etc/passwd" },
72+
);
73+
assert.equal(r.code, 1, `expected exit 1, got ${r.code}\nstderr:\n${r.stderr}`);
74+
assert.match(r.stderr, /RUN402_API_BASE/);
75+
assert.match(r.stderr, /file:/);
76+
});
77+
78+
it("no scheme (api.run402.com) produces clear error mentioning the env var", async () => {
79+
const r = await runCli(
80+
["allowance", "status"],
81+
{ RUN402_API_BASE: "api.run402.com" },
82+
);
83+
assert.equal(r.code, 1);
84+
assert.match(r.stderr, /RUN402_API_BASE/,
85+
`must mention env var so the user knows where to look; got: ${r.stderr}`);
86+
assert.match(r.stderr, /not a valid URL|http\(s\)/i);
87+
});
88+
89+
it("empty string emits a stderr warning and falls back to default", async () => {
90+
// We avoid actually hitting the real production API by checking only that
91+
// the warning is on stderr — we also force the command to be one that
92+
// exits without making a network call (--help).
93+
const r = await runCli(
94+
["allowance", "--help"],
95+
{ RUN402_API_BASE: "" },
96+
);
97+
assert.equal(r.code, 0, `--help should exit 0; got ${r.code}\nstderr:\n${r.stderr}`);
98+
assert.match(r.stderr, /RUN402_API_BASE/,
99+
`expected a warning naming the env var; got stderr: ${r.stderr}`);
100+
assert.match(r.stderr, /empty/i);
101+
});
102+
103+
it("valid http://localhost URL is accepted (test/local dev shape)", async () => {
104+
// We give the CLI an unreachable localhost port for a command that DOES
105+
// try to hit the API (status). The validation should pass (we don't
106+
// expect the structured "BAD_ENV" / scheme-validation message), and any
107+
// failure must come from the network attempt, not URL validation.
108+
const r = await runCli(
109+
["service", "health"],
110+
{ RUN402_API_BASE: "http://127.0.0.1:1" },
111+
);
112+
// We don't care what exit code we get from the unreachable port —
113+
// we only care that the URL validation didn't reject it.
114+
assert.doesNotMatch(r.stderr, /not a valid URL/i,
115+
`valid http:// URL must not be rejected; got: ${r.stderr}`);
116+
assert.doesNotMatch(r.stderr, /must use http\(s\):/i,
117+
`valid http:// URL must not be rejected; got: ${r.stderr}`);
118+
});
119+
});

cli-help.test.mjs

Lines changed: 98 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const MATRIX = {
5353
shared: ["status", "create", "fund", "balance", "export"],
5454
specific: ["checkout", "history"],
5555
},
56-
tier: { shared: ["status", "set"], specific: [] },
56+
tier: { shared: [], specific: ["status", "set"] },
5757
projects: {
5858
shared: [
5959
"quote", "use", "list", "info", "keys", "rest",
@@ -63,48 +63,49 @@ const MATRIX = {
6363
},
6464
deploy: { shared: [], specific: [] },
6565
functions: {
66-
shared: ["list", "delete"],
67-
specific: ["deploy", "invoke", "logs", "update"],
66+
shared: [],
67+
specific: ["deploy", "invoke", "logs", "update", "list", "delete"],
6868
},
69-
secrets: { shared: ["list", "delete"], specific: ["set"] },
69+
secrets: { shared: [], specific: ["set", "list", "delete"] },
7070
blob: {
7171
shared: [],
7272
specific: ["put", "get", "ls", "rm", "sign"],
7373
},
7474
sites: { shared: ["status"], specific: ["deploy", "deploy-dir"] },
75-
subdomains: { shared: ["delete", "list"], specific: ["claim"] },
76-
domains: { shared: ["add", "list", "status", "delete"], specific: [] },
75+
subdomains: { shared: [], specific: ["claim", "list", "delete"] },
76+
domains: { shared: [], specific: ["add", "list", "status", "delete"] },
7777
apps: {
78-
shared: ["versions", "inspect", "delete"],
79-
specific: ["browse", "fork", "publish", "update"],
78+
shared: [],
79+
specific: ["browse", "fork", "publish", "update", "versions", "inspect", "delete"],
8080
},
81-
ai: { shared: ["moderate", "usage"], specific: ["translate"] },
82-
image: { shared: ["generate"], specific: [] },
81+
ai: { shared: [], specific: ["translate", "moderate", "usage"] },
82+
image: { shared: [], specific: ["generate"] },
8383
email: {
84-
shared: ["create", "get"],
85-
specific: ["info", "status", "send", "list", "get-raw", "reply", "delete"],
84+
shared: [],
85+
specific: ["info", "status", "send", "list", "get-raw", "reply", "delete", "create", "get"],
8686
},
87-
message: { shared: ["send"], specific: [] },
87+
message: { shared: [], specific: ["send"] },
8888
auth: {
8989
shared: [],
9090
specific: ["magic-link", "verify", "set-password", "settings", "providers"],
9191
},
9292
"sender-domain": {
93-
shared: ["register", "status", "remove", "inbound-enable", "inbound-disable"],
94-
specific: [],
93+
shared: [],
94+
specific: ["register", "status", "remove", "inbound-enable", "inbound-disable"],
9595
},
9696
billing: {
97-
shared: ["create-email", "link-wallet", "balance"],
98-
specific: ["tier-checkout", "buy-email-pack", "auto-recharge", "history"],
97+
shared: [],
98+
specific: ["tier-checkout", "buy-email-pack", "auto-recharge", "history", "create-email", "link-wallet", "balance"],
9999
},
100100
contracts: {
101-
shared: ["get-wallet", "list-wallets", "status"],
101+
shared: [],
102102
specific: [
103103
"provision-wallet", "set-recovery", "set-alert", "call", "read", "drain", "delete",
104+
"get-wallet", "list-wallets", "status",
104105
],
105106
},
106-
agent: { shared: ["contact"], specific: [] },
107-
service: { shared: ["status", "health"], specific: [] },
107+
agent: { shared: [], specific: ["contact"] },
108+
service: { shared: [], specific: ["status", "health"] },
108109
};
109110

110111
// `run402 email webhooks <action>` delegates to lib/webhooks.mjs.
@@ -272,4 +273,81 @@ describe("CLI --help contract", () => {
272273
});
273274
}
274275
});
276+
277+
// GH-198 — `run402 deploy --help` was showing the v1 bundle manifest format
278+
// (top-level `migrations` string, `secrets` array, `functions` array, `files`
279+
// array) which the v2 gateway rejects. The example must match the v2
280+
// ReleaseSpec shape documented in cli/llms-cli.txt: object trees rooted at
281+
// `database`, `site`, `functions.replace`, `secrets.set`, `subdomains`.
282+
describe("run402 deploy --help shows v2 manifest format (GH-198)", () => {
283+
it("deploy --help example uses v2 keys (database/site/replace), not v1 arrays", async () => {
284+
const result = await runCli(["deploy", "--help"]);
285+
assertHelp(result, "run402 deploy --help");
286+
287+
const out = result.stdout;
288+
289+
// v2 keys must be present in the example block.
290+
assert.match(out, /"database":/,
291+
`deploy --help: example must contain a top-level "database": key (v2 ReleaseSpec)\nstdout:\n${out}`);
292+
assert.match(out, /"site":/,
293+
`deploy --help: example must contain a top-level "site": key (v2 ReleaseSpec)\nstdout:\n${out}`);
294+
assert.match(out, /"replace":/,
295+
`deploy --help: example must contain a "replace": key (v2 site/functions shape)\nstdout:\n${out}`);
296+
assert.match(out, /"set":/,
297+
`deploy --help: example must contain a "set": key (v2 secrets/subdomains shape)\nstdout:\n${out}`);
298+
assert.match(out, /"subdomains":/,
299+
`deploy --help: example must contain a "subdomains": key (v2 ReleaseSpec)\nstdout:\n${out}`);
300+
301+
// v1 shapes that are NOT accepted by the v2 gateway must be gone.
302+
// - top-level `"migrations":` as a string (v1) — v2 nests under database.migrations as an array of {id, sql}
303+
assert.doesNotMatch(out, /^\s*"migrations":\s*"/m,
304+
`deploy --help: example must not have v1 top-level "migrations": "<string>" (use database.migrations: [{id, sql}])\nstdout:\n${out}`);
305+
// - top-level `"files":` as an array (v1) — v2 uses site.replace as an object map
306+
assert.doesNotMatch(out, /^\s*"files":\s*\[/m,
307+
`deploy --help: example must not have v1 top-level "files": [...] array (use site.replace: { "<path>": {...} })\nstdout:\n${out}`);
308+
// - top-level `"secrets":` as an array (v1) — v2 uses secrets.set as an object map
309+
assert.doesNotMatch(out, /^\s*"secrets":\s*\[/m,
310+
`deploy --help: example must not have v1 top-level "secrets": [...] array (use secrets.set: { "<KEY>": {...} })\nstdout:\n${out}`);
311+
// - top-level `"functions":` as an array (v1) — v2 uses functions.replace as an object map
312+
assert.doesNotMatch(out, /^\s*"functions":\s*\[/m,
313+
`deploy --help: example must not have v1 top-level "functions": [...] array (use functions.replace: { "<name>": {...} })\nstdout:\n${out}`);
314+
// - top-level `"subdomain":` (singular, v1) — v2 uses `subdomains.set: ["..."]`
315+
assert.doesNotMatch(out, /^\s*"subdomain":/m,
316+
`deploy --help: example must not have v1 top-level "subdomain" (singular); use "subdomains": { "set": [...] }\nstdout:\n${out}`);
317+
});
318+
});
319+
320+
// Regression test for GH-188: confirm the previously-broken subcommands now
321+
// print their own per-subcommand help instead of falling back to the parent
322+
// namespace's help page. The MATRIX above also covers these (each one moved
323+
// from `shared` to `specific`), but this explicit suite spot-checks a
324+
// representative sample across namespaces and asserts both that the
325+
// subcommand-specific title is present AND that the parent's general
326+
// "Manage ..." headline is NOT.
327+
describe("GH-188 regression — SUB_HELP entries for previously-broken subs", () => {
328+
const cases = [
329+
// [command sequence, parent-help heading that should NOT appear]
330+
[["secrets", "list"], "run402 secrets — Manage project secrets"],
331+
[["functions", "list"], "run402 functions — Manage serverless functions"],
332+
[["domains", "list"], "run402 domains — Manage custom domains"],
333+
[["ai", "moderate"], "run402 ai — AI translation and moderation tools"],
334+
[["tier", "status"], "run402 tier — Manage your Run402 tier subscription"],
335+
[["service", "status"], "run402 service — Run402 service health and availability"],
336+
[["sender-domain", "register"], "run402 sender-domain — Manage custom email sender domain"],
337+
[["contracts", "get-wallet"], "run402 contracts — KMS-backed Ethereum wallets"],
338+
];
339+
for (const [argv, parentHeading] of cases) {
340+
it(`run402 ${argv.join(" ")} --help shows per-subcommand title`, async () => {
341+
const result = await runCli([...argv, "--help"]);
342+
const expectedHeading = `run402 ${argv.join(" ")}`;
343+
assertHelp(result, `run402 ${argv.join(" ")} --help`, {
344+
expectHeadingStartsWith: expectedHeading,
345+
});
346+
// Belt-and-suspenders: parent-only headline must NOT be the first line.
347+
const firstLine = result.stdout.trimStart().split(/\r?\n/, 1)[0];
348+
assert.ok(!firstLine.startsWith(parentHeading.split(" — ")[0] + " —"),
349+
`run402 ${argv.join(" ")} --help fell through to parent help: first line was "${firstLine}"`);
350+
});
351+
}
352+
});
275353
});

cli/cli.mjs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,23 @@ if (!cmd || cmd === '--help' || cmd === '-h') {
7474
process.exit(0);
7575
}
7676

77+
try {
78+
await dispatch();
79+
} catch (err) {
80+
// Surface env/config errors (e.g. invalid RUN402_API_BASE) as a clean
81+
// JSON envelope on stderr instead of a raw stack trace. We import the
82+
// helper lazily so a broken env doesn't fail this catch handler too.
83+
const { fail } = await import("./lib/sdk-errors.mjs");
84+
fail({
85+
code: "BAD_ENV",
86+
message: err && err.message ? err.message : String(err),
87+
hint: typeof err?.message === "string" && err.message.includes("RUN402_API_BASE")
88+
? "Check the RUN402_API_BASE env var."
89+
: undefined,
90+
});
91+
}
92+
93+
async function dispatch() {
7794
switch (cmd) {
7895
case "init": {
7996
const { run } = await import("./lib/init.mjs");
@@ -200,3 +217,4 @@ switch (cmd) {
200217
console.log(HELP);
201218
process.exit(1);
202219
}
220+
}

cli/lib/agent.mjs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { allowanceAuthHeaders } from "./config.mjs";
22
import { getSdk } from "./sdk.mjs";
33
import { reportSdkError, fail } from "./sdk-errors.mjs";
4+
import { validateWebhookUrl } from "./argparse.mjs";
45

56
const HELP = `run402 agent — Manage agent identity
67
@@ -17,6 +18,29 @@ Examples:
1718
run402 agent contact --name my-agent --email ops@example.com --webhook https://example.com/hook
1819
`;
1920

21+
const SUB_HELP = {
22+
contact: `run402 agent contact — Register agent contact info
23+
24+
Usage:
25+
run402 agent contact --name <name> [--email <email>] [--webhook <url>]
26+
27+
Options:
28+
--name <name> Required: agent name (e.g. "my-agent")
29+
--email <email> Optional: contact email address
30+
--webhook <url> Optional: webhook URL Run402 can call to reach the
31+
agent
32+
33+
Notes:
34+
- Free with allowance auth (run an 'allowance create' first)
35+
- Registers contact info so Run402 can reach your agent
36+
37+
Examples:
38+
run402 agent contact --name my-agent
39+
run402 agent contact --name my-agent --email ops@example.com \\
40+
--webhook https://example.com/hook
41+
`,
42+
};
43+
2044
async function contact(args) {
2145
let name = null, email = null, webhook = null;
2246
for (let i = 0; i < args.length; i++) {
@@ -27,6 +51,10 @@ async function contact(args) {
2751
if (!name) {
2852
fail({ code: "BAD_USAGE", message: "Missing --name <name>" });
2953
}
54+
// GH-192: validate webhook scheme locally BEFORE the allowance check so
55+
// bad URLs fail fast even without an allowance configured. No-op when
56+
// --webhook is omitted (it's optional).
57+
validateWebhookUrl(webhook, "--webhook");
3058
// Preserve the aggressive early exit when no allowance is configured.
3159
allowanceAuthHeaders("/agent/v1/contact");
3260

@@ -45,7 +73,7 @@ async function contact(args) {
4573
export async function run(sub, args) {
4674
if (!sub || sub === '--help' || sub === '-h') { console.log(HELP); process.exit(0); }
4775
if (Array.isArray(args) && (args.includes("--help") || args.includes("-h"))) {
48-
console.log(HELP);
76+
console.log(SUB_HELP[sub] || HELP);
4977
process.exit(0);
5078
}
5179
if (sub !== "contact") {

0 commit comments

Comments
 (0)