diff --git a/bin/butter b/bin/butter index 572951f..4bc0f53 100755 --- a/bin/butter +++ b/bin/butter @@ -540,6 +540,161 @@ async function buildsList(args) { } } +async function buildsShow(args) { + const projectId = args.project || loadConfig().defaultProject; + const buildId = args._[2]; + if (!projectId || !buildId) { + console.error(`${colors.err}✗ Usage: butter builds show --project ${colors.reset}`); + process.exit(1); + } + + try { + const res = await request("GET", `/api/v1/projects/${projectId}/build_runs/${buildId}`); + if (args.json) { + console.log(JSON.stringify(res, null, 2)); + return; + } + + const statusColor = + res.status === "completed" ? colors.ok : res.status === "failed" ? colors.err : colors.warn; + console.log(`\n${colors.bold}BUILD ${res.id}${colors.reset}`); + console.log("----------------------------------------------------------------------"); + console.log(` Status: ${statusColor}${String(res.status).toUpperCase()}${colors.reset}`); + console.log(` Overall: ${res.overall_status || "-"}`); + console.log(` Job: ${res.ci_job_name || "-"}`); + console.log(` Commit: ${res.commit_hash || "-"}`); + console.log(` Type: ${res.build_type || "-"}${res.target_type ? ` / ${res.target_type}` : ""}`); + console.log(` Duration: ${res.duration != null ? `${res.duration}s` : "-"}`); + console.log(` Created: ${res.created_at || "-"}`); + + // #1940: an investigation with no log to read cannot say anything useful, + // so surface that here rather than leaving it to be inferred from a + // vague diagnosis. + if (res.log_available === false) { + console.log(` Build log: ${colors.warn}not available${colors.reset}`); + if (res.log_unavailable_reason) console.log(` ${colors.muted}${res.log_unavailable_reason}${colors.reset}`); + } else if (res.log_available === true) { + console.log(` Build log: ${colors.ok}available${colors.reset}`); + } + + if (Array.isArray(res.steps) && res.steps.length) { + console.log(`\n${colors.bold} Steps:${colors.reset}`); + res.steps.forEach((s) => { + const c = s.status === "completed" ? colors.ok : s.status === "failed" ? colors.err : colors.warn; + console.log( + ` ${c}[${s.status}]${colors.reset} ${s.step_type}${s.message ? ` ${colors.muted}- ${s.message}${colors.reset}` : ""}` + ); + }); + } + + // Three distinct cases, and conflating them is how #1938 stayed confusing: + // an investigation exists; none has been run; or the server predates the + // read path and cannot tell us either way. A server without the key must + // not be reported as "no investigation" - that is the exact false negative + // that sent people back to POSTing `investigate` to find out. + if (res.investigation) { + console.log(""); + printInvestigation(res.investigation); + } else if (Object.prototype.hasOwnProperty.call(res, "investigation")) { + console.log(`\n ${colors.muted}No AI investigation has been run for this build.${colors.reset}`); + console.log(` ${colors.muted}Run: butter builds investigate ${res.id} --project ${projectId}${colors.reset}`); + } else { + console.log(`\n ${colors.warn}This ButterStack server does not expose investigations on the build detail.${colors.reset}`); + console.log(` ${colors.muted}Try: butter builds investigation ${res.id} --project ${projectId}${colors.reset}`); + } + console.log(""); + } catch (err) { + console.error(`${colors.err}✗ Failed to show build: ${err.message}${colors.reset}`); + process.exit(1); + } +} + +function printInvestigation(inv) { + const statusColor = + inv.status === "completed" ? colors.ok : inv.status === "failed" ? colors.err : colors.warn; + console.log(`${colors.bold}AI Failure Investigation:${colors.reset}`); + console.log(` Status: ${statusColor}${inv.status}${colors.reset}`); + + if (inv.status === "pending" || inv.status === "running") { + console.log(` ${colors.muted}Still in flight. Re-read with \`butter builds investigation\` - do not`); + console.log(` re-run \`investigate\`, which starts a new paid analysis.${colors.reset}`); + return; + } + + if (inv.status === "failed") { + console.log(` Error: ${colors.err}${inv.error_message || "unknown"}${colors.reset}`); + return; + } + + if (inv.diagnosis_category) { + console.log(` Category: ${inv.diagnosis_category}${inv.severity ? ` (${inv.severity})` : ""}`); + } + if (inv.confidence) console.log(` Confidence: ${inv.confidence}`); + if (inv.summary) console.log(` Summary: ${inv.summary}`); + if (inv.diagnosis) console.log(` Diagnosis: ${inv.diagnosis}`); + if (inv.suggested_fix) console.log(` Fix: ${colors.accent}${inv.suggested_fix}${colors.reset}`); + + // Attribution is printed only when the investigation actually made one. + // "unattributed" is a real, common and correct answer -- a file that was + // never submitted has no offending changelist and no culpable author. + if (inv.attributed_changelist || inv.attributed_user_id) { + console.log( + ` Attributed: ${inv.attributed_changelist ? `CL ${inv.attributed_changelist}` : ""}${ + inv.attributed_user_id ? ` (user ${inv.attributed_user_id})` : "" + }` + ); + } else { + console.log(` Attributed: ${colors.muted}unattributed${colors.reset}`); + } + + if (Array.isArray(inv.affected_files) && inv.affected_files.length) { + console.log(` Files: ${inv.affected_files.join(", ")}`); + } + if (Array.isArray(inv.evidence) && inv.evidence.length) { + console.log(`${colors.bold} Evidence:${colors.reset}`); + inv.evidence.forEach((e) => console.log(` ${colors.muted}- ${e}${colors.reset}`)); + } +} + +// Read-only counterpart to `investigate` (#1938). Before this existed, the +// only way to see a result was to POST investigate again, which spends +// credits and starts a fresh Bedrock run once the previous one has finished. +async function buildsInvestigation(args) { + const projectId = args.project || loadConfig().defaultProject; + const buildId = args._[2]; + if (!projectId || !buildId) { + console.error(`${colors.err}✗ Usage: butter builds investigation --project ${colors.reset}`); + process.exit(1); + } + + try { + const res = await request("GET", `/api/v1/projects/${projectId}/build_runs/${buildId}/investigation`); + if (args.json) { + console.log(JSON.stringify(res, null, 2)); + return; + } + console.log(""); + printInvestigation(res); + if (args.steps && Array.isArray(res.steps) && res.steps.length) { + console.log(`\n${colors.bold} Steps:${colors.reset}`); + res.steps.forEach((s) => { + console.log(` ${colors.muted}${String(s.step).padStart(3)} ${s.step_type}${colors.reset}${s.content ? ` ${s.content}` : ""}`); + }); + } else if (Array.isArray(res.steps) && res.steps.length) { + console.log(`\n ${colors.muted}${res.steps.length} step(s). Pass --steps to print the trail.${colors.reset}`); + } + console.log(""); + } catch (err) { + if (err.statusCode === 404) { + console.error(`${colors.warn}No investigation has been run for build ${buildId}.${colors.reset}`); + console.error(`${colors.muted}Start one with: butter builds investigate ${buildId} --project ${projectId}${colors.reset}`); + process.exit(1); + } + console.error(`${colors.err}✗ Failed to read investigation: ${err.message}${colors.reset}`); + process.exit(1); + } +} + async function buildsInvestigate(args) { const projectId = args.project || loadConfig().defaultProject; const buildId = args._[2]; @@ -549,17 +704,24 @@ async function buildsInvestigate(args) { } try { - console.log(`${colors.accent}Triggering AI Failure Investigation for Build #${buildId}...${colors.reset}`); + if (!args.json) { + console.log(`${colors.accent}Triggering AI Failure Investigation for Build #${buildId}...${colors.reset}`); + } const res = await request("POST", `/api/v1/projects/${projectId}/build_runs/${buildId}/investigate`); if (args.json) { console.log(JSON.stringify(res, null, 2)); return; } - console.log(`\n${colors.bold}AI Failure Investigation Report:${colors.reset}`); - console.log(` Status: ${colors.ok}${res.status}${colors.reset}`); - console.log(` Diagnosis: ${res.diagnosis || res.summary || "Pending analysis..."}`); - if (res.suggested_fix) console.log(` Fix: ${colors.accent}${res.suggested_fix}${colors.reset}`); - if (res.attributed_author) console.log(` Attributed: ${res.attributed_author}`); + console.log(""); + printInvestigation(res); + // An investigation runs asynchronously, so the POST almost always returns + // `pending`. Point at the read path rather than letting the caller poll + // by re-POSTing, which is what made this cost money to check on (#1938). + if (res.status === "pending" || res.status === "running") { + console.log( + `\n ${colors.muted}Check the result with: butter builds investigation ${buildId} --project ${projectId}${colors.reset}` + ); + } console.log(""); } catch (err) { console.error(`${colors.err}✗ Failed to investigate build: ${err.message}${colors.reset}`); @@ -649,16 +811,45 @@ async function assetsDeny(args) { } } +// Flags that are switches, never name/value pairs (#1938). +// +// The parser used to treat every `--flag` as taking a value whenever the next +// argv entry did not start with `-`. That is fine at the end of a line, but +// `butter builds investigate --json ` parsed as +// `json: ""` with no positional left, so the command printed its +// usage error and exited 1 -- looking, from the caller's side, like `--json` +// silently produced no output while the plain form worked. Position in the +// command line must not change what a boolean flag means. +const BOOLEAN_FLAGS = new Set([ + "json", + "help", + "h", + "pending", + "verbose", + "yes", + "force", + "no-color", + "steps" +]); + +// `--key=value` is always explicit, so it is honored for any key including +// the booleans above (`--json=false` is not special-cased: anything after `=` +// is taken literally, which is the same behavior every other key gets). function parseArgs(rawArgs) { const parsed = { _: [] }; for (let i = 0; i < rawArgs.length; i++) { const arg = rawArgs[i]; if (arg.startsWith("--")) { - const key = arg.substring(2); - if (i + 1 < rawArgs.length && !rawArgs[i + 1].startsWith("-")) { - parsed[key] = rawArgs[++i]; + const body = arg.substring(2); + const eq = body.indexOf("="); + if (eq !== -1) { + parsed[body.substring(0, eq)] = body.substring(eq + 1); + } else if (BOOLEAN_FLAGS.has(body)) { + parsed[body] = true; + } else if (i + 1 < rawArgs.length && !rawArgs[i + 1].startsWith("-")) { + parsed[body] = rawArgs[++i]; } else { - parsed[key] = true; + parsed[body] = true; } } else if (arg.startsWith("-")) { const key = arg.substring(1); @@ -682,7 +873,10 @@ ${colors.bold}COMMANDS:${colors.reset} ${colors.accent}auth${colors.reset} login | whoami | logout Authenticate via browser OAuth loopback ${colors.accent}projects${colors.reset} list | show View and inspect game projects ${colors.accent}tasks${colors.reset} list | create | update Manage tasks, bugs, and backlog - ${colors.accent}builds${colors.reset} list | show | investigate Inspect CI/CD engine builds and AI failure logs + ${colors.accent}builds${colors.reset} list | show | investigate | investigation + Inspect CI/CD builds and AI failure investigations. + ${colors.bold}investigate${colors.reset} STARTS a paid AI analysis; + ${colors.bold}investigation${colors.reset} reads the result back for free. ${colors.accent}assets${colors.reset} list | show | approve | deny Review and approve textures, models, and audio ${colors.bold}GLOBAL OPTIONS:${colors.reset} @@ -732,7 +926,9 @@ async function main() { case "builds": case "build": if (subcmd === "list" || !subcmd) return buildsList(args); + if (subcmd === "show") return buildsShow(args); if (subcmd === "investigate") return buildsInvestigate(args); + if (subcmd === "investigation") return buildsInvestigation(args); break; case "assets": case "asset": @@ -745,6 +941,15 @@ async function main() { printHelp(); process.exit(1); } + + // Reaching here means the command matched but its subcommand did not, and + // every branch above `break`s rather than returning. That used to fall out + // of main() silently: `butter builds show ` printed nothing and exited + // 0, which reads as "this build has no data" rather than "this subcommand + // does not exist" (#1938). An unhandled subcommand is an error. + console.error(`${colors.err}Unknown subcommand: ${cmd} ${subcmd || ""}${colors.reset}`.trimEnd()); + printHelp(); + process.exit(1); } if (require.main === module) { diff --git a/package.json b/package.json index 43e340f..a836bf5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "butterstack-cli", - "version": "0.1.1", + "version": "0.2.0", "description": "Fast, scriptable, zero-dependency command line interface for the ButterStack game development pipeline: auth, projects, tasks, builds, and asset approvals.", "bin": { "butter": "./bin/butter" diff --git a/test/butter.test.js b/test/butter.test.js index 5fe79bd..9a70cf8 100644 --- a/test/butter.test.js +++ b/test/butter.test.js @@ -364,3 +364,216 @@ test("F2 (real flow): auth login persists the token when granted scope matches w rmHome(home); } }); + +// -- #1938: the builds read path --------------------------------------------- +// +// Three defects, all of which made a build's data look absent rather than +// unreachable: `builds show` was never wired into the dispatcher (silent +// exit 0), `--json` before a positional swallowed it as the flag's value, +// and there was no GET counterpart to `investigate`, so checking a result +// meant paying for a new one. + +// A server that answers the build endpoints with fixed JSON and records +// every request, so a test can assert on the METHOD used, not just output. +function startApiServer(routes) { + const requests = createQueue(); + const server = http.createServer((req, res) => { + requests.push({ method: req.method, url: req.url }); + const key = `${req.method} ${req.url.split("?")[0]}`; + const route = routes[key]; + const status = route ? route.status || 200 : 404; + const body = JSON.stringify(route ? route.body : { error: "not_found", message: "no route" }); + res.writeHead(status, { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(body) }); + res.end(body); + }); + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => resolve({ server, port: server.address().port, requests })); + }); +} + +const BUILD_ID = "aeae7611-6d48-4601-8654-80169314e5ec"; + +const BUILD_DETAIL = { + id: BUILD_ID, + status: "failed", + overall_status: "failed", + build_type: "custom", + target_type: null, + commit_hash: "p4-207", + ci_job_name: "PilotLight_Verify", + duration: 0.764199, + created_at: "2026-09-18T10:55:02-04:00", + log_available: false, + log_unavailable_reason: + "TeamCity cannot be polled by ButterStack, so its logs must be pushed on the webhook payload as `logs_tail`.", + steps: [{ step_type: "external_build", status: "failed", duration: null, message: "TeamCity build failed" }], + investigation: null +}; + +const INVESTIGATION = { + investigation_id: "inv-1", + status: "completed", + diagnosis_category: "compile_error", + severity: "high", + confidence: "medium", + summary: "Godot parse error in main.gd", + diagnosis: "main.gd calls show_results() with 7 arguments against a 6-parameter definition.", + suggested_fix: "Reconcile screen_flow.gd with the current show_results() signature.", + attributed_user_id: null, + attributed_changelist: null, + affected_files: ["src/main.gd"], + evidence: ["SCRIPT ERROR: Parse Error: Too many arguments"], + error_message: null, + steps: [{ step: 1, step_type: "tool_use", content: null }] +}; + +test("#1938: `builds show` renders a build instead of exiting 0 with no output", async () => { + const home = mkHome(); + const { server, port } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}`]: { body: BUILD_DETAIL } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stdout, success } = await runButter(["builds", "show", BUILD_ID, "--project", "108"], { home }); + + assert.equal(success, true); + assert.ok(stdout.includes(BUILD_ID), "the build id should be printed"); + assert.ok(stdout.includes("PilotLight_Verify"), "the job name should be printed"); + assert.ok(stdout.includes("p4-207"), "the commit should be printed"); + assert.ok(stdout.trim().length > 0, "the whole defect was that this printed nothing"); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1940: `builds show` says the build log is missing, and why", async () => { + const home = mkHome(); + const { server, port } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}`]: { body: BUILD_DETAIL } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stdout } = await runButter(["builds", "show", BUILD_ID, "--project", "108"], { home }); + + assert.ok(stdout.includes("not available"), "a missing log must be called out"); + assert.ok(stdout.includes("logs_tail"), "and the reason must name the fix"); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1938: a boolean flag before a positional no longer swallows it", async () => { + const home = mkHome(); + const { server, port } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}`]: { body: BUILD_DETAIL } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + + // --json BEFORE the build id: the exact shape that used to parse as + // json: "" and leave no positional argument behind. + const before = await runButter(["builds", "show", "--json", BUILD_ID, "--project", "108"], { home }); + assert.equal(before.success, true, `expected success, got stderr: ${before.stderr}`); + assert.equal(JSON.parse(before.stdout).id, BUILD_ID); + + // ...and after it, which always worked, must keep working. + const after = await runButter(["builds", "show", BUILD_ID, "--project", "108", "--json"], { home }); + assert.equal(after.success, true); + assert.equal(JSON.parse(after.stdout).id, BUILD_ID); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1938: `builds investigation` reads the result with GET, never POST", async () => { + const home = mkHome(); + const { server, port, requests } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}/investigation`]: { body: INVESTIGATION } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stdout, success } = await runButter( + ["builds", "investigation", BUILD_ID, "--project", "108"], + { home } + ); + + assert.equal(success, true); + assert.ok(stdout.includes("Godot parse error in main.gd")); + assert.ok(stdout.includes("compile_error")); + + const req = await requests.pop(2000); + assert.equal(req.method, "GET", "reading a result must not be a POST -- a POST is a paid AI call"); + assert.ok(req.url.endsWith("/investigation")); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1940: an unattributed investigation prints 'unattributed', not a guess", async () => { + const home = mkHome(); + const { server, port } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}/investigation`]: { body: INVESTIGATION } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stdout } = await runButter(["builds", "investigation", BUILD_ID, "--project", "108"], { home }); + assert.ok(stdout.includes("unattributed")); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1938: a missing investigation is an explicit message, not an empty success", async () => { + const home = mkHome(); + const { server, port } = await startApiServer({}); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stderr, success } = await runButter( + ["builds", "investigation", BUILD_ID, "--project", "108"], + { home } + ); + + assert.equal(success, false, "nothing to read is a failure exit, not a silent 0"); + assert.ok(stderr.includes("No investigation has been run")); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +}); + +test("#1938: an unknown subcommand fails loudly instead of exiting 0 silently", async () => { + const home = mkHome(); + try { + const { stderr, success } = await runButter(["builds", "frobnicate", "--project", "108"], { home }); + assert.equal(success, false); + assert.ok(stderr.includes("Unknown subcommand")); + } finally { + rmHome(home); + } +}); + +test("#1938: a server with no investigation key is not reported as 'no investigation'", async () => { + const home = mkHome(); + const { investigation, ...withoutKey } = BUILD_DETAIL; // eslint-disable-line no-unused-vars + const { server, port } = await startApiServer({ + [`GET /api/v1/projects/108/build_runs/${BUILD_ID}`]: { body: withoutKey } + }); + try { + writeCredentials(home, { host: `http://127.0.0.1:${port}` }); + const { stdout } = await runButter(["builds", "show", BUILD_ID, "--project", "108"], { home }); + + assert.ok(stdout.includes("does not expose investigations")); + assert.ok( + !stdout.includes("No AI investigation has been run"), + "an old server must not be reported as a build with no investigation" + ); + } finally { + await stopCaptureServer(server); + rmHome(home); + } +});