Skip to content

Commit 922a650

Browse files
committed
fix: api command preserves raw string responses, fix null body header divergence
Address three bot review comments: 1. String response quoting regression (Seer + BugBot): the api command is a raw proxy — non-JSON responses (plain text, HTML error pages) must not be wrapped in JSON quotes. Revert to output: 'json' (flag-only) and write response body imperatively via writeResponseBody, which writes strings directly and JSON-formats objects with --fields. 2. Null body header divergence (BugBot): resolveEffectiveHeaders had an extra body !== null check that rawApiRequest doesn't. For --data "null", dry-run would omit Content-Type while the real request adds it. Aligned condition to match rawApiRequest exactly: !(isStringBody || hasContentType) && body !== undefined. 3. Stale comment about dry-run always outputting JSON (BugBot on old commit 70e7f2d): the referenced code was completely rewritten in 6980508. No action needed — will reply to dismiss. Also reverts OutputConfig.human back to required (the JSON-only config form is no longer needed since the api command doesn't use it).
1 parent 997570c commit 922a650

File tree

3 files changed

+113
-33
lines changed

3 files changed

+113
-33
lines changed

src/commands/api.ts

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ type ApiFlags = {
2727
readonly silent: boolean;
2828
readonly verbose: boolean;
2929
readonly "dry-run": boolean;
30-
/** Injected by buildCommand via output: { json: true } */
30+
/** Injected by buildCommand via output: "json" */
3131
readonly json: boolean;
32-
/** Injected by buildCommand via output: { json: true } */
32+
/** Injected by buildCommand via output: "json" */
3333
readonly fields?: string[];
3434
};
3535

@@ -864,6 +864,31 @@ export function writeResponseHeaders(
864864
stdout.write("\n");
865865
}
866866

867+
/**
868+
* Write API response body to stdout.
869+
*
870+
* Preserves raw strings (plain text, HTML error pages) without JSON quoting.
871+
* Objects/arrays are JSON-formatted with optional `--fields` filtering.
872+
* Null/undefined bodies produce no output.
873+
*
874+
* @internal Exported for testing
875+
*/
876+
export function writeResponseBody(
877+
stdout: Writer,
878+
body: unknown,
879+
fields?: string[]
880+
): void {
881+
if (body === null || body === undefined) {
882+
return;
883+
}
884+
885+
if (typeof body === "object") {
886+
writeJson(stdout, body, fields);
887+
} else {
888+
stdout.write(`${String(body)}\n`);
889+
}
890+
}
891+
867892
/**
868893
* Write verbose request output (curl-style format)
869894
* @internal Exported for testing
@@ -933,13 +958,14 @@ export function resolveEffectiveHeaders(
933958
customHeaders: Record<string, string> | undefined,
934959
body: unknown
935960
): Record<string, string> {
961+
// Mirror rawApiRequest exactly: auto-add Content-Type for any non-string,
962+
// non-undefined body when no Content-Type was explicitly provided.
963+
const isStringBody = typeof body === "string";
936964
const headers = { ...(customHeaders ?? {}) };
937-
if (
938-
body !== undefined &&
939-
body !== null &&
940-
typeof body !== "string" &&
941-
!Object.keys(headers).some((k) => k.toLowerCase() === "content-type")
942-
) {
965+
const hasContentType = Object.keys(headers).some(
966+
(k) => k.toLowerCase() === "content-type"
967+
);
968+
if (!(isStringBody || hasContentType) && body !== undefined) {
943969
headers["Content-Type"] = "application/json";
944970
}
945971
return headers;
@@ -1068,7 +1094,7 @@ export async function resolveBody(
10681094
// Command Definition
10691095

10701096
export const apiCommand = buildCommand({
1071-
output: { json: true },
1097+
output: "json",
10721098
docs: {
10731099
brief: "Make an authenticated API request",
10741100
fullDescription:
@@ -1191,16 +1217,20 @@ export const apiCommand = buildCommand({
11911217
? parseHeaders(flags.header)
11921218
: undefined;
11931219

1220+
// Dry-run mode: preview the request that would be sent
11941221
// Dry-run mode: preview the request that would be sent
11951222
if (flags["dry-run"]) {
1196-
return {
1197-
data: {
1223+
writeJson(
1224+
stdout,
1225+
{
11981226
method: flags.method,
11991227
url: resolveRequestUrl(normalizedEndpoint, params),
12001228
headers: resolveEffectiveHeaders(headers, body),
12011229
body: body ?? null,
12021230
},
1203-
};
1231+
flags.fields
1232+
);
1233+
return;
12041234
}
12051235

12061236
// Verbose mode: show request details before the response
@@ -1225,21 +1255,22 @@ export const apiCommand = buildCommand({
12251255
return;
12261256
}
12271257

1228-
// Output response headers when requested (side-effect before body)
1258+
// Output response headers when requested
12291259
if (flags.verbose) {
12301260
writeVerboseResponse(stdout, response.status, response.headers);
12311261
} else if (flags.include) {
12321262
writeResponseHeaders(stdout, response.status, response.headers);
12331263
}
12341264

1235-
// Error responses: Stricli overwrites process.exitCode after the
1236-
// command returns, so we must use process.exit(1) directly.
1237-
// Return { data } for success so the output wrapper renders it.
1265+
// Write response body — preserve raw strings, JSON-format objects.
1266+
// The api command is a raw proxy; non-JSON responses (plain text,
1267+
// HTML error pages) must not be wrapped in JSON string quotes.
1268+
writeResponseBody(stdout, response.body, flags.fields);
1269+
1270+
// Error exit: Stricli overwrites process.exitCode after the command
1271+
// returns, so we must call process.exit(1) directly.
12381272
if (isError) {
1239-
writeJson(stdout, response.body, flags.fields);
12401273
process.exit(1);
12411274
}
1242-
1243-
return { data: response.body };
12441275
},
12451276
});

src/lib/formatters/output.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type WriteOutputOptions<T> = {
6060
/**
6161
* Output configuration declared on `buildCommand` for automatic rendering.
6262
*
63-
* Three forms:
63+
* Two forms:
6464
*
6565
* 1. **Flag-only** — `output: "json"` — injects `--json` and `--fields` flags
6666
* but does not intercept returns. Commands handle their own output.
@@ -69,23 +69,14 @@ type WriteOutputOptions<T> = {
6969
* AND auto-renders the command's return value. Commands return
7070
* `{ data }` or `{ data, hint }` objects.
7171
*
72-
* 3. **JSON-only config** — `output: { json: true }` — like full config but
73-
* without a `human` formatter. Data is always serialized as JSON.
74-
*
7572
* @typeParam T - Type of data the command returns (used by `human` formatter
7673
* and serialized as-is to JSON)
7774
*/
7875
export type OutputConfig<T> = {
7976
/** Enable `--json` and `--fields` flag injection */
8077
json: true;
81-
/**
82-
* Format data as a human-readable string for terminal output.
83-
*
84-
* When omitted the command is **JSON-only**: data is always serialized
85-
* as JSON regardless of whether `--json` was passed. The `--json` and
86-
* `--fields` flags are still injected for consistency.
87-
*/
88-
human?: (data: T) => string;
78+
/** Format data as a human-readable string for terminal output */
79+
human: (data: T) => string;
8980
/**
9081
* Top-level keys to strip from JSON output.
9182
*
@@ -145,8 +136,7 @@ export function renderCommandOutput(
145136
config: OutputConfig<any>,
146137
ctx: RenderContext
147138
): void {
148-
// JSON mode: explicit --json flag, or no human formatter (JSON-only command)
149-
if (ctx.json || !config.human) {
139+
if (ctx.json) {
150140
let jsonData = data;
151141
if (
152142
config.jsonExclude &&

test/commands/api.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
resolveEffectiveHeaders,
2929
resolveRequestUrl,
3030
setNestedValue,
31+
writeResponseBody,
3132
writeResponseHeaders,
3233
writeVerboseRequest,
3334
writeVerboseResponse,
@@ -912,6 +913,54 @@ describe("writeResponseHeaders", () => {
912913
});
913914
});
914915

916+
describe("writeResponseBody", () => {
917+
test("writes JSON object with pretty-printing", () => {
918+
const writer = createMockWriter();
919+
writeResponseBody(writer, { key: "value", num: 42 });
920+
expect(writer.output).toBe('{\n "key": "value",\n "num": 42\n}\n');
921+
});
922+
923+
test("writes JSON array with pretty-printing", () => {
924+
const writer = createMockWriter();
925+
writeResponseBody(writer, [1, 2, 3]);
926+
expect(writer.output).toBe("[\n 1,\n 2,\n 3\n]\n");
927+
});
928+
929+
test("writes string directly without JSON quoting", () => {
930+
const writer = createMockWriter();
931+
writeResponseBody(writer, "plain text response");
932+
expect(writer.output).toBe("plain text response\n");
933+
});
934+
935+
test("writes number as string", () => {
936+
const writer = createMockWriter();
937+
writeResponseBody(writer, 42);
938+
expect(writer.output).toBe("42\n");
939+
});
940+
941+
test("does not write null", () => {
942+
const writer = createMockWriter();
943+
writeResponseBody(writer, null);
944+
expect(writer.output).toBe("");
945+
});
946+
947+
test("does not write undefined", () => {
948+
const writer = createMockWriter();
949+
writeResponseBody(writer, undefined);
950+
expect(writer.output).toBe("");
951+
});
952+
953+
test("applies --fields filtering to objects", () => {
954+
const writer = createMockWriter();
955+
writeResponseBody(writer, { id: "1", name: "test", extra: "data" }, [
956+
"id",
957+
"name",
958+
]);
959+
const parsed = JSON.parse(writer.output);
960+
expect(parsed).toEqual({ id: "1", name: "test" });
961+
});
962+
});
963+
915964
describe("writeVerboseRequest", () => {
916965
test("writes method and endpoint", () => {
917966
const writer = createMockWriter();
@@ -1111,6 +1160,16 @@ describe("resolveEffectiveHeaders", () => {
11111160
const headers = resolveEffectiveHeaders(undefined, undefined);
11121161
expect(headers).toEqual({});
11131162
});
1163+
1164+
test("adds Content-Type for null body (matches rawApiRequest)", () => {
1165+
const headers = resolveEffectiveHeaders(undefined, null);
1166+
expect(headers["Content-Type"]).toBe("application/json");
1167+
});
1168+
1169+
test("does not add Content-Type for undefined body", () => {
1170+
const headers = resolveEffectiveHeaders(undefined, undefined);
1171+
expect(headers["Content-Type"]).toBeUndefined();
1172+
});
11141173
});
11151174

11161175
// --data/-d and JSON auto-detection (CLI-AF)

0 commit comments

Comments
 (0)