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
18 changes: 18 additions & 0 deletions .changeset/unsupported-reason.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 3 additions & 16 deletions packages/cli/src/commands/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -223,7 +210,7 @@ const createCommand = defineCommand({
break;
}
case "unsupported": {
fail(unsupportedMessage(), "RULE_UNSUPPORTED");
fail(unsupportedMessage(status.error), "RULE_UNSUPPORTED");
break;
}
case "failed": {
Expand Down Expand Up @@ -480,7 +467,7 @@ const improveCommand = defineCommand({
break;
}
case "unsupported": {
fail(unsupportedMessage(), "RULE_UNSUPPORTED");
fail(unsupportedMessage(status.error), "RULE_UNSUPPORTED");
break;
}
case "failed": {
Expand Down
35 changes: 35 additions & 0 deletions packages/cli/src/rules/unsupported.ts
Original file line number Diff line number Diff line change
@@ -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");
}
70 changes: 70 additions & 0 deletions packages/cli/test/unsupported.test.ts
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
Loading