From bbfed9fb3d94fa0d9b85115e980f761eb2378ab2 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Thu, 14 May 2026 12:06:09 +0900 Subject: [PATCH 01/16] feat: implement CLAUDECODE strict mode for arkor init and create-arkor - Added strict mode for `arkor init` and `create-arkor` when CLAUDECODE=1 is set, requiring explicit flags for all interactive prompts. - Introduced helper functions to check for missing flags and format error messages for the user. - Updated tests to cover new strict mode behavior, ensuring proper exit codes and error messages when required flags are missing. - Modified CLI environment handling to prevent unintended interactions with CLAUDECODE during tests. --- docs/cli/init.mdx | 29 ++++ docs/ja/cli/init.mdx | 29 ++++ e2e/cli/src/arkor-init.test.ts | 68 +++++++++ e2e/cli/src/create-arkor.test.ts | 82 ++++++++++ e2e/cli/src/spawn-cli.ts | 10 ++ packages/arkor/src/cli/commands/init.test.ts | 13 +- packages/arkor/src/cli/commands/login.test.ts | 7 + .../arkor/src/cli/commands/logout.test.ts | 8 + packages/arkor/src/cli/main.ts | 42 +++++- packages/arkor/src/cli/prompts.test.ts | 29 ++++ packages/arkor/src/cli/prompts.ts | 6 +- packages/cli-internal/src/claude-code.test.ts | 141 ++++++++++++++++++ packages/cli-internal/src/claude-code.ts | 140 +++++++++++++++++ packages/cli-internal/src/index.ts | 7 + packages/create-arkor/src/bin.ts | 41 ++++- 15 files changed, 647 insertions(+), 5 deletions(-) create mode 100644 packages/cli-internal/src/claude-code.test.ts create mode 100644 packages/cli-internal/src/claude-code.ts diff --git a/docs/cli/init.mdx b/docs/cli/init.mdx index fa337d4d..5f515d78 100644 --- a/docs/cli/init.mdx +++ b/docs/cli/init.mdx @@ -122,6 +122,35 @@ bun arkor init --yes --template triage --use-bun --skip-git If you forget `--yes` in a non-interactive shell, the command still completes (prompts use their defaults) but the experience is silent and easy to miss in logs. Prefer the explicit form above. +### Claude Code (`CLAUDECODE=1`) strict mode + +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `arkor init` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. + +Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): + +- `--template ` +- `--git` (recommended; matches the interactive default) or `--skip-git` +- A package-manager flag: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`, or `--skip-install` +- `--agents-md` (recommended, especially under CLAUDECODE) or `--no-agents-md` + +Sample stderr when a flag is missing. Each flag is paired with a one-line description so the agent can pick a value without round-tripping to the docs: + +```text +arkor init: CLAUDECODE=1 detected. Interactive prompts are disabled. +Re-run with explicit flags: + --template + Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). + --git (recommended) or --skip-git + `--git` runs `git init` and creates an initial commit (matches the interactive default); `--skip-git` leaves git setup to the user. + --use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install) + Which package manager to run `install` with after scaffolding. `--skip-install` leaves the install step to the user. + --agents-md (recommended) or --no-agents-md + `--agents-md` writes `AGENTS.md` + `CLAUDE.md` to brief AI coding agents that arkor post-dates their training data (recommended, especially under CLAUDECODE); `--no-agents-md` skips them. +Or pass -y/--yes to accept all defaults. +``` + +The exit code is `1` and no scaffold work happens before the early exit, so the directory stays pristine. The same rule applies to [`create-arkor`](/quickstart#1-scaffold-a-project), with the additional requirement of `[dir]` or `--name ` to make the project name an explicit decision. + ## Errors | Message | What it means | Fix | diff --git a/docs/ja/cli/init.mdx b/docs/ja/cli/init.mdx index 9712ff89..e4fb2f62 100644 --- a/docs/ja/cli/init.mdx +++ b/docs/ja/cli/init.mdx @@ -122,6 +122,35 @@ bun arkor init --yes --template triage --use-bun --skip-git 非対話シェルで `--yes` を忘れてもコマンドは完走します（プロンプトはデフォルトを使う）が、静かに動作するためログからは見落としやすい挙動になります。上記の明示形を推奨します。 +### Claude Code（`CLAUDECODE=1`）厳格モード + +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `arkor init` は厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。 + +必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトインしてください）: + +- `--template ` +- `--git`（推奨。対話モードのデフォルトと同じ）または `--skip-git` +- パッケージマネージャー系フラグ: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`、または `--skip-install` +- `--agents-md`（特に CLAUDECODE 下では推奨）または `--no-agents-md` + +フラグが不足したときの stderr 例。各フラグには一行の説明が併記され、ドキュメントを往復しなくてもエージェントが値を決められるようになっています: + +```text +arkor init: CLAUDECODE=1 detected. Interactive prompts are disabled. +Re-run with explicit flags: + --template + Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). + --git (recommended) or --skip-git + `--git` runs `git init` and creates an initial commit (matches the interactive default); `--skip-git` leaves git setup to the user. + --use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install) + Which package manager to run `install` with after scaffolding. `--skip-install` leaves the install step to the user. + --agents-md (recommended) or --no-agents-md + `--agents-md` writes `AGENTS.md` + `CLAUDE.md` to brief AI coding agents that arkor post-dates their training data (recommended, especially under CLAUDECODE); `--no-agents-md` skips them. +Or pass -y/--yes to accept all defaults. +``` + +終了コードは `1`、早期終了の前にファイル生成は一切行われないので、ディレクトリは元の状態のままです。同じルールは [`create-arkor`](/ja/quickstart#1-プロジェクトを生成する) にも適用され、加えて `[dir]` か `--name ` も必須となります（プロジェクト名を明示的に決めさせるため）。 + ## エラー | メッセージ | 意味 | 対処 | diff --git a/e2e/cli/src/arkor-init.test.ts b/e2e/cli/src/arkor-init.test.ts index 82a16f72..829f7f60 100644 --- a/e2e/cli/src/arkor-init.test.ts +++ b/e2e/cli/src/arkor-init.test.ts @@ -230,6 +230,74 @@ describe("arkor init (E2E)", () => { expect(pkg.name).toBe("foo-bar"); }); + describe("CLAUDECODE=1 strict mode", () => { + // Claude Code (the Anthropic agent CLI) spawns child processes with + // `CLAUDECODE=1` and cannot answer interactive prompts. Falling through + // to silent defaults would hide decisions the agent should be making, so + // `arkor init` refuses to run unless every interactive-equivalent flag + // is supplied (or `--yes` opts back into the legacy "accept defaults" + // path). + it("exits 1 with a flag list (and per-flag description) when no options are given", async () => { + const result = await runCli(ARKOR_BIN, ["init"], cwd, { + CLAUDECODE: "1", + }); + expect(result.code).toBe(1); + expect(result.stderr).toContain( + "arkor init: CLAUDECODE=1 detected", + ); + expect(result.stderr).toContain("--template "); + expect(result.stderr).toContain("--git (recommended) or --skip-git"); + expect(result.stderr).toContain("--use-pnpm"); + expect(result.stderr).toContain( + "--agents-md (recommended) or --no-agents-md", + ); + expect(result.stderr).toContain("-y/--yes"); + // Each flag is paired with a description so the agent can pick a + // value without round-tripping to the docs. + expect(result.stderr).toContain("Starter template"); + expect(result.stderr).toContain("git init"); + expect(result.stderr).toContain("package manager"); + expect(result.stderr).toContain("AGENTS.md"); + // No scaffold side effects must have occurred; the tree must be + // pristine after the early exit. + expect(existsSync(join(cwd, "src/arkor/index.ts"))).toBe(false); + expect(existsSync(join(cwd, "package.json"))).toBe(false); + }); + + it("runs to completion when every required flag is set", async () => { + const result = await runCli( + ARKOR_BIN, + [ + "init", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + cwd, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(0); + expect(existsSync(join(cwd, "src/arkor/index.ts"))).toBe(true); + // --no-agents-md was honoured. + expect(existsSync(join(cwd, "AGENTS.md"))).toBe(false); + }); + + it("accepts --yes as a wholesale opt-out of the strict check", async () => { + // `-y` keeps the legacy "use defaults for everything" semantics for + // callers who have explicitly delegated those decisions. + const result = await runCli( + ARKOR_BIN, + ["init", "-y", "--skip-install", "--skip-git"], + cwd, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(0); + expect(existsSync(join(cwd, "src/arkor/index.ts"))).toBe(true); + }); + }); + it("skips git init when the target is already a git repo", async () => { // Pre-seed a git repo so isInGitRepo() short-circuits. const initRes = await runGit(cwd, ["init", "-q"]); diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index cd94a5df..9513d4ec 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -511,4 +511,86 @@ describe("create-arkor (E2E)", () => { }, 180_000, ); + + describe("CLAUDECODE=1 strict mode", () => { + // Claude Code (the Anthropic agent CLI) spawns child processes with + // `CLAUDECODE=1` and cannot answer interactive prompts. Falling through + // to silent defaults would hide decisions the agent should be making, so + // `create-arkor` refuses to run unless every interactive-equivalent flag + // is supplied (or `--yes` opts back into the legacy "accept defaults" + // path). Unlike `arkor init`, this includes `[dir]` / `--name` because + // the otherwise-default project name (`arkor-project`) is generic enough + // that it almost always reflects an oversight rather than intent. + it("exits 1 with a flag list (and per-flag description) when no options are given (missing [dir])", async () => { + // Bypass `runCreateArkor` because that helper always injects + // `target` as the positional; to exercise the missing-[dir] + // branch we need an argv with no positional at all. + const result = await runCli(CREATE_ARKOR_BIN, [], parentDir, { + CLAUDECODE: "1", + }); + expect(result.code).toBe(1); + expect(result.stderr).toContain( + "create-arkor: CLAUDECODE=1 detected", + ); + expect(result.stderr).toContain("[dir]"); + expect(result.stderr).toContain("--template "); + expect(result.stderr).toContain("--git (recommended) or --skip-git"); + expect(result.stderr).toContain("--use-pnpm"); + expect(result.stderr).toContain( + "--agents-md (recommended) or --no-agents-md", + ); + // Each flag is paired with a description so the agent can pick a + // value without round-tripping to the docs. + expect(result.stderr).toContain("Project directory"); + expect(result.stderr).toContain("Starter template"); + expect(result.stderr).toContain("git init"); + expect(result.stderr).toContain("package manager"); + expect(result.stderr).toContain("AGENTS.md"); + // Sanity: exit happened before any scaffold work. + expect(existsSync(join(parentDir, "package.json"))).toBe(false); + }); + + it("still exits 1 when [dir] is given but other prompts are missing", async () => { + // Mirror an agent invocation that knows the project name but hasn't + // yet committed to template / git / pm / agents-md; the [dir] + // alone is not enough to bypass the strict check. + const { result } = await runCreateArkor([], { CLAUDECODE: "1" }); + expect(result.code).toBe(1); + // `[dir]` is satisfied (runCreateArkor passes "target"), so the + // missing list must omit it. + expect(result.stderr).not.toContain("[dir]"); + expect(result.stderr).toContain("--template"); + expect(result.stderr).toContain("--git (recommended) or --skip-git"); + expect(result.stderr).toContain( + "--agents-md (recommended) or --no-agents-md", + ); + }); + + it("runs to completion when every required flag is set", async () => { + const { result, targetDir } = await runCreateArkor( + [ + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(0); + expect(existsSync(join(targetDir, "src/arkor/index.ts"))).toBe(true); + expect(existsSync(join(targetDir, "AGENTS.md"))).toBe(false); + }); + + it("accepts --yes as a wholesale opt-out of the strict check", async () => { + // `-y` keeps the legacy "use defaults for everything" semantics for + // callers who have explicitly delegated those decisions. + const { result, targetDir } = await runCreateArkor( + ["-y", "--skip-install", "--skip-git"], + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(0); + expect(existsSync(join(targetDir, "src/arkor/index.ts"))).toBe(true); + }); + }); }); diff --git a/e2e/cli/src/spawn-cli.ts b/e2e/cli/src/spawn-cli.ts index 3b060a50..34cfb806 100644 --- a/e2e/cli/src/spawn-cli.ts +++ b/e2e/cli/src/spawn-cli.ts @@ -280,6 +280,16 @@ function runCliOnce( env: { ...cleanEnv, CI: "1", + // The CLI's CLAUDECODE strict mode hard-fails on any invocation + // that doesn't declare every interactive-equivalent flag. When + // the e2e suite itself is run from a Claude Code session, + // `process.env.CLAUDECODE` is `"1"` in the parent and leaks + // through `cleanEnv` to every spawned child, which then turns + // unrelated assertions (e.g. tests that only set `--name` + + // `--template`) into surprise exit-1s. Clear it here so the + // default test environment matches a vanilla CI shell; the + // CLAUDECODE-specific tests opt back in via `extraEnv`. + CLAUDECODE: "", npm_config_user_agent: "", GIT_AUTHOR_NAME: "Arkor E2E", GIT_AUTHOR_EMAIL: "e2e@arkor.test", diff --git a/packages/arkor/src/cli/commands/init.test.ts b/packages/arkor/src/cli/commands/init.test.ts index a3640fdd..179528b1 100644 --- a/packages/arkor/src/cli/commands/init.test.ts +++ b/packages/arkor/src/cli/commands/init.test.ts @@ -63,8 +63,12 @@ const ORIG_CWD = process.cwd(); // Capture the original env / TTY state so the after-each can restore // conditionally. CI runners typically have CI already set, and an // unconditional `delete` would leak a different environment to later -// test files when vitest reuses a worker. +// test files when vitest reuses a worker. CLAUDECODE is captured +// because vitest workers spawned from Claude Code inherit it (=`1`), +// which would otherwise force isInteractive() to false and break the +// "interactive" branch tests. const ORIG_CI = process.env.CI; +const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_TTY = process.stdout.isTTY; beforeEach(() => { @@ -76,8 +80,11 @@ beforeEach(() => { cwd = realpathSync(mkdtempSync(join(tmpdir(), "arkor-init-test-"))); process.chdir(cwd); // Pin non-interactive so promptText/Select fall through to skipWith / - // initialValue without opening clack. + // initialValue without opening clack. Strip CLAUDECODE so individual + // tests can opt back into the interactive branch by unsetting CI + + // setting isTTY without inheriting Claude Code's CLAUDECODE=1. process.env.CI = "1"; + delete process.env.CLAUDECODE; vi.mocked(scaffold).mockClear(); vi.mocked(install).mockClear(); vi.mocked(gitInitialCommit).mockClear(); @@ -92,6 +99,8 @@ afterEach(() => { rmSync(cwd, { recursive: true, force: true }); if (ORIG_CI === undefined) delete process.env.CI; else process.env.CI = ORIG_CI; + if (ORIG_CLAUDECODE === undefined) delete process.env.CLAUDECODE; + else process.env.CLAUDECODE = ORIG_CLAUDECODE; // Restore the TTY flag in case an interactive test mutated it — // otherwise a later test that unsets CI would unexpectedly enter // interactive prompt paths. diff --git a/packages/arkor/src/cli/commands/login.test.ts b/packages/arkor/src/cli/commands/login.test.ts index 88883d5a..f61e3501 100644 --- a/packages/arkor/src/cli/commands/login.test.ts +++ b/packages/arkor/src/cli/commands/login.test.ts @@ -23,6 +23,10 @@ const ORIG_HOME = process.env.HOME; // tests via `~/.arkor/credentials.json`. const ORIG_USERPROFILE = process.env.USERPROFILE; const ORIG_CI = process.env.CI; +// CLAUDECODE is captured because vitest workers spawned from Claude Code +// inherit `CLAUDECODE=1`, which forces isInteractive() to false and +// breaks the "interactive" branch tests below that flip CI off + isTTY on. +const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_FETCH = globalThis.fetch; const ORIG_URL = process.env.ARKOR_CLOUD_API_URL; @@ -34,6 +38,7 @@ beforeEach(() => { // Force isInteractive() → false so promptSelect returns its initialValue // instead of trying to open a real clack prompt. process.env.CI = "1"; + delete process.env.CLAUDECODE; vi.mocked(open).mockReset(); }); @@ -44,6 +49,8 @@ afterEach(() => { else delete process.env.USERPROFILE; if (ORIG_CI !== undefined) process.env.CI = ORIG_CI; else delete process.env.CI; + if (ORIG_CLAUDECODE !== undefined) process.env.CLAUDECODE = ORIG_CLAUDECODE; + else delete process.env.CLAUDECODE; if (ORIG_URL !== undefined) process.env.ARKOR_CLOUD_API_URL = ORIG_URL; else delete process.env.ARKOR_CLOUD_API_URL; globalThis.fetch = ORIG_FETCH; diff --git a/packages/arkor/src/cli/commands/logout.test.ts b/packages/arkor/src/cli/commands/logout.test.ts index c9f96c0f..e78f9260 100644 --- a/packages/arkor/src/cli/commands/logout.test.ts +++ b/packages/arkor/src/cli/commands/logout.test.ts @@ -32,6 +32,11 @@ const ORIG_USERPROFILE = process.env.USERPROFILE; const ORIG_HOMEDRIVE = process.env.HOMEDRIVE; const ORIG_HOMEPATH = process.env.HOMEPATH; const ORIG_CI = process.env.CI; +// CLAUDECODE is captured because vitest workers spawned from Claude Code +// inherit `CLAUDECODE=1`, which forces isInteractive() to false and +// breaks the interactive-branch test below. Strip it in beforeEach and +// restore in afterEach. +const ORIG_CLAUDECODE = process.env.CLAUDECODE; // Capture the original `process.stdout.isTTY` so the interactive test // below can flip it without leaking into other test files when vitest // reuses a worker process. @@ -56,6 +61,7 @@ beforeEach(() => { // promptConfirm honours skipWith first, but in non-skip paths it falls // through to initialValue when CI=1. Pin CI so the prompt never opens. process.env.CI = "1"; + delete process.env.CLAUDECODE; }); afterEach(() => { @@ -69,6 +75,8 @@ afterEach(() => { else delete process.env.HOMEPATH; if (ORIG_CI !== undefined) process.env.CI = ORIG_CI; else delete process.env.CI; + if (ORIG_CLAUDECODE !== undefined) process.env.CLAUDECODE = ORIG_CLAUDECODE; + else delete process.env.CLAUDECODE; // Restore the TTY flag in case the interactive test mutated it — // otherwise a later test that unsets CI would unexpectedly enter // interactive prompt paths. diff --git a/packages/arkor/src/cli/main.ts b/packages/arkor/src/cli/main.ts index 65340fa8..553b32e5 100644 --- a/packages/arkor/src/cli/main.ts +++ b/packages/arkor/src/cli/main.ts @@ -6,7 +6,13 @@ import { runInit } from "./commands/init"; import { runDev } from "./commands/dev"; import { runBuild } from "./commands/build"; import { runStart } from "./commands/start"; -import { resolvePackageManager, type TemplateId } from "@arkor/cli-internal"; +import { + formatClaudeCodeMissingMessage, + isClaudeCode, + missingClaudeCodeFlags, + resolvePackageManager, + type TemplateId, +} from "@arkor/cli-internal"; import { getRecordedDeprecation } from "../core/deprecation"; import { shutdownTelemetry, withTelemetry } from "../core/telemetry"; import { detectedUpgradeCommand } from "../core/upgrade-hint"; @@ -80,6 +86,40 @@ export async function main(argv: string[]): Promise { "Pick one of --agents-md / --no-agents-md, not both.", ); } + // Under CLAUDECODE=1, refuse to fall through to interactive prompts + // (they'd hang) or to silent defaults (they'd hide decisions the + // agent should be making). Print the suggested re-invocation and + // exit 1 so the agent can re-run with explicit flags. `agentsMd` + // is checked from raw argv so default-on doesn't satisfy the + // requirement: the agent should opt in or out deliberately. + if (isClaudeCode()) { + const agentsMdSpecified = + flagsArgv.includes("--agents-md") || + flagsArgv.includes("--no-agents-md"); + const missing = missingClaudeCodeFlags({ + yes: opts.yes, + template: opts.template, + git: opts.git, + skipGit: opts.skipGit, + skipInstall: opts.skipInstall, + useNpm: opts.useNpm, + usePnpm: opts.usePnpm, + useYarn: opts.useYarn, + useBun: opts.useBun, + name: opts.name, + agentsMd: agentsMdSpecified ? opts.agentsMd ?? true : undefined, + // arkor init operates on `process.cwd()`; basename(cwd) is a + // meaningful default name, so an explicit --name isn't + // required. + requireProjectName: false, + }); + if (missing.length > 0) { + process.stderr.write( + formatClaudeCodeMissingMessage("arkor init", missing), + ); + process.exit(1); + } + } const packageManager = resolvePackageManager({ useNpm: opts.useNpm, usePnpm: opts.usePnpm, diff --git a/packages/arkor/src/cli/prompts.test.ts b/packages/arkor/src/cli/prompts.test.ts index 0f99c034..a86f1605 100644 --- a/packages/arkor/src/cli/prompts.test.ts +++ b/packages/arkor/src/cli/prompts.test.ts @@ -31,12 +31,14 @@ import { const CLACK_CANCEL = Symbol.for("clack:cancel"); const ORIG_CI = process.env.CI; +const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_TTY = process.stdout.isTTY; beforeEach(() => { // Default to non-interactive: CI set OR no TTY. Individual tests can flip // the toggle. process.env.CI = "1"; + delete process.env.CLAUDECODE; Object.defineProperty(process.stdout, "isTTY", { value: false, configurable: true, @@ -46,6 +48,8 @@ beforeEach(() => { afterEach(() => { if (ORIG_CI === undefined) delete process.env.CI; else process.env.CI = ORIG_CI; + if (ORIG_CLAUDECODE === undefined) delete process.env.CLAUDECODE; + else process.env.CLAUDECODE = ORIG_CLAUDECODE; Object.defineProperty(process.stdout, "isTTY", { value: ORIG_TTY, configurable: true, @@ -80,6 +84,31 @@ describe("isInteractive", () => { }); expect(isInteractive()).toBe(true); }); + + it("returns false when CLAUDECODE === '1' even if stdout is a TTY", () => { + // Claude Code spawns the CLI with CLAUDECODE=1 and cannot answer + // interactive prompts; falling through to clack would hang forever. + delete process.env.CI; + Object.defineProperty(process.stdout, "isTTY", { + value: true, + configurable: true, + }); + process.env.CLAUDECODE = "1"; + expect(isInteractive()).toBe(false); + }); + + it("ignores CLAUDECODE values other than the literal '1'", () => { + // Exact-match contract: any other value (legacy CI envs that + // happen to set CLAUDECODE for unrelated reasons) must not flip + // the CLI into the strict mode. + delete process.env.CI; + Object.defineProperty(process.stdout, "isTTY", { + value: true, + configurable: true, + }); + process.env.CLAUDECODE = "true"; + expect(isInteractive()).toBe(true); + }); }); describe("promptText", () => { diff --git a/packages/arkor/src/cli/prompts.ts b/packages/arkor/src/cli/prompts.ts index 14848187..14d0bffd 100644 --- a/packages/arkor/src/cli/prompts.ts +++ b/packages/arkor/src/cli/prompts.ts @@ -13,7 +13,11 @@ import * as clack from "@clack/prompts"; */ export function isInteractive(): boolean { - return Boolean(process.stdout.isTTY) && !process.env.CI; + return ( + Boolean(process.stdout.isTTY) && + !process.env.CI && + process.env.CLAUDECODE !== "1" + ); } class CliCancelled extends Error { diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts new file mode 100644 index 00000000..dc1cac79 --- /dev/null +++ b/packages/cli-internal/src/claude-code.test.ts @@ -0,0 +1,141 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + formatClaudeCodeMissingMessage, + isClaudeCode, + missingClaudeCodeFlags, +} from "./claude-code"; + +const ORIG = process.env.CLAUDECODE; + +beforeEach(() => { + delete process.env.CLAUDECODE; +}); + +afterEach(() => { + if (ORIG === undefined) delete process.env.CLAUDECODE; + else process.env.CLAUDECODE = ORIG; +}); + +describe("isClaudeCode", () => { + it("returns true only when CLAUDECODE === '1'", () => { + process.env.CLAUDECODE = "1"; + expect(isClaudeCode()).toBe(true); + }); + + it("returns false when CLAUDECODE is unset", () => { + expect(isClaudeCode()).toBe(false); + }); + + it("returns false for any other value (e.g. 'true', '0', '')", () => { + // The contract is exact-string match on "1": Claude Code itself + // sets `CLAUDECODE=1` literally. Loosening this risks accidentally + // tripping the strict mode for unrelated CI envs that happen to set + // CLAUDECODE for some other reason. + for (const v of ["true", "0", "", "yes", "false"]) { + process.env.CLAUDECODE = v; + expect(isClaudeCode()).toBe(false); + } + }); +}); + +describe("missingClaudeCodeFlags", () => { + it("returns empty when --yes is set, regardless of other flags", () => { + expect(missingClaudeCodeFlags({ yes: true })).toEqual([]); + }); + + it("requires --template, --git/--skip-git, a pm flag, and an agents-md flag (init mode)", () => { + const missing = missingClaudeCodeFlags({ requireProjectName: false }); + expect(missing.map((m) => m.flag)).toEqual([ + "--template ", + "--git (recommended) or --skip-git", + "--use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install)", + "--agents-md (recommended) or --no-agents-md", + ]); + // Every entry pairs the flag with a non-empty description so the + // stderr block is self-explanatory. + for (const m of missing) { + expect(m.description.length).toBeGreaterThan(0); + } + }); + + it("additionally requires [dir] / --name when requireProjectName is true (create-arkor mode)", () => { + const missing = missingClaudeCodeFlags({ requireProjectName: true }); + expect(missing[0]?.flag).toMatch(/\[dir\]/); + }); + + it("[dir] requirement is satisfied by an explicit positional", () => { + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + dir: "my-app", + template: "triage", + git: true, + skipInstall: true, + agentsMd: true, + }); + expect(missing).toEqual([]); + }); + + it("[dir] requirement is satisfied by --name", () => { + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + name: "my-app", + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + }); + + it("counts --skip-install as satisfying the package-manager requirement", () => { + const missing = missingClaudeCodeFlags({ + template: "triage", + git: true, + skipInstall: true, + agentsMd: true, + }); + expect(missing).toEqual([]); + }); + + it("counts --skip-git as satisfying the git requirement", () => { + const missing = missingClaudeCodeFlags({ + template: "triage", + skipGit: true, + usePnpm: true, + agentsMd: true, + }); + expect(missing).toEqual([]); + }); + + it("treats --no-agents-md (agentsMd === false) as a valid explicit choice", () => { + const missing = missingClaudeCodeFlags({ + template: "triage", + skipGit: true, + skipInstall: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + }); +}); + +describe("formatClaudeCodeMissingMessage", () => { + it("renders a stderr block with the command name, every missing flag + description, and the --yes escape hatch", () => { + const out = formatClaudeCodeMissingMessage("arkor init", [ + { + flag: "--template ", + description: "Pick the starter template.", + }, + { + flag: "--git or --skip-git", + description: "Init a git repo or skip git.", + }, + ]); + expect(out).toContain("arkor init: CLAUDECODE=1 detected"); + expect(out).toContain("--template "); + expect(out).toContain("Pick the starter template."); + expect(out).toContain("--git or --skip-git"); + expect(out).toContain("Init a git repo or skip git."); + expect(out).toContain("-y/--yes"); + expect(out.endsWith("\n")).toBe(true); + }); +}); diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts new file mode 100644 index 00000000..01e22254 --- /dev/null +++ b/packages/cli-internal/src/claude-code.ts @@ -0,0 +1,140 @@ +/** + * Claude Code (the Anthropic agent CLI) sets `CLAUDECODE=1` in every spawned + * shell. It cannot answer interactive prompts, and silently falling through to + * defaults masks decisions the agent should be making explicitly. So the + * scaffolders (`create-arkor`, `arkor init`) flip into a strict mode under + * this env var: every flag that mirrors an interactive prompt becomes + * required, and missing ones produce a stderr message listing the suggested + * re-invocation rather than running with hidden defaults. + * + * `--yes` opts back into the legacy "accept all defaults" path; callers that + * have explicitly delegated those decisions to the CLI keep working unchanged. + */ +export function isClaudeCode(): boolean { + return process.env.CLAUDECODE === "1"; +} + +export interface ClaudeCodeOptionsCheck { + /** True when `--yes`/`-y` was passed; bypasses the missing-flag check. */ + yes?: boolean; + /** Resolved template id, or undefined when `--template` was not passed. */ + template?: string; + /** True when `--git` was passed. */ + git?: boolean; + /** True when `--skip-git` was passed. */ + skipGit?: boolean; + /** True when `--skip-install` was passed. */ + skipInstall?: boolean; + /** True when any `--use-` flag was passed. */ + useNpm?: boolean; + usePnpm?: boolean; + useYarn?: boolean; + useBun?: boolean; + /** Project name (`--name`), or undefined. */ + name?: string; + /** + * Positional `[dir]` (create-arkor only). Pass `undefined` for `arkor init` + * which always operates on `process.cwd()`. + */ + dir?: string; + /** + * `false` only when `--no-agents-md` was passed; `true` when `--agents-md` + * was passed; `undefined` when neither flag was set. The default-on + * resolution lives in the CLI action handler. + */ + agentsMd?: boolean; + /** + * Whether the caller wants `[dir]` / `--name` to be required. `arkor init` + * uses `basename(cwd)` as a meaningful default and sets this to false; the + * `create-arkor` scaffolder defaults a missing `[dir]` to a generic + * `arkor-project/` subdirectory and sets this to true so the agent picks a + * project name deliberately. + */ + requireProjectName?: boolean; +} + +export interface MissingClaudeCodeFlag { + /** Flag (or flag group) the agent should add to the next invocation. */ + flag: string; + /** One-sentence explanation of what the flag controls. */ + description: string; +} + +/** + * Returns the list of missing flags under CLAUDECODE=1, each paired with a + * short description of what it controls, so the stderr message is + * self-explanatory and the agent can pick a value without round-tripping to + * the docs. Empty array means the invocation is fully specified (or `--yes` + * opted out). + */ +export function missingClaudeCodeFlags( + opts: ClaudeCodeOptionsCheck, +): MissingClaudeCodeFlag[] { + if (opts.yes) return []; + const missing: MissingClaudeCodeFlag[] = []; + if ( + opts.requireProjectName && + opts.dir === undefined && + opts.name === undefined + ) { + missing.push({ + flag: "[dir] (e.g. `my-arkor-app`) or --name ", + description: + "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used.", + }); + } + if (opts.template === undefined) { + missing.push({ + flag: "--template ", + description: + "Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal).", + }); + } + if (!opts.git && !opts.skipGit) { + missing.push({ + flag: "--git (recommended) or --skip-git", + description: + "`--git` runs `git init` and creates an initial commit (matches the interactive default); `--skip-git` leaves git setup to the user.", + }); + } + const pmFlagSet = + opts.useNpm || opts.usePnpm || opts.useYarn || opts.useBun || opts.skipInstall; + if (!pmFlagSet) { + missing.push({ + flag: "--use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install)", + description: + "Which package manager to run `install` with after scaffolding. `--skip-install` leaves the install step to the user.", + }); + } + if (opts.agentsMd === undefined) { + missing.push({ + flag: "--agents-md (recommended) or --no-agents-md", + description: + "`--agents-md` writes `AGENTS.md` + `CLAUDE.md` to brief AI coding agents that arkor post-dates their training data (recommended, especially under CLAUDECODE); `--no-agents-md` skips them.", + }); + } + return missing; +} + +/** + * Render the stderr block printed when `missingClaudeCodeFlags` returned + * a non-empty list. Kept as a pure function so both CLIs share the wording + * and tests can assert on it without invoking the action handler. Each + * missing flag is rendered on two lines (flag, then indented description) + * to avoid alignment churn when the longest flag changes. + */ +export function formatClaudeCodeMissingMessage( + command: string, + missing: MissingClaudeCodeFlag[], +): string { + const lines = [ + `${command}: CLAUDECODE=1 detected. Interactive prompts are disabled.`, + "Re-run with explicit flags:", + ]; + for (const m of missing) { + lines.push(` ${m.flag}`); + lines.push(` ${m.description}`); + } + lines.push("Or pass -y/--yes to accept all defaults."); + return `${lines.join("\n")}\n`; +} diff --git a/packages/cli-internal/src/index.ts b/packages/cli-internal/src/index.ts index c0d2dcd0..8e31b8b7 100644 --- a/packages/cli-internal/src/index.ts +++ b/packages/cli-internal/src/index.ts @@ -24,3 +24,10 @@ export { type InitialCommitResult, } from "./git"; export { sanitise } from "./sanitise"; +export { + isClaudeCode, + missingClaudeCodeFlags, + formatClaudeCodeMissingMessage, + type ClaudeCodeOptionsCheck, + type MissingClaudeCodeFlag, +} from "./claude-code"; diff --git a/packages/create-arkor/src/bin.ts b/packages/create-arkor/src/bin.ts index 66340515..f0a7f34e 100644 --- a/packages/create-arkor/src/bin.ts +++ b/packages/create-arkor/src/bin.ts @@ -6,9 +6,12 @@ import process from "node:process"; import * as clack from "@clack/prompts"; import { Command } from "commander"; import { + formatClaudeCodeMissingMessage, gitInitialCommit, install, + isClaudeCode, isInGitRepo, + missingClaudeCodeFlags, resolvePackageManager, sanitise, scaffold, @@ -41,7 +44,11 @@ const MANUAL_INSTALL_HINT = "install dependencies (npm i / pnpm install / yarn / bun install)"; function isInteractive(): boolean { - return Boolean(process.stdout.isTTY) && !process.env.CI; + return ( + Boolean(process.stdout.isTTY) && + !process.env.CI && + process.env.CLAUDECODE !== "1" + ); } /** @@ -368,6 +375,38 @@ program "Pick one of --agents-md / --no-agents-md, not both.", ); } + // Under CLAUDECODE=1, refuse to fall through to interactive prompts + // (they'd hang) or to silent defaults (they'd hide decisions the + // agent should be making). Print the suggested re-invocation and + // exit 1 so the agent can re-run with explicit flags. `agentsMd` + // is checked from raw argv so default-on doesn't satisfy the + // requirement: the agent should opt in or out deliberately. + if (isClaudeCode()) { + const agentsMdSpecified = + flagsArgv.includes("--agents-md") || + flagsArgv.includes("--no-agents-md"); + const missing = missingClaudeCodeFlags({ + yes: opts.yes, + template: opts.template, + git: opts.git, + skipGit: opts.skipGit, + skipInstall: opts.skipInstall, + useNpm: opts.useNpm, + usePnpm: opts.usePnpm, + useYarn: opts.useYarn, + useBun: opts.useBun, + name: opts.name, + dir, + agentsMd: agentsMdSpecified ? opts.agentsMd ?? true : undefined, + requireProjectName: true, + }); + if (missing.length > 0) { + process.stderr.write( + formatClaudeCodeMissingMessage("create-arkor", missing), + ); + process.exit(1); + } + } // Use `Object.hasOwn` (not `in`) so prototype keys like `toString` / // `__proto__` can't pass validation and crash later inside scaffold(). // Reject typos with an explicit error rather than silently coercing them From 6df7a9d20193c7bad54fbd221ef1b546ddfa2d90 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Thu, 14 May 2026 17:04:32 +0900 Subject: [PATCH 02/16] feat: implement strict mode for CLAUDECODE in init and create-arkor commands --- docs/cli/init.mdx | 2 +- docs/ja/cli/init.mdx | 2 +- docs/ja/quickstart.mdx | 32 +++++++++++ docs/quickstart.mdx | 32 +++++++++++ e2e/cli/src/create-arkor.test.ts | 36 ++++++++++++ packages/arkor/src/bin.ts | 14 ++++- packages/arkor/src/cli/commands/init.test.ts | 46 ++++++++++++++- packages/arkor/src/cli/commands/init.ts | 12 +++- packages/arkor/src/cli/main.test.ts | 53 ++++++++++++++++++ packages/arkor/src/cli/main.ts | 7 ++- packages/arkor/src/cli/prompts.test.ts | 24 +++----- packages/arkor/src/cli/prompts.ts | 6 +- packages/cli-internal/src/claude-code.test.ts | 56 +++++++++++++++++++ packages/cli-internal/src/claude-code.ts | 45 +++++++++++++-- packages/cli-internal/src/index.ts | 1 + packages/create-arkor/src/bin.ts | 27 ++++++--- 16 files changed, 353 insertions(+), 42 deletions(-) diff --git a/docs/cli/init.mdx b/docs/cli/init.mdx index 5f515d78..7f18cf52 100644 --- a/docs/cli/init.mdx +++ b/docs/cli/init.mdx @@ -149,7 +149,7 @@ Re-run with explicit flags: Or pass -y/--yes to accept all defaults. ``` -The exit code is `1` and no scaffold work happens before the early exit, so the directory stays pristine. The same rule applies to [`create-arkor`](/quickstart#1-scaffold-a-project), with the additional requirement of `[dir]` or `--name ` to make the project name an explicit decision. +The exit code is `1` and no scaffold work happens before the early exit, so the directory stays pristine. The same rule applies to [`create-arkor`](/quickstart#running-under-claude-code-claudecode-1) (Quickstart documents the create-arkor specifics, including the additional `[dir]` / `--name` requirement to make the project name an explicit decision). ## Errors diff --git a/docs/ja/cli/init.mdx b/docs/ja/cli/init.mdx index e4fb2f62..7c73df0a 100644 --- a/docs/ja/cli/init.mdx +++ b/docs/ja/cli/init.mdx @@ -149,7 +149,7 @@ Re-run with explicit flags: Or pass -y/--yes to accept all defaults. ``` -終了コードは `1`、早期終了の前にファイル生成は一切行われないので、ディレクトリは元の状態のままです。同じルールは [`create-arkor`](/ja/quickstart#1-プロジェクトを生成する) にも適用され、加えて `[dir]` か `--name ` も必須となります（プロジェクト名を明示的に決めさせるため）。 +終了コードは `1`、早期終了の前にファイル生成は一切行われないので、ディレクトリは元の状態のままです。同じルールは [`create-arkor`](/ja/quickstart#claude-code-下での実行claudecode1) にも適用されます（Quickstart に create-arkor 固有の詳細を記載しています。プロジェクト名を明示的に決めさせるための `[dir]` / `--name` 必須化も含む）。 ## エラー diff --git a/docs/ja/quickstart.mdx b/docs/ja/quickstart.mdx index ea9a0f48..7eced4d3 100644 --- a/docs/ja/quickstart.mdx +++ b/docs/ja/quickstart.mdx @@ -71,6 +71,38 @@ bun create arkor my-arkor-app --template triage +### Claude Code 下での実行（`CLAUDECODE=1`） + +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code-claudecode-1-厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。デフォルトの `arkor-project` は意図的に選ぶより誤指定であることがほとんどなので、明示的に決めさせる方針です。空白だけ・記号だけのように `sanitise()` がフォールバックしてしまう値も拒否されます。 + +必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトイン）: + +- `[dir]`（例 `my-arkor-app`）または `--name `: 明示的なプロジェクト名。`sanitise()` で `arkor-project` フォールバックに落ちる値は不可。 +- `--template ` +- `--git`（推奨。対話モードのデフォルトと同じ）または `--skip-git` +- パッケージマネージャー系フラグ: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`、または `--skip-install` +- `--agents-md`（特に CLAUDECODE 下では推奨）または `--no-agents-md` + +フラグが不足したときの stderr 例: + +```text +create-arkor: CLAUDECODE=1 detected. Interactive prompts are disabled. +Re-run with explicit flags: + [dir] (e.g. `my-arkor-app`) or --name + Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit. + --template + Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). + --git (recommended) or --skip-git + `--git` runs `git init` and creates an initial commit (matches the interactive default); `--skip-git` leaves git setup to the user. + --use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install) + Which package manager to run `install` with after scaffolding. `--skip-install` leaves the install step to the user. + --agents-md (recommended) or --no-agents-md + `--agents-md` writes `AGENTS.md` + `CLAUDE.md` to brief AI coding agents that arkor post-dates their training data (recommended, especially under CLAUDECODE); `--no-agents-md` skips them. +Or pass -y/--yes to accept all defaults. +``` + +終了コードは `1`、早期終了の前にファイル生成は一切行われないので、親ディレクトリは元の状態のままです。 + ## 2. 生成されたものを眺める ``` diff --git a/docs/quickstart.mdx b/docs/quickstart.mdx index af2ac19e..d72d54a9 100644 --- a/docs/quickstart.mdx +++ b/docs/quickstart.mdx @@ -71,6 +71,38 @@ bun create arkor my-arkor-app --template triage +### Running under Claude Code (`CLAUDECODE=1`) + +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode-1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since the default fallback (`arkor-project`) is almost always an oversight rather than an intentional choice. Empty / whitespace-only / punctuation-only values that `sanitise()` would collapse back to that fallback are also rejected. + +Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): + +- `[dir]` (e.g. `my-arkor-app`) or `--name `: explicit project name; values that sanitise to the generic `arkor-project` fallback do not count. +- `--template ` +- `--git` (recommended; matches the interactive default) or `--skip-git` +- A package-manager flag: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`, or `--skip-install` +- `--agents-md` (recommended, especially under CLAUDECODE) or `--no-agents-md` + +Sample stderr when a flag is missing: + +```text +create-arkor: CLAUDECODE=1 detected. Interactive prompts are disabled. +Re-run with explicit flags: + [dir] (e.g. `my-arkor-app`) or --name + Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit. + --template + Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). + --git (recommended) or --skip-git + `--git` runs `git init` and creates an initial commit (matches the interactive default); `--skip-git` leaves git setup to the user. + --use-pnpm (or --use-npm / --use-yarn / --use-bun, or --skip-install) + Which package manager to run `install` with after scaffolding. `--skip-install` leaves the install step to the user. + --agents-md (recommended) or --no-agents-md + `--agents-md` writes `AGENTS.md` + `CLAUDE.md` to brief AI coding agents that arkor post-dates their training data (recommended, especially under CLAUDECODE); `--no-agents-md` skips them. +Or pass -y/--yes to accept all defaults. +``` + +The exit code is `1` and no scaffold work happens before the early exit, so the parent directory stays pristine. + ## 2. Look at what was generated ``` diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index 9513d4ec..7db85e0b 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -566,6 +566,42 @@ describe("create-arkor (E2E)", () => { ); }); + it.each([ + ["empty string", ""], + ["whitespace only", " "], + ["punctuation only", "!!!"], + ])( + "rejects --name %s because sanitise() would collapse it to the generic arkor-project fallback", + async (_label, name) => { + // ENG-736 PR review (#141): the previous check only looked at + // whether `--name` was defined, so empty strings and inputs that + // sanitise away to nothing both satisfied strict mode and then + // silently became `package.json: { name: "arkor-project" }`, + // exactly the silent-default outcome strict mode is meant to + // surface. + const result = await runCli( + CREATE_ARKOR_BIN, + [ + "--name", + name, + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + parentDir, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(1); + expect(result.stderr).toContain("[dir]"); + // Even though `--name ` was passed, the requirement + // is reported as missing because the value would have collapsed + // to the fallback. The description hints at that subtlety. + expect(result.stderr).toContain("arkor-project"); + }, + ); + it("runs to completion when every required flag is set", async () => { const { result, targetDir } = await runCreateArkor( [ diff --git a/packages/arkor/src/bin.ts b/packages/arkor/src/bin.ts index cb44b3ec..10be138c 100644 --- a/packages/arkor/src/bin.ts +++ b/packages/arkor/src/bin.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node import process from "node:process"; +import { ClaudeCodeStrictExit } from "@arkor/cli-internal"; import { main } from "./cli/main"; // Catch top-level await rejections explicitly instead of letting Node's @@ -11,6 +12,15 @@ import { main } from "./cli/main"; try { await main(process.argv.slice(2)); } catch (err) { - console.error(err instanceof Error ? (err.stack ?? err.message) : String(err)); - process.exitCode = 1; + // `ClaudeCodeStrictExit` is thrown by the strict-mode validator after + // it has already written the missing-flags block to stderr. Throwing + // (rather than `process.exit(1)`-ing inside the action) lets main's + // `finally` run telemetry shutdown + any deprecation notice; here we + // just set the exit code without re-printing the message or its stack. + if (err instanceof ClaudeCodeStrictExit) { + process.exitCode = 1; + } else { + console.error(err instanceof Error ? (err.stack ?? err.message) : String(err)); + process.exitCode = 1; + } } diff --git a/packages/arkor/src/cli/commands/init.test.ts b/packages/arkor/src/cli/commands/init.test.ts index 179528b1..8f5f6302 100644 --- a/packages/arkor/src/cli/commands/init.test.ts +++ b/packages/arkor/src/cli/commands/init.test.ts @@ -26,6 +26,7 @@ vi.mock("@arkor/cli-internal", () => { return { gitInitialCommit: vi.fn(async () => ({ signingFallback: false })), install: vi.fn(async () => undefined), + isClaudeCode: () => process.env.CLAUDECODE === "1", isInGitRepo: vi.fn(async () => false), sanitise: (s: string) => s @@ -71,7 +72,7 @@ const ORIG_CI = process.env.CI; const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_TTY = process.stdout.isTTY; -beforeEach(() => { +beforeEach(async () => { // macOS resolves `/tmp/...` through realpath to `/private/tmp/...`, so // a chdir into the raw `mkdtemp` result leaves `process.cwd()` (the // value runInit reads internally) and our captured `cwd` mismatched. @@ -92,6 +93,14 @@ beforeEach(() => { vi.mocked(isInGitRepo).mockResolvedValue(false); vi.mocked(gitInitialCommit).mockResolvedValue({ signingFallback: false }); vi.mocked(install).mockResolvedValue(undefined); + // Reset call counts on the clack prompt mocks so per-test + // assertions like `expect(clack.text).not.toHaveBeenCalled()` aren't + // tripped by earlier tests' invocations accumulating in the shared + // mock state. + const clack = await import("@clack/prompts"); + vi.mocked(clack.text).mockClear(); + vi.mocked(clack.select).mockClear(); + vi.mocked(clack.confirm).mockClear(); }); afterEach(() => { @@ -450,6 +459,41 @@ describe("runInit", () => { expect(installOrder).toBeLessThan(commitOrder!); }); + it("under CLAUDECODE=1 + TTY (no --yes), skips clack prompts via skipWith instead of opening them", async () => { + // ENG-736 PR review (#141): the original implementation forced + // `isInteractive()` to false under CLAUDECODE, which broke other + // commands (e.g. `arkor logout` would silently delete credentials). + // The CLAUDECODE awareness now lives inside `runInit` itself, + // applied per-call as `skipWith` so promptText/promptSelect resolve + // without opening a real clack prompt. Mirror a Claude Code session + // that managed to acquire a TTY (theoretical but the safe path): + // clack.text / clack.select / clack.confirm must NOT be invoked. + delete process.env.CI; + Object.defineProperty(process.stdout, "isTTY", { + value: true, + configurable: true, + }); + process.env.CLAUDECODE = "1"; + const clack = await import("@clack/prompts"); + + await runInit({ + // No --yes: the strict-mode validator in main.ts is what enforces + // explicit flags; once we reach runInit, CLAUDECODE alone must be + // enough to bypass the prompts. + name: "my-app", + template: "triage", + packageManager: "pnpm", + skipInstall: true, + skipGit: true, + }); + expect(clack.text).not.toHaveBeenCalled(); + expect(clack.select).not.toHaveBeenCalled(); + expect(clack.confirm).not.toHaveBeenCalled(); + expect(scaffold).toHaveBeenCalledWith( + expect.objectContaining({ name: "my-app", template: "triage" }), + ); + }); + it("interactive: prompts for git init before install so the user can walk away", async () => { // The whole point of ENG-625's swap: surface the git-init confirm before // the multi-minute ` install` so the user isn't blocked at an diff --git a/packages/arkor/src/cli/commands/init.ts b/packages/arkor/src/cli/commands/init.ts index d292e2ff..7a75a95d 100644 --- a/packages/arkor/src/cli/commands/init.ts +++ b/packages/arkor/src/cli/commands/init.ts @@ -2,6 +2,7 @@ import { basename } from "node:path"; import { gitInitialCommit, install, + isClaudeCode, isInGitRepo, sanitise, scaffold, @@ -128,17 +129,24 @@ export async function runInit(options: InitOptions): Promise { } ui.intro("arkor init"); + // Under CLAUDECODE=1 the action handler in `main.ts` has already gated + // strict mode: if we reach `runInit`, either every flag is present or + // `--yes` was passed. Force `skipWith` so the helpers never try to open + // a clack prompt under a Claude Code TTY (which can't answer); the + // other commands keep their pre-existing interactive semantics because + // `isInteractive()` is no longer overridden globally. + const bypassPrompts = options.yes || isClaudeCode(); const projectName = await promptText({ message: "Project name?", initialValue: options.name ?? defaultName, - skipWith: options.yes ? options.name ?? defaultName : undefined, + skipWith: bypassPrompts ? options.name ?? defaultName : undefined, }); // An explicit `--template ` is authoritative: skip the prompt and use it as-is. const template = await promptSelect({ message: "Starter template?", initialValue: options.template ?? "triage", options: templateChoices(), - skipWith: options.template ?? (options.yes ? "triage" : undefined), + skipWith: options.template ?? (bypassPrompts ? "triage" : undefined), }); // Sanitise here so `--name "Foo Bar"` (which bypasses prompts under diff --git a/packages/arkor/src/cli/main.test.ts b/packages/arkor/src/cli/main.test.ts index dea73cc6..5db0ee3b 100644 --- a/packages/arkor/src/cli/main.test.ts +++ b/packages/arkor/src/cli/main.test.ts @@ -218,6 +218,59 @@ describe("main (CLI Commander wiring)", () => { expect(shutdownTelemetry).toHaveBeenCalledOnce(); }); + it("under CLAUDECODE=1 + missing flags, throws ClaudeCodeStrictExit so the finally block still runs (shutdownTelemetry fires, no project name skip)", async () => { + // ENG-736 PR review (#141): the validator used to `process.exit(1)` + // inside the action handler, which bypassed main()'s finally and + // lost telemetry / deprecation flushing. The validator now throws + // a sentinel so the finally still runs; bin.ts is the layer that + // recognises the sentinel and sets exitCode without re-printing. + process.env.CLAUDECODE = "1"; + const stderrChunks: string[] = []; + const spy = vi + .spyOn(process.stderr, "write") + .mockImplementation(((c: unknown) => { + stderrChunks.push(String(c)); + return true; + }) as typeof process.stderr.write); + try { + await expect( + main(["init", "--template", "triage", "--skip-git"]), + // Missing: pm flag + agents-md flag. + ).rejects.toMatchObject({ name: "ClaudeCodeStrictExit" }); + } finally { + spy.mockRestore(); + delete process.env.CLAUDECODE; + } + expect(shutdownTelemetry).toHaveBeenCalledOnce(); + const buf = stderrChunks.join(""); + expect(buf).toContain("arkor init: CLAUDECODE=1 detected"); + expect(buf).toContain("--use-pnpm"); + expect(buf).toContain("--agents-md (recommended) or --no-agents-md"); + expect(runInit).not.toHaveBeenCalled(); + }); + + it("under CLAUDECODE=1 with every flag set (no --yes), passes the strict check and delegates to runInit", async () => { + // The happy-path mirror of the test above: a fully-specified + // invocation still runs to completion under CLAUDECODE, since the + // validator is gated on missing flags only. + process.env.CLAUDECODE = "1"; + try { + await main([ + "init", + "--name", + "my-app", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ]); + } finally { + delete process.env.CLAUDECODE; + } + expect(runInit).toHaveBeenCalledOnce(); + }); + it("omits the Cutoff suffix when the deprecation has no sunset value", async () => { // Branch coverage for the `notice.sunset ? ` Cutoff: …` : ""` ternary. mockDeprecation.value = { diff --git a/packages/arkor/src/cli/main.ts b/packages/arkor/src/cli/main.ts index 553b32e5..21c10d86 100644 --- a/packages/arkor/src/cli/main.ts +++ b/packages/arkor/src/cli/main.ts @@ -7,6 +7,7 @@ import { runDev } from "./commands/dev"; import { runBuild } from "./commands/build"; import { runStart } from "./commands/start"; import { + ClaudeCodeStrictExit, formatClaudeCodeMissingMessage, isClaudeCode, missingClaudeCodeFlags, @@ -117,7 +118,11 @@ export async function main(argv: string[]): Promise { process.stderr.write( formatClaudeCodeMissingMessage("arkor init", missing), ); - process.exit(1); + // Throw (don't `process.exit`) so the `finally` at the bottom of + // main() still runs telemetry shutdown / the deprecation notice. + // bin.ts recognises this sentinel and exits without re-printing + // the message. + throw new ClaudeCodeStrictExit(); } } const packageManager = resolvePackageManager({ diff --git a/packages/arkor/src/cli/prompts.test.ts b/packages/arkor/src/cli/prompts.test.ts index a86f1605..82569341 100644 --- a/packages/arkor/src/cli/prompts.test.ts +++ b/packages/arkor/src/cli/prompts.test.ts @@ -85,28 +85,20 @@ describe("isInteractive", () => { expect(isInteractive()).toBe(true); }); - it("returns false when CLAUDECODE === '1' even if stdout is a TTY", () => { - // Claude Code spawns the CLI with CLAUDECODE=1 and cannot answer - // interactive prompts; falling through to clack would hang forever. + it("does NOT special-case CLAUDECODE (only init / create-arkor opt in to that branch via their own check, so the shared helper stays neutral for other commands)", () => { + // ENG-736 PR review (#141): forcing this helper to false under + // CLAUDECODE=1 leaked into commands that don't have strict-mode + // validation (e.g. `arkor logout` would silently delete credentials + // because its `promptConfirm` would fall through to + // `initialValue: true`). The CLAUDECODE awareness now lives in + // `runInit` / create-arkor's `run()` instead, gated to the + // already-strict scaffold commands. delete process.env.CI; Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true, }); process.env.CLAUDECODE = "1"; - expect(isInteractive()).toBe(false); - }); - - it("ignores CLAUDECODE values other than the literal '1'", () => { - // Exact-match contract: any other value (legacy CI envs that - // happen to set CLAUDECODE for unrelated reasons) must not flip - // the CLI into the strict mode. - delete process.env.CI; - Object.defineProperty(process.stdout, "isTTY", { - value: true, - configurable: true, - }); - process.env.CLAUDECODE = "true"; expect(isInteractive()).toBe(true); }); }); diff --git a/packages/arkor/src/cli/prompts.ts b/packages/arkor/src/cli/prompts.ts index 14d0bffd..14848187 100644 --- a/packages/arkor/src/cli/prompts.ts +++ b/packages/arkor/src/cli/prompts.ts @@ -13,11 +13,7 @@ import * as clack from "@clack/prompts"; */ export function isInteractive(): boolean { - return ( - Boolean(process.stdout.isTTY) && - !process.env.CI && - process.env.CLAUDECODE !== "1" - ); + return Boolean(process.stdout.isTTY) && !process.env.CI; } class CliCancelled extends Error { diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index dc1cac79..f54cc173 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -87,6 +87,62 @@ describe("missingClaudeCodeFlags", () => { expect(missing).toEqual([]); }); + it("rejects --name values that sanitise() collapses to the `arkor-project` fallback", () => { + // The strict-mode contract is that the agent commits to a real + // project name. `--name ""` / `--name "!!!"` / `--name " "` all + // pass `sanitise()` through the empty-string fallback path and end + // up as `arkor-project` in `package.json`, which is exactly the + // silent default we want to surface. + for (const name of ["", " ", "!!!", "***", "@@@"]) { + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + name, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain( + "[dir] (e.g. `my-arkor-app`) or --name ", + ); + } + }); + + it("rejects [dir] basenames that sanitise() collapses to the fallback", () => { + // Same trap via the positional: `create-arkor " "` would otherwise + // count as a satisfied project-name requirement even though it + // also lands on `arkor-project`. + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + dir: "!!!", + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain( + "[dir] (e.g. `my-arkor-app`) or --name ", + ); + }); + + it("still accepts the literal `arkor-project` when the caller asked for it explicitly", () => { + // Carve-out so a user who genuinely wants the name `arkor-project` + // isn't blocked by the fallback-detection heuristic. Case- and + // whitespace-insensitive on the input side; the sanitised slug is + // already canonical. + for (const name of ["arkor-project", "ARKOR-PROJECT", " arkor-project "]) { + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + name, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + } + }); + it("counts --skip-install as satisfying the package-manager requirement", () => { const missing = missingClaudeCodeFlags({ template: "triage", diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index 01e22254..cfb168c3 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -1,3 +1,6 @@ +import { basename } from "node:path"; +import { sanitise } from "./sanitise"; + /** * Claude Code (the Anthropic agent CLI) sets `CLAUDECODE=1` in every spawned * shell. It cannot answer interactive prompts, and silently falling through to @@ -14,6 +17,22 @@ export function isClaudeCode(): boolean { return process.env.CLAUDECODE === "1"; } +/** + * Thrown by the CLI strict-mode validator after it has already written + * the human-readable missing-flags block to stderr. The outer bin.ts + * `catch` recognises this class and exits `1` *without* re-printing the + * message or its stack; the throw still unwinds through + * `program.parseAsync()` so the `finally` in `main()` runs (telemetry + * flush, deprecation notice, etc.), which a bare `process.exit(1)` + * inside the action handler would have skipped. + */ +export class ClaudeCodeStrictExit extends Error { + constructor() { + super("CLAUDECODE strict-mode early exit"); + this.name = "ClaudeCodeStrictExit"; + } +} + export interface ClaudeCodeOptionsCheck { /** True when `--yes`/`-y` was passed; bypasses the missing-flag check. */ yes?: boolean; @@ -60,6 +79,24 @@ export interface MissingClaudeCodeFlag { description: string; } +/** + * True when `opts` supplies a project name that survives `sanitise()` + * without falling back to the generic `arkor-project` default. Empty + * strings, whitespace-only inputs, and strings that contain no + * `[a-z0-9-]` characters all collapse to `arkor-project` inside + * `sanitise()`, which is exactly the silent-default outcome strict mode + * is trying to prevent. The literal `arkor-project` (case-insensitive + * after trimming) is still accepted because the caller has explicitly + * asked for that name. + */ +function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { + const raw = opts.name ?? (opts.dir !== undefined ? basename(opts.dir) : undefined); + if (raw === undefined) return false; + const sanitised = sanitise(raw); + if (sanitised !== "arkor-project") return true; + return raw.trim().toLowerCase() === "arkor-project"; +} + /** * Returns the list of missing flags under CLAUDECODE=1, each paired with a * short description of what it controls, so the stderr message is @@ -72,15 +109,11 @@ export function missingClaudeCodeFlags( ): MissingClaudeCodeFlag[] { if (opts.yes) return []; const missing: MissingClaudeCodeFlag[] = []; - if ( - opts.requireProjectName && - opts.dir === undefined && - opts.name === undefined - ) { + if (opts.requireProjectName && !hasMeaningfulProjectName(opts)) { missing.push({ flag: "[dir] (e.g. `my-arkor-app`) or --name ", description: - "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used.", + "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit.", }); } if (opts.template === undefined) { diff --git a/packages/cli-internal/src/index.ts b/packages/cli-internal/src/index.ts index 8e31b8b7..8185cce7 100644 --- a/packages/cli-internal/src/index.ts +++ b/packages/cli-internal/src/index.ts @@ -25,6 +25,7 @@ export { } from "./git"; export { sanitise } from "./sanitise"; export { + ClaudeCodeStrictExit, isClaudeCode, missingClaudeCodeFlags, formatClaudeCodeMissingMessage, diff --git a/packages/create-arkor/src/bin.ts b/packages/create-arkor/src/bin.ts index f0a7f34e..5bbd9965 100644 --- a/packages/create-arkor/src/bin.ts +++ b/packages/create-arkor/src/bin.ts @@ -6,6 +6,7 @@ import process from "node:process"; import * as clack from "@clack/prompts"; import { Command } from "commander"; import { + ClaudeCodeStrictExit, formatClaudeCodeMissingMessage, gitInitialCommit, install, @@ -44,11 +45,7 @@ const MANUAL_INSTALL_HINT = "install dependencies (npm i / pnpm install / yarn / bun install)"; function isInteractive(): boolean { - return ( - Boolean(process.stdout.isTTY) && - !process.env.CI && - process.env.CLAUDECODE !== "1" - ); + return Boolean(process.stdout.isTTY) && !process.env.CI; } /** @@ -157,7 +154,13 @@ async function run(options: RunOptions): Promise { let name = sanitise(options.name ?? defaultName); let template: TemplateId = options.template ?? "triage"; - if (!options.yes && isInteractive()) { + // Under CLAUDECODE=1 the action handler below has already gated strict + // mode: by the time we get here either every flag is present or `--yes` + // was passed. Treat CLAUDECODE as an implicit "skip prompts" so a Claude + // Code TTY (which can't answer clack) never opens a real prompt; other + // CLIs keep their pre-existing interactive semantics because + // `isInteractive()` is no longer overridden globally. + if (!options.yes && !isClaudeCode() && isInteractive()) { // Re-prompt loop: when the project name is auto-derived into a fresh // `.//` subdirectory, refuse to merge into an existing non-empty // directory (likely a typo or a forgotten earlier scaffold). Explicit @@ -404,7 +407,11 @@ program process.stderr.write( formatClaudeCodeMissingMessage("create-arkor", missing), ); - process.exit(1); + // Throw (don't `process.exit`) so the outer `program.parseAsync` + // catch block recognises this sentinel and exits silently. The + // "create-arkor failed:" prefix it normally adds would double up + // on our already-printed missing-flags block. + throw new ClaudeCodeStrictExit(); } } // Use `Object.hasOwn` (not `in`) so prototype keys like `toString` / @@ -444,6 +451,12 @@ program ); program.parseAsync(process.argv).catch((err) => { + // The strict-mode validator throws this sentinel after writing the + // missing-flags block; exit silently so the prefix below doesn't + // double up on the already-printed message. + if (err instanceof ClaudeCodeStrictExit) { + process.exit(1); + } process.stderr.write( `create-arkor failed: ${err instanceof Error ? err.message : String(err)}\n`, ); From 0523aa676d2edf0b8c0ea3163c157915456b0500 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Thu, 14 May 2026 18:27:51 +0900 Subject: [PATCH 03/16] feat: enhance strict mode handling in create-arkor and related commands --- e2e/cli/src/create-arkor.test.ts | 27 ++++++++++++ packages/arkor/src/cli/commands/init.test.ts | 8 ++-- packages/arkor/src/cli/commands/login.test.ts | 9 ++-- .../arkor/src/cli/commands/logout.test.ts | 10 +++-- packages/arkor/src/cli/main.test.ts | 42 ++++++++++++------- packages/cli-internal/src/claude-code.test.ts | 27 ++++++++++++ packages/cli-internal/src/claude-code.ts | 12 +++++- packages/create-arkor/src/bin.ts | 12 +++++- 8 files changed, 117 insertions(+), 30 deletions(-) diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index 7db85e0b..73e72f2c 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -602,6 +602,33 @@ describe("create-arkor (E2E)", () => { }, ); + it("accepts `create-arkor .` (resolves to the parent dir basename, not the literal `.`)", async () => { + // Regression for PR #141 review (codex + Copilot): the strict + // check used to compute the project name as `basename(opts.dir)` + // which is `"."` for `create-arkor .`, then sanitise() collapsed + // it to the `arkor-project` fallback and strict mode falsely + // refused the run. The check now mirrors `create-arkor`'s own + // default-name derivation (`basename(resolve(opts.dir))`), so + // `.` resolves to the parent dir's basename: a meaningful name. + const result = await runCli( + CREATE_ARKOR_BIN, + [ + ".", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + parentDir, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(0); + // Sanity: the run reached scaffold (package.json is present in + // the parent dir because `.` was the target). + expect(existsSync(join(parentDir, "package.json"))).toBe(true); + }); + it("runs to completion when every required flag is set", async () => { const { result, targetDir } = await runCreateArkor( [ diff --git a/packages/arkor/src/cli/commands/init.test.ts b/packages/arkor/src/cli/commands/init.test.ts index 8f5f6302..ef9a3ee5 100644 --- a/packages/arkor/src/cli/commands/init.test.ts +++ b/packages/arkor/src/cli/commands/init.test.ts @@ -65,9 +65,11 @@ const ORIG_CWD = process.cwd(); // conditionally. CI runners typically have CI already set, and an // unconditional `delete` would leak a different environment to later // test files when vitest reuses a worker. CLAUDECODE is captured -// because vitest workers spawned from Claude Code inherit it (=`1`), -// which would otherwise force isInteractive() to false and break the -// "interactive" branch tests. +// because the new strict-mode tests below toggle it (and the helper +// `isClaudeCode()` reads it directly inside `runInit` for the +// prompt-bypass branch), so it must be restored to whatever the +// outer environment supplied (vitest workers spawned from a Claude +// Code session inherit `CLAUDECODE=1` in the parent). const ORIG_CI = process.env.CI; const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_TTY = process.stdout.isTTY; diff --git a/packages/arkor/src/cli/commands/login.test.ts b/packages/arkor/src/cli/commands/login.test.ts index f61e3501..5af3092f 100644 --- a/packages/arkor/src/cli/commands/login.test.ts +++ b/packages/arkor/src/cli/commands/login.test.ts @@ -23,9 +23,12 @@ const ORIG_HOME = process.env.HOME; // tests via `~/.arkor/credentials.json`. const ORIG_USERPROFILE = process.env.USERPROFILE; const ORIG_CI = process.env.CI; -// CLAUDECODE is captured because vitest workers spawned from Claude Code -// inherit `CLAUDECODE=1`, which forces isInteractive() to false and -// breaks the "interactive" branch tests below that flip CI off + isTTY on. +// `arkor login` itself doesn't read CLAUDECODE (since `isInteractive()` +// was reverted to its pre-PR shape), but vitest workers spawned from a +// Claude Code session inherit `CLAUDECODE=1`. Strip it in beforeEach +// (and restore from ORIG_CLAUDECODE in afterEach) so any future +// strict-mode wiring added to login doesn't surprise these tests, and +// so the test environment matches what CI runners see. const ORIG_CLAUDECODE = process.env.CLAUDECODE; const ORIG_FETCH = globalThis.fetch; const ORIG_URL = process.env.ARKOR_CLOUD_API_URL; diff --git a/packages/arkor/src/cli/commands/logout.test.ts b/packages/arkor/src/cli/commands/logout.test.ts index e78f9260..e0c5649a 100644 --- a/packages/arkor/src/cli/commands/logout.test.ts +++ b/packages/arkor/src/cli/commands/logout.test.ts @@ -32,10 +32,12 @@ const ORIG_USERPROFILE = process.env.USERPROFILE; const ORIG_HOMEDRIVE = process.env.HOMEDRIVE; const ORIG_HOMEPATH = process.env.HOMEPATH; const ORIG_CI = process.env.CI; -// CLAUDECODE is captured because vitest workers spawned from Claude Code -// inherit `CLAUDECODE=1`, which forces isInteractive() to false and -// breaks the interactive-branch test below. Strip it in beforeEach and -// restore in afterEach. +// `arkor logout` itself doesn't read CLAUDECODE (since `isInteractive()` +// was reverted to its pre-PR shape), but vitest workers spawned from a +// Claude Code session inherit `CLAUDECODE=1`. Strip it in beforeEach +// (and restore from ORIG_CLAUDECODE in afterEach) so any future +// strict-mode wiring added to logout doesn't surprise these tests, and +// so the test environment matches what CI runners see. const ORIG_CLAUDECODE = process.env.CLAUDECODE; // Capture the original `process.stdout.isTTY` so the interactive test // below can flip it without leaking into other test files when vitest diff --git a/packages/arkor/src/cli/main.test.ts b/packages/arkor/src/cli/main.test.ts index 5db0ee3b..452ce725 100644 --- a/packages/arkor/src/cli/main.test.ts +++ b/packages/arkor/src/cli/main.test.ts @@ -43,10 +43,15 @@ import { runWhoami } from "./commands/whoami"; import { shutdownTelemetry } from "../core/telemetry"; import { main } from "./main"; -// Capture once so the after-each can restore — without this, tests that +// Capture once so the after-each can restore. Without this, tests that // set `npm_config_user_agent` to drive package-manager detection leak the // override into later test files when vitest reuses a worker process. const ORIG_USER_AGENT = process.env.npm_config_user_agent; +// Same rationale for CLAUDECODE: the new strict-mode tests below set it +// to "1" and the worker may already have it set (vitest spawned from a +// Claude Code session), so afterEach restores rather than unconditionally +// deletes. +const ORIG_CLAUDECODE = process.env.CLAUDECODE; beforeEach(() => { vi.mocked(runInit).mockReset(); @@ -67,6 +72,11 @@ afterEach(() => { } else { delete process.env.npm_config_user_agent; } + if (ORIG_CLAUDECODE !== undefined) { + process.env.CLAUDECODE = ORIG_CLAUDECODE; + } else { + delete process.env.CLAUDECODE; + } vi.restoreAllMocks(); }); @@ -224,6 +234,11 @@ describe("main (CLI Commander wiring)", () => { // lost telemetry / deprecation flushing. The validator now throws // a sentinel so the finally still runs; bin.ts is the layer that // recognises the sentinel and sets exitCode without re-printing. + // afterEach restores CLAUDECODE from ORIG_CLAUDECODE, so the + // explicit `delete` we used to do in the test's own `finally` + // would have leaked into later tests when vitest was launched + // from a Claude Code session (CLAUDECODE already set in the + // parent worker). process.env.CLAUDECODE = "1"; const stderrChunks: string[] = []; const spy = vi @@ -239,7 +254,6 @@ describe("main (CLI Commander wiring)", () => { ).rejects.toMatchObject({ name: "ClaudeCodeStrictExit" }); } finally { spy.mockRestore(); - delete process.env.CLAUDECODE; } expect(shutdownTelemetry).toHaveBeenCalledOnce(); const buf = stderrChunks.join(""); @@ -254,20 +268,16 @@ describe("main (CLI Commander wiring)", () => { // invocation still runs to completion under CLAUDECODE, since the // validator is gated on missing flags only. process.env.CLAUDECODE = "1"; - try { - await main([ - "init", - "--name", - "my-app", - "--template", - "triage", - "--skip-git", - "--skip-install", - "--no-agents-md", - ]); - } finally { - delete process.env.CLAUDECODE; - } + await main([ + "init", + "--name", + "my-app", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ]); expect(runInit).toHaveBeenCalledOnce(); }); diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index f54cc173..85246cc6 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -125,6 +125,33 @@ describe("missingClaudeCodeFlags", () => { ); }); + it("resolves [dir] before taking the basename so `.` / `..` are treated like the surrounding directory", () => { + // Regression for PR #141 review (codex + Copilot): `basename(".")` + // is `"."` which sanitise() collapses to the fallback, so the + // previous implementation falsely rejected `create-arkor . + // --template ...`. Mirroring `basename(resolve(opts.dir))` (what + // the scaffolder itself uses to derive `defaultName`) makes the + // strict check agree with the runtime: `.` / `..` resolve to the + // current / parent directory's basename, and only then are + // sanitised. + const baseOpts = { + requireProjectName: true, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + } as const; + // `process.cwd()` is the vitest worker's cwd; its basename is + // some non-empty slug (e.g. `cli-internal` here). The exact value + // doesn't matter as long as it isn't the `arkor-project` fallback. + expect(missingClaudeCodeFlags({ ...baseOpts, dir: "." })).toEqual([]); + expect(missingClaudeCodeFlags({ ...baseOpts, dir: "./" })).toEqual([]); + // Relative path that resolves to a meaningful basename also passes. + expect( + missingClaudeCodeFlags({ ...baseOpts, dir: "foo/bar/my-app" }), + ).toEqual([]); + }); + it("still accepts the literal `arkor-project` when the caller asked for it explicitly", () => { // Carve-out so a user who genuinely wants the name `arkor-project` // isn't blocked by the fallback-detection heuristic. Case- and diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index cfb168c3..c05c5ecb 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -1,4 +1,4 @@ -import { basename } from "node:path"; +import { basename, resolve } from "node:path"; import { sanitise } from "./sanitise"; /** @@ -88,9 +88,17 @@ export interface MissingClaudeCodeFlag { * is trying to prevent. The literal `arkor-project` (case-insensitive * after trimming) is still accepted because the caller has explicitly * asked for that name. + * + * `[dir]` is derived through `basename(resolve(opts.dir))` to mirror + * `create-arkor`'s own default-name derivation. Without `resolve()`, + * `basename(".")` returns `"."` (which sanitises to the fallback) and + * the validator would falsely reject explicit invocations like + * `create-arkor . --template ...` even though the scaffolder would + * pick up the current directory's name at runtime. */ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { - const raw = opts.name ?? (opts.dir !== undefined ? basename(opts.dir) : undefined); + const raw = + opts.name ?? (opts.dir !== undefined ? basename(resolve(opts.dir)) : undefined); if (raw === undefined) return false; const sanitised = sanitise(raw); if (sanitised !== "arkor-project") return true; diff --git a/packages/create-arkor/src/bin.ts b/packages/create-arkor/src/bin.ts index 5bbd9965..85b92e11 100644 --- a/packages/create-arkor/src/bin.ts +++ b/packages/create-arkor/src/bin.ts @@ -454,11 +454,19 @@ program.parseAsync(process.argv).catch((err) => { // The strict-mode validator throws this sentinel after writing the // missing-flags block; exit silently so the prefix below doesn't // double up on the already-printed message. + // + // Both branches set `process.exitCode` (rather than calling + // `process.exit(1)`) so Node lets the event loop drain naturally + // before exiting. On piped stdio, `process.exit()` is synchronous + // and can truncate queued writes; the strict-mode message is a + // multi-line block that agents need to read to re-invoke, so + // dropping its tail would defeat the whole point. if (err instanceof ClaudeCodeStrictExit) { - process.exit(1); + process.exitCode = 1; + return; } process.stderr.write( `create-arkor failed: ${err instanceof Error ? err.message : String(err)}\n`, ); - process.exit(1); + process.exitCode = 1; }); From 57fe6a00a76d3e90ec1d92f2a55da025c4759878 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Thu, 14 May 2026 23:51:16 +0900 Subject: [PATCH 04/16] test: enhance assertions for strict mode in create-arkor E2E tests --- e2e/cli/src/create-arkor.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index 73e72f2c..647f9176 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -546,8 +546,17 @@ describe("create-arkor (E2E)", () => { expect(result.stderr).toContain("git init"); expect(result.stderr).toContain("package manager"); expect(result.stderr).toContain("AGENTS.md"); - // Sanity: exit happened before any scaffold work. + // Sanity: exit happened before any scaffold work. Without [dir], + // a non-strict create-arkor run would have created + // `./arkor-project/` (its auto-derived default subdirectory) and + // a `package.json` inside it — not in parentDir itself — so + // asserting `parentDir/package.json` alone wouldn't catch a + // regression where strict mode failed and the scaffolder still + // ran. Assert that `parentDir` is byte-for-byte unchanged + // instead (it was empty going in via `makeTempDir`). expect(existsSync(join(parentDir, "package.json"))).toBe(false); + expect(existsSync(join(parentDir, "arkor-project"))).toBe(false); + expect(readdirSync(parentDir)).toEqual([]); }); it("still exits 1 when [dir] is given but other prompts are missing", async () => { From 4a0bb466397db5552efd02db42531b556ed27342 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 00:03:24 +0900 Subject: [PATCH 05/16] feat: improve sanitise function to handle non-alphanumeric runs and optimize regex for ReDoS safety --- packages/cli-internal/src/sanitise.test.ts | 21 ++++++++++++++++++++- packages/cli-internal/src/sanitise.ts | 21 +++++++++++++++------ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/cli-internal/src/sanitise.test.ts b/packages/cli-internal/src/sanitise.test.ts index 8798d8ed..7ca3c430 100644 --- a/packages/cli-internal/src/sanitise.test.ts +++ b/packages/cli-internal/src/sanitise.test.ts @@ -6,10 +6,17 @@ describe("sanitise", () => { expect(sanitise("MyApp")).toBe("myapp"); }); - it("replaces non `[a-z0-9-]` characters with dashes", () => { + it("replaces non `[a-z0-9]` runs with a single dash", () => { expect(sanitise("hello world")).toBe("hello-world"); expect(sanitise("Hello! World")).toBe("hello-world"); expect(sanitise("foo_bar.baz")).toBe("foo-bar-baz"); + // Adjacent runs of non-alphanumerics (including mixed `-` and + // other punctuation) collapse to one dash in a single pass. + // This matters for the ReDoS-safety contract documented in + // `sanitise.ts`: the chain no longer relies on a separate `-+` + // collapse step that walked the same characters twice. + expect(sanitise("a---b")).toBe("a-b"); + expect(sanitise("a-_-b")).toBe("a-b"); }); it("collapses leading and trailing dashes after substitution", () => { @@ -17,6 +24,18 @@ describe("sanitise", () => { expect(sanitise("---abc---")).toBe("abc"); }); + it("terminates in linear time on adversarial dash-heavy inputs (ReDoS regression)", () => { + // CodeQL flagged the previous regex chain as a polynomial-ReDoS + // hotspot once `sanitise()` started receiving CLI-arg input from + // the CLAUDECODE strict-mode check. The current chain consumes + // each character at most a constant number of times, so even a + // pathological input completes in well under the test's timeout. + const adversarial = "-".repeat(100_000); + const start = Date.now(); + expect(sanitise(adversarial)).toBe("arkor-project"); + expect(Date.now() - start).toBeLessThan(1_000); + }); + it("falls back to `arkor-project` when the result would be empty", () => { expect(sanitise("")).toBe("arkor-project"); expect(sanitise("!!!")).toBe("arkor-project"); diff --git a/packages/cli-internal/src/sanitise.ts b/packages/cli-internal/src/sanitise.ts index 93a2c4a1..1d19f764 100644 --- a/packages/cli-internal/src/sanitise.ts +++ b/packages/cli-internal/src/sanitise.ts @@ -1,19 +1,28 @@ /** * Normalise a candidate project name into a safe lowercase slug: * - lowercased - * - non `[a-z0-9-]` characters replaced with `-` - * - consecutive dashes collapsed to a single `-` - * - leading / trailing `-` trimmed + * - non-alphanumeric runs (including existing `-`) collapsed to a single `-` + * - a single leading / trailing `-` trimmed * - capped at 60 characters * - falls back to `"arkor-project"` if the result is empty + * + * The regexes are deliberately written so each input character is + * consumed at most a constant number of times, avoiding the + * polynomial-backtracking shape CodeQL flagged when the previous + * chain combined `/[^a-z0-9-]/g` (which kept `-` as a literal) with a + * separate `/-+/g` collapse: an adversarial input of many `-` runs + * walked the same characters across two `+`-quantified passes. The + * new first regex includes `-` in the negated class so a single linear + * scan does both substitution and run-collapsing; the trim step then + * only ever has to strip a single dash at each anchor because runs + * have already been collapsed. */ export function sanitise(name: string): string { return ( name .toLowerCase() - .replace(/[^a-z0-9-]/g, "-") - .replace(/-+/g, "-") - .replace(/^-+|-+$/g, "") + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-|-$/g, "") .slice(0, 60) || "arkor-project" ); } From 42895402a39f6dc09db3bfe6b1e5f376c216ad38 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 01:31:26 +0900 Subject: [PATCH 06/16] fix: update documentation and tests for strict mode handling in arkor init and create-arkor --- docs/cli/init.mdx | 2 +- docs/ja/cli/init.mdx | 2 +- docs/ja/quickstart.mdx | 2 +- docs/quickstart.mdx | 2 +- packages/cli-internal/src/claude-code.test.ts | 23 ++++++++++++----- packages/cli-internal/src/claude-code.ts | 25 +++++++++---------- packages/cli-internal/src/sanitise.test.ts | 13 ++++++++++ packages/cli-internal/src/sanitise.ts | 15 ++++++++--- 8 files changed, 57 insertions(+), 27 deletions(-) diff --git a/docs/cli/init.mdx b/docs/cli/init.mdx index 7f18cf52..73080fe0 100644 --- a/docs/cli/init.mdx +++ b/docs/cli/init.mdx @@ -149,7 +149,7 @@ Re-run with explicit flags: Or pass -y/--yes to accept all defaults. ``` -The exit code is `1` and no scaffold work happens before the early exit, so the directory stays pristine. The same rule applies to [`create-arkor`](/quickstart#running-under-claude-code-claudecode-1) (Quickstart documents the create-arkor specifics, including the additional `[dir]` / `--name` requirement to make the project name an explicit decision). +The exit code is `1` and no scaffold work happens before the early exit, so the directory stays pristine. The same rule applies to [`create-arkor`](/quickstart#running-under-claude-code-claudecode=1) (Quickstart documents the create-arkor specifics, including the additional `[dir]` / `--name` requirement to make the project name an explicit decision). ## Errors diff --git a/docs/ja/cli/init.mdx b/docs/ja/cli/init.mdx index 7c73df0a..eadf8ff4 100644 --- a/docs/ja/cli/init.mdx +++ b/docs/ja/cli/init.mdx @@ -149,7 +149,7 @@ Re-run with explicit flags: Or pass -y/--yes to accept all defaults. ``` -終了コードは `1`、早期終了の前にファイル生成は一切行われないので、ディレクトリは元の状態のままです。同じルールは [`create-arkor`](/ja/quickstart#claude-code-下での実行claudecode1) にも適用されます（Quickstart に create-arkor 固有の詳細を記載しています。プロジェクト名を明示的に決めさせるための `[dir]` / `--name` 必須化も含む）。 +終了コードは `1`、早期終了の前にファイル生成は一切行われないので、ディレクトリは元の状態のままです。同じルールは [`create-arkor`](/ja/quickstart#claude-code-下での実行（claudecode=1）) にも適用されます（Quickstart に create-arkor 固有の詳細を記載しています。プロジェクト名を明示的に決めさせるための `[dir]` / `--name` 必須化も含む）。 ## エラー diff --git a/docs/ja/quickstart.mdx b/docs/ja/quickstart.mdx index 7eced4d3..0ecc18a9 100644 --- a/docs/ja/quickstart.mdx +++ b/docs/ja/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Claude Code 下での実行（`CLAUDECODE=1`） -Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code-claudecode-1-厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。デフォルトの `arkor-project` は意図的に選ぶより誤指定であることがほとんどなので、明示的に決めさせる方針です。空白だけ・記号だけのように `sanitise()` がフォールバックしてしまう値も拒否されます。 +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code（claudecode=1）厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。デフォルトの `arkor-project` は意図的に選ぶより誤指定であることがほとんどなので、明示的に決めさせる方針です。空白だけ・記号だけのように `sanitise()` がフォールバックしてしまう値も拒否されます。 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトイン）: diff --git a/docs/quickstart.mdx b/docs/quickstart.mdx index d72d54a9..76ca3965 100644 --- a/docs/quickstart.mdx +++ b/docs/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Running under Claude Code (`CLAUDECODE=1`) -Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode-1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since the default fallback (`arkor-project`) is almost always an oversight rather than an intentional choice. Empty / whitespace-only / punctuation-only values that `sanitise()` would collapse back to that fallback are also rejected. +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode=1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since the default fallback (`arkor-project`) is almost always an oversight rather than an intentional choice. Empty / whitespace-only / punctuation-only values that `sanitise()` would collapse back to that fallback are also rejected. Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index 85246cc6..4122d015 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -152,12 +152,23 @@ describe("missingClaudeCodeFlags", () => { ).toEqual([]); }); - it("still accepts the literal `arkor-project` when the caller asked for it explicitly", () => { - // Carve-out so a user who genuinely wants the name `arkor-project` - // isn't blocked by the fallback-detection heuristic. Case- and - // whitespace-insensitive on the input side; the sanitised slug is - // already canonical. - for (const name of ["arkor-project", "ARKOR-PROJECT", " arkor-project "]) { + it("accepts deliberate names that happen to sanitise to `arkor-project`", () => { + // PR #141 review (codex + Copilot): the previous check compared the + // raw input to the literal `arkor-project` string, so inputs like + // `Arkor Project`, `arkor_project`, or a `[dir]` basename + // `Arkor_Project` were rejected because their trimmed/lowercased + // raw form differed from the sanitised slug. They are deliberate + // names, not silent defaults; the rewritten check (input contains + // any `[a-z0-9]`) accepts them because the alphanumeric content + // proves the user typed a real name. + for (const name of [ + "arkor-project", + "ARKOR-PROJECT", + " arkor-project ", + "Arkor Project", + "arkor_project", + "ArkorProject", + ]) { const missing = missingClaudeCodeFlags({ requireProjectName: true, name, diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index c05c5ecb..4e6de7fe 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -1,5 +1,4 @@ import { basename, resolve } from "node:path"; -import { sanitise } from "./sanitise"; /** * Claude Code (the Anthropic agent CLI) sets `CLAUDECODE=1` in every spawned @@ -81,18 +80,20 @@ export interface MissingClaudeCodeFlag { /** * True when `opts` supplies a project name that survives `sanitise()` - * without falling back to the generic `arkor-project` default. Empty - * strings, whitespace-only inputs, and strings that contain no - * `[a-z0-9-]` characters all collapse to `arkor-project` inside - * `sanitise()`, which is exactly the silent-default outcome strict mode - * is trying to prevent. The literal `arkor-project` (case-insensitive - * after trimming) is still accepted because the caller has explicitly - * asked for that name. + * without falling back to the generic `arkor-project` default. The + * fallback only fires when `sanitise()`'s pre-fallback slug is empty, + * which happens iff the input contains zero `[a-z0-9]` characters + * (case-insensitive). So an input is "meaningful" iff it has at least + * one alphanumeric character — this single check captures every silent + * default we want strict mode to surface (empty strings, whitespace + * only, punctuation only) without falsely rejecting deliberate names + * that happen to *sanitise* to `arkor-project` such as `Arkor Project` + * or `arkor_project`. * * `[dir]` is derived through `basename(resolve(opts.dir))` to mirror * `create-arkor`'s own default-name derivation. Without `resolve()`, - * `basename(".")` returns `"."` (which sanitises to the fallback) and - * the validator would falsely reject explicit invocations like + * `basename(".")` returns `"."` (no alphanumerics) and the validator + * would falsely reject explicit invocations like * `create-arkor . --template ...` even though the scaffolder would * pick up the current directory's name at runtime. */ @@ -100,9 +101,7 @@ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { const raw = opts.name ?? (opts.dir !== undefined ? basename(resolve(opts.dir)) : undefined); if (raw === undefined) return false; - const sanitised = sanitise(raw); - if (sanitised !== "arkor-project") return true; - return raw.trim().toLowerCase() === "arkor-project"; + return /[a-z0-9]/i.test(raw); } /** diff --git a/packages/cli-internal/src/sanitise.test.ts b/packages/cli-internal/src/sanitise.test.ts index 7ca3c430..c7beac13 100644 --- a/packages/cli-internal/src/sanitise.test.ts +++ b/packages/cli-internal/src/sanitise.test.ts @@ -47,6 +47,19 @@ describe("sanitise", () => { expect(sanitise(long)).toBe("a".repeat(60)); }); + it("does not leak a trailing dash when the 60-char cap cuts on a separator", () => { + // PR #141 review (Copilot): the trim used to run before the cap, + // so an input like 59 alphanumerics followed by `_b` ended up as + // `aaa…a-` (60 chars, ends with `-`) because the cap landed on + // the dash. The chain is now slice-then-trim, so the trailing + // separator gets stripped after the cap and the slug satisfies + // the documented contract. + const input = `${"a".repeat(59)}_b`; + const result = sanitise(input); + expect(result.endsWith("-")).toBe(false); + expect(result).toBe("a".repeat(59)); + }); + it("preserves digits and dashes", () => { expect(sanitise("v2-runner-01")).toBe("v2-runner-01"); }); diff --git a/packages/cli-internal/src/sanitise.ts b/packages/cli-internal/src/sanitise.ts index 1d19f764..7a5e5c79 100644 --- a/packages/cli-internal/src/sanitise.ts +++ b/packages/cli-internal/src/sanitise.ts @@ -2,8 +2,8 @@ * Normalise a candidate project name into a safe lowercase slug: * - lowercased * - non-alphanumeric runs (including existing `-`) collapsed to a single `-` - * - a single leading / trailing `-` trimmed * - capped at 60 characters + * - a single leading / trailing `-` trimmed (after the cap) * - falls back to `"arkor-project"` if the result is empty * * The regexes are deliberately written so each input character is @@ -12,17 +12,24 @@ * chain combined `/[^a-z0-9-]/g` (which kept `-` as a literal) with a * separate `/-+/g` collapse: an adversarial input of many `-` runs * walked the same characters across two `+`-quantified passes. The - * new first regex includes `-` in the negated class so a single linear + * first regex includes `-` in the negated class so a single linear * scan does both substitution and run-collapsing; the trim step then * only ever has to strip a single dash at each anchor because runs * have already been collapsed. + * + * Trim runs *after* `.slice(0, 60)` so an alphanumeric-then-separator + * input that gets cut on the separator (e.g. 59 letters + `_b` → after + * the run-collapse `aaa…a-b`, then sliced to `aaa…a-`) doesn't leak a + * trailing dash into the final slug. The 60-char cap is a budget and + * the trim is the contract; doing them in this order makes the + * contract win. */ export function sanitise(name: string): string { return ( name .toLowerCase() .replace(/[^a-z0-9]+/g, "-") - .replace(/^-|-$/g, "") - .slice(0, 60) || "arkor-project" + .slice(0, 60) + .replace(/^-|-$/g, "") || "arkor-project" ); } From 6a891289ed00b9dcab27c6625aa857b1516480de Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 10:45:51 +0900 Subject: [PATCH 07/16] feat: enhance strict mode handling for CLAUDECODE in init and create-arkor commands to require curated flags and reject invalid inputs --- docs/cli/init.mdx | 2 +- docs/ja/cli/init.mdx | 2 +- docs/ja/quickstart.mdx | 2 +- docs/quickstart.mdx | 2 +- e2e/cli/src/create-arkor.test.ts | 33 +++++++++++++++ packages/cli-internal/src/claude-code.test.ts | 28 +++++++++++++ packages/cli-internal/src/claude-code.ts | 41 +++++++++++++++---- 7 files changed, 98 insertions(+), 12 deletions(-) diff --git a/docs/cli/init.mdx b/docs/cli/init.mdx index 73080fe0..15591b22 100644 --- a/docs/cli/init.mdx +++ b/docs/cli/init.mdx @@ -124,7 +124,7 @@ If you forget `--yes` in a non-interactive shell, the command still completes (p ### Claude Code (`CLAUDECODE=1`) strict mode -Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `arkor init` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `arkor init` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. Project name is intentionally not required here, because `arkor init` already derives it from `basename(cwd)`, which is the same value the interactive prompt would have suggested. The package-manager flag is required even though it isn't strictly a prompt, so the agent picks `--use-*` vs. `--skip-install` deliberately rather than relying on `npm_config_user_agent` detection. Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): diff --git a/docs/ja/cli/init.mdx b/docs/ja/cli/init.mdx index eadf8ff4..f34bdc16 100644 --- a/docs/ja/cli/init.mdx +++ b/docs/ja/cli/init.mdx @@ -124,7 +124,7 @@ bun arkor init --yes --template triage --use-bun --skip-git ### Claude Code（`CLAUDECODE=1`）厳格モード -Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `arkor init` は厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。 +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `arkor init` は厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。プロジェクト名は意図的に必須にしていません。`arkor init` は `basename(cwd)` から派生させており、これは対話プロンプトでのデフォルト提案値と同じだからです。パッケージマネージャーは厳密にはプロンプトではありません（UA 自動検出か検出失敗時の silent skip）が、`npm_config_user_agent` 任せにせず `--use-*` か `--skip-install` をエージェントが明示的に選ぶように、こちらは必須としています。 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトインしてください）: diff --git a/docs/ja/quickstart.mdx b/docs/ja/quickstart.mdx index 0ecc18a9..2ccb158a 100644 --- a/docs/ja/quickstart.mdx +++ b/docs/ja/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Claude Code 下での実行（`CLAUDECODE=1`） -Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。対話プロンプトに対応するフラグはすべて必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code（claudecode=1）厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。デフォルトの `arkor-project` は意図的に選ぶより誤指定であることがほとんどなので、明示的に決めさせる方針です。空白だけ・記号だけのように `sanitise()` がフォールバックしてしまう値も拒否されます。 +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code（claudecode=1）厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。これは positional 省略時のフォールバック先 `arkor-project/` サブディレクトリが意図的に選ばれることはまずなく、ほぼ常に指定漏れだからです。ASCII の英数字を 1 文字も含まない入力（空、空白だけ、`!!!` のような記号だけ）も拒否されます。`sanitise()` が同じ generic フォールバックに collapse させてしまうためです。一方、サニタイズ結果がたまたま `arkor-project` になるような **意図的な名前**（例: `Arkor Project`、`arkor_project`、`ArkorProject`）は受け入れます。英数字が含まれていることがエージェントの明示的入力の証拠になるためです。 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトイン）: diff --git a/docs/quickstart.mdx b/docs/quickstart.mdx index 76ca3965..435ac89f 100644 --- a/docs/quickstart.mdx +++ b/docs/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Running under Claude Code (`CLAUDECODE=1`) -Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: every flag that mirrors an interactive prompt becomes required, and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode=1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since the default fallback (`arkor-project`) is almost always an oversight rather than an intentional choice. Empty / whitespace-only / punctuation-only values that `sanitise()` would collapse back to that fallback are also rejected. +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode=1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since `create-arkor`'s default fallback for a missing positional is the generic `arkor-project/` subdirectory, almost always an oversight rather than an intentional choice. Inputs that contain no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!`) are rejected as well, because `sanitise()` would collapse them back to that same generic fallback. Deliberate names whose final slug *happens* to be `arkor-project` (for example `Arkor Project`, `arkor_project`, `ArkorProject`) are accepted: the alphanumeric content shows the agent typed a real name. Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index 647f9176..709cb453 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -611,6 +611,39 @@ describe("create-arkor (E2E)", () => { }, ); + it.each([ + ["empty positional", ""], + ["whitespace-only positional", " "], + ])( + "rejects %s without leaning on resolve()'s cwd fallback", + async (_label, dir) => { + // PR #141 review (Copilot): `resolve("")` returns the current + // working directory, whose basename is usually alphanumeric. + // Without an early guard, `create-arkor "" --template ...` (or a + // quoted empty shell variable) would silently scaffold in-place + // under the parent dir's basename, defeating the rejection we + // already enforce for `--name ""`. The check now refuses any + // positional that is empty/whitespace before resolution. + const result = await runCli( + CREATE_ARKOR_BIN, + [ + dir, + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + parentDir, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(1); + expect(result.stderr).toContain("[dir]"); + // Sanity: parent dir stays empty. + expect(readdirSync(parentDir)).toEqual([]); + }, + ); + it("accepts `create-arkor .` (resolves to the parent dir basename, not the literal `.`)", async () => { // Regression for PR #141 review (codex + Copilot): the strict // check used to compute the project name as `basename(opts.dir)` diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index 4122d015..b4607e15 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -125,6 +125,34 @@ describe("missingClaudeCodeFlags", () => { ); }); + it.each([ + ["empty positional", ""], + ["whitespace-only positional", " "], + ["tab-only positional", "\t"], + ])( + "rejects %s without consulting `resolve()` (which would yield process.cwd())", + (_label, dir) => { + // PR #141 review (Copilot): `resolve("")` returns the current + // working directory, whose basename is usually alphanumeric. So + // before this guard, `create-arkor "" --template ...` or a quoted + // empty shell variable would silently scaffold in-place under the + // cwd basename, defeating the rejection we already enforce for + // `--name ""`. `.` / `./` still pass because they are deliberate + // "scaffold in this directory" idioms, not accidental empties. + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + dir, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain( + "[dir] (e.g. `my-arkor-app`) or --name ", + ); + }, + ); + it("resolves [dir] before taking the basename so `.` / `..` are treated like the surrounding directory", () => { // Regression for PR #141 review (codex + Copilot): `basename(".")` // is `"."` which sanitise() collapses to the fallback, so the diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index 4e6de7fe..487f881c 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -5,9 +5,24 @@ import { basename, resolve } from "node:path"; * shell. It cannot answer interactive prompts, and silently falling through to * defaults masks decisions the agent should be making explicitly. So the * scaffolders (`create-arkor`, `arkor init`) flip into a strict mode under - * this env var: every flag that mirrors an interactive prompt becomes - * required, and missing ones produce a stderr message listing the suggested - * re-invocation rather than running with hidden defaults. + * this env var: a curated set of flags becomes required, and missing ones + * produce a stderr message listing the suggested re-invocation rather than + * running with hidden defaults. + * + * "Curated" (not "every interactive prompt") because the runtime defaults + * for a couple of decisions are well-understood enough that forcing them + * would just add noise: + * + * - **Project name** is not required by `arkor init`: the scaffolder + * derives it from `basename(cwd)`, which is the same value the + * interactive prompt offers. `create-arkor` *does* require it because + * a missing `[dir]` falls all the way back to the generic + * `arkor-project` slug, which is almost always an oversight. + * - **Package manager** is not actually prompted for at runtime; it's + * a defaulted decision (UA-sniffed, or skipped silently when + * detection fails). It's required here because the agent should + * pick `--use-*` vs. `--skip-install` deliberately, not because the + * interactive UX would have asked. * * `--yes` opts back into the legacy "accept all defaults" path; callers that * have explicitly delegated those decisions to the CLI keep working unchanged. @@ -98,10 +113,20 @@ export interface MissingClaudeCodeFlag { * pick up the current directory's name at runtime. */ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { - const raw = - opts.name ?? (opts.dir !== undefined ? basename(resolve(opts.dir)) : undefined); - if (raw === undefined) return false; - return /[a-z0-9]/i.test(raw); + if (opts.name !== undefined) return /[a-z0-9]/i.test(opts.name); + if (opts.dir === undefined) return false; + // Reject empty / whitespace-only positionals BEFORE resolving them. + // `resolve("")` (and `resolve(" ")` after trimming through shell + // word-splitting in practice) returns `process.cwd()`, whose basename + // is almost always alphanumeric; so without this guard, + // `create-arkor "" --template ...` or a quoted shell variable that + // happened to be empty would slip through strict mode and scaffold + // in-place against the cwd basename, mirroring the silent-default + // outcome we reject for `--name ""`. `.` / `./` etc. still pass + // because they are *deliberate* "scaffold in this directory" idioms + // that share the same runtime semantics as a non-empty path. + if (opts.dir.trim() === "") return false; + return /[a-z0-9]/i.test(basename(resolve(opts.dir))); } /** @@ -120,7 +145,7 @@ export function missingClaudeCodeFlags( missing.push({ flag: "[dir] (e.g. `my-arkor-app`) or --name ", description: - "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit.", + "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Inputs with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!` / `***`) are rejected because they would collapse to the generic `arkor-project` fallback inside `sanitise()`.", }); } if (opts.template === undefined) { From 7224433a2d0630bccc718bb8471a5e3ea3f5d4d5 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 16:42:40 +0900 Subject: [PATCH 08/16] feat: enhance project name validation to reject empty and non-ASCII inputs in create-arkor and related commands --- docs/ja/quickstart.mdx | 4 +- docs/quickstart.mdx | 4 +- e2e/cli/src/create-arkor.test.ts | 18 +++---- packages/cli-internal/src/claude-code.test.ts | 48 ++++++++++--------- packages/cli-internal/src/claude-code.ts | 25 ++++++---- packages/cli-internal/src/sanitise.test.ts | 16 ++++--- packages/cli-internal/src/sanitise.ts | 30 ++++++------ 7 files changed, 79 insertions(+), 66 deletions(-) diff --git a/docs/ja/quickstart.mdx b/docs/ja/quickstart.mdx index 2ccb158a..a9434c6a 100644 --- a/docs/ja/quickstart.mdx +++ b/docs/ja/quickstart.mdx @@ -77,7 +77,7 @@ Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプ 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトイン）: -- `[dir]`（例 `my-arkor-app`）または `--name `: 明示的なプロジェクト名。`sanitise()` で `arkor-project` フォールバックに落ちる値は不可。 +- `[dir]`（例 `my-arkor-app`）または `--name `: 明示的なプロジェクト名。ASCII の英数字を 1 文字も含まない入力（空、空白だけ、`!!!` のような記号だけ）は、`arkor-project` フォールバックに collapse させてしまうため拒否。`Arkor Project`、`arkor_project` のように **意図的でたまたま** サニタイズ結果が `arkor-project` になる入力は受け入れます。 - `--template ` - `--git`（推奨。対話モードのデフォルトと同じ）または `--skip-git` - パッケージマネージャー系フラグ: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`、または `--skip-install` @@ -89,7 +89,7 @@ Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプ create-arkor: CLAUDECODE=1 detected. Interactive prompts are disabled. Re-run with explicit flags: [dir] (e.g. `my-arkor-app`) or --name - Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit. + Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Inputs with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!` / `***`) are rejected because they would collapse to the generic `arkor-project` fallback inside `sanitise()`. --template Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). --git (recommended) or --skip-git diff --git a/docs/quickstart.mdx b/docs/quickstart.mdx index 435ac89f..1059bff0 100644 --- a/docs/quickstart.mdx +++ b/docs/quickstart.mdx @@ -77,7 +77,7 @@ Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer int Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): -- `[dir]` (e.g. `my-arkor-app`) or `--name `: explicit project name; values that sanitise to the generic `arkor-project` fallback do not count. +- `[dir]` (e.g. `my-arkor-app`) or `--name `: explicit project name. Values with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!`) are rejected because they would collapse to the generic `arkor-project` fallback; deliberate inputs that *happen* to sanitise to `arkor-project` (`Arkor Project`, `arkor_project`, …) are accepted. - `--template ` - `--git` (recommended; matches the interactive default) or `--skip-git` - A package-manager flag: `--use-npm` / `--use-pnpm` / `--use-yarn` / `--use-bun`, or `--skip-install` @@ -89,7 +89,7 @@ Sample stderr when a flag is missing: create-arkor: CLAUDECODE=1 detected. Interactive prompts are disabled. Re-run with explicit flags: [dir] (e.g. `my-arkor-app`) or --name - Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Empty / whitespace-only values that `sanitise()` would collapse to the generic `arkor-project` fallback do not count as explicit. + Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Inputs with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!` / `***`) are rejected because they would collapse to the generic `arkor-project` fallback inside `sanitise()`. --template Starter template: `triage` (support routing), `translate` (9-language translation), or `redaction` (PII removal). --git (recommended) or --skip-git diff --git a/e2e/cli/src/create-arkor.test.ts b/e2e/cli/src/create-arkor.test.ts index 709cb453..980adddc 100644 --- a/e2e/cli/src/create-arkor.test.ts +++ b/e2e/cli/src/create-arkor.test.ts @@ -615,15 +615,17 @@ describe("create-arkor (E2E)", () => { ["empty positional", ""], ["whitespace-only positional", " "], ])( - "rejects %s without leaning on resolve()'s cwd fallback", + "rejects %s under strict mode", async (_label, dir) => { - // PR #141 review (Copilot): `resolve("")` returns the current - // working directory, whose basename is usually alphanumeric. - // Without an early guard, `create-arkor "" --template ...` (or a - // quoted empty shell variable) would silently scaffold in-place - // under the parent dir's basename, defeating the rejection we - // already enforce for `--name ""`. The check now refuses any - // positional that is empty/whitespace before resolution. + // PR #141 review (Copilot): the motivating case is the empty + // string. `path.resolve("")` returns `process.cwd()` and + // would scaffold against the parent's alphanumeric basename + // without the early trim guard. Whitespace inputs would also + // fail strict mode via the downstream alphanumeric check on + // their own (since `resolve(" ")` resolves to a whitespace- + // basename path), but pinning both shapes here keeps the + // contract that empty-shaped positionals get the same + // rejection as `--name ""`. const result = await runCli( CREATE_ARKOR_BIN, [ diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index b4607e15..7749d09e 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -129,29 +129,31 @@ describe("missingClaudeCodeFlags", () => { ["empty positional", ""], ["whitespace-only positional", " "], ["tab-only positional", "\t"], - ])( - "rejects %s without consulting `resolve()` (which would yield process.cwd())", - (_label, dir) => { - // PR #141 review (Copilot): `resolve("")` returns the current - // working directory, whose basename is usually alphanumeric. So - // before this guard, `create-arkor "" --template ...` or a quoted - // empty shell variable would silently scaffold in-place under the - // cwd basename, defeating the rejection we already enforce for - // `--name ""`. `.` / `./` still pass because they are deliberate - // "scaffold in this directory" idioms, not accidental empties. - const missing = missingClaudeCodeFlags({ - requireProjectName: true, - dir, - template: "triage", - skipGit: true, - useNpm: true, - agentsMd: false, - }); - expect(missing.map((m) => m.flag)).toContain( - "[dir] (e.g. `my-arkor-app`) or --name ", - ); - }, - ); + ])("rejects %s before resolve()", (_label, dir) => { + // PR #141 review (Copilot): the motivating case is the EMPTY + // string. `path.resolve("")` returns `process.cwd()` whose + // basename is usually alphanumeric, so without the trim guard + // `create-arkor "" --template ...` (or a quoted empty shell + // variable) would slip through and scaffold against the cwd + // basename. Whitespace-only inputs would also be rejected by the + // downstream alphanumeric regex on their own (since + // `resolve(" ")` keeps the spaces as a basename), but exercising + // them here pins the guard's contract: trim semantics apply to + // *every* empty-shaped input, not only `""`. `.` / `./` are + // covered elsewhere as deliberate "scaffold in this directory" + // idioms. + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + dir, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain( + "[dir] (e.g. `my-arkor-app`) or --name ", + ); + }); it("resolves [dir] before taking the basename so `.` / `..` are treated like the surrounding directory", () => { // Regression for PR #141 review (codex + Copilot): `basename(".")` diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index 487f881c..e176ed78 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -115,16 +115,21 @@ export interface MissingClaudeCodeFlag { function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { if (opts.name !== undefined) return /[a-z0-9]/i.test(opts.name); if (opts.dir === undefined) return false; - // Reject empty / whitespace-only positionals BEFORE resolving them. - // `resolve("")` (and `resolve(" ")` after trimming through shell - // word-splitting in practice) returns `process.cwd()`, whose basename - // is almost always alphanumeric; so without this guard, - // `create-arkor "" --template ...` or a quoted shell variable that - // happened to be empty would slip through strict mode and scaffold - // in-place against the cwd basename, mirroring the silent-default - // outcome we reject for `--name ""`. `.` / `./` etc. still pass - // because they are *deliberate* "scaffold in this directory" idioms - // that share the same runtime semantics as a non-empty path. + // Reject empty / whitespace-only positionals before consulting + // `resolve()`. The motivating case is the **empty string**: + // `path.resolve("")` returns `process.cwd()` whose basename is + // almost always alphanumeric, so `create-arkor "" --template ...` + // (or a quoted shell variable that expanded to empty) would slip + // through strict mode and scaffold in-place against the cwd + // basename, the exact silent-default outcome we already reject + // for `--name ""`. Whitespace-only inputs (`" "`, `"\t"`) would + // *also* survive `resolve()` as a path with a whitespace basename, + // and the alphanumeric check below would catch them on its own; + // the trim here is the cheaper, earlier guard that gives both + // shapes the same rejection without depending on the downstream + // regex. `.` / `./` etc. still pass because they are deliberate + // "scaffold in this directory" idioms with the same runtime + // semantics as any non-empty path. if (opts.dir.trim() === "") return false; return /[a-z0-9]/i.test(basename(resolve(opts.dir))); } diff --git a/packages/cli-internal/src/sanitise.test.ts b/packages/cli-internal/src/sanitise.test.ts index c7beac13..a289e107 100644 --- a/packages/cli-internal/src/sanitise.test.ts +++ b/packages/cli-internal/src/sanitise.test.ts @@ -24,12 +24,16 @@ describe("sanitise", () => { expect(sanitise("---abc---")).toBe("abc"); }); - it("terminates in linear time on adversarial dash-heavy inputs (ReDoS regression)", () => { - // CodeQL flagged the previous regex chain as a polynomial-ReDoS - // hotspot once `sanitise()` started receiving CLI-arg input from - // the CLAUDECODE strict-mode check. The current chain consumes - // each character at most a constant number of times, so even a - // pathological input completes in well under the test's timeout. + it("stays linear on adversarial dash-heavy inputs (CodeQL hardening)", () => { + // CodeQL's "polynomial regex on uncontrolled data" query flagged + // the previous chain once `claude-code.ts` started feeding it + // CLI-arg input. The original `+`-quantified replaces were already + // linear in practice (this isn't a real ReDoS regression test), + // but a 100k-character pathological input is a cheap belt-and- + // suspenders check that the rewritten chain remains linear, and + // anchors the alert resolution so a future rewrite that + // accidentally introduced an ambiguous quantifier would trip the + // timeout instead of silently re-opening the CodeQL alert. const adversarial = "-".repeat(100_000); const start = Date.now(); expect(sanitise(adversarial)).toBe("arkor-project"); diff --git a/packages/cli-internal/src/sanitise.ts b/packages/cli-internal/src/sanitise.ts index 7a5e5c79..54ba07fa 100644 --- a/packages/cli-internal/src/sanitise.ts +++ b/packages/cli-internal/src/sanitise.ts @@ -6,23 +6,23 @@ * - a single leading / trailing `-` trimmed (after the cap) * - falls back to `"arkor-project"` if the result is empty * - * The regexes are deliberately written so each input character is - * consumed at most a constant number of times, avoiding the - * polynomial-backtracking shape CodeQL flagged when the previous - * chain combined `/[^a-z0-9-]/g` (which kept `-` as a literal) with a - * separate `/-+/g` collapse: an adversarial input of many `-` runs - * walked the same characters across two `+`-quantified passes. The - * first regex includes `-` in the negated class so a single linear - * scan does both substitution and run-collapsing; the trim step then - * only ever has to strip a single dash at each anchor because runs - * have already been collapsed. + * The original chain was three separate replaces (`/[^a-z0-9-]/g`, + * `/-+/g`, `/^-+|-+$/g`), each a linear character-class or literal + * scan. CodeQL's "polynomial-regex on uncontrolled data" query + * conservatively flagged the chain once `claude-code.ts` started + * feeding it CLI-arg input, even though no pattern actually + * backtracks. Rather than annotate a suppression, the chain was + * rewritten into a single negated-class collapse plus an anchored + * trim: it is provably linear (each input character consumed a + * constant number of times) and shorter, which removes the alert + * without losing readability. * * Trim runs *after* `.slice(0, 60)` so an alphanumeric-then-separator - * input that gets cut on the separator (e.g. 59 letters + `_b` → after - * the run-collapse `aaa…a-b`, then sliced to `aaa…a-`) doesn't leak a - * trailing dash into the final slug. The 60-char cap is a budget and - * the trim is the contract; doing them in this order makes the - * contract win. + * input that gets cut on the separator (e.g. 59 letters + `_b` after + * the run-collapse becomes `aaa…a-b`, then sliced to `aaa…a-`) + * doesn't leak a trailing dash into the final slug. The 60-char cap + * is a budget and the trim is the contract; doing them in this order + * makes the contract win. */ export function sanitise(name: string): string { return ( From fc33b4cc548f376f161994efe98974da7da113f0 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 23:13:01 +0900 Subject: [PATCH 09/16] test: add case for parent directory in missingClaudeCodeFlags tests --- docs/cli/overview.mdx | 2 +- docs/ja/cli/overview.mdx | 2 +- packages/cli-internal/src/claude-code.test.ts | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/cli/overview.mdx b/docs/cli/overview.mdx index 63708020..1520534d 100644 --- a/docs/cli/overview.mdx +++ b/docs/cli/overview.mdx @@ -32,7 +32,7 @@ The `arkor` CLI is the command surface around an Arkor project. The [scaffolder] | `DO_NOT_TRACK=1` | Disable telemetry across every command. The persistent telemetry-id file is not created or read. | | `ARKOR_TELEMETRY_DISABLED=1` | Same as `DO_NOT_TRACK`. Provided as an Arkor-specific alias for setups that already gate other tools on `DO_NOT_TRACK` and want a separate switch. | | `ARKOR_TELEMETRY_DEBUG=1` | Enable verbose internal logging from the telemetry module (init failures, identity-file read/write errors, capture failures, shutdown errors). Telemetry **is still sent** when this flag is on; this is a diagnostic aid, not a dry-run switch. To stop sending events, use `DO_NOT_TRACK=1` or `ARKOR_TELEMETRY_DISABLED=1`. | -| `CI=1` (or any non-TTY stdout) | Force non-interactive mode. `arkor init` skips its prompts and uses the values you passed via flags (or built-in defaults under `--yes`); git init runs without asking when `--git` or `--yes` is set, and is skipped silently otherwise. See [`arkor init` § CI / non-interactive shells](/cli/init#ci--non-interactive-shells). | +| `CI=1` (or any non-TTY stdout) | Force non-interactive mode. `arkor init` skips its prompts and uses the values you passed via flags (or built-in defaults under `--yes`); git init runs without asking when `--git` or `--yes` is set, and is skipped silently otherwise. See [`arkor init` § CI / non-interactive shells](/cli/init#ci-/-non-interactive-shells). | ### On the roadmap diff --git a/docs/ja/cli/overview.mdx b/docs/ja/cli/overview.mdx index 66238c33..3eae7fc7 100644 --- a/docs/ja/cli/overview.mdx +++ b/docs/ja/cli/overview.mdx @@ -32,7 +32,7 @@ description: "arkor CLI: 全コマンドリファレンス。" | `DO_NOT_TRACK=1` | すべてのコマンドでテレメトリーを無効化。永続テレメトリー ID ファイルは作成も読込もされません。 | | `ARKOR_TELEMETRY_DISABLED=1` | `DO_NOT_TRACK` と同じ。`DO_NOT_TRACK` で他ツールをゲートしているがスイッチを分けたい構成のための Arkor 専用エイリアス。 | | `ARKOR_TELEMETRY_DEBUG=1` | テレメトリーモジュール内部の冗長ログを有効化（init 失敗、ID ファイルの read/write エラー、capture 失敗、shutdown エラー）。このフラグが ON でも **テレメトリーは送信されます**。診断補助で、dry-run スイッチではありません。送信を止めるには `DO_NOT_TRACK=1` か `ARKOR_TELEMETRY_DISABLED=1` を使ってください。 | -| `CI=1`（または stdout が非 TTY） | 非対話モードを強制。`arkor init` はプロンプトをスキップしてフラグで渡された値（`--yes` 時は組み込みのデフォルト）を使い、`--git` か `--yes` 指定時は git init が確認なしで走り、それ以外では黙ってスキップされます。[`arkor init` § CI / 非対話シェル](/ja/cli/init#ci--非対話シェル) を参照。 | +| `CI=1`（または stdout が非 TTY） | 非対話モードを強制。`arkor init` はプロンプトをスキップしてフラグで渡された値（`--yes` 時は組み込みのデフォルト）を使い、`--git` か `--yes` 指定時は git init が確認なしで走り、それ以外では黙ってスキップされます。[`arkor init` § CI / 非対話シェル](/ja/cli/init#ci-/-非対話シェル) を参照。 | ### ロードマップ上 diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index 7749d09e..e7b89f20 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -176,6 +176,12 @@ describe("missingClaudeCodeFlags", () => { // doesn't matter as long as it isn't the `arkor-project` fallback. expect(missingClaudeCodeFlags({ ...baseOpts, dir: "." })).toEqual([]); expect(missingClaudeCodeFlags({ ...baseOpts, dir: "./" })).toEqual([]); + // `..` resolves to the parent directory (e.g. `packages` when + // vitest is run from `packages/cli-internal`), whose basename is + // also a meaningful slug. Pinning this case matches the test + // title's claim and guards the symmetric `.` / `..` branch in + // the `basename(resolve(opts.dir))` derivation. + expect(missingClaudeCodeFlags({ ...baseOpts, dir: ".." })).toEqual([]); // Relative path that resolves to a meaningful basename also passes. expect( missingClaudeCodeFlags({ ...baseOpts, dir: "foo/bar/my-app" }), From be609761153c3d7f62c8b4a6b0c26f307c616f9a Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Fri, 15 May 2026 23:15:16 +0900 Subject: [PATCH 10/16] docs: clarify Mintlify heading slug behavior and update related documentation --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 28065a71..683d33b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,6 +100,7 @@ Don't split these into "docs in a follow-up PR" or "tests later" — land them i ## Non-obvious gotchas - **Docs CLI CodeGroup convention.** Pages under `/cli/*` and `/guides/cli/*` (and their JA mirrors) show unscripted subcommands (`arkor init`, `arkor login`, `arkor logout`, `arkor whoami`) as a 5-tab CodeGroup in the form ` arkor `: `pnpm arkor init`, `npm arkor init`, `yarn arkor init`, `yarn run arkor init` (yarn-berry needs `run`), `bun arkor init`. The `npm` tab stays as `npm arkor init` even though npm itself does not run package binaries via `npm `. These commands are intended to be run after `arkor` is already in the project's `package.json` (post `create-arkor`, or post manual `pnpm i arkor` / equivalent), and the 5-tab form is about consistent presentation per package manager, not about being copy-pasteable for users without a project. Do **not** rewrite the `npm` tab to `npx arkor ` or `npm exec arkor `, and do not act on review comments (human or bot) saying "`npm arkor init` does not work": that comment misses the premise. +- **Mintlify heading slugs preserve punctuation.** The common assumption (GitHub-style / `github-slugger`) is that `=`, `/`, parens, etc. get stripped from heading ids. Mintlify does **not** strip them. `### Claude Code (\`CLAUDECODE=1\`) strict mode` renders `id="claude-code-claudecode=1-strict-mode"`; `### CI / non-interactive shells` renders `id="ci-/-non-interactive-shells"`. Cross-page links in `docs/` must therefore use the literal-punctuation form (`/cli/init#claude-code-claudecode=1-strict-mode`), not the GitHub-style stripped variant. To verify the actual id before adding or editing an anchor link, curl the rendered preview and grep: `curl -s "/" | grep -oE 'id="[^"]*[^"]*"'`. Do not act on review comments (human or bot) claiming "Mintlify strips punctuation in headings": Copilot in particular gets this wrong repeatedly by extrapolating from `github-slugger`, and has cited the pre-existing broken `#ci--non-interactive-shells` link as supposed evidence. The link from `docs/cli/overview.mdx` is the canonical correct example after [PR #141](https://github.com/arkorlab/arkor/pull/141) fixed it to `#ci-/-non-interactive-shells`. - **Don't call a HuggingFace model name "non-existent"** based on training-data alone. Templates reference real models (e.g. `unsloth/gemma-4-E4B-it`) that may post-date Claude's knowledge cutoff. Verify (e.g. `WebFetch`) before flagging in issues or PR comments. If unverifiable, hedge ("could not confirm") rather than asserting absence. - **Generated files** copied into package dirs are gitignored: `packages/*/CONTRIBUTING.md` (from root), `packages/arkor/docs/` (from root `docs/`). Edit the source under repo root, not the copies. - **Node version**: published packages declare `engines.node >=22.22.0` (raised from 22.6 to dodge the [Jan 2026 async-hooks DoS CVE](https://nodejs.org/en/blog/vulnerability/january-2026-dos-mitigation-async-hooks)). Use Node 24 (latest preferred) for development per [CONTRIBUTING.md](CONTRIBUTING.md). From d9d714a3849c5f1aa10473754cc4e5df2837df37 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sat, 16 May 2026 10:33:13 +0900 Subject: [PATCH 11/16] feat: enhance strict mode validation in create-arkor and related commands to reject empty and whitespace-only inputs --- AGENTS.md | 7 ++- docs/ja/quickstart.mdx | 2 +- docs/quickstart.mdx | 2 +- packages/cli-internal/src/claude-code.test.ts | 52 ++++++++++++++++- packages/cli-internal/src/claude-code.ts | 56 +++++++++++-------- packages/cli-internal/src/sanitise.test.ts | 9 +-- packages/cli-internal/src/sanitise.ts | 16 +++--- 7 files changed, 105 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 683d33b1..801b4130 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,7 +100,12 @@ Don't split these into "docs in a follow-up PR" or "tests later" — land them i ## Non-obvious gotchas - **Docs CLI CodeGroup convention.** Pages under `/cli/*` and `/guides/cli/*` (and their JA mirrors) show unscripted subcommands (`arkor init`, `arkor login`, `arkor logout`, `arkor whoami`) as a 5-tab CodeGroup in the form ` arkor `: `pnpm arkor init`, `npm arkor init`, `yarn arkor init`, `yarn run arkor init` (yarn-berry needs `run`), `bun arkor init`. The `npm` tab stays as `npm arkor init` even though npm itself does not run package binaries via `npm `. These commands are intended to be run after `arkor` is already in the project's `package.json` (post `create-arkor`, or post manual `pnpm i arkor` / equivalent), and the 5-tab form is about consistent presentation per package manager, not about being copy-pasteable for users without a project. Do **not** rewrite the `npm` tab to `npx arkor ` or `npm exec arkor `, and do not act on review comments (human or bot) saying "`npm arkor init` does not work": that comment misses the premise. -- **Mintlify heading slugs preserve punctuation.** The common assumption (GitHub-style / `github-slugger`) is that `=`, `/`, parens, etc. get stripped from heading ids. Mintlify does **not** strip them. `### Claude Code (\`CLAUDECODE=1\`) strict mode` renders `id="claude-code-claudecode=1-strict-mode"`; `### CI / non-interactive shells` renders `id="ci-/-non-interactive-shells"`. Cross-page links in `docs/` must therefore use the literal-punctuation form (`/cli/init#claude-code-claudecode=1-strict-mode`), not the GitHub-style stripped variant. To verify the actual id before adding or editing an anchor link, curl the rendered preview and grep: `curl -s "/" | grep -oE 'id="[^"]*[^"]*"'`. Do not act on review comments (human or bot) claiming "Mintlify strips punctuation in headings": Copilot in particular gets this wrong repeatedly by extrapolating from `github-slugger`, and has cited the pre-existing broken `#ci--non-interactive-shells` link as supposed evidence. The link from `docs/cli/overview.mdx` is the canonical correct example after [PR #141](https://github.com/arkorlab/arkor/pull/141) fixed it to `#ci-/-non-interactive-shells`. +- **Mintlify heading slugs are not GitHub-style; verify with curl.** The common assumption (`github-slugger`) is that all punctuation gets stripped from heading ids. Mintlify's actual behaviour is mixed: some punctuation is **preserved** in the id (`=`, `/`), some is **stripped without replacement** (parens `(` `)`, backticks `` ` ``, brackets), and spaces become `-` as usual. Verified examples in the rendered preview at the time of [PR #141](https://github.com/arkorlab/arkor/pull/141): + - `### CI / non-interactive shells` → `id="ci-/-non-interactive-shells"` (slash kept, slash-flanking spaces still become hyphens, hence `-/-`) + - `` ### Claude Code (`CLAUDECODE=1`) strict mode `` → `id="claude-code-claudecode=1-strict-mode"` (parens + backticks stripped, `=` kept, surrounding spaces become hyphens) + - `### Running under Claude Code (` `` `CLAUDECODE=1` `` `)` → `id="running-under-claude-code-claudecode=1"` + + Cross-page links in `docs/` must therefore mirror the actual rendered id, which sometimes preserves `/` and `=` and sometimes strips parens / backticks. Always confirm before adding or editing an anchor link: `curl -s "/" | grep -oE 'id="[^"]*[^"]*"'`. Do not act on review comments (human or bot) claiming "Mintlify strips punctuation in headings" as a universal rule: Copilot in particular gets this wrong repeatedly by extrapolating from `github-slugger`, and has even cited the pre-existing broken `#ci--non-interactive-shells` link as supposed evidence. PR #141 fixed that broken link to `#ci-/-non-interactive-shells`, and the surviving anchors in `docs/cli/overview.mdx` and the strict-mode sections of `docs/quickstart.mdx` / `docs/cli/init.mdx` are the canonical examples to copy from. - **Don't call a HuggingFace model name "non-existent"** based on training-data alone. Templates reference real models (e.g. `unsloth/gemma-4-E4B-it`) that may post-date Claude's knowledge cutoff. Verify (e.g. `WebFetch`) before flagging in issues or PR comments. If unverifiable, hedge ("could not confirm") rather than asserting absence. - **Generated files** copied into package dirs are gitignored: `packages/*/CONTRIBUTING.md` (from root), `packages/arkor/docs/` (from root `docs/`). Edit the source under repo root, not the copies. - **Node version**: published packages declare `engines.node >=22.22.0` (raised from 22.6 to dodge the [Jan 2026 async-hooks DoS CVE](https://nodejs.org/en/blog/vulnerability/january-2026-dos-mitigation-async-hooks)). Use Node 24 (latest preferred) for development per [CONTRIBUTING.md](CONTRIBUTING.md). diff --git a/docs/ja/quickstart.mdx b/docs/ja/quickstart.mdx index a9434c6a..5597f55e 100644 --- a/docs/ja/quickstart.mdx +++ b/docs/ja/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Claude Code 下での実行（`CLAUDECODE=1`） -Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code（claudecode=1）厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。これは positional 省略時のフォールバック先 `arkor-project/` サブディレクトリが意図的に選ばれることはまずなく、ほぼ常に指定漏れだからです。ASCII の英数字を 1 文字も含まない入力（空、空白だけ、`!!!` のような記号だけ）も拒否されます。`sanitise()` が同じ generic フォールバックに collapse させてしまうためです。一方、サニタイズ結果がたまたま `arkor-project` になるような **意図的な名前**（例: `Arkor Project`、`arkor_project`、`ArkorProject`）は受け入れます。英数字が含まれていることがエージェントの明示的入力の証拠になるためです。 +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `create-arkor` も厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。同じルールは [`arkor init`](/ja/cli/init#claude-code（claudecode=1）厳格モード) にも適用されますが、`create-arkor` だけ追加で `[dir]`（または `--name`）も必須となります。これは positional 省略時のフォールバック先 `arkor-project/` サブディレクトリが意図的に選ばれることはまずなく、ほぼ常に指定漏れだからです。ASCII の英数字を 1 文字も含まない入力（空、空白だけ、`!!!` のような記号だけ）も拒否されます。`sanitise()` が同じ generic フォールバックに collapse させてしまうためです。一方、サニタイズ結果がたまたま `arkor-project` になるような **意図的な名前**（例: `Arkor Project`、`arkor_project`、`Arkor.Project` のようにセパレータ文字が `-` に書き換えられるもの）は受け入れます。英数字が含まれていることがエージェントの明示的入力の証拠になるためです。 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトイン）: diff --git a/docs/quickstart.mdx b/docs/quickstart.mdx index 1059bff0..50b76772 100644 --- a/docs/quickstart.mdx +++ b/docs/quickstart.mdx @@ -73,7 +73,7 @@ bun create arkor my-arkor-app --template triage ### Running under Claude Code (`CLAUDECODE=1`) -Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode=1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since `create-arkor`'s default fallback for a missing positional is the generic `arkor-project/` subdirectory, almost always an oversight rather than an intentional choice. Inputs that contain no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!`) are rejected as well, because `sanitise()` would collapse them back to that same generic fallback. Deliberate names whose final slug *happens* to be `arkor-project` (for example `Arkor Project`, `arkor_project`, `ArkorProject`) are accepted: the alphanumeric content shows the agent typed a real name. +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `create-arkor` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. The same rule applies to [`arkor init`](/cli/init#claude-code-claudecode=1-strict-mode), with one extra requirement here: `[dir]` (or `--name`) is also mandatory, since `create-arkor`'s default fallback for a missing positional is the generic `arkor-project/` subdirectory, almost always an oversight rather than an intentional choice. Inputs that contain no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!`) are rejected as well, because `sanitise()` would collapse them back to that same generic fallback. Deliberate names whose final slug *happens* to be `arkor-project` (for example `Arkor Project`, `arkor_project`, `Arkor.Project`, or anything else where a separator gets rewritten to `-`) are accepted: the alphanumeric content shows the agent typed a real name. Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index e7b89f20..9fc6d671 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -125,6 +125,28 @@ describe("missingClaudeCodeFlags", () => { ); }); + it("rejects empty positional even when --name is set (positional drives the runtime target dir)", () => { + // PR #141 review (Copilot): `create-arkor` uses `opts.dir` for the + // *target directory*, not just the slug. So + // `CLAUDECODE=1 create-arkor "" --name my-app --template ...` + // would otherwise pass strict mode (because `--name` is meaningful) + // and then scaffold in-place under the parent dir at runtime via + // `resolve("")`. The empty-positional guard now fires before the + // `--name` shortcut so this mis-input is rejected up front. + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + dir: "", + name: "my-app", + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain( + "[dir] (e.g. `my-arkor-app`) or --name ", + ); + }); + it.each([ ["empty positional", ""], ["whitespace-only positional", " "], @@ -192,18 +214,23 @@ describe("missingClaudeCodeFlags", () => { // PR #141 review (codex + Copilot): the previous check compared the // raw input to the literal `arkor-project` string, so inputs like // `Arkor Project`, `arkor_project`, or a `[dir]` basename - // `Arkor_Project` were rejected because their trimmed/lowercased + // `Arkor.Project` were rejected because their trimmed/lowercased // raw form differed from the sanitised slug. They are deliberate // names, not silent defaults; the rewritten check (input contains // any `[a-z0-9]`) accepts them because the alphanumeric content - // proves the user typed a real name. + // proves the user typed a real name. Every entry below sanitises + // exactly to `arkor-project` (separator rewritten to `-`) which + // is what makes this test about the *fallback collision*; names + // with no separator (e.g. `ArkorProject` → `arkorproject`) are + // covered by the sibling "any alphanumeric content" test so the + // theme of each test stays honest. for (const name of [ "arkor-project", "ARKOR-PROJECT", " arkor-project ", "Arkor Project", "arkor_project", - "ArkorProject", + "Arkor.Project", ]) { const missing = missingClaudeCodeFlags({ requireProjectName: true, @@ -217,6 +244,25 @@ describe("missingClaudeCodeFlags", () => { } }); + it("accepts deliberate names whose sanitised slug is unrelated to `arkor-project`", () => { + // Sibling case to the fallback-collision test above: deliberate + // names that contain alphanumerics but have no separator (so + // `sanitise()` lowercases them straight through, producing + // `arkorproject` etc.) are also accepted. The check is "contains + // an alphanumeric", not "sanitised slug equals X". + for (const name of ["ArkorProject", "MyApp", "v2"]) { + const missing = missingClaudeCodeFlags({ + requireProjectName: true, + name, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + } + }); + it("counts --skip-install as satisfying the package-manager requirement", () => { const missing = missingClaudeCodeFlags({ template: "triage", diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index e176ed78..c4f7273b 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -33,12 +33,26 @@ export function isClaudeCode(): boolean { /** * Thrown by the CLI strict-mode validator after it has already written - * the human-readable missing-flags block to stderr. The outer bin.ts - * `catch` recognises this class and exits `1` *without* re-printing the - * message or its stack; the throw still unwinds through - * `program.parseAsync()` so the `finally` in `main()` runs (telemetry - * flush, deprecation notice, etc.), which a bare `process.exit(1)` - * inside the action handler would have skipped. + * the human-readable missing-flags block to stderr. Both consumers + * (`arkor` and `create-arkor`) catch this class at the outermost layer + * and exit `1` *without* re-printing the message or its stack; what + * each call path gains by throwing instead of `process.exit(1)`-ing + * inside the action handler differs: + * + * - `arkor`: the throw unwinds through `program.parseAsync()` so the + * `finally` in `main()` runs (telemetry flush, deprecation notice). + * A bare `process.exit(1)` inside the action would have skipped it. + * - `create-arkor`: there is no `main()` / telemetry path to preserve, + * but the outer `program.parseAsync(...).catch(...)` prefixes every + * non-sentinel error with `"create-arkor failed: "`. Recognising the + * sentinel lets the catch exit silently so that prefix does not + * double up on the multi-line missing-flags block already on stderr. + * + * Both consumers set `process.exitCode = 1` (rather than calling + * `process.exit(1)`) so Node lets the event loop drain naturally before + * exiting; on piped stdio that prevents truncating queued writes, which + * on the strict-mode path would lose the trailing lines of the + * missing-flags block. */ export class ClaudeCodeStrictExit extends Error { constructor() { @@ -113,24 +127,22 @@ export interface MissingClaudeCodeFlag { * pick up the current directory's name at runtime. */ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { + // Reject an empty / whitespace-only positional even when `--name` + // is set, because `create-arkor`'s runtime uses `opts.dir` for the + // **target directory** (not just the project name): `resolve("")` + // returns `process.cwd()`, so `create-arkor "" --name my-app + // --template ...` would scaffold *in-place* under the parent dir + // instead of into `./my-app/`. Strict mode should surface that + // mis-input, not let `--name` mask it. Whitespace-only inputs + // (`" "`, `"\t"`) survive `resolve()` as a whitespace-basename + // path and would also be caught by the alphanumeric check below, + // but the early trim guard handles both shapes uniformly without + // depending on downstream regex behaviour. `.` / `./` etc. still + // pass because they are deliberate "scaffold in this directory" + // idioms with the same runtime semantics as any non-empty path. + if (opts.dir !== undefined && opts.dir.trim() === "") return false; if (opts.name !== undefined) return /[a-z0-9]/i.test(opts.name); if (opts.dir === undefined) return false; - // Reject empty / whitespace-only positionals before consulting - // `resolve()`. The motivating case is the **empty string**: - // `path.resolve("")` returns `process.cwd()` whose basename is - // almost always alphanumeric, so `create-arkor "" --template ...` - // (or a quoted shell variable that expanded to empty) would slip - // through strict mode and scaffold in-place against the cwd - // basename, the exact silent-default outcome we already reject - // for `--name ""`. Whitespace-only inputs (`" "`, `"\t"`) would - // *also* survive `resolve()` as a path with a whitespace basename, - // and the alphanumeric check below would catch them on its own; - // the trim here is the cheaper, earlier guard that gives both - // shapes the same rejection without depending on the downstream - // regex. `.` / `./` etc. still pass because they are deliberate - // "scaffold in this directory" idioms with the same runtime - // semantics as any non-empty path. - if (opts.dir.trim() === "") return false; return /[a-z0-9]/i.test(basename(resolve(opts.dir))); } diff --git a/packages/cli-internal/src/sanitise.test.ts b/packages/cli-internal/src/sanitise.test.ts index a289e107..f029aeda 100644 --- a/packages/cli-internal/src/sanitise.test.ts +++ b/packages/cli-internal/src/sanitise.test.ts @@ -26,10 +26,11 @@ describe("sanitise", () => { it("stays linear on adversarial dash-heavy inputs (CodeQL hardening)", () => { // CodeQL's "polynomial regex on uncontrolled data" query flagged - // the previous chain once `claude-code.ts` started feeding it - // CLI-arg input. The original `+`-quantified replaces were already - // linear in practice (this isn't a real ReDoS regression test), - // but a 100k-character pathological input is a cheap belt-and- + // the previous chain because `sanitise()` sits on the + // `--name` / `[dir]` data flow from both CLI scaffolders. The + // original `+`-quantified replaces were already linear in practice + // (this isn't a real ReDoS regression test), but a 100k-character + // pathological input is a cheap belt-and- // suspenders check that the rewritten chain remains linear, and // anchors the alert resolution so a future rewrite that // accidentally introduced an ambiguous quantifier would trip the diff --git a/packages/cli-internal/src/sanitise.ts b/packages/cli-internal/src/sanitise.ts index 54ba07fa..36a753a0 100644 --- a/packages/cli-internal/src/sanitise.ts +++ b/packages/cli-internal/src/sanitise.ts @@ -9,13 +9,15 @@ * The original chain was three separate replaces (`/[^a-z0-9-]/g`, * `/-+/g`, `/^-+|-+$/g`), each a linear character-class or literal * scan. CodeQL's "polynomial-regex on uncontrolled data" query - * conservatively flagged the chain once `claude-code.ts` started - * feeding it CLI-arg input, even though no pattern actually - * backtracks. Rather than annotate a suppression, the chain was - * rewritten into a single negated-class collapse plus an anchored - * trim: it is provably linear (each input character consumed a - * constant number of times) and shorter, which removes the alert - * without losing readability. + * conservatively flagged the chain even though no pattern actually + * backtracks; the alert tracks any `+`-quantified regex applied to + * CLI-arg input, and this function has been on that data flow ever + * since `arkor init` / `create-arkor` started running it on + * `--name`, `[dir]` basenames, and prompt values. Rather than annotate + * a suppression, the chain was rewritten into a single negated-class + * collapse plus an anchored trim: it is provably linear (each input + * character consumed a constant number of times) and shorter, which + * removes the alert without losing readability. * * Trim runs *after* `.slice(0, 60)` so an alphanumeric-then-separator * input that gets cut on the separator (e.g. 59 letters + `_b` after From 316642b48c77dd7fc71e3578af6e94fc67b18683 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sat, 16 May 2026 17:15:16 +0900 Subject: [PATCH 12/16] fix: validate explicit --name in arkor init strict mode and harden related paths - claude-code: under requireProjectName=false, treat an explicit --name as a deliberate commitment so garbage values (e.g. "!!!") no longer slip through sanitise() to the `arkor-project` fallback; omitting --name still falls back to basename(cwd). - e2e/spawn-cli: drop inherited CLAUDECODE case-insensitively so Windows env-name semantics can't reintroduce strict mode for unrelated assertions. - create-arkor/bin: keep process.exitCode for the strict-mode path (drain piped stderr) but use process.exit(1) for generic failures so a lingering clack.spinner interval can't stall the exit. - sanitise.test: drop the wall-clock threshold on the 100k-dash case; keep it as an output-correctness anchor for the CodeQL ReDoS alert, leaving timeout enforcement to vitest. - docs: add CLAUDECODE=1 row to cli/overview env-var table (EN+JA); AGENTS.md notes full-width JP parens are preserved in Mintlify slugs. --- AGENTS.md | 9 ++-- docs/cli/overview.mdx | 1 + docs/ja/cli/overview.mdx | 1 + e2e/cli/src/arkor-init.test.ts | 29 +++++++++++ e2e/cli/src/spawn-cli.ts | 27 ++++++---- packages/cli-internal/src/claude-code.test.ts | 42 ++++++++++++++++ packages/cli-internal/src/claude-code.ts | 49 +++++++++++++++---- packages/cli-internal/src/sanitise.test.ts | 20 ++++---- packages/create-arkor/src/bin.ts | 20 ++++---- 9 files changed, 155 insertions(+), 43 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 801b4130..b3e4da4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,12 +100,13 @@ Don't split these into "docs in a follow-up PR" or "tests later" — land them i ## Non-obvious gotchas - **Docs CLI CodeGroup convention.** Pages under `/cli/*` and `/guides/cli/*` (and their JA mirrors) show unscripted subcommands (`arkor init`, `arkor login`, `arkor logout`, `arkor whoami`) as a 5-tab CodeGroup in the form ` arkor `: `pnpm arkor init`, `npm arkor init`, `yarn arkor init`, `yarn run arkor init` (yarn-berry needs `run`), `bun arkor init`. The `npm` tab stays as `npm arkor init` even though npm itself does not run package binaries via `npm `. These commands are intended to be run after `arkor` is already in the project's `package.json` (post `create-arkor`, or post manual `pnpm i arkor` / equivalent), and the 5-tab form is about consistent presentation per package manager, not about being copy-pasteable for users without a project. Do **not** rewrite the `npm` tab to `npx arkor ` or `npm exec arkor `, and do not act on review comments (human or bot) saying "`npm arkor init` does not work": that comment misses the premise. -- **Mintlify heading slugs are not GitHub-style; verify with curl.** The common assumption (`github-slugger`) is that all punctuation gets stripped from heading ids. Mintlify's actual behaviour is mixed: some punctuation is **preserved** in the id (`=`, `/`), some is **stripped without replacement** (parens `(` `)`, backticks `` ` ``, brackets), and spaces become `-` as usual. Verified examples in the rendered preview at the time of [PR #141](https://github.com/arkorlab/arkor/pull/141): +- **Mintlify heading slugs are not GitHub-style; verify with curl.** The common assumption (`github-slugger`) is that all punctuation gets stripped from heading ids. Mintlify's actual behaviour is mixed: some punctuation is **preserved** in the id (`=`, `/`, and the full-width JP parens `（` `）`), some is **stripped without replacement** (ASCII parens `(` `)`, backticks `` ` ``, brackets), and spaces become `-` as usual. Verified examples in the rendered preview at the time of [PR #141](https://github.com/arkorlab/arkor/pull/141): - `### CI / non-interactive shells` → `id="ci-/-non-interactive-shells"` (slash kept, slash-flanking spaces still become hyphens, hence `-/-`) - - `` ### Claude Code (`CLAUDECODE=1`) strict mode `` → `id="claude-code-claudecode=1-strict-mode"` (parens + backticks stripped, `=` kept, surrounding spaces become hyphens) - - `### Running under Claude Code (` `` `CLAUDECODE=1` `` `)` → `id="running-under-claude-code-claudecode=1"` + - `` ### Claude Code (`CLAUDECODE=1`) strict mode `` → `id="claude-code-claudecode=1-strict-mode"` (ASCII parens + backticks stripped, `=` kept, surrounding spaces become hyphens) + - `` ### Running under Claude Code (`CLAUDECODE=1`) `` → `id="running-under-claude-code-claudecode=1"` + - `` ### Claude Code（`CLAUDECODE=1`）厳格モード `` (JA) → `id="claude-code（claudecode=1）厳格モード"` (full-width parens kept verbatim, backticks stripped, `=` kept) - Cross-page links in `docs/` must therefore mirror the actual rendered id, which sometimes preserves `/` and `=` and sometimes strips parens / backticks. Always confirm before adding or editing an anchor link: `curl -s "/" | grep -oE 'id="[^"]*[^"]*"'`. Do not act on review comments (human or bot) claiming "Mintlify strips punctuation in headings" as a universal rule: Copilot in particular gets this wrong repeatedly by extrapolating from `github-slugger`, and has even cited the pre-existing broken `#ci--non-interactive-shells` link as supposed evidence. PR #141 fixed that broken link to `#ci-/-non-interactive-shells`, and the surviving anchors in `docs/cli/overview.mdx` and the strict-mode sections of `docs/quickstart.mdx` / `docs/cli/init.mdx` are the canonical examples to copy from. + Cross-page links in `docs/` must therefore mirror the actual rendered id, which preserves `/`, `=`, and full-width parens but strips ASCII parens and backticks. Always confirm before adding or editing an anchor link: `curl -s "/" | grep -oE 'id="[^"]*[^"]*"'`. Do not act on review comments (human or bot) claiming "Mintlify strips punctuation in headings" as a universal rule: Copilot in particular gets this wrong repeatedly by extrapolating from `github-slugger`, and has even cited the pre-existing broken `#ci--non-interactive-shells` link as supposed evidence. PR #141 fixed that broken link to `#ci-/-non-interactive-shells`, and the surviving anchors in `docs/cli/overview.mdx` and the strict-mode sections of `docs/quickstart.mdx` / `docs/cli/init.mdx` (plus their JA mirrors) are the canonical examples to copy from. - **Don't call a HuggingFace model name "non-existent"** based on training-data alone. Templates reference real models (e.g. `unsloth/gemma-4-E4B-it`) that may post-date Claude's knowledge cutoff. Verify (e.g. `WebFetch`) before flagging in issues or PR comments. If unverifiable, hedge ("could not confirm") rather than asserting absence. - **Generated files** copied into package dirs are gitignored: `packages/*/CONTRIBUTING.md` (from root), `packages/arkor/docs/` (from root `docs/`). Edit the source under repo root, not the copies. - **Node version**: published packages declare `engines.node >=22.22.0` (raised from 22.6 to dodge the [Jan 2026 async-hooks DoS CVE](https://nodejs.org/en/blog/vulnerability/january-2026-dos-mitigation-async-hooks)). Use Node 24 (latest preferred) for development per [CONTRIBUTING.md](CONTRIBUTING.md). diff --git a/docs/cli/overview.mdx b/docs/cli/overview.mdx index 1520534d..1fbf2933 100644 --- a/docs/cli/overview.mdx +++ b/docs/cli/overview.mdx @@ -33,6 +33,7 @@ The `arkor` CLI is the command surface around an Arkor project. The [scaffolder] | `ARKOR_TELEMETRY_DISABLED=1` | Same as `DO_NOT_TRACK`. Provided as an Arkor-specific alias for setups that already gate other tools on `DO_NOT_TRACK` and want a separate switch. | | `ARKOR_TELEMETRY_DEBUG=1` | Enable verbose internal logging from the telemetry module (init failures, identity-file read/write errors, capture failures, shutdown errors). Telemetry **is still sent** when this flag is on; this is a diagnostic aid, not a dry-run switch. To stop sending events, use `DO_NOT_TRACK=1` or `ARKOR_TELEMETRY_DISABLED=1`. | | `CI=1` (or any non-TTY stdout) | Force non-interactive mode. `arkor init` skips its prompts and uses the values you passed via flags (or built-in defaults under `--yes`); git init runs without asking when `--git` or `--yes` is set, and is skipped silently otherwise. See [`arkor init` § CI / non-interactive shells](/cli/init#ci-/-non-interactive-shells). | +| `CLAUDECODE=1` | Strict mode for AI agents that spawn the CLI without being able to answer interactive prompts. `arkor init` (and `create-arkor`) refuse to run unless every curated flag is present, printing a multi-line stderr block with the suggested re-invocation; pass `-y`/`--yes` to opt back into "accept all defaults". See [`arkor init` § Claude Code (`CLAUDECODE=1`) strict mode](/cli/init#claude-code-claudecode=1-strict-mode). | ### On the roadmap diff --git a/docs/ja/cli/overview.mdx b/docs/ja/cli/overview.mdx index 3eae7fc7..6422275a 100644 --- a/docs/ja/cli/overview.mdx +++ b/docs/ja/cli/overview.mdx @@ -33,6 +33,7 @@ description: "arkor CLI: 全コマンドリファレンス。" | `ARKOR_TELEMETRY_DISABLED=1` | `DO_NOT_TRACK` と同じ。`DO_NOT_TRACK` で他ツールをゲートしているがスイッチを分けたい構成のための Arkor 専用エイリアス。 | | `ARKOR_TELEMETRY_DEBUG=1` | テレメトリーモジュール内部の冗長ログを有効化（init 失敗、ID ファイルの read/write エラー、capture 失敗、shutdown エラー）。このフラグが ON でも **テレメトリーは送信されます**。診断補助で、dry-run スイッチではありません。送信を止めるには `DO_NOT_TRACK=1` か `ARKOR_TELEMETRY_DISABLED=1` を使ってください。 | | `CI=1`（または stdout が非 TTY） | 非対話モードを強制。`arkor init` はプロンプトをスキップしてフラグで渡された値（`--yes` 時は組み込みのデフォルト）を使い、`--git` か `--yes` 指定時は git init が確認なしで走り、それ以外では黙ってスキップされます。[`arkor init` § CI / 非対話シェル](/ja/cli/init#ci-/-非対話シェル) を参照。 | +| `CLAUDECODE=1` | 対話プロンプトに応答できない AI エージェント（Claude Code 等）向けの厳格モード。`arkor init`（および `create-arkor`）は curated フラグセットがすべて揃わない限り実行を拒否し、再実行用コマンドを stderr に複数行で出力します。「すべてデフォルトで進める」に戻すには `-y`/`--yes` を渡します。[`arkor init` § Claude Code（`CLAUDECODE=1`）厳格モード](/ja/cli/init#claude-code（claudecode=1）厳格モード) を参照。 | ### ロードマップ上 diff --git a/e2e/cli/src/arkor-init.test.ts b/e2e/cli/src/arkor-init.test.ts index 829f7f60..06f80e6e 100644 --- a/e2e/cli/src/arkor-init.test.ts +++ b/e2e/cli/src/arkor-init.test.ts @@ -264,6 +264,35 @@ describe("arkor init (E2E)", () => { expect(existsSync(join(cwd, "package.json"))).toBe(false); }); + it("rejects explicit garbage --name even though strict mode does not require a name", async () => { + // PR #141 review (Copilot): under strict mode `arkor init` does + // not require `--name` (the runtime derives it from + // `basename(cwd)`), but if the agent went out of its way to + // pass `--name "!!!"` strict mode used to ignore it and let + // `sanitise()` quietly collapse the value to `arkor-project`. + // Now an explicit `--name` is validated regardless of the + // `requireProjectName` branch. + const result = await runCli( + ARKOR_BIN, + [ + "init", + "--name", + "!!!", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + cwd, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(1); + expect(result.stderr).toContain("--name "); + // Sanity: nothing was scaffolded. + expect(existsSync(join(cwd, "package.json"))).toBe(false); + }); + it("runs to completion when every required flag is set", async () => { const result = await runCli( ARKOR_BIN, diff --git a/e2e/cli/src/spawn-cli.ts b/e2e/cli/src/spawn-cli.ts index 34cfb806..9a2277d4 100644 --- a/e2e/cli/src/spawn-cli.ts +++ b/e2e/cli/src/spawn-cli.ts @@ -260,6 +260,18 @@ function runCliOnce( if (lower.startsWith("npm_config_") || lower.startsWith("pnpm_config_")) { continue; } + // Strip CLAUDECODE case-insensitively for the same Windows reason + // as the npm-config / pnpm-config keys above: env-var names are + // case-insensitive on Windows (CreateProcessW deduplicates by + // uppercased key), so an inherited `claudecode` / `ClaudeCode` + // could coexist with our explicit `CLAUDECODE` override below and + // be picked nondeterministically, defeating the strict-mode + // opt-out we rely on for non-strict-mode tests. Tests that need + // to opt INTO strict mode still set `CLAUDECODE` via `extraEnv`, + // which is spread last so it wins. + if (lower === "claudecode") { + continue; + } cleanEnv[key] = value; } return new Promise((resolve, reject) => { @@ -280,16 +292,11 @@ function runCliOnce( env: { ...cleanEnv, CI: "1", - // The CLI's CLAUDECODE strict mode hard-fails on any invocation - // that doesn't declare every interactive-equivalent flag. When - // the e2e suite itself is run from a Claude Code session, - // `process.env.CLAUDECODE` is `"1"` in the parent and leaks - // through `cleanEnv` to every spawned child, which then turns - // unrelated assertions (e.g. tests that only set `--name` + - // `--template`) into surprise exit-1s. Clear it here so the - // default test environment matches a vanilla CI shell; the - // CLAUDECODE-specific tests opt back in via `extraEnv`. - CLAUDECODE: "", + // No explicit `CLAUDECODE` override here: any inherited + // entry (any case variant) has already been dropped from + // `cleanEnv` above so the default test environment matches a + // vanilla CI shell. CLAUDECODE-specific tests opt in via + // `extraEnv`, which is spread last. npm_config_user_agent: "", GIT_AUTHOR_NAME: "Arkor E2E", GIT_AUTHOR_EMAIL: "e2e@arkor.test", diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index 9fc6d671..e0cd3dd4 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -125,6 +125,48 @@ describe("missingClaudeCodeFlags", () => { ); }); + it.each([ + ["empty --name", ""], + ["whitespace --name", " "], + ["punctuation-only --name", "!!!"], + ])( + "in init mode (requireProjectName=false), still rejects %s because the validator surfaces explicit-but-garbage names", + (_label, name) => { + // PR #141 review (Copilot): under `requireProjectName: false` + // the old check skipped the helper entirely, so + // `arkor init --name "!!!" --template ...` passed strict mode + // and `runInit` silently sanitised the value to the generic + // `arkor-project` fallback. The validator now treats an + // explicit `--name` as a deliberate commitment regardless of + // `requireProjectName`, and asks the agent to pick a value with + // at least one ASCII letter or digit (or drop `--name`). + const missing = missingClaudeCodeFlags({ + requireProjectName: false, + name, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain("--name "); + }, + ); + + it("in init mode, accepts the absence of --name (runtime uses basename(cwd))", () => { + // Mirror of the test above: omitting `--name` is the documented + // default path for `arkor init`, so strict mode must NOT demand + // it. Pin this branch so a regression that forces `--name` + // everywhere is caught immediately. + const missing = missingClaudeCodeFlags({ + requireProjectName: false, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + }); + it("rejects empty positional even when --name is set (positional drives the runtime target dir)", () => { // PR #141 review (Copilot): `create-arkor` uses `opts.dir` for the // *target directory*, not just the slug. So diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index c4f7273b..49dc7079 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -49,10 +49,14 @@ export function isClaudeCode(): boolean { * double up on the multi-line missing-flags block already on stderr. * * Both consumers set `process.exitCode = 1` (rather than calling - * `process.exit(1)`) so Node lets the event loop drain naturally before - * exiting; on piped stdio that prevents truncating queued writes, which - * on the strict-mode path would lose the trailing lines of the - * missing-flags block. + * `process.exit(1)`) **for the strict-mode path specifically** so Node + * lets the event loop drain naturally before exiting; on piped stdio + * that prevents truncating queued writes, which on the strict-mode + * path would lose the trailing lines of the missing-flags block. + * Other failure paths in `create-arkor` (where `run()` may have + * started a `clack.spinner()`) still use `process.exit(1)` so a + * lingering UI interval can't keep the event loop alive past the + * catch. */ export class ClaudeCodeStrictExit extends Error { constructor() { @@ -141,7 +145,18 @@ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { // pass because they are deliberate "scaffold in this directory" // idioms with the same runtime semantics as any non-empty path. if (opts.dir !== undefined && opts.dir.trim() === "") return false; + // An explicitly-passed `--name` is validated even for `arkor init` + // (where `requireProjectName` is false). The reasoning: when the + // agent went out of its way to write `--name `, that value + // is its intended project name, so if `value` would collapse to + // the silent `arkor-project` fallback inside `sanitise()` the + // mis-input should surface here instead of being masked. if (opts.name !== undefined) return /[a-z0-9]/i.test(opts.name); + // No `--name` at all: only `create-arkor` (requireProjectName=true) + // needs a meaningful `[dir]`; `arkor init` falls back to `basename(cwd)` + // at runtime, which is the same value its interactive prompt would + // have offered, so strict mode does not require an explicit name there. + if (!opts.requireProjectName) return true; if (opts.dir === undefined) return false; return /[a-z0-9]/i.test(basename(resolve(opts.dir))); } @@ -158,12 +173,26 @@ export function missingClaudeCodeFlags( ): MissingClaudeCodeFlag[] { if (opts.yes) return []; const missing: MissingClaudeCodeFlag[] = []; - if (opts.requireProjectName && !hasMeaningfulProjectName(opts)) { - missing.push({ - flag: "[dir] (e.g. `my-arkor-app`) or --name ", - description: - "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Inputs with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!` / `***`) are rejected because they would collapse to the generic `arkor-project` fallback inside `sanitise()`.", - }); + if (!hasMeaningfulProjectName(opts)) { + // The two consumers want slightly different prompts: `create-arkor` + // is the only one that demands an explicit name (positional or + // `--name`), so its message lists both. `arkor init` falls back to + // `basename(cwd)` when `--name` is unset, but if the agent did pass + // `--name ` strict mode still trips here; surface it as + // a name-only ask rather than asking for a `[dir]` it doesn't have. + if (opts.requireProjectName) { + missing.push({ + flag: "[dir] (e.g. `my-arkor-app`) or --name ", + description: + "Project directory (positional) and the `package.json` name. Without `--name`, the basename of `[dir]` is used. Inputs with no ASCII letter or digit (empty, whitespace-only, or punctuation-only like `!!!` / `***`) are rejected because they would collapse to the generic `arkor-project` fallback inside `sanitise()`.", + }); + } else { + missing.push({ + flag: "--name ", + description: + "The `--name` you passed contains no ASCII letter or digit, so `sanitise()` would collapse it to the generic `arkor-project` fallback. Pick a value with at least one alphanumeric character, or drop `--name` entirely to let `arkor init` derive the name from `basename(cwd)`.", + }); + } } if (opts.template === undefined) { missing.push({ diff --git a/packages/cli-internal/src/sanitise.test.ts b/packages/cli-internal/src/sanitise.test.ts index f029aeda..80da6964 100644 --- a/packages/cli-internal/src/sanitise.test.ts +++ b/packages/cli-internal/src/sanitise.test.ts @@ -24,21 +24,21 @@ describe("sanitise", () => { expect(sanitise("---abc---")).toBe("abc"); }); - it("stays linear on adversarial dash-heavy inputs (CodeQL hardening)", () => { + it("returns the fallback on a 100k-char dash run (CodeQL hardening anchor)", () => { // CodeQL's "polynomial regex on uncontrolled data" query flagged // the previous chain because `sanitise()` sits on the // `--name` / `[dir]` data flow from both CLI scaffolders. The - // original `+`-quantified replaces were already linear in practice - // (this isn't a real ReDoS regression test), but a 100k-character - // pathological input is a cheap belt-and- - // suspenders check that the rewritten chain remains linear, and - // anchors the alert resolution so a future rewrite that - // accidentally introduced an ambiguous quantifier would trip the - // timeout instead of silently re-opening the CodeQL alert. + // original `+`-quantified replaces were already linear in + // practice, so this isn't a real ReDoS regression test; the + // 100k-char input is an output-correctness assertion that + // doubles as a smoke check that the rewritten chain still + // handles long, mostly-separator inputs (which was the alert's + // pathological shape) without any change in observable result. + // Vitest's per-file timeout catches a truly exponential rewrite + // for free, so no fixed wall-clock threshold is asserted here + // (PR #141 review: those make the suite flaky on overloaded CI). const adversarial = "-".repeat(100_000); - const start = Date.now(); expect(sanitise(adversarial)).toBe("arkor-project"); - expect(Date.now() - start).toBeLessThan(1_000); }); it("falls back to `arkor-project` when the result would be empty", () => { diff --git a/packages/create-arkor/src/bin.ts b/packages/create-arkor/src/bin.ts index 85b92e11..6a4cd184 100644 --- a/packages/create-arkor/src/bin.ts +++ b/packages/create-arkor/src/bin.ts @@ -452,15 +452,17 @@ program program.parseAsync(process.argv).catch((err) => { // The strict-mode validator throws this sentinel after writing the - // missing-flags block; exit silently so the prefix below doesn't - // double up on the already-printed message. + // missing-flags block; exit silently so the `create-arkor failed:` + // prefix below doesn't double up on the already-printed message. // - // Both branches set `process.exitCode` (rather than calling - // `process.exit(1)`) so Node lets the event loop drain naturally - // before exiting. On piped stdio, `process.exit()` is synchronous - // and can truncate queued writes; the strict-mode message is a - // multi-line block that agents need to read to re-invoke, so - // dropping its tail would defeat the whole point. + // Use `process.exitCode` for the strict path (not `process.exit`): + // Node then lets the event loop drain naturally so the multi-line + // stderr block flushes on piped stdio. Generic failures still go + // through `process.exit(1)` because `run()` may have started a + // `clack.spinner()` whose internal interval would otherwise keep + // the event loop alive past the catch and stall the exit; a forced + // exit is safer there than waiting for unknown UI resources to + // tidy up (PR #141 review). if (err instanceof ClaudeCodeStrictExit) { process.exitCode = 1; return; @@ -468,5 +470,5 @@ program.parseAsync(process.argv).catch((err) => { process.stderr.write( `create-arkor failed: ${err instanceof Error ? err.message : String(err)}\n`, ); - process.exitCode = 1; + process.exit(1); }); From 96239aee5df5369e0b02240d697159d61010b80b Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sat, 16 May 2026 22:22:48 +0900 Subject: [PATCH 13/16] feat: enhance strict mode validation in arkor init to reject non-alphanumeric cwd basenames and improve project name handling --- e2e/cli/src/arkor-init.test.ts | 29 ++++++++++++++ packages/arkor/src/cli/main.ts | 11 ++++-- packages/cli-internal/src/claude-code.test.ts | 38 +++++++++++++++++++ packages/cli-internal/src/claude-code.ts | 28 +++++++++++--- packages/cli-internal/src/sanitise.ts | 21 ++++++---- 5 files changed, 111 insertions(+), 16 deletions(-) diff --git a/e2e/cli/src/arkor-init.test.ts b/e2e/cli/src/arkor-init.test.ts index 06f80e6e..74b3b705 100644 --- a/e2e/cli/src/arkor-init.test.ts +++ b/e2e/cli/src/arkor-init.test.ts @@ -264,6 +264,35 @@ describe("arkor init (E2E)", () => { expect(existsSync(join(cwd, "package.json"))).toBe(false); }); + it("rejects an arkor init run whose cwd basename has no alphanumerics (would silently fall back to `arkor-project`)", async () => { + // PR #141 review (Copilot): `arkor init` without `--name` + // derives the project name from `basename(process.cwd())`. In + // a directory like `!!!`, runtime would sanitise that to the + // generic `arkor-project` fallback, which is exactly the + // silent default strict mode is meant to surface for explicit + // `--name` values. Strict mode now validates `basename(cwd)` + // through the same alphanumeric check. + const punctDir = join(cwd, "!!!"); + mkdirSync(punctDir, { recursive: true }); + const result = await runCli( + ARKOR_BIN, + [ + "init", + "--template", + "triage", + "--skip-git", + "--skip-install", + "--no-agents-md", + ], + punctDir, + { CLAUDECODE: "1" }, + ); + expect(result.code).toBe(1); + expect(result.stderr).toContain("--name "); + // Sanity: nothing scaffolded. + expect(existsSync(join(punctDir, "package.json"))).toBe(false); + }); + it("rejects explicit garbage --name even though strict mode does not require a name", async () => { // PR #141 review (Copilot): under strict mode `arkor init` does // not require `--name` (the runtime derives it from diff --git a/packages/arkor/src/cli/main.ts b/packages/arkor/src/cli/main.ts index 21c10d86..d54b6637 100644 --- a/packages/arkor/src/cli/main.ts +++ b/packages/arkor/src/cli/main.ts @@ -109,10 +109,15 @@ export async function main(argv: string[]): Promise { useBun: opts.useBun, name: opts.name, agentsMd: agentsMdSpecified ? opts.agentsMd ?? true : undefined, - // arkor init operates on `process.cwd()`; basename(cwd) is a - // meaningful default name, so an explicit --name isn't - // required. + // arkor init operates on `process.cwd()`; basename(cwd) is + // the runtime default for the project name, so strict mode + // doesn't require an explicit `--name`. Passing `initCwd` + // lets the validator reject the pathological case where + // the cwd basename itself has no alphanumerics (e.g. + // `/tmp/!!!/`), which would otherwise sanitise to the + // generic `arkor-project` fallback. requireProjectName: false, + initCwd: process.cwd(), }); if (missing.length > 0) { process.stderr.write( diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index e0cd3dd4..952896bd 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -167,6 +167,44 @@ describe("missingClaudeCodeFlags", () => { expect(missing).toEqual([]); }); + it.each([ + ["punctuation-only cwd basename", "/tmp/!!!"], + ["whitespace-only cwd basename", "/tmp/ "], + ["empty cwd basename (filesystem root)", "/"], + ])( + "in init mode without --name, rejects %s because basename(cwd) would collapse to the fallback", + (_label, initCwd) => { + // PR #141 review (Copilot): `arkor init` without `--name` + // derives the project name from `basename(process.cwd())`. If + // the user runs the CLI from `/tmp/!!!/` or similar, strict + // mode used to return early (no `--name` to validate) and + // `runInit` silently sanitised the basename to the generic + // `arkor-project` fallback. The validator now mirrors the + // explicit `--name` check against `basename(initCwd)`. + const missing = missingClaudeCodeFlags({ + requireProjectName: false, + initCwd, + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing.map((m) => m.flag)).toContain("--name "); + }, + ); + + it("in init mode, accepts a meaningful cwd basename without --name", () => { + const missing = missingClaudeCodeFlags({ + requireProjectName: false, + initCwd: "/home/alice/my-arkor-app", + template: "triage", + skipGit: true, + useNpm: true, + agentsMd: false, + }); + expect(missing).toEqual([]); + }); + it("rejects empty positional even when --name is set (positional drives the runtime target dir)", () => { // PR #141 review (Copilot): `create-arkor` uses `opts.dir` for the // *target directory*, not just the slug. So diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index 49dc7079..db1a4f8f 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -102,6 +102,18 @@ export interface ClaudeCodeOptionsCheck { * project name deliberately. */ requireProjectName?: boolean; + /** + * The working directory `arkor init` would derive its default project + * name from (`basename(initCwd)`). Used only when `requireProjectName` + * is false and `--name` was not passed: the strict-mode validator + * runs the same alphanumeric check it applies to explicit `--name` + * values against this basename, so an agent running `arkor init` in a + * directory like `/tmp/!!!/` doesn't silently collapse to the + * `arkor-project` fallback. Callers pass `process.cwd()` here; we + * take it as a parameter rather than reading `process.cwd()` inside + * this helper so the function stays pure and easy to test. + */ + initCwd?: string; } export interface MissingClaudeCodeFlag { @@ -153,10 +165,16 @@ function hasMeaningfulProjectName(opts: ClaudeCodeOptionsCheck): boolean { // mis-input should surface here instead of being masked. if (opts.name !== undefined) return /[a-z0-9]/i.test(opts.name); // No `--name` at all: only `create-arkor` (requireProjectName=true) - // needs a meaningful `[dir]`; `arkor init` falls back to `basename(cwd)` - // at runtime, which is the same value its interactive prompt would - // have offered, so strict mode does not require an explicit name there. - if (!opts.requireProjectName) return true; + // needs a meaningful `[dir]`; `arkor init` falls back to + // `basename(initCwd)` at runtime. We still validate that basename so + // an agent running `arkor init` inside `/tmp/!!!/` doesn't silently + // collapse to the generic `arkor-project` fallback that strict mode + // rejects for explicit `--name` and `[dir]` values. Callers may omit + // `initCwd` (e.g. tests), in which case we conservatively accept. + if (!opts.requireProjectName) { + if (opts.initCwd === undefined) return true; + return /[a-z0-9]/i.test(basename(opts.initCwd)); + } if (opts.dir === undefined) return false; return /[a-z0-9]/i.test(basename(resolve(opts.dir))); } @@ -190,7 +208,7 @@ export function missingClaudeCodeFlags( missing.push({ flag: "--name ", description: - "The `--name` you passed contains no ASCII letter or digit, so `sanitise()` would collapse it to the generic `arkor-project` fallback. Pick a value with at least one alphanumeric character, or drop `--name` entirely to let `arkor init` derive the name from `basename(cwd)`.", + "Strict mode could not derive a meaningful project name. Either `--name` was passed but its value contains no ASCII letter or digit (so `sanitise()` would collapse it to the generic `arkor-project` fallback), or `--name` was omitted and the current directory's basename has no alphanumerics either. Pass `--name ` with at least one ASCII letter or digit.", }); } } diff --git a/packages/cli-internal/src/sanitise.ts b/packages/cli-internal/src/sanitise.ts index 36a753a0..9a7ef6d0 100644 --- a/packages/cli-internal/src/sanitise.ts +++ b/packages/cli-internal/src/sanitise.ts @@ -10,14 +10,19 @@ * `/-+/g`, `/^-+|-+$/g`), each a linear character-class or literal * scan. CodeQL's "polynomial-regex on uncontrolled data" query * conservatively flagged the chain even though no pattern actually - * backtracks; the alert tracks any `+`-quantified regex applied to - * CLI-arg input, and this function has been on that data flow ever - * since `arkor init` / `create-arkor` started running it on - * `--name`, `[dir]` basenames, and prompt values. Rather than annotate - * a suppression, the chain was rewritten into a single negated-class - * collapse plus an anchored trim: it is provably linear (each input - * character consumed a constant number of times) and shorter, which - * removes the alert without losing readability. + * backtracks; what the query keys off is the *combination* of + * `+`-quantified regexes running over the same CLI-arg input from + * `arkor init` / `create-arkor` (on `--name`, `[dir]` basenames, and + * prompt values), where a pathological all-`-` input is consumed by + * the literal-keeping `/[^a-z0-9-]/g` and then re-scanned by the + * collapsing `/-+/g`. A single `+`-quantified replace on its own is + * not flagged (the rewrite below still uses `/[^a-z0-9]+/g` and + * passes the query). Rather than annotate a suppression, the chain + * was rewritten into one negated-class collapse plus an anchored + * trim: a run of any non-alphanumeric characters becomes one `-` in a + * single linear pass (`-` is inside the negated class now, so the + * separate run-collapse step is gone), which removes the chained-pass + * shape the query flagged. * * Trim runs *after* `.slice(0, 60)` so an alphanumeric-then-separator * input that gets cut on the separator (e.g. 59 letters + `_b` after From 1f00216262ea8c4f9e467d5eb2189e768609ff3a Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sat, 16 May 2026 22:55:52 +0900 Subject: [PATCH 14/16] fix: improve strict mode handling in CLI and related documentation for better clarity and consistency --- docs/cli/init.mdx | 2 +- docs/ja/cli/init.mdx | 2 +- e2e/cli/src/spawn-cli.ts | 11 ++++++----- packages/cli-internal/src/claude-code.ts | 7 ++++++- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/cli/init.mdx b/docs/cli/init.mdx index 15591b22..e6157d75 100644 --- a/docs/cli/init.mdx +++ b/docs/cli/init.mdx @@ -124,7 +124,7 @@ If you forget `--yes` in a non-interactive shell, the command still completes (p ### Claude Code (`CLAUDECODE=1`) strict mode -Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `arkor init` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. Project name is intentionally not required here, because `arkor init` already derives it from `basename(cwd)`, which is the same value the interactive prompt would have suggested. The package-manager flag is required even though it isn't strictly a prompt, so the agent picks `--use-*` vs. `--skip-install` deliberately rather than relying on `npm_config_user_agent` detection. +Claude Code spawns child processes with `CLAUDECODE=1`, and it cannot answer interactive prompts. To prevent the agent from silently scaffolding with hidden defaults, `arkor init` flips into a strict mode under this env var: a curated set of flags becomes required (see the list below), and a missing one produces a stderr message listing the suggested re-invocation rather than running. Project name is intentionally not required here in the common case: `arkor init` derives it from `basename(cwd)`, which is the same value the interactive prompt would have suggested. The strict-mode validator still asks for `--name ` in two narrow cases that would otherwise silently fall back to the generic `arkor-project` slug, namely (a) when `--name` *was* passed but its value has no ASCII letter or digit (e.g. `--name "!!!"`) and (b) when `--name` was omitted and the current directory's basename has no ASCII letter or digit either (e.g. running `arkor init` from `/tmp/!!!/`). The package-manager flag is required even though it isn't strictly a prompt, so the agent picks `--use-*` vs. `--skip-install` deliberately rather than relying on `npm_config_user_agent` detection. Required flags (or pass `-y`/`--yes` to opt back into "accept all defaults"): diff --git a/docs/ja/cli/init.mdx b/docs/ja/cli/init.mdx index f34bdc16..8982a613 100644 --- a/docs/ja/cli/init.mdx +++ b/docs/ja/cli/init.mdx @@ -124,7 +124,7 @@ bun arkor init --yes --template triage --use-bun --skip-git ### Claude Code（`CLAUDECODE=1`）厳格モード -Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `arkor init` は厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。プロジェクト名は意図的に必須にしていません。`arkor init` は `basename(cwd)` から派生させており、これは対話プロンプトでのデフォルト提案値と同じだからです。パッケージマネージャーは厳密にはプロンプトではありません（UA 自動検出か検出失敗時の silent skip）が、`npm_config_user_agent` 任せにせず `--use-*` か `--skip-install` をエージェントが明示的に選ぶように、こちらは必須としています。 +Claude Code は子プロセスに `CLAUDECODE=1` を渡し、対話プロンプトに応答できません。エージェントが隠れたデフォルトで黙ってプロジェクトを生成してしまうのを防ぐため、この環境変数下では `arkor init` は厳格モードに切り替わります。下記のキュレーション済みフラグセットが必須となり、不足があれば実行ではなく、再実行用のコマンドを stderr に出して終了します。通常はプロジェクト名を必須にしていません。`arkor init` は `basename(cwd)` から派生させており、これは対話プロンプトでのデフォルト提案値と同じだからです。ただし、generic な `arkor-project` フォールバックに silent collapse してしまう次の 2 ケースは厳格モードでも `--name ` を要求します: (a) `--name` が渡されていてその値に ASCII 英数字が 1 つも含まれない (例: `--name "!!!"`)、(b) `--name` 省略時のカレントディレクトリ basename にも ASCII 英数字が含まれない (例: `/tmp/!!!/` から `arkor init` を実行)。パッケージマネージャーは厳密にはプロンプトではありません（UA 自動検出か検出失敗時の silent skip）が、`npm_config_user_agent` 任せにせず `--use-*` か `--skip-install` をエージェントが明示的に選ぶように、こちらは必須としています。 必須フラグ（または `-y`/`--yes` で「すべてデフォルトで進める」にオプトインしてください）: diff --git a/e2e/cli/src/spawn-cli.ts b/e2e/cli/src/spawn-cli.ts index 9a2277d4..0e3f1e08 100644 --- a/e2e/cli/src/spawn-cli.ts +++ b/e2e/cli/src/spawn-cli.ts @@ -264,11 +264,12 @@ function runCliOnce( // as the npm-config / pnpm-config keys above: env-var names are // case-insensitive on Windows (CreateProcessW deduplicates by // uppercased key), so an inherited `claudecode` / `ClaudeCode` - // could coexist with our explicit `CLAUDECODE` override below and - // be picked nondeterministically, defeating the strict-mode - // opt-out we rely on for non-strict-mode tests. Tests that need - // to opt INTO strict mode still set `CLAUDECODE` via `extraEnv`, - // which is spread last so it wins. + // could otherwise survive `cleanEnv` under a different JS key and + // reach the spawned CLI nondeterministically, flipping it into + // strict mode for tests that don't expect it. There is no default + // `CLAUDECODE` value in the env block below; tests that need to + // opt INTO strict mode set `CLAUDECODE` via `extraEnv`, which is + // spread last so it wins. if (lower === "claudecode") { continue; } diff --git a/packages/cli-internal/src/claude-code.ts b/packages/cli-internal/src/claude-code.ts index db1a4f8f..0e9e8c37 100644 --- a/packages/cli-internal/src/claude-code.ts +++ b/packages/cli-internal/src/claude-code.ts @@ -111,7 +111,12 @@ export interface ClaudeCodeOptionsCheck { * directory like `/tmp/!!!/` doesn't silently collapse to the * `arkor-project` fallback. Callers pass `process.cwd()` here; we * take it as a parameter rather than reading `process.cwd()` inside - * this helper so the function stays pure and easy to test. + * this helper so init-path tests can pin a deterministic cwd + * without `chdir`-ing the vitest worker. (The create-arkor branch + * still calls `resolve(opts.dir)` for its own check, which leans on + * the real `process.cwd()` for relative paths like `.`/`..`; this + * helper isn't fully cwd-free, just decoupled where the init path + * needs it most.) */ initCwd?: string; } From dde60c3cb66ad30852020146ee28061551866616 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sun, 17 May 2026 14:07:31 +0900 Subject: [PATCH 15/16] test: enhance CLAUDECODE environment handling in runCli tests for stricter validation --- e2e/cli/src/spawn-cli.test.ts | 92 +++++++++++++++++++ packages/cli-internal/src/claude-code.test.ts | 56 ++++++++--- 2 files changed, 133 insertions(+), 15 deletions(-) diff --git a/e2e/cli/src/spawn-cli.test.ts b/e2e/cli/src/spawn-cli.test.ts index 556385a0..fbe81ef7 100644 --- a/e2e/cli/src/spawn-cli.test.ts +++ b/e2e/cli/src/spawn-cli.test.ts @@ -443,3 +443,95 @@ describe("runCli orchestration", () => { expect(result.code).toBe(0); }); }); + +describe("runCli CLAUDECODE env scrub", () => { + // PR #141 review: when the vitest worker (or any parent shell) sets + // `CLAUDECODE=1`, `cleanEnv` must strip it case-insensitively before + // the child env is assembled so the spawned CLI doesn't surprise-flip + // into strict mode for tests that don't expect it. Tests that DO + // want strict mode opt back in via `extraEnv`, which is spread last. + let cwd: string; + const ORIG_ENV = { ...process.env }; + + beforeEach(() => { + cwd = mkdtempSync(join(tmpdir(), "spawn-cli-claudecode-")); + spawnMock.mockReset(); + // Make the canonical retry gate unreachable so a stray SIGKILL + // close-emit doesn't trigger a snapshot+restore branch that's + // irrelevant to this suite. + Object.defineProperty(process, "platform", { + value: "linux", + configurable: true, + }); + delete process.env.CI; + // Reset to a clean baseline; individual tests seed the variants + // they want to assert on. + delete process.env.CLAUDECODE; + delete process.env.claudecode; + delete process.env.ClaudeCode; + }); + + afterEach(() => { + rmSync(cwd, { recursive: true, force: true }); + // Restore the captured env so other suites in the same worker + // (notably the orchestration suite above) see their original state. + for (const key of ["CI", "CLAUDECODE", "claudecode", "ClaudeCode"]) { + const original = ORIG_ENV[key]; + if (original === undefined) delete process.env[key]; + else process.env[key] = original; + } + }); + + /** + * Resolve the env object passed to the (mocked) `spawn` call. The + * child is faked out with a clean close so `runCli` returns + * promptly; we only care about how the env was assembled. + */ + async function captureSpawnedEnv( + extraEnv?: NodeJS.ProcessEnv, + ): Promise { + spawnMock.mockImplementationOnce(() => + makeFakeChild({ code: 0, signal: null }), + ); + await runCli("/fake/bin", [], cwd, extraEnv); + expect(spawnMock).toHaveBeenCalledTimes(1); + const opts = spawnMock.mock.calls[0]![2] as { env: NodeJS.ProcessEnv }; + return opts.env; + } + + it("drops an inherited CLAUDECODE=1 from the spawned env by default", async () => { + process.env.CLAUDECODE = "1"; + const env = await captureSpawnedEnv(); + expect(env.CLAUDECODE).toBeUndefined(); + }); + + it.each(["claudecode", "ClaudeCode", "CLAUDECODE", "claudeCODE"])( + "drops case variant %s case-insensitively (Windows env-var safety)", + async (key) => { + // CreateProcessW dedupes env names case-insensitively on Windows, + // so any case spelling inherited from the parent must be + // stripped before it can collide with our intended state. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.env as any)[key] = "1"; + const env = await captureSpawnedEnv(); + // Every case variant has been removed from the spawned env. + for (const k of Object.keys(env)) { + expect(k.toLowerCase()).not.toBe("claudecode"); + } + }, + ); + + it("lets extraEnv opt INTO strict mode even when no inherited CLAUDECODE exists", async () => { + const env = await captureSpawnedEnv({ CLAUDECODE: "1" }); + expect(env.CLAUDECODE).toBe("1"); + }); + + it("extraEnv CLAUDECODE wins over an inherited value (spread last)", async () => { + // Inverse safety: even if the scrub somehow missed a variant, the + // `extraEnv` opt-in must still take precedence so the strict-mode + // tests can rely on it. + process.env.CLAUDECODE = "0"; + const env = await captureSpawnedEnv({ CLAUDECODE: "1" }); + expect(env.CLAUDECODE).toBe("1"); + }); +}); diff --git a/packages/cli-internal/src/claude-code.test.ts b/packages/cli-internal/src/claude-code.test.ts index 952896bd..8354f7bf 100644 --- a/packages/cli-internal/src/claude-code.test.ts +++ b/packages/cli-internal/src/claude-code.test.ts @@ -1,3 +1,6 @@ +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { formatClaudeCodeMissingMessage, @@ -266,6 +269,15 @@ describe("missingClaudeCodeFlags", () => { // strict check agree with the runtime: `.` / `..` resolve to the // current / parent directory's basename, and only then are // sanitised. + // + // The two `.`/`..` cases used to lean on `process.cwd()` directly, + // which made the test passively dependent on the worker checkout + // path having an alphanumeric basename (PR #141 review: a checkout + // under e.g. `/tmp/!!!/` would have falsely failed strict mode + // here). Pin a controlled cwd inside a fresh tmp dir whose + // basename ends in a guaranteed-alphanumeric prefix so the + // assertion exercises the `resolve()`-then-`basename()` derivation + // without depending on ambient state. const baseOpts = { requireProjectName: true, template: "triage", @@ -273,21 +285,35 @@ describe("missingClaudeCodeFlags", () => { useNpm: true, agentsMd: false, } as const; - // `process.cwd()` is the vitest worker's cwd; its basename is - // some non-empty slug (e.g. `cli-internal` here). The exact value - // doesn't matter as long as it isn't the `arkor-project` fallback. - expect(missingClaudeCodeFlags({ ...baseOpts, dir: "." })).toEqual([]); - expect(missingClaudeCodeFlags({ ...baseOpts, dir: "./" })).toEqual([]); - // `..` resolves to the parent directory (e.g. `packages` when - // vitest is run from `packages/cli-internal`), whose basename is - // also a meaningful slug. Pinning this case matches the test - // title's claim and guards the symmetric `.` / `..` branch in - // the `basename(resolve(opts.dir))` derivation. - expect(missingClaudeCodeFlags({ ...baseOpts, dir: ".." })).toEqual([]); - // Relative path that resolves to a meaningful basename also passes. - expect( - missingClaudeCodeFlags({ ...baseOpts, dir: "foo/bar/my-app" }), - ).toEqual([]); + const tmpRoot = mkdtempSync(join(tmpdir(), "arkor-strict-dot-")); + // `mkdtempSync` appends 6 random alphanumeric characters; the + // basename is therefore non-empty even if `tmpdir()` itself happens + // to land somewhere weird. Create a child so the `..` case also + // resolves to a known meaningful basename (`tmpRoot` itself). + const child = join(tmpRoot, "child"); + mkdirSync(child); + const ORIG_CWD = process.cwd(); + try { + process.chdir(child); + // `.` resolves to `child`, basename `child`. Alphanumeric. + expect(missingClaudeCodeFlags({ ...baseOpts, dir: "." })).toEqual([]); + expect(missingClaudeCodeFlags({ ...baseOpts, dir: "./" })).toEqual([]); + // `..` resolves to `tmpRoot`, basename starts with + // `arkor-strict-dot-` so it always satisfies the + // alphanumeric requirement regardless of which dir vitest + // happened to be launched from. + expect(missingClaudeCodeFlags({ ...baseOpts, dir: ".." })).toEqual([]); + // Relative path that resolves to a meaningful basename also + // passes. This case never depended on cwd-basename letters since + // the trailing segment (`my-app`) is what `basename(resolve(...))` + // returns, but pin it here too for parity. + expect( + missingClaudeCodeFlags({ ...baseOpts, dir: "foo/bar/my-app" }), + ).toEqual([]); + } finally { + process.chdir(ORIG_CWD); + rmSync(tmpRoot, { recursive: true, force: true }); + } }); it("accepts deliberate names that happen to sanitise to `arkor-project`", () => { From 66ea1acf962ff876170344c18c746121d99b9670 Mon Sep 17 00:00:00 2001 From: k-taro56 <121674121+k-taro56@users.noreply.github.com> Date: Sun, 17 May 2026 14:39:22 +0900 Subject: [PATCH 16/16] test: prevent CLAUDECODE scrub suite from leaking platform/env into later tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate the CLAUDECODE case variants into a single CLAUDECODE_VARIANTS list so before/after hooks can strip and restore them in lockstep — the previous list omitted `claudeCODE` and leaked it into subsequent tests. Also restore the original process.platform in afterEach so other suites in the same worker that assert on macOS-specific retry behaviour aren't mutated by the linux override. --- e2e/cli/src/spawn-cli.test.ts | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/e2e/cli/src/spawn-cli.test.ts b/e2e/cli/src/spawn-cli.test.ts index fbe81ef7..e11256a6 100644 --- a/e2e/cli/src/spawn-cli.test.ts +++ b/e2e/cli/src/spawn-cli.test.ts @@ -452,13 +452,27 @@ describe("runCli CLAUDECODE env scrub", () => { // want strict mode opt back in via `extraEnv`, which is spread last. let cwd: string; const ORIG_ENV = { ...process.env }; + const ORIG_PLATFORM = process.platform; + // Every CLAUDECODE case variant a test in this suite seeds: keeping + // them in one list lets the before/after loop strip and restore the + // full set without drifting from the `it.each` matrix below (PR #141 + // review: `claudeCODE` was missed from the previous cleanup and + // leaked `claudeCODE=1` into later tests). + const CLAUDECODE_VARIANTS = [ + "CLAUDECODE", + "claudecode", + "ClaudeCode", + "claudeCODE", + ] as const; beforeEach(() => { cwd = mkdtempSync(join(tmpdir(), "spawn-cli-claudecode-")); spawnMock.mockReset(); // Make the canonical retry gate unreachable so a stray SIGKILL // close-emit doesn't trigger a snapshot+restore branch that's - // irrelevant to this suite. + // irrelevant to this suite. `afterEach` restores the original + // platform so subsequent tests in the same worker (which may + // assert on macOS-specific retry behaviour) aren't mutated. Object.defineProperty(process, "platform", { value: "linux", configurable: true, @@ -466,20 +480,24 @@ describe("runCli CLAUDECODE env scrub", () => { delete process.env.CI; // Reset to a clean baseline; individual tests seed the variants // they want to assert on. - delete process.env.CLAUDECODE; - delete process.env.claudecode; - delete process.env.ClaudeCode; + for (const key of CLAUDECODE_VARIANTS) { + delete process.env[key]; + } }); afterEach(() => { rmSync(cwd, { recursive: true, force: true }); // Restore the captured env so other suites in the same worker // (notably the orchestration suite above) see their original state. - for (const key of ["CI", "CLAUDECODE", "claudecode", "ClaudeCode"]) { + for (const key of ["CI", ...CLAUDECODE_VARIANTS]) { const original = ORIG_ENV[key]; if (original === undefined) delete process.env[key]; else process.env[key] = original; } + Object.defineProperty(process, "platform", { + value: ORIG_PLATFORM, + configurable: true, + }); }); /**