Skip to content
Merged
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
42 changes: 42 additions & 0 deletions packages/plugin/src/agent-registration-drift.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,48 @@ describe("hidden-agent registration drift guard", () => {
expect(permissionDisabled("task", taskRules(merged.task))).toBe(true);
});

test("named task allows survive an ambient wildcard deny (#777)", () => {
const internalAgentIds = [DREAMER_REVIEWER_AGENT, HISTORIAN_AGENT];
const merged = denyTaskRoutingToAgents(
{ task: { "*": "deny", "my-scout": "allow" } },
internalAgentIds,
) as { task: Record<string, string> };

// The agent's own ordering is kept: the wildcard stays where the user
// put it, so the named allow after it still wins under OpenCode's
// last-matching-rule evaluation instead of being overridden by a
// re-appended wildcard.
expect(merged.task).toEqual({
"*": "deny",
"my-scout": "allow",
[DREAMER_REVIEWER_AGENT]: "deny",
[HISTORIAN_AGENT]: "deny",
});

const rules = taskRules(merged.task);
const lastMatching = (target: string) =>
[...rules].reverse().find((rule) => rule.pattern === target || rule.pattern === "*");
expect(lastMatching("my-scout")?.action).toBe("allow");
expect(lastMatching("other-agent")?.action).toBe("deny");

// A mixed map must not hide the whole Task tool: the last rule is a
// named deny, not the wildcard.
expect(permissionDisabled("task", rules)).toBe(false);
});

test("mixed wildcard-deny task maps re-merge idempotently (#777)", () => {
const internalAgentIds = [DREAMER_REVIEWER_AGENT, HISTORIAN_AGENT];
const first = denyTaskRoutingToAgents(
{ task: { "*": "deny", "my-scout": "allow" } },
internalAgentIds,
);
const second = denyTaskRoutingToAgents(first, internalAgentIds);

expect(second).toEqual(first);
const task = (second as { task: Record<string, string> }).task;
expect(Object.keys(task).filter((pattern) => pattern === "*")).toHaveLength(1);
});

test("Task wildcard re-emission is idempotent across repeated registration", () => {
const internalAgentIds = [DREAMER_REVIEWER_AGENT, HISTORIAN_AGENT];
const first = denyTaskRoutingToAgents({ task: { "*": "deny" } }, internalAgentIds);
Expand Down
28 changes: 22 additions & 6 deletions packages/plugin/src/agents/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,14 @@ function isPermissionMap(value: unknown): value is Record<string, unknown> {
* OpenCode accepts either a whole-permission action or a pattern map for
* `permission.task`; its visibility evaluator uses the last matching rule.
* Normalize the whole-permission form, retain unrelated user rules, then append
* our exact agent-id denies. When the ambient task policy has a wildcard deny,
* re-emit that wildcard after the named denies so the Task tool is hidden rather
* than shown as a callable-but-refused tool.
* our exact agent-id denies. When the ambient task policy is a wildcard deny
* whose only companions are our own appended agent-id denies, re-emit that
* wildcard after the named denies so the Task tool is hidden rather than shown
* as a callable-but-refused tool. When the wildcard deny sits next to any of
* the agent's own rules (the deny-by-default-with-exceptions shape), keep the
* agent's own ordering untouched: OpenCode resolves with the last matching
* rule, so moving the wildcard to the end there would override every named
* allow and empty the Task tool (#777).
*/
export function denyTaskRoutingToAgents(
permission: unknown,
Expand All @@ -133,10 +138,19 @@ export function denyTaskRoutingToAgents(
: {};
const internalAgentIdSet = new Set(internalAgentIds);
const hasAmbientWildcardDeny = configuredTask["*"] === "deny";
// Only our own appended agent-id denies may accompany the wildcard for the
// move to stay correct — a user's named allow next to it must keep winning.
// Excluding internal ids here also keeps re-merging our own output
// idempotent: that output is exactly internal denies plus the wildcard.
const movesAmbientWildcardDeny =
hasAmbientWildcardDeny &&
Object.keys(configuredTask).every(
(agentId) => agentId === "*" || internalAgentIdSet.has(agentId),
);
const retainedTask = Object.fromEntries(
Object.entries(configuredTask).filter(
([agentId]) =>
!internalAgentIdSet.has(agentId) && (agentId !== "*" || !hasAmbientWildcardDeny),
!internalAgentIdSet.has(agentId) && (agentId !== "*" || !movesAmbientWildcardDeny),
),
);

Expand All @@ -147,8 +161,10 @@ export function denyTaskRoutingToAgents(
...Object.fromEntries(internalAgentIds.map((agentId) => [agentId, "deny"])),
// OpenCode's Permission.disabled uses findLast. Re-inserting the
// ambient wildcard after our named routing denies keeps its whole-tool
// deny as the final match and makes the Task tool invisible.
...(hasAmbientWildcardDeny ? { "*": "deny" } : {}),
// deny as the final match and makes the Task tool invisible — but
// only when it stood alone; with named rules the agent's own
// ordering must win (#777).
...(movesAmbientWildcardDeny ? { "*": "deny" } : {}),
},
};
}
Expand Down
Loading