diff --git a/packages/gittensory-miner/lib/coding-agent-construction.js b/packages/gittensory-miner/lib/coding-agent-construction.js index d463649b1..bcdb40c1d 100644 --- a/packages/gittensory-miner/lib/coding-agent-construction.js +++ b/packages/gittensory-miner/lib/coding-agent-construction.js @@ -62,11 +62,10 @@ export function createRealCliSubprocessSpawn() { * automatic-enforcement guarantee `runHouseRulesEnforcedCodingAgentAttempt` gives task-level callers, but at * the raw driver-construction level `attempt-runner.js`'s `deps.driver` actually needs. * - * The default only applies to `agent-sdk`, the one provider with a real hook-registration surface. CLI - * subprocess providers (`claude-cli`/`codex-cli`) have none, and the engine's `createCliProvider` fails closed - * if `hooks` is supplied at all (driver-factory.ts) -- filling the default for them here would make every CLI - * construction throw. An explicitly-supplied `options.hooks` always wins and is forwarded as-is, so a caller - * that deliberately asks a CLI provider to enforce hooks still gets that same fail-closed rejection. + * The default is deliberately built before provider dispatch. `agent-sdk` can enforce it with its PreToolUse + * surface; CLI subprocess providers (`claude-cli`/`codex-cli`) cannot, so the engine rejects the hooks and + * production construction fails closed instead of running untrusted issue prompts without house-rule enforcement. + * An explicitly-supplied `options.hooks` always wins and is forwarded as-is. * * Fails closed (throws) when no provider is configured, or when a CLI provider is selected without a real * spawn available — never silently falls back to a driver that can never run. @@ -86,11 +85,7 @@ export function constructProductionCodingAgentDriver(env, options = {}) { if (!providerName) { throw new Error("unconfigured_coding_agent_driver:no_provider_in_MINER_CODING_AGENT_PROVIDER"); } - const hooks = - options.hooks ?? - (providerName.trim().toLowerCase() === "agent-sdk" - ? buildHouseRulesAgentSdkHooks(options.houseRulesConfig, options.houseRulesOptions) - : undefined); + const hooks = options.hooks ?? buildHouseRulesAgentSdkHooks(options.houseRulesConfig, options.houseRulesOptions); return createCodingAgentDriver({ providerName, env, diff --git a/packages/gittensory-miner/lib/coding-agent-house-rules.js b/packages/gittensory-miner/lib/coding-agent-house-rules.js index c989335ef..2a0193985 100644 --- a/packages/gittensory-miner/lib/coding-agent-house-rules.js +++ b/packages/gittensory-miner/lib/coding-agent-house-rules.js @@ -42,10 +42,9 @@ export function buildHouseRulesAgentSdkHooks(config = {}, options = {}) { * {@link buildHouseRulesAgentSdkHooks} for the `agent-sdk` provider, so house-rule enforcement (#2343) is ON * by default rather than opt-in. An explicitly-supplied `hooks` option always wins (e.g. a test injecting its * own hook double, or a caller composing additional hooks of its own) -- this only fills the gap when the - * caller omitted it entirely. CLI providers (`claude-cli`, `codex-cli`) have no hook-registration surface, and - * the engine fails closed if `hooks` is supplied to them at all -- so the default is scoped to `agent-sdk` - * only; a CLI attempt with no explicit `hooks` gets none (today's inert no-op), while one that explicitly - * supplies `hooks` still gets the engine's real fail-closed rejection instead of a silently unenforced run. + * caller omitted it entirely. CLI providers (`claude-cli`, `codex-cli`) have no hook-registration surface; passing + * the default hooks to them intentionally triggers the engine's fail-closed rejection instead of silently running + * untrusted issue prompts without the requested house-rule policy. * * @param {Parameters[0] & { * houseRulesConfig?: Parameters[0], @@ -55,10 +54,6 @@ export function buildHouseRulesAgentSdkHooks(config = {}, options = {}) { */ export function runHouseRulesEnforcedCodingAgentAttempt(options) { const { houseRulesConfig, houseRulesOptions, ...attemptOptions } = options; - const hooks = - attemptOptions.hooks ?? - (attemptOptions.providerName.trim().toLowerCase() === "agent-sdk" - ? buildHouseRulesAgentSdkHooks(houseRulesConfig, houseRulesOptions) - : undefined); + const hooks = attemptOptions.hooks ?? buildHouseRulesAgentSdkHooks(houseRulesConfig, houseRulesOptions); return runCodingAgentAttempt({ ...attemptOptions, ...(hooks !== undefined ? { hooks } : {}) }); } diff --git a/test/unit/miner-coding-agent-construction.test.ts b/test/unit/miner-coding-agent-construction.test.ts index c001818f4..86dcea693 100644 --- a/test/unit/miner-coding-agent-construction.test.ts +++ b/test/unit/miner-coding-agent-construction.test.ts @@ -97,34 +97,14 @@ describe("constructProductionCodingAgentDriver (#5131)", () => { expect(result.changedFiles).toEqual([]); }); - it("constructs a claude-cli driver wired to an injected spawn, without invoking it during construction", async () => { - const calls: Array<{ cmd: string; args: readonly string[] }> = []; - const driver = constructProductionCodingAgentDriver( - { MINER_CODING_AGENT_PROVIDER: "claude-cli" }, - { - spawn: async (cmd, args) => { - calls.push({ cmd, args }); - return { stdout: "done", code: 0 }; - }, - }, - ); - expect(calls).toHaveLength(0); // construction alone must not spawn anything - const result = await driver.run(task); - expect(calls).toHaveLength(1); - expect(calls[0]!.cmd).toBe("claude"); - expect(result.ok).toBe(true); - }); - - it("defaults to a real (non-injected) spawn for a CLI provider when the caller supplies none", () => { - // Construction alone must succeed without ever invoking the real spawn (a real "claude" binary is not - // present in CI) — proving the `options.spawn ?? createRealCliSubprocessSpawn()` default branch is taken. - const driver = constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" }); - expect(typeof driver.run).toBe("function"); - }); - - it("REGRESSION: does NOT default-fill house-rule hooks for claude-cli/codex-cli — the default only applies to agent-sdk, the one provider that can enforce them", () => { - expect(() => constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" })).not.toThrow(); - expect(() => constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "codex-cli" })).not.toThrow(); + it("REGRESSION: fails closed for claude-cli/codex-cli by default because production house-rule hooks cannot be enforced there", () => { + const spawn = async () => ({ stdout: "done", code: 0 }); + expect(() => + constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" }, { spawn }), + ).toThrow(/unsupported_coding_agent_driver_hooks:claude-cli/); + expect(() => + constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "codex-cli" }, { spawn }), + ).toThrow(/unsupported_coding_agent_driver_hooks:codex-cli/); }); it("still fails closed for claude-cli/codex-cli when the caller EXPLICITLY supplies hooks (a real request the engine correctly rejects rather than silently dropping)", () => { diff --git a/test/unit/miner-coding-agent-house-rules.test.ts b/test/unit/miner-coding-agent-house-rules.test.ts index a92f2499d..5d494507f 100644 --- a/test/unit/miner-coding-agent-house-rules.test.ts +++ b/test/unit/miner-coding-agent-house-rules.test.ts @@ -121,13 +121,14 @@ describe("runHouseRulesEnforcedCodingAgentAttempt (#2343 follow-up)", () => { expect(result.result.ok).toBe(true); }); - it("REGRESSION: does NOT default-fill hooks for claude-cli — the default only applies to agent-sdk, the one provider that can enforce them", async () => { - const result = await runHouseRulesEnforcedCodingAgentAttempt({ - providerName: "claude-cli", - task, - spawn: async () => ({ stdout: "done", code: 0 }), - }); - expect(result.result.ok).toBe(true); + it("REGRESSION: fails closed for claude-cli by default because house-rule hooks cannot be enforced there", async () => { + await expect( + runHouseRulesEnforcedCodingAgentAttempt({ + providerName: "claude-cli", + task, + spawn: async () => ({ stdout: "done", code: 0 }), + }), + ).rejects.toThrow(/unsupported_coding_agent_driver_hooks:claude-cli/); }); it("still fails closed for claude-cli when the caller EXPLICITLY supplies hooks (a real request the engine correctly rejects rather than silently dropping)", async () => {