From e26bc119fb91bc31aa0bbb9e0d870fc15e5c2d43 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 31 Aug 2026 22:04:27 -0700 Subject: [PATCH 1/2] fix(rules): let a terminal unsupported say why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answers the generator's N7, which asked whether a terminal `unsupported` reaches a user before they land the change that starts producing them. It does reach a user: `unsupported` is already handled as terminal and calls `fail()` with `RULE_UNSUPPORTED`, on stderr or in the `--json` envelope. What reaches them is the wrong reason. The message was hardcoded to an entitlement explanation and never read `status.error`, which was accurate while the only route to `unsupported` was an account lacking a capability. The service now also terminates a request when the CLI is below the floor a runtime rule needs, with a reason saying so — and we would have told a user on 0.10.x to upgrade their PLAN. They ask an administrator for a capability they already have, and the one command that fixes it is never mentioned. `failed` already surfaced `status.error`; `unsupported` did not. So the service's reason wins whenever it sends one, and the entitlement text is the fallback for an `unsupported` that arrives bare. A blank or whitespace reason counts as none, rather than printing a heading with nothing under it. Extracted to its own module so the message is testable without driving the command, matching the reload-notice precedent. Tests pin the regression directly: given a CLI-floor reason the output must not mention a plan or an administrator, and no branch may read as a failure, since `unsupported` is deliberately not `failed` and a reader told otherwise hunts a service bug instead of the thing they have to change. --- .changeset/unsupported-reason.md | 18 ++++++++ packages/cli/src/commands/rules.ts | 19 ++------- packages/cli/src/rules/unsupported.ts | 29 +++++++++++++ packages/cli/test/unsupported.test.ts | 59 +++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 .changeset/unsupported-reason.md create mode 100644 packages/cli/src/rules/unsupported.ts create mode 100644 packages/cli/test/unsupported.test.ts diff --git a/.changeset/unsupported-reason.md b/.changeset/unsupported-reason.md new file mode 100644 index 00000000..417d05b8 --- /dev/null +++ b/.changeset/unsupported-reason.md @@ -0,0 +1,18 @@ +--- +"@taskless/cli": patch +--- + +A terminal `unsupported` now says why, instead of always blaming the plan. + +The message was hardcoded to an entitlement explanation and ignored the +`error` the service sends with the status. That was accurate while the only +way to reach `unsupported` was an account lacking a capability. The service +now also terminates a request as unsupported when the CLI is below the floor +a runtime rule needs, and a user on an older CLI was being told to upgrade +their plan: they would ask an administrator for a capability they already +had, and the one command that would have fixed it was never mentioned. + +The service's reason now wins whenever it sends one. The entitlement text +remains the fallback for an `unsupported` that arrives without a reason, and +a blank reason counts as none rather than printing a heading with nothing +under it. diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index 152a061c..bdc2481f 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -19,6 +19,7 @@ import { deleteRuleFiles, } from "../rules/files"; import { RULES_DIRECTORY } from "../rules/layout"; +import { unsupportedMessage } from "../rules/unsupported"; import { inputSchema as createInputSchema, outputSchema as createOutputSchema, @@ -43,20 +44,6 @@ function getTimestamp(): string { const POLL_INTERVAL_MS = 15_000; -/** - * `unsupported` is a terminal status: the request asked for a rule generation - * the account can't access — e.g. runtime rules that aren't enabled on the - * current plan. It is not a transient failure to retry; the plan or entitlement - * has to change first. - */ -function unsupportedMessage(): string { - return [ - "This rule generation isn't available on your current Taskless plan.", - "", - "It may need a capability that isn't enabled for your organization yet (for example, runtime rules). Ask your Taskless administrator or upgrade your plan to enable it.", - ].join("\n"); -} - const createCommand = defineCommand({ meta: { name: "create", @@ -223,7 +210,7 @@ const createCommand = defineCommand({ break; } case "unsupported": { - fail(unsupportedMessage(), "RULE_UNSUPPORTED"); + fail(unsupportedMessage(status.error), "RULE_UNSUPPORTED"); break; } case "failed": { @@ -480,7 +467,7 @@ const improveCommand = defineCommand({ break; } case "unsupported": { - fail(unsupportedMessage(), "RULE_UNSUPPORTED"); + fail(unsupportedMessage(status.error), "RULE_UNSUPPORTED"); break; } case "failed": { diff --git a/packages/cli/src/rules/unsupported.ts b/packages/cli/src/rules/unsupported.ts new file mode 100644 index 00000000..9aa6e81b --- /dev/null +++ b/packages/cli/src/rules/unsupported.ts @@ -0,0 +1,29 @@ +/** + * `unsupported` is a terminal status: the request was understood, and the + * answer is that it cannot be served as asked. Not a transient failure to + * retry, and deliberately not `failed` — nothing failed, so a user reading + * "generation failed" would go looking for a service bug rather than for the + * thing they actually have to change. + * + * WHAT HAS TO CHANGE IS NOT ALWAYS THE PLAN, AND WE USED TO SAY IT WAS. This + * message was hardcoded to an entitlement explanation and ignored the `error` + * the service sends with the status. The service now terminates a request as + * unsupported when the CLI is below the floor a runtime rule needs, with a + * reason that says so — and a user on an old CLI was being told to upgrade + * their PLAN. They would have asked an administrator for a capability they + * already had, and the one command that would have fixed it was never + * mentioned. + * + * So the service's reason wins whenever it sends one, and the entitlement text + * is the fallback for a terminal `unsupported` that arrives without one. + */ +export function unsupportedMessage(reason?: string): string { + if (reason !== undefined && reason.trim() !== "") { + return `This rule generation can't be served as requested.\n\n${reason}`; + } + return [ + "This rule generation isn't available on your current Taskless plan.", + "", + "It may need a capability that isn't enabled for your organization yet (for example, runtime rules). Ask your Taskless administrator or upgrade your plan to enable it.", + ].join("\n"); +} diff --git a/packages/cli/test/unsupported.test.ts b/packages/cli/test/unsupported.test.ts new file mode 100644 index 00000000..235f5513 --- /dev/null +++ b/packages/cli/test/unsupported.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; + +import { unsupportedMessage } from "../src/rules/unsupported"; + +/** + * A terminal `unsupported` is the service saying "understood, and no". What it + * cannot say for us is WHY, so the reason it sends has to reach the reader. + * + * This used to be one hardcoded sentence about plans and entitlements, written + * when the only way to reach `unsupported` was an account that lacked a + * capability. The service now also terminates a request as unsupported when + * the CLI is below the floor a runtime rule needs — the same status, an + * entirely different fix. + */ + +const CLI_FLOOR = + "This request needs a runtime rule, which requires Taskless CLI 0.11.1 or newer."; + +describe("explaining a terminal unsupported", () => { + it("uses the reason the service sent", () => { + const message = unsupportedMessage(CLI_FLOOR); + expect(message).toContain(CLI_FLOOR); + }); + + it("does not tell a reader to upgrade their plan when the CLI is the problem", () => { + // The specific regression. A user on 0.10.x was told to ask an + // administrator for a capability they already had, while the one command + // that would have fixed it went unmentioned. + const message = unsupportedMessage(CLI_FLOOR); + expect(message).not.toMatch(/plan/i); + expect(message).not.toMatch(/administrator/i); + }); + + it.each([ + ["no reason at all", undefined], + ["an empty reason", ""], + ["a whitespace-only reason", " \n "], + ])("falls back to the entitlement text given %s", (_label, reason) => { + // The original meaning, kept for an `unsupported` that arrives bare. A + // blank string is not a reason, and printing one would leave the reader + // with a heading and nothing under it. + const message = unsupportedMessage(reason); + expect(message).toMatch(/plan/i); + expect(message).toContain("runtime rules"); + }); + + it("never reads as a failure", () => { + // `unsupported` is deliberately not `failed`: nothing failed, and a reader + // told otherwise goes looking for a service bug instead of for the thing + // they have to change. + for (const message of [ + unsupportedMessage(CLI_FLOOR), + unsupportedMessage(), + ]) { + expect(message).not.toMatch(/\bfailed\b/i); + expect(message).not.toMatch(/\berror\b/i); + } + }); +}); From 3ed03fdacdc8ffe6d021d2009960c6b27b0806a9 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 1 Sep 2026 06:29:49 -0700 Subject: [PATCH 2/2] fix(rules): use the trimmed reason, not just test it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #231. The blank check ran on `reason.trim()` and the interpolation then spliced in `reason`, so a reason arriving with a trailing newline — the ordinary shape of a templated server string — passed the check and carried its padding into the printed message and the `--json` envelope. Trimmed once and used, which also collapses the reviewer's second note: `reason !== undefined && reason.trim() !== ""` was the long spelling of the same question. The gap that let it through is that nothing exercised a reason that was non-empty but padded, so there is now a test for exactly that. Confirmed it bites by restoring the raw interpolation. --- packages/cli/src/rules/unsupported.ts | 10 ++++++++-- packages/cli/test/unsupported.test.ts | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/rules/unsupported.ts b/packages/cli/src/rules/unsupported.ts index 9aa6e81b..62b6cac9 100644 --- a/packages/cli/src/rules/unsupported.ts +++ b/packages/cli/src/rules/unsupported.ts @@ -18,8 +18,14 @@ * is the fallback for a terminal `unsupported` that arrives without one. */ export function unsupportedMessage(reason?: string): string { - if (reason !== undefined && reason.trim() !== "") { - return `This rule generation can't be served as requested.\n\n${reason}`; + // Trimmed once and used, rather than tested and then re-read raw. The blank + // check was on `reason.trim()` while the interpolation spliced in `reason`, + // so a reason arriving with a trailing newline — the ordinary shape of a + // templated server string — passed the check and carried its padding into + // the printed message and the `--json` envelope. + const trimmed = reason?.trim(); + if (trimmed !== undefined && trimmed !== "") { + return `This rule generation can't be served as requested.\n\n${trimmed}`; } return [ "This rule generation isn't available on your current Taskless plan.", diff --git a/packages/cli/test/unsupported.test.ts b/packages/cli/test/unsupported.test.ts index 235f5513..dad93677 100644 --- a/packages/cli/test/unsupported.test.ts +++ b/packages/cli/test/unsupported.test.ts @@ -31,6 +31,17 @@ describe("explaining a terminal unsupported", () => { expect(message).not.toMatch(/administrator/i); }); + it("strips padding the service sent with its reason", () => { + // The gap that let this through: nothing exercised a reason that was + // non-empty but padded, which is the ordinary shape of a templated server + // string. The old code tested `reason.trim()` and then interpolated + // `reason`, so the newline survived into the message and the envelope. + const message = unsupportedMessage(`\n ${CLI_FLOOR}\n\n`); + expect(message).toContain(CLI_FLOOR); + expect(message.endsWith(CLI_FLOOR)).toBe(true); + expect(message).not.toMatch(/\n{3}/); + }); + it.each([ ["no reason at all", undefined], ["an empty reason", ""],