From 534ee6a7a27bfad238c8e4ca1a42a15cc1d83c7e Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 25 Aug 2026 21:09:33 -0700 Subject: [PATCH] fix: carry error codes on the error, not in the message text `rule create` and `rule improve` picked between `AUTH_REQUIRED` and `NO_GITHUB_REMOTE` by running `/git remote|origin/i` over the human-readable message `resolveIdentity` threw. The stable codes exist so a machine consumer never parses English, and the code itself was chosen by parsing English. It was correct only because both repository-URL messages happen to contain the words "git remote". Each expected failure inside `resolveIdentity` now throws a `CLIError` carrying its own `CLIErrorCode`, following the class the CLI already uses for this. Both call sites read that field through one shared `identityFailureCode` helper, so they cannot drift. The org-resolution step gets no code because it cannot fail: `fetchWhoami` swallows every network and HTTP error, and `decodeOrgId` falls back to `NIL_ORG_ID`. An unexpected throw there reports `INTERNAL_ERROR`. Tests assert the `--json` envelope's `code` for every failure origin, on both commands, so a reworded message fails a test instead of silently changing what a consumer is told. Fixes #181 Refs #179 --- .changeset/typed-identity-error-codes.md | 32 ++++ packages/cli/src/auth/identity.ts | 34 +++- packages/cli/src/commands/rules.ts | 17 +- packages/cli/src/util/git-remote.ts | 18 +- packages/cli/test/error-envelope.test.ts | 204 ++++++++++++++++++++++- 5 files changed, 285 insertions(+), 20 deletions(-) create mode 100644 .changeset/typed-identity-error-codes.md diff --git a/.changeset/typed-identity-error-codes.md b/.changeset/typed-identity-error-codes.md new file mode 100644 index 00000000..46679461 --- /dev/null +++ b/.changeset/typed-identity-error-codes.md @@ -0,0 +1,32 @@ +--- +"@taskless/cli": patch +--- + +Stop deriving `--json` error codes from the text of an error message. + +`rule create` and `rule improve` chose between `AUTH_REQUIRED` and +`NO_GITHUB_REMOTE` by running `/git remote|origin/i` over the human-readable +message that `resolveIdentity` threw. The codes exist so a machine consumer +never parses English, and the code itself was being picked by parsing English. +It happened to be right only because both repository-URL messages contain the +words "git remote"; rewording or translating either one would have silently +told every `--json` consumer to log in when the real problem was the project's +git remote. + +Each failure now throws a `CLIError` carrying its own `CLIErrorCode`, and both +call sites read that field through one shared helper. No new code was added. + +The emitted codes for existing scenarios are unchanged: + +| Condition | Code | +| ------------------------------------------- | ------------------ | +| Not logged in | `AUTH_REQUIRED` | +| Not a git repository, or no `origin` remote | `NO_GITHUB_REMOTE` | +| `origin` remote is not on `github.com` | `NO_GITHUB_REMOTE` | + +One code does change, for a scenario that is unreachable today: an unexpected +throw from the org-resolution step now reports `INTERNAL_ERROR` rather than +whichever of the two codes its wording happened to match. That step swallows +every network and HTTP error and falls back to a nil-UUID org subject, so it +cannot fail in normal operation; anything escaping it is a CLI bug rather than +a state the caller can act on. diff --git a/packages/cli/src/auth/identity.ts b/packages/cli/src/auth/identity.ts index 9a76717a..6697d3ef 100644 --- a/packages/cli/src/auth/identity.ts +++ b/packages/cli/src/auth/identity.ts @@ -1,7 +1,9 @@ import { getToken } from "./token"; import { resolveOrgSubject } from "./org"; +import { CLIError } from "../util/cli-error"; import { resolveRepositoryUrl } from "../util/git-remote"; import { getCliPrefix } from "../util/package-manager"; +import type { CLIErrorCode } from "../types/errors"; export interface Identity { token: string; @@ -20,13 +22,17 @@ export interface Identity { * to the nil-UUID `NIL_ORG_ID` so a subject is always present * - repositoryUrl: inferred from `git remote get-url origin` * - * Throws if auth is missing or the git remote is unavailable. + * Throws a `CLIError` carrying a stable `CLIErrorCode` if auth is missing + * (`AUTH_REQUIRED`) or the repository URL cannot be resolved + * (`NO_GITHUB_REMOTE`). Read the code off the error with + * `identityFailureCode`; never re-derive it from the message. */ export async function resolveIdentity(cwd: string): Promise { const token = await getToken(cwd); if (!token) { - throw new Error( - `Authentication required. Run \`${getCliPrefix()} auth login\` to authenticate.` + throw new CLIError( + `Authentication required. Run \`${getCliPrefix()} auth login\` to authenticate.`, + "AUTH_REQUIRED" ); } @@ -35,3 +41,25 @@ export async function resolveIdentity(cwd: string): Promise { return { token, orgSubject, repositoryUrl }; } + +/** + * The stable error code for a `resolveIdentity` failure. + * + * Every expected failure inside `resolveIdentity` throws a `CLIError` that + * already carries its code, so this reads a field rather than matching on + * prose. It is shared by every caller so the mapping cannot drift between + * them. + * + * `resolveOrgSubject` is deliberately absent from the list of expected + * failures: it cannot throw. `fetchWhoami` swallows every network and HTTP + * error and returns `undefined`, and `decodeOrgId` falls back to the nil-UUID + * `NIL_ORG_ID`, so a repo with no matching org resolves a subject rather than + * failing. Anything that does escape it is a bug in the CLI, not a state the + * caller can act on, so it reports as `INTERNAL_ERROR`. + */ +export function identityFailureCode(error: unknown): CLIErrorCode { + if (error instanceof CLIError && error.code) { + return error.code; + } + return "INTERNAL_ERROR"; +} diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index c800495c..14cf647a 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -4,7 +4,7 @@ import { defineCommand } from "citty"; import { ZodError } from "zod"; -import { resolveIdentity } from "../auth/identity"; +import { identityFailureCode, resolveIdentity } from "../auth/identity"; import { submitRule, pollRuleStatus, iterateRule } from "../api/rules"; import { writeRuleFile, @@ -164,13 +164,10 @@ const createCommand = defineCommand({ try { identity = await resolveIdentity(cwd); } catch (error) { - // resolveIdentity throws on missing auth or missing git remote; - // surface the original message but pick a best-guess code. + // resolveIdentity throws a CLIError carrying its own code. Read the + // field; never re-derive the code from the message. const message = error instanceof Error ? error.message : String(error); - const code: CLIErrorCode = /git remote|origin/i.test(message) - ? "NO_GITHUB_REMOTE" - : "AUTH_REQUIRED"; - fail(message, code); + fail(message, identityFailureCode(error)); } // 3. Submit rule to API @@ -405,11 +402,9 @@ const improveCommand = defineCommand({ try { identity = await resolveIdentity(cwd); } catch (error) { + // Same contract as `rule create`: the code travels on the error. const message = error instanceof Error ? error.message : String(error); - const code: CLIErrorCode = /git remote|origin/i.test(message) - ? "NO_GITHUB_REMOTE" - : "AUTH_REQUIRED"; - fail(message, code); + fail(message, identityFailureCode(error)); } // 3. Submit iterate request to API diff --git a/packages/cli/src/util/git-remote.ts b/packages/cli/src/util/git-remote.ts index cde5fbbc..16df9519 100644 --- a/packages/cli/src/util/git-remote.ts +++ b/packages/cli/src/util/git-remote.ts @@ -1,8 +1,16 @@ import { execFile } from "node:child_process"; +import { CLIError } from "./cli-error"; + /** * Resolve the repository URL from `git remote get-url origin`, * canonicalized to `https://github.com/{owner}/{repo}`. + * + * Both failure modes (no readable `origin`, and an `origin` that is not on + * github.com) throw a `CLIError` carrying `NO_GITHUB_REMOTE`. The code travels + * with the failure so a caller reads a field instead of matching on the + * message: rewording either string must never change what a `--json` consumer + * is told. */ export async function resolveRepositoryUrl(cwd: string): Promise { const rawUrl = await getOriginUrl(cwd); @@ -18,8 +26,9 @@ function getOriginUrl(cwd: string): Promise { (error, stdout) => { if (error) { reject( - new Error( - "Could not determine repository URL from git remote. Ensure your repository has an 'origin' remote pointing to GitHub." + new CLIError( + "Could not determine repository URL from git remote. Ensure your repository has an 'origin' remote pointing to GitHub.", + "NO_GITHUB_REMOTE" ) ); return; @@ -48,8 +57,9 @@ export function canonicalizeGitHubUrl(rawUrl: string): string { return `https://github.com/${httpsMatch.groups.path}`; } - throw new Error( - `Unsupported git remote URL: "${rawUrl}". Only GitHub repositories (github.com) are supported.` + throw new CLIError( + `Unsupported git remote URL: "${rawUrl}". Only GitHub repositories (github.com) are supported.`, + "NO_GITHUB_REMOTE" ); } diff --git a/packages/cli/test/error-envelope.test.ts b/packages/cli/test/error-envelope.test.ts index 576f55b7..7e29139f 100644 --- a/packages/cli/test/error-envelope.test.ts +++ b/packages/cli/test/error-envelope.test.ts @@ -5,6 +5,8 @@ import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { resolveOrgSubject } from "../src/auth/org"; + const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); @@ -15,10 +17,20 @@ interface ErrorEnvelope { } async function runCli( - args: string[] + args: string[], + env?: NodeJS.ProcessEnv ): Promise<{ stdout: string; stderr: string; exitCode: number }> { + // Never inherit an ambient TASKLESS_TOKEN: it decides whether the very first + // step of `resolveIdentity` fails, so a developer who is logged in would + // otherwise see different codes than CI. + const { TASKLESS_TOKEN: _ignored, ...baseEnvironment } = process.env; + const options = { env: { ...baseEnvironment, ...env } }; try { - const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + const { stdout, stderr } = await execFileAsync( + "node", + [binPath, ...args], + options + ); return { stdout, stderr, exitCode: 0 }; } catch (error) { const execError = error as { @@ -221,3 +233,191 @@ describe("standardized error envelope (--json)", () => { }); }); }); + +/** + * The four origins a `resolveIdentity` failure can have, and the code each + * one must produce. These codes are a public interface: an agent reads + * `code`, never `message`. The point of asserting them here is that + * rewording any of these strings fails a test instead of silently telling a + * machine consumer to log in when the real problem is the git remote. + * + * Origin 4, a `resolveOrgSubject` failure, is absent on purpose and is + * covered by its own test below: it cannot throw. + */ +describe("resolveIdentity failure codes (--json)", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "taskless-identity-")); + await mkdir(join(cwd, ".taskless"), { recursive: true }); + await writeFile( + join(cwd, ".taskless", "taskless.json"), + JSON.stringify({ + version: "2026-03-03", + orgId: 123, + repositoryUrl: "https://github.com/test/test", + }) + ); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + /** A `--from` payload valid enough to reach the identity step. */ + async function writeCreateInput(): Promise { + const file = join(cwd, "create.json"); + await writeFile(file, JSON.stringify({ prompt: "no eval" })); + return file; + } + + async function writeImproveInput(): Promise { + const file = join(cwd, "improve.json"); + await writeFile( + file, + JSON.stringify({ ruleId: "some-rule", guidance: "be stricter" }) + ); + return file; + } + + /** Turn `cwd` into a git repo whose `origin` is `url`. */ + async function initRepoWithOrigin(url: string): Promise { + await execFileAsync("git", ["init"], { cwd }); + await execFileAsync("git", ["remote", "add", "origin", url], { cwd }); + } + + const withToken = { TASKLESS_TOKEN: "test-token-not-a-real-credential" }; + + describe("origin 1: no token", () => { + it("emits AUTH_REQUIRED from rule create", async () => { + const from = await writeCreateInput(); + const result = await runCli([ + "rule", + "create", + "--from", + from, + "--json", + "-d", + cwd, + ]); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("AUTH_REQUIRED"); + }); + + it("emits AUTH_REQUIRED from rule improve", async () => { + const from = await writeImproveInput(); + const result = await runCli([ + "rule", + "improve", + "--from", + from, + "--json", + "-d", + cwd, + ]); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("AUTH_REQUIRED"); + }); + }); + + describe("origin 2: not a git repository / no origin remote", () => { + it("emits NO_GITHUB_REMOTE from rule create", async () => { + const from = await writeCreateInput(); + const result = await runCli( + ["rule", "create", "--from", from, "--json", "-d", cwd], + withToken + ); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("NO_GITHUB_REMOTE"); + }); + + it("emits NO_GITHUB_REMOTE from rule improve", async () => { + const from = await writeImproveInput(); + const result = await runCli( + ["rule", "improve", "--from", from, "--json", "-d", cwd], + withToken + ); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("NO_GITHUB_REMOTE"); + }); + }); + + describe("origin 3: origin remote is not GitHub", () => { + it("emits NO_GITHUB_REMOTE from rule create", async () => { + await initRepoWithOrigin("https://gitlab.com/acme/widgets.git"); + const from = await writeCreateInput(); + const result = await runCli( + ["rule", "create", "--from", from, "--json", "-d", cwd], + withToken + ); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("NO_GITHUB_REMOTE"); + }); + + it("emits NO_GITHUB_REMOTE from rule improve", async () => { + await initRepoWithOrigin("git@gitea.example.com:acme/widgets.git"); + const from = await writeImproveInput(); + const result = await runCli( + ["rule", "improve", "--from", from, "--json", "-d", cwd], + withToken + ); + expect(result.exitCode).not.toBe(0); + expect(parseEnvelope(result.stdout).code).toBe("NO_GITHUB_REMOTE"); + }); + }); + + it("does not read the code out of the message text", async () => { + // The old implementation picked the code with `/git remote|origin/i` over + // the message. Pin the property that replaced it: an AUTH_REQUIRED + // failure keeps its code even though its message names neither term, and + // a NO_GITHUB_REMOTE failure keeps its code independently of its wording. + const from = await writeCreateInput(); + const args = ["rule", "create", "--from", from, "--json", "-d", cwd]; + + const noTokenRun = await runCli(args); + const noToken = parseEnvelope(noTokenRun.stdout); + expect(noToken.code).toBe("AUTH_REQUIRED"); + expect(noToken.message).not.toMatch(/git remote|origin/i); + + const noRemoteRun = await runCli(args, withToken); + const noRemote = parseEnvelope(noRemoteRun.stdout); + expect(noRemote.code).toBe("NO_GITHUB_REMOTE"); + }); +}); + +/** + * Origin 4 of #181: a `resolveOrgSubject` failure. There is no such failure + * to give a code to. `fetchWhoami` returns `undefined` on any network or HTTP + * error, and `decodeOrgId` falls back to the nil-UUID `NIL_ORG_ID`, so the + * step resolves a subject rather than throwing. This test holds that shape in + * place: if `resolveOrgSubject` ever grows a throw, it needs a code of its + * own and this test says so by failing. + */ +describe("resolveOrgSubject", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "taskless-org-")); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it("resolves a subject instead of throwing when whoami is unreachable", async () => { + // An unroutable base url makes the whoami request fail outright. + const previous = process.env.TASKLESS_API_URL; + process.env.TASKLESS_API_URL = "http://127.0.0.1:1/cli"; + try { + await expect( + resolveOrgSubject(cwd, "not-a-real-token") + ).resolves.toBeDefined(); + } finally { + if (previous === undefined) { + delete process.env.TASKLESS_API_URL; + } else { + process.env.TASKLESS_API_URL = previous; + } + } + }); +});