From 2c1d2375fcad04298ed2c396dbab70c136294087 Mon Sep 17 00:00:00 2001 From: "Ricardo Q. Bazan" Date: Sat, 17 Oct 2026 19:02:28 -0500 Subject: [PATCH 1/5] fix(cli): keep run-run.config.mts imports sorted, together and newline-terminated magicast's imports.$add puts each new import first, so after `rr plugins add tsdown`, `ts` and `biome` the imports came in reverse, and recast dropped the final newline: `rr jsc` failed on the file rr had just written. Move each added import to its place in Biome's organizeImports order, without reordering the others, join the blank lines recast leaves between imports, and end the file with a newline. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tj9MBgi2cQcRxSD9uikx6k --- .changeset/cli-config-import-order.md | 5 ++ .../src/services/__tests__/config-ast.test.ts | 70 +++++++++++++++++++ run-run/cli/src/services/config-ast.ts | 59 +++++++++++++--- 3 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 .changeset/cli-config-import-order.md diff --git a/.changeset/cli-config-import-order.md b/.changeset/cli-config-import-order.md new file mode 100644 index 00000000..1acf4e05 --- /dev/null +++ b/.changeset/cli-config-import-order.md @@ -0,0 +1,5 @@ +--- +"@rrlab/cli": patch +--- + +`rr plugins add` writes `run-run.config.mts` with its imports together, in Biome's order, and with a final newline, so `rr jsc` and oxfmt pass on it. Each new import used to go first, so they ended up in reverse order of adding, and the file had no final newline. diff --git a/run-run/cli/src/services/__tests__/config-ast.test.ts b/run-run/cli/src/services/__tests__/config-ast.test.ts index adffad9f..26a20949 100644 --- a/run-run/cli/src/services/__tests__/config-ast.test.ts +++ b/run-run/cli/src/services/__tests__/config-ast.test.ts @@ -7,6 +7,10 @@ import { ConfigAstService } from "#src/services/config-ast.ts"; const svc = new ConfigAstService(); +function importSources(code: string): string[] { + return [...code.matchAll(/^import .* from "(.+)";$/gm)].map((match) => match[1] ?? ""); +} + const EMPTY_CONFIG = `import { defineConfig } from "@rrlab/cli/config"; export default defineConfig({ @@ -76,6 +80,34 @@ describe("ConfigAstService", () => { }); }); + describe("addPlugin import order", () => { + it("puts each import where Biome's organizeImports wants it, in whatever order the plugins come", () => { + const mod = parseModule(EMPTY_CONFIG); + svc.addPlugin(mod, { exportName: "tsdown", pkgName: "@rrlab/tsdown-plugin" }); + svc.addPlugin(mod, { exportName: "ts", pkgName: "@rrlab/ts-plugin" }); + svc.addPlugin(mod, { exportName: "biome", pkgName: "@rrlab/biome-plugin" }); + expect(importSources(generateCode(mod).code)).toEqual([ + "@rrlab/biome-plugin", + "@rrlab/cli/config", + "@rrlab/ts-plugin", + "@rrlab/tsdown-plugin", + ]); + }); + + it("leaves the other imports in their order, and goes after a protocol import and before a path", () => { + const mod = parseModule(`import fs from "node:fs"; +import { defineConfig } from "@rrlab/cli/config"; +import local from "./local.ts"; + +export default defineConfig({ + plugins: [], +}); +`); + svc.addPlugin(mod, { exportName: "ts", pkgName: "@rrlab/ts-plugin" }); + expect(importSources(generateCode(mod).code)).toEqual(["node:fs", "@rrlab/cli/config", "@rrlab/ts-plugin", "./local.ts"]); + }); + }); + describe("removePlugin", () => { it("removes the call and the import binding", () => { const mod = parseModule(TWO_PLUGINS_CONFIG); @@ -129,6 +161,44 @@ describe("ConfigAstService", () => { expect(written).toMatch(/plugins:\s*\[\s*biome\(\),\s*ts\(\)\s*\]/); }); + it("keeps the imports together and sorted across separate `rr plugins add` runs", async () => { + const cfgPath = path.join(tmpDir, "run-run.config.mts"); + await fs.writeFile(cfgPath, EMPTY_CONFIG, "utf8"); + for (const [exportName, pkgName] of [ + ["tsdown", "@rrlab/tsdown-plugin"], + ["ts", "@rrlab/ts-plugin"], + ["biome", "@rrlab/biome-plugin"], + ] as const) { + const loaded = await svc.load(tmpDir); + svc.addPlugin(loaded.mod, { exportName, pkgName }); + await svc.save(loaded); + } + + expect(await fs.readFile(cfgPath, "utf8")).toBe(`import biome from "@rrlab/biome-plugin"; +import { defineConfig } from "@rrlab/cli/config"; +import ts from "@rrlab/ts-plugin"; +import tsdown from "@rrlab/tsdown-plugin"; + +export default defineConfig({ + plugins: [tsdown(), ts(), biome()], +}); +`); + }); + + it.each([ + ["an existing config", ONE_PLUGIN_CONFIG], + ["a new config", undefined], + ])("ends %s with a newline", async (_label, existing) => { + const cfgPath = path.join(tmpDir, "run-run.config.mts"); + if (existing) await fs.writeFile(cfgPath, existing, "utf8"); + + const loaded = await svc.load(tmpDir); + svc.addPlugin(loaded.mod, { exportName: "ts", pkgName: "@rrlab/ts-plugin" }); + await svc.save(loaded); + + expect(await fs.readFile(cfgPath, "utf8")).toMatch(/\}\);\n$/); + }); + it("writes a brand-new minimal config when none existed and a plugin was added", async () => { const loaded = await svc.load(tmpDir); svc.addPlugin(loaded.mod, { exportName: "biome", pkgName: "@rrlab/biome-plugin" }); diff --git a/run-run/cli/src/services/config-ast.ts b/run-run/cli/src/services/config-ast.ts index b71d6ba3..a7febdcd 100644 --- a/run-run/cli/src/services/config-ast.ts +++ b/run-run/cli/src/services/config-ast.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { builders, generateCode, loadFile, type ProxifiedModule, parseModule, writeFile } from "magicast"; +import { builders, generateCode, loadFile, type ProxifiedModule, parseModule } from "magicast"; const CONFIG_FILENAMES = ["run-run.config.ts", "run-run.config.mts"] as const; @@ -50,14 +50,7 @@ export class ConfigAstService { } async save(loaded: LoadedConfig): Promise { - if (loaded.isNew) { - // For a fresh file, write the generated code instead of using - // `writeFile` — writeFile expects an existing file as a starting point. - const { code } = generateCode(loaded.mod); - await fs.writeFile(loaded.filepath, code, "utf8"); - return; - } - await writeFile(loaded.mod.$ast, loaded.filepath); + await fs.writeFile(loaded.filepath, tidy(generateCode(loaded.mod).code), "utf8"); } /** @@ -84,6 +77,7 @@ export class ConfigAstService { // Ensure the import binding exists. if (!mod.imports[entry.exportName]) { mod.imports.$add({ from: entry.pkgName, imported: "default", local: entry.exportName }); + this.#placeImport(mod, entry.pkgName); } // Ensure `defineConfig` import + default export shape. @@ -151,9 +145,30 @@ export class ConfigAstService { return opts.plugins; } + /** + * Moves the import of `source`, which `imports.$add` puts first, to its + * sorted place among the other imports, leaving those in their order: a + * sorted file stays sorted, so `rr jsc` passes on what rr wrote. + */ + #placeImport(mod: ProxifiedModule, source: string): void { + const program = mod.$ast; + if (program.type !== "Program") return; + const { body } = program; + const from = body.findIndex((node) => node.type === "ImportDeclaration" && node.source.value === source); + if (from === -1) return; + const [added] = body.splice(from, 1); + if (!added) return; + const before = body.findIndex( + (node) => node.type === "ImportDeclaration" && compareImportSources(node.source.value, source) > 0, + ); + const lastImport = body.reduce((last, node, i) => (node.type === "ImportDeclaration" ? i : last), -1); + body.splice(before === -1 ? lastImport + 1 : before, 0, added); + } + #ensureDefineConfig(mod: ProxifiedModule): void { if (!mod.imports.defineConfig) { mod.imports.$add({ from: "@rrlab/cli/config", imported: "defineConfig", local: "defineConfig" }); + this.#placeImport(mod, "@rrlab/cli/config"); } // biome-ignore lint/suspicious/noExplicitAny: magicast proxies are opaque const def = (mod.exports as any).default; @@ -194,6 +209,32 @@ export class ConfigAstService { } } +/** + * Undoes two recast habits that formatters reject: it keeps the blank line that + * followed an import when rr inserts another after it, and it drops the final + * newline. + */ +function tidy(code: string): string { + const joined = code.replace(/^(import\b[^;]*;)\n(?:[ \t]*\n)+(?=import\b)/gm, "$1\n"); + return joined.endsWith("\n") ? joined : `${joined}\n`; +} + +/** + * Biome's organizeImports order: protocol imports (`node:fs`), then packages, + * then aliases (`#src/…`, `@/…`, `~/…`), then paths. Within a group, plain + * code-unit order, which matches Biome for these names (`ts-plugin` before + * `tsdown-plugin`; `localeCompare` flips them). + */ +function compareImportSources(a: string, b: string): number { + const group = (source: string) => { + if (/^[a-z]+:/.test(source)) return 0; + if (/^(#|@\/|~)/.test(source)) return 2; + if (source.startsWith(".") || source.startsWith("/")) return 3; + return 1; + }; + return group(a) - group(b) || (a < b ? -1 : a > b ? 1 : 0); +} + const MINIMAL_TEMPLATE = `import { defineConfig } from "@rrlab/cli/config"; export default defineConfig({ From 3775cb8cab4205a12379d40625da57e5542c5c47 Mon Sep 17 00:00:00 2001 From: "Ricardo Q. Bazan" Date: Sat, 17 Oct 2026 19:02:28 -0500 Subject: [PATCH 2/5] fix(biome-plugin): scaffold a biome.json that passes rr jsc The scaffold came out of JSON.stringify, which puts the one-item `extends` array on three lines, so Biome's own format check failed. Its `$schema` was a 2.4.4 URL while the plugin installs `@biomejs/biome@^2.0.0`. Write it in Biome's format, and point `$schema` at the installed Biome's configuration_schema.json, which always matches its version. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tj9MBgi2cQcRxSD9uikx6k --- .changeset/biome-plugin-scaffold-format.md | 5 +++++ run-run/biome-plugin/src/index.ts | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 .changeset/biome-plugin-scaffold-format.md diff --git a/.changeset/biome-plugin-scaffold-format.md b/.changeset/biome-plugin-scaffold-format.md new file mode 100644 index 00000000..9b53e4b8 --- /dev/null +++ b/.changeset/biome-plugin-scaffold-format.md @@ -0,0 +1,5 @@ +--- +"@rrlab/biome-plugin": patch +--- + +`rr plugins add biome` scaffolds a `biome.json` that passes `rr jsc`: in Biome's own format, with `$schema` pointing at the installed Biome's schema (`./node_modules/@biomejs/biome/configuration_schema.json`). It used to fail the format check, and its pinned 2.4.4 schema URL drew a version-mismatch note against the `^2.0.0` it installs. diff --git a/run-run/biome-plugin/src/index.ts b/run-run/biome-plugin/src/index.ts index dfacf362..50b76313 100644 --- a/run-run/biome-plugin/src/index.ts +++ b/run-run/biome-plugin/src/index.ts @@ -25,7 +25,8 @@ const FROM = import.meta.url; const COMMON_FLAGS = ["--colors=force", "--no-errors-on-unmatched"]; const BIOME_JSON = "biome.json"; const BIOME_CONFIG_PKG = "@rrlab/biome-config"; -const BIOME_SCHEMA = "https://biomejs.dev/schemas/2.4.4/schema.json"; +// The installed Biome's own schema: a versioned URL drifts from `^2.0.0` and Biome flags the mismatch. +const BIOME_SCHEMA = "./node_modules/@biomejs/biome/configuration_schema.json"; export { TOOL_VERSIONS } from "./tool-versions.ts"; @@ -81,7 +82,8 @@ export async function install(ctx: InstallContext): Promise { ? { kind: "create", path: BIOME_JSON, - content: `${JSON.stringify({ $schema: BIOME_SCHEMA, extends: [BIOME_CONFIG_PKG] }, null, 2)}\n`, + // In Biome's own format, which keeps a short array on one line, so `rr jsc` passes on it. + content: `{\n "$schema": "${BIOME_SCHEMA}",\n "extends": ["${BIOME_CONFIG_PKG}"]\n}\n`, overwrite: scaffoldDecision === "overwrite" || ctx.flags.force, } : { From 70633af584ad9824792c998c4e173a357e8090fa Mon Sep 17 00:00:00 2001 From: "Ricardo Q. Bazan" Date: Sat, 17 Oct 2026 19:02:28 -0500 Subject: [PATCH 3/5] fix(oxc-plugin): stop passing oxlint and oxfmt flags they don't have `rr lint` ran `oxlint --check`, which exits with "`--check` is not expected in this context" (1.66 and 1.88 alike), and `rr format --fix` ran `oxfmt --fix` where oxfmt writes with `--write`. dx only uses oxc for the type-check, so no test ran either. Run oxlint without a mode flag to check, and oxfmt with `--write` to fix. Add oxc cases to the lint and format integration tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tj9MBgi2cQcRxSD9uikx6k --- .changeset/oxc-plugin-cli-flags.md | 5 ++++ run-run/cli/test/integration/format.test.ts | 28 +++++++++++++++++++++ run-run/cli/test/integration/lint.test.ts | 25 ++++++++++++++++++ run-run/oxc-plugin/src/index.ts | 5 ++-- 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 .changeset/oxc-plugin-cli-flags.md diff --git a/.changeset/oxc-plugin-cli-flags.md b/.changeset/oxc-plugin-cli-flags.md new file mode 100644 index 00000000..3a8f9291 --- /dev/null +++ b/.changeset/oxc-plugin-cli-flags.md @@ -0,0 +1,5 @@ +--- +"@rrlab/oxc-plugin": patch +--- + +`rr lint`, `rr jsc` and `rr check` run `oxlint` without `--check`, a flag it doesn't have, so they failed wherever oxc provides `lint`. `rr format --fix` runs `oxfmt --write`, since `oxfmt` has no `--fix` either. diff --git a/run-run/cli/test/integration/format.test.ts b/run-run/cli/test/integration/format.test.ts index 03a1b782..6c239f70 100644 --- a/run-run/cli/test/integration/format.test.ts +++ b/run-run/cli/test/integration/format.test.ts @@ -1,3 +1,5 @@ +import { readFileSync } from "node:fs"; +import path from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { createTestCli, fixtures, makeFixture } from "#test/helpers.ts"; @@ -31,3 +33,29 @@ describe("rr format", () => { expect(r.status).toBe(0); }); }); + +describe("rr format (oxc)", () => { + let fixture: { dir: string; cleanup: () => void }; + + beforeEach(() => { + fixture = makeFixture("format-oxc", { + "package.json": fixtures.pkg(), + "run-run.config.mts": fixtures.config(["oxc"]), + "src/ugly.ts": "export const ugly=1\n", + }); + }); + + afterEach(() => fixture.cleanup()); + + test("checks with oxfmt and fails on an unformatted file", () => { + const r = cli("format", { cwd: fixture.dir }); + expect(r.stdout + r.stderr).toContain("src/ugly.ts"); + expect(r.status).not.toBe(0); + }); + + test("--fix writes the file formatted", () => { + const r = cli("format --fix", { cwd: fixture.dir }); + expect(r.status).toBe(0); + expect(readFileSync(path.join(fixture.dir, "src/ugly.ts"), "utf8")).toBe("export const ugly = 1;\n"); + }); +}); diff --git a/run-run/cli/test/integration/lint.test.ts b/run-run/cli/test/integration/lint.test.ts index d51e326b..db473ebb 100644 --- a/run-run/cli/test/integration/lint.test.ts +++ b/run-run/cli/test/integration/lint.test.ts @@ -31,3 +31,28 @@ describe("rr lint", () => { expect(r.status).toBe(0); }); }); + +describe("rr lint (oxc)", () => { + let fixture: { dir: string; cleanup: () => void }; + + beforeEach(() => { + fixture = makeFixture("lint-oxc", { + // oxlint skips a real node_modules but walks the fixture's symlinked one, unless it's ignored. + ".gitignore": "node_modules\n", + "package.json": fixtures.pkg(), + "run-run.config.mts": fixtures.config(["oxc"]), + "src/debug.ts": "export function f() {\n debugger;\n}\n", + }); + }); + + afterEach(() => fixture.cleanup()); + + // oxlint has no `--check` flag: it checks by default, and a warning doesn't fail it. + test("runs oxlint in its default check mode", () => { + const r = cli("lint", { cwd: fixture.dir }); + const combined = r.stdout + r.stderr; + expect(combined).toContain("$ oxlint --report-unused-disable-directives\n"); + expect(combined).toContain("no-debugger"); + expect(r.status).toBe(0); + }); +}); diff --git a/run-run/oxc-plugin/src/index.ts b/run-run/oxc-plugin/src/index.ts index 6b7926fb..95d8b950 100644 --- a/run-run/oxc-plugin/src/index.ts +++ b/run-run/oxc-plugin/src/index.ts @@ -27,7 +27,8 @@ export class OxlintService extends ToolService implements Linter { } async lint(options: LintOptions): Promise { - return this.runReport(["--report-unused-disable-directives", options.fix ? "--fix" : "--check"]); + // oxlint checks by default; it has no `--check` flag. + return this.runReport(["--report-unused-disable-directives", ...(options.fix ? ["--fix"] : [])]); } } @@ -37,7 +38,7 @@ export class OxfmtService extends ToolService implements Formatter { } async format(options: FormatOptions): Promise { - return this.runReport(["--no-error-on-unmatched-pattern", options.fix ? "--fix" : "--check"]); + return this.runReport(["--no-error-on-unmatched-pattern", options.fix ? "--write" : "--check"]); } } From 2023743363d8d55645afaceba1ba3501cbab194e Mon Sep 17 00:00:00 2001 From: "Ricardo Q. Bazan" Date: Sat, 17 Oct 2026 19:02:28 -0500 Subject: [PATCH 4/5] fix(ts-plugin): default to the React preset for a package that depends on react With `--yes` or without a TTY, `rr plugins add ts` always picked the Node.js app preset, which can't type-check a React app (no DOM lib, no JSX). Default to the React preset when react is in dependencies, devDependencies or peerDependencies. It's also where the interactive prompt now starts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tj9MBgi2cQcRxSD9uikx6k --- .changeset/ts-plugin-react-preset.md | 5 +++ .../ts-plugin/src/__tests__/install.test.ts | 33 +++++++++++++++++-- run-run/ts-plugin/src/index.ts | 13 ++++++-- 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 .changeset/ts-plugin-react-preset.md diff --git a/.changeset/ts-plugin-react-preset.md b/.changeset/ts-plugin-react-preset.md new file mode 100644 index 00000000..72649f72 --- /dev/null +++ b/.changeset/ts-plugin-react-preset.md @@ -0,0 +1,5 @@ +--- +"@rrlab/ts-plugin": patch +--- + +`rr plugins add ts` defaults to the React preset when the package depends on `react` (in `dependencies`, `devDependencies` or `peerDependencies`), and to the Node.js app preset otherwise. That's what `--yes` and a run without a terminal pick, and where the prompt starts. Before, they always picked the Node.js preset, which can't type-check a React app. diff --git a/run-run/ts-plugin/src/__tests__/install.test.ts b/run-run/ts-plugin/src/__tests__/install.test.ts index 6fcdecd7..5db888aa 100644 --- a/run-run/ts-plugin/src/__tests__/install.test.ts +++ b/run-run/ts-plugin/src/__tests__/install.test.ts @@ -16,12 +16,16 @@ afterEach(async () => { await fs.rm(tmpDir, { recursive: true, force: true }); }); +function appPkg(packageJson: Pkg["packageJson"] = {}): Pkg { + return { dirPath: tmpDir, packageJson } as Pkg; +} + function installCtx(overrides: Partial = {}): InstallContext { return { shell: {} as ShellService, // biome-ignore lint/suspicious/noExplicitAny: minimal stub for the test boundary logger: {} as any, - appPkg: { dirPath: tmpDir } as Pkg, + appPkg: appPkg(), prompts: stubPrompts(), flags: { force: false, yes: true, nonInteractive: true }, release: new ReleaseService(undefined), @@ -34,7 +38,7 @@ function uninstallCtx(overrides: Partial = {}): UninstallConte shell: {} as ShellService, // biome-ignore lint/suspicious/noExplicitAny: minimal stub logger: {} as any, - appPkg: { dirPath: tmpDir } as Pkg, + appPkg: appPkg(), prompts: stubPrompts(), flags: { yes: true, nonInteractive: true }, ...overrides, @@ -112,6 +116,31 @@ describe("@rrlab/ts-plugin install()", () => { }); }); + describe("default preset", () => { + it.each([ + ["dependencies", { dependencies: { react: "^19.0.0" } }], + ["peerDependencies", { peerDependencies: { react: ">=18" } }], + ])("is the React preset when react is in %s", async (_field, packageJson) => { + const result = await install(installCtx({ appPkg: appPkg(packageJson) })); + const op = result.files?.[0]; + if (op?.kind !== "create") throw new Error("expected create op"); + expect(JSON.parse(op.content).extends).toBe("@rrlab/ts-config/react"); + expect(result.devDependencies).not.toHaveProperty("@types/node"); + }); + + it("is where the interactive prompt starts", async () => { + const select = vi.fn(async () => "react" as unknown) as unknown as ClackPrompts["select"]; + await install( + installCtx({ + appPkg: appPkg({ devDependencies: { react: "^19.0.0" } }), + flags: { force: false, yes: false, nonInteractive: false }, + prompts: stubPrompts({ select }), + }), + ); + expect(select).toHaveBeenCalledWith(expect.objectContaining({ initialValue: "react" })); + }); + }); + describe("preset → devDeps", () => { it("omits @types/node for dom presets", async () => { const select = vi.fn(async () => "react" as unknown) as unknown as ClackPrompts["select"]; diff --git a/run-run/ts-plugin/src/index.ts b/run-run/ts-plugin/src/index.ts index db28c5bc..522a3827 100644 --- a/run-run/ts-plugin/src/index.ts +++ b/run-run/ts-plugin/src/index.ts @@ -53,7 +53,16 @@ const PRESETS: Record = { "no-dom-lib": { extendsPath: "@rrlab/ts-config/no-dom/lib", label: "Node.js library", needsNode: true }, }; -const DEFAULT_PRESET: Preset = "no-dom-app"; +/** + * A package that depends on `react` (peer included, for a component library) + * gets the React preset; anything else, a Node.js app. It's the answer `--yes` + * and a run without a TTY take, and the one the prompt starts on. + */ +function defaultPreset(pkg: InstallContext["appPkg"]): Preset { + const { dependencies, devDependencies, peerDependencies } = pkg.packageJson; + const dependsOnReact = [dependencies, devDependencies, peerDependencies].some((deps) => !!deps && "react" in deps); + return dependsOnReact ? "react" : "no-dom-app"; +} export async function install(ctx: InstallContext): Promise { const tsconfigPath = path.join(ctx.appPkg.dirPath, TSCONFIG); @@ -71,7 +80,7 @@ export async function install(ctx: InstallContext): Promise { const preset = await pickPreset(ctx, { message: "Which kind of TS project do you need?", presets: PRESETS, - defaultPreset: DEFAULT_PRESET, + defaultPreset: defaultPreset(ctx.appPkg), }); const presetInfo = PRESETS[preset]; From c9903ab1a2d3c9a227d2dd8ebe1575ed7d3b44d9 Mon Sep 17 00:00:00 2001 From: "Ricardo Q. Bazan" Date: Sat, 17 Oct 2026 19:02:28 -0500 Subject: [PATCH 5/5] fix(cli): run rr plugins add without a TTY, and make remove ask for --yes Only --yes made `rr plugins add` non-interactive, so from an agent or CI it waited on "Scaffold biome.json?" or exited 13 on a closed stdin, with the plugin installed but not configured. `rr plugins remove` waited on its confirmation the same way. Treat a run without a TTY on stdin and stdout as unattended. add takes the defaults, as with --yes. remove, which deletes files and dependencies, stops with an error that asks for --yes. The integration tests add plugins with a pnpm stub and then run `rr jsc` on the result. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Tj9MBgi2cQcRxSD9uikx6k --- .changeset/cli-plugins-no-tty.md | 5 ++ run-run/cli/src/actions/plugins/add.ts | 4 +- run-run/cli/src/actions/plugins/remove.ts | 7 +- .../cli/src/errors/confirmation-required.ts | 7 ++ run-run/cli/src/services/prompts.ts | 9 +++ run-run/cli/test/helpers.ts | 2 + run-run/cli/test/integration/plugins.test.ts | 68 ++++++++++++++++++- 7 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 .changeset/cli-plugins-no-tty.md create mode 100644 run-run/cli/src/errors/confirmation-required.ts diff --git a/.changeset/cli-plugins-no-tty.md b/.changeset/cli-plugins-no-tty.md new file mode 100644 index 00000000..91617bd4 --- /dev/null +++ b/.changeset/cli-plugins-no-tty.md @@ -0,0 +1,5 @@ +--- +"@rrlab/cli": patch +--- + +`rr plugins add` without a terminal (an agent, a script, CI) takes the defaults, as with `--yes`, instead of prompting: it used to wait on the prompt, or exit 13 with the plugin half installed. `rr plugins remove` without a terminal and without `--yes` stops with an error that asks for `--yes`, since it deletes files and dependencies. Both count as a terminal only when stdin and stdout are TTYs. diff --git a/run-run/cli/src/actions/plugins/add.ts b/run-run/cli/src/actions/plugins/add.ts index 3fe0a517..753e6923 100644 --- a/run-run/cli/src/actions/plugins/add.ts +++ b/run-run/cli/src/actions/plugins/add.ts @@ -10,7 +10,7 @@ import { ConfigAstService } from "#src/services/config-ast.ts"; import type { ContextValue } from "#src/services/context.ts"; import { applyFileOp } from "#src/services/file-ops.ts"; import { logger } from "#src/services/logger.ts"; -import { createClackPrompts } from "#src/services/prompts.ts"; +import { canPrompt, createClackPrompts } from "#src/services/prompts.ts"; import { ReleaseService } from "#src/services/release.ts"; import { describeWorkspaceChoice, resolveWorkspaceChoice, toNypmWorkspace } from "#src/services/workspace-target.ts"; import { type AddOptions, hasInPackageJson, type PluginModule, reportFileOp } from "./shared.ts"; @@ -104,7 +104,7 @@ export async function addPluginAction({ ctx, args, options }: AddPluginActionCon flags: { force: !!options.force, yes: !!options.yes, - nonInteractive: !!options.yes, + nonInteractive: !!options.yes || !canPrompt(), }, release: new ReleaseService(tag), }; diff --git a/run-run/cli/src/actions/plugins/remove.ts b/run-run/cli/src/actions/plugins/remove.ts index 18246283..8e45cc92 100644 --- a/run-run/cli/src/actions/plugins/remove.ts +++ b/run-run/cli/src/actions/plugins/remove.ts @@ -1,6 +1,7 @@ import path from "node:path"; import * as clack from "@clack/prompts"; import { detectPackageManager, removeDependency } from "nypm"; +import { ConfirmationRequiredError } from "#src/errors/confirmation-required.ts"; import { PLUGINS_DIRECTORY, type PluginName } from "#src/lib/plugin/directory.ts"; import type { UninstallResult } from "#src/lib/plugin/types.ts"; import { withSpinner } from "#src/render/steps.ts"; @@ -8,7 +9,7 @@ import { ConfigAstService } from "#src/services/config-ast.ts"; import type { ContextValue } from "#src/services/context.ts"; import { applyFileOp, describeFileOp } from "#src/services/file-ops.ts"; import { logger } from "#src/services/logger.ts"; -import { createClackPrompts } from "#src/services/prompts.ts"; +import { canPrompt, createClackPrompts } from "#src/services/prompts.ts"; import { describeWorkspaceChoice, resolveWorkspaceChoice, toNypmWorkspace } from "#src/services/workspace-target.ts"; import { hasInPackageJson, type PluginModule, type RemoveOptions, reportFileOp } from "./shared.ts"; @@ -41,7 +42,7 @@ export async function removePluginAction({ ctx, args, options }: RemovePluginAct logger, appPkg: ctx.appPkg, prompts: createClackPrompts(), - flags: { yes: !!options.yes, nonInteractive: !!options.yes }, + flags: { yes: !!options.yes, nonInteractive: !!options.yes || !canPrompt() }, }); } } catch (err) { @@ -83,6 +84,8 @@ export async function removePluginAction({ ctx, args, options }: RemovePluginAct } if (!options.yes) { + // Removing deletes files and dependencies: without a terminal to confirm in, insist on --yes. + if (!canPrompt()) throw new ConfirmationRequiredError(`rr plugins remove ${name}`); const choice = await clack.confirm({ message: "Proceed?", initialValue: false }); if (clack.isCancel(choice) || choice !== true) { clack.outro("Aborted."); diff --git a/run-run/cli/src/errors/confirmation-required.ts b/run-run/cli/src/errors/confirmation-required.ts new file mode 100644 index 00000000..325e4c9e --- /dev/null +++ b/run-run/cli/src/errors/confirmation-required.ts @@ -0,0 +1,7 @@ +import { KnownError } from "./known-error.ts"; + +export class ConfirmationRequiredError extends KnownError { + constructor(command: string) { + super(`'${command}' asks for confirmation, and there's no terminal to ask in. Re-run it with --yes.`); + } +} diff --git a/run-run/cli/src/services/prompts.ts b/run-run/cli/src/services/prompts.ts index 3efb2436..5522bb02 100644 --- a/run-run/cli/src/services/prompts.ts +++ b/run-run/cli/src/services/prompts.ts @@ -1,6 +1,15 @@ import * as clack from "@clack/prompts"; import type { ClackPrompts } from "#src/lib/plugin/types.ts"; +/** + * Whether a prompt can run: clack reads keys from stdin and draws on stdout, + * so both have to be terminals. An agent, a script or CI usually has neither, + * and a prompt there waits forever or exits on a closed stdin. + */ +export function canPrompt(): boolean { + return !!process.stdin.isTTY && !!process.stdout.isTTY; +} + /** * Adapter that exposes the subset of `@clack/prompts` matching the * `ClackPrompts` contract from `@rrlab/cli/plugin`. The contract intentionally diff --git a/run-run/cli/test/helpers.ts b/run-run/cli/test/helpers.ts index 927ca8a8..bc292713 100644 --- a/run-run/cli/test/helpers.ts +++ b/run-run/cli/test/helpers.ts @@ -11,6 +11,7 @@ const WORKSPACE_NODE_MODULES = resolve(dirnameOf(import.meta), "../../../node_mo type CliOptions = { cwd?: string; + env?: NodeJS.ProcessEnv; }; export function createTestCli() { @@ -23,6 +24,7 @@ export function createTestCli() { NODE_ENV: "production", TEST: undefined, NO_COLOR: "1", + ...opts.env, }, }); }; diff --git a/run-run/cli/test/integration/plugins.test.ts b/run-run/cli/test/integration/plugins.test.ts index 23d30355..9c6b2b3d 100644 --- a/run-run/cli/test/integration/plugins.test.ts +++ b/run-run/cli/test/integration/plugins.test.ts @@ -1,10 +1,26 @@ -import { readFileSync } from "node:fs"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; import { afterEach, describe, expect, test } from "vitest"; import { createTestCli, fixtures, makeFixture } from "#test/helpers.ts"; const cli = createTestCli(); +/** + * A pnpm that installs nothing, first on PATH: the plugins and their tools + * already resolve from the workspace's node_modules. nypm only calls pnpm + * directly when `corepack --version` fails. + */ +function stubPackageManager(dir: string): NodeJS.ProcessEnv { + const bin = path.join(dir, ".stub-bin"); + mkdirSync(bin); + writeFileSync(path.join(bin, "pnpm"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync(path.join(bin, "corepack"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + return { PATH: `${bin}${path.delimiter}${process.env.PATH}` }; +} + +const pnpmPkg = (extra: Record = {}) => + `${JSON.stringify({ name: "rr-test-fixture", version: "0.0.0", private: true, ...extra, packageManager: "pnpm@10.30.3" }, null, 2)}\n`; + describe("rr plugins", () => { let fixture: { dir: string; cleanup: () => void }; @@ -133,6 +149,56 @@ describe("rr plugins", () => { }); }); + // The test runner's stdin and stdout are pipes, like an agent's or CI's. + describe("add without a TTY", () => { + test("takes the defaults, and what it writes passes `rr jsc`", () => { + fixture = makeFixture("plugins-add-no-tty", { + ".gitignore": "node_modules\n.stub-bin\n", + "package.json": pnpmPkg(), + }); + const env = stubPackageManager(fixture.dir); + + const r = cli("plugins add biome", { cwd: fixture.dir, env }); + expect(r.stdout + r.stderr).toContain("Created biome.json"); + expect(r.status).toBe(0); + + expect(readFileSync(path.join(fixture.dir, "biome.json"), "utf8")).toBe(`{ + "$schema": "./node_modules/@biomejs/biome/configuration_schema.json", + "extends": ["@rrlab/biome-config"] +} +`); + const jsc = cli("jsc", { cwd: fixture.dir }); + expect(jsc.status, jsc.stdout + jsc.stderr).toBe(0); + }); + + test("ts picks the React preset for a package that depends on react", () => { + fixture = makeFixture("plugins-add-no-tty-react", { + "package.json": pnpmPkg({ dependencies: { react: "^19.0.0" } }), + }); + + const r = cli("plugins add ts", { cwd: fixture.dir, env: stubPackageManager(fixture.dir) }); + expect(r.status).toBe(0); + + const tsconfig = JSON.parse(readFileSync(path.join(fixture.dir, "tsconfig.json"), "utf8")); + expect(tsconfig.extends).toBe("@rrlab/ts-config/react"); + }); + }); + + describe("remove without a TTY", () => { + test("asks for --yes instead of a confirmation it can't get, and changes nothing", () => { + const config = fixtures.config(["biome"]); + fixture = makeFixture("plugins-remove-no-tty", { + "package.json": fixtures.pkg(), + "run-run.config.mts": config, + }); + + const r = cli("plugins remove biome", { cwd: fixture.dir }); + expect(r.stdout + r.stderr).toContain("Re-run it with --yes"); + expect(r.status).toBe(1); + expect(readFileSync(path.join(fixture.dir, "run-run.config.mts"), "utf8")).toBe(config); + }); + }); + describe("remove --yes", () => { test("removes the plugin entry from the config without prompting", () => { fixture = makeFixture("plugins-remove-yes", {