Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/init-json-envelope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@taskless/cli": patch
---

`init --json` now writes only the parseable envelope to stdout. Previously,
the non-interactive install path (also reached from `init --no-interactive
--json`) unconditionally logged human-readable prose — the "no tools
detected" fallback notice and the per-target skill/command summary — to
stdout ahead of the JSON envelope, so `taskless init --json | jq .` failed
with a JSON parse error. That prose now goes to stderr, where it stays
visible to a person watching the terminal without corrupting a machine
consumer's view of stdout, matching how `verify`/`test` and the migration
notice already behave under `--json`.
54 changes: 38 additions & 16 deletions packages/cli/src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const initCommand = defineCommand({
);
}

const result = await runNonInteractive(cwd);
const result = await runNonInteractive(cwd, { json: args.json });
if (args.json) {
console.log(
JSON.stringify({
Expand Down Expand Up @@ -245,11 +245,20 @@ export const updateCommand = defineCommand({
},
});

async function runNonInteractive(cwd: string): Promise<{
async function runNonInteractive(
cwd: string,
options: { json?: boolean } = {}
): Promise<{
commandsInstalled: boolean;
reloadNotice: string | undefined;
migrated: MigrationReport | undefined;
}> {
// Under `--json`, stdout carries only the envelope printed by the caller.
// This per-target summary is not on that envelope (it is finer-grained than
// `migrated`/`commandsInstalled`), so rather than drop it, it goes to
// stderr — visible to a person watching the terminal, invisible to a
// machine consumer parsing stdout.
const log = options.json ? console.error : console.log;
Comment thread
thecodedrift marked this conversation as resolved.
// Sampled BEFORE the directory is created, and that order is the whole
// point. `ensureTasklessDirectory` mkdir -p's, so afterwards a pre-existing
// project is indistinguishable from a fresh one.
Expand All @@ -263,7 +272,24 @@ async function runNonInteractive(cwd: string): Promise<{
// can report what a migration moved. `check`, `verify` and `test` used to
// carry this on their own envelopes and refuse rather than migrate now, so
// the field followed the behaviour rather than being dropped.
const migrated = await ensureTasklessDirectory(cwd);
//
// The migration notice is suppressed entirely under `--json`, rather than
// moved to stderr like the per-target summary above: unlike that summary,
// this information IS already on the envelope, as `migrated`, so printing
// it a second time would just be noise. This is the actual `verify`/`test`
// convention (`verify.ts`'s `onNotice: (message) => { if (!json)
// console.error(message); }`), and the case `EnsureOptions.onNotice`'s own
// doc comment describes: "callers that emit `--json` should pass a
// callback that suppresses output under that flag: the same information is
// on the envelope's `migrated` field". Omitting `onNotice` here, as before,
// left it on the default fallback (unconditional `console.error`), which
// never corrupts stdout but doesn't suppress the duplicate under `--json`
// either — the gap a reviewer of this PR caught.
const migrated = await ensureTasklessDirectory(cwd, {
onNotice: (message: string) => {
if (!options.json) console.error(message);
},
});
if (wasNewProject) {
// A project this CLI just created has no entries to walk: everything the
// ledger describes is already true of the scaffold it wrote.
Expand Down Expand Up @@ -291,7 +317,7 @@ async function runNonInteractive(cwd: string): Promise<{
const reloadNotice = getReloadNotice({ previousCliVersion, cliVersion });

if (detected.length === 0) {
console.log(`No tools detected. Using fallback: ${DEFAULT_SHIM_DIR}/`);
log(`No tools detected. Using fallback: ${DEFAULT_SHIM_DIR}/`);
}

const skillsByTarget = groupValuesByTarget(
Expand Down Expand Up @@ -332,33 +358,29 @@ async function runNonInteractive(cwd: string): Promise<{
removedSkills.length === 0 &&
removedCommands.length === 0
) {
console.log(`${target.label} (${target.dir}/): up to date`);
log(`${target.label} (${target.dir}/): up to date`);
continue;
}

console.log(
log(
`${target.label} (${target.dir}/): wrote ${String(writtenSkills.length)} skill ${noun}(s)`
);
for (const name of writtenSkills) {
console.log(` - ${name}`);
log(` - ${name}`);
}
if (writtenCommands.length > 0) {
console.log(` + ${String(writtenCommands.length)} command ${noun}(s)`);
log(` + ${String(writtenCommands.length)} command ${noun}(s)`);
}
if (removedSkills.length > 0) {
console.log(
` removed ${String(removedSkills.length)} obsolete skill(s):`
);
log(` removed ${String(removedSkills.length)} obsolete skill(s):`);
for (const name of removedSkills) {
console.log(` - ${name}`);
log(` - ${name}`);
}
}
if (removedCommands.length > 0) {
console.log(
` removed ${String(removedCommands.length)} obsolete command(s):`
);
log(` removed ${String(removedCommands.length)} obsolete command(s):`);
for (const name of removedCommands) {
console.log(` - ${name}`);
log(` - ${name}`);
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions packages/cli/test/error-envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ async function runCli(
function parseEnvelope(stdout: string): ErrorEnvelope {
// The envelope is the last JSON line in stdout. (Some commands also
// print progress to stderr, so we ignore that.)
const lines = stdout.split("\n").filter((l) => l.trim().startsWith("{"));
expect(lines.length).toBeGreaterThan(0);
const last = lines.at(-1)!;
return JSON.parse(last) as ErrorEnvelope;
return JSON.parse(stdout.trim()) as ErrorEnvelope;
}

describe("standardized error envelope (--json)", () => {
Expand Down
33 changes: 31 additions & 2 deletions packages/cli/test/migrated-envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,18 @@ async function runCli(
}
}

/**
* Parse `--json` stdout as the WHOLE envelope, not just its last line.
*
* `stdout.trim().split("\n").at(-1)` was the earlier shape of this helper,
* and it is exactly why #279 (`init --json` printing prose ahead of the
* envelope) stayed invisible: a helper that only ever reads the last line
* cannot fail on anything printed before it. `JSON.parse` on the trimmed
* whole string fails loudly the moment stdout carries a second thing,
* whichever end it lands on.
*/
function parseEnvelope(stdout: string): Record<string, unknown> {
const line = stdout.trim().split("\n").at(-1) ?? "";
return JSON.parse(line) as Record<string, unknown>;
return JSON.parse(stdout.trim()) as Record<string, unknown>;
}

/** The versions a project seeded at 3 must be carried through. */
Expand Down Expand Up @@ -120,6 +129,26 @@ describe("who migrates, and who refuses", () => {
expectSeededMigration(envelope.migrated);
});

it("init --json reports the migration on stdout and stays silent about it on stderr", async () => {
// The migration notice duplicates the envelope's `migrated` field, so
// under `--json` it is suppressed rather than moved to stderr - unlike
// the per-target install summary, which stderr DOES carry under `--json`
// because that detail has no field of its own. Catches a regression that
// routes this notice back through the unconditional `console.error`
// fallback `ensureTasklessDirectory` uses when no `onNotice` is passed.
await seedVersion3();

const { stderr } = await runCli([
"init",
"--no-interactive",
"--json",
"-d",
temporaryDirectory,
]);

expect(stderr).not.toContain("Migrat");
});

it("init --json omits the field when nothing migrated", async () => {
// Absence is the signal, so a consumer never reads empty arrays to decide.
await seedVersion3();
Expand Down
31 changes: 23 additions & 8 deletions packages/cli/test/no-implicit-migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,21 @@ async function runCli(
}
}

/**
* Parse `--json` stdout as the WHOLE envelope, not just its last line.
*
* The earlier shape of every call site here was
* `JSON.parse(stdout.trim().split("\n").at(-1) ?? "{}")`, and that is exactly
* why `init --json` printing prose ahead of its envelope (#279) went
* undetected: a helper that only ever reads the last line cannot fail on
* anything printed before it. `JSON.parse` on the trimmed whole string fails
* loudly the moment stdout carries a second thing, whichever end it lands on
* — do not narrow this back to a last-line read.
*/
function parseEnvelope<T>(stdout: string): T {
return JSON.parse(stdout.trim()) as T;
}

const FLAT_RULE =
"id: no-eval\nlanguage: TypeScript\nseverity: error\nmessage: no eval\nrule:\n pattern: eval($A)\n";

Expand Down Expand Up @@ -114,10 +129,10 @@ describe("a reporting command never migrates", () => {
"%s --json carries the code an agent branches on",
async (command) => {
const { stdout } = await runCli([command, "--json", "-d", directory]);
const envelope = JSON.parse(stdout.trim().split("\n").at(-1) ?? "{}") as {
const envelope = parseEnvelope<{
ok?: boolean;
code?: string;
};
}>(stdout);
expect(envelope.ok).toBe(false);
// Distinct from SCAFFOLD_VERSION_MISMATCH, which is the opposite
// direction and asks the caller to upgrade the CLI instead.
Expand Down Expand Up @@ -231,9 +246,9 @@ describe("a reporting command never migrates", () => {
directory,
]);

const envelope = JSON.parse(stdout.trim().split("\n").at(-1) ?? "{}") as {
const envelope = parseEnvelope<{
migrated?: { from: number; to: number };
};
}>(stdout);
expect(envelope.migrated?.from).toBe(3);
expect(envelope.migrated?.to).toBe(LATEST_SCHEMA_VERSION);
await expect(
Expand Down Expand Up @@ -278,11 +293,11 @@ describe("a manifest that cannot be parsed", () => {
"%s --json reports the file, not a version it guessed",
async (command) => {
const { stdout } = await runCli([command, "--json", "-d", directory]);
const envelope = JSON.parse(stdout.trim().split("\n").at(-1) ?? "{}") as {
const envelope = parseEnvelope<{
ok?: boolean;
code?: string;
message?: string;
};
}>(stdout);
expect(envelope.ok).toBe(false);
expect(envelope.code).toBe("SCAFFOLD_MANIFEST_UNREADABLE");
expect(envelope.message).toContain("taskless.json");
Expand Down Expand Up @@ -331,9 +346,9 @@ describe("a manifest that cannot be parsed", () => {
"-d",
bare,
]);
const envelope = JSON.parse(stdout.trim().split("\n").at(-1) ?? "{}") as {
const envelope = parseEnvelope<{
migrated?: { from: number; to: number };
};
}>(stdout);
expect(envelope.migrated?.from).toBe(0);
expect(envelope.migrated?.to).toBe(LATEST_SCHEMA_VERSION);
} finally {
Expand Down
Loading