fix(thread): 🐛 hide custom agent tool calls and display proper names#204
fix(thread): 🐛 hide custom agent tool calls and display proper names#204jorben wants to merge 3 commits into
Conversation
Unify isRuntimeOrchestrationToolName to use startsWith("agent_") pattern
so custom agent_{slug} calls are hidden from timeline alongside built-in
agent_explore/agent_review. Add slugToNameMap support to formatHelperKind
so custom agents show user-defined names instead of raw helper_custom_{slug}.
AI Code Review SummaryPR: #204 (fix(thread): 🐛 hide custom agent tool calls and display proper names) Overall AssessmentDetected 3 actionable findings, prioritize CRITICAL/HIGH before merge. Major Findings by Severity
Actionable Suggestions
Potential Risks
Test Suggestions
File-Level Coverage Notes
Inline Downgraded Items (processed but not inline)
Coverage Status
Uncovered list:
No-patch covered list:
Runtime/Budget
|
| .join(" "); | ||
| } | ||
|
|
||
| export function formatHelperKind( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const agentProfiles = useStore(settingsStore, (s) => s.agentProfiles); | ||
| const customSubagents = useStore(settingsStore, (s) => s.customSubagents, shallowEqual); | ||
| const customAgentSlugToName = useMemo( | ||
| () => new Map(customSubagents.map((a) => [a.slug, a.name])), |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| export function isRuntimeOrchestrationToolName(toolName: string): boolean { | ||
| return (RUNTIME_ORCHESTRATION_TOOLS as ReadonlyArray<string>).includes(toolName); | ||
| return toolName.startsWith(RUNTIME_ORCHESTRATION_TOOL_PREFIX); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| export function isRuntimeOrchestrationToolName(toolName: string): boolean { | ||
| return (RUNTIME_ORCHESTRATION_TOOLS as ReadonlyArray<string>).includes(toolName); | ||
| return toolName.startsWith(RUNTIME_ORCHESTRATION_TOOL_PREFIX); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…tests Require at least one character after the agent_ prefix so bare "agent_" is no longer classified as a runtime orchestration tool. Add unit tests for formatHelperKind, formatHelperName, formatHelperSummary covering built-in kinds, custom agent name lookup, slug fallback capitalization, and edge cases.
| @@ -26,7 +29,10 @@ export const DEFAULT_COLLAPSED_TOOLS = [ | |||
| ] as const; | |||
|
|
|||
| export function isRuntimeOrchestrationToolName(toolName: string): boolean { | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| export function formatHelperKind(kind: string) { | ||
| const CUSTOM_PREFIX = "helper_custom_"; | ||
|
|
||
| function capitalizeSlug(slug: string) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add .filter(Boolean) after split("-") so that hypothetical consecutive
hyphens or trailing hyphens do not produce stray empty-string segments.
| export function isRuntimeOrchestrationToolName(toolName: string): boolean { | ||
| return (RUNTIME_ORCHESTRATION_TOOLS as ReadonlyArray<string>).includes(toolName); | ||
| return ( | ||
| toolName.startsWith(RUNTIME_ORCHESTRATION_TOOL_PREFIX) |
There was a problem hiding this comment.
[MEDIUM] Prefix-based identification of privileged orchestration tools
Buddy, the function isRuntimeOrchestrationToolName has been changed from checking an explicit allowlist to checking a simple prefix ('agent_'). If runtime orchestration tools receive elevated privileges, bypass consent prompts, or run in a different isolation boundary, any untrusted custom tool or plugin could gain these privileges simply by prefixing its name with 'agent_'.
Suggestion: Instead of relying on a string prefix to identify privileged orchestration tools, maintain a strict and secure registry of authorized orchestration tools, or require explicit registration with verification rather than basic prefix matching.
Risk: Untrusted or malicious extensions/tools can masquerade as orchestration tools, potentially escalating privileges, bypassing validation, or execution policies.
Confidence: 0.85
| case "helper_review": | ||
| return "Review Agent"; | ||
| default: | ||
| if (kind.startsWith(CUSTOM_PREFIX)) { |
There was a problem hiding this comment.
[LOW] Fallback handling for empty slug in custom helper kind
If the helper kind is exactly 'helper_custom_' (with an empty slug), capitalizeSlug returns an empty string, which causes formatHelperKind to return an empty string. This could result in blank names rendering in the UI.
Suggestion: Add a defensive fallback in formatHelperKind to return a default string like 'Custom Agent' or the raw 'kind' string if the sliced slug is falsy.
Risk: Minor visual glitch (empty label) in the UI if an empty custom helper kind ever reaches the client.
Confidence: 0.90
|
|
||
| export function isRuntimeOrchestrationToolName(toolName: string): boolean { | ||
| return (RUNTIME_ORCHESTRATION_TOOLS as ReadonlyArray<string>).includes(toolName); | ||
| return ( |
There was a problem hiding this comment.
[LOW] Namespace Collision Risk with Prefix-Based Tool Classification
Switching from an explicit allowlist to a prefix-based check ('agent_') means any tool (including user-defined or third-party extensions) that happens to begin with 'agent_' will be treated as a runtime orchestration tool. This could lead to runtime errors or security/routing misbehaviors if standard tools are mistakenly processed as orchestration tools.
Suggestion: Buddy, make sure there is documentation or validation at the tool registration layer that reserves the 'agent_' prefix exclusively for runtime orchestration tools, or enforce this via system-level validation.
Risk: Custom tools from extensions or integrations prefixed with 'agent_' might be mistakenly routed as runtime orchestration tools, leading to unexpected behavior or execution failures.
Confidence: 0.90
Summary
agent_{slug}tool calls leaking into the timeline alongside helper cardshelper_custom_{slug}stringsisRuntimeOrchestrationToolNameto usestartsWith("agent_")pattern, eliminating duplicate implementationsTest Plan
npm run typecheckpassesnpm run test:unitpasses (752 tests)agent_refactortool card🤖 Generated with TiyCode