diff --git a/.changeset/ignored-mode-preflight.md b/.changeset/ignored-mode-preflight.md new file mode 100644 index 0000000..fde6d0a --- /dev/null +++ b/.changeset/ignored-mode-preflight.md @@ -0,0 +1,7 @@ +--- +"@bdsqqq/lnr-cli": patch +--- + +reject ignored read/write/deletion combinations and blank batch fields before acquiring credentials. preserve relative operation precedence while treating false subscription actions as inactive. + +route the documented two-token `issue batch` invocation to the batch handler, retaining the quoted legacy form. diff --git a/.todo.md b/.todo.md index 627a342..0973e5d 100644 --- a/.todo.md +++ b/.todo.md @@ -173,9 +173,11 @@ test dispatch + payload wiring, not schema validation. requires DI refactor for - document owner/session summary, project creation fields, issue update fields, and atomic label deltas have argv-to-sdk coverage - unsupported issue/project create actions and orphan/empty companion flags now reject before credentials; creation prioritySortOrder forwards zero - [x] 0099 - forward document project updates and preserve empty creation content, with argv-to-sdk regressions (live reassignment not yet verified) - - [ ] 0102 - reject existing-record mutation/read/delete flag combinations that discard intent, preserving operation precedence - - [ ] 0103 - reject empty batch state/assignee/priority/label values before credentials + - [x] 0102 - reject ignored issue/project read-action combinations and generated deletion/mutation conflicts before credentials; preserve relative precedence and inactive subscription booleans + - [x] 0103 - reject empty batch state/assignee/priority/label values before credentials - [ ] 0104 - honor mutation output modes or reject unsupported modes before writes; do not silently emit ordinary text + - [ ] 0105 - validate priority strings before credentials; unknown values currently fall back to zero in `priorityFromString` + - [x] 0106 - normalize natural `issue batch` argv to its flat router command; preserve quoted legacy invocation and unrelated routes - exhaustive accepted-flag/operation bindings remain unverified; do not mark complete from these targeted regressions - start with agent-session summary and document ownerId; audit every accepted flag for dispatch and api effect - test create/update differences, omitted/null/false/zero/empty-list semantics, enum validation, and argv parsing diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 3a03761..7ddb971 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -3,6 +3,7 @@ import { createCli } from "trpc-cli"; import { appRouter } from "./router"; import pkg from "../package.json"; +import { normalizeArgv } from "./lib/argv"; // parse global --api-key flag before trpc-cli // precedence: --api-key > LINEAR_API_KEY env > config file @@ -25,4 +26,4 @@ const cli = createCli({ description: "command-line interface for Linear", }); -void cli.run(); +void cli.run({ argv: normalizeArgv(process.argv.slice(2)) }); diff --git a/packages/cli/src/generated/doc.ts b/packages/cli/src/generated/doc.ts index 589aa31..8afff96 100644 --- a/packages/cli/src/generated/doc.ts +++ b/packages/cli/src/generated/doc.ts @@ -273,6 +273,15 @@ export const generatedDocsRouter = router({ .input(docInput) .mutation(async ({ input }) => { const operation = inferOperation(input); + if (operation === "create" && input.delete === true) { + throw new Error("--delete requires an existing doc"); + } + if (operation === "delete" && docMutationFlags.some(flag => + input[flag] !== undefined && input[flag] !== false + )) { + throw new Error("--delete cannot be combined with doc mutation flags"); + } + switch (operation) { case "create": diff --git a/packages/cli/src/generated/issue.ts b/packages/cli/src/generated/issue.ts index 34a0e58..5eeb557 100644 --- a/packages/cli/src/generated/issue.ts +++ b/packages/cli/src/generated/issue.ts @@ -148,7 +148,10 @@ export const issueMutationFlags: readonly (keyof IssueInput)[] = [ export function inferOperation(input: IssueInput): Operation { if (input.idOrNew === "new") return "create"; - const hasMutationFlags = issueMutationFlags.some(flag => input[flag] !== undefined); + const hasMutationFlags = issueMutationFlags.some(flag => + input[flag] !== undefined && + (!(flag === "subscribe" || flag === "unsubscribe") || input[flag] !== false) + ); if (input.archive && hasMutationFlags) return "update"; if (input.archive) return "archive"; @@ -808,6 +811,14 @@ export const generatedIssuesRouter = router({ .input(issueInput) .mutation(async ({ input }) => { const operation = inferOperation(input); + const readActions = ["branch", "open", "comments", "subIssues"] as const; + const activeReadActions = readActions.filter(flag => input[flag] === true); + if (activeReadActions.length > 1) { + throw new Error("only one issue read action allowed per invocation"); + } + if (operation !== "read" && activeReadActions.length > 0) { + throw new Error("issue read actions cannot be combined with " + operation); + } // reject ignored actions before handlers acquire credentials or create a resource. for (const flag of [ "comment", "editComment", "replyTo", "deleteComment", @@ -861,6 +872,11 @@ export const generatedIssuesRouter = router({ }) .input(batchUpdateInput) .mutation(async ({ input }) => { + for (const flag of ["state", "assignee", "priority", "label"] as const) { + if (input[flag] !== undefined && !input[flag].trim()) { + throw new Error("--" + flag + " must not be blank"); + } + } await handleBatchUpdate(input); }), }); diff --git a/packages/cli/src/generated/label.ts b/packages/cli/src/generated/label.ts index 2313219..1c99fab 100644 --- a/packages/cli/src/generated/label.ts +++ b/packages/cli/src/generated/label.ts @@ -255,6 +255,15 @@ export const generatedLabelsRouter = router({ .input(labelInput) .mutation(async ({ input }) => { const operation = inferOperation(input); + if (operation === "create" && input.delete === true) { + throw new Error("--delete requires an existing label"); + } + if (operation === "delete" && labelMutationFlags.some(flag => + input[flag] !== undefined && input[flag] !== false + )) { + throw new Error("--delete cannot be combined with label mutation flags"); + } + switch (operation) { case "create": diff --git a/packages/cli/src/generated/project.ts b/packages/cli/src/generated/project.ts index b13a634..c6b3bbe 100644 --- a/packages/cli/src/generated/project.ts +++ b/packages/cli/src/generated/project.ts @@ -120,7 +120,8 @@ export function inferOperation(input: ProjectInput): Operation { if (input.delete) return "delete"; for (const flag of projectMutationFlags) { - if (input[flag] !== undefined) return "update"; + if (input[flag] !== undefined && + (!(flag === "subscribe" || flag === "unsubscribe") || input[flag] !== false)) return "update"; } return "read"; @@ -570,6 +571,19 @@ export const generatedProjectsRouter = router({ .input(projectInput) .mutation(async ({ input }) => { const operation = inferOperation(input); + const readActions = ["issues", "updates", "labels", "showStatus", "milestones", "links"] as const; + const activeReadActions = readActions.filter(flag => input[flag] === true); + if (activeReadActions.length > 1) { + throw new Error("only one project read action allowed per invocation"); + } + if (operation !== "read" && activeReadActions.length > 0) { + throw new Error("project read actions cannot be combined with " + operation); + } + if (operation === "delete" && projectMutationFlags.some(flag => + input[flag] !== undefined && input[flag] !== false + )) { + throw new Error("--delete cannot be combined with project mutation flags"); + } // companion flags must be validated before an otherwise valid write. for (const flag of ["react", "emoji", "unreact"] as const) { if (input[flag] === "") throw new Error(flag + " must not be empty"); diff --git a/packages/cli/src/lib/argv.ts b/packages/cli/src/lib/argv.ts new file mode 100644 index 0000000..440b9d7 --- /dev/null +++ b/packages/cli/src/lib/argv.ts @@ -0,0 +1,6 @@ +/** `issue` already names a procedure, so its flat batch sibling needs one routing token. */ +export function normalizeArgv(argv: string[]): string[] { + return argv[0] === "issue" && argv[1] === "batch" + ? ["issue batch", ...argv.slice(2)] + : argv; +} diff --git a/packages/cli/src/router/mode-preflight.test.ts b/packages/cli/src/router/mode-preflight.test.ts new file mode 100644 index 0000000..665ed4a --- /dev/null +++ b/packages/cli/src/router/mode-preflight.test.ts @@ -0,0 +1,180 @@ +import { expect, mock, test } from "bun:test"; +import { createCli, FailedToExitError } from "trpc-cli"; +import { t } from "./trpc"; +import { normalizeArgv } from "../lib/argv"; + +const core = await import("@bdsqqq/lnr-core"); +const credentialMarker = "credential factory reached"; +const getClient = mock(() => { throw new Error(credentialMarker); }); +mock.module("@bdsqqq/lnr-core", () => ({ ...core, getClient })); +mock.module("../lib/error", () => ({ + handleApiError: (error: unknown) => { throw error; }, + exitWithError: (message: string) => { throw new Error(message); }, + EXIT_CODES: { GENERAL_ERROR: 1, NOT_FOUND: 3 }, +})); +const issue = await import("../generated/issue"); +const project = await import("../generated/project"); +const doc = await import("../generated/doc"); +const label = await import("../generated/label"); +const router = t.mergeRouters(issue.generatedIssuesRouter, project.generatedProjectsRouter, + doc.generatedDocsRouter, label.generatedLabelsRouter); +const caller = router.createCaller({}); + +// Valid controls prove admission, not execution: the credential factory deliberately stops them. +async function checkArgv(argv: string[], message: string, accesses: number) { + getClient.mockClear(); + const errors: string[] = []; + const previous = process.exitCode; + try { + await expect(createCli({ router }).run({ + argv: normalizeArgv(argv), + process: { exit(code): never { + throw new FailedToExitError("test exit", { exitCode: code, cause: undefined }); + } }, + logger: { info() {}, error: (...args) => { errors.push(args.map(String).join(" ")); } }, + })).rejects.toMatchObject({ exitCode: 1 }); + expect(errors.join("\n")).toContain(message); + expect(getClient).toHaveBeenCalledTimes(accesses); + } finally { process.exitCode = previous; } +} +const issueReads = ["branch", "open", "comments", "sub-issues"]; +const projectReads = ["issues", "updates", "labels", "show-status", "milestones", "links"]; + +test("batch argv normalization preserves values and unrelated routes", () => { + expect(normalizeArgv(["issue", "batch", "ENG-1,ENG-2", "--label", "two words"])) + .toEqual(["issue batch", "ENG-1,ENG-2", "--label", "two words"]); + for (const argv of [ + ["issue", "ENG-1", "--title", "batch"], ["issue", "--help"], + ["issue batch", "ENG-1", "--state", "Done"], ["oauth", "authorize"], + ]) expect(normalizeArgv(argv)).toEqual(argv); +}); + +test("show-only actions reject mutating modes before credentials", async () => { + for (const flag of issueReads) { + for (const [mode, args] of [ + ["create", ["new", "--team", "ENG", "--title", "test"]], + ["update", ["ENG-1", "--title", "revised"]], + ["archive", ["ENG-1", "--archive"]], + ] as const) { + await checkArgv(["issue", ...args, "--" + flag], + "issue read actions cannot be combined with " + mode, 0); + } + } + for (const flag of projectReads) { + for (const [mode, args] of [ + ["create", ["new", "--new-name", "test"]], + ["update", ["Apollo", "--description", "revised"]], + ["delete", ["Apollo", "--delete"]], + ] as const) { + await checkArgv(["project", ...args, "--" + flag], + "project read actions cannot be combined with " + mode, 0); + } + } +}); +test("read-action pairs reject; every standalone action is admitted", async () => { + for (const [command, id, flags] of [ + ["issue", "ENG-1", issueReads], ["project", "Apollo", projectReads], + ] as const) { + for (const [index, flag] of flags.entries()) { + await checkArgv([command, id, "--" + flag], credentialMarker, 1); + for (const other of flags.slice(index + 1)) { + await checkArgv([command, id, "--" + flag, "--" + other], + "only one " + command + " read action allowed", 0); + } + } + } +}); +test("project deletion rejects active mutators, including zero and empty text", async () => { + for (const flags of [ + ["--new-name", "renamed"], ["--description", ""], ["--content", ""], + ["--status", "started"], ["--start-date", "2026-10-01"], + ["--target-date", "2026-11-01"], ["--priority", "0"], + ["--lead", "@me"], ["--team", "ENG"], + ["--react", "U", "--emoji", "+1"], ["--unreact", "R"], + ["--subscribe"], ["--unsubscribe"], + ]) { + await checkArgv(["project", "Apollo", "--delete", ...flags], + "--delete cannot be combined with project mutation flags", 0); + } +}); +test("doc/label creation rejects deletion; deletion rejects mutations", async () => { + await checkArgv(["doc", "new", "--title", "test", "--delete"], + "--delete requires an existing doc", 0); + await checkArgv(["label", "new", "--name", "test", "--team", "ENG", "--delete"], + "--delete requires an existing label", 0); + for (const flags of [["--title", "revised"], ["--content", ""]]) { + await checkArgv(["doc", "D", "--delete", ...flags], + "--delete cannot be combined with doc mutation flags", 0); + } + for (const flags of [ + ["--name", "revised"], ["--color", "#ffffff"], + ["--description", ""], ["--group-type", "null"], + ]) { + await checkArgv(["label", "L", "--delete", ...flags], + "--delete cannot be combined with label mutation flags", 0); + } + for (const flags of [["--owner-id", "null"], ["--project", "Apollo"]]) { + await checkArgv(["doc", "D", "--delete", ...flags], "cannot be combined with --delete", 0); + } +}); +test("batch rejects blank fields even with another effective update", async () => { + for (const flag of ["state", "assignee", "priority", "label"]) { + for (const value of ["", " ", "\t\n"]) { + const companion = flag === "priority" ? ["--state", "Done"] : ["--priority", "high"]; + await checkArgv(["issue", "batch", "ENG-1,ENG-2", "--" + flag, value, ...companion], + "--" + flag + " must not be blank", 0); + } + } +}); +test("supported writes, selectors and nonblank batch values reach credentials", async () => { + for (const argv of [ + ["project", "Apollo", "--delete"], ["doc", "D", "--delete"], ["label", "L", "--delete"], + ["doc", "new", "--title", "test", "--content", "", "--project", "Apollo"], + ["label", "new", "--name", "test", "--team", "ENG"], + ["issue", "ENG-1", "--title", "", "--estimate", "0", "--archive"], + ["issue", "ENG-1", "--project", "Apollo", "--milestone", "launch"], + ["issue", "ENG-1", "--comment", "body", "--blocks", "ENG-2"], + ["issue", "ENG-1", "--subscribe", "--unsubscribe"], + ["issue", "new", "--team", "ENG", "--title", "test", "--blocks", "ENG-2", + "--blocked-by", "ENG-3", "--relates-to", "ENG-4", + "--pr", "https://github.com/example/repo/pull/1", "--priority-sort-order", "0"], + ["project", "Apollo", "--priority", "0", "--description", "", + "--react", "U", "--emoji", "+1", "--unreact", "R", "--subscribe", "--unsubscribe"], + ["issue", "batch", "ENG-1,ENG-2", "--priority", "0"], + ["issue", "batch", "ENG-1,ENG-2", "--priority", "none"], + ["issue", "batch", "ENG-1,ENG-2", "--state", "Done", "--assignee", "@me"], + ["issue", "batch", "ENG-1,ENG-2", "--label", "bug"], + ["issue", "batch", "ENG-1,ENG-2", "--label", "+bug"], + ]) await checkArgv(argv, credentialMarker, 1); +}); +test("false action booleans remain inactive without suppressing false-valued data mutations", async () => { + for (const invoke of [ + () => caller.issue({ idOrNew: "ENG-1", branch: true, open: false, comments: false, + subIssues: false, subscribe: false, unsubscribe: false }), + () => caller.issue({ idOrNew: "ENG-1", title: "revised", branch: false, open: false, + comments: false, subIssues: false }), + () => caller.project({ name: "Apollo", issues: true, updates: false, labels: false, + showStatus: false, milestones: false, links: false, subscribe: false, unsubscribe: false }), + () => caller.project({ name: "Apollo", delete: true, subscribe: false, unsubscribe: false, + issues: false, updates: false, labels: false, showStatus: false, milestones: false, links: false }), + () => caller.doc({ id: "new", title: "test", delete: false }), + () => caller.label({ id: "new", name: "test", team: "ENG", delete: false }), + ]) { + getClient.mockClear(); + await expect(invoke()).rejects.toThrow(credentialMarker); + expect(getClient).toHaveBeenCalledTimes(1); + } + getClient.mockClear(); + await expect(caller.issue({ idOrNew: "ENG-1", inheritsSharedAccess: false, comments: true })) + .rejects.toThrow("issue read actions cannot be combined with update"); + expect(getClient).not.toHaveBeenCalled(); +}); +test("relative operation precedence remains; inactive subscription booleans do not select writes", () => { + expect(issue.inferOperation({ idOrNew: "ENG-1", archive: true, title: "revised" })).toBe("update"); + expect(project.inferOperation({ name: "Apollo", delete: true, description: "revised" })).toBe("delete"); + expect(doc.inferOperation({ id: "new", delete: true, title: "test" })).toBe("create"); + expect(label.inferOperation({ id: "new", delete: true, name: "test" })).toBe("create"); + expect(issue.inferOperation({ idOrNew: "ENG-1", subscribe: false, unsubscribe: false })).toBe("read"); + expect(project.inferOperation({ name: "Apollo", subscribe: false, unsubscribe: false })).toBe("read"); + expect(issue.inferOperation({ idOrNew: "ENG-1", inheritsSharedAccess: false })).toBe("update"); +}); diff --git a/packages/codegen/generate-commands.ts b/packages/codegen/generate-commands.ts index 5e99855..73fd04f 100644 --- a/packages/codegen/generate-commands.ts +++ b/packages/codegen/generate-commands.ts @@ -335,7 +335,10 @@ export const issueMutationFlags: readonly (keyof IssueInput)[] = [ export function inferOperation(input: IssueInput): Operation { if (input.idOrNew === "new") return "create"; - const hasMutationFlags = issueMutationFlags.some(flag => input[flag] !== undefined); + const hasMutationFlags = issueMutationFlags.some(flag => + input[flag] !== undefined && + (!(flag === "subscribe" || flag === "unsubscribe") || input[flag] !== false) + ); if (input.archive && hasMutationFlags) return "update"; if (input.archive) return "archive"; @@ -377,6 +380,11 @@ type BatchUpdateInput = typeof batchUpdateInput.infer;`, }) .input(batchUpdateInput) .mutation(async ({ input }) => { + for (const flag of ["state", "assignee", "priority", "label"] as const) { + if (input[flag] !== undefined && !input[flag].trim()) { + throw new Error("--" + flag + " must not be blank"); + } + } await handleBatchUpdate(input); }),`, }; @@ -497,7 +505,8 @@ export function inferOperation(input: ProjectInput): Operation { if (input.delete) return "delete"; for (const flag of projectMutationFlags) { - if (input[flag] !== undefined) return "update"; + if (input[flag] !== undefined && + (!(flag === "subscribe" || flag === "unsubscribe") || input[flag] !== false)) return "update"; } return "read"; @@ -1058,6 +1067,14 @@ export const generated${TypeName}sRouter = router({ .input(${config.singularCommand}Input) .mutation(async ({ input }) => { const operation = inferOperation(input);${config.singularCommand === "issue" ? ` + const readActions = ["branch", "open", "comments", "subIssues"] as const; + const activeReadActions = readActions.filter(flag => input[flag] === true); + if (activeReadActions.length > 1) { + throw new Error("only one issue read action allowed per invocation"); + } + if (operation !== "read" && activeReadActions.length > 0) { + throw new Error("issue read actions cannot be combined with " + operation); + } // reject ignored actions before handlers acquire credentials or create a resource. for (const flag of [ "comment", "editComment", "replyTo", "deleteComment", @@ -1086,6 +1103,19 @@ export const generated${TypeName}sRouter = router({ } } ` : config.singularCommand === "project" ? ` + const readActions = ["issues", "updates", "labels", "showStatus", "milestones", "links"] as const; + const activeReadActions = readActions.filter(flag => input[flag] === true); + if (activeReadActions.length > 1) { + throw new Error("only one project read action allowed per invocation"); + } + if (operation !== "read" && activeReadActions.length > 0) { + throw new Error("project read actions cannot be combined with " + operation); + } + if (operation === "delete" && projectMutationFlags.some(flag => + input[flag] !== undefined && input[flag] !== false + )) { + throw new Error("--delete cannot be combined with project mutation flags"); + } // companion flags must be validated before an otherwise valid write. for (const flag of ["react", "emoji", "unreact"] as const) { if (input[flag] === "") throw new Error(flag + " must not be empty"); @@ -1102,6 +1132,15 @@ export const generated${TypeName}sRouter = router({ )) { throw new Error("reaction, deletion and subscription actions require an existing project"); } +` : ["doc", "label"].includes(config.singularCommand) ? ` + if (operation === "create" && input.delete === true) { + throw new Error("--delete requires an existing ${config.singularCommand}"); + } + if (operation === "delete" && ${config.singularCommand}MutationFlags.some(flag => + input[flag] !== undefined && input[flag] !== false + )) { + throw new Error("--delete cannot be combined with ${config.singularCommand} mutation flags"); + } ` : ""} switch (operation) {