Skip to content

Commit 221eac2

Browse files
ryanlitalienclaude
andcommitted
fix(#1938): add the builds read path, and stop flags from eating positionals
Three defects, all of which made a build's data look absent rather than unreachable. `butter builds show <id>` printed nothing and exited 0. The subcommand was never wired into the dispatcher, so it fell through the switch and out of main(). An exit 0 with no output reads as "this build has no data", which is why it was reported as a backend problem - the endpoint existed and worked the whole time. Unknown subcommands now fail loudly instead of returning 0. `butter builds investigate --json <id>` appeared to print nothing while the plain form rendered fine. parseArgs treated every `--flag` as taking a value whenever the next argv entry did not start with `-`, so `--json` before a positional parsed as `json: "<build id>"` and left no build id behind; the command printed its usage error and exited 1. Position in the command line must not change what a boolean flag means, so switches are now declared, and `--key=value` is honored for any key. There was no GET counterpart to `investigate`. Checking whether an investigation had finished meant POSTing again, and each POST is a paid AI call. `butter builds investigation <id>` reads the result for free, `builds show` embeds it, and `investigate` now points at the read path rather than letting callers poll by re-POSTing. `builds show` also distinguishes three cases a single falsy check used to collapse: an investigation exists, none has been run, and the server predates the read path. Reporting an old server as "no investigation" is the exact false negative that sent people back to POSTing to find out. Attribution prints "unattributed" when the investigation made none, rather than omitting the line. A failure caused by a file that was never submitted has no offending changelist and no culpable author, and that is a real answer. Minor version: four new behaviours, no breaking changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 874cd0b commit 221eac2

3 files changed

Lines changed: 430 additions & 12 deletions

File tree

‎bin/butter‎

Lines changed: 216 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,161 @@ async function buildsList(args) {
540540
}
541541
}
542542

543+
async function buildsShow(args) {
544+
const projectId = args.project || loadConfig().defaultProject;
545+
const buildId = args._[2];
546+
if (!projectId || !buildId) {
547+
console.error(`${colors.err}✗ Usage: butter builds show <build_id> --project <id>${colors.reset}`);
548+
process.exit(1);
549+
}
550+
551+
try {
552+
const res = await request("GET", `/api/v1/projects/${projectId}/build_runs/${buildId}`);
553+
if (args.json) {
554+
console.log(JSON.stringify(res, null, 2));
555+
return;
556+
}
557+
558+
const statusColor =
559+
res.status === "completed" ? colors.ok : res.status === "failed" ? colors.err : colors.warn;
560+
console.log(`\n${colors.bold}BUILD ${res.id}${colors.reset}`);
561+
console.log("----------------------------------------------------------------------");
562+
console.log(` Status: ${statusColor}${String(res.status).toUpperCase()}${colors.reset}`);
563+
console.log(` Overall: ${res.overall_status || "-"}`);
564+
console.log(` Job: ${res.ci_job_name || "-"}`);
565+
console.log(` Commit: ${res.commit_hash || "-"}`);
566+
console.log(` Type: ${res.build_type || "-"}${res.target_type ? ` / ${res.target_type}` : ""}`);
567+
console.log(` Duration: ${res.duration != null ? `${res.duration}s` : "-"}`);
568+
console.log(` Created: ${res.created_at || "-"}`);
569+
570+
// #1940: an investigation with no log to read cannot say anything useful,
571+
// so surface that here rather than leaving it to be inferred from a
572+
// vague diagnosis.
573+
if (res.log_available === false) {
574+
console.log(` Build log: ${colors.warn}not available${colors.reset}`);
575+
if (res.log_unavailable_reason) console.log(` ${colors.muted}${res.log_unavailable_reason}${colors.reset}`);
576+
} else if (res.log_available === true) {
577+
console.log(` Build log: ${colors.ok}available${colors.reset}`);
578+
}
579+
580+
if (Array.isArray(res.steps) && res.steps.length) {
581+
console.log(`\n${colors.bold} Steps:${colors.reset}`);
582+
res.steps.forEach((s) => {
583+
const c = s.status === "completed" ? colors.ok : s.status === "failed" ? colors.err : colors.warn;
584+
console.log(
585+
` ${c}[${s.status}]${colors.reset} ${s.step_type}${s.message ? ` ${colors.muted}- ${s.message}${colors.reset}` : ""}`
586+
);
587+
});
588+
}
589+
590+
// Three distinct cases, and conflating them is how #1938 stayed confusing:
591+
// an investigation exists; none has been run; or the server predates the
592+
// read path and cannot tell us either way. A server without the key must
593+
// not be reported as "no investigation" - that is the exact false negative
594+
// that sent people back to POSTing `investigate` to find out.
595+
if (res.investigation) {
596+
console.log("");
597+
printInvestigation(res.investigation);
598+
} else if (Object.prototype.hasOwnProperty.call(res, "investigation")) {
599+
console.log(`\n ${colors.muted}No AI investigation has been run for this build.${colors.reset}`);
600+
console.log(` ${colors.muted}Run: butter builds investigate ${res.id} --project ${projectId}${colors.reset}`);
601+
} else {
602+
console.log(`\n ${colors.warn}This ButterStack server does not expose investigations on the build detail.${colors.reset}`);
603+
console.log(` ${colors.muted}Try: butter builds investigation ${res.id} --project ${projectId}${colors.reset}`);
604+
}
605+
console.log("");
606+
} catch (err) {
607+
console.error(`${colors.err}✗ Failed to show build: ${err.message}${colors.reset}`);
608+
process.exit(1);
609+
}
610+
}
611+
612+
function printInvestigation(inv) {
613+
const statusColor =
614+
inv.status === "completed" ? colors.ok : inv.status === "failed" ? colors.err : colors.warn;
615+
console.log(`${colors.bold}AI Failure Investigation:${colors.reset}`);
616+
console.log(` Status: ${statusColor}${inv.status}${colors.reset}`);
617+
618+
if (inv.status === "pending" || inv.status === "running") {
619+
console.log(` ${colors.muted}Still in flight. Re-read with \`butter builds investigation\` - do not`);
620+
console.log(` re-run \`investigate\`, which starts a new paid analysis.${colors.reset}`);
621+
return;
622+
}
623+
624+
if (inv.status === "failed") {
625+
console.log(` Error: ${colors.err}${inv.error_message || "unknown"}${colors.reset}`);
626+
return;
627+
}
628+
629+
if (inv.diagnosis_category) {
630+
console.log(` Category: ${inv.diagnosis_category}${inv.severity ? ` (${inv.severity})` : ""}`);
631+
}
632+
if (inv.confidence) console.log(` Confidence: ${inv.confidence}`);
633+
if (inv.summary) console.log(` Summary: ${inv.summary}`);
634+
if (inv.diagnosis) console.log(` Diagnosis: ${inv.diagnosis}`);
635+
if (inv.suggested_fix) console.log(` Fix: ${colors.accent}${inv.suggested_fix}${colors.reset}`);
636+
637+
// Attribution is printed only when the investigation actually made one.
638+
// "unattributed" is a real, common and correct answer -- a file that was
639+
// never submitted has no offending changelist and no culpable author.
640+
if (inv.attributed_changelist || inv.attributed_user_id) {
641+
console.log(
642+
` Attributed: ${inv.attributed_changelist ? `CL ${inv.attributed_changelist}` : ""}${
643+
inv.attributed_user_id ? ` (user ${inv.attributed_user_id})` : ""
644+
}`
645+
);
646+
} else {
647+
console.log(` Attributed: ${colors.muted}unattributed${colors.reset}`);
648+
}
649+
650+
if (Array.isArray(inv.affected_files) && inv.affected_files.length) {
651+
console.log(` Files: ${inv.affected_files.join(", ")}`);
652+
}
653+
if (Array.isArray(inv.evidence) && inv.evidence.length) {
654+
console.log(`${colors.bold} Evidence:${colors.reset}`);
655+
inv.evidence.forEach((e) => console.log(` ${colors.muted}- ${e}${colors.reset}`));
656+
}
657+
}
658+
659+
// Read-only counterpart to `investigate` (#1938). Before this existed, the
660+
// only way to see a result was to POST investigate again, which spends
661+
// credits and starts a fresh Bedrock run once the previous one has finished.
662+
async function buildsInvestigation(args) {
663+
const projectId = args.project || loadConfig().defaultProject;
664+
const buildId = args._[2];
665+
if (!projectId || !buildId) {
666+
console.error(`${colors.err}✗ Usage: butter builds investigation <build_id> --project <id>${colors.reset}`);
667+
process.exit(1);
668+
}
669+
670+
try {
671+
const res = await request("GET", `/api/v1/projects/${projectId}/build_runs/${buildId}/investigation`);
672+
if (args.json) {
673+
console.log(JSON.stringify(res, null, 2));
674+
return;
675+
}
676+
console.log("");
677+
printInvestigation(res);
678+
if (args.steps && Array.isArray(res.steps) && res.steps.length) {
679+
console.log(`\n${colors.bold} Steps:${colors.reset}`);
680+
res.steps.forEach((s) => {
681+
console.log(` ${colors.muted}${String(s.step).padStart(3)} ${s.step_type}${colors.reset}${s.content ? ` ${s.content}` : ""}`);
682+
});
683+
} else if (Array.isArray(res.steps) && res.steps.length) {
684+
console.log(`\n ${colors.muted}${res.steps.length} step(s). Pass --steps to print the trail.${colors.reset}`);
685+
}
686+
console.log("");
687+
} catch (err) {
688+
if (err.statusCode === 404) {
689+
console.error(`${colors.warn}No investigation has been run for build ${buildId}.${colors.reset}`);
690+
console.error(`${colors.muted}Start one with: butter builds investigate ${buildId} --project ${projectId}${colors.reset}`);
691+
process.exit(1);
692+
}
693+
console.error(`${colors.err}✗ Failed to read investigation: ${err.message}${colors.reset}`);
694+
process.exit(1);
695+
}
696+
}
697+
543698
async function buildsInvestigate(args) {
544699
const projectId = args.project || loadConfig().defaultProject;
545700
const buildId = args._[2];
@@ -549,17 +704,24 @@ async function buildsInvestigate(args) {
549704
}
550705

551706
try {
552-
console.log(`${colors.accent}Triggering AI Failure Investigation for Build #${buildId}...${colors.reset}`);
707+
if (!args.json) {
708+
console.log(`${colors.accent}Triggering AI Failure Investigation for Build #${buildId}...${colors.reset}`);
709+
}
553710
const res = await request("POST", `/api/v1/projects/${projectId}/build_runs/${buildId}/investigate`);
554711
if (args.json) {
555712
console.log(JSON.stringify(res, null, 2));
556713
return;
557714
}
558-
console.log(`\n${colors.bold}AI Failure Investigation Report:${colors.reset}`);
559-
console.log(` Status: ${colors.ok}${res.status}${colors.reset}`);
560-
console.log(` Diagnosis: ${res.diagnosis || res.summary || "Pending analysis..."}`);
561-
if (res.suggested_fix) console.log(` Fix: ${colors.accent}${res.suggested_fix}${colors.reset}`);
562-
if (res.attributed_author) console.log(` Attributed: ${res.attributed_author}`);
715+
console.log("");
716+
printInvestigation(res);
717+
// An investigation runs asynchronously, so the POST almost always returns
718+
// `pending`. Point at the read path rather than letting the caller poll
719+
// by re-POSTing, which is what made this cost money to check on (#1938).
720+
if (res.status === "pending" || res.status === "running") {
721+
console.log(
722+
`\n ${colors.muted}Check the result with: butter builds investigation ${buildId} --project ${projectId}${colors.reset}`
723+
);
724+
}
563725
console.log("");
564726
} catch (err) {
565727
console.error(`${colors.err}✗ Failed to investigate build: ${err.message}${colors.reset}`);
@@ -649,16 +811,45 @@ async function assetsDeny(args) {
649811
}
650812
}
651813

814+
// Flags that are switches, never name/value pairs (#1938).
815+
//
816+
// The parser used to treat every `--flag` as taking a value whenever the next
817+
// argv entry did not start with `-`. That is fine at the end of a line, but
818+
// `butter builds investigate --json <build_id>` parsed as
819+
// `json: "<build_id>"` with no positional left, so the command printed its
820+
// usage error and exited 1 -- looking, from the caller's side, like `--json`
821+
// silently produced no output while the plain form worked. Position in the
822+
// command line must not change what a boolean flag means.
823+
const BOOLEAN_FLAGS = new Set([
824+
"json",
825+
"help",
826+
"h",
827+
"pending",
828+
"verbose",
829+
"yes",
830+
"force",
831+
"no-color",
832+
"steps"
833+
]);
834+
835+
// `--key=value` is always explicit, so it is honored for any key including
836+
// the booleans above (`--json=false` is not special-cased: anything after `=`
837+
// is taken literally, which is the same behavior every other key gets).
652838
function parseArgs(rawArgs) {
653839
const parsed = { _: [] };
654840
for (let i = 0; i < rawArgs.length; i++) {
655841
const arg = rawArgs[i];
656842
if (arg.startsWith("--")) {
657-
const key = arg.substring(2);
658-
if (i + 1 < rawArgs.length && !rawArgs[i + 1].startsWith("-")) {
659-
parsed[key] = rawArgs[++i];
843+
const body = arg.substring(2);
844+
const eq = body.indexOf("=");
845+
if (eq !== -1) {
846+
parsed[body.substring(0, eq)] = body.substring(eq + 1);
847+
} else if (BOOLEAN_FLAGS.has(body)) {
848+
parsed[body] = true;
849+
} else if (i + 1 < rawArgs.length && !rawArgs[i + 1].startsWith("-")) {
850+
parsed[body] = rawArgs[++i];
660851
} else {
661-
parsed[key] = true;
852+
parsed[body] = true;
662853
}
663854
} else if (arg.startsWith("-")) {
664855
const key = arg.substring(1);
@@ -682,7 +873,10 @@ ${colors.bold}COMMANDS:${colors.reset}
682873
${colors.accent}auth${colors.reset} login | whoami | logout Authenticate via browser OAuth loopback
683874
${colors.accent}projects${colors.reset} list | show <id> View and inspect game projects
684875
${colors.accent}tasks${colors.reset} list | create | update Manage tasks, bugs, and backlog
685-
${colors.accent}builds${colors.reset} list | show | investigate Inspect CI/CD engine builds and AI failure logs
876+
${colors.accent}builds${colors.reset} list | show | investigate | investigation
877+
Inspect CI/CD builds and AI failure investigations.
878+
${colors.bold}investigate${colors.reset} STARTS a paid AI analysis;
879+
${colors.bold}investigation${colors.reset} reads the result back for free.
686880
${colors.accent}assets${colors.reset} list | show | approve | deny Review and approve textures, models, and audio
687881
688882
${colors.bold}GLOBAL OPTIONS:${colors.reset}
@@ -732,7 +926,9 @@ async function main() {
732926
case "builds":
733927
case "build":
734928
if (subcmd === "list" || !subcmd) return buildsList(args);
929+
if (subcmd === "show") return buildsShow(args);
735930
if (subcmd === "investigate") return buildsInvestigate(args);
931+
if (subcmd === "investigation") return buildsInvestigation(args);
736932
break;
737933
case "assets":
738934
case "asset":
@@ -745,6 +941,15 @@ async function main() {
745941
printHelp();
746942
process.exit(1);
747943
}
944+
945+
// Reaching here means the command matched but its subcommand did not, and
946+
// every branch above `break`s rather than returning. That used to fall out
947+
// of main() silently: `butter builds show <id>` printed nothing and exited
948+
// 0, which reads as "this build has no data" rather than "this subcommand
949+
// does not exist" (#1938). An unhandled subcommand is an error.
950+
console.error(`${colors.err}Unknown subcommand: ${cmd} ${subcmd || ""}${colors.reset}`.trimEnd());
951+
printHelp();
952+
process.exit(1);
748953
}
749954

750955
if (require.main === module) {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "butterstack-cli",
3-
"version": "0.1.1",
3+
"version": "0.2.0",
44
"description": "Fast, scriptable, zero-dependency command line interface for the ButterStack game development pipeline: auth, projects, tasks, builds, and asset approvals.",
55
"bin": {
66
"butter": "./bin/butter"

0 commit comments

Comments
 (0)