Skip to content

Commit 20955b2

Browse files
committed
fix(miner): fail closed for unenforceable CLI house rules
1 parent 0d78f24 commit 20955b2

4 files changed

Lines changed: 25 additions & 54 deletions

File tree

packages/gittensory-miner/lib/coding-agent-construction.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,10 @@ export function createRealCliSubprocessSpawn() {
6262
* automatic-enforcement guarantee `runHouseRulesEnforcedCodingAgentAttempt` gives task-level callers, but at
6363
* the raw driver-construction level `attempt-runner.js`'s `deps.driver` actually needs.
6464
*
65-
* The default only applies to `agent-sdk`, the one provider with a real hook-registration surface. CLI
66-
* subprocess providers (`claude-cli`/`codex-cli`) have none, and the engine's `createCliProvider` fails closed
67-
* if `hooks` is supplied at all (driver-factory.ts) -- filling the default for them here would make every CLI
68-
* construction throw. An explicitly-supplied `options.hooks` always wins and is forwarded as-is, so a caller
69-
* that deliberately asks a CLI provider to enforce hooks still gets that same fail-closed rejection.
65+
* The default is deliberately built before provider dispatch. `agent-sdk` can enforce it with its PreToolUse
66+
* surface; CLI subprocess providers (`claude-cli`/`codex-cli`) cannot, so the engine rejects the hooks and
67+
* production construction fails closed instead of running untrusted issue prompts without house-rule enforcement.
68+
* An explicitly-supplied `options.hooks` always wins and is forwarded as-is.
7069
*
7170
* Fails closed (throws) when no provider is configured, or when a CLI provider is selected without a real
7271
* spawn available — never silently falls back to a driver that can never run.
@@ -86,11 +85,7 @@ export function constructProductionCodingAgentDriver(env, options = {}) {
8685
if (!providerName) {
8786
throw new Error("unconfigured_coding_agent_driver:no_provider_in_MINER_CODING_AGENT_PROVIDER");
8887
}
89-
const hooks =
90-
options.hooks ??
91-
(providerName.trim().toLowerCase() === "agent-sdk"
92-
? buildHouseRulesAgentSdkHooks(options.houseRulesConfig, options.houseRulesOptions)
93-
: undefined);
88+
const hooks = options.hooks ?? buildHouseRulesAgentSdkHooks(options.houseRulesConfig, options.houseRulesOptions);
9489
return createCodingAgentDriver({
9590
providerName,
9691
env,

packages/gittensory-miner/lib/coding-agent-house-rules.js

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ export function buildHouseRulesAgentSdkHooks(config = {}, options = {}) {
4242
* {@link buildHouseRulesAgentSdkHooks} for the `agent-sdk` provider, so house-rule enforcement (#2343) is ON
4343
* by default rather than opt-in. An explicitly-supplied `hooks` option always wins (e.g. a test injecting its
4444
* own hook double, or a caller composing additional hooks of its own) -- this only fills the gap when the
45-
* caller omitted it entirely. CLI providers (`claude-cli`, `codex-cli`) have no hook-registration surface, and
46-
* the engine fails closed if `hooks` is supplied to them at all -- so the default is scoped to `agent-sdk`
47-
* only; a CLI attempt with no explicit `hooks` gets none (today's inert no-op), while one that explicitly
48-
* supplies `hooks` still gets the engine's real fail-closed rejection instead of a silently unenforced run.
45+
* caller omitted it entirely. CLI providers (`claude-cli`, `codex-cli`) have no hook-registration surface; passing
46+
* the default hooks to them intentionally triggers the engine's fail-closed rejection instead of silently running
47+
* untrusted issue prompts without the requested house-rule policy.
4948
*
5049
* @param {Parameters<typeof runCodingAgentAttempt>[0] & {
5150
* houseRulesConfig?: Parameters<typeof buildHouseRulesPreToolUseHook>[0],
@@ -55,10 +54,6 @@ export function buildHouseRulesAgentSdkHooks(config = {}, options = {}) {
5554
*/
5655
export function runHouseRulesEnforcedCodingAgentAttempt(options) {
5756
const { houseRulesConfig, houseRulesOptions, ...attemptOptions } = options;
58-
const hooks =
59-
attemptOptions.hooks ??
60-
(attemptOptions.providerName.trim().toLowerCase() === "agent-sdk"
61-
? buildHouseRulesAgentSdkHooks(houseRulesConfig, houseRulesOptions)
62-
: undefined);
57+
const hooks = attemptOptions.hooks ?? buildHouseRulesAgentSdkHooks(houseRulesConfig, houseRulesOptions);
6358
return runCodingAgentAttempt({ ...attemptOptions, ...(hooks !== undefined ? { hooks } : {}) });
6459
}

test/unit/miner-coding-agent-construction.test.ts

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -97,34 +97,14 @@ describe("constructProductionCodingAgentDriver (#5131)", () => {
9797
expect(result.changedFiles).toEqual([]);
9898
});
9999

100-
it("constructs a claude-cli driver wired to an injected spawn, without invoking it during construction", async () => {
101-
const calls: Array<{ cmd: string; args: readonly string[] }> = [];
102-
const driver = constructProductionCodingAgentDriver(
103-
{ MINER_CODING_AGENT_PROVIDER: "claude-cli" },
104-
{
105-
spawn: async (cmd, args) => {
106-
calls.push({ cmd, args });
107-
return { stdout: "done", code: 0 };
108-
},
109-
},
110-
);
111-
expect(calls).toHaveLength(0); // construction alone must not spawn anything
112-
const result = await driver.run(task);
113-
expect(calls).toHaveLength(1);
114-
expect(calls[0]!.cmd).toBe("claude");
115-
expect(result.ok).toBe(true);
116-
});
117-
118-
it("defaults to a real (non-injected) spawn for a CLI provider when the caller supplies none", () => {
119-
// Construction alone must succeed without ever invoking the real spawn (a real "claude" binary is not
120-
// present in CI) — proving the `options.spawn ?? createRealCliSubprocessSpawn()` default branch is taken.
121-
const driver = constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" });
122-
expect(typeof driver.run).toBe("function");
123-
});
124-
125-
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", () => {
126-
expect(() => constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" })).not.toThrow();
127-
expect(() => constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "codex-cli" })).not.toThrow();
100+
it("REGRESSION: fails closed for claude-cli/codex-cli by default because production house-rule hooks cannot be enforced there", () => {
101+
const spawn = async () => ({ stdout: "done", code: 0 });
102+
expect(() =>
103+
constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "claude-cli" }, { spawn }),
104+
).toThrow(/unsupported_coding_agent_driver_hooks:claude-cli/);
105+
expect(() =>
106+
constructProductionCodingAgentDriver({ MINER_CODING_AGENT_PROVIDER: "codex-cli" }, { spawn }),
107+
).toThrow(/unsupported_coding_agent_driver_hooks:codex-cli/);
128108
});
129109

130110
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)", () => {

test/unit/miner-coding-agent-house-rules.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,14 @@ describe("runHouseRulesEnforcedCodingAgentAttempt (#2343 follow-up)", () => {
121121
expect(result.result.ok).toBe(true);
122122
});
123123

124-
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 () => {
125-
const result = await runHouseRulesEnforcedCodingAgentAttempt({
126-
providerName: "claude-cli",
127-
task,
128-
spawn: async () => ({ stdout: "done", code: 0 }),
129-
});
130-
expect(result.result.ok).toBe(true);
124+
it("REGRESSION: fails closed for claude-cli by default because house-rule hooks cannot be enforced there", async () => {
125+
await expect(
126+
runHouseRulesEnforcedCodingAgentAttempt({
127+
providerName: "claude-cli",
128+
task,
129+
spawn: async () => ({ stdout: "done", code: 0 }),
130+
}),
131+
).rejects.toThrow(/unsupported_coding_agent_driver_hooks:claude-cli/);
131132
});
132133

133134
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 () => {

0 commit comments

Comments
 (0)