From d8adabbe3357d3fc6e6204f206d1bb84fe43fecd Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 9 Sep 2026 19:02:44 +0200 Subject: [PATCH] refactor(cli): a --token travels as an argument, never through the process environment `plugin install --token` and `marketplace add --token` hand the flag to `createDeps`, which composes it into the auth reader every fetcher consults, ahead of `AIDD_TOKEN` and the stored credentials. Nothing writes the environment any more: the install use case loses its `Environment` and the port loses `set`. Two reads of the user's own `AIDD_TOKEN` remain, both in `runtime/auth`, both reads. Red first: the auth reader's explicit-token test failed with `expected 'env-token' to be 'flag-token'`, the marketplace wiring test with `expected "spy" to be called with arguments`. Closes #797 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011x4ms5qcGuZgYhCxfdHMUb AIDD-Session-Id: 4acc9a1c-19bc-4468-b8b6-e86644bcba60 --- .../plugin/plugin-install-use-case.ts | 6 +-- .../framework/domain/ports/environment.ts | 4 +- .../infrastructure/environment-adapter.ts | 8 +--- cli/src/presentation/commands/marketplace.ts | 3 +- cli/src/presentation/commands/plugin.ts | 3 +- cli/src/runtime/auth/auth-reader-adapter.ts | 8 +++- cli/src/runtime/wiring/framework.ts | 17 +++++--- .../orchestrator-deps.arch.test.ts | 2 +- .../plugin-install-use-case.unit.test.ts | 39 +------------------ .../helpers/ports/in-memory-environment.ts | 4 -- .../marketplace-wiring.integration.test.ts | 23 ++++++----- .../plugin-wiring.integration.test.ts | 16 ++++++-- .../auth/auth-reader.integration.test.ts | 29 ++++++++++++++ 13 files changed, 82 insertions(+), 80 deletions(-) diff --git a/cli/src/contexts/framework/application/plugin/plugin-install-use-case.ts b/cli/src/contexts/framework/application/plugin/plugin-install-use-case.ts index 0791d9da7..4a1360678 100644 --- a/cli/src/contexts/framework/application/plugin/plugin-install-use-case.ts +++ b/cli/src/contexts/framework/application/plugin/plugin-install-use-case.ts @@ -11,7 +11,6 @@ import type { PluginPick } from "../../../../presentation/prompts/plugin-pick-us import type { MarketplaceTrustStore } from "../../../distribution/domain/ports/marketplace-trust-store.js"; import { assertToolSupportsScope, type InstallScope } from "../../domain/install-scope.js"; import { parsePluginSpec } from "../../domain/plugins/installed-plugin.js"; -import type { Environment } from "../../domain/ports/environment.js"; import type { ManifestRepository } from "../../domain/ports/manifest-repository.js"; import type { PluginAdd } from "./plugin-add-use-case.js"; import type { PluginInstallFromMarketplace } from "./plugin-install-from-marketplace-use-case.js"; @@ -22,7 +21,6 @@ export interface PluginInstallOptions { projectRoot: string; interactive: boolean; fromMarketplace?: string; - token?: string; yes?: boolean; scope?: InstallScope; } @@ -39,8 +37,7 @@ export class PluginInstallUseCase { private readonly pluginInstallFromMarketplaceUseCase: PluginInstallFromMarketplace, private readonly manifestRepo: ManifestRepository, private readonly trustStore: MarketplaceTrustStore, - private readonly prompter: Prompter, - private readonly environment: Environment + private readonly prompter: Prompter ) {} async execute(options: PluginInstallOptions): Promise { @@ -109,7 +106,6 @@ export class PluginInstallUseCase { private async executeMarketplace(options: PluginInstallOptions): Promise { const { name, version } = parsePluginSpec(options.pluginArg as string); - if (options.token) this.environment.set("AIDD_TOKEN", options.token); const result = await this.pluginInstallFromMarketplaceUseCase.execute({ pluginName: name, version, diff --git a/cli/src/contexts/framework/domain/ports/environment.ts b/cli/src/contexts/framework/domain/ports/environment.ts index 107e51c56..462246a3c 100644 --- a/cli/src/contexts/framework/domain/ports/environment.ts +++ b/cli/src/contexts/framework/domain/ports/environment.ts @@ -1,6 +1,4 @@ -/** The ambient environment a use case reads a switch from, and publishes a token to. A port, so - * neither layer reaches a global: the composition root supplies what owns `process.env`. */ +/** The environment a use case reads a switch from; the composition root owns `process.env`. */ export interface Environment { get(name: string): string | undefined; - set(name: string, value: string): void; } diff --git a/cli/src/contexts/framework/infrastructure/environment-adapter.ts b/cli/src/contexts/framework/infrastructure/environment-adapter.ts index d87e7efb2..20bbdd99b 100644 --- a/cli/src/contexts/framework/infrastructure/environment-adapter.ts +++ b/cli/src/contexts/framework/infrastructure/environment-adapter.ts @@ -1,13 +1,9 @@ import type { Environment } from "../domain/ports/environment.js"; -/** Reads and writes at call time, never snapshotting at construction: an e2e run sets its - * switches in the child process it spawns, after this adapter exists. */ +/** Reads at call time, never snapshotting at construction: an e2e run sets its switches in + * the child process it spawns, after this adapter exists. */ export class EnvironmentAdapter implements Environment { get(name: string): string | undefined { return process.env[name]; } - - set(name: string, value: string): void { - process.env[name] = value; - } } diff --git a/cli/src/presentation/commands/marketplace.ts b/cli/src/presentation/commands/marketplace.ts index 2dfc2aa20..7efa3baae 100644 --- a/cli/src/presentation/commands/marketplace.ts +++ b/cli/src/presentation/commands/marketplace.ts @@ -67,9 +67,8 @@ export function registerMarketplaceCommand(program: Command): void { process.exit(1); } try { - if (cmdOptions.token) process.env.AIDD_TOKEN = cmdOptions.token; const scope: MarketplaceScope = cmdOptions.scope === "user" ? "user" : "project"; - const deps = await createDeps(projectRoot, { verbose }, output); + const deps = await createDeps(projectRoot, { verbose, token: cmdOptions.token }, output); const name = nameArg ?? (await deps.prompter.input("Marketplace name:")); const rawSource = sourceArg ?? (await deps.prompter.input("Source (path or user/repo):")); const source = parsePluginSourceShorthand(rawSource); diff --git a/cli/src/presentation/commands/plugin.ts b/cli/src/presentation/commands/plugin.ts index b6d1d11dc..ce5256c26 100644 --- a/cli/src/presentation/commands/plugin.ts +++ b/cli/src/presentation/commands/plugin.ts @@ -98,14 +98,13 @@ export function registerPluginCommand(program: Command): void { try { assertValidAiToolId(cmdOptions.tool); const scope = parseInstallScope(cmdOptions.scope); - const deps = await createDeps(projectRoot, { verbose }, output); + const deps = await createDeps(projectRoot, { verbose, token: cmdOptions.token }, output); const result = await deps.pluginInstallUseCase.execute({ pluginArg, toolIds: parseToolOption(cmdOptions.tool), projectRoot, interactive: process.stdout.isTTY, fromMarketplace: cmdOptions.from, - token: cmdOptions.token, yes: cmdOptions.yes, scope, }); diff --git a/cli/src/runtime/auth/auth-reader-adapter.ts b/cli/src/runtime/auth/auth-reader-adapter.ts index 5d497d348..1de24a142 100644 --- a/cli/src/runtime/auth/auth-reader-adapter.ts +++ b/cli/src/runtime/auth/auth-reader-adapter.ts @@ -19,7 +19,9 @@ export class AuthReaderAdapter implements TokenProvider { private readonly storage: AuthStorage, private readonly projectRoot: string, private readonly logger?: Logger, - private readonly externalProvider: TokenResolver = noopExternalProvider + private readonly externalProvider: TokenResolver = noopExternalProvider, + /** A token the command line carried: composed in, never published to the environment. */ + private readonly explicitToken?: string ) {} resolve(): Promise { @@ -28,6 +30,10 @@ export class AuthReaderAdapter implements TokenProvider { } private async resolveUncached(): Promise { + if (this.explicitToken) { + this.logger?.debug("Token given on the command line"); + return this.explicitToken; + } const envToken = process.env.AIDD_TOKEN; if (envToken) { this.logger?.debug("Token resolved from AIDD_TOKEN env"); diff --git a/cli/src/runtime/wiring/framework.ts b/cli/src/runtime/wiring/framework.ts index b17bcdfee..67c67b184 100644 --- a/cli/src/runtime/wiring/framework.ts +++ b/cli/src/runtime/wiring/framework.ts @@ -105,6 +105,7 @@ import { createFrameworkBuildUseCase } from "./translate.js"; interface GlobalOptions { verbose: boolean; + token?: string; } interface Deps extends TelemetryDeps { @@ -182,7 +183,8 @@ export async function createDeps( options: GlobalOptions, output?: CLIOutput ): Promise { - const cached = _cache.get(projectRoot); + const cacheKey = `${projectRoot}\u0000${options.token ?? ""}`; + const cached = _cache.get(cacheKey); if (cached !== undefined) return cached; const hasher = new HasherAdapter(); const logger = output ?? new CLIOutput(options.verbose); @@ -193,7 +195,13 @@ export async function createDeps( const http = new HttpClient(); const authStorage = new AuthStorage(); const ghCliAdapter = new GhCliAdapter(); - const authReader = new AuthReaderAdapter(authStorage, projectRoot, logger, ghCliAdapter); + const authReader = new AuthReaderAdapter( + authStorage, + projectRoot, + logger, + ghCliAdapter, + options.token + ); const credentialStore = new AuthProviderAdapter( authStorage, new Map([["gh", ghCliAdapter]]), @@ -358,8 +366,7 @@ export async function createDeps( pluginInstallFromMarketplaceUseCase, manifestRepo, marketplaceTrustStore, - prompter, - environment + prompter ); const installAiToolUseCase = new InstallAiToolUseCase( installRuntimeConfigUseCase, @@ -575,6 +582,6 @@ export async function createDeps( listInstalledRulesUseCase, checkUpdateUseCase, }; - _cache.set(projectRoot, deps); + _cache.set(cacheKey, deps); return deps; } diff --git a/cli/tests/architecture/orchestrator-deps.arch.test.ts b/cli/tests/architecture/orchestrator-deps.arch.test.ts index b58ffe356..47d95749b 100644 --- a/cli/tests/architecture/orchestrator-deps.arch.test.ts +++ b/cli/tests/architecture/orchestrator-deps.arch.test.ts @@ -56,7 +56,7 @@ const BASELINE: readonly { readonly path: string; readonly injected: number }[] { path: "src/contexts/telemetry/application/read-local-cost-use-case.ts", injected: 7 }, { path: "src/contexts/telemetry/application/report-cost-use-case.ts", injected: 7 }, { path: "src/contexts/framework/application/install/install-ide-tool-use-case.ts", injected: 6 }, - { path: "src/contexts/framework/application/plugin/plugin-install-use-case.ts", injected: 7 }, + { path: "src/contexts/framework/application/plugin/plugin-install-use-case.ts", injected: 6 }, { path: "src/contexts/framework/application/plugin/plugin-update-use-case.ts", injected: 6 }, { path: "src/contexts/framework/application/restore/restore-all-plugins-use-case.ts", diff --git a/cli/tests/contexts/framework/application/plugin/plugin-install-use-case.unit.test.ts b/cli/tests/contexts/framework/application/plugin/plugin-install-use-case.unit.test.ts index 9d2e72355..cd9345d57 100644 --- a/cli/tests/contexts/framework/application/plugin/plugin-install-use-case.unit.test.ts +++ b/cli/tests/contexts/framework/application/plugin/plugin-install-use-case.unit.test.ts @@ -13,7 +13,6 @@ import { } from "../../../../../src/kernel/errors.js"; import type { Prompter } from "../../../../../src/kernel/ports/prompter.js"; import type { PluginPick } from "../../../../../src/presentation/prompts/plugin-pick-use-case.js"; -import { InMemoryEnvironment } from "../../../../helpers/ports/in-memory-environment.js"; import { InMemoryManifestRepository } from "../../../../helpers/ports/in-memory-manifest-repository.js"; const PLUGIN_FIXTURE = join(process.cwd(), "tests/fixtures/plugins/claude-format/sample-plugin"); @@ -43,7 +42,6 @@ function makeUseCases(overrides?: { marketplaceExecute?: ReturnType; trustStore?: MarketplaceTrustStore; prompter?: Prompter; - environment?: InMemoryEnvironment; }) { const pickExecute = overrides?.pickExecute ?? vi.fn(); const addExecute = overrides?.addExecute ?? vi.fn(); @@ -56,7 +54,6 @@ function makeUseCases(overrides?: { const manifestRepo = new InMemoryManifestRepository(); const trustStore = overrides?.trustStore ?? makeAlwaysTrustStore(); const prompter = overrides?.prompter ?? makeSilentPrompter(); - const environment = overrides?.environment ?? new InMemoryEnvironment(); return { pluginPickUseCase, pluginAddUseCase, @@ -64,7 +61,6 @@ function makeUseCases(overrides?: { manifestRepo, trustStore, prompter, - environment, pickExecute, addExecute, marketplaceExecute, @@ -79,7 +75,6 @@ function makeUseCase(overrides?: Parameters[0]): PluginInst manifestRepo, trustStore, prompter, - environment, } = makeUseCases(overrides); return new PluginInstallUseCase( pluginPickUseCase, @@ -87,8 +82,7 @@ function makeUseCase(overrides?: Parameters[0]): PluginInst pluginInstallFromMarketplaceUseCase, manifestRepo, trustStore, - prompter, - environment + prompter ); } @@ -284,35 +278,4 @@ describe("PluginInstallUseCase", () => { expect(trustStore.isTrusted).not.toHaveBeenCalled(); }); }); - - describe("token publication", () => { - it("publishes --token through the environment, for a fetcher built before the flag arrived", async () => { - const environment = new InMemoryEnvironment(); - const marketplaceExecute = vi.fn().mockResolvedValue({ entry: { name: "my-plugin" } }); - - await makeUseCase({ marketplaceExecute, environment }).execute({ - pluginArg: "my-plugin", - toolIds: "all", - projectRoot: PROJECT_ROOT, - interactive: false, - token: "ghp_from_flag", - }); - - expect(environment.get("AIDD_TOKEN")).toBe("ghp_from_flag"); - }); - - it("publishes nothing when no token is passed", async () => { - const environment = new InMemoryEnvironment(); - const marketplaceExecute = vi.fn().mockResolvedValue({ entry: { name: "my-plugin" } }); - - await makeUseCase({ marketplaceExecute, environment }).execute({ - pluginArg: "my-plugin", - toolIds: "all", - projectRoot: PROJECT_ROOT, - interactive: false, - }); - - expect(environment.get("AIDD_TOKEN")).toBeUndefined(); - }); - }); }); diff --git a/cli/tests/helpers/ports/in-memory-environment.ts b/cli/tests/helpers/ports/in-memory-environment.ts index d82478a5c..b2e3037ad 100644 --- a/cli/tests/helpers/ports/in-memory-environment.ts +++ b/cli/tests/helpers/ports/in-memory-environment.ts @@ -10,8 +10,4 @@ export class InMemoryEnvironment implements Environment { get(name: string): string | undefined { return this.values.get(name); } - - set(name: string, value: string): void { - this.values.set(name, value); - } } diff --git a/cli/tests/presentation/commands/marketplace-wiring.integration.test.ts b/cli/tests/presentation/commands/marketplace-wiring.integration.test.ts index 4966292e1..6491de134 100644 --- a/cli/tests/presentation/commands/marketplace-wiring.integration.test.ts +++ b/cli/tests/presentation/commands/marketplace-wiring.integration.test.ts @@ -38,7 +38,6 @@ const PROJECT_ROOT = process.cwd(); let written: string[] = []; let errors: string[] = []; -let tokenBefore: string | undefined; function pretendTerminal(isTTY: boolean): void { Object.defineProperty(process.stdout, "isTTY", { value: isTTY, configurable: true }); @@ -48,7 +47,6 @@ beforeEach(() => { vi.clearAllMocks(); written = []; errors = []; - tokenBefore = process.env.AIDD_TOKEN; pretendTerminal(false); vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { written.push(String(chunk)); @@ -77,8 +75,6 @@ beforeEach(() => { afterEach(() => { vi.restoreAllMocks(); - if (tokenBefore === undefined) delete process.env.AIDD_TOKEN; - else process.env.AIDD_TOKEN = tokenBefore; process.exitCode = undefined; }); @@ -148,10 +144,15 @@ describe("aidd marketplace add", () => { ); }); - it("puts the given token where the fetch will read it", async () => { + it("hands the given token to the composition root, where every fetcher reads it", async () => { await run("add", "market-b", "/some/source", "--token", "ghp_x"); - expect(process.env.AIDD_TOKEN).toBe("ghp_x"); + expect(vi.mocked(createDeps)).toHaveBeenCalledWith( + PROJECT_ROOT, + { verbose: false, token: "ghp_x" }, + expect.anything() + ); + expect(process.env.AIDD_TOKEN).not.toBe("ghp_x"); }); it("asks for the name and the source it was not given, on a terminal", async () => { @@ -192,12 +193,14 @@ describe("aidd marketplace add", () => { }); }); -it("leaves the token environment alone when none was given", async () => { - delete process.env.AIDD_TOKEN; - +it("hands no token when none was given", async () => { await run("add", "market-b", "/some/source"); - expect(process.env.AIDD_TOKEN).toBeUndefined(); + expect(vi.mocked(createDeps)).toHaveBeenCalledWith( + PROJECT_ROOT, + { verbose: false, token: undefined }, + expect.anything() + ); }); it("names a failed registration on stderr and fails the process", async () => { diff --git a/cli/tests/presentation/commands/plugin-wiring.integration.test.ts b/cli/tests/presentation/commands/plugin-wiring.integration.test.ts index 41c51e528..467c2ea4b 100644 --- a/cli/tests/presentation/commands/plugin-wiring.integration.test.ts +++ b/cli/tests/presentation/commands/plugin-wiring.integration.test.ts @@ -169,7 +169,6 @@ describe("aidd plugin install", () => { projectRoot: PROJECT_ROOT, interactive: false, fromMarketplace: undefined, - token: undefined, yes: undefined, scope: undefined, }); @@ -179,6 +178,17 @@ describe("aidd plugin install", () => { }); }); + it("hands --token to the composition root, where every fetcher reads it, and to nothing else", async () => { + await run("install", "aidd-dev", "--token", "ghp_x"); + + expect(vi.mocked(createDeps)).toHaveBeenCalledWith( + PROJECT_ROOT, + { verbose: false, token: "ghp_x" }, + expect.anything() + ); + expect(pluginInstall.mock.calls[0][0]).not.toHaveProperty("token"); + }); + it("narrows activation to the marketplace the install was told to use", async () => { await run("install", "aidd-dev", "--from", "market-b"); @@ -191,7 +201,7 @@ describe("aidd plugin install", () => { }); }); - it("carries the scope, the token and the auto-answer a scripted run gave", async () => { + it("carries the scope and the auto-answer a scripted run gave", async () => { await run( "install", "aidd-dev", @@ -205,7 +215,7 @@ describe("aidd plugin install", () => { ); expect(pluginInstall).toHaveBeenCalledWith( - expect.objectContaining({ scope: "user", token: "ghp_x", yes: true, toolIds: ["claude"] }) + expect.objectContaining({ scope: "user", yes: true, toolIds: ["claude"] }) ); }); diff --git a/cli/tests/runtime/auth/auth-reader.integration.test.ts b/cli/tests/runtime/auth/auth-reader.integration.test.ts index 64ef242a7..2705feaad 100644 --- a/cli/tests/runtime/auth/auth-reader.integration.test.ts +++ b/cli/tests/runtime/auth/auth-reader.integration.test.ts @@ -55,6 +55,35 @@ function withEnv(vars: Record, fn: () => T): T { } describe("AuthReaderAdapter", () => { + describe("a token given on the command line (path 0)", () => { + it("comes before AIDD_TOKEN and before anything stored", async () => { + const storage = makeStorage({ + projectConfig: { + version: 1, + method: "stored", + level: "project", + token: "project-token", + createdAt: "2026-03-20T00:00:00.000Z", + }, + }); + const reader = new AuthReaderAdapter(storage, "/project", undefined, undefined, "flag-token"); + const result = await withEnv({ AIDD_TOKEN: "env-token" }, () => reader.resolve()); + expect(result).toBe("flag-token"); + }); + + it("is not consulted when none was given, so AIDD_TOKEN keeps its place", async () => { + const reader = new AuthReaderAdapter( + makeStorage(), + "/project", + undefined, + undefined, + undefined + ); + const result = await withEnv({ AIDD_TOKEN: "env-token" }, () => reader.resolve()); + expect(result).toBe("env-token"); + }); + }); + describe("AIDD_TOKEN env var (path 1)", () => { it("returns env token immediately", async () => { const storage = makeStorage();