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..62b6cac9 --- /dev/null +++ b/packages/cli/src/rules/unsupported.ts @@ -0,0 +1,35 @@ +/** + * `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 { + // 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.", + "", + "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..dad93677 --- /dev/null +++ b/packages/cli/test/unsupported.test.ts @@ -0,0 +1,70 @@ +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("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", ""], + ["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); + } + }); +});