fix(plugins): enforce high-priority override precedence

Make before_agent_start override merging preserve the first defined
model/provider override so higher-priority hooks cannot be overwritten by
lower-priority handlers, and align the corresponding test title and
expectation with the intended precedence behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Nate Fikru
2026-02-16 16:16:37 -05:00
committed by Peter Steinberger
parent 2456b17587
commit 6d31d1ecc6
2 changed files with 5 additions and 11 deletions

View File

@@ -74,10 +74,7 @@ describe("before_agent_start hook merger", () => {
expect(result?.providerOverride).toBe("ollama");
});
it("higher-priority plugin wins for modelOverride (last-writer-wins by priority order)", async () => {
// Lower priority runs first (priority 1), higher priority runs second (priority 10).
// Since merger is sequential and uses `next ?? acc`, the higher-priority plugin's
// value (which runs later) overwrites the earlier one.
it("higher-priority plugin wins for modelOverride", async () => {
addBeforeAgentStartHook(registry, "low-priority", () => ({ modelOverride: "gpt-4o" }), 1);
addBeforeAgentStartHook(
registry,
@@ -89,11 +86,7 @@ describe("before_agent_start hook merger", () => {
const runner = createHookRunner(registry);
const result = await runner.runBeforeAgentStart({ prompt: "PII prompt" }, stubCtx);
// Higher priority (10) runs first in sorted order, then lower priority (1) runs.
// Lower priority's value overwrites since merger uses next ?? acc (next wins if defined).
// Actually: sorted by descending priority, so 10 runs first, then 1.
// Merger: acc starts as 10's result, then next=1's result overwrites.
expect(result?.modelOverride).toBe("gpt-4o");
expect(result?.modelOverride).toBe("llama3.3:8b");
});
it("lower-priority plugin does not overwrite if it returns undefined", async () => {

View File

@@ -200,8 +200,9 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp
acc?.prependContext && next.prependContext
? `${acc.prependContext}\n\n${next.prependContext}`
: (next.prependContext ?? acc?.prependContext),
modelOverride: next.modelOverride ?? acc?.modelOverride,
providerOverride: next.providerOverride ?? acc?.providerOverride,
// Keep the first defined override so higher-priority hooks win.
modelOverride: acc?.modelOverride ?? next.modelOverride,
providerOverride: acc?.providerOverride ?? next.providerOverride,
}),
);
}