Revert "improvement(mship): clean up dead tools, add enrichments"#5057
Conversation
…)" This reverts commit 6f4189f.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR reverts #5056 ("clean up dead tools, add enrichments"), swapping
Confidence Score: 4/5Safe to merge — the swap of enrichment_run for touch_plan is self-contained and the new tool carries its own access checks. The apps/sim/lib/copilot/tools/server/files/touch-plan.ts — the unused Important Files Changed
Sequence DiagramsequenceDiagram
participant AI as Copilot AI
participant Router as router.ts (getServerToolRegistry)
participant Tool as touchPlanServerTool
participant Resolver as resolveWorkflowAliasForWorkspace
participant Writer as writeWorkspaceFileByPath
AI->>Router: routeExecution("touch_plan", params)
alt "isMothershipBetaFeaturesEnabled = false"
Router-->>AI: tool not in registry → Error
else "flag = true"
Router->>Tool: execute(params, context)
Tool->>Tool: normalizeWorkflowPath / normalizePlanRelativePath
Tool->>Resolver: "resolveWorkflowAliasForWorkspace({ workspaceId, path: aliasPath })"
alt "alias not found / kind != plan_file"
Resolver-->>Tool: null
Tool-->>AI: "{ success: false, message: "Workflow not found …" }"
else alias resolved
Resolver-->>Tool: alias (kind: plan_file, scope, workflowId, …)
Tool->>Writer: "writeWorkspaceFileByPath({ mode: "create", … })"
Writer-->>Tool: "{ id, name, vfsPath, backingVfsPath }"
Tool-->>AI: "{ success: true, data: { id, vfsPath, scope, workflowId } }"
end
end
Reviews (1): Last reviewed commit: "Revert "improvement(mship): clean up dea..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| scope?: 'workspace' | 'workflow' | ||
| workflowPath?: string | ||
| name: string | ||
| title?: string |
There was a problem hiding this comment.
title parameter declared but never consumed
TouchPlanArgs includes title?: string and the catalog describes it as "Optional short user-visible label for the plan creation", but nothing in execute reads or forwards title. Any value passed by the AI model is silently dropped, which means the described capability is advertised but not delivered.
| import { isMothershipBetaFeaturesEnabled } from '@/lib/core/config/feature-flags' | ||
|
|
||
| const logger = createLogger('TouchPlanServerTool') | ||
| const TOUCH_PLAN_TOOL_ID = 'touch_plan' |
There was a problem hiding this comment.
TOUCH_PLAN_TOOL_ID constant duplicates the catalog ID
The tool declares its own string constant instead of importing TouchPlan.id from the catalog — unlike enrichment-run.ts, which used EnrichmentRun.id directly. If the catalog entry's id field is ever renamed, this constant and the catalog can silently diverge and break tool routing.
| const TOUCH_PLAN_TOOL_ID = 'touch_plan' | |
| import { TouchPlan } from '@/lib/copilot/generated/tool-catalog-v1' |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if (!isMothershipBetaFeaturesEnabled) { | ||
| return { success: false, message: 'touch_plan is not available' } | ||
| } |
There was a problem hiding this comment.
getServerToolRegistry() in router.ts already deletes touchPlanServerTool from the registry when isMothershipBetaFeaturesEnabled is false, so this guard inside execute can never be reached through normal dispatch. The double-gate isn't harmful, but it adds maintenance surface — future readers may update one gate and miss the other.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Reverts #5056