Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions packages/gittensory-miner/lib/coding-agent-construction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
13 changes: 4 additions & 9 deletions packages/gittensory-miner/lib/coding-agent-house-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof runCodingAgentAttempt>[0] & {
* houseRulesConfig?: Parameters<typeof buildHouseRulesPreToolUseHook>[0],
Expand All @@ -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 } : {}) });
}
36 changes: 8 additions & 28 deletions test/unit/miner-coding-agent-construction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)", () => {
Expand Down
15 changes: 8 additions & 7 deletions test/unit/miner-coding-agent-house-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down