refactor: simplify task tool subagent filtering #7165
Conversation
Move subagent filtering logic from prompt.ts into task.ts, following the same pattern used by skill.ts. This makes the task tool self-contained and removes coupling between files. Changes: - task.ts: Filter agents using ctx?.agent permissions during description generation - prompt.ts: Remove regeneration block and imports (TASK_DESCRIPTION, filterSubagents) - test: Define filterSubagents helper locally instead of importing No functional changes - just cleaner architecture.
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
|
@rekram1-node Just tidying up, no functionality changes, just moved the filtration logic to task took to be self contained. |
The filterSubagents tests were testing a local helper function that mirrored the production code. The PermissionNext.evaluate tests already cover the same logic, making these tests redundant. - Remove filterSubagents helper and its test block (~130 lines) - Update integration tests to use PermissionNext.evaluate directly - All test coverage preserved through existing evaluate tests
Replace the userInvokedAgents array with a simple boolean that only checks the current turn's user message. This prevents the bypass from persisting across the entire session. - Check only last user message instead of all messages - Simplify task.ts permission check to just use bypassAgentCheck - Command subtasks continue to use bypassAgentCheck: true
|
Hey! Looked into simplifying The tricky part was that the old code looked at ALL user messages in the session, so once you Fixed it by only checking the last user message (current turn): // Before: checks all messages (persists across session)
const userInvokedAgents = msgs
.filter((m) => m.info.role === "user")
.flatMap((m) => m.parts.filter((p) => p.type === "agent"))
.map((p) => p.name)
// After: checks only current turn
const lastUserMsg = msgs.findLast((m) => m.info.role === "user")
const bypassAgentCheck = lastUserMsg?.parts.some((p) => p.type === "agent") ?? falseThis way:
Also cleaned up the redundant |
|
/review |
|
lgtm |
2 participants
Summary
Small cleanup to align the task tool's filtering approach with how skill.ts handles it, plus simplification of the
userInvokedAgentsmechanism.Self-contained filtering:
task.tsitself (usingctx?.agentfrom fix: restore skill filtering by agent permissions #7042)prompt.tsSimplify bypass logic:
userInvokedAgentsarray withbypassAgentCheckbooleanTest cleanup:
filterSubagentstests (already covered byPermissionNext.evaluatetests)Changes
task.ts: Filter during description generation + simplify permission checkprompt.ts: Remove regeneration block + usebypassAgentCheckbooleantest: Remove redundant tests (~130 lines)No functional changes to permission behavior - just cleaner code!