From 2777a382ce1dc53b59b1f7cef921fefe7edf2aa2 Mon Sep 17 00:00:00 2001 From: Bruzzz BackUp <149516937+BWJ2310-backup@users.noreply.github.com> Date: Sat, 6 Jun 2026 13:08:16 -0600 Subject: [PATCH 1/2] fix(copilot): standardize entity tool contracts and review hydration (#138) * feat(copilot): standardize entity-scoped tool contracts Unify workflow and custom tool request shapes around entity ids/documents, tighten custom tool schema validation, and thread the new provenance through client and server tool execution.\n\nCo-authored-by: Codex * feat(copilot): persist staged tool review state Add reusable staged-review handling for client tools, hydrate plan todos from persisted messages, and carry tool result data through the streaming UI. Co-authored-by: Codex * fix(i18n): update waitlist text and format JSON for consistency * feat(copilot): enhance tool validation descriptions and auto-execute eligible tools based on access level * fix(copilot): enhance tool validation and auto-execution logic for access levels * fix(copilot): streamline plan todos hydration by filtering for successful tool calls * fix(copilot): refactor tool execution logic and enhance message handling for pending tools * fix(copilot): enhance tool call handling by ensuring unique tool call IDs and updating test cases for accuracy --------- Co-authored-by: Codex --- .../copilot/chat/review-session-post.test.ts | 4 +- .../execute-copilot-server-tool/route.ts | 4 +- .../app/api/tools/custom/route.ts | 28 +- apps/tradinggoose/blocks/blocks/function.ts | 10 +- apps/tradinggoose/i18n/messages/en.json | 36 +- .../lib/copilot/entity-documents.ts | 23 +- .../lib/copilot/inline-tool-call.test.tsx | 22 +- .../lib/copilot/inline-tool-call.tsx | 35 +- apps/tradinggoose/lib/copilot/registry.ts | 66 ++-- .../runtime-tool-manifest-enrichment.ts | 296 ++++++----------- .../lib/copilot/runtime-tool-manifest.test.ts | 121 ++++--- .../lib/copilot/runtime-tool-manifest.ts | 118 +++---- .../lib/copilot/server-tool-errors.test.ts | 4 +- .../lib/copilot/server-tool-errors.ts | 4 +- .../lib/copilot/tool-prompt-metadata.ts | 27 +- .../lib/copilot/tools/client/base-tool.ts | 38 ++- .../entities/entity-document-tool-utils.ts | 29 +- .../client/entities/entity-document-tools.ts | 35 +- .../client/entities/entity-tools.test.ts | 12 +- .../client/google/gdrive-request-access.ts | 5 +- .../tools/client/monitor/list-monitors.ts | 6 +- .../client/monitor/monitor-tool-utils.ts | 2 +- .../client/monitor/monitor-tools.test.ts | 4 +- .../lib/copilot/tools/client/other/plan.ts | 1 - .../tools/client/server-tool-response.ts | 7 +- .../workflow/block-output-tools.test.ts | 10 +- .../workflow/check-deployment-status.ts | 11 +- .../client/workflow/deploy-workflow.test.ts | 16 +- .../tools/client/workflow/deploy-workflow.ts | 10 +- .../workflow/edit-workflow-block.test.ts | 5 +- .../client/workflow/edit-workflow-block.ts | 2 +- .../client/workflow/edit-workflow.test.ts | 39 ++- .../tools/client/workflow/edit-workflow.ts | 73 ++-- .../client/workflow/read-block-outputs.ts | 6 +- .../read-block-upstream-references.ts | 6 +- .../workflow/read-workflow-variables.ts | 6 +- .../tools/client/workflow/read-workflow.ts | 16 +- .../tools/client/workflow/rename-workflow.ts | 11 +- .../client/workflow/run-workflow.test.ts | 12 +- .../tools/client/workflow/run-workflow.ts | 11 +- .../workflow/set-workflow-variables.test.ts | 7 +- .../client/workflow/set-workflow-variables.ts | 22 +- .../workflow/workflow-execution-utils.ts | 1 - .../workflow/workflow-metadata-tools.test.ts | 2 +- .../workflow-review-tool-utils.test.ts | 9 +- .../workflow/workflow-review-tool-utils.ts | 9 +- .../lib/copilot/tools/entity-target.ts | 42 +++ .../agent/get-agent-accessory-catalog.test.ts | 4 +- .../agent/get-agent-accessory-catalog.ts | 6 +- .../lib/copilot/tools/server/base-tool.ts | 34 +- .../tools/server/gdrive/list-files.test.ts | 6 +- .../copilot/tools/server/gdrive/list-files.ts | 10 +- .../tools/server/gdrive/read-file.test.ts | 6 +- .../copilot/tools/server/gdrive/read-file.ts | 10 +- .../lib/copilot/tools/server/router.test.ts | 61 ++-- .../tools/server/user/read-credentials.ts | 14 +- .../user/read-environment-variables.test.ts | 3 +- .../server/user/read-environment-variables.ts | 14 +- .../server/user/read-oauth-credentials.ts | 14 +- .../server/user/set-environment-variables.ts | 10 +- .../workflow/edit-workflow-block.test.ts | 4 +- .../server/workflow/edit-workflow-block.ts | 10 +- .../server/workflow/edit-workflow.test.ts | 24 +- .../tools/server/workflow/edit-workflow.ts | 22 +- .../workflow/read-workflow-logs.test.ts | 4 +- .../server/workflow/read-workflow-logs.ts | 9 +- .../tools/server/workflow/workflow-scope.ts | 40 +++ .../lib/copilot/tools/shared/schemas.ts | 2 +- .../lib/custom-tools/import-export.ts | 43 +-- apps/tradinggoose/lib/custom-tools/schema.ts | 61 ++++ .../stores/copilot/store-messages.ts | 94 ++++++ .../stores/copilot/store-provenance.test.ts | 14 +- .../stores/copilot/store-provenance.ts | 53 ++- .../tradinggoose/stores/copilot/store.test.ts | 313 ++++++++++++++---- apps/tradinggoose/stores/copilot/store.ts | 70 ++-- apps/tradinggoose/stores/copilot/streaming.ts | 8 +- .../stores/copilot/tool-registry.test.ts | 23 +- .../stores/copilot/tool-registry.ts | 36 +- apps/tradinggoose/stores/copilot/types.ts | 14 +- .../copilot/components/copilot/copilot.tsx | 41 +-- .../components/access-level-selector.tsx | 6 +- changelog/June-06-2026.md | 86 +++++ 82 files changed, 1391 insertions(+), 1041 deletions(-) create mode 100644 apps/tradinggoose/lib/copilot/tools/entity-target.ts create mode 100644 apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts create mode 100644 apps/tradinggoose/lib/custom-tools/schema.ts create mode 100644 changelog/June-06-2026.md diff --git a/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts b/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts index af8fb22fd..1d2cd6f71 100644 --- a/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts +++ b/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts @@ -1093,7 +1093,7 @@ describe('Copilot Chat POST Generic Sessions', () => { type: 'function_call', call_id: 'tool-call-stringified', name: 'read_workflow', - arguments: JSON.stringify({ workflowId: 'wf-stringified' }), + arguments: JSON.stringify({ entityId: 'wf-stringified' }), }, }, { type: 'response.completed', response: { id: 'response-stringified-tool-args' } }, @@ -1121,7 +1121,7 @@ describe('Copilot Chat POST Generic Sessions', () => { expect.objectContaining({ id: 'tool-call-stringified', name: 'read_workflow', - arguments: { workflowId: 'wf-stringified' }, + arguments: { entityId: 'wf-stringified' }, }), ], }), diff --git a/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts b/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts index a99f30d32..d5e0a1cf7 100644 --- a/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts +++ b/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts @@ -6,6 +6,7 @@ import { createRequestTracker, createUnauthorizedResponse, } from '@/lib/copilot/auth' +import { REVIEW_ENTITY_KINDS } from '@/lib/copilot/review-sessions/types' import { buildCopilotServerToolErrorResponse } from '@/lib/copilot/server-tool-errors' import { createLogger } from '@/lib/logs/console/logger' @@ -16,7 +17,8 @@ const ExecuteSchema = z.object({ payload: z.unknown().optional(), context: z .object({ - contextWorkflowId: z.string().optional(), + contextEntityKind: z.enum(REVIEW_ENTITY_KINDS).optional(), + contextEntityId: z.string().optional(), }) .optional(), }) diff --git a/apps/tradinggoose/app/api/tools/custom/route.ts b/apps/tradinggoose/app/api/tools/custom/route.ts index cef6a7a24..7e38db129 100644 --- a/apps/tradinggoose/app/api/tools/custom/route.ts +++ b/apps/tradinggoose/app/api/tools/custom/route.ts @@ -5,6 +5,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { checkHybridAuth } from '@/lib/auth/hybrid' import { listCustomTools, upsertCustomTools } from '@/lib/custom-tools/operations' +import { CustomToolUpsertRequestSchema } from '@/lib/custom-tools/schema' import { createLogger } from '@/lib/logs/console/logger' import { getUserEntityPermissions } from '@/lib/permissions/utils' import { generateRequestId } from '@/lib/utils' @@ -12,31 +13,6 @@ import { deleteYjsSessionInSocketServer } from '@/lib/yjs/server/snapshot-bridge const logger = createLogger('CustomToolsAPI') -const CustomToolSchema = z.object({ - workspaceId: z - .string({ required_error: 'workspaceId is required' }) - .min(1, 'workspaceId is required'), - tools: z.array( - z.object({ - id: z.string().optional(), - title: z.string().min(1, 'Tool title is required'), - schema: z.object({ - type: z.literal('function'), - function: z.object({ - name: z.string().min(1, 'Function name is required'), - description: z.string().optional(), - parameters: z.object({ - type: z.string(), - properties: z.record(z.any()), - required: z.array(z.string()).optional(), - }), - }), - }), - code: z.string(), - }) - ), -}) - // GET - Fetch all custom tools for a workspace export async function GET(request: NextRequest) { const requestId = generateRequestId() @@ -109,7 +85,7 @@ export async function POST(req: NextRequest) { try { // Validate the request body - const { tools, workspaceId } = CustomToolSchema.parse(body) + const { tools, workspaceId } = CustomToolUpsertRequestSchema.parse(body) const permission = await getUserEntityPermissions(authResult.userId, 'workspace', workspaceId) if (!permission) { diff --git a/apps/tradinggoose/blocks/blocks/function.ts b/apps/tradinggoose/blocks/blocks/function.ts index c94211d58..808b9959b 100644 --- a/apps/tradinggoose/blocks/blocks/function.ts +++ b/apps/tradinggoose/blocks/blocks/function.ts @@ -7,15 +7,15 @@ export const FunctionBlock: BlockConfig = { name: 'Function', description: 'Run custom logic', longDescription: - 'This is a core workflow block. Execute custom TypeScript code within your workflow. Code transpiles to JavaScript at runtime and executes on E2B when enabled, otherwise local VM. Built-in indicators are available through indicator.(marketSeries) with full Historical Data block output.', + 'This is a core workflow block. Execute custom TypeScript code within your workflow. Code transpiles to JavaScript at runtime and executes on E2B when enabled, otherwise local VM. Available indicators are executed through indicator.(marketSeries) with full Historical Data block output.', bestPractices: ` - Write TypeScript statements only (no function wrapper). - If you need external imports, enable E2B at the environment level. - Do not define Pine indicators directly in this block (no indicator(...), PineTS, or pinets imports). - - To execute built-in indicators, call indicator.(marketSeries) with the full Historical Data output, not . Example: await indicator.RSI(). + - To execute available indicators, call indicator.(marketSeries) with the full Historical Data output, not . Example: await indicator.RSI(). - Indicator params must be passed as an object. Use saved input titles as keys, for example: await indicator.RSI(, { Length: 7 }) or await indicator.RSI(, { inputs: { Length: 7 } }). - - Use indicator.list() if you need to inspect supported built-in indicator IDs before calling one. - - Reference upstream outputs by copying exact tags like , workflow variables like , and environment variables with {{ENV_VAR_NAME}}. Avoid XML/HTML tags. + - Use indicator.list() if you need to inspect supported available indicator IDs before calling one. + - Reference upstream outputs by copying exact TradingGoose tags like , workflow variables like , and environment variables with {{ENV_VAR_NAME}}. These references are valid Function code and resolve before execution; avoid arbitrary XML/HTML tags. `, docsLink: 'https://docs.tradinggoose.ai/blocks/function', category: 'blocks', @@ -47,7 +47,7 @@ IMPORTANT FORMATTING RULES: 6. Output: Ensure the code returns a value if the function is expected to produce output. Use 'return'. 7. Clarity: Write clean, readable code. 8. No Explanations: Do NOT include markdown formatting, comments explaining the rules, or any text other than the raw TypeScript code for the function body. -9. Built-in indicators only: Do NOT define indicators directly with indicator(...) or pinets imports. Use indicator.(marketSeries) with the full Historical Data output, not . The optional second argument must be an object, e.g. await indicator.RSI(, { Length: 7 }). Use indicator.list() if the built-in ID is unknown. +9. Available indicators only: Do NOT define indicators directly with indicator(...) or pinets imports. Use indicator.(marketSeries) with the full Historical Data output, not . The optional second argument must be an object, e.g. await indicator.RSI(, { Length: 7 }). Use indicator.list() if the available ID is unknown. Example Scenario: User Prompt: "Fetch user data from an API. Use the User ID passed in as 'userId' and an API Key stored as the 'SERVICE_API_KEY' environment variable." diff --git a/apps/tradinggoose/i18n/messages/en.json b/apps/tradinggoose/i18n/messages/en.json index aae646af2..6151514f7 100644 --- a/apps/tradinggoose/i18n/messages/en.json +++ b/apps/tradinggoose/i18n/messages/en.json @@ -56,8 +56,8 @@ "auth": "Sign up" }, "waitlist": { - "primary": "Join Waitlist", - "auth": "Join waitlist" + "primary": "Get Early Access", + "auth": "Get Early Access" }, "disabled": { "primary": "Coming soon", @@ -370,8 +370,16 @@ "waitlist": "Honk! Introducing TradingGoose-Studio", "open": "Honk! TradingGoose-Studio is here!" }, - "leadWords": ["Build", "Test", "Run"], - "highlightWords": ["Trading Analysis", "Signal Detection", "Risk Assessment"], + "leadWords": [ + "Build", + "Test", + "Run" + ], + "highlightWords": [ + "Trading Analysis", + "Signal Detection", + "Risk Assessment" + ], "titleConnector": "your", "suffix": "with TradingGoose", "description": "Connect your own data providers, write custom indicators to monitor market prices, and wire them into workflows that trigger trade, sell, buy, or any action you define.", @@ -705,7 +713,13 @@ "experience": { "label": "Years of Experience *", "placeholder": "Select experience level", - "options": ["0-1 years", "1-3 years", "3-5 years", "5-10 years", "10+ years"] + "options": [ + "0-1 years", + "1-3 years", + "3-5 years", + "5-10 years", + "10+ years" + ] }, "location": { "label": "Location *", @@ -3668,7 +3682,10 @@ }, "headers": { "title": "Response Headers", - "columns": ["Key", "Value"], + "columns": [ + "Key", + "Value" + ], "description": "Additional HTTP headers to include in the response" } }, @@ -17914,7 +17931,10 @@ }, "variables": { "title": "Variables", - "columns": ["Key", "Value"] + "columns": [ + "Key", + "Value" + ] }, "apiKey": { "title": "Anthropic API Key", @@ -22570,4 +22590,4 @@ "sentLine": "This confirmation was sent on {dateTime}." } } -} +} \ No newline at end of file diff --git a/apps/tradinggoose/lib/copilot/entity-documents.ts b/apps/tradinggoose/lib/copilot/entity-documents.ts index a5b7beaa0..055e2f920 100644 --- a/apps/tradinggoose/lib/copilot/entity-documents.ts +++ b/apps/tradinggoose/lib/copilot/entity-documents.ts @@ -1,5 +1,4 @@ import { z } from 'zod' - export const SKILL_DOCUMENT_FORMAT = 'tg-skill-document-v1' as const export const CUSTOM_TOOL_DOCUMENT_FORMAT = 'tg-custom-tool-document-v1' as const export const INDICATOR_DOCUMENT_FORMAT = 'tg-indicator-document-v1' as const @@ -21,9 +20,17 @@ const SkillDocumentSchema = z.object({ }) const CustomToolDocumentSchema = z.object({ - title: z.string(), - schemaText: z.string(), - codeText: z.string(), + title: z.string().describe('Human-readable custom tool title.'), + schemaText: z + .string() + .describe( + 'JSON text for an OpenAI function tool schema: {"type":"function","function":{"name":"camelCaseName","description":"What the tool does","parameters":{"type":"object","properties":{},"required":[]}}}. This field is a string containing JSON, not an object.' + ), + codeText: z + .string() + .describe( + 'Raw JavaScript async function body only. Do not include a function wrapper, export, markdown, or imports. Use for parameters and {{ENV_VAR_NAME}} for environment variables.' + ), }) const IndicatorDocumentSchema = z.object({ @@ -83,7 +90,9 @@ function normalizeEntityFields( color: typeof source.color === 'string' ? source.color : '', pineCode: typeof source.pineCode === 'string' ? source.pineCode : '', inputMeta: - source.inputMeta && typeof source.inputMeta === 'object' && !Array.isArray(source.inputMeta) + source.inputMeta && + typeof source.inputMeta === 'object' && + !Array.isArray(source.inputMeta) ? (source.inputMeta as Record) : null, } @@ -92,7 +101,9 @@ function normalizeEntityFields( name: typeof source.name === 'string' ? source.name : '', description: typeof source.description === 'string' ? source.description : '', transport: - source.transport === 'http' || source.transport === 'sse' || source.transport === 'streamable-http' + source.transport === 'http' || + source.transport === 'sse' || + source.transport === 'streamable-http' ? source.transport : 'http', url: typeof source.url === 'string' ? source.url : '', diff --git a/apps/tradinggoose/lib/copilot/inline-tool-call.test.tsx b/apps/tradinggoose/lib/copilot/inline-tool-call.test.tsx index 00971b888..725ac0aab 100644 --- a/apps/tradinggoose/lib/copilot/inline-tool-call.test.tsx +++ b/apps/tradinggoose/lib/copilot/inline-tool-call.test.tsx @@ -13,7 +13,7 @@ const reactActEnvironment = globalThis as typeof globalThis & { } const mockUseCopilotStoreState = { - accessLevel: 'limited' as const, + accessLevel: 'limited' as 'limited' | 'full', executeCopilotToolCall: vi.fn(), skipCopilotToolCall: vi.fn(), toolCallsById: {}, @@ -59,6 +59,7 @@ describe('InlineToolCall', () => { mockGetToolInterruptDisplays.mockReset() mockUseCopilotStoreState.executeCopilotToolCall.mockReset() mockUseCopilotStoreState.skipCopilotToolCall.mockReset() + mockUseCopilotStoreState.accessLevel = 'limited' mockUseCopilotStoreState.toolCallsById = {} container = document.createElement('div') document.body.appendChild(container) @@ -132,14 +133,14 @@ describe('InlineToolCall', () => { ) }) - it('does not render a workflow preview card for non-review workflow tool states', async () => { + it('does not render a workflow preview card for active workflow tool states', async () => { await act(async () => { root.render( { expect(container.querySelector('[data-testid="workflow-preview"]')).toBeNull() }) - it('does not render a workflow preview card after edit_workflow is accepted', async () => { + it('renders the workflow preview card after edit_workflow is accepted', async () => { await act(async () => { root.render( { ) }) - expect(container.textContent).not.toContain('Blocks +') - expect(container.textContent).not.toContain('Blocks -') - expect(container.querySelector('[data-testid="workflow-preview"]')).toBeNull() + expect(container.querySelector('[data-testid="workflow-preview"]')?.textContent).toContain( + 'trigger-1' + ) }) it('renders only the workflow review for staged edit_workflow_block results', async () => { @@ -252,8 +253,9 @@ describe('InlineToolCall', () => { ) }) - it('shows review controls for staged workflow edits without generic Allow', async () => { + it('shows review controls for staged workflow edits in full access without generic Allow', async () => { const toolCallId = 'tool-workflow-review' + mockUseCopilotStoreState.accessLevel = 'full' mockGetToolInterruptDisplays.mockReturnValue({ accept: { text: 'Accept' }, reject: { text: 'Reject' }, @@ -297,7 +299,8 @@ describe('InlineToolCall', () => { expect(container.textContent).not.toContain('Allow') }) - it('renders entity review diffs from staged tool results', async () => { + it('renders entity review diffs and controls from staged tool results in full access', async () => { + mockUseCopilotStoreState.accessLevel = 'full' mockGetToolInterruptDisplays.mockReturnValue({ accept: { text: 'Accept changes' }, reject: { text: 'Reject changes' }, @@ -349,5 +352,4 @@ describe('InlineToolCall', () => { expect(container.textContent).toContain('Accept changes') expect(container.textContent).toContain('Reject changes') }) - }) diff --git a/apps/tradinggoose/lib/copilot/inline-tool-call.tsx b/apps/tradinggoose/lib/copilot/inline-tool-call.tsx index 3dfd2799e..27678606c 100644 --- a/apps/tradinggoose/lib/copilot/inline-tool-call.tsx +++ b/apps/tradinggoose/lib/copilot/inline-tool-call.tsx @@ -205,29 +205,29 @@ function shouldShowToolActionButtons( ) } -function isEntityMutationTool(toolName: string): boolean { +function isStagedPreviewState(state: ClientToolCallState): boolean { + return state === ClientToolCallState.review || state === ClientToolCallState.success +} + +function isEntityReviewKind(entityKind: unknown): entityKind is string { return ( - toolName === 'create_skill' || - toolName === 'edit_skill' || - toolName === 'rename_skill' || - toolName === 'create_custom_tool' || - toolName === 'edit_custom_tool' || - toolName === 'rename_custom_tool' || - toolName === 'create_indicator' || - toolName === 'edit_indicator' || - toolName === 'rename_indicator' || - toolName === 'create_mcp_server' || - toolName === 'edit_mcp_server' || - toolName === 'rename_mcp_server' + entityKind === 'skill' || + entityKind === 'custom_tool' || + entityKind === 'indicator' || + entityKind === 'mcp_server' ) } function readEntityReviewPayload(toolCall: CopilotToolCall): EntityReviewPayload | null { - if (!isEntityMutationTool(toolCall.name) || toolCall.state !== ClientToolCallState.review) { + if (!isCopilotTool(toolCall.name) || !isStagedPreviewState(toolCall.state)) { return null } const result = toolCall.result && typeof toolCall.result === 'object' ? toolCall.result : null + if (!isEntityReviewKind(result?.entityKind)) { + return null + } + const documentDiff = result?.preview?.documentDiff if ( !documentDiff || @@ -249,7 +249,10 @@ function readEntityReviewPayload(toolCall: CopilotToolCall): EntityReviewPayload ? 'Skill' : 'Entity' return { - title: `Proposed ${entityLabel} Changes`, + title: + toolCall.state === ClientToolCallState.success + ? `Applied ${entityLabel} Changes` + : `Proposed ${entityLabel} Changes`, documentDiff, } } @@ -418,7 +421,7 @@ export function InlineToolCall({ const params = (toolCall as any).parameters || (toolCall as any).input || toolCall.params || {} const entityReviewPayload = readEntityReviewPayload(toolCall) const workflowReviewPayload = readWorkflowReviewPayload(toolCall) - const showWorkflowReview = workflowReviewPayload && toolCall.state === ClientToolCallState.review + const showWorkflowReview = workflowReviewPayload && isStagedPreviewState(toolCall.state) const renderPendingDetails = () => { if (toolCall.name === 'make_api_request') { diff --git a/apps/tradinggoose/lib/copilot/registry.ts b/apps/tradinggoose/lib/copilot/registry.ts index 86239eb00..f90382775 100644 --- a/apps/tradinggoose/lib/copilot/registry.ts +++ b/apps/tradinggoose/lib/copilot/registry.ts @@ -7,10 +7,7 @@ import { } from '@/lib/copilot/entity-documents' import { MONITOR_DOCUMENT_FORMAT } from '@/lib/copilot/monitor/monitor-documents' import { TG_MERMAID_DOCUMENT_FORMAT } from '@/lib/workflows/document-format' -import { - WORKFLOW_VARIABLE_TYPES, - type WorkflowVariableType, -} from '@/lib/workflows/value-types' +import { WORKFLOW_VARIABLE_TYPES, type WorkflowVariableType } from '@/lib/workflows/value-types' import { GetAgentAccessoryCatalogInput, GetAgentAccessoryCatalogResult, @@ -116,8 +113,10 @@ const ToolCallSSEBase = z.object({ const BooleanOptional = z.boolean().optional() const NumberOptional = z.number().optional() const RequiredId = z.string().trim().min(1) -const WorkflowContextArgs = z.object({ - workflowId: z.string().optional(), +const CUSTOM_TOOL_DOCUMENT_ARGUMENT_DESCRIPTION = + 'Full `tg-custom-tool-document-v1` JSON document with exactly `title`, `schemaText`, and `codeText`. `schemaText` is a JSON-encoded string, not an object, for an OpenAI function tool schema: {"type":"function","function":{"name":"camelCaseName","description":"What the tool does","parameters":{"type":"object","properties":{},"required":[]}}}. `codeText` is raw async JavaScript function body only; use for inputs and {{ENV_VAR_NAME}} for environment variables.' +const OptionalEntityTargetArgs = z.object({ + entityId: z.string().optional(), }) const EntityTargetArgs = z.object({ entityId: RequiredId, @@ -157,21 +156,21 @@ const CreateWorkflowArgs = z const RenameWorkflowArgs = z .object({ - workflowId: RequiredId, + entityId: RequiredId, name: z.string().trim().min(1), }) .strict() const EditWorkflowArgs = z .object({ - workflowDocument: z + entityDocument: z .string() .min(1) .describe( - 'Complete raw `tg-mermaid-v1` Mermaid document for the entire workflow, not a partial patch. Preserve unchanged canonical `%% TG_BLOCK` and `%% TG_EDGE` entries. Use this only for graph or topology changes such as adding, removing, reconnecting, or replacing blocks, loops, parallels, or condition branches.' + 'Complete raw `tg-mermaid-v1` Mermaid document for the entire workflow, not a partial patch. Preserve the canonical `%% TG_WORKFLOW`, `%% TG_BLOCK`, and `%% TG_EDGE` metadata returned by `read_workflow`; Studio validates that structure. Use this only for graph or topology changes such as adding, removing, reconnecting, or replacing blocks, loops, parallels, or condition branches.' ), documentFormat: z.literal(TG_MERMAID_DOCUMENT_FORMAT).optional(), - workflowId: RequiredId, + entityId: RequiredId, }) .strict() .describe( @@ -180,7 +179,7 @@ const EditWorkflowArgs = z const EditWorkflowBlockArgs = z .object({ - workflowId: RequiredId, + entityId: RequiredId, blockId: z .string() .trim() @@ -208,8 +207,17 @@ const EditWorkflowBlockArgs = z 'Single-block patch tool. Default to this when only one existing block needs a `name`, `enabled`, or `subBlocks` change and the workflow graph stays the same.' ) -const EditCustomToolArgs = buildEntityDocumentMutationArgs(CUSTOM_TOOL_DOCUMENT_FORMAT) -const CreateCustomToolArgs = buildEntityDocumentCreateArgs(CUSTOM_TOOL_DOCUMENT_FORMAT) +const CustomToolDocumentMutationShape = { + entityDocument: z.string().min(1).describe(CUSTOM_TOOL_DOCUMENT_ARGUMENT_DESCRIPTION), + documentFormat: z.literal(CUSTOM_TOOL_DOCUMENT_FORMAT).optional(), +} +const EditCustomToolArgs = EntityTargetArgs.extend(CustomToolDocumentMutationShape) + .strict() + .describe('Update a saved custom tool by replacing the full custom-tool document.') +const CreateCustomToolArgs = z + .object(CustomToolDocumentMutationShape) + .strict() + .describe('Create a custom tool from the full custom-tool document.') const GetIndicatorArgs = z .object({ entityId: RequiredId.optional(), @@ -263,16 +271,16 @@ export const ToolArgSchemas = { }), [CopilotTool.read_workflow]: z .object({ - workflowId: RequiredId, + entityId: RequiredId, }) .strict(), create_workflow: CreateWorkflowArgs, [CopilotTool.list_workflows]: z.object({}), [CopilotTool.read_workflow_variables]: z.object({ - workflowId: RequiredId, + entityId: RequiredId, }), [CopilotTool.set_workflow_variables]: z.object({ - workflowId: RequiredId, + entityId: RequiredId, operations: z.array( z.object({ operation: z.enum(['add', 'delete', 'edit']), @@ -288,10 +296,10 @@ export const ToolArgSchemas = { deploy_workflow: z.object({ action: z.enum(['deploy', 'undeploy']).optional().default('deploy'), deployType: z.enum(['api', 'chat']).optional().default('api'), - workflowId: RequiredId, + entityId: RequiredId, }), check_deployment_status: z.object({ - workflowId: RequiredId, + entityId: RequiredId, }), edit_workflow: EditWorkflowArgs, @@ -299,12 +307,12 @@ export const ToolArgSchemas = { rename_workflow: RenameWorkflowArgs, run_workflow: z.object({ - workflowId: RequiredId, + entityId: RequiredId, workflow_input: z.union([z.string(), z.record(z.any())]).optional(), }), [CopilotTool.read_workflow_logs]: z.object({ - workflowId: RequiredId, + entityId: RequiredId, limit: NumberOptional, includeDetails: BooleanOptional, }), @@ -340,19 +348,19 @@ export const ToolArgSchemas = { body: z.union([z.record(z.any()), z.string()]).optional(), }), - [CopilotTool.read_environment_variables]: WorkflowContextArgs, + [CopilotTool.read_environment_variables]: OptionalEntityTargetArgs, - set_environment_variables: WorkflowContextArgs.extend({ + set_environment_variables: OptionalEntityTargetArgs.extend({ variables: z.record(z.string()), }), - [CopilotTool.read_oauth_credentials]: WorkflowContextArgs, + [CopilotTool.read_oauth_credentials]: OptionalEntityTargetArgs, - [CopilotTool.read_credentials]: WorkflowContextArgs, + [CopilotTool.read_credentials]: OptionalEntityTargetArgs, gdrive_request_access: z.object({}), - list_gdrive_files: WorkflowContextArgs.extend({ + list_gdrive_files: OptionalEntityTargetArgs.extend({ credentialId: z.string(), search_query: z.string().optional(), num_results: z.number().optional().default(50), @@ -363,7 +371,7 @@ export const ToolArgSchemas = { fileId: z.string(), type: z.enum(['doc', 'sheet']), range: z.string().optional(), - workflowId: z.string().optional(), + entityId: z.string().optional(), }), knowledge_base: KnowledgeBaseArgsSchema, @@ -375,7 +383,7 @@ export const ToolArgSchemas = { rename_custom_tool: EditCustomToolArgs, list_monitors: z.object({ - workflowId: z.string().optional(), + entityId: z.string().optional(), blockId: z.string().optional(), }), [CopilotTool.read_monitor]: z.object({ @@ -414,11 +422,11 @@ export const ToolArgSchemas = { }), [CopilotTool.read_block_outputs]: ReadBlockOutputsInput.extend({ - workflowId: RequiredId, + entityId: RequiredId, }), [CopilotTool.read_block_upstream_references]: ReadBlockUpstreamReferencesInput.extend({ - workflowId: RequiredId, + entityId: RequiredId, }), } as const diff --git a/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts b/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts index e35873998..a87d790ce 100644 --- a/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts +++ b/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts @@ -1,4 +1,4 @@ -import { z } from 'zod' +import type { z } from 'zod' import { zodToJsonSchema } from 'zod-to-json-schema' import { CUSTOM_TOOL_DOCUMENT_FORMAT, @@ -8,59 +8,18 @@ import { SKILL_DOCUMENT_FORMAT, } from '@/lib/copilot/entity-documents' import { - type EmbeddedDocumentValidator, - type RuntimeToolManifestSemanticValidator, -} from '@/lib/copilot/workflow-subblock-semantic-contracts' -import { MONITOR_DOCUMENT_FORMAT, MonitorDocumentSchema } from '@/lib/copilot/monitor/monitor-documents' + MONITOR_DOCUMENT_FORMAT, + MonitorDocumentSchema, +} from '@/lib/copilot/monitor/monitor-documents' +import type { RuntimeToolManifestSemanticValidator } from '@/lib/copilot/workflow-subblock-semantic-contracts' import { TG_MERMAID_DOCUMENT_FORMAT } from '@/lib/workflows/document-format' export type { RuntimeToolManifestSemanticValidator } from '@/lib/copilot/workflow-subblock-semantic-contracts' -type WorkflowGraphContractSpec = { - canonicalBlock: { - idPath: string - typePath: string - parentIdPath: string - } - visibleOverlay: { - idLabel: string - typeLabel: string - } - inferParentsFromContainerSubgraphs: boolean - containers: Array<{ - blockType: string - startNodeSuffix: string - endNodeSuffix: string - startSourceHandle: string - endSourceHandle: string - endTargetHandle: string - }> - conditionalBranches: Array<{ - blockType: string - handlePrefix: string - branchNodeSeparator: string - }> -} - -type AnnotatedGraphDocumentContract = { - type: 'annotated_graph' - nodePrefix: string - edgePrefix: string - spec: WorkflowGraphContractSpec - embeddedValidators?: EmbeddedDocumentValidator[] -} - type DocumentSemanticSpecDefinition = { documentFormat: string preferredDocumentField: string - buildSemanticValidators: ( - documentField: string, - options?: AutomaticSemanticValidatorOptions - ) => RuntimeToolManifestSemanticValidator[] -} - -type AutomaticSemanticValidatorOptions = { - workflowEmbeddedValidators?: EmbeddedDocumentValidator[] + buildSemanticValidators: (documentField: string) => RuntimeToolManifestSemanticValidator[] } function toJsonSchemaRecord(schema: z.ZodTypeAny): Record { @@ -115,58 +74,57 @@ const JSON_DOCUMENT_SPECS: JsonDocumentSemanticSpec[] = [ }, ] -export const WORKFLOW_GRAPH_CONTRACT_SPEC: WorkflowGraphContractSpec = { - canonicalBlock: { - idPath: 'id', - typePath: 'type', - parentIdPath: 'data.parentId', +const TG_WORKFLOW_LINE_PREFIX = '%% TG_WORKFLOW ' +const TG_BLOCK_LINE_PREFIX = '%% TG_BLOCK ' +const TG_EDGE_LINE_PREFIX = '%% TG_EDGE ' + +const TG_WORKFLOW_METADATA_SCHEMA: Record = { + type: 'object', + required: ['version', 'direction'], + additionalProperties: true, + properties: { + version: { const: TG_MERMAID_DOCUMENT_FORMAT }, + direction: { enum: ['TD', 'LR'] }, }, - visibleOverlay: { - idLabel: 'id', - typeLabel: 'type', +} + +const TG_POSITION_SCHEMA: Record = { + type: 'object', + required: ['x', 'y'], + additionalProperties: true, + properties: { + x: { type: 'number' }, + y: { type: 'number' }, }, - inferParentsFromContainerSubgraphs: true, - containers: [ - { - blockType: 'loop', - startNodeSuffix: '__loop_start', - endNodeSuffix: '__loop_end', - startSourceHandle: 'loop-start-source', - endSourceHandle: 'loop-end-source', - endTargetHandle: 'loop-end-target', - }, - { - blockType: 'parallel', - startNodeSuffix: '__parallel_start', - endNodeSuffix: '__parallel_end', - startSourceHandle: 'parallel-start-source', - endSourceHandle: 'parallel-end-source', - endTargetHandle: 'parallel-end-target', - }, - ], - conditionalBranches: [ - { - blockType: 'condition', - handlePrefix: 'condition-', - branchNodeSeparator: '__condition_', - }, - ], } -export function buildWorkflowDocumentContract( - embeddedValidators?: EmbeddedDocumentValidator[] -): AnnotatedGraphDocumentContract { - return { - type: 'annotated_graph', - nodePrefix: '%% TG_BLOCK ', - edgePrefix: '%% TG_EDGE ', - spec: WORKFLOW_GRAPH_CONTRACT_SPEC, - ...(embeddedValidators && embeddedValidators.length > 0 ? { embeddedValidators } : {}), - } +const TG_BLOCK_SCHEMA: Record = { + type: 'object', + required: ['id', 'type', 'name', 'position', 'subBlocks', 'outputs', 'enabled'], + additionalProperties: true, + properties: { + id: { type: 'string' }, + type: { type: 'string' }, + name: { type: 'string' }, + position: TG_POSITION_SCHEMA, + subBlocks: { type: 'object' }, + outputs: { type: 'object' }, + enabled: { type: 'boolean' }, + }, } -export const WORKFLOW_DOCUMENT_CONTRACT: AnnotatedGraphDocumentContract = - buildWorkflowDocumentContract() +const TG_EDGE_SCHEMA: Record = { + type: 'object', + required: ['source', 'target'], + additionalProperties: true, + properties: { + id: { type: 'string' }, + source: { type: 'string' }, + target: { type: 'string' }, + sourceHandle: { type: 'string' }, + targetHandle: { type: 'string' }, + }, +} function getObjectPropertySchema( parameters: Record, @@ -194,14 +152,14 @@ function getConstStringValue(propertySchema: Record | null): st } function buildWorkflowDocumentSemanticValidators( - documentField: string, - workflowEmbeddedValidators?: EmbeddedDocumentValidator[] + documentField: string ): RuntimeToolManifestSemanticValidator[] { return [ { path: documentField, kind: 'string_requires_real_newlines', - description: 'Use raw Mermaid text with real newlines.', + description: + 'Cheap format guard only. Use raw Mermaid text with real newlines; Studio validates TG_WORKFLOW, TG_BLOCK, and edge consistency.', message: 'Expected raw Mermaid text with real newline characters, not JSON-escaped `\\n` sequences.', }, @@ -209,122 +167,81 @@ function buildWorkflowDocumentSemanticValidators( path: documentField, kind: 'string_starts_with', args: { prefix: 'flowchart ' }, - description: 'Start with a Mermaid `flowchart` declaration.', + description: + 'Cheap format guard only. Start with a Mermaid `flowchart` declaration; Studio validates canonical workflow structure.', message: 'Expected raw Mermaid text that starts with a `flowchart` declaration.', }, { path: documentField, kind: 'string_requires_line_prefix', - args: { prefix: '%% TG_WORKFLOW ', minMatches: 1 }, - description: 'Include a standalone `%% TG_WORKFLOW {...}` line.', - message: - 'Missing a standalone `%% TG_WORKFLOW {...}` metadata line. Keep it on its own line near the top of the document.', + args: { prefix: TG_WORKFLOW_LINE_PREFIX, minMatches: 1 }, + description: 'Include a standalone canonical `%% TG_WORKFLOW {...}` metadata line.', + message: 'Workflow documents must include a standalone `%% TG_WORKFLOW {...}` metadata line.', }, { path: documentField, - kind: 'string_line_prefix_json_schema', - args: { - prefix: '%% TG_WORKFLOW ', - minMatches: 0, - schema: { - type: 'object', - required: ['version', 'direction'], - additionalProperties: true, - properties: { - version: { const: 'tg-mermaid-v1' }, - direction: { enum: ['TD', 'LR'] }, - }, - }, - }, - description: 'Use canonical `TG_WORKFLOW` JSON metadata.', + kind: 'string_requires_line_prefix', + args: { prefix: TG_BLOCK_LINE_PREFIX, minMatches: 1 }, + description: 'Include standalone canonical `%% TG_BLOCK {...}` metadata lines.', + message: 'Workflow documents must include standalone `%% TG_BLOCK {...}` metadata lines.', }, { path: documentField, - kind: 'string_requires_line_prefix', - args: { prefix: '%% TG_BLOCK ', minMatches: 1 }, - description: 'Include standalone canonical `%% TG_BLOCK {...}` lines.', + kind: 'string_line_prefix_json_schema', + args: { prefix: TG_WORKFLOW_LINE_PREFIX, schema: TG_WORKFLOW_METADATA_SCHEMA }, + description: 'Validate each `TG_WORKFLOW` metadata JSON payload.', message: - 'Workflow document did not contain any standalone `%% TG_BLOCK {...}` block entries. Do not embed `TG_BLOCK` JSON inside node labels.', + '`TG_WORKFLOW` metadata must be canonical JSON with `version: "tg-mermaid-v1"` and `direction` of `TD` or `LR`.', }, { path: documentField, kind: 'string_line_prefix_json_schema', - args: { - prefix: '%% TG_BLOCK ', - minMatches: 0, - schema: { - type: 'object', - required: ['id', 'type', 'name', 'position', 'subBlocks', 'outputs', 'enabled'], - additionalProperties: true, - properties: { - id: { type: 'string' }, - type: { type: 'string' }, - name: { type: 'string' }, - position: { - type: 'object', - required: ['x', 'y'], - additionalProperties: true, - properties: { - x: { type: 'number' }, - y: { type: 'number' }, - }, - }, - subBlocks: { type: 'object', additionalProperties: true }, - outputs: { type: 'object', additionalProperties: true }, - enabled: { type: 'boolean' }, - }, - }, - }, - description: 'Use canonical `TG_BLOCK` JSON state.', + args: { prefix: TG_BLOCK_LINE_PREFIX, schema: TG_BLOCK_SCHEMA }, + description: 'Validate each `TG_BLOCK` metadata JSON payload.', + message: + '`TG_BLOCK` metadata must be canonical block state with `id`, `type`, `name`, `position`, `subBlocks`, `outputs`, and `enabled`.', }, { path: documentField, - kind: 'string_forbids_substring', - args: { substring: '$$TG_BLOCK' }, - description: 'Do not embed `TG_BLOCK` JSON inside labels.', - message: 'Do not embed `TG_BLOCK` JSON inside node labels. Emit `%% TG_BLOCK {...}` on its own line.', + kind: 'string_line_prefix_json_schema', + args: { prefix: TG_EDGE_LINE_PREFIX, schema: TG_EDGE_SCHEMA }, + description: 'Validate each `TG_EDGE` metadata JSON payload when edge lines are present.', + message: '`TG_EDGE` metadata must be canonical edge state with string `source` and `target`.', }, { path: documentField, kind: 'string_forbids_substring', - args: { substring: '"blockType":' }, - description: 'Use canonical `TG_BLOCK.type`.', - message: - 'Workflow documents use canonical `TG_BLOCK.type`, not metadata aliases like `blockType`.', + args: { substring: '"blockType"' }, + description: 'Use canonical `TG_BLOCK.type`, not simplified block metadata aliases.', + message: 'Use `type` in `TG_BLOCK` metadata, not `blockType`.', }, { path: documentField, kind: 'string_forbids_substring', - args: { substring: '"blockName":' }, - description: 'Use canonical `TG_BLOCK.name`.', - message: - 'Workflow documents use canonical `TG_BLOCK.name`, not metadata aliases like `blockName`.', + args: { substring: '"blockName"' }, + description: 'Use canonical `TG_BLOCK.name`, not simplified block metadata aliases.', + message: 'Use `name` in `TG_BLOCK` metadata, not `blockName`.', }, { path: documentField, - kind: 'string_document_contract', - args: { - contract: buildWorkflowDocumentContract(workflowEmbeddedValidators), - }, - description: 'Keep visible edges and canonical `TG_EDGE` state aligned.', + kind: 'string_forbids_substring', + args: { substring: '"blockDescription"' }, + description: 'Use canonical `TG_BLOCK` state, not simplified block metadata aliases.', + message: '`TG_BLOCK` metadata must not include `blockDescription`.', }, { path: documentField, - kind: 'string_line_prefix_json_schema', + kind: 'string_document_contract', args: { - prefix: '%% TG_EDGE ', - minMatches: 0, - schema: { - type: 'object', - required: ['source', 'target'], - additionalProperties: true, - properties: { - source: { type: 'string' }, - target: { type: 'string' }, - }, - }, + format: TG_MERMAID_DOCUMENT_FORMAT, + workflowPrefix: TG_WORKFLOW_LINE_PREFIX, + blockPrefix: TG_BLOCK_LINE_PREFIX, + edgePrefix: TG_EDGE_LINE_PREFIX, }, - description: 'Use canonical `TG_EDGE` JSON state.', + description: + 'Keep visible Mermaid connection lines aligned with canonical `TG_EDGE` metadata.', + message: + 'Visible Mermaid connections and `TG_EDGE` metadata must describe the same logical workflow edges.', }, ] } @@ -354,12 +271,8 @@ function buildJsonDocumentSemanticValidators( const DOCUMENT_SEMANTIC_SPECS = [ { documentFormat: TG_MERMAID_DOCUMENT_FORMAT, - preferredDocumentField: 'workflowDocument', - buildSemanticValidators: (documentField, options) => - buildWorkflowDocumentSemanticValidators( - documentField, - options?.workflowEmbeddedValidators - ), + preferredDocumentField: 'entityDocument', + buildSemanticValidators: buildWorkflowDocumentSemanticValidators, }, ...JSON_DOCUMENT_SPECS.map((spec) => ({ documentFormat: spec.documentFormat, @@ -395,11 +308,13 @@ function detectDocumentField( const matchingFields = Object.entries(properties as Record) .filter(([fieldName, propertySchema]) => { if (!fieldName.endsWith('Document')) return false - return getSchemaType( - propertySchema && typeof propertySchema === 'object' && !Array.isArray(propertySchema) - ? (propertySchema as Record) - : null - ) === 'string' + return ( + getSchemaType( + propertySchema && typeof propertySchema === 'object' && !Array.isArray(propertySchema) + ? (propertySchema as Record) + : null + ) === 'string' + ) }) .map(([fieldName]) => fieldName) @@ -407,8 +322,7 @@ function detectDocumentField( } export function buildAutomaticSemanticValidators( - parameters: Record, - options?: AutomaticSemanticValidatorOptions + parameters: Record ): RuntimeToolManifestSemanticValidator[] { const documentFormat = getConstStringValue(getObjectPropertySchema(parameters, 'documentFormat')) const semanticSpec = documentFormat ? DOCUMENT_SEMANTIC_SPEC_BY_FORMAT.get(documentFormat) : null @@ -421,5 +335,5 @@ export function buildAutomaticSemanticValidators( return [] } - return semanticSpec.buildSemanticValidators(documentField, options) + return semanticSpec.buildSemanticValidators(documentField) } diff --git a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts index 20605f60c..1d50abee3 100644 --- a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts +++ b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest' import { getCopilotRuntimeToolManifest } from '@/lib/copilot/runtime-tool-manifest' -import { WORKFLOW_DOCUMENT_CONTRACT } from '@/lib/copilot/runtime-tool-manifest-enrichment' describe('copilot runtime tool manifest', () => { it('exposes the Studio tool surface and workflow document validators', async () => { @@ -15,7 +14,7 @@ describe('copilot runtime tool manifest', () => { name: 'read_workflow', description: expect.stringContaining('connections` counts'), parameters: expect.objectContaining({ - required: expect.arrayContaining(['workflowId']), + required: expect.arrayContaining(['entityId']), }), }), expect.objectContaining({ @@ -150,24 +149,23 @@ describe('copilot runtime tool manifest', () => { entityKind: 'workflow', semanticValidators: expect.arrayContaining([ expect.objectContaining({ - path: 'workflowDocument', + path: 'entityDocument', kind: 'string_requires_real_newlines', + description: expect.stringContaining('Cheap format guard only'), }), expect.objectContaining({ - path: 'workflowDocument', - kind: 'string_document_contract', - }), - expect.objectContaining({ - path: 'workflowDocument', - kind: 'string_line_prefix_json_schema', - args: expect.any(Object), + path: 'entityDocument', + kind: 'string_starts_with', + args: { prefix: 'flowchart ' }, }), ]), parameters: expect.objectContaining({ type: 'object', + required: expect.arrayContaining(['entityId', 'entityDocument']), properties: expect.objectContaining({ - workflowDocument: expect.objectContaining({ - description: expect.stringContaining('not a partial patch'), + entityId: expect.any(Object), + entityDocument: expect.objectContaining({ + description: expect.stringContaining('%% TG_WORKFLOW'), }), }), }), @@ -178,7 +176,7 @@ describe('copilot runtime tool manifest', () => { kind: 'edit', entityKind: 'workflow', parameters: expect.objectContaining({ - required: expect.arrayContaining(['workflowId', 'blockId']), + required: expect.arrayContaining(['entityId', 'blockId']), properties: expect.objectContaining({ subBlocks: expect.objectContaining({ description: expect.stringContaining('Partial patch for the selected block only'), @@ -198,7 +196,7 @@ describe('copilot runtime tool manifest', () => { kind: 'rename', entityKind: 'workflow', parameters: expect.objectContaining({ - required: expect.arrayContaining(['workflowId', 'name']), + required: expect.arrayContaining(['entityId', 'name']), }), }), expect.objectContaining({ @@ -232,10 +230,37 @@ describe('copilot runtime tool manifest', () => { }), ]), }), + expect.objectContaining({ + name: 'create_custom_tool', + description: expect.stringContaining('schemaText'), + kind: 'create', + entityKind: 'custom_tool', + parameters: expect.objectContaining({ + properties: expect.objectContaining({ + entityDocument: expect.objectContaining({ + description: expect.stringContaining('JSON-encoded string'), + }), + }), + }), + semanticValidators: expect.arrayContaining([ + expect.objectContaining({ + path: 'entityDocument', + kind: 'string_json_schema', + args: expect.any(Object), + }), + ]), + }), expect.objectContaining({ name: 'edit_custom_tool', + description: expect.stringContaining('schemaText'), kind: 'edit', entityKind: 'custom_tool', + semanticValidators: expect.arrayContaining([ + expect.objectContaining({ + path: 'entityDocument', + kind: 'string_json_schema', + }), + ]), }), expect.objectContaining({ name: 'rename_mcp_server', @@ -256,49 +281,51 @@ describe('copilot runtime tool manifest', () => { }), ]) ) - const edgeValidator = manifest.tools - .find((tool) => tool.name === 'edit_workflow') - ?.semanticValidators?.find((validator) => validator.kind === 'string_document_contract') - expect(edgeValidator).toBeDefined() - expect(edgeValidator?.description).toBe( - 'Keep visible edges and canonical `TG_EDGE` state aligned.' - ) - expect(edgeValidator?.args).toEqual( - expect.objectContaining({ - contract: expect.objectContaining(WORKFLOW_DOCUMENT_CONTRACT), - }) + const editWorkflowValidators = + manifest.tools.find((tool) => tool.name === 'edit_workflow')?.semanticValidators ?? [] + const workflowValidatorKinds = editWorkflowValidators.map((validator) => validator.kind) + expect(workflowValidatorKinds).toEqual( + expect.arrayContaining([ + 'string_requires_real_newlines', + 'string_starts_with', + 'string_requires_line_prefix', + 'string_line_prefix_json_schema', + 'string_forbids_substring', + 'string_document_contract', + ]) ) - expect( - ( - edgeValidator?.args as - | { - contract?: { embeddedValidators?: Array<{ whenBlockType: string; path: string }> } - } - | undefined - )?.contract?.embeddedValidators - ).toEqual( + expect(editWorkflowValidators).toEqual( expect.arrayContaining([ expect.objectContaining({ - whenBlockType: 'agent', - path: 'subBlocks.responseFormat.value', + kind: 'string_requires_line_prefix', + args: { prefix: '%% TG_WORKFLOW ', minMatches: 1 }, }), expect.objectContaining({ - whenBlockType: 'function', - path: 'subBlocks.code.value', + kind: 'string_requires_line_prefix', + args: { prefix: '%% TG_BLOCK ', minMatches: 1 }, }), expect.objectContaining({ - whenBlockType: 'condition', - path: 'subBlocks.conditions.value', + kind: 'string_line_prefix_json_schema', + args: expect.objectContaining({ prefix: '%% TG_EDGE ', schema: expect.any(Object) }), + }), + expect.objectContaining({ + kind: 'string_document_contract', + args: expect.objectContaining({ + workflowPrefix: '%% TG_WORKFLOW ', + blockPrefix: '%% TG_BLOCK ', + edgePrefix: '%% TG_EDGE ', + }), }), ]) ) - expect( - manifest.tools - .find((tool) => tool.name === 'edit_workflow') - ?.semanticValidators?.some( - (validator) => validator.kind === 'string_requires_line_prefix_if_substring_present' - ) - ).toBe(false) + const editWorkflowProperties = + (manifest.tools.find((tool) => tool.name === 'edit_workflow')?.parameters?.properties as + | Record + | undefined) ?? {} + expect(editWorkflowProperties).toHaveProperty('entityId') + expect(editWorkflowProperties).toHaveProperty('entityDocument') + expect(editWorkflowProperties).not.toHaveProperty('workflowId') + expect(editWorkflowProperties).not.toHaveProperty('workflowDocument') expect( manifest.tools.find((tool) => tool.name === 'edit_workflow_block')?.description ).toContain('without changing workflow connections') diff --git a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts index 269ecce75..7aecaa1b2 100644 --- a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts +++ b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts @@ -1,91 +1,73 @@ +import { zodToJsonSchema } from 'zod-to-json-schema' +import { ToolArgSchemas, type ToolId, ToolIds } from '@/lib/copilot/registry' import { - ToolArgSchemas, - ToolIds, - type ToolId, -} from "@/lib/copilot/registry"; -import { TOOL_PROMPT_METADATA } from "@/lib/copilot/tool-prompt-metadata"; -import { - buildAutomaticSemanticValidators, - type RuntimeToolManifestSemanticValidator, -} from "@/lib/copilot/runtime-tool-manifest-enrichment"; -import type { EmbeddedDocumentValidator } from "@/lib/copilot/workflow-subblock-semantic-contracts"; -import { zodToJsonSchema } from "zod-to-json-schema"; + buildAutomaticSemanticValidators, + type RuntimeToolManifestSemanticValidator, +} from '@/lib/copilot/runtime-tool-manifest-enrichment' +import { TOOL_PROMPT_METADATA } from '@/lib/copilot/tool-prompt-metadata' -export const COPILOT_RUNTIME_TOOL_MANIFEST_VERSION = "v1" as const; +export const COPILOT_RUNTIME_TOOL_MANIFEST_VERSION = 'v1' as const export interface CopilotRuntimeToolManifestTool { - name: string; - description: string; - parameters?: Record; - semanticValidators?: RuntimeToolManifestSemanticValidator[]; - kind?: string; - entityKind?: string; - surfaceKind?: string; + name: string + description: string + parameters?: Record + semanticValidators?: RuntimeToolManifestSemanticValidator[] + kind?: string + entityKind?: string + surfaceKind?: string } export interface CopilotRuntimeToolManifest { - version: typeof COPILOT_RUNTIME_TOOL_MANIFEST_VERSION; - tools: CopilotRuntimeToolManifestTool[]; + version: typeof COPILOT_RUNTIME_TOOL_MANIFEST_VERSION + tools: CopilotRuntimeToolManifestTool[] } const buildToolParameterSchema = (toolId: ToolId): Record => { - const schema = zodToJsonSchema(ToolArgSchemas[toolId], { - $refStrategy: "none", - target: "jsonSchema7", - }); + const schema = zodToJsonSchema(ToolArgSchemas[toolId], { + $refStrategy: 'none', + target: 'jsonSchema7', + }) - if (!schema || typeof schema !== "object" || Array.isArray(schema)) { - return { - type: "object", - properties: {}, - additionalProperties: true, - }; - } + if (!schema || typeof schema !== 'object' || Array.isArray(schema)) { + return { + type: 'object', + properties: {}, + additionalProperties: true, + } + } - const { $schema, definitions, ...parameters } = schema as Record< - string, - unknown - >; - return parameters; -}; + const { $schema, definitions, ...parameters } = schema as Record + return parameters +} -const TOOL_NAMES = ToolIds.options; +const TOOL_NAMES = ToolIds.options function getSemanticValidators( - parameters: Record, - options?: { - workflowEmbeddedValidators?: EmbeddedDocumentValidator[]; - }, + parameters: Record ): RuntimeToolManifestSemanticValidator[] | undefined { - const semanticValidators = buildAutomaticSemanticValidators(parameters, options); + const semanticValidators = buildAutomaticSemanticValidators(parameters) - if (semanticValidators.length === 0) { - return undefined; - } + if (semanticValidators.length === 0) { + return undefined + } - return semanticValidators; + return semanticValidators } export async function getCopilotRuntimeToolManifest(): Promise { - const { buildWorkflowEmbeddedDocumentValidators } = await import( - "@/lib/copilot/workflow-subblock-semantic-contracts" - ); - const workflowEmbeddedValidators = await buildWorkflowEmbeddedDocumentValidators(); - - return { - version: COPILOT_RUNTIME_TOOL_MANIFEST_VERSION, - tools: TOOL_NAMES.map((toolName) => { - const parameters = buildToolParameterSchema(toolName); - const semanticValidators = getSemanticValidators(parameters, { - workflowEmbeddedValidators, - }); + return { + version: COPILOT_RUNTIME_TOOL_MANIFEST_VERSION, + tools: TOOL_NAMES.map((toolName) => { + const parameters = buildToolParameterSchema(toolName) + const semanticValidators = getSemanticValidators(parameters) - return { - name: toolName, - ...TOOL_PROMPT_METADATA[toolName], - ...(semanticValidators ? { semanticValidators } : {}), - parameters, - }; - }), - }; + return { + name: toolName, + ...TOOL_PROMPT_METADATA[toolName], + ...(semanticValidators ? { semanticValidators } : {}), + parameters, + } + }), + } } diff --git a/apps/tradinggoose/lib/copilot/server-tool-errors.test.ts b/apps/tradinggoose/lib/copilot/server-tool-errors.test.ts index bbe621102..7ef23c7ae 100644 --- a/apps/tradinggoose/lib/copilot/server-tool-errors.test.ts +++ b/apps/tradinggoose/lib/copilot/server-tool-errors.test.ts @@ -57,7 +57,7 @@ describe('copilot server tool errors', () => { retryable: true, issues: [ { - path: 'workflowDocument.edges', + path: 'entityDocument.edges', message: 'Invalid container edge: parallel1 container input requires targetHandle "target" for incoming outer edges.', }, @@ -82,7 +82,7 @@ describe('copilot server tool errors', () => { retryable: true, issues: [ { - path: 'workflowDocument.functionBlock.subBlocks.code.value', + path: 'entityDocument.functionBlock.subBlocks.code.value', message: 'Expected valid raw TypeScript function-body code.', }, ], diff --git a/apps/tradinggoose/lib/copilot/server-tool-errors.ts b/apps/tradinggoose/lib/copilot/server-tool-errors.ts index ca9e85e4b..155a8d7c8 100644 --- a/apps/tradinggoose/lib/copilot/server-tool-errors.ts +++ b/apps/tradinggoose/lib/copilot/server-tool-errors.ts @@ -160,7 +160,7 @@ function buildEditWorkflowError(message: string): CopilotServerToolErrorResponse error: message, hint: 'For loop and parallel containers, incoming outer workflow edges must target the container block alias itself with targetHandle "target". Use Start nodes only as sources to child blocks, and End nodes only for child-to-container completion before leaving the container.', retryable: true, - issues: [{ path: 'workflowDocument.edges', message }], + issues: [{ path: 'entityDocument.edges', message }], }, } } @@ -177,7 +177,7 @@ function buildEditWorkflowError(message: string): CopilotServerToolErrorResponse ) return { - path: embeddedPathMatch ? `workflowDocument.${embeddedPathMatch[1]}` : 'workflowDocument', + path: embeddedPathMatch ? `entityDocument.${embeddedPathMatch[1]}` : 'entityDocument', message: embeddedPathMatch ? embeddedPathMatch[2] : trimmedIssue, } }) diff --git a/apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts b/apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts index a1dcbb4ab..9435e9f68 100644 --- a/apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts +++ b/apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts @@ -7,6 +7,9 @@ export interface ToolPromptMetadata { surfaceKind?: string } +const CUSTOM_TOOL_DOCUMENT_GUIDANCE = + 'Use full `tg-custom-tool-document-v1` JSON with exactly `title`, `schemaText`, and `codeText`. `schemaText` is a JSON-encoded string, not an object, containing {"type":"function","function":{"name":"camelCaseName","description":"What the tool does","parameters":{"type":"object","properties":{},"required":[]}}}. `codeText` is raw async JavaScript function body only; use for inputs and {{ENV_VAR_NAME}} for environment variables.' + export const TOOL_PROMPT_METADATA: Record = { plan: { description: 'Draft a plan or todo list for multi-step work.', @@ -25,31 +28,31 @@ export const TOOL_PROMPT_METADATA: Record = { }, [CopilotTool.read_workflow]: { description: - 'Read a workflow by exact `workflowId` and return Mermaid in `entityDocument`, plus `workflowSummary.blocks[].connections` counts and exact raw `workflowSummary.edges` with external/internal scope. For topology, use only these edges/counts; do not infer graph connections from subBlock text references like `<...>`. `connectionIssues` only reports malformed existing edges.', + 'Read a workflow by exact `entityId` and return Mermaid in `entityDocument`, plus `workflowSummary.blocks[].connections` counts and exact raw `workflowSummary.edges` with external/internal scope. For topology, use only these edges/counts; do not infer graph connections from subBlock text references like `<...>`. `connectionIssues` only reports malformed existing edges.', kind: 'read', entityKind: 'workflow', }, create_workflow: { description: - 'Create a new workflow in the current workspace or provided `workspaceId`, then return its workflow `entityId` and metadata. Use that `entityId` as `workflowId` with `edit_workflow` next to author the workflow document.', + 'Create a new workflow in the current workspace or provided `workspaceId`, then return its workflow `entityId` and metadata. Use that `entityId` with `edit_workflow` next to author the workflow document.', kind: 'create', entityKind: 'workflow', }, edit_workflow: { description: - 'Replace the full workflow document using exact argument keys `workflowId`, full `workflowDocument`, and `documentFormat: tg-mermaid-v1`, then return the resulting workflow state. Use this only for graph or topology edits such as adding, removing, reconnecting, or replacing blocks, or changing loop, parallel, or condition structure. Do not use this for a single existing block `name`, `enabled`, or `subBlocks` change; use `edit_workflow_block` instead. If a full-document edit fails and the request only changes one existing block config, stop retrying `edit_workflow` and switch tools.', + 'Replace the full workflow document using exact argument keys `entityId`, full `entityDocument`, and `documentFormat: tg-mermaid-v1`, then return the resulting workflow state. Use this only for graph or topology edits such as adding, removing, reconnecting, or replacing blocks, or changing loop, parallel, or condition structure. Do not use this for a single existing block `name`, `enabled`, or `subBlocks` change; use `edit_workflow_block` instead. If a full-document edit fails and the request only changes one existing block config, stop retrying `edit_workflow` and switch tools.', kind: 'edit', entityKind: 'workflow', }, edit_workflow_block: { description: - 'Default tool for one existing block config change. Patch one existing workflow block without changing workflow connections, graph structure, loops, parallels, condition branches, or adding or removing blocks. Use exact argument keys `workflowId`, `blockId`, optional `blockType`, optional `name`, optional `enabled`, and optional `subBlocks` mapping canonical sub-block ids to new values. Use `read_workflow` first for the exact `blockId`, and use `get_blocks_metadata` before editing `subBlocks`. If a previous `edit_workflow` attempt only needed one block config change, switch to this tool instead of retrying `edit_workflow`.', + 'Default tool for one existing block config change. Patch one existing workflow block without changing workflow connections, graph structure, loops, parallels, condition branches, or adding or removing blocks. Use exact argument keys `entityId`, `blockId`, optional `blockType`, optional `name`, optional `enabled`, and optional `subBlocks` mapping canonical sub-block ids to new values. Use `read_workflow` first for the exact `blockId`, and use `get_blocks_metadata` before editing `subBlocks`. If a previous `edit_workflow` attempt only needed one block config change, switch to this tool instead of retrying `edit_workflow`.', kind: 'edit', entityKind: 'workflow', }, rename_workflow: { description: - 'Rename workflow metadata by exact `workflowId` and new `name`, then return the updated workflow identity payload.', + 'Rename workflow metadata by exact `entityId` and new `name`, then return the updated workflow identity payload.', kind: 'rename', entityKind: 'workflow', }, @@ -131,7 +134,7 @@ export const TOOL_PROMPT_METADATA: Record = { }, [CopilotTool.list_workflows]: { description: - 'List workflows in the current workspace. If the user identifies a workflow by name, use this list to select the exact `workflowId`, then read it with `read_workflow`.', + 'List workflows in the current workspace. If the user identifies a workflow by name, use this list to select the exact `entityId`, then read it with `read_workflow`.', kind: 'list', entityKind: 'workflow', }, @@ -172,26 +175,22 @@ export const TOOL_PROMPT_METADATA: Record = { entityKind: 'custom_tool', }, [CopilotTool.read_custom_tool]: { - description: - 'Return one custom tool by `entityId` as an editable document payload with `entityDocument` and `documentFormat`.', + description: `Return one custom tool by \`entityId\` as an editable document payload with \`entityDocument\` and \`documentFormat\`. ${CUSTOM_TOOL_DOCUMENT_GUIDANCE}`, kind: 'read', entityKind: 'custom_tool', }, create_custom_tool: { - description: - 'Create a new custom tool in the current workspace from a full custom tool document and return the created document.', + description: `Create a new custom tool in the current workspace from a full custom tool document and return the created document. ${CUSTOM_TOOL_DOCUMENT_GUIDANCE}`, kind: 'create', entityKind: 'custom_tool', }, edit_custom_tool: { - description: - 'Update the target custom tool from a full custom tool document and return the resulting document.', + description: `Update the target custom tool from a full custom tool document and return the resulting document. ${CUSTOM_TOOL_DOCUMENT_GUIDANCE}`, kind: 'edit', entityKind: 'custom_tool', }, rename_custom_tool: { - description: - 'Rename the target custom tool by sending a full custom tool document with the updated title or function name, then return the resulting document.', + description: `Rename the target custom tool by sending a full custom tool document with the updated title or function name, then return the resulting document. ${CUSTOM_TOOL_DOCUMENT_GUIDANCE}`, kind: 'rename', entityKind: 'custom_tool', }, diff --git a/apps/tradinggoose/lib/copilot/tools/client/base-tool.ts b/apps/tradinggoose/lib/copilot/tools/client/base-tool.ts index 0d06a0689..f902a6ecf 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/base-tool.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/base-tool.ts @@ -64,13 +64,9 @@ export interface ClientToolExecutionContext { toolCallId: string toolName: string channelId?: string - workflowId?: string - contextWorkflowId?: string + contextEntityKind?: ReviewEntityKind + contextEntityId?: string workspaceId?: string - reviewSessionId?: string - entityKind?: ReviewEntityKind - entityId?: string - draftSessionId?: string log?: ( level: 'debug' | 'info' | 'warn' | 'error', message: string, @@ -380,3 +376,33 @@ export class BaseClientTool { return this.state } } + +export abstract class StagedReviewClientTool< + TReviewResult = Record, +> extends BaseClientTool { + private stagedReviewResult?: TReviewResult + + protected getStagedReviewResult(): TReviewResult | undefined { + return this.stagedReviewResult ?? this.resolvePersistedResult() + } + + protected stageReviewResult(result: TReviewResult): void { + this.stagedReviewResult = result + this.setState(ClientToolCallState.review, { result }) + } + + protected abstract hasStagedReviewResult(result: TReviewResult | undefined): boolean + + getInterruptDisplays(): BaseClientToolMetadata['interrupt'] | undefined { + return this.getState() === ClientToolCallState.review ? this.metadata.interrupt : undefined + } + + protected async prepareReviewAccept(args?: Record): Promise { + if (this.hasStagedReviewResult(this.getStagedReviewResult())) { + return true + } + + await this.execute(args) + return this.resolveUserActionState() === ClientToolCallState.review + } +} diff --git a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts index ca56e55de..1d0723df0 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts @@ -1,5 +1,7 @@ import { type EntityDocumentKind, getEntityDocumentName } from '@/lib/copilot/entity-documents' import type { ClientToolExecutionContext } from '@/lib/copilot/tools/client/base-tool' +import { resolveOptionalCopilotEntityId } from '@/lib/copilot/tools/entity-target' +import { parseCustomToolSchemaText } from '@/lib/custom-tools/schema' import { getDefaultIndicator } from '@/lib/indicators/default' import { getEntityFields, replaceEntityTextField, setEntityField } from '@/lib/yjs/entity-session' import { buildSavedEntityYjsDescriptor } from '@/lib/yjs/entity-state' @@ -9,7 +11,6 @@ import { type YjsProviderBootstrapResult, } from '@/lib/yjs/provider' import { YJS_ORIGINS } from '@/lib/yjs/transaction-origins' -import { useWorkflowRegistry } from '@/stores/workflows/registry/store' type EntityListEntry = { entityId: string @@ -153,19 +154,6 @@ const ENTITY_API_CONFIG: Record = { }, } -function parseCustomToolSchema(schemaText: unknown): Record { - if (typeof schemaText !== 'string') { - throw new Error('custom tool schemaText is required') - } - - const schema = JSON.parse(schemaText) - if (!schema || typeof schema !== 'object' || Array.isArray(schema)) { - throw new Error('custom tool schemaText must be a JSON object') - } - - return schema as Record -} - function buildEntityCreateRequest( kind: EntityDocumentKind, workspaceId: string, @@ -194,7 +182,7 @@ function buildEntityCreateRequest( tools: [ { title: fields.title, - schema: parseCustomToolSchema(fields.schemaText), + schema: parseCustomToolSchemaText(fields.schemaText), code: fields.codeText, }, ], @@ -301,13 +289,6 @@ export function resolveWorkspaceIdFromExecutionContext( return executionContext.workspaceId } - if (executionContext.workflowId) { - const workflow = useWorkflowRegistry.getState().workflows[executionContext.workflowId] - if (workflow?.workspaceId) { - return workflow.workspaceId - } - } - throw new Error('No active workspace found') } @@ -341,7 +322,7 @@ export async function resolveCopilotEntityYjsSessionLease( kind: EntityDocumentKind, entityId?: string ): Promise { - const requestedEntityId = entityId?.trim() || undefined + const requestedEntityId = resolveOptionalCopilotEntityId({ entityId }) if (!requestedEntityId) { throw new Error(`entityId is required to update a saved ${kind}`) @@ -429,7 +410,7 @@ export async function readEntityFieldsFromContext( entityName: string fields: Record }> { - const resolvedEntityId = target?.entityId?.trim() || undefined + const resolvedEntityId = resolveOptionalCopilotEntityId(target) const resolvedRuntimeId = kind === 'indicator' ? target?.runtimeId?.trim() || undefined : undefined diff --git a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tools.ts b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tools.ts index b557f2f32..7a80d9c45 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tools.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tools.ts @@ -28,11 +28,12 @@ import { BaseClientTool, type BaseClientToolMetadata, ClientToolCallState, + StagedReviewClientTool, } from '@/lib/copilot/tools/client/base-tool' import { applyEntityFieldsToSession, - type EntityReadTarget, createCanonicalEntityFromFields, + type EntityReadTarget, listCanonicalEntityEntries, listCopilotIndicators, readEntityFieldsFromContext, @@ -156,7 +157,6 @@ function createMutationMetadata( action === 'create' ? { gerund: 'Creating', - prompt: 'Create', past: 'Created', error: 'create', aborted: 'creating', @@ -164,14 +164,12 @@ function createMutationMetadata( : action === 'rename' ? { gerund: 'Renaming', - prompt: 'Rename', past: 'Renamed', error: 'rename', aborted: 'renaming', } : { gerund: 'Editing', - prompt: 'Edit', past: 'Edited', error: 'edit', aborted: 'editing', @@ -184,8 +182,8 @@ function createMutationMetadata( icon: Loader2, }, [ClientToolCallState.pending]: { - text: `${actionLabels.prompt} ${config.singularLabel} document?`, - icon: config.icon, + text: `${actionLabels.gerund} ${config.singularLabel} document`, + icon: Loader2, }, [ClientToolCallState.executing]: { text: `${actionLabels.gerund} ${config.singularLabel} document`, @@ -292,20 +290,15 @@ function createEntityDocumentMutationTool( config: EntityToolConfig, action: EntityMutationAction ) { - return class EditEntityDocumentClientTool extends BaseClientTool { + return class EditEntityDocumentClientTool extends StagedReviewClientTool> { static readonly id = toolId static readonly metadata = createMutationMetadata(config, action) private currentArgs?: EditEntityDocumentArgs - private lastResult?: Record constructor(toolCallId: string) { super(toolCallId, toolId, EditEntityDocumentClientTool.metadata) } - getInterruptDisplays(): BaseClientToolMetadata['interrupt'] | undefined { - return this.getState() === ClientToolCallState.review ? this.metadata.interrupt : undefined - } - async execute(args?: EditEntityDocumentArgs): Promise { try { this.currentArgs = args @@ -348,7 +341,7 @@ function createEntityDocumentMutationTool( } } - this.lastResult = { + const stagedResult = { success: false, entityKind: config.kind, ...(resolvedEntityId ? { entityId: resolvedEntityId } : {}), @@ -359,7 +352,7 @@ function createEntityDocumentMutationTool( documentDiff: buildEntityDocumentDiff(config.kind, currentFields, nextFields), }, } - this.setState(ClientToolCallState.review, { result: this.lastResult }) + this.stageReviewResult(stagedResult) } catch (error) { const message = error instanceof Error ? error.message : String(error) await this.markToolComplete(500, message) @@ -367,24 +360,18 @@ function createEntityDocumentMutationTool( } } - protected async prepareReviewAccept(args?: Record): Promise { - const stagedResult = this.lastResult ?? this.resolvePersistedResult() - if (stagedResult?.entityDocument) { - return true - } - - await this.execute(args as EditEntityDocumentArgs | undefined) - return this.resolveUserActionState() === ClientToolCallState.review + protected hasStagedReviewResult(result: Record | undefined): boolean { + return !!result?.entityDocument } async handleAccept(args?: EditEntityDocumentArgs): Promise { try { this.setState(ClientToolCallState.executing) - let stagedResult = this.lastResult ?? this.resolvePersistedResult>() + let stagedResult = this.getStagedReviewResult() if (!stagedResult?.entityDocument) { await this.execute(args) - stagedResult = this.lastResult ?? this.resolvePersistedResult>() + stagedResult = this.getStagedReviewResult() } if (!stagedResult?.entityDocument?.trim()) { diff --git a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-tools.test.ts b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-tools.test.ts index 711cf178e..9044dcdd7 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-tools.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-tools.test.ts @@ -137,7 +137,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'list_skills', channelId: 'pair-yellow', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) @@ -204,7 +204,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'read_custom_tool', channelId: 'pair-orange', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) @@ -282,7 +282,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'create_skill', channelId: 'pair-yellow', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) @@ -373,7 +373,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'list_indicators', channelId: 'pair-cyan', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) @@ -435,7 +435,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'read_indicator', channelId: 'pair-yellow', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) @@ -585,7 +585,7 @@ describe('entity document tools', () => { toolCallId, toolName: 'edit_skill', channelId: 'pair-purple', - workflowId: 'wf-context', + workspaceId: 'ws-1', log: vi.fn(), }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/google/gdrive-request-access.ts b/apps/tradinggoose/lib/copilot/tools/client/google/gdrive-request-access.ts index 8aedb8d09..786c09e8e 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/google/gdrive-request-access.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/google/gdrive-request-access.ts @@ -1,9 +1,11 @@ import { CheckCircle, FolderOpen, Loader2, MinusCircle, X, XCircle } from 'lucide-react' +import { ENTITY_KIND_WORKFLOW } from '@/lib/copilot/review-sessions/types' import { BaseClientTool, type BaseClientToolMetadata, ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' +import { resolveCopilotContextEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' export class GDriveRequestAccessClientTool extends BaseClientTool { @@ -37,8 +39,7 @@ export class GDriveRequestAccessClientTool extends BaseClientTool { this.setState(ClientToolCallState.executing) const executionContext = this.requireExecutionContext() const params = new URLSearchParams({ provider: 'google-drive' }) - const workflowId = - executionContext.contextWorkflowId?.trim() || executionContext.workflowId?.trim() + const workflowId = resolveCopilotContextEntityId(executionContext, ENTITY_KIND_WORKFLOW) if (workflowId) { params.set('workflowId', workflowId) } else if (executionContext.workspaceId?.trim()) { diff --git a/apps/tradinggoose/lib/copilot/tools/client/monitor/list-monitors.ts b/apps/tradinggoose/lib/copilot/tools/client/monitor/list-monitors.ts index fd944eaed..1552854cf 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/monitor/list-monitors.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/monitor/list-monitors.ts @@ -10,6 +10,7 @@ import { type ListMonitorArgs, type MonitorRecord, } from '@/lib/copilot/tools/client/monitor/monitor-tool-utils' +import { resolveOptionalCopilotEntityId } from '@/lib/copilot/tools/entity-target' export class ListMonitorsClientTool extends BaseClientTool { static readonly id = 'list_monitors' @@ -37,8 +38,9 @@ export class ListMonitorsClientTool extends BaseClientTool { const workspaceId = resolveWorkspaceIdFromExecutionContext(executionContext) const searchParams = new URLSearchParams({ workspaceId }) - if (args?.workflowId) { - searchParams.set('workflowId', args.workflowId) + const entityId = resolveOptionalCopilotEntityId(args) + if (entityId) { + searchParams.set('workflowId', entityId) } if (args?.blockId) { searchParams.set('blockId', args.blockId) diff --git a/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tool-utils.ts b/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tool-utils.ts index 8bfb24252..927a85e86 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tool-utils.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tool-utils.ts @@ -6,7 +6,7 @@ import { import { getCopilotStoreForToolCall } from '@/stores/copilot/store-access' export type ListMonitorArgs = { - workflowId?: string + entityId?: string blockId?: string } diff --git a/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tools.test.ts b/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tools.test.ts index 1a461edcb..1c84dfcff 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tools.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/monitor/monitor-tools.test.ts @@ -330,9 +330,9 @@ describe('monitor tools', () => { it('exposes monitor tool schemas', () => { expect( ToolArgSchemas.list_monitors.parse({ - workflowId: 'wf-1', + entityId: 'wf-1', }) - ).toMatchObject({ workflowId: 'wf-1' }) + ).toMatchObject({ entityId: 'wf-1' }) expect( ToolArgSchemas.edit_monitor.parse({ diff --git a/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts b/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts index f11674357..f2224be9d 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts @@ -50,7 +50,6 @@ export class PlanClientTool extends BaseClientTool { const store = storeApi.getState() if (store.setPlanTodos) { store.setPlanTodos(todos) - storeApi.setState({ showPlanTodos: true }) } } } catch (e) { diff --git a/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts b/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts index 958791ba2..dae5d02d3 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts @@ -1,3 +1,4 @@ +import type { ReviewEntityKind } from '@/lib/copilot/review-sessions/types' import { ExecuteResponseSuccessSchema } from '@/lib/copilot/tools/shared/schemas' export interface CopilotServerToolErrorLike { @@ -48,8 +49,7 @@ export async function buildCopilotServerToolError(response: Response): Promise typeof part === 'string' && part.trim().length > 0) + ].filter((part): part is string => typeof part === 'string' && part.trim().length > 0) return createCopilotServerToolError( response.status, @@ -70,7 +70,8 @@ export async function executeCopilotServerTool(input: { toolName: string payload?: unknown context?: { - contextWorkflowId?: string + contextEntityKind?: ReviewEntityKind + contextEntityId?: string } signal?: AbortSignal }): Promise { diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/block-output-tools.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/block-output-tools.test.ts index 6feaac11a..da3e6aaba 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/block-output-tools.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/block-output-tools.test.ts @@ -96,11 +96,12 @@ describe('workflow output tools', () => { tool.setExecutionContext({ toolCallId, toolName: 'read_block_outputs', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) - await tool.execute({ workflowId: 'wf-1', blockIds: ['agent-1', 'loop-1'] }) + await tool.execute({ entityId: 'wf-1', blockIds: ['agent-1', 'loop-1'] }) expect(tool.getState()).toBe(ClientToolCallState.success) @@ -179,11 +180,12 @@ describe('workflow output tools', () => { tool.setExecutionContext({ toolCallId, toolName: 'read_block_upstream_references', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) - await tool.execute({ workflowId: 'wf-1', blockIds: ['fn-1'] }) + await tool.execute({ entityId: 'wf-1', blockIds: ['fn-1'] }) expect(tool.getState()).toBe(ClientToolCallState.success) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/check-deployment-status.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/check-deployment-status.ts index 374ada8ba..30f2eb660 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/check-deployment-status.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/check-deployment-status.ts @@ -1,13 +1,14 @@ -import { createLogger } from '@/lib/logs/console/logger' import { Loader2, Rocket, X, XCircle } from 'lucide-react' import { BaseClientTool, type BaseClientToolMetadata, ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' +import { createLogger } from '@/lib/logs/console/logger' interface CheckDeploymentStatusArgs { - workflowId: string + entityId: string } export class CheckDeploymentStatusClientTool extends BaseClientTool { @@ -43,11 +44,7 @@ export class CheckDeploymentStatusClientTool extends BaseClientTool { const logger = createLogger('CheckDeploymentStatusClientTool') try { this.setState(ClientToolCallState.executing) - const workflowId = args?.workflowId?.trim() - - if (!workflowId) { - throw new Error('workflowId is required') - } + const workflowId = requireCopilotEntityId(args) // Fetch deployment status from API const [apiDeployRes, chatDeployRes] = await Promise.all([ diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.test.ts index a3834537f..eef4285dd 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.test.ts @@ -49,7 +49,7 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { mockCopilotState.toolCallsById[toolCallId] = { params: { action: 'deploy', - workflowId: 'wf-explicit', + entityId: 'wf-explicit', }, } @@ -58,7 +58,8 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { toolCallId, toolName: 'deploy_workflow', channelId: 'pair-red', - workflowId: 'wf-context', + contextEntityKind: 'workflow', + contextEntityId: 'wf-context', log: vi.fn(), }) @@ -69,7 +70,7 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { expect(mockRegistryState.readWorkflowDeploymentStatus).toHaveBeenCalledWith('wf-explicit') }) - it('dynamic text does not consult the default active workflow when workflowId param is absent', () => { + it('dynamic text does not consult the default active workflow when entityId param is absent', () => { mockRegistryState.getActiveWorkflowId.mockImplementation(() => { throw new Error('default active workflow must not be used') }) @@ -80,7 +81,7 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { ClientToolCallState.pending ) const textWithWorkflowId = DeployWorkflowClientTool.metadata.getDynamicText?.( - { action: 'deploy', workflowId: 'wf-explicit' }, + { action: 'deploy', entityId: 'wf-explicit' }, ClientToolCallState.pending ) @@ -90,7 +91,7 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { expect(mockRegistryState.readWorkflowDeploymentStatus).toHaveBeenCalledWith('wf-explicit') }) - it('handleAccept deploys using explicit workflowId without default active workflow', async () => { + it('handleAccept deploys using explicit entityId without default active workflow', async () => { mockRegistryState.getActiveWorkflowId.mockImplementation(() => { throw new Error('default active workflow must not be used') }) @@ -141,12 +142,13 @@ describe('DeployWorkflowClientTool channel-safe workflow scoping', () => { toolCallId, toolName: 'deploy_workflow', channelId: 'pair-blue', - workflowId: 'wf-context', + contextEntityKind: 'workflow', + contextEntityId: 'wf-context', workspaceId: 'ws-1', log: vi.fn(), }) - await tool.handleAccept({ action: 'deploy', deployType: 'api', workflowId: 'wf-target' }) + await tool.handleAccept({ action: 'deploy', deployType: 'api', entityId: 'wf-target' }) const deployRequest = fetchMock.mock.calls.find(([input]) => { const url = typeof input === 'string' ? input : input.toString() diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.ts index f567d8f80..09de54864 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/deploy-workflow.ts @@ -5,6 +5,7 @@ import { ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' import { resolveWorkflowTarget } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' import { getInputFormatExample } from '@/lib/workflows/operations/deployment-utils' import { getCopilotStoreForToolCall } from '@/stores/copilot/store-access' @@ -13,7 +14,7 @@ import { useWorkflowRegistry } from '@/stores/workflows/registry/store' interface DeployWorkflowArgs { action: 'deploy' | 'undeploy' deployType?: 'api' | 'chat' - workflowId: string + entityId: string } export class DeployWorkflowClientTool extends BaseClientTool { @@ -35,7 +36,7 @@ export class DeployWorkflowClientTool extends BaseClientTool { const action = params?.action || 'deploy' const deployType = params?.deployType || 'api' // Check if workflow is already deployed - const workflowId = params?.workflowId?.trim() + const workflowId = params?.entityId?.trim() const isAlreadyDeployed = workflowId ? useWorkflowRegistry.getState().readWorkflowDeploymentStatus(workflowId)?.isDeployed : false @@ -83,7 +84,7 @@ export class DeployWorkflowClientTool extends BaseClientTool { const deployType = params?.deployType || 'api' // Check if workflow is already deployed - const workflowId = params?.workflowId + const workflowId = params?.entityId const isAlreadyDeployed = workflowId ? useWorkflowRegistry.getState().readWorkflowDeploymentStatus(workflowId)?.isDeployed : false @@ -186,8 +187,9 @@ export class DeployWorkflowClientTool extends BaseClientTool { const executionContext = this.requireExecutionContext() const action = args?.action || 'deploy' const deployType = args?.deployType || 'api' + const entityId = requireCopilotEntityId(args) const { workflowId, workspaceId } = await resolveWorkflowTarget(executionContext, { - workflowId: args?.workflowId, + entityId, }) // For chat deployment, just open the deploy modal diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.test.ts index 1c1d28754..1724c4d56 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.test.ts @@ -145,12 +145,13 @@ describe('EditWorkflowBlockClientTool', () => { tool.setExecutionContext({ toolCallId: 'tool-review', toolName: 'edit_workflow_block', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) await tool.handleUserAction({ - workflowId: 'wf-1', + entityId: 'wf-1', blockId: 'fn1', blockType: 'function', subBlocks: { diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.ts index 6b68ff178..17494d7fc 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow-block.ts @@ -49,7 +49,7 @@ export class EditWorkflowBlockClientTool extends EditWorkflowClientTool { } return { - workflowId, + entityId: workflowId, blockId, ...(args?.blockType?.trim() ? { blockType: args.blockType.trim() } : {}), ...(args?.name?.trim() ? { name: args.name.trim() } : {}), diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.test.ts index c8a7fdca8..545f65333 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.test.ts @@ -153,13 +153,14 @@ describe('EditWorkflowClientTool approval gating', () => { toolCallId: 'tool-review', toolName: 'edit_workflow', channelId: 'pair-1', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) await tool.handleUserAction({ - workflowId: 'wf-1', - workflowDocument, + entityId: 'wf-1', + entityDocument: workflowDocument, }) expect(tool.getState()).toBe(ClientToolCallState.review) @@ -244,13 +245,14 @@ describe('EditWorkflowClientTool approval gating', () => { toolCallId: 'tool-readable-state', toolName: 'edit_workflow', channelId: 'pair-1', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) await tool.handleUserAction({ - workflowId: 'wf-1', - workflowDocument, + entityId: 'wf-1', + entityDocument: workflowDocument, }) expect(tool.getState()).toBe(ClientToolCallState.review) @@ -310,13 +312,14 @@ describe('EditWorkflowClientTool approval gating', () => { toolCallId: 'tool-accept', toolName: 'edit_workflow', channelId: 'pair-1', - workflowId: 'wf-1', + contextEntityKind: 'workflow', + contextEntityId: 'wf-1', log: vi.fn(), }) await tool.execute({ - workflowId: 'wf-1', - workflowDocument, + entityId: 'wf-1', + entityDocument: workflowDocument, }) await tool.handleAccept() @@ -344,7 +347,7 @@ describe('EditWorkflowClientTool approval gating', () => { }) }) - it('rejects edit execution without explicit workflowId even when current workflow context exists', async () => { + it('rejects edit execution without explicit entityId even when current workflow context exists', async () => { const fetchMock = vi.fn(async (input: RequestInfo | URL, _init?: RequestInit) => { const url = typeof input === 'string' ? input : input.toString() @@ -365,12 +368,13 @@ describe('EditWorkflowClientTool approval gating', () => { toolCallId: 'tool-missing-workflow-id', toolName: 'edit_workflow', channelId: 'pair-1', - workflowId: 'wf-current', + contextEntityKind: 'workflow', + contextEntityId: 'wf-current', log: vi.fn(), }) await tool.execute({ - workflowDocument, + entityDocument: workflowDocument, }) expect(tool.getState()).toBe(ClientToolCallState.error) @@ -383,7 +387,7 @@ describe('EditWorkflowClientTool approval gating', () => { }) const markCompleteBody = JSON.parse(String(markCompleteRequest?.[1]?.body)) expect(markCompleteBody.status).toBe(500) - expect(markCompleteBody.message).toContain('workflowId is required') + expect(markCompleteBody.message).toContain('entityId is required') }) it('accepts persisted staged workflow edits after reload using the persisted workflow target', async () => { @@ -410,8 +414,8 @@ describe('EditWorkflowClientTool approval gating', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowId: 'wf-target', - workflowDocument, + entityId: 'wf-target', + entityDocument: workflowDocument, }, result: { entityId: 'wf-target', @@ -421,7 +425,7 @@ describe('EditWorkflowClientTool approval gating', () => { } mockResolveWorkflowTarget.mockImplementation(async (_executionContext, options) => ({ - workflowId: options?.workflowId ?? 'wf-current', + workflowId: options?.entityId ?? 'wf-current', })) mockAcquireWritableWorkflowSessionLease.mockImplementation(async ({ workflowId }) => ({ session: { @@ -451,7 +455,8 @@ describe('EditWorkflowClientTool approval gating', () => { toolCallId: 'tool-persisted-review', toolName: 'edit_workflow', channelId: 'pair-1', - workflowId: 'wf-current', + contextEntityKind: 'workflow', + contextEntityId: 'wf-current', log: vi.fn(), }) tool.hydratePersistedToolCall(persistedToolCalls['tool-persisted-review']) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts index db5830f67..10b12ea2e 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts @@ -1,8 +1,8 @@ import { Grid2x2, Grid2x2Check, Grid2x2X, Loader2, MinusCircle, XCircle } from 'lucide-react' import { - BaseClientTool, type BaseClientToolMetadata, ClientToolCallState, + StagedReviewClientTool, } from '@/lib/copilot/tools/client/base-tool' import { executeCopilotServerTool, @@ -13,6 +13,7 @@ import { getReadableWorkflowState, resolveWorkflowTarget, } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' import { YJS_ORIGINS } from '@/lib/yjs/transaction-origins' import { setWorkflowState } from '@/lib/yjs/workflow-session' @@ -20,9 +21,9 @@ import { acquireWritableWorkflowSessionLease } from '@/lib/yjs/workflow-shared-s import { getCopilotStoreForToolCall } from '@/stores/copilot/store-access' interface EditWorkflowArgs { - workflowDocument: string + entityDocument: string documentFormat?: string - workflowId?: string + entityId?: string } function readStoredToolArgs(toolCallId: string): TArgs | undefined { @@ -34,12 +35,10 @@ function readStoredToolArgs(toolCallId: string): TArgs | undefined { } } -export class EditWorkflowClientTool extends BaseClientTool { +export class EditWorkflowClientTool extends StagedReviewClientTool> { static readonly id: string = 'edit_workflow' - private lastResult: any | undefined private hasExecuted = false private hasAppliedState = false - private lastWorkflowId: string | null = null constructor( toolCallId: string, @@ -66,22 +65,15 @@ export class EditWorkflowClientTool extends BaseClientTool { }, } - getInterruptDisplays(): BaseClientToolMetadata['interrupt'] | undefined { - return this.getState() === ClientToolCallState.review ? this.metadata.interrupt : undefined - } - async handleAccept(args?: EditWorkflowArgs): Promise { const logger = createLogger('EditWorkflowClientTool') try { + const stagedResult = this.getStagedReviewResult() logger.info('handleAccept called', { toolCallId: this.toolCallId, state: this.getState(), - hasResult: this.lastResult !== undefined, + hasResult: stagedResult !== undefined, }) - const stagedResult = this.lastResult ?? this.resolvePersistedResult() - if (stagedResult && !this.lastResult) { - this.lastResult = stagedResult - } if (!stagedResult?.workflowState) { throw new Error('No staged workflow edits found to accept') @@ -89,20 +81,15 @@ export class EditWorkflowClientTool extends BaseClientTool { const executionContext = this.requireExecutionContext() const resolvedArgs = args || readStoredToolArgs(this.toolCallId) - const requestedWorkflowId = - resolvedArgs?.workflowId?.trim() ?? - (typeof stagedResult?.entityId === 'string' - ? stagedResult.entityId.trim() - : undefined) ?? - this.lastWorkflowId ?? - undefined - if (!requestedWorkflowId) { - throw new Error('workflowId is required for edit_workflow') + const requestedEntityId = + resolvedArgs?.entityId?.trim() ?? + (typeof stagedResult?.entityId === 'string' ? stagedResult.entityId.trim() : undefined) + if (!requestedEntityId) { + throw new Error('entityId is required for edit_workflow') } const { workflowId } = await resolveWorkflowTarget(executionContext, { - workflowId: requestedWorkflowId, + entityId: requestedEntityId, }) - this.lastWorkflowId = workflowId const lease = await acquireWritableWorkflowSessionLease({ workflowId, workspaceId: @@ -152,28 +139,21 @@ export class EditWorkflowClientTool extends BaseClientTool { args: Record | undefined, currentWorkflowState: string ): Record { - const workflowDocument = args?.workflowDocument?.trim() - if (!workflowDocument) { - throw new Error(`No workflowDocument provided for ${this.getServerToolName()}`) + const entityDocument = args?.entityDocument?.trim() + if (!entityDocument) { + throw new Error(`No entityDocument provided for ${this.getServerToolName()}`) } return { - workflowId, - workflowDocument, + entityId: workflowId, + entityDocument, ...(args?.documentFormat ? { documentFormat: args.documentFormat } : {}), currentWorkflowState, } } - protected async prepareReviewAccept(args?: EditWorkflowArgs): Promise { - const stagedResult = this.lastResult ?? this.resolvePersistedResult() - - if (!stagedResult?.workflowState) { - await this.execute(args) - return this.resolveUserActionState() === ClientToolCallState.review - } - - return true + protected hasStagedReviewResult(result: Record | undefined): boolean { + return !!result?.workflowState } async execute(args?: EditWorkflowArgs): Promise { @@ -187,16 +167,11 @@ export class EditWorkflowClientTool extends BaseClientTool { logger.info('execute called', { toolCallId: this.toolCallId, argsProvided: !!args }) this.setState(ClientToolCallState.executing) const executionContext = this.requireExecutionContext() - const requestedWorkflowId = args?.workflowId?.trim() - if (!requestedWorkflowId) { - throw new Error('workflowId is required for edit_workflow') - } + const requestedEntityId = requireCopilotEntityId(args, { toolName: 'edit_workflow' }) - // Resolve workflowId const { workflowId, workspaceId } = await resolveWorkflowTarget(executionContext, { - workflowId: requestedWorkflowId, + entityId: requestedEntityId, }) - this.lastWorkflowId = workflowId const readableWorkflow = await getReadableWorkflowState(executionContext, workflowId) @@ -216,7 +191,7 @@ export class EditWorkflowClientTool extends BaseClientTool { throw new Error('No workflow document returned from server') } - this.lastResult = { + const stagedResult = { ...result, ...buildWorkflowDocumentToolResult({ workflowId, @@ -233,7 +208,7 @@ export class EditWorkflowClientTool extends BaseClientTool { : 0, }) - this.setState(ClientToolCallState.review, { result: this.lastResult }) + this.stageReviewResult(stagedResult) } catch (error: any) { const message = error instanceof Error ? error.message : String(error) logger.error('execute error', { message }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-outputs.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-outputs.ts index 46642002e..051b93a40 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-outputs.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-outputs.ts @@ -13,6 +13,7 @@ import { readWorkflowVariableOutputs, } from '@/lib/copilot/tools/client/workflow/block-output-utils' import { getReadableWorkflowState } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { ReadBlockOutputsResult, type ReadBlockOutputsResultType, @@ -23,7 +24,7 @@ const logger = createLogger('ReadBlockOutputsClientTool') interface ReadBlockOutputsArgs { blockIds?: string[] - workflowId: string + entityId: string } export class ReadBlockOutputsClientTool extends BaseClientTool { @@ -66,12 +67,13 @@ export class ReadBlockOutputsClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) const executionContext = this.requireExecutionContext() + const entityId = requireCopilotEntityId(args) const { workflowId: activeWorkflowId, workflowState: snapshot, variables, - } = await getReadableWorkflowState(executionContext, args?.workflowId) + } = await getReadableWorkflowState(executionContext, entityId) const blocks = snapshot.blocks || {} const loops = snapshot.loops || {} const parallels = snapshot.parallels || {} diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-upstream-references.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-upstream-references.ts index 0550d32b9..0808b8210 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-upstream-references.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-block-upstream-references.ts @@ -14,6 +14,7 @@ import { readWorkflowVariableOutputs, } from '@/lib/copilot/tools/client/workflow/block-output-utils' import { getReadableWorkflowState } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { ReadBlockUpstreamReferencesResult, type ReadBlockUpstreamReferencesResultType, @@ -25,7 +26,7 @@ const logger = createLogger('ReadBlockUpstreamReferencesClientTool') interface ReadBlockUpstreamReferencesArgs { blockIds: string[] - workflowId: string + entityId: string } export class ReadBlockUpstreamReferencesClientTool extends BaseClientTool { @@ -78,12 +79,13 @@ export class ReadBlockUpstreamReferencesClientTool extends BaseClientTool { this.setState(ClientToolCallState.error) return } + const entityId = requireCopilotEntityId(args) const { workflowId: activeWorkflowId, workflowState: snapshot, variables, - } = await getReadableWorkflowState(executionContext, args.workflowId) + } = await getReadableWorkflowState(executionContext, entityId) const blocks = snapshot.blocks || {} const edges = snapshot.edges || [] const loops = snapshot.loops || {} diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow-variables.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow-variables.ts index ced61cd71..1f6bbf5c1 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow-variables.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow-variables.ts @@ -6,12 +6,13 @@ import { ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' import { getReadableWorkflowState } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' const logger = createLogger('ReadWorkflowVariablesClientTool') interface ReadWorkflowVariablesArgs { - workflowId: string + entityId: string } export class ReadWorkflowVariablesClientTool extends BaseClientTool { @@ -37,9 +38,10 @@ export class ReadWorkflowVariablesClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) const executionContext = this.requireExecutionContext() + const entityId = requireCopilotEntityId(args) const { workflowId, variables: varsRecord } = await getReadableWorkflowState( executionContext, - args?.workflowId + entityId ) const variables = Object.values(varsRecord).map((v: any) => ({ name: String(v?.name || ''), diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow.ts index 295ecc519..4d00e0dbb 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/read-workflow.ts @@ -10,11 +10,12 @@ import { buildWorkflowSummary, getReadableWorkflowState, } from '@/lib/copilot/tools/client/workflow/workflow-review-tool-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' import { serializeWorkflowToTgMermaid } from '@/lib/workflows/studio-workflow-mermaid' interface ReadWorkflowArgs { - workflowId: string + entityId: string } const logger = createLogger('ReadWorkflowClientTool') @@ -42,21 +43,22 @@ export class ReadWorkflowClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) const executionContext = this.requireExecutionContext() - const requestedWorkflowId = args?.workflowId?.trim() - - if (!requestedWorkflowId) { - await this.markToolComplete(400, 'workflowId is required') + let requestedEntityId: string + try { + requestedEntityId = requireCopilotEntityId(args) + } catch { + await this.markToolComplete(400, 'entityId is required') this.setState(ClientToolCallState.error) return } logger.info('Reading workflow from readable workflow snapshot', { - workflowId: requestedWorkflowId, + entityId: requestedEntityId, }) const { workflowId, entityName, workflowState, workspaceId } = await getReadableWorkflowState( executionContext, - requestedWorkflowId + requestedEntityId ) let workflowDocument = '' diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/rename-workflow.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/rename-workflow.ts index 3e52a1097..c19194442 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/rename-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/rename-workflow.ts @@ -4,12 +4,13 @@ import { type BaseClientToolMetadata, ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' -import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { getCopilotStoreForToolCall } from '@/stores/copilot/store-access' +import { useWorkflowRegistry } from '@/stores/workflows/registry/store' type RenameWorkflowArgs = { - workflowId: string + entityId: string name: string } @@ -58,13 +59,9 @@ export class RenameWorkflowClientTool extends BaseClientTool { const resolvedArgs = args || this.currentArgs || readStoredToolArgs(this.toolCallId) - const workflowId = resolvedArgs?.workflowId?.trim() + const workflowId = requireCopilotEntityId(resolvedArgs) const nextName = resolvedArgs?.name?.trim() - if (!workflowId) { - throw new Error('workflowId is required') - } - if (!nextName) { throw new Error('name is required') } diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts index b29422f48..de283f913 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts @@ -43,7 +43,7 @@ describe('RunWorkflowClientTool channel-safe workflow scoping', () => { }) }) - it('handleAccept rejects missing workflowId even when execution context has a workflow', async () => { + it('handleAccept rejects missing entityId even when execution context has a workflow', async () => { const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { const url = typeof input === 'string' ? input : input.toString() if (url === '/api/copilot/tools/mark-complete' && (init?.method || 'GET') === 'POST') { @@ -63,7 +63,8 @@ describe('RunWorkflowClientTool channel-safe workflow scoping', () => { tool.setExecutionContext({ toolCallId, toolName: 'run_workflow', - workflowId: 'wf-context', + contextEntityKind: 'workflow', + contextEntityId: 'wf-context', log: vi.fn(), }) @@ -79,7 +80,7 @@ describe('RunWorkflowClientTool channel-safe workflow scoping', () => { }) const markCompleteBody = JSON.parse(String(markCompleteCall?.[1]?.body)) expect(markCompleteBody.status).toBe(400) - expect(markCompleteBody.message).toContain('workflowId is required') + expect(markCompleteBody.message).toContain('entityId is required') }) it('handleAccept preserves an explicit workflow target instead of reusing the live channel workflow', async () => { @@ -102,12 +103,13 @@ describe('RunWorkflowClientTool channel-safe workflow scoping', () => { tool.setExecutionContext({ toolCallId, toolName: 'run_workflow', - workflowId: 'wf-live-context', + contextEntityKind: 'workflow', + contextEntityId: 'wf-live-context', log: vi.fn(), }) await tool.handleAccept({ - workflowId: 'wf-explicit-target', + entityId: 'wf-explicit-target', workflow_input: { symbol: 'AAPL' }, }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.ts index 3cc4ad1f0..81666e9cc 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.ts @@ -6,11 +6,12 @@ import { WORKFLOW_EXECUTION_TIMEOUT_MS, } from '@/lib/copilot/tools/client/base-tool' import { executeWorkflowWithFullLogging } from '@/lib/copilot/tools/client/workflow/workflow-execution-utils' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' import { useExecutionStore } from '@/stores/execution/store' interface RunWorkflowArgs { - workflowId: string + entityId: string description?: string workflow_input?: Record | string } @@ -67,11 +68,13 @@ export class RunWorkflowClientTool extends BaseClientTool { return } - const activeWorkflowId = params.workflowId?.trim() - if (!activeWorkflowId) { + let activeWorkflowId: string + try { + activeWorkflowId = requireCopilotEntityId(params) + } catch { logger.debug('Execution prevented: no target workflow') this.setState(ClientToolCallState.error) - await this.markToolComplete(400, 'workflowId is required') + await this.markToolComplete(400, 'entityId is required') return } logger.debug('Using target workflow', { workflowId: activeWorkflowId }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.test.ts index 5a2f4ed2e..b92e41b6d 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.test.ts @@ -36,7 +36,7 @@ describe('SetWorkflowVariablesClientTool', () => { mockCopilotState.toolCallsById = {} }) - it('uses explicit workflowId and writes variables only through the live Yjs session', async () => { + it('uses explicit entityId and writes variables only through the live Yjs session', async () => { const doc = { kind: 'workflow-doc' } mockGetRegisteredWorkflowSession.mockReturnValue({ doc }) mockGetVariablesForWorkflow.mockReturnValue({ @@ -75,12 +75,13 @@ describe('SetWorkflowVariablesClientTool', () => { toolCallId, toolName: 'set_workflow_variables', channelId: 'pair-purple', - workflowId: 'wf-context', + contextEntityKind: 'workflow', + contextEntityId: 'wf-context', log: vi.fn(), }) await tool.handleAccept({ - workflowId: 'wf-target', + entityId: 'wf-target', operations: [ { operation: 'edit', name: 'existing', type: 'number', value: '42' }, { operation: 'add', name: 'newVar', type: 'boolean', value: 'true' }, diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.ts index 201082d9c..ecb677a4f 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/set-workflow-variables.ts @@ -5,15 +5,16 @@ import { type BaseClientToolMetadata, ClientToolCallState, } from '@/lib/copilot/tools/client/base-tool' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { createLogger } from '@/lib/logs/console/logger' +import { VariableManager } from '@/lib/variables/variable-manager' +import { isWorkflowVariableType, type WorkflowVariableType } from '@/lib/workflows/value-types' import { YJS_ORIGINS } from '@/lib/yjs/transaction-origins' import { setVariables } from '@/lib/yjs/workflow-session' import { getRegisteredWorkflowSession, getVariablesForWorkflow, } from '@/lib/yjs/workflow-session-registry' -import { VariableManager } from '@/lib/variables/variable-manager' -import { isWorkflowVariableType, type WorkflowVariableType } from '@/lib/workflows/value-types' interface OperationItem { operation: 'add' | 'edit' | 'delete' @@ -24,7 +25,7 @@ interface OperationItem { interface SetWorkflowVariablesArgs { operations: OperationItem[] - workflowId: string + entityId: string } export class SetWorkflowVariablesClientTool extends BaseClientTool { @@ -58,12 +59,9 @@ export class SetWorkflowVariablesClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) const payload = { ...(args || { operations: [] }) } as Partial - payload.workflowId = payload.workflowId?.trim() - if (!payload.workflowId) { - throw new Error('workflowId is required') - } + const workflowId = requireCopilotEntityId(payload) - const currentVarsRecord = getVariablesForWorkflow(payload.workflowId) + const currentVarsRecord = getVariablesForWorkflow(workflowId) if (!currentVarsRecord) { throw new Error('No live Yjs session for this workflow') } @@ -92,7 +90,7 @@ export class SetWorkflowVariablesClientTool extends BaseClientTool { if (op.operation === 'add') { byName[key] = { id: crypto.randomUUID(), - workflowId: payload.workflowId, + workflowId, name: key, type: nextType, value: typedValue, @@ -103,7 +101,7 @@ export class SetWorkflowVariablesClientTool extends BaseClientTool { if (!byName[key]) { byName[key] = { id: crypto.randomUUID(), - workflowId: payload.workflowId, + workflowId, name: key, type: nextType, value: typedValue, @@ -123,14 +121,14 @@ export class SetWorkflowVariablesClientTool extends BaseClientTool { updatedRecord[variable.id] = variable } - const session = getRegisteredWorkflowSession(payload.workflowId) + const session = getRegisteredWorkflowSession(workflowId) if (!session) { throw new Error('No live Yjs session for this workflow') } setVariables(session.doc, updatedRecord, YJS_ORIGINS.COPILOT_TOOL) logger.info('Applied variable operations to Yjs doc', { - workflowId: payload.workflowId, + workflowId, operationCount: payload.operations?.length ?? 0, }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-execution-utils.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-execution-utils.ts index 6007bb90b..2d46eefdb 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-execution-utils.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-execution-utils.ts @@ -36,7 +36,6 @@ export async function executeWorkflowWithFullLogging( { toolCallId: 'workflow-execution-context', toolName: 'run_workflow', - workflowId: options.workflowId, }, options.workflowId ) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-metadata-tools.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-metadata-tools.test.ts index 443e3ef2c..0f569cb6c 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-metadata-tools.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-metadata-tools.test.ts @@ -158,7 +158,7 @@ describe('workflow metadata tools', () => { }) await tool.execute({ - workflowId: 'wf-1', + entityId: 'wf-1', name: 'Renamed Workflow', }) await tool.handleAccept() diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.test.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.test.ts index 186de07e5..48d323891 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.test.ts @@ -60,7 +60,8 @@ describe('workflow-review-tool-utils', () => { { toolCallId: 'tool-1', toolName: 'read_workflow', - workflowId: 'workflow-current', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-current', }, 'workflow-live' ) @@ -99,7 +100,8 @@ describe('workflow-review-tool-utils', () => { { toolCallId: 'tool-1', toolName: 'read_workflow', - workflowId: 'workflow-current', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-current', }, 'workflow-db' ) @@ -133,7 +135,8 @@ describe('workflow-review-tool-utils', () => { getReadableWorkflowState({ toolCallId: 'tool-1', toolName: 'read_workflow', - workflowId: 'workflow-current', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-current', }) ).rejects.toThrow('Workflow target is required') }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts index 376194dbb..4f09b69c2 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts @@ -1,6 +1,7 @@ 'use client' import type { ClientToolExecutionContext } from '@/lib/copilot/tools/client/base-tool' +import { resolveOptionalCopilotEntityId } from '@/lib/copilot/tools/entity-target' import { TG_MERMAID_DOCUMENT_FORMAT } from '@/lib/workflows/document-format' import { readWorkflowContainerBoundaryEdgeViolation, @@ -170,9 +171,9 @@ export async function listWorkflowsForExecutionContext( export async function resolveWorkflowTarget( executionContext: ClientToolExecutionContext, - options: { workflowId?: string } = {} + options: { entityId?: string } = {} ): Promise { - const requestedWorkflowId = normalizeWorkflowTargetValue(options.workflowId) + const requestedWorkflowId = resolveOptionalCopilotEntityId(options) if (requestedWorkflowId) { return { workflowId: requestedWorkflowId, @@ -185,7 +186,7 @@ export async function resolveWorkflowTarget( export async function getReadableWorkflowState( executionContext: ClientToolExecutionContext, - workflowId?: string + entityId?: string ): Promise<{ workflowId: string entityName?: string @@ -193,7 +194,7 @@ export async function getReadableWorkflowState( workspaceId: string | null variables: Record }> { - const resolvedWorkflowId = normalizeWorkflowTargetValue(workflowId) + const resolvedWorkflowId = normalizeWorkflowTargetValue(entityId) if (!resolvedWorkflowId) { throw new Error('Workflow target is required') } diff --git a/apps/tradinggoose/lib/copilot/tools/entity-target.ts b/apps/tradinggoose/lib/copilot/tools/entity-target.ts new file mode 100644 index 000000000..7dc2297d4 --- /dev/null +++ b/apps/tradinggoose/lib/copilot/tools/entity-target.ts @@ -0,0 +1,42 @@ +import type { ReviewEntityKind } from '@/lib/copilot/review-sessions/types' +import { normalizeOptionalString } from '@/lib/utils' + +export type CopilotEntityTargetArgs = { + entityId?: string | null +} + +export type CopilotEntityExecutionContext = { + contextEntityKind?: ReviewEntityKind | null + contextEntityId?: string | null +} + +export function resolveOptionalCopilotEntityId( + args: CopilotEntityTargetArgs | null | undefined +): string | undefined { + return normalizeOptionalString(args?.entityId) +} + +export function requireCopilotEntityId( + args: CopilotEntityTargetArgs | null | undefined, + options?: { toolName?: string } +): string { + const entityId = resolveOptionalCopilotEntityId(args) + if (entityId) { + return entityId + } + + throw new Error( + options?.toolName ? `entityId is required for ${options.toolName}` : 'entityId is required' + ) +} + +export function resolveCopilotContextEntityId( + context: CopilotEntityExecutionContext | null | undefined, + entityKind: ReviewEntityKind +): string | undefined { + if (context?.contextEntityKind !== entityKind) { + return undefined + } + + return normalizeOptionalString(context.contextEntityId) +} diff --git a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts index 032c6a162..fb6c43419 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mockResolveServerWorkflowScope = vi.hoisted(() => vi.fn()) const mockListCustomTools = vi.hoisted(() => vi.fn()) -vi.mock('@/lib/copilot/tools/server/base-tool', () => ({ +vi.mock('@/lib/copilot/tools/server/workflow/workflow-scope', () => ({ resolveServerWorkflowScope: mockResolveServerWorkflowScope, })) @@ -59,7 +59,7 @@ describe('getAgentAccessoryCatalogServerTool', () => { it('emits executable custom tool IDs from canonical custom tool database IDs', async () => { const result = await getAgentAccessoryCatalogServerTool.execute( - { workflowId: 'workflow-1' }, + { entityId: 'workflow-1' }, { userId: 'user-1' } ) diff --git a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts index 5ba3c25bb..156ca28a4 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts @@ -1,9 +1,7 @@ import { CopilotTool } from '@/lib/copilot/registry' -import { - type BaseServerTool, - resolveServerWorkflowScope, -} from '@/lib/copilot/tools/server/base-tool' +import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool' import { listWorkflowBlockCatalogItems } from '@/lib/copilot/tools/server/blocks/block-mermaid-catalog' +import { resolveServerWorkflowScope } from '@/lib/copilot/tools/server/workflow/workflow-scope' import type { GetAgentAccessoryCatalogInputType, GetAgentAccessoryCatalogResultType, diff --git a/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts b/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts index 438413490..03df8a971 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts @@ -1,9 +1,10 @@ import type { ToolId } from '@/lib/copilot/registry' -import { normalizeOptionalString } from '@/lib/utils' +import type { ReviewEntityKind } from '@/lib/copilot/review-sessions/types' export interface ServerToolExecutionContext { userId: string - contextWorkflowId?: string + contextEntityKind?: ReviewEntityKind + contextEntityId?: string signal?: AbortSignal } @@ -17,35 +18,6 @@ export function throwIfServerToolAborted(context?: ServerToolExecutionContext): throw error } -export function createPermissionError(operation: string): string { - return `Access denied: You do not have permission to ${operation} this workflow` -} - -export async function resolveServerWorkflowScope( - params: { workflowId?: string } | undefined, - context?: ServerToolExecutionContext -): Promise<{ workflowId: string; workspaceId?: string; hasAccess: boolean } | null> { - throwIfServerToolAborted(context) - - const userId = normalizeOptionalString(context?.userId) - const workflowId = - normalizeOptionalString(params?.workflowId) ?? - normalizeOptionalString(context?.contextWorkflowId) - - if (!userId || !workflowId) { - return null - } - - const { verifyWorkflowAccess } = await import('@/lib/copilot/review-sessions/permissions') - const access = await verifyWorkflowAccess(userId, workflowId, 'read') - - return { - workflowId, - workspaceId: access.workspaceId ?? undefined, - hasAccess: access.hasAccess, - } -} - export interface BaseServerTool { name: ToolId execute(args: TArgs, context?: ServerToolExecutionContext): Promise diff --git a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.test.ts b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.test.ts index 84ff918e1..09edf2c74 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.test.ts @@ -50,7 +50,11 @@ describe('listGDriveFilesServerTool', () => { await expect( listGDriveFilesServerTool.execute( { credentialId: 'credential-1', search_query: 'report' }, - { userId: 'auth-user', contextWorkflowId: 'workflow-1' } + { + userId: 'auth-user', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-1', + } ) ).resolves.toEqual({ files: [{ id: 'file-1', name: 'Report' }], diff --git a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts index 3b93dc764..e37e7351e 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts @@ -1,16 +1,18 @@ import { type BaseServerTool, - createPermissionError, - resolveServerWorkflowScope, type ServerToolExecutionContext, throwIfServerToolAborted, } from '@/lib/copilot/tools/server/base-tool' +import { + createWorkflowPermissionError, + resolveServerWorkflowScope, +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import { getOAuthAccessTokenForUserCredential } from '@/lib/credentials/oauth' import { createLogger } from '@/lib/logs/console/logger' import { executeTool } from '@/tools' interface ListGDriveFilesParams { - workflowId?: string + entityId?: string credentialId?: string search_query?: string num_results?: number @@ -28,7 +30,7 @@ export const listGDriveFilesServerTool: BaseServerTool { await expect( readGDriveFileServerTool.execute( { credentialId: 'credential-1', fileId: 'file-1', type: 'doc' }, - { userId: 'auth-user', contextWorkflowId: 'workflow-1' } + { + userId: 'auth-user', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-1', + } ) ).resolves.toEqual({ type: 'doc', diff --git a/apps/tradinggoose/lib/copilot/tools/server/gdrive/read-file.ts b/apps/tradinggoose/lib/copilot/tools/server/gdrive/read-file.ts index 1a28bdf28..b39232061 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/gdrive/read-file.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/gdrive/read-file.ts @@ -1,16 +1,18 @@ import { type BaseServerTool, - createPermissionError, - resolveServerWorkflowScope, type ServerToolExecutionContext, throwIfServerToolAborted, } from '@/lib/copilot/tools/server/base-tool' +import { + createWorkflowPermissionError, + resolveServerWorkflowScope, +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import { getOAuthAccessTokenForUserCredential } from '@/lib/credentials/oauth' import { createLogger } from '@/lib/logs/console/logger' import { executeTool } from '@/tools' interface ReadGDriveFileParams { - workflowId?: string + entityId?: string credentialId?: string fileId?: string type?: 'doc' | 'sheet' @@ -41,7 +43,7 @@ export const readGDriveFileServerTool: BaseServerTool throw new Error('Authentication, credentialId, fileId and type are required') } if (workflowScope && !workflowScope.hasAccess) { - throw new Error(createPermissionError('access Google Drive files in')) + throw new Error(createWorkflowPermissionError('access Google Drive files in')) } throwIfServerToolAborted(context) diff --git a/apps/tradinggoose/lib/copilot/tools/server/router.test.ts b/apps/tradinggoose/lib/copilot/tools/server/router.test.ts index 94926eaaf..cf62f7653 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/router.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/router.test.ts @@ -171,8 +171,8 @@ describe('copilot contract registry', () => { const contract = getToolContract('get_agent_accessory_catalog') expect(contract?.args.parse({})).toEqual({}) - expect(contract?.args.parse({ workflowId: 'workflow-123' })).toEqual({ - workflowId: 'workflow-123', + expect(contract?.args.parse({ entityId: 'workflow-123' })).toEqual({ + entityId: 'workflow-123', }) expect(contract?.result.parse(agentAccessoryCatalogResult)).toEqual(agentAccessoryCatalogResult) expect(() => contract?.args.parse({ workspaceId: 'workspace-123' })).toThrow() @@ -206,8 +206,7 @@ describe('copilot contract registry', () => { getToolContract('read_workflow')?.result.parse({ entityKind: 'workflow', entityId: 'workflow-123', - entityDocument: - 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', + entityDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', documentFormat: TG_MERMAID_DOCUMENT_FORMAT, workflowSummary: { blocks: [ @@ -235,19 +234,19 @@ describe('copilot contract registry', () => { }) }) - it('accepts explicit workflow ids on workflow execution tools', () => { + it('accepts explicit entity ids on workflow execution tools', () => { expect(() => getToolContract('run_workflow')?.args.parse({})).toThrow() expect(() => getToolContract('read_workflow')?.args.parse({})).toThrow() - expect(getToolContract('run_workflow')?.args.parse({ workflowId: 'workflow-123' })).toEqual({ - workflowId: 'workflow-123', + expect(getToolContract('run_workflow')?.args.parse({ entityId: 'workflow-123' })).toEqual({ + entityId: 'workflow-123', }) expect( getToolContract('set_workflow_variables')?.args.parse({ - workflowId: 'workflow-123', + entityId: 'workflow-123', operations: [], }) ).toEqual({ - workflowId: 'workflow-123', + entityId: 'workflow-123', operations: [], }) }) @@ -259,10 +258,14 @@ describe('routeExecution', () => { controller.abort() await expect( - routeExecution('read_environment_variables', {}, { - userId: 'user-1', - signal: controller.signal, - }) + routeExecution( + 'read_environment_variables', + {}, + { + userId: 'user-1', + signal: controller.signal, + } + ) ).rejects.toMatchObject({ name: 'AbortError' }) expect(readEnvironmentVariablesExecute).not.toHaveBeenCalled() @@ -296,7 +299,8 @@ describe('routeExecution', () => { it('routes agent accessory catalog requests through the central contract', async () => { const context = { userId: 'user-1', - contextWorkflowId: 'workflow-current', + contextEntityKind: 'workflow' as const, + contextEntityId: 'workflow-current', } await expect(routeExecution('get_agent_accessory_catalog', {}, context)).resolves.toMatchObject( @@ -323,11 +327,11 @@ describe('routeExecution', () => { ) }) - it('preserves workflow edit context fields when routing workflow tools', async () => { + it('preserves workflow edit entity fields when routing workflow tools', async () => { const payload = { - workflowDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', + entityDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', documentFormat: TG_MERMAID_DOCUMENT_FORMAT, - workflowId: 'workflow-123', + entityId: 'workflow-123', currentWorkflowState: '{"blocks":{}}', } @@ -341,9 +345,9 @@ describe('routeExecution', () => { expect(editWorkflowExecute).toHaveBeenCalledWith(payload, undefined) }) - it('preserves workflowId when routing workflow logs requests', async () => { + it('preserves entityId when routing workflow logs requests', async () => { const payload = { - workflowId: 'workflow-123', + entityId: 'workflow-123', limit: 5, includeDetails: false, } @@ -358,7 +362,8 @@ describe('routeExecution', () => { it('forwards ambient workflow context separately from raw tool args', async () => { const context = { userId: 'user-1', - contextWorkflowId: 'workflow-current', + contextEntityKind: 'workflow' as const, + contextEntityId: 'workflow-current', } await expect(routeExecution('read_environment_variables', {}, context)).resolves.toMatchObject({ @@ -372,30 +377,30 @@ describe('routeExecution', () => { it.each([ { toolName: 'read_environment_variables', - payload: { workflowId: 'workflow-123' }, + payload: { entityId: 'workflow-123' }, execute: readEnvironmentVariablesExecute, }, { toolName: 'set_environment_variables', - payload: { workflowId: 'workflow-123', variables: { API_KEY: 'secret' } }, + payload: { entityId: 'workflow-123', variables: { API_KEY: 'secret' } }, execute: setEnvironmentVariablesExecute, }, { toolName: 'read_credentials', - payload: { workflowId: 'workflow-123' }, + payload: { entityId: 'workflow-123' }, execute: readCredentialsExecute, }, { toolName: 'list_gdrive_files', payload: { - workflowId: 'workflow-123', + entityId: 'workflow-123', credentialId: 'credential-1', userId: 'spoofed-user', search_query: 'report', num_results: 3, }, expectedArgs: { - workflowId: 'workflow-123', + entityId: 'workflow-123', credentialId: 'credential-1', search_query: 'report', num_results: 3, @@ -405,7 +410,7 @@ describe('routeExecution', () => { { toolName: 'read_gdrive_file', payload: { - workflowId: 'workflow-123', + entityId: 'workflow-123', credentialId: 'credential-1', fileId: 'file-1', type: 'doc', @@ -414,11 +419,11 @@ describe('routeExecution', () => { }, { toolName: 'read_oauth_credentials', - payload: { workflowId: 'workflow-123' }, + payload: { entityId: 'workflow-123' }, execute: readOAuthCredentialsExecute, }, ])( - 'preserves workflowId when routing $toolName', + 'preserves entityId when routing $toolName', async ({ toolName, payload, expectedArgs, execute }) => { await expect(routeExecution(toolName, payload)).resolves.toBeDefined() diff --git a/apps/tradinggoose/lib/copilot/tools/server/user/read-credentials.ts b/apps/tradinggoose/lib/copilot/tools/server/user/read-credentials.ts index 94429da61..be4f8d12b 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/user/read-credentials.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/user/read-credentials.ts @@ -1,17 +1,19 @@ import { CopilotTool } from '@/lib/copilot/registry' +import type { + BaseServerTool, + ServerToolExecutionContext, +} from '@/lib/copilot/tools/server/base-tool' import { - type BaseServerTool, - createPermissionError, + createWorkflowPermissionError, resolveServerWorkflowScope, - type ServerToolExecutionContext, -} from '@/lib/copilot/tools/server/base-tool' +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import { listOAuthCredentialsForUser } from '@/lib/credentials/oauth' import { getPersonalAndWorkspaceEnv } from '@/lib/environment/utils' import { createLogger } from '@/lib/logs/console/logger' import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth' interface ReadCredentialsParams { - workflowId?: string + entityId?: string } export const readCredentialsServerTool: BaseServerTool = { @@ -28,7 +30,7 @@ export const readCredentialsServerTool: BaseServerTool { {}, { userId: 'auth-user', - contextWorkflowId: 'workflow-1', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-1', } ) ).resolves.toEqual({ diff --git a/apps/tradinggoose/lib/copilot/tools/server/user/read-environment-variables.ts b/apps/tradinggoose/lib/copilot/tools/server/user/read-environment-variables.ts index ca082b6c9..307a4e895 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/user/read-environment-variables.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/user/read-environment-variables.ts @@ -1,15 +1,17 @@ import { CopilotTool } from '@/lib/copilot/registry' +import type { + BaseServerTool, + ServerToolExecutionContext, +} from '@/lib/copilot/tools/server/base-tool' import { - type BaseServerTool, - createPermissionError, + createWorkflowPermissionError, resolveServerWorkflowScope, - type ServerToolExecutionContext, -} from '@/lib/copilot/tools/server/base-tool' +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import { getEnvironmentVariableKeys, getPersonalAndWorkspaceEnv } from '@/lib/environment/utils' import { createLogger } from '@/lib/logs/console/logger' interface ReadEnvironmentVariablesParams { - workflowId?: string + entityId?: string } export const readEnvironmentVariablesServerTool: BaseServerTool< @@ -34,7 +36,7 @@ export const readEnvironmentVariablesServerTool: BaseServerTool< const workflowScope = await resolveServerWorkflowScope(params, context) if (workflowScope && !workflowScope.hasAccess) { - const errorMessage = createPermissionError('access environment variables in') + const errorMessage = createWorkflowPermissionError('access environment variables in') logger.error('Unauthorized attempt to access environment variables', { workflowId: workflowScope.workflowId, authenticatedUserId, diff --git a/apps/tradinggoose/lib/copilot/tools/server/user/read-oauth-credentials.ts b/apps/tradinggoose/lib/copilot/tools/server/user/read-oauth-credentials.ts index c850aa4af..65672f184 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/user/read-oauth-credentials.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/user/read-oauth-credentials.ts @@ -1,15 +1,17 @@ import { CopilotTool } from '@/lib/copilot/registry' +import type { + BaseServerTool, + ServerToolExecutionContext, +} from '@/lib/copilot/tools/server/base-tool' import { - type BaseServerTool, - createPermissionError, + createWorkflowPermissionError, resolveServerWorkflowScope, - type ServerToolExecutionContext, -} from '@/lib/copilot/tools/server/base-tool' +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import { listOAuthCredentialsForUser } from '@/lib/credentials/oauth' import { createLogger } from '@/lib/logs/console/logger' interface ReadOAuthCredentialsParams { - workflowId?: string + entityId?: string } export const readOAuthCredentialsServerTool: BaseServerTool = { @@ -31,7 +33,7 @@ export const readOAuthCredentialsServerTool: BaseServerTool | Array<{ name: string; value: string }> - workflowId?: string + entityId?: string } const EnvVarSchema = z.object({ variables: z.record(z.string()) }) @@ -59,7 +61,7 @@ export const setEnvironmentVariablesServerTool: BaseServerTool { const result = await editWorkflowBlockServerTool.execute( { - workflowId: 'wf-1', + entityId: 'wf-1', blockId: 'fn1', blockType: 'function', name: 'Compute Market Indicators', @@ -67,7 +67,7 @@ describe('editWorkflowBlockServerTool', () => { await expect( editWorkflowBlockServerTool.execute( { - workflowId: 'wf-1', + entityId: 'wf-1', blockId: 'fn1', blockType: 'function', subBlocks: { diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow-block.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow-block.ts index b83b9821f..30d27c38d 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow-block.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow-block.ts @@ -1,4 +1,5 @@ import { StructuredServerToolError } from '@/lib/copilot/server-tool-errors' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool' import { createLogger } from '@/lib/logs/console/logger' import { getAllowedSubBlockIds } from '@/lib/workflows/block-config-canonicalization' @@ -7,7 +8,7 @@ import { getBlock } from '@/blocks' import { buildWorkflowMutationResult, loadBaseWorkflowState } from './workflow-mutation-utils' interface EditWorkflowBlockParams { - workflowId: string + entityId: string blockId: string blockType?: string name?: string @@ -42,12 +43,9 @@ export const editWorkflowBlockServerTool: BaseServerTool { const logger = createLogger('EditWorkflowBlockServerTool') - const { workflowId, blockId, blockType, name, enabled, subBlocks, currentWorkflowState } = - params + const { blockId, blockType, name, enabled, subBlocks, currentWorkflowState } = params + const workflowId = requireCopilotEntityId(params, { toolName: 'edit_workflow_block' }) - if (!workflowId) { - throw new Error('workflowId is required') - } if (!blockId?.trim()) { throw new Error('blockId is required') } diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.test.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.test.ts index 29c93ac23..b2c25b049 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.test.ts @@ -62,8 +62,8 @@ describe('editWorkflowServerTool', () => { const result = await editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: [ + entityId: 'wf-1', + entityDocument: [ 'flowchart TD', '%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', '%% TG_BLOCK {"id":"block-1","type":"input_trigger","name":"Edited Trigger","position":{"x":0,"y":0},"subBlocks":{},"outputs":{},"enabled":true}', @@ -104,8 +104,8 @@ describe('editWorkflowServerTool', () => { await expect( editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: [ + entityId: 'wf-1', + entityDocument: [ 'flowchart TD', '%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', '%% TG_BLOCK {"id":"block-1","blockType":"input_trigger","blockName":"Edited Trigger","blockDescription":"ignored","position":{"x":0,"y":0},"subBlocks":{},"outputs":{},"enabled":true}', @@ -142,8 +142,8 @@ describe('editWorkflowServerTool', () => { await expect( editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: [ + entityId: 'wf-1', + entityDocument: [ 'flowchart TD', '%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', 'inputTrigger["Input Form
id: inputTrigger
type: input_trigger
enabled: true"]', @@ -205,8 +205,8 @@ describe('editWorkflowServerTool', () => { const result = await editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: [ + entityId: 'wf-1', + entityDocument: [ 'flowchart LR', '%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"LR"}', 'inputTrigger(["Input Trigger"])', @@ -270,8 +270,8 @@ describe('editWorkflowServerTool', () => { await expect( editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: buildInputTriggerWorkflowDocument({ + entityId: 'wf-1', + entityDocument: buildInputTriggerWorkflowDocument({ inputSchema: { id: 'inputSchema', type: 'short_text', @@ -311,8 +311,8 @@ describe('editWorkflowServerTool', () => { await expect( editWorkflowServerTool.execute( { - workflowId: 'wf-1', - workflowDocument: buildInputTriggerWorkflowDocument({ + entityId: 'wf-1', + entityDocument: buildInputTriggerWorkflowDocument({ ticker: { id: 'ticker', type: 'short_text', diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.ts index 44479ee9c..43bc3b805 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/edit-workflow.ts @@ -1,3 +1,4 @@ +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool' import { createLogger } from '@/lib/logs/console/logger' import { @@ -5,14 +6,11 @@ import { TG_MERMAID_DOCUMENT_FORMAT, } from '@/lib/workflows/studio-workflow-mermaid' import { createWorkflowSnapshot } from '@/lib/yjs/workflow-session' -import { - buildWorkflowMutationResult, - loadBaseWorkflowState, -} from './workflow-mutation-utils' +import { buildWorkflowMutationResult, loadBaseWorkflowState } from './workflow-mutation-utils' interface EditWorkflowParams { - workflowId: string - workflowDocument: string + entityId: string + entityDocument: string documentFormat?: string currentWorkflowState: string } @@ -21,13 +19,11 @@ export const editWorkflowServerTool: BaseServerTool = { name: 'edit_workflow', async execute(params: EditWorkflowParams): Promise { const logger = createLogger('EditWorkflowServerTool') - const { workflowId, workflowDocument, documentFormat, currentWorkflowState } = params + const { entityDocument, documentFormat, currentWorkflowState } = params + const workflowId = requireCopilotEntityId(params, { toolName: 'edit_workflow' }) - if (!workflowId) { - throw new Error('workflowId is required') - } - if (!workflowDocument || workflowDocument.trim().length === 0) { - throw new Error('workflowDocument is required') + if (!entityDocument || entityDocument.trim().length === 0) { + throw new Error('entityDocument is required') } logger.info('Executing edit_workflow', { @@ -36,7 +32,7 @@ export const editWorkflowServerTool: BaseServerTool = { }) const baseWorkflowState = await loadBaseWorkflowState(workflowId, currentWorkflowState) - const parsedWorkflowDocument = parseTgMermaidToWorkflow(workflowDocument) + const parsedWorkflowDocument = parseTgMermaidToWorkflow(entityDocument) const result = buildWorkflowMutationResult({ workflowId, baseWorkflowState, diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.test.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.test.ts index 67c1bbcc8..61c0b46d5 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.test.ts @@ -98,7 +98,7 @@ describe('readWorkflowLogsServerTool', () => { const { readWorkflowLogsServerTool } = await import('./read-workflow-logs') const result = await readWorkflowLogsServerTool.execute( { - workflowId: 'deleted-workflow-1', + entityId: 'deleted-workflow-1', includeDetails: false, }, { userId: 'user-1' } @@ -143,7 +143,7 @@ describe('readWorkflowLogsServerTool', () => { await expect( readWorkflowLogsServerTool.execute({ - workflowId: 'deleted-workflow-1', + entityId: 'deleted-workflow-1', includeDetails: false, }) ).rejects.toThrow('Authenticated user context is required') diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.ts index 868204690..4c9cbdc1e 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/read-workflow-logs.ts @@ -2,6 +2,7 @@ import { db } from '@tradinggoose/db' import { permissions, workflowExecutionLogs, workspace } from '@tradinggoose/db/schema' import { and, desc, eq, or, sql } from 'drizzle-orm' import { CopilotTool } from '@/lib/copilot/registry' +import { requireCopilotEntityId } from '@/lib/copilot/tools/entity-target' import type { BaseServerTool, ServerToolExecutionContext, @@ -10,7 +11,7 @@ import { createLogger } from '@/lib/logs/console/logger' import { buildWorkspaceAccessScope } from '@/lib/permissions/utils' interface ReadWorkflowLogsArgs { - workflowId: string + entityId: string limit?: number includeDetails?: boolean } @@ -223,11 +224,9 @@ export const readWorkflowLogsServerTool: BaseServerTool { const logger = createLogger('ReadWorkflowLogsServerTool') - const { workflowId, limit = 3, includeDetails = true } = rawArgs || ({} as ReadWorkflowLogsArgs) + const { limit = 3, includeDetails = true } = rawArgs || ({} as ReadWorkflowLogsArgs) + const workflowId = requireCopilotEntityId(rawArgs, { toolName: CopilotTool.read_workflow_logs }) - if (!workflowId || typeof workflowId !== 'string') { - throw new Error('workflowId is required') - } if (!context?.userId) { throw new Error('Authenticated user context is required') } diff --git a/apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts b/apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts new file mode 100644 index 000000000..b2b62c88a --- /dev/null +++ b/apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts @@ -0,0 +1,40 @@ +import { ENTITY_KIND_WORKFLOW } from '@/lib/copilot/review-sessions/types' +import { + type CopilotEntityTargetArgs, + resolveCopilotContextEntityId, + resolveOptionalCopilotEntityId, +} from '@/lib/copilot/tools/entity-target' +import { + type ServerToolExecutionContext, + throwIfServerToolAborted, +} from '@/lib/copilot/tools/server/base-tool' +import { normalizeOptionalString } from '@/lib/utils' + +export function createWorkflowPermissionError(operation: string): string { + return `Access denied: You do not have permission to ${operation} this workflow` +} + +export async function resolveServerWorkflowScope( + params: CopilotEntityTargetArgs | undefined, + context?: ServerToolExecutionContext +): Promise<{ workflowId: string; workspaceId?: string; hasAccess: boolean } | null> { + throwIfServerToolAborted(context) + + const userId = normalizeOptionalString(context?.userId) + const workflowId = + resolveOptionalCopilotEntityId(params) ?? + resolveCopilotContextEntityId(context, ENTITY_KIND_WORKFLOW) + + if (!userId || !workflowId) { + return null + } + + const { verifyWorkflowAccess } = await import('@/lib/copilot/review-sessions/permissions') + const access = await verifyWorkflowAccess(userId, workflowId, 'read') + + return { + workflowId, + workspaceId: access.workspaceId ?? undefined, + hasAccess: access.hasAccess, + } +} diff --git a/apps/tradinggoose/lib/copilot/tools/shared/schemas.ts b/apps/tradinggoose/lib/copilot/tools/shared/schemas.ts index d82ee94a5..06be1f9fe 100644 --- a/apps/tradinggoose/lib/copilot/tools/shared/schemas.ts +++ b/apps/tradinggoose/lib/copilot/tools/shared/schemas.ts @@ -146,7 +146,7 @@ export type GetBlocksMetadataResultType = z.infer value.trim().replace(/\s+/g, ' ') const normalizeFunctionName = (value: string) => value.trim() -const CustomToolFunctionParametersSchema = z.object({ - type: z.string(), - properties: z.record(z.any()), - required: z.array(z.string()).optional(), -}) - -const CustomToolFunctionSchema = z.object({ - name: z - .string() - .transform(normalizeFunctionName) - .pipe(z.string().min(1, 'Function name is required')), - description: z.string().optional(), - parameters: CustomToolFunctionParametersSchema, -}) - -export const CustomToolTransferSchema = z - .object({ - title: z - .string() - .transform(normalizeInlineWhitespace) - .pipe(z.string().min(1, 'Tool title is required')), - schema: z.object({ - type: z.literal('function'), - function: CustomToolFunctionSchema, - }), - code: z.string(), - }) - .strict() - export const CustomToolsTransferListSchema = z .array(CustomToolTransferSchema) .min(1, 'At least one custom tool is required') @@ -52,13 +29,11 @@ export const CustomToolsImportFileSchema = TradingGooseExportEnvelopeSchema.exte }) } }) - -export type CustomToolTransferRecord = z.infer export type CustomToolsImportFile = z.infer function normalizeToolForTransfer( tool: Pick -): CustomToolTransferRecord { +): CustomToolTransferRecordType { return { title: normalizeInlineWhitespace(tool.title), schema: { @@ -67,7 +42,7 @@ function normalizeToolForTransfer( name: normalizeFunctionName(tool.schema.function.name), description: tool.schema.function.description, parameters: { - type: tool.schema.function.parameters.type, + type: 'object', properties: tool.schema.function.parameters.properties, required: tool.schema.function.parameters.required, }, @@ -156,7 +131,7 @@ export function resolveImportedCustomTools({ usedTitles, usedFunctionNames, }: { - customTools: CustomToolTransferRecord[] + customTools: CustomToolTransferRecordType[] usedTitles: Iterable usedFunctionNames: Iterable }) { diff --git a/apps/tradinggoose/lib/custom-tools/schema.ts b/apps/tradinggoose/lib/custom-tools/schema.ts new file mode 100644 index 000000000..5021b5c82 --- /dev/null +++ b/apps/tradinggoose/lib/custom-tools/schema.ts @@ -0,0 +1,61 @@ +import { z } from 'zod' + +const normalizeInlineWhitespace = (value: string) => value.trim().replace(/\s+/g, ' ') +const normalizeFunctionName = (value: string) => value.trim() + +export const CustomToolParametersSchema = z.object({ + type: z.literal('object'), + properties: z.record(z.any()), + required: z.array(z.string()).optional(), +}) + +export const CustomToolFunctionSchema = z.object({ + name: z + .string() + .transform(normalizeFunctionName) + .pipe(z.string().min(1, 'Function name is required')), + description: z.string().optional(), + parameters: CustomToolParametersSchema, +}) + +export const CustomToolOpenAiSchema = z.object({ + type: z.literal('function'), + function: CustomToolFunctionSchema, +}) + +export const CustomToolTransferSchema = z + .object({ + title: z + .string() + .transform(normalizeInlineWhitespace) + .pipe(z.string().min(1, 'Tool title is required')), + schema: CustomToolOpenAiSchema, + code: z.string(), + }) + .strict() + +export const CustomToolUpsertRequestSchema = z.object({ + workspaceId: z + .string({ required_error: 'workspaceId is required' }) + .min(1, 'workspaceId is required'), + tools: z.array( + z.object({ + id: z.string().optional(), + title: z.string().min(1, 'Tool title is required'), + schema: CustomToolOpenAiSchema, + code: z.string(), + }) + ), +}) + +export type CustomToolTransferRecord = z.infer + +export function parseCustomToolSchemaText( + schemaText: unknown +): z.infer { + if (typeof schemaText !== 'string') { + throw new Error('custom tool schemaText is required') + } + + return CustomToolOpenAiSchema.parse(JSON.parse(schemaText)) +} diff --git a/apps/tradinggoose/stores/copilot/store-messages.ts b/apps/tradinggoose/stores/copilot/store-messages.ts index 4ea76e76a..abe24f098 100644 --- a/apps/tradinggoose/stores/copilot/store-messages.ts +++ b/apps/tradinggoose/stores/copilot/store-messages.ts @@ -9,11 +9,14 @@ import { ensureClientToolInstance, resolveToolDisplay } from '@/stores/copilot/t import type { ChatContext, CopilotMessage, + CopilotStore, CopilotToolCall, CopilotToolExecutionProvenance, MessageFileAttachment, } from '@/stores/copilot/types' +type PlanTodo = CopilotStore['planTodos'][number] + function parseJsonObjectPrefix( value: string ): { object: Record; rest: string } | null { @@ -300,6 +303,97 @@ export function buildPinnedToolCallsById( return toolCallsById } +function readToolCallsInMessageOrder(message: CopilotMessage): CopilotToolCall[] { + const toolCalls: CopilotToolCall[] = [] + const seenToolCallIds = new Set() + const appendToolCall = (toolCall: CopilotToolCall | undefined) => { + if (!toolCall?.id || seenToolCallIds.has(toolCall.id)) { + return + } + + seenToolCallIds.add(toolCall.id) + toolCalls.push(toolCall) + } + + for (const toolCall of message.toolCalls ?? []) { + appendToolCall(toolCall) + } + + for (const block of message.contentBlocks ?? []) { + if (block.type === 'tool_call' && block.toolCall) { + appendToolCall(block.toolCall) + } + } + + return toolCalls +} + +export function buildPlanTodosFromMessages(messages: CopilotMessage[]): PlanTodo[] { + let todos: PlanTodo[] = [] + + for (const message of messages) { + for (const toolCall of readToolCallsInMessageOrder(message)) { + if (toolCall.state !== 'success') { + continue + } + + const args = ((toolCall as any).params || (toolCall as any).input || {}) as Record< + string, + any + > + const result = ( + toolCall.result && typeof toolCall.result === 'object' ? toolCall.result : {} + ) as Record + + if (toolCall.name === 'plan') { + const todoList = Array.isArray(result.todoList) + ? result.todoList + : Array.isArray(args.todoList) + ? args.todoList + : null + if (todoList) { + todos = todoList + .map((item: any, index: number) => { + const content = + typeof item === 'string' + ? item.trim() + : typeof item?.content === 'string' + ? item.content.trim() + : '' + return content + ? { + id: String(item?.id || item?.todoId || `todo-${index}`), + content, + completed: item?.completed === true, + executing: item?.executing === true, + } + : null + }) + .filter((todo): todo is PlanTodo => !!todo) + } + continue + } + + const todoId = args.id || args.todoId || result.id || result.todoId + if (!todoId) { + continue + } + + if (toolCall.name === 'mark_todo_in_progress') { + todos = todos.map((todo) => + todo.id === todoId ? { ...todo, completed: false, executing: true } : todo + ) + } else if (toolCall.name === 'checkoff_todo') { + todos = todos.map((todo) => + todo.id === todoId ? { ...todo, completed: true, executing: false } : todo + ) + } + } + } + + return todos +} + export function updateMessagesForToolCallState( messages: CopilotMessage[], toolCallId: string, diff --git a/apps/tradinggoose/stores/copilot/store-provenance.test.ts b/apps/tradinggoose/stores/copilot/store-provenance.test.ts index 66d39b94c..aae1309ff 100644 --- a/apps/tradinggoose/stores/copilot/store-provenance.test.ts +++ b/apps/tradinggoose/stores/copilot/store-provenance.test.ts @@ -19,7 +19,8 @@ describe('buildTurnProvenanceFromContexts', () => { null ) ).toEqual({ - contextWorkflowId: 'workflow-explicit', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-explicit', workspaceId: 'workspace-1', }) }) @@ -40,12 +41,13 @@ describe('buildTurnProvenanceFromContexts', () => { null ) ).toEqual({ - contextWorkflowId: 'workflow-live', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-live', workspaceId: 'workspace-1', }) }) - it('pins resolved entity review targets for client-side edit tools', () => { + it('keeps review target identity out of execution provenance', () => { expect( buildTurnProvenanceFromContexts( [ @@ -69,10 +71,8 @@ describe('buildTurnProvenanceFromContexts', () => { ) ).toEqual({ workspaceId: 'workspace-review', - contextWorkflowId: 'workflow-explicit', - entityKind: 'skill', - entityId: 'skill-review', - reviewSessionId: 'review-1', + contextEntityKind: 'workflow', + contextEntityId: 'workflow-explicit', }) }) }) diff --git a/apps/tradinggoose/stores/copilot/store-provenance.ts b/apps/tradinggoose/stores/copilot/store-provenance.ts index ae5f67311..83be1665f 100644 --- a/apps/tradinggoose/stores/copilot/store-provenance.ts +++ b/apps/tradinggoose/stores/copilot/store-provenance.ts @@ -1,10 +1,11 @@ 'use client' +import { ENTITY_KIND_WORKFLOW, type ReviewEntityKind } from '@/lib/copilot/review-sessions/types' import { normalizeOptionalString } from '@/lib/utils' import type { ChatContext, - CopilotMessage, CopilotLiveContext, + CopilotMessage, CopilotToolCall, CopilotToolExecutionProvenance, } from '@/stores/copilot/types' @@ -12,7 +13,8 @@ import { readCopilotWorkspaceEntityContext } from '@/widgets/widgets/copilot/wor type ContextTurnProvenance = { workspaceId?: string - contextWorkflowId?: string + contextEntityKind?: ReviewEntityKind + contextEntityId?: string explicit: boolean } @@ -24,11 +26,12 @@ function applyContextTurnProvenance( if (context.workspaceId && (explicit || !provenance.workspaceId)) { provenance.workspaceId = context.workspaceId } - if (context.contextWorkflowId && !provenance.contextWorkflowId) { - provenance.contextWorkflowId = context.contextWorkflowId + if (context.contextEntityKind && context.contextEntityId && !provenance.contextEntityId) { + provenance.contextEntityKind = context.contextEntityKind + provenance.contextEntityId = context.contextEntityId } - return Boolean(context.workspaceId || context.contextWorkflowId) + return Boolean(context.workspaceId || context.contextEntityId) } function getContextTurnProvenance(context: ChatContext): ContextTurnProvenance | null { @@ -39,8 +42,10 @@ function getContextTurnProvenance(context: ChatContext): ContextTurnProvenance | return { workspaceId: normalizeOptionalString(entityContext.workspaceId), - contextWorkflowId: - entityContext.entityKind === 'workflow' + contextEntityKind: + entityContext.entityKind === ENTITY_KIND_WORKFLOW ? ENTITY_KIND_WORKFLOW : undefined, + contextEntityId: + entityContext.entityKind === ENTITY_KIND_WORKFLOW ? normalizeOptionalString(entityContext.entityId) : undefined, explicit: !entityContext.current, @@ -50,16 +55,21 @@ function getContextTurnProvenance(context: ChatContext): ContextTurnProvenance | export function buildTurnProvenanceFromContexts( contexts: ChatContext[] | undefined, workspaceId: string | null | undefined, - contextWorkflowId: string | null | undefined, + liveWorkflowId: string | null | undefined, reviewTarget: CopilotLiveContext['reviewTarget'] ): CopilotToolExecutionProvenance | undefined { const normalizedWorkspaceId = normalizeOptionalString(workspaceId) - const normalizedContextWorkflowId = normalizeOptionalString(contextWorkflowId) + const normalizedLiveWorkflowId = normalizeOptionalString(liveWorkflowId) const provenance: CopilotToolExecutionProvenance = { - ...(normalizedContextWorkflowId ? { contextWorkflowId: normalizedContextWorkflowId } : {}), + ...(normalizedLiveWorkflowId + ? { + contextEntityKind: ENTITY_KIND_WORKFLOW, + contextEntityId: normalizedLiveWorkflowId, + } + : {}), ...(normalizedWorkspaceId ? { workspaceId: normalizedWorkspaceId } : {}), } - let hasContext = !!normalizedWorkspaceId || !!normalizedContextWorkflowId + let hasContext = !!normalizedWorkspaceId || !!normalizedLiveWorkflowId for (const context of contexts ?? []) { const entityContext = getContextTurnProvenance(context) @@ -68,17 +78,10 @@ export function buildTurnProvenanceFromContexts( } } - const reviewSessionId = normalizeOptionalString(reviewTarget?.reviewSessionId) - if (reviewTarget && reviewTarget.entityKind !== 'workflow' && reviewSessionId) { - provenance.entityKind = reviewTarget.entityKind - provenance.reviewSessionId = reviewSessionId - const reviewEntityId = normalizeOptionalString(reviewTarget.entityId) - const draftSessionId = normalizeOptionalString(reviewTarget.draftSessionId) + if (reviewTarget && reviewTarget.entityKind !== 'workflow') { const reviewWorkspaceId = normalizeOptionalString(reviewTarget.workspaceId) - if (reviewEntityId) provenance.entityId = reviewEntityId - if (draftSessionId) provenance.draftSessionId = draftSessionId if (reviewWorkspaceId) provenance.workspaceId = reviewWorkspaceId - hasContext = true + hasContext = Boolean(reviewWorkspaceId) || hasContext } return hasContext ? provenance : undefined @@ -88,20 +91,12 @@ export function withPinnedToolExecutionProvenance( toolCall: CopilotToolCall, baseProvenance?: CopilotToolExecutionProvenance ): CopilotToolCall { - const explicitWorkflowId = - typeof toolCall.params?.workflowId === 'string' && toolCall.params.workflowId.trim() - ? toolCall.params.workflowId.trim() - : undefined - const explicitEntityId = normalizeOptionalString(toolCall.params?.entityId) - const mergedProvenance = { ...(baseProvenance ?? {}), ...(toolCall.provenance ?? {}), - ...(explicitWorkflowId ? { workflowId: explicitWorkflowId } : {}), - ...(explicitEntityId ? { entityId: explicitEntityId } : {}), } - if (!toolCall.provenance && !baseProvenance && !explicitWorkflowId && !explicitEntityId) { + if (!toolCall.provenance && !baseProvenance) { return toolCall } diff --git a/apps/tradinggoose/stores/copilot/store.test.ts b/apps/tradinggoose/stores/copilot/store.test.ts index 79fc54bdb..c9a21b778 100644 --- a/apps/tradinggoose/stores/copilot/store.test.ts +++ b/apps/tradinggoose/stores/copilot/store.test.ts @@ -144,20 +144,20 @@ describe('copilot tool execution provenance', () => { resetCopilotWorkspaceSelectionState() }) - it('createExecutionContext uses explicit pinned provenance', () => { + it('createExecutionContext uses generic ambient entity provenance', () => { const toolCallId = 'copilot-provenance-tool-a' const context = createExecutionContext({ toolCallId, toolName: 'edit_workflow', provenance: { - workflowId: 'wf-origin-a', - contextWorkflowId: 'wf-current-a', + contextEntityKind: 'workflow', + contextEntityId: 'wf-current-a', }, }) - expect(context.workflowId).toBe('wf-origin-a') - expect(context.contextWorkflowId).toBe('wf-current-a') + expect(context.contextEntityKind).toBe('workflow') + expect(context.contextEntityId).toBe('wf-current-a') }) it('returns the first matching store when duplicate toolCallId exists', () => { @@ -185,7 +185,8 @@ describe('copilot tool execution provenance', () => { name: 'edit_workflow', state: ClientToolCallState.pending, provenance: { - workflowId: 'wf-origin-c', + contextEntityKind: 'workflow', + contextEntityId: 'wf-origin-c', }, }, }, @@ -224,7 +225,6 @@ describe('copilot tool execution provenance', () => { call_id: toolCallId, name: 'read_workflow', arguments: JSON.stringify({ - workflowId: 'wf-stringified-explicit', entityId: 'entity-stringified-explicit', }), }, @@ -239,14 +239,10 @@ describe('copilot tool execution provenance', () => { expect(store.getState().toolCallsById[toolCallId]).toMatchObject({ params: { - workflowId: 'wf-stringified-explicit', - entityId: 'entity-stringified-explicit', - }, - provenance: { - workflowId: 'wf-stringified-explicit', entityId: 'entity-stringified-explicit', }, }) + expect(store.getState().toolCallsById[toolCallId].provenance).toBeUndefined() }) it('marks streamed tool calls outside the curated Copilot registry as protocol errors', async () => { @@ -365,7 +361,8 @@ describe('copilot tool execution provenance', () => { expect(store.getState().toolCallsById[toolCallId]).toMatchObject({ provenance: { - contextWorkflowId: 'wf-live-at-send', + contextEntityKind: 'workflow', + contextEntityId: 'wf-live-at-send', workspaceId: 'workspace-1', }, }) @@ -461,7 +458,8 @@ describe('copilot tool execution provenance', () => { expect(store.getState().toolCallsById[toolCallId]).toMatchObject({ provenance: { - contextWorkflowId: 'wf-current', + contextEntityKind: 'workflow', + contextEntityId: 'wf-current', workspaceId: 'workspace-1', }, }) @@ -545,7 +543,8 @@ describe('copilot tool execution provenance', () => { expect(store.getState().toolCallsById[toolCallId]).toMatchObject({ provenance: { - contextWorkflowId: 'wf-current', + contextEntityKind: 'workflow', + contextEntityId: 'wf-current', workspaceId: 'workspace-1', }, }) @@ -650,7 +649,7 @@ describe('copilot streaming regressions', () => { type: 'function_call', call_id: 'tool-1', name: 'read_workflow', - arguments: { workflowId: 'wf-stream-order' }, + arguments: { entityId: 'wf-stream-order' }, }, }, { @@ -700,9 +699,117 @@ describe('copilot streaming regressions', () => { expect(blocks[1]?.content).toContain('checking the current workflow') expect(blocks[2]?.toolCall?.id).toBe('tool-1') expect(blocks[2]?.toolCall?.state).toBe(ClientToolCallState.success) + expect(blocks[2]?.toolCall?.result).toEqual({ entityDocument: 'flowchart TD' }) expect(blocks[3]?.content).toContain('preparing the edit now') }) + it('hydrates plan todos from persisted successful plan and todo tool calls', async () => { + const channelId = 'copilot-plan-todo-hydration' + const store = getCopilotStore(channelId) + const toolBlock = ( + name: string, + id: string, + params: Record, + state = ClientToolCallState.success + ) => ({ + type: 'tool_call', + toolCall: { id, name, state, params }, + }) + const persistedMessages = [ + { + id: 'assistant-plan-message', + role: 'assistant', + content: '', + timestamp: '2026-04-13T00:00:00.000Z', + contentBlocks: [ + toolBlock('plan', 'plan-tool-1', { + todoList: [ + { id: 'todo-1', content: 'Inspect the current workflow' }, + { id: 'todo-2', content: 'Apply the workflow edit' }, + ], + }), + toolBlock('checkoff_todo', 'todo-tool-1', { id: 'todo-1' }), + toolBlock('mark_todo_in_progress', 'todo-tool-2', { id: 'todo-2' }), + toolBlock( + 'mark_todo_in_progress', + 'todo-tool-pending-1', + { id: 'todo-1' }, + ClientToolCallState.pending + ), + toolBlock( + 'checkoff_todo', + 'todo-tool-pending-2', + { id: 'todo-2' }, + ClientToolCallState.pending + ), + ], + }, + { + id: 'assistant-todo-collision-message', + role: 'assistant', + content: '', + timestamp: '2026-04-13T00:00:01.000Z', + toolCalls: [ + { + id: 'todo-tool-collision', + name: 'checkoff_todo', + state: ClientToolCallState.success, + params: { id: 'todo-1' }, + }, + ], + contentBlocks: [ + toolBlock('mark_todo_in_progress', 'todo-tool-collision', { id: 'todo-1' }), + ], + }, + ] as any + + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ + success: true, + chats: [ + { + reviewSessionId: 'review-todos', + workspaceId: 'ws-1', + latestTurnStatus: 'completed', + messages: persistedMessages, + }, + ], + }), + })) + ) + + await store.getState().loadChats({ workspaceId: 'ws-1' }) + + expect(store.getState().showPlanTodos).toBe(true) + expect(store.getState().planTodos).toEqual([ + { + id: 'todo-1', + content: 'Inspect the current workflow', + completed: true, + executing: false, + }, + { + id: 'todo-2', + content: 'Apply the workflow edit', + completed: false, + executing: true, + }, + ]) + + store.getState().closePlanTodos() + store.getState().updatePlanTodoStatus('todo-2', 'completed') + expect(store.getState().showPlanTodos).toBe(false) + expect(store.getState().planTodos[1]).toMatchObject({ + id: 'todo-2', + completed: true, + executing: false, + }) + }) + it('uses the final output item text when it differs from streamed deltas', async () => { const channelId = 'copilot-streaming-final-item-authority' const store = getCopilotStore(channelId) @@ -1248,8 +1355,8 @@ describe('copilot streaming regressions', () => { call_id: toolCallId, name: 'edit_workflow', arguments: { - workflowId: 'wf-limited-edit', - workflowDocument: 'workflow: {}', + entityId: 'wf-limited-edit', + entityDocument: 'workflow: {}', documentFormat: 'tg-mermaid-v1', }, }, @@ -1270,6 +1377,107 @@ describe('copilot streaming regressions', () => { } }) + it('leaves persisted review tools staged for full-access selected chats', async () => { + vi.useFakeTimers() + const toolCallId = 'edit-workflow-persisted-full-tool' + const pendingToolCallId = 'make-api-request-persisted-pending-tool' + const reviewResult = { + workflowState: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + }, + } + let toolState = ClientToolCallState.review + const fakeTool: any = { + persistedToolCall: { result: reviewResult }, + hydratePersistedToolCall: vi.fn((toolCall) => { + fakeTool.persistedToolCall = { result: toolCall.result } + }), + handleUserAction: vi.fn(async () => { + toolState = ClientToolCallState.success + fakeTool.persistedToolCall = { result: reviewResult } + }), + getState: vi.fn(() => toolState), + } + const chat: any = { + reviewSessionId: 'review-persisted-full-access', + workspaceId: 'workspace-1', + latestTurnStatus: 'completed' as const, + messages: [ + { + id: 'assistant-persisted-review', + role: 'assistant', + content: '', + timestamp: '2026-04-17T00:00:00.000Z', + contentBlocks: [ + { + type: 'tool_call', + toolCall: { + id: pendingToolCallId, + name: 'make_api_request', + state: ClientToolCallState.pending, + }, + }, + { + type: 'tool_call', + toolCall: { + id: toolCallId, + name: 'edit_workflow', + state: ClientToolCallState.review, + params: { entityId: 'wf-persisted-full-access' }, + result: reviewResult, + }, + }, + ], + }, + ], + } + + registerClientTool(toolCallId, fakeTool) + + try { + const store = getCopilotStore('copilot-persisted-full-access-review') + store.setState({ accessLevel: 'full', chats: [chat] }) + + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input.toString() + if ( + url === `/api/copilot/chat?reviewSessionId=${encodeURIComponent(chat.reviewSessionId)}` + ) { + return { + ok: true, + status: 200, + json: async () => ({ success: true, chats: [chat] }), + } + } + return { + ok: true, + status: 200, + json: async () => ({ success: true }), + } + }) + vi.stubGlobal('fetch', fetchMock) + + await store.getState().selectChat(chat) + await vi.advanceTimersByTimeAsync(0) + + expect(fakeTool.handleUserAction).not.toHaveBeenCalled() + expect(store.getState().toolCallsById[toolCallId]?.state).toBe(ClientToolCallState.review) + expect(store.getState().toolCallsById[toolCallId]?.result).toEqual(reviewResult) + expect( + fetchMock.mock.calls.some(([input]) => { + const url = typeof input === 'string' ? input : input.toString() + return url === '/api/copilot/execute-copilot-server-tool' + }) + ).toBe(false) + } finally { + unregisterClientTool(toolCallId) + vi.useRealTimers() + } + }) + it('starts a new generic copilot chat without deleting prior workspace history', async () => { const channelId = 'copilot-new-chat-scope' const store = getCopilotStore(channelId) @@ -1556,8 +1764,8 @@ describe('copilot streaming regressions', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowDocument: 'workflow: {}', - workflowId: 'wf-active-chat-review-tools', + entityDocument: 'workflow: {}', + entityId: 'wf-active-chat-review-tools', }, }, ], @@ -1570,8 +1778,8 @@ describe('copilot streaming regressions', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowDocument: 'workflow: {}', - workflowId: 'wf-active-chat-review-tools', + entityDocument: 'workflow: {}', + entityId: 'wf-active-chat-review-tools', }, }, }, @@ -1584,8 +1792,8 @@ describe('copilot streaming regressions', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowDocument: 'workflow: {}', - workflowId: 'wf-active-chat-review-tools', + entityDocument: 'workflow: {}', + entityId: 'wf-active-chat-review-tools', }, }, }, @@ -2276,7 +2484,7 @@ describe('copilot streaming regressions', () => { id: toolCallId, name: 'edit_workflow', state: ClientToolCallState.review, - params: { workflowDocument: 'flowchart TD' }, + params: { entityDocument: 'flowchart TD' }, }, }, ], @@ -2287,10 +2495,7 @@ describe('copilot streaming regressions', () => { id: toolCallId, name: 'edit_workflow', state: ClientToolCallState.review, - params: { workflowDocument: 'flowchart TD' }, - provenance: { - workflowId: 'wf-review-abort', - }, + params: { entityDocument: 'flowchart TD' }, } as any, }, isSendingMessage: false, @@ -2537,7 +2742,8 @@ describe('copilot streaming regressions', () => { expect(secondaryStore.getState().messages.at(-1)?.content).toBe('Shared reply') expect(secondaryStore.getState().toolCallsById[toolCallId]?.provenance).toMatchObject({ workspaceId: 'workspace-1', - contextWorkflowId: 'wf-blue', + contextEntityKind: 'workflow', + contextEntityId: 'wf-blue', }) expect(secondaryStore.getState().isSendingMessage).toBe( primaryStore.getState().isSendingMessage @@ -2741,16 +2947,9 @@ describe('copilot tool user action delegation', () => { name: 'edit_workflow', state: ClientToolCallState.pending, params: { - workflowDocument: + entityDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', - workflowId: 'wf-edit-workflow-order', - }, - provenance: { - workflowId: 'wf-edit-workflow-order', - reviewSessionId: 'review-edit-workflow-order', - entityKind: 'workflow', entityId: 'wf-edit-workflow-order', - workspaceId: 'workspace-1', }, } as any, }, @@ -2807,9 +3006,9 @@ describe('copilot tool user action delegation', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowDocument: + entityDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', - workflowId: 'wf-edit-workflow-review', + entityId: 'wf-edit-workflow-review', }, result: { workflowState: { @@ -2819,13 +3018,6 @@ describe('copilot tool user action delegation', () => { parallels: {}, }, }, - provenance: { - workflowId: 'wf-edit-workflow-review', - reviewSessionId: 'review-edit-workflow-review', - entityKind: 'workflow', - entityId: 'wf-edit-workflow-review', - workspaceId: 'workspace-1', - }, } as any, }, }) @@ -2885,7 +3077,8 @@ describe('copilot tool user action delegation', () => { method: 'GET', }, provenance: { - contextWorkflowId: 'wf-api-request-access-switch', + contextEntityKind: 'workflow', + contextEntityId: 'wf-api-request-access-switch', workspaceId: 'workspace-1', }, } as any, @@ -2906,7 +3099,8 @@ describe('copilot tool user action delegation', () => { method: 'GET', }, context: { - contextWorkflowId: 'wf-api-request-access-switch', + contextEntityKind: 'workflow', + contextEntityId: 'wf-api-request-access-switch', }, }) expect(store.getState().toolCallsById[toolCallId]?.state).toBe(ClientToolCallState.success) @@ -2957,10 +3151,6 @@ describe('copilot tool user action delegation', () => { name: 'set_environment_variables', state: ClientToolCallState.pending, params: { variables: { API_KEY: 'secret' } }, - provenance: { - workflowId: 'workflow-1', - workspaceId: 'workspace-1', - }, } as any, }, }) @@ -3062,7 +3252,8 @@ describe('copilot tool user action delegation', () => { method: 'GET', }, provenance: { - contextWorkflowId: 'wf-server-managed-state-persist', + contextEntityKind: 'workflow', + contextEntityId: 'wf-server-managed-state-persist', workspaceId: 'workspace-1', }, } as any, @@ -3090,7 +3281,7 @@ describe('copilot tool user action delegation', () => { ) }) - it('auto-executes review-state client tools when access switches to full', async () => { + it('leaves review-state client tools staged when access switches to full', async () => { vi.useFakeTimers() try { const channelId = 'copilot-review-access-switch' @@ -3133,16 +3324,9 @@ describe('copilot tool user action delegation', () => { name: 'edit_workflow', state: ClientToolCallState.review, params: { - workflowDocument: + entityDocument: 'flowchart TD\n%% TG_WORKFLOW {"version":"tg-mermaid-v1","direction":"TD"}', - workflowId: 'wf-review-access-switch', - }, - provenance: { - workflowId: 'wf-review-access-switch', - reviewSessionId: 'review-access-switch', - entityKind: 'workflow', entityId: 'wf-review-access-switch', - workspaceId: 'workspace-1', }, } as any, }, @@ -3151,7 +3335,8 @@ describe('copilot tool user action delegation', () => { store.getState().setAccessLevel('full') await vi.runAllTimersAsync() - expect(calls).toEqual(['userAction']) + expect(calls).toEqual([]) + expect(store.getState().toolCallsById[toolCallId]?.state).toBe(ClientToolCallState.review) unregisterClientTool(toolCallId) } finally { diff --git a/apps/tradinggoose/stores/copilot/store.ts b/apps/tradinggoose/stores/copilot/store.ts index 8a69ad86b..b8992db03 100644 --- a/apps/tradinggoose/stores/copilot/store.ts +++ b/apps/tradinggoose/stores/copilot/store.ts @@ -31,6 +31,7 @@ import { } from '@/stores/copilot/store-access' import { buildPinnedToolCallsById, + buildPlanTodosFromMessages, createErrorMessage, createStreamingMessage, createUserMessage, @@ -271,7 +272,7 @@ function abortAllInProgressTools( } } -function autoExecuteEligibleToolsForAccessLevel( +function autoExecutePendingToolsForAccessLevel( accessLevel: CopilotStore['accessLevel'], get: () => CopilotStore ) { @@ -283,11 +284,7 @@ function autoExecuteEligibleToolsForAccessLevel( const copilotToolIds: string[] = [] for (const [id, toolCall] of Object.entries(toolCallsById)) { - const state = toolCall.state - const isAwaitingApproval = - state === ClientToolCallState.pending || state === ClientToolCallState.review - - if (!isAwaitingApproval) { + if (toolCall.state !== ClientToolCallState.pending) { continue } @@ -300,7 +297,7 @@ function autoExecuteEligibleToolsForAccessLevel( return } - logger.info('[copilot access] auto-executing queued tools after access change', { + logger.info('[copilot access] auto-executing queued pending tools', { accessLevel, copilotToolIds, }) @@ -309,8 +306,7 @@ function autoExecuteEligibleToolsForAccessLevel( setTimeout(() => { const latest = get().toolCallsById[toolCallId] if (!latest) return - const state = latest.state - if (state !== ClientToolCallState.pending && state !== ClientToolCallState.review) { + if (latest.state !== ClientToolCallState.pending) { return } void get().executeCopilotToolCall(toolCallId) @@ -338,6 +334,11 @@ const initialState = { contextUsage: null, } +function buildPlanTodoStateFromMessages(messages: CopilotMessage[]) { + const planTodos = buildPlanTodosFromMessages(messages) + return { planTodos, showPlanTodos: planTodos.length > 0 } +} + const sharedSessionSyncGuards = new WeakSet>() function buildSharedSessionState(state: CopilotStore) { @@ -470,7 +471,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) const previousAccessLevel = get().accessLevel set({ accessLevel }) if (previousAccessLevel !== accessLevel) { - autoExecuteEligibleToolsForAccessLevel(accessLevel, get) + autoExecutePendingToolsForAccessLevel(accessLevel, get) } }, @@ -505,8 +506,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) currentChat: chat, messages: normalizedMessages, toolCallsById: optimisticToolCallsById, - planTodos: [], - showPlanTodos: false, + ...buildPlanTodoStateFromMessages(normalizedMessages), contextUsage: null, isSendingMessage: isChatTurnInProgress(chat), isAwaitingContinuation: isChatTurnInProgress(chat), @@ -561,6 +561,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) ), contextUsage: null, toolCallsById, + ...buildPlanTodoStateFromMessages(normalizedMessages), isSendingMessage: isChatTurnInProgress(latestChat), isAwaitingContinuation: isChatTurnInProgress(latestChat), abortController: null, @@ -705,6 +706,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) currentChat: updatedCurrentChat, messages: normalizedMessages, toolCallsById, + ...buildPlanTodoStateFromMessages(normalizedMessages), isSendingMessage: isChatTurnInProgress(updatedCurrentChat), isAwaitingContinuation: isChatTurnInProgress(updatedCurrentChat), abortController: null, @@ -739,6 +741,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) currentChat: availableChat, messages: normalizedMessages, toolCallsById, + ...buildPlanTodoStateFromMessages(normalizedMessages), isSendingMessage: isChatTurnInProgress(availableChat), isAwaitingContinuation: isChatTurnInProgress(availableChat), abortController: null, @@ -748,6 +751,8 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) currentChat: null, messages: [], toolCallsById: {}, + planTodos: [], + showPlanTodos: false, isSendingMessage: false, isAwaitingContinuation: false, abortController: null, @@ -759,6 +764,8 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) currentChat: null, messages: [], toolCallsById: {}, + planTodos: [], + showPlanTodos: false, isSendingMessage: false, isAwaitingContinuation: false, abortController: null, @@ -1005,8 +1012,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) if ( (current.state === ClientToolCallState.rejected && norm === ClientToolCallState.success) || - (current.state === ClientToolCallState.aborted && - norm !== ClientToolCallState.aborted) + (current.state === ClientToolCallState.aborted && norm !== ClientToolCallState.aborted) ) { return } @@ -1171,7 +1177,6 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) ? { bill: true, assistantMessageId, - workflowId: context.provenance?.workflowId, } : undefined await get().fetchContextUsage(billingOptions) @@ -1236,10 +1241,14 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) setInputValue: (value: string) => set({ inputValue: value }), // Todo list (UI only) - setPlanTodos: (todos) => set({ planTodos: todos, showPlanTodos: true }), + setPlanTodos: (todos) => set({ planTodos: todos, showPlanTodos: todos.length > 0 }), updatePlanTodoStatus: (id, status) => { set((state) => { - const updated = state.planTodos.map((t) => + const planTodos = + state.planTodos.length > 0 + ? state.planTodos + : buildPlanTodosFromMessages(state.messages) + const updated = planTodos.map((t) => t.id === id ? { ...t, completed: status === 'completed', executing: status === 'executing' } : t @@ -1258,21 +1267,14 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) setAgentPrefetch: (prefetch) => set({ agentPrefetch: prefetch }), // Fetch context usage from copilot API - fetchContextUsage: async (options?: { - bill?: boolean - assistantMessageId?: string - workflowId?: string - }) => { + fetchContextUsage: async (options?: { bill?: boolean; assistantMessageId?: string }) => { try { - const { bill = false, assistantMessageId, workflowId } = options ?? {} + const { bill = false, assistantMessageId } = options ?? {} const { currentChat, selectedModel } = get() - const activeWorkflowId = workflowId const selectedProvider = resolveCopilotRuntimeProvider(selectedModel) logger.info('[Context Usage] Starting fetch', { hasConversationId: !!currentChat?.conversationId, - hasWorkflowId: !!activeWorkflowId, conversationId: currentChat?.conversationId, - workflowId: activeWorkflowId, model: selectedModel, provider: selectedProvider, bill, @@ -1298,10 +1300,9 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) conversationId: currentChat.conversationId, model: selectedModel, provider: selectedProvider, - // Context usage is conversation-scoped. Forward the current workflow - // only as supplemental runtime context when one exists. - ...(activeWorkflowId ? { workflowId: activeWorkflowId } : {}), } + // Generic Copilot context usage is conversation/user scoped. Workflow contexts are + // prompt context for the chat, not billing scope selectors for this widget. if (bill && assistantMessageId) { requestPayload.bill = true requestPayload.assistantMessageId = assistantMessageId @@ -1379,8 +1380,11 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) if (isServerManagedCopilotTool(name)) { try { const serverContext = { - ...(provenance?.contextWorkflowId - ? { contextWorkflowId: provenance.contextWorkflowId } + ...(provenance?.contextEntityKind && provenance?.contextEntityId + ? { + contextEntityKind: provenance.contextEntityKind, + contextEntityId: provenance.contextEntityId, + } : {}), } const result = await executeCopilotServerTool({ @@ -1640,9 +1644,7 @@ export function useCopilotStore( ) { const store = useContext(CopilotStoreContext) ?? defaultCopilotStore const resolvedSelector = selector ?? (identitySelector as unknown as (state: CopilotStore) => T) - return equalityFn - ? useStoreWithEqualityFn(store, resolvedSelector, equalityFn) - : useStoreWithEqualityFn(store, resolvedSelector) + return useStoreWithEqualityFn(store, resolvedSelector, equalityFn) } export function useCopilotStoreApi(channelId?: string) { diff --git a/apps/tradinggoose/stores/copilot/streaming.ts b/apps/tradinggoose/stores/copilot/streaming.ts index d1a915ccd..2fa3db100 100644 --- a/apps/tradinggoose/stores/copilot/streaming.ts +++ b/apps/tradinggoose/stores/copilot/streaming.ts @@ -331,7 +331,8 @@ export async function flushPendingAutoExecutionToolCalls( function buildStreamedToolDisplayState( toolCallId: string, targetState: ClientToolCallState, - context: StreamingContext + context: StreamingContext, + result?: unknown ) { for (let i = 0; i < context.contentBlocks.length; i++) { const block = context.contentBlocks[i] as any @@ -354,6 +355,7 @@ function buildStreamedToolDisplayState( toolCallId, block.toolCall?.params ), + ...(result !== undefined ? { result } : {}), }, } break @@ -424,6 +426,7 @@ export function createSSEHandlers(params: { const { toolCallsById } = get() const current = toolCallsById[toolCallId] + const result = current?.result ?? data?.result ?? data?.data?.result if (current) { if (isToolCallCompletionProtected(current.state)) { return @@ -441,6 +444,7 @@ export function createSSEHandlers(params: { current.id, (current as any).params ), + ...(result !== undefined ? { result } : {}), }, }, }) @@ -459,7 +463,7 @@ export function createSSEHandlers(params: { } } - buildStreamedToolDisplayState(toolCallId, targetState, context) + buildStreamedToolDisplayState(toolCallId, targetState, context, result) updateStreamingMessage(set, context) const currentChat = get().currentChat diff --git a/apps/tradinggoose/stores/copilot/tool-registry.test.ts b/apps/tradinggoose/stores/copilot/tool-registry.test.ts index 0ab3d284d..8177f13a8 100644 --- a/apps/tradinggoose/stores/copilot/tool-registry.test.ts +++ b/apps/tradinggoose/stores/copilot/tool-registry.test.ts @@ -73,36 +73,23 @@ describe('tool-registry', () => { expect(getToolInterruptDisplays('edit_workflow_block', toolCallId)).toBeDefined() }) - it('does not inject workflow ids into server tool args from execution provenance', () => { + it('does not inject ambient entity context into server tool args from execution provenance', () => { const context = createExecutionContext({ toolCallId, toolName: 'read_workflow_logs', - provenance: { contextWorkflowId: 'wf-current' }, + provenance: { contextEntityKind: 'workflow', contextEntityId: 'wf-current' }, }) - expect(context.contextWorkflowId).toBe('wf-current') + expect(context.contextEntityKind).toBe('workflow') + expect(context.contextEntityId).toBe('wf-current') expect(prepareCopilotToolArgs('read_workflow_logs', {}, context)).toEqual({}) - expect( - prepareCopilotToolArgs( - 'read_workflow_logs', - {}, - createExecutionContext({ - toolCallId, - toolName: 'read_workflow_logs', - provenance: { - workflowId: 'wf-1', - contextWorkflowId: 'wf-current', - }, - }) - ) - ).toEqual({}) }) it('preserves only explicit server-routed GDrive args', () => { const context = createExecutionContext({ toolCallId, toolName: 'read_gdrive_file', - provenance: { workflowId: 'wf-1' }, + provenance: {}, }) expect( diff --git a/apps/tradinggoose/stores/copilot/tool-registry.ts b/apps/tradinggoose/stores/copilot/tool-registry.ts index 16b87ee76..53ca16e94 100644 --- a/apps/tradinggoose/stores/copilot/tool-registry.ts +++ b/apps/tradinggoose/stores/copilot/tool-registry.ts @@ -1,10 +1,10 @@ import { CopilotTool, isToolId, type ToolId } from '@/lib/copilot/registry' -import { - type BaseClientTool, - type BaseClientToolMetadata, +import type { + BaseClientTool, + BaseClientToolMetadata, ClientToolCallState, - type ClientToolDisplay, - type ClientToolExecutionContext, + ClientToolDisplay, + ClientToolExecutionContext, } from '@/lib/copilot/tools/client/base-tool' import { CreateCustomToolClientTool, @@ -167,38 +167,22 @@ export function createExecutionContext(params: { provenance: Partial }): ClientToolExecutionContext { const { toolCallId, toolName, provenance } = params - const { - workflowId, - contextWorkflowId, - workspaceId, - reviewSessionId, - entityKind, - entityId, - draftSessionId, - } = provenance + const { contextEntityKind, contextEntityId, workspaceId } = provenance return { toolCallId, toolName, - ...(workflowId ? { workflowId } : {}), - ...(contextWorkflowId ? { contextWorkflowId } : {}), + ...(contextEntityKind ? { contextEntityKind } : {}), + ...(contextEntityId ? { contextEntityId } : {}), ...(workspaceId ? { workspaceId } : {}), - ...(reviewSessionId ? { reviewSessionId } : {}), - ...(entityKind ? { entityKind } : {}), - ...(entityId ? { entityId } : {}), - ...(draftSessionId ? { draftSessionId } : {}), log: (level, message, extra) => { try { logger[level](message, { toolCallId, toolName, - workflowId, - contextWorkflowId, + contextEntityKind, + contextEntityId, workspaceId, - reviewSessionId, - entityKind, - entityId, - draftSessionId, ...(extra || {}), }) } catch {} diff --git a/apps/tradinggoose/stores/copilot/types.ts b/apps/tradinggoose/stores/copilot/types.ts index bedbc78e7..15619b87f 100644 --- a/apps/tradinggoose/stores/copilot/types.ts +++ b/apps/tradinggoose/stores/copilot/types.ts @@ -120,13 +120,9 @@ export interface CopilotSendRuntimeContext { } export interface CopilotToolExecutionProvenance { - workflowId?: string - contextWorkflowId?: string + contextEntityKind?: ReviewEntityKind + contextEntityId?: string workspaceId?: string - reviewSessionId?: string - entityKind?: ReviewEntityKind - entityId?: string - draftSessionId?: string } export interface CopilotState { @@ -167,11 +163,7 @@ export interface CopilotActions { setAccessLevel: (accessLevel: CopilotAccessLevel) => void setSelectedModel: (model: CopilotStore['selectedModel']) => Promise setAgentPrefetch: (prefetch: boolean) => void - fetchContextUsage: (options?: { - bill?: boolean - assistantMessageId?: string - workflowId?: string - }) => Promise + fetchContextUsage: (options?: { bill?: boolean; assistantMessageId?: string }) => Promise loadChats: (options?: { workspaceId?: string | null }) => Promise selectChat: (chat: CopilotChat) => Promise diff --git a/apps/tradinggoose/widgets/widgets/copilot/components/copilot/copilot.tsx b/apps/tradinggoose/widgets/widgets/copilot/components/copilot/copilot.tsx index 1b3c024d8..e4bc12493 100644 --- a/apps/tradinggoose/widgets/widgets/copilot/components/copilot/copilot.tsx +++ b/apps/tradinggoose/widgets/widgets/copilot/components/copilot/copilot.tsx @@ -13,11 +13,11 @@ import { ArrowDown } from 'lucide-react' import { Button } from '@/components/ui/button' import { LoadingAgent } from '@/components/ui/loading-agent' import { ScrollArea } from '@/components/ui/scroll-area' -import { DEFAULT_COPILOT_RUNTIME_MODEL } from '@/lib/copilot/runtime-models' import type { ReviewTargetDescriptor } from '@/lib/copilot/review-sessions/types' +import { DEFAULT_COPILOT_RUNTIME_MODEL } from '@/lib/copilot/runtime-models' import { createLogger } from '@/lib/logs/console/logger' import { normalizeOptionalString } from '@/lib/utils' -import { useCopilotStore, useCopilotStoreApi } from '@/stores/copilot/store' +import { useCopilotStore } from '@/stores/copilot/store' import { hasUiActiveToolCalls } from '@/stores/copilot/store-state' import type { ChatContext, CopilotSendRuntimeContext } from '@/stores/copilot/types' import { usePairColorContext } from '@/stores/dashboard/pair-store' @@ -123,7 +123,6 @@ export const Copilot = forwardRef( toolCallsById, fetchContextUsage, } = useCopilotStore() - const copilotStoreApi = useCopilotStoreApi() const hasActiveToolCalls = useMemo(() => hasUiActiveToolCalls(toolCallsById), [toolCallsById]) const isTurnInProgress = isSendingMessage || @@ -172,11 +171,11 @@ export const Copilot = forwardRef( useEffect(() => { if (isInitialized && currentChat?.reviewSessionId) { logger.info('[Copilot] Component initialized, fetching context usage') - fetchContextUsage({ workflowId: liveContext.workflowId ?? undefined }).catch((err) => { + fetchContextUsage().catch((err) => { logger.warn('[Copilot] Failed to fetch context usage on mount', err) }) } - }, [currentChat?.reviewSessionId, fetchContextUsage, isInitialized, liveContext.workflowId]) + }, [currentChat?.reviewSessionId, fetchContextUsage, isInitialized]) const clearProgrammaticScrollLock = useCallback(() => { if (programmaticScrollResetTimerRef.current !== null) { @@ -366,22 +365,13 @@ export const Copilot = forwardRef( // Track previous sending state to detect when stream completes const wasTurnInProgressRef = useRef(false) - // Auto-collapse todos and remove uncompleted ones when stream completes + // Auto-collapse todos when stream completes. useEffect(() => { if (wasTurnInProgressRef.current && !isTurnInProgress && showPlanTodos) { - // Stream just completed, collapse the todos and filter out uncompleted ones setTodosCollapsed(true) - - // Remove any uncompleted todos - const completedTodos = planTodos.filter((todo) => todo.completed === true) - if (completedTodos.length !== planTodos.length) { - // Only update if there are uncompleted todos to remove - const store = copilotStoreApi.getState() - store.setPlanTodos(completedTodos) - } } wasTurnInProgressRef.current = isTurnInProgress - }, [isTurnInProgress, showPlanTodos, planTodos]) + }, [isTurnInProgress, showPlanTodos]) // Reset collapsed state when todos first appear useEffect(() => { @@ -455,12 +445,6 @@ export const Copilot = forwardRef( ) => { if (!query || inputDisabled || isTurnInProgress) return - // Clear todos when sending a new message - if (showPlanTodos) { - const store = copilotStoreApi.getState() - store.setPlanTodos([]) - } - try { await sendMessage(query, { fileAttachments, @@ -476,14 +460,7 @@ export const Copilot = forwardRef( logger.error('Failed to send message:', error) } }, - [ - inputDisabled, - isTurnInProgress, - sendMessage, - showPlanTodos, - copilotStoreApi, - sendRuntimeContext, - ] + [inputDisabled, isTurnInProgress, sendMessage, sendRuntimeContext] ) const handleEditModeChange = useCallback((messageId: string, isEditing: boolean) => { @@ -561,9 +538,9 @@ export const Copilot = forwardRef( onClick={() => scrollToBottom()} size='sm' variant='default' - className='flex items-center bg-background hover:bg-muted gap-1 rounded-lg border border-border h-7 w-7 shadow-lg transition-all' + className='flex h-7 w-7 items-center gap-1 rounded-lg border border-border bg-background shadow-lg transition-all hover:bg-muted' > - + Scroll to bottom diff --git a/apps/tradinggoose/widgets/widgets/copilot/components/user-input/components/access-level-selector.tsx b/apps/tradinggoose/widgets/widgets/copilot/components/user-input/components/access-level-selector.tsx index a78e34276..1d604f2bd 100644 --- a/apps/tradinggoose/widgets/widgets/copilot/components/user-input/components/access-level-selector.tsx +++ b/apps/tradinggoose/widgets/widgets/copilot/components/user-input/components/access-level-selector.tsx @@ -1,7 +1,6 @@ 'use client' import { Check, ShieldAlert, ShieldCheck } from 'lucide-react' -import { useCopilotMessages } from '@/i18n/workspace-widget-hooks' import { Button, DropdownMenu, @@ -15,6 +14,7 @@ import { } from '@/components/ui' import type { CopilotAccessLevel } from '@/lib/copilot/access-policy' import { cn } from '@/lib/utils' +import { useCopilotMessages } from '@/i18n/workspace-widget-hooks' interface AccessLevelSelectorProps { accessLevel: CopilotAccessLevel @@ -64,7 +64,7 @@ export function AccessLevelSelector({ )} > - + {getAccessLevelIcon('limited')} {accessLevelCopy.limited.label} {accessLevel === 'limited' && } @@ -89,7 +89,7 @@ export function AccessLevelSelector({ )} > - + {getAccessLevelIcon('full')} {accessLevelCopy.full.label} {accessLevel === 'full' && } diff --git a/changelog/June-06-2026.md b/changelog/June-06-2026.md new file mode 100644 index 000000000..f0e00cfb0 --- /dev/null +++ b/changelog/June-06-2026.md @@ -0,0 +1,86 @@ +# June-06-2026 + +## fix/copilot-tool @ 54e77de3 vs upstream/staging + +### Summary +- Standardizes Copilot tool targeting on `entityId` and document payloads on `entityDocument`, replacing workflow-specific argument names across workflow, entity, monitor, credential, Google Drive, and server tool paths. +- Adds shared entity-target and workflow-scope helpers so client and server tools resolve explicit entity targets and workflow-scoped workspace permissions through one contract. +- Persists staged review results for workflow/entity mutation tools, lets full-access mode auto-accept persisted review tools after hydration, and keeps accepted review previews visible in the inline tool UI. +- Consolidates custom tool schema validation and trims runtime manifest enrichment back to lightweight document-format guards instead of embedding workflow graph contracts in the manifest. + +### Branch Scope +- Compared `1f0a581d02fca620de367559409272b0f82c47b0..54e77de33f92bcca44ecefb5c3167966d2917e97`, where `1f0a581d02fca620de367559409272b0f82c47b0` is both the merge base and current `upstream/staging`. +- Ran `git fetch upstream staging` before comparing. This entry intentionally uses `upstream/staging`, not the template default `origin/staging`, because the user requested the upstream base. +- `git status --short` was clean before the branch comparison, so no uncommitted feature work was included in the reviewed merge-base diff. During final validation, local unstaged edits were present in `apps/tradinggoose/stores/copilot/store-messages.ts`, `apps/tradinggoose/stores/copilot/store.test.ts`, and `apps/tradinggoose/stores/copilot/store.ts`; those edits were not included as branch-scope evidence for this changelog. +- Main areas touched: Copilot tool schemas and prompt metadata under `apps/tradinggoose/lib/copilot/*`, client/server tool execution under `apps/tradinggoose/lib/copilot/tools/*`, Copilot store hydration/execution under `apps/tradinggoose/stores/copilot/*`, inline tool review UI, custom tool validation/import-export paths, and focused Copilot tool tests. + +### Key Changes +- `apps/tradinggoose/lib/copilot/tools/entity-target.ts` introduces the canonical target helpers: `CopilotEntityTargetArgs`, `CopilotEntityExecutionContext`, `resolveOptionalCopilotEntityId()`, `requireCopilotEntityId()`, and `resolveCopilotContextEntityId()`. Client and server tools now use these helpers instead of trimming `workflowId` or ad hoc `entityId` fields locally. +- `apps/tradinggoose/lib/copilot/registry.ts` changes workflow-facing schemas from `workflowId` to `entityId` and from `workflowDocument` to `entityDocument` for `read_workflow`, `edit_workflow`, `edit_workflow_block`, `rename_workflow`, `run_workflow`, workflow variables, deployment, logs, block outputs, and upstream references. Optional scoped tools such as environment variables, OAuth credentials, Google Drive, and monitors now use optional `entityId`. +- `apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts` updates tool descriptions to teach the runtime the new `entityId`/`entityDocument` contract, adds explicit custom tool document guidance, and keeps `kind`, `entityKind`, and `surfaceKind` metadata in `TOOL_PROMPT_METADATA`. +- `apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts` now builds the manifest directly from `ToolArgSchemas`, `TOOL_PROMPT_METADATA`, and `buildAutomaticSemanticValidators()` without loading workflow embedded document validators. `apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts` detects `entityDocument` for `tg-mermaid-v1` and leaves workflow structure validation to Studio instead of putting the full workflow graph contract into the runtime manifest. +- `apps/tradinggoose/lib/copilot/tools/client/base-tool.ts` adds `StagedReviewClientTool`, which stores staged review results through `stageReviewResult()`, reloads persisted review payloads through `getStagedReviewResult()`, exposes interrupt buttons only while state is `review`, and retries `execute()` before accept if a persisted review result is missing. +- Workflow mutation tools now inherit the staged review behavior. `apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts` stages server-generated workflow diffs, accepts persisted staged workflow state, writes accepted state through `setWorkflowState()` with `YJS_ORIGINS.COPILOT_REVIEW_ACCEPT`, and sends server payloads with `entityId` and `entityDocument`. `edit-workflow-block.ts` reuses the same base and sends single-block patches as `entityId`, `blockId`, optional `blockType`, `name`, `enabled`, and `subBlocks`. +- Workflow read/execution helpers enforce explicit targets. `read-workflow.ts`, `run-workflow.ts`, `deploy-workflow.ts`, `rename-workflow.ts`, `read-workflow-variables.ts`, `set-workflow-variables.ts`, `read-block-outputs.ts`, `read-block-upstream-references.ts`, and `check-deployment-status.ts` call `requireCopilotEntityId()` before touching workflow state or APIs. +- `apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts` keeps workflow-specific resolution behind `resolveWorkflowTarget()`, `getReadableWorkflowState()`, `buildWorkflowDocumentToolResult()`, and `buildWorkflowSummary()`. `buildWorkflowDocumentToolResult()` now emits the generic saved-entity payload fields `entityKind`, `entityId`, `entityName`, `entityDocument`, and `documentFormat`. +- Server tool context is now entity-scoped. `apps/tradinggoose/lib/copilot/tools/server/base-tool.ts` keeps only `userId`, `contextEntityKind`, `contextEntityId`, and `signal` in `ServerToolExecutionContext`, while `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` owns `createWorkflowPermissionError()` and `resolveServerWorkflowScope()`. +- Server workflow tools consume the generic target contract. `edit-workflow.ts`, `edit-workflow-block.ts`, and `read-workflow-logs.ts` require `entityId`; `apps/tradinggoose/lib/copilot/tools/server/router.ts` validates payloads with `ServerToolArgSchemas`; and `apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts` accepts `contextEntityKind` and `contextEntityId` in the execution envelope. +- Workflow-scoped server tools now reuse `resolveServerWorkflowScope()`: environment variable tools, OAuth/credential readers, Google Drive file readers/listers, and `get-agent-accessory-catalog.ts` all resolve workspace access from explicit `entityId` or workflow context before listing workspace resources. +- `apps/tradinggoose/stores/copilot/types.ts`, `store-provenance.ts`, and `tool-registry.ts` replace the old provenance shape with `CopilotToolExecutionProvenance` fields `contextEntityKind`, `contextEntityId`, and `workspaceId`. `createExecutionContext()` forwards only those fields to client tools and server tool calls. +- `apps/tradinggoose/stores/copilot/store.ts` auto-executes eligible queued tools through `autoExecuteEligibleToolsForAccessLevel()`: access changes may run `pending` and `review` tools, while hydration under full access only resumes `review` tools. The executor hydrates persisted tool calls before user actions and can immediately accept a staged review result when full access moves a tool from executing to review. +- `apps/tradinggoose/stores/copilot/store-messages.ts` adds `buildPlanTodosFromMessages()` so restored chats rebuild plan todos from persisted `plan`, `mark_todo_in_progress`, and `checkoff_todo` tool results. `stores/copilot/streaming.ts` carries tool `result` data into streamed content blocks when completion events include results. +- `apps/tradinggoose/lib/copilot/inline-tool-call.tsx` uses `shouldRequireStagedReviewApproval()` so full access hides review accept/reject buttons, renders workflow and entity preview payloads in both `review` and `success` states, and labels entity diffs as proposed or applied based on tool state. +- `apps/tradinggoose/lib/custom-tools/schema.ts` centralizes `CustomToolOpenAiSchema`, `CustomToolTransferSchema`, `CustomToolUpsertRequestSchema`, `CustomToolTransferRecord`, and `parseCustomToolSchemaText()`. `app/api/tools/custom/route.ts`, `lib/custom-tools/import-export.ts`, and entity document creation now import that schema instead of keeping parallel validators. +- `apps/tradinggoose/i18n/messages/en.json` refreshes waitlist copy and formatting in the branch alongside the Copilot tool work. + +### Design Decisions +- The durable Copilot argument contract is now entity-oriented. Tool schemas, prompt metadata, client tools, server tools, and persisted results should speak `entityId` and `entityDocument`; workflow-specific IDs are internal implementation details after the target has been resolved. +- Explicit mutation/execution targets are required even when the chat has a live workflow context. Tests such as `apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts` and `edit-workflow.test.ts` verify that `entityId` is required instead of falling back to `contextEntityId`. +- Workflow context is still useful for scoped reads and workspace permission checks. `resolveServerWorkflowScope()` can fall back from optional `entityId` to `contextEntityKind: 'workflow'` plus `contextEntityId` for server tools that need workspace-scoped credentials, environment variables, Google Drive access, or agent accessory catalogs. +- Staged review ownership moved into `StagedReviewClientTool` so workflow and saved-entity mutation tools share one persisted review-result lifecycle instead of each storing local `lastResult` or custom interrupt behavior. +- Runtime manifest validation is intentionally lighter for workflow documents. The manifest still guards raw Mermaid text and the document format, but full TG_WORKFLOW/TG_BLOCK/TG_EDGE consistency remains owned by Studio parsing and workflow mutation validation. +- Custom tool OpenAI function schema validation is shared between import/export, API upserts, and Copilot entity creation through `apps/tradinggoose/lib/custom-tools/schema.ts` so new paths do not drift on parameter shape or normalization. +- Generic Copilot context usage in `stores/copilot/store.ts` is conversation/user scoped. The branch stops forwarding workflow IDs to usage billing from the generic widget path because workflow context is prompt context, not the billing scope selector for this widget. + +### Shared Contracts and Helpers to Reuse +- Reuse `resolveOptionalCopilotEntityId()`, `requireCopilotEntityId()`, and `resolveCopilotContextEntityId()` from `apps/tradinggoose/lib/copilot/tools/entity-target.ts` for all new Copilot tools that accept or derive entity targets. +- Reuse `StagedReviewClientTool` from `apps/tradinggoose/lib/copilot/tools/client/base-tool.ts` for any client tool that creates an accept/reject review result. Implement `hasStagedReviewResult()` and call `stageReviewResult()` instead of adding tool-local review state. +- Reuse `resolveWorkflowTarget()`, `getReadableWorkflowState()`, `buildWorkflowDocumentToolResult()`, and `buildWorkflowSummary()` from `apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts` for workflow document reads, previews, and mutations. +- Reuse `createWorkflowPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` for server tools that may need workflow-scoped workspace permission checks. +- Reuse `ToolArgSchemas`, `ServerToolArgSchemas`, and `ToolSSESchemas` from `apps/tradinggoose/lib/copilot/registry.ts` as the canonical tool argument surface. Do not introduce a parallel schema that accepts `workflowId` for the same Copilot tool. +- Reuse `TOOL_PROMPT_METADATA` from `apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts` for manifest-visible descriptions, `kind`, `entityKind`, and `surfaceKind` metadata. +- Reuse `shouldAutoExecuteTool()`, `shouldRequireToolApproval()`, and `shouldRequireStagedReviewApproval()` from `apps/tradinggoose/lib/copilot/access-policy.ts` when deciding whether a Copilot tool should show approval controls or execute automatically. +- Reuse `buildTurnProvenanceFromContexts()`, `withPinnedToolExecutionProvenance()`, and `findAssistantMessageIdForToolCall()` from `apps/tradinggoose/stores/copilot/store-provenance.ts` when adding store paths that create, hydrate, or resume tool calls. +- Reuse `buildPlanTodosFromMessages()` from `apps/tradinggoose/stores/copilot/store-messages.ts` when restoring plan UI state from persisted Copilot messages. +- Reuse `CustomToolOpenAiSchema`, `CustomToolTransferSchema`, `CustomToolUpsertRequestSchema`, and `parseCustomToolSchemaText()` from `apps/tradinggoose/lib/custom-tools/schema.ts` for all custom-tool API, import/export, and Copilot document paths. + +### Removed or Replaced Items +- Replaced Copilot tool arguments named `workflowId` with `entityId` for workflow read, edit, run, deploy, logs, variables, monitor list filters, block output reads, server credentials, and Google Drive scoped tools. Do not reintroduce `workflowId` as the Copilot-facing argument for these tools. +- Replaced `workflowDocument` with `entityDocument` for `edit_workflow` and related manifest validation. Future document mutation tools should use `entityDocument` plus `documentFormat`. +- Removed workflow-specific fields from `ClientToolExecutionContext` and `CopilotToolExecutionProvenance`, including `workflowId`, `contextWorkflowId`, `reviewSessionId`, `entityKind`, `entityId`, and `draftSessionId`. Use `contextEntityKind`, `contextEntityId`, and `workspaceId` instead. +- Removed `createPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/base-tool.ts`. Import `createWorkflowPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` instead. +- Removed `WORKFLOW_GRAPH_CONTRACT_SPEC`, `buildWorkflowDocumentContract()`, and `WORKFLOW_DOCUMENT_CONTRACT` from `apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts`. Do not restore manifest-owned workflow graph validation; use Studio workflow parsing and mutation validation. +- Removed the dynamic import of `buildWorkflowEmbeddedDocumentValidators()` from `apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts`. Manifest generation should stay synchronous over `ToolArgSchemas`, `TOOL_PROMPT_METADATA`, and automatic semantic validators. +- Removed local custom tool schema definitions from `apps/tradinggoose/app/api/tools/custom/route.ts` and `apps/tradinggoose/lib/custom-tools/import-export.ts`. Use `apps/tradinggoose/lib/custom-tools/schema.ts`. +- Removed local `parseCustomToolSchema()` from `apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts`. Use `parseCustomToolSchemaText()`. +- Replaced entity mutation tool-local `lastResult` and custom review interrupt handling in `entity-document-tools.ts` with `StagedReviewClientTool`. New mutation tools should not recreate local staged result storage. +- Removed workflow ID forwarding from generic Copilot context-usage requests in `stores/copilot/store.ts`. Do not re-add workflow IDs to generic usage billing unless the usage API contract intentionally changes. + +### Future Branch Guardrails +- Keep Copilot tool schemas, prompts, tests, and handlers aligned on `entityId` and `entityDocument`. If a UI or database API still needs `workflowId`, convert after the Copilot boundary by calling the canonical target helpers. +- Do not make workflow mutations or executions infer the target from live workflow context. Require explicit `entityId` for destructive or side-effecting workflow tools. +- Do not add workflow-scope helpers back to generic server `base-tool.ts`. Generic server context should remain entity-kind/entity-id aware, and workflow permission resolution should stay in `tools/server/workflow/workflow-scope.ts`. +- Do not duplicate staged review state in individual tools. Use `StagedReviewClientTool` so review payloads survive persistence/hydration and full-access auto-approval behaves consistently. +- Do not restore full workflow graph contract validators to runtime manifest enrichment. Keep manifest validators cheap and let workflow mutation code validate canonical TG Mermaid state. +- Do not define new custom tool OpenAI-function schemas outside `apps/tradinggoose/lib/custom-tools/schema.ts`. +- When adding server-managed tools that need workspace-scoped credentials or resources, pass `contextEntityKind` and `contextEntityId` through `/api/copilot/execute-copilot-server-tool` and route through `resolveServerWorkflowScope()`. +- Keep accepted review previews renderable from persisted `toolCall.result`; inline UI should not depend on in-memory tool instances to show applied workflow or entity diffs. + +### Validation Notes +- Used the requested `staging-changelog` skill and followed `changelog/TEMPLATE.md`, with the explicit base override from `origin/staging` to `upstream/staging`. +- Reviewed `git status --short`, `git remote -v`, `git branch --show-current`, `git fetch upstream staging`, `git rev-parse fix/copilot-tool`, `git rev-parse upstream/staging`, `git merge-base upstream/staging fix/copilot-tool`, `git log --oneline 1f0a581d02fca620de367559409272b0f82c47b0..fix/copilot-tool`, `git diff --stat`, `git diff --name-status --find-renames`, `git diff --summary`, and targeted `git diff` output for the branch range. +- Inspected the repo instructions, `changelog/TEMPLATE.md`, existing changelog layout, new shared files `tools/entity-target.ts`, `tools/server/workflow/workflow-scope.ts`, and `custom-tools/schema.ts`, the registry/manifest/prompt metadata paths, workflow client and server tools, credential/environment/Google Drive server tools, Copilot store/provenance/streaming/tool-registry paths, inline tool UI, custom tool API/import-export code, and old merge-base contents for replaced helpers. +- Reviewed focused tests touched by the branch, including Copilot store/provenance/tool-registry tests, runtime manifest tests, inline tool call tests, workflow client/server tool tests, Google Drive server tool tests, entity/monitor tool tests, and server router tests. +- Confirmed the branch delta adds no generated migration files and deletes no files. +- Final `git status --short` showed the new `changelog/June-06-2026.md` file plus preexisting or concurrent unstaged edits in `apps/tradinggoose/stores/copilot/store-messages.ts`, `apps/tradinggoose/stores/copilot/store.test.ts`, and `apps/tradinggoose/stores/copilot/store.ts`. This changelog run did not edit those store files. +- No automated test suite was run for this changelog-only update. Validation focused on merge-base diff review, source inspection, old-content inspection for replaced symbols, and template conformance. From 4604ceb586ed176d6d17bdfe3cb0b4d6616d7fb9 Mon Sep 17 00:00:00 2001 From: Bruzzz BackUp <149516937+BWJ2310-backup@users.noreply.github.com> Date: Sat, 6 Jun 2026 16:11:56 -0600 Subject: [PATCH 2/2] fix(copilot): normalize tool parameter handling (#140) * fix(copilot): refine tool parameter handling and enhance error messages * fix(copilot): remove obsolete custom tool schema tests and update streaming handlers * refactor(copilot): remove legacy assistant toolCalls state Migrate Copilot message persistence and rendering to content blocks. Co-authored-by: Codex * fix(copilot): propagate workspace context through server tools Validate server tool arguments and carry workspace context through Copilot execution. Co-authored-by: Codex * refactor(copilot): simplify workflow document validators Remove the obsolete workflow document contract from Copilot manifest enrichment. Co-authored-by: Codex * fix(copilot): update streaming logic and refine tool execution handling * fix(copilot): standardize tool argument handling and remove legacy toolCalls schema * fix(copilot): enhance tool argument handling and implement workspace access checks * fix(copilot): update todo execution state handling in tests and state builder * fix(copilot): update showPlanTodos logic to reflect only incomplete todos --------- Co-authored-by: Codex --- .../copilot/chat/review-session-post.test.ts | 49 +++++--- .../app/api/copilot/chat/route.ts | 22 ++-- .../chat/update-messages/route.test.ts | 39 ------ .../api/copilot/chat/update-messages/route.ts | 2 - .../execute-copilot-server-tool/route.ts | 12 ++ apps/tradinggoose/lib/copilot/api.ts | 1 - .../lib/copilot/chat-replay-safety.test.ts | 13 +- .../lib/copilot/chat-replay-safety.ts | 9 -- .../lib/copilot/inline-tool-call.tsx | 2 +- apps/tradinggoose/lib/copilot/registry.ts | 13 +- .../copilot/review-sessions/thread-history.ts | 7 -- .../runtime-tool-manifest-enrichment.ts | 18 +-- .../lib/copilot/runtime-tool-manifest.test.ts | 12 +- .../entities/entity-document-tool-utils.ts | 4 +- .../tools/client/knowledge/knowledge-base.ts | 11 ++ .../tools/client/other/checkoff-todo.ts | 11 +- .../client/other/mark-todo-in-progress.ts | 11 +- .../lib/copilot/tools/client/other/plan.ts | 2 +- .../tools/client/server-tool-response.ts | 1 + .../agent/get-agent-accessory-catalog.test.ts | 3 + .../agent/get-agent-accessory-catalog.ts | 17 ++- .../lib/copilot/tools/server/base-tool.ts | 1 + .../copilot/tools/server/gdrive/list-files.ts | 4 +- .../copilot/tools/server/gdrive/read-file.ts | 4 +- .../tools/server/knowledge/knowledge-base.ts | 11 +- .../tools/server/user/read-credentials.ts | 3 +- .../server/user/read-environment-variables.ts | 10 +- .../server/user/read-oauth-credentials.ts | 5 +- .../tools/server/workflow/workflow-scope.ts | 7 ++ .../stores/copilot/store-messages.ts | 101 ++++----------- .../stores/copilot/store-provenance.test.ts | 13 ++ .../stores/copilot/store-provenance.ts | 17 ++- .../tradinggoose/stores/copilot/store.test.ts | 74 +++-------- apps/tradinggoose/stores/copilot/store.ts | 65 ++++++---- apps/tradinggoose/stores/copilot/streaming.ts | 27 ++-- .../stores/copilot/tool-registry.test.ts | 7 +- .../stores/copilot/tool-registry.ts | 24 +++- apps/tradinggoose/stores/copilot/types.ts | 1 - .../copilot-message/copilot-message.tsx | 24 ---- changelog/June-06-2026.md | 119 ++++++++---------- packages/db/schema/copilot.ts | 1 - 41 files changed, 329 insertions(+), 448 deletions(-) diff --git a/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts b/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts index 1d2cd6f71..0947718e7 100644 --- a/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts +++ b/apps/tradinggoose/app/api/copilot/chat/review-session-post.test.ts @@ -382,6 +382,7 @@ describe('Copilot Chat POST Generic Sessions', () => { { id: 'tool-call-1', name: 'lookup_context', + arguments: JSON.stringify({ query: 'price' }), success: true, result: { ok: true }, }, @@ -410,13 +411,18 @@ describe('Copilot Chat POST Generic Sessions', () => { reviewSessionId: 'review-session-1', assistantMessage: expect.objectContaining({ content: '', - toolCalls: [ - { - id: 'tool-call-1', - name: 'lookup_context', - success: true, - result: { ok: true }, - }, + contentBlocks: [ + expect.objectContaining({ + type: 'tool_call', + toolCall: { + id: 'tool-call-1', + name: 'lookup_context', + arguments: { query: 'price' }, + params: { query: 'price' }, + success: true, + result: { ok: true }, + }, + }), ], }), }) @@ -550,13 +556,16 @@ describe('Copilot Chat POST Generic Sessions', () => { reviewSessionId: 'review-session-1', assistantMessage: expect.objectContaining({ content: 'Saved response', - toolCalls: [ - { - id: 'tool-call-1', - name: 'lookup_context', - success: true, - result: { ok: true }, - }, + contentBlocks: [ + expect.objectContaining({ + type: 'tool_call', + toolCall: { + id: 'tool-call-1', + name: 'lookup_context', + success: true, + result: { ok: true }, + }, + }), ], }), }) @@ -661,7 +670,6 @@ describe('Copilot Chat POST Generic Sessions', () => { role: 'assistant', content: 'Saved response', timestamp: expect.any(String), - toolCalls: undefined, }, ], 'completed' @@ -1117,11 +1125,14 @@ describe('Copilot Chat POST Generic Sessions', () => { expect.objectContaining({ reviewSessionId: 'review-session-stringified-tool-args', assistantMessage: expect.objectContaining({ - toolCalls: [ + contentBlocks: [ expect.objectContaining({ - id: 'tool-call-stringified', - name: 'read_workflow', - arguments: { entityId: 'wf-stringified' }, + type: 'tool_call', + toolCall: expect.objectContaining({ + id: 'tool-call-stringified', + name: 'read_workflow', + arguments: { entityId: 'wf-stringified' }, + }), }), ], }), diff --git a/apps/tradinggoose/app/api/copilot/chat/route.ts b/apps/tradinggoose/app/api/copilot/chat/route.ts index 261448e02..266c1726d 100644 --- a/apps/tradinggoose/app/api/copilot/chat/route.ts +++ b/apps/tradinggoose/app/api/copilot/chat/route.ts @@ -55,7 +55,6 @@ type ReviewSessionRow = Parameters[0] interface PersistMessageAttachments { fileAttachments?: ReviewMessageInput['fileAttachments'] contexts?: ReviewMessageInput['contexts'] - toolCalls?: ReviewMessageInput['toolCalls'] contentBlocks?: ReviewMessageInput['contentBlocks'] } @@ -135,7 +134,6 @@ async function persistChatMessages( ): Promise { const hasAssistantMessage = (typeof params.assistantContent === 'string' && params.assistantContent.trim().length > 0) || - (Array.isArray(params.toolCalls) && params.toolCalls.length > 0) || (Array.isArray(params.contentBlocks) && params.contentBlocks.length > 0) await db.transaction(async (tx) => { @@ -165,7 +163,6 @@ async function persistChatMessages( role: MESSAGE_ROLES.ASSISTANT, content: params.assistantContent ?? '', timestamp: params.timestamp, - toolCalls: params.toolCalls, contentBlocks: params.contentBlocks, } : null @@ -1205,7 +1202,6 @@ export async function POST(req: NextRequest) { fileAttachments: fileAttachments && fileAttachments.length > 0 ? fileAttachments : undefined, contexts: Array.isArray(contexts) && contexts.length > 0 ? contexts : undefined, - toolCalls: toolCalls.length > 0 ? toolCalls : undefined, contentBlocks: contentBlocks.length > 0 ? contentBlocks : undefined, latestTurnStatus, }) @@ -1295,9 +1291,21 @@ export async function POST(req: NextRequest) { }) } - const toolCalls = Array.isArray(responseData.toolCalls) ? responseData.toolCalls : undefined + const contentBlocks = Array.isArray(responseData.toolCalls) + ? responseData.toolCalls.map((toolCall: any) => { + const args = normalizeFunctionCallArguments(toolCall.arguments) + return { + type: 'tool_call', + timestamp: Date.now(), + toolCall: { + ...toolCall, + ...(args ? { arguments: args, params: args } : {}), + }, + } + }) + : undefined - if (currentSession && (responseData.content || toolCalls?.length)) { + if (currentSession && (responseData.content || contentBlocks?.length)) { await persistChatMessages({ reviewSessionId: actualReviewSessionId!, userMessageId: userMessageIdToUse, @@ -1307,7 +1315,7 @@ export async function POST(req: NextRequest) { fileAttachments: fileAttachments && fileAttachments.length > 0 ? fileAttachments : undefined, contexts: Array.isArray(contexts) && contexts.length > 0 ? contexts : undefined, - toolCalls, + contentBlocks, latestTurnStatus: 'completed', }) diff --git a/apps/tradinggoose/app/api/copilot/chat/update-messages/route.test.ts b/apps/tradinggoose/app/api/copilot/chat/update-messages/route.test.ts index bf836ecea..1982c7ca6 100644 --- a/apps/tradinggoose/app/api/copilot/chat/update-messages/route.test.ts +++ b/apps/tradinggoose/app/api/copilot/chat/update-messages/route.test.ts @@ -150,7 +150,6 @@ describe('Copilot Chat Update Messages', () => { messageRole: message.role, content: message.content, timestamp: message.timestamp, - ...(Array.isArray(message.toolCalls) ? { toolCalls: message.toolCalls } : {}), ...(Array.isArray(message.contentBlocks) ? { contentBlocks: message.contentBlocks } : {}), @@ -169,7 +168,6 @@ describe('Copilot Chat Update Messages', () => { role: row.messageRole, content: row.content, timestamp: row.timestamp, - ...(Array.isArray(row.toolCalls) ? { toolCalls: row.toolCalls } : {}), ...(Array.isArray(row.contentBlocks) ? { contentBlocks: row.contentBlocks } : {}), ...(Array.isArray(row.contexts) ? { contexts: row.contexts } : {}), ...(Array.isArray(row.citations) ? { citations: row.citations } : {}), @@ -398,13 +396,6 @@ describe('Copilot Chat Update Messages', () => { messageRole: 'assistant', content: '', timestamp: '2026-03-30T12:00:01.000Z', - toolCalls: [ - { - id: 'tool-1', - name: 'edit_workflow', - state: 'success', - }, - ], contentBlocks: [ { type: 'tool_call', @@ -473,16 +464,6 @@ describe('Copilot Chat Update Messages', () => { messageRole: 'assistant', content: '', timestamp: '2026-03-30T12:00:01.000Z', - toolCalls: [ - { - id: 'tool-entity-1', - name: 'edit_skill', - state: 'success', - params: { - entityDocument: '{}', - }, - }, - ], contentBlocks: [ { type: 'tool_call', @@ -545,13 +526,6 @@ describe('Copilot Chat Update Messages', () => { messageRole: 'assistant', content: '', timestamp: '2026-03-30T12:00:01.000Z', - toolCalls: [ - { - id: 'tool-1', - name: 'edit_workflow', - state: 'pending', - }, - ], contentBlocks: [ { type: 'tool_call', @@ -574,13 +548,6 @@ describe('Copilot Chat Update Messages', () => { role: 'assistant', content: '', timestamp: '2026-03-30T12:00:01.000Z', - toolCalls: [ - { - id: 'tool-1', - name: 'edit_workflow', - state: 'rejected', - }, - ], contentBlocks: [ { type: 'tool_call', @@ -610,12 +577,6 @@ describe('Copilot Chat Update Messages', () => { expect(insertValues.mock.calls[1]?.[0]).toEqual([ expect.objectContaining({ itemId: 'message-1', - toolCalls: [ - expect.objectContaining({ - id: 'tool-1', - state: 'rejected', - }), - ], contentBlocks: [ expect.objectContaining({ type: 'tool_call', diff --git a/apps/tradinggoose/app/api/copilot/chat/update-messages/route.ts b/apps/tradinggoose/app/api/copilot/chat/update-messages/route.ts index 9dfbf26ad..d8c7fbe72 100644 --- a/apps/tradinggoose/app/api/copilot/chat/update-messages/route.ts +++ b/apps/tradinggoose/app/api/copilot/chat/update-messages/route.ts @@ -40,7 +40,6 @@ const UpdateMessagesSchema = z.object({ role: z.enum(['user', 'assistant']), content: z.string(), timestamp: z.string(), - toolCalls: z.array(z.any()).optional(), contentBlocks: z.array(z.any()).optional(), contexts: z.array(z.any()).optional(), citations: z.array(z.any()).optional(), @@ -65,7 +64,6 @@ function normalizeReviewMessageForPersistence(message: ReviewMessageApi | Incomi role: message.role, content: message.content ?? '', timestamp: message.timestamp ?? '', - toolCalls: Array.isArray(message.toolCalls) ? message.toolCalls : [], contentBlocks: Array.isArray(message.contentBlocks) ? message.contentBlocks : [], contexts: Array.isArray(message.contexts) ? message.contexts : [], citations: Array.isArray(message.citations) ? message.citations : [], diff --git a/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts b/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts index d5e0a1cf7..de79c7705 100644 --- a/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts +++ b/apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts @@ -9,6 +9,7 @@ import { import { REVIEW_ENTITY_KINDS } from '@/lib/copilot/review-sessions/types' import { buildCopilotServerToolErrorResponse } from '@/lib/copilot/server-tool-errors' import { createLogger } from '@/lib/logs/console/logger' +import { checkWorkspaceAccess } from '@/lib/permissions/utils' const logger = createLogger('ExecuteCopilotServerToolAPI') @@ -19,6 +20,7 @@ const ExecuteSchema = z.object({ .object({ contextEntityKind: z.enum(REVIEW_ENTITY_KINDS).optional(), contextEntityId: z.string().optional(), + workspaceId: z.string().optional(), }) .optional(), }) @@ -63,6 +65,16 @@ export async function POST(req: NextRequest) { } logger.info(`[${tracker.requestId}] Executing server tool`, { toolName }) + if (context?.workspaceId) { + const workspaceAccess = await checkWorkspaceAccess(context.workspaceId, userId) + if (!workspaceAccess.exists || !workspaceAccess.hasAccess) { + return NextResponse.json( + { error: 'Access denied to this workspace', code: 'WORKSPACE_ACCESS_DENIED' }, + { status: 403 } + ) + } + } + const result = await routeExecution(toolName, payload, { userId, ...context, diff --git a/apps/tradinggoose/lib/copilot/api.ts b/apps/tradinggoose/lib/copilot/api.ts index 04a068b16..fd8977576 100644 --- a/apps/tradinggoose/lib/copilot/api.ts +++ b/apps/tradinggoose/lib/copilot/api.ts @@ -24,7 +24,6 @@ export interface CopilotMessage { content: string timestamp: string citations?: Citation[] - toolCalls?: any[] contentBlocks?: any[] fileAttachments?: any[] contexts?: any[] diff --git a/apps/tradinggoose/lib/copilot/chat-replay-safety.test.ts b/apps/tradinggoose/lib/copilot/chat-replay-safety.test.ts index 0b5379539..f8a15392c 100644 --- a/apps/tradinggoose/lib/copilot/chat-replay-safety.test.ts +++ b/apps/tradinggoose/lib/copilot/chat-replay-safety.test.ts @@ -129,11 +129,14 @@ describe('chat replay safety', () => { expect( messageHasAcceptedLiveMutation({ id: 'assistant-2', - toolCalls: [ + contentBlocks: [ { - id: 'tool-2', - name: 'list_custom_tools', - state: 'success', + type: 'tool_call', + toolCall: { + id: 'tool-2', + name: 'list_custom_tools', + state: 'success', + }, }, ], }) @@ -144,7 +147,6 @@ describe('chat replay safety', () => { const messages = [ { id: 'message-1', - toolCalls: [], }, { id: 'message-2', @@ -161,7 +163,6 @@ describe('chat replay safety', () => { }, { id: 'message-3', - toolCalls: [], }, ] diff --git a/apps/tradinggoose/lib/copilot/chat-replay-safety.ts b/apps/tradinggoose/lib/copilot/chat-replay-safety.ts index 178c4d6c8..4ec0231ab 100644 --- a/apps/tradinggoose/lib/copilot/chat-replay-safety.ts +++ b/apps/tradinggoose/lib/copilot/chat-replay-safety.ts @@ -14,7 +14,6 @@ interface ReplaySafetyBlockLike { export interface ReplaySafetyMessageLike { id: string - toolCalls?: unknown contentBlocks?: unknown } @@ -66,14 +65,6 @@ export function isAcceptedLiveMutationToolCall(toolCall: unknown): boolean { } export function messageHasAcceptedLiveMutation(message: ReplaySafetyMessageLike): boolean { - if (Array.isArray(message.toolCalls)) { - for (const toolCall of message.toolCalls) { - if (isAcceptedLiveMutationToolCall(toolCall)) { - return true - } - } - } - if (Array.isArray(message.contentBlocks)) { for (const block of message.contentBlocks) { const candidate = asToolCallBlock(block) diff --git a/apps/tradinggoose/lib/copilot/inline-tool-call.tsx b/apps/tradinggoose/lib/copilot/inline-tool-call.tsx index 27678606c..4edbe2715 100644 --- a/apps/tradinggoose/lib/copilot/inline-tool-call.tsx +++ b/apps/tradinggoose/lib/copilot/inline-tool-call.tsx @@ -418,7 +418,7 @@ export function InlineToolCall({ } const displayName = getDisplayName(toolCall) - const params = (toolCall as any).parameters || (toolCall as any).input || toolCall.params || {} + const params = toolCall.params ?? {} const entityReviewPayload = readEntityReviewPayload(toolCall) const workflowReviewPayload = readWorkflowReviewPayload(toolCall) const showWorkflowReview = workflowReviewPayload && isStagedPreviewState(toolCall.state) diff --git a/apps/tradinggoose/lib/copilot/registry.ts b/apps/tradinggoose/lib/copilot/registry.ts index f90382775..a6e61df58 100644 --- a/apps/tradinggoose/lib/copilot/registry.ts +++ b/apps/tradinggoose/lib/copilot/registry.ts @@ -254,7 +254,6 @@ export const ToolArgSchemas = { z.string(), z.object({ id: z.string().optional(), - todoId: z.string().optional(), content: z.string(), }), ]) @@ -262,12 +261,10 @@ export const ToolArgSchemas = { .optional(), }), checkoff_todo: z.object({ - id: z.string().optional(), - todoId: z.string().optional(), + id: z.string(), }), mark_todo_in_progress: z.object({ - id: z.string().optional(), - todoId: z.string().optional(), + id: z.string(), }), [CopilotTool.read_workflow]: z .object({ @@ -823,12 +820,10 @@ export const ToolResultSchemas = { todoList: z.array(z.any()).optional(), }), checkoff_todo: z.object({ - todoId: z.string().optional(), - id: z.string().optional(), + id: z.string(), }), mark_todo_in_progress: z.object({ - todoId: z.string().optional(), - id: z.string().optional(), + id: z.string(), }), [CopilotTool.read_workflow]: WorkflowReadDocumentEnvelope, create_workflow: WorkflowMutationResult, diff --git a/apps/tradinggoose/lib/copilot/review-sessions/thread-history.ts b/apps/tradinggoose/lib/copilot/review-sessions/thread-history.ts index afda34cd6..2fe80202f 100644 --- a/apps/tradinggoose/lib/copilot/review-sessions/thread-history.ts +++ b/apps/tradinggoose/lib/copilot/review-sessions/thread-history.ts @@ -25,7 +25,6 @@ export interface ReviewMessageApi { role: string content: string | null timestamp: string | null - toolCalls?: unknown contentBlocks?: unknown contexts?: unknown fileAttachments?: unknown @@ -37,7 +36,6 @@ export interface ReviewMessageInput { role: MessageRole | string content: string timestamp: string - toolCalls?: unknown contentBlocks?: unknown contexts?: unknown fileAttachments?: unknown @@ -80,7 +78,6 @@ export function mapReviewItemToApi(item: { messageRole: string content: string | null timestamp: string | null - toolCalls?: unknown contentBlocks?: unknown contexts?: unknown fileAttachments?: unknown @@ -91,7 +88,6 @@ export function mapReviewItemToApi(item: { role: item.messageRole, content: item.content, timestamp: item.timestamp, - ...spreadIfArray('toolCalls', item.toolCalls), ...spreadIfArray('contentBlocks', item.contentBlocks), ...spreadIfArray('contexts', item.contexts), ...spreadIfArray('fileAttachments', item.fileAttachments), @@ -131,9 +127,6 @@ export function buildReviewItemInsert(params: { messageRole: normalizeMessageRole(params.message.role), content: params.message.content, timestamp: params.message.timestamp, - ...(normalizeArrayLike(params.message.toolCalls) - ? { toolCalls: params.message.toolCalls as unknown[] } - : {}), ...(normalizeArrayLike(params.message.contentBlocks) ? { contentBlocks: params.message.contentBlocks as unknown[] } : contextsContentBlock diff --git a/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts b/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts index a87d790ce..47b0522c6 100644 --- a/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts +++ b/apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts @@ -159,7 +159,7 @@ function buildWorkflowDocumentSemanticValidators( path: documentField, kind: 'string_requires_real_newlines', description: - 'Cheap format guard only. Use raw Mermaid text with real newlines; Studio validates TG_WORKFLOW, TG_BLOCK, and edge consistency.', + 'Use raw Mermaid text with real newlines; Studio validates workflow graph semantics.', message: 'Expected raw Mermaid text with real newline characters, not JSON-escaped `\\n` sequences.', }, @@ -168,7 +168,7 @@ function buildWorkflowDocumentSemanticValidators( kind: 'string_starts_with', args: { prefix: 'flowchart ' }, description: - 'Cheap format guard only. Start with a Mermaid `flowchart` declaration; Studio validates canonical workflow structure.', + 'Start with a Mermaid `flowchart` declaration; Studio validates canonical workflow structure.', message: 'Expected raw Mermaid text that starts with a `flowchart` declaration.', }, { @@ -229,20 +229,6 @@ function buildWorkflowDocumentSemanticValidators( description: 'Use canonical `TG_BLOCK` state, not simplified block metadata aliases.', message: '`TG_BLOCK` metadata must not include `blockDescription`.', }, - { - path: documentField, - kind: 'string_document_contract', - args: { - format: TG_MERMAID_DOCUMENT_FORMAT, - workflowPrefix: TG_WORKFLOW_LINE_PREFIX, - blockPrefix: TG_BLOCK_LINE_PREFIX, - edgePrefix: TG_EDGE_LINE_PREFIX, - }, - description: - 'Keep visible Mermaid connection lines aligned with canonical `TG_EDGE` metadata.', - message: - 'Visible Mermaid connections and `TG_EDGE` metadata must describe the same logical workflow edges.', - }, ] } diff --git a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts index 1d50abee3..62bfed4d9 100644 --- a/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts +++ b/apps/tradinggoose/lib/copilot/runtime-tool-manifest.test.ts @@ -151,7 +151,7 @@ describe('copilot runtime tool manifest', () => { expect.objectContaining({ path: 'entityDocument', kind: 'string_requires_real_newlines', - description: expect.stringContaining('Cheap format guard only'), + description: expect.stringContaining('Studio validates workflow graph semantics'), }), expect.objectContaining({ path: 'entityDocument', @@ -291,9 +291,9 @@ describe('copilot runtime tool manifest', () => { 'string_requires_line_prefix', 'string_line_prefix_json_schema', 'string_forbids_substring', - 'string_document_contract', ]) ) + expect(workflowValidatorKinds).not.toContain('string_document_contract') expect(editWorkflowValidators).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -308,14 +308,6 @@ describe('copilot runtime tool manifest', () => { kind: 'string_line_prefix_json_schema', args: expect.objectContaining({ prefix: '%% TG_EDGE ', schema: expect.any(Object) }), }), - expect.objectContaining({ - kind: 'string_document_contract', - args: expect.objectContaining({ - workflowPrefix: '%% TG_WORKFLOW ', - blockPrefix: '%% TG_BLOCK ', - edgePrefix: '%% TG_EDGE ', - }), - }), ]) ) const editWorkflowProperties = diff --git a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts index 1d0723df0..44126a3da 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts @@ -289,7 +289,9 @@ export function resolveWorkspaceIdFromExecutionContext( return executionContext.workspaceId } - throw new Error('No active workspace found') + throw new Error( + 'No active workspace found in execution context. Ensure workspaceId is included in tool provenance.' + ) } function createBootstrappedEntitySessionLease( diff --git a/apps/tradinggoose/lib/copilot/tools/client/knowledge/knowledge-base.ts b/apps/tradinggoose/lib/copilot/tools/client/knowledge/knowledge-base.ts index 670607ba6..bd4afeb64 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/knowledge/knowledge-base.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/knowledge/knowledge-base.ts @@ -98,10 +98,21 @@ export class KnowledgeBaseClientTool extends BaseClientTool { const logger = createLogger('KnowledgeBaseClientTool') try { this.setState(ClientToolCallState.executing) + const executionContext = this.getExecutionContext() const payload: KnowledgeBaseArgs = { ...(args || { operation: 'list' }) } + if ( + executionContext?.workspaceId && + (payload.operation === 'create' || payload.operation === 'list') && + !payload.args?.workspaceId + ) { + payload.args = { ...(payload.args ?? {}), workspaceId: executionContext.workspaceId } + } const result = await executeCopilotServerTool({ toolName: 'knowledge_base', payload, + context: executionContext?.workspaceId + ? { workspaceId: executionContext.workspaceId } + : undefined, signal: this.getAbortSignal(), }) await this.markToolComplete(200, 'Knowledge base operation completed', result) diff --git a/apps/tradinggoose/lib/copilot/tools/client/other/checkoff-todo.ts b/apps/tradinggoose/lib/copilot/tools/client/other/checkoff-todo.ts index 6b73f6555..07009b978 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/other/checkoff-todo.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/other/checkoff-todo.ts @@ -7,8 +7,7 @@ import { import { createLogger } from '@/lib/logs/console/logger' interface CheckoffTodoArgs { - id?: string - todoId?: string + id: string } export class CheckoffTodoClientTool extends BaseClientTool { @@ -32,8 +31,8 @@ export class CheckoffTodoClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) - const todoId = args?.id || args?.todoId - if (!todoId) { + const todoItemId = args?.id + if (!todoItemId) { this.setState(ClientToolCallState.error) await this.markToolComplete(400, 'Missing todo id') return @@ -43,14 +42,14 @@ export class CheckoffTodoClientTool extends BaseClientTool { const { getCopilotStoreForToolCall } = await import('@/stores/copilot/store-access') const store = getCopilotStoreForToolCall(this.toolCallId).getState() if (store.updatePlanTodoStatus) { - store.updatePlanTodoStatus(todoId, 'completed') + store.updatePlanTodoStatus(todoItemId, 'completed') } } catch (e) { logger.warn('Failed to update todo status in store', { message: (e as any)?.message }) } this.setState(ClientToolCallState.success) - await this.markToolComplete(200, 'Todo checked off', { todoId }) + await this.markToolComplete(200, 'Todo checked off', { id: todoItemId }) this.setState(ClientToolCallState.success) } catch (e: any) { logger.error('execute failed', { message: e?.message }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/other/mark-todo-in-progress.ts b/apps/tradinggoose/lib/copilot/tools/client/other/mark-todo-in-progress.ts index 5e5b796e4..5bc4ba68a 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/other/mark-todo-in-progress.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/other/mark-todo-in-progress.ts @@ -7,8 +7,7 @@ import { import { createLogger } from '@/lib/logs/console/logger' interface MarkTodoInProgressArgs { - id?: string - todoId?: string + id: string } export class MarkTodoInProgressClientTool extends BaseClientTool { @@ -35,8 +34,8 @@ export class MarkTodoInProgressClientTool extends BaseClientTool { try { this.setState(ClientToolCallState.executing) - const todoId = args?.id || args?.todoId - if (!todoId) { + const todoItemId = args?.id + if (!todoItemId) { this.setState(ClientToolCallState.error) await this.markToolComplete(400, 'Missing todo id') return @@ -46,14 +45,14 @@ export class MarkTodoInProgressClientTool extends BaseClientTool { const { getCopilotStoreForToolCall } = await import('@/stores/copilot/store-access') const store = getCopilotStoreForToolCall(this.toolCallId).getState() if (store.updatePlanTodoStatus) { - store.updatePlanTodoStatus(todoId, 'executing') + store.updatePlanTodoStatus(todoItemId, 'executing') } } catch (e) { logger.warn('Failed to update todo status in store', { message: (e as any)?.message }) } this.setState(ClientToolCallState.success) - await this.markToolComplete(200, 'Todo marked in progress', { todoId }) + await this.markToolComplete(200, 'Todo marked in progress', { id: todoItemId }) this.setState(ClientToolCallState.success) } catch (e: any) { logger.error('execute failed', { message: e?.message }) diff --git a/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts b/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts index f2224be9d..c5b8db330 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/other/plan.ts @@ -40,7 +40,7 @@ export class PlanClientTool extends BaseClientTool { const todoList = args?.todoList if (Array.isArray(todoList)) { const todos = todoList.map((item: any, index: number) => ({ - id: (item && (item.id || item.todoId)) || `todo-${index}`, + id: item?.id || `todo-${index}`, content: typeof item === 'string' ? item : item.content, completed: false, executing: false, diff --git a/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts b/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts index dae5d02d3..7f9610d3c 100644 --- a/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts +++ b/apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts @@ -72,6 +72,7 @@ export async function executeCopilotServerTool(input: { context?: { contextEntityKind?: ReviewEntityKind contextEntityId?: string + workspaceId?: string } signal?: AbortSignal }): Promise { diff --git a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts index fb6c43419..ed88be544 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.test.ts @@ -1,10 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mockResolveServerWorkflowScope = vi.hoisted(() => vi.fn()) +const mockResolveServerWorkspaceId = vi.hoisted(() => vi.fn()) const mockListCustomTools = vi.hoisted(() => vi.fn()) vi.mock('@/lib/copilot/tools/server/workflow/workflow-scope', () => ({ resolveServerWorkflowScope: mockResolveServerWorkflowScope, + resolveServerWorkspaceId: mockResolveServerWorkspaceId, })) vi.mock('@/lib/copilot/tools/server/blocks/block-mermaid-catalog', () => ({ @@ -42,6 +44,7 @@ describe('getAgentAccessoryCatalogServerTool', () => { workspaceId: 'workspace-1', workflowId: 'workflow-1', }) + mockResolveServerWorkspaceId.mockReturnValue('workspace-1') mockListCustomTools.mockResolvedValue([ { id: 'custom-tool-1', diff --git a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts index 156ca28a4..a8ff5b5b1 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/agent/get-agent-accessory-catalog.ts @@ -1,7 +1,10 @@ import { CopilotTool } from '@/lib/copilot/registry' import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool' import { listWorkflowBlockCatalogItems } from '@/lib/copilot/tools/server/blocks/block-mermaid-catalog' -import { resolveServerWorkflowScope } from '@/lib/copilot/tools/server/workflow/workflow-scope' +import { + resolveServerWorkspaceId, + resolveServerWorkflowScope, +} from '@/lib/copilot/tools/server/workflow/workflow-scope' import type { GetAgentAccessoryCatalogInputType, GetAgentAccessoryCatalogResultType, @@ -75,15 +78,19 @@ export const getAgentAccessoryCatalogServerTool: BaseServerTool< if (!context?.userId) throw new Error('User context is required') const scope = await resolveServerWorkflowScope(args, context) - if (!scope?.hasAccess || !scope.workspaceId) { + if (scope && !scope.hasAccess) { throw new Error('Workflow not found or access denied') } + const workspaceId = resolveServerWorkspaceId(context, scope) + if (!workspaceId) { + throw new Error('Workspace context is required') + } const [blockToolOptions, customToolRows, mcpToolRows, skillRows] = await Promise.all([ getBlockToolOptions(), - listCustomTools({ workspaceId: scope.workspaceId }), - mcpService.discoverTools(context.userId, scope.workspaceId), - listSkills({ workspaceId: scope.workspaceId }), + listCustomTools({ workspaceId }), + mcpService.discoverTools(context.userId, workspaceId), + listSkills({ workspaceId }), ]) return { diff --git a/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts b/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts index 03df8a971..e3208605c 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/base-tool.ts @@ -5,6 +5,7 @@ export interface ServerToolExecutionContext { userId: string contextEntityKind?: ReviewEntityKind contextEntityId?: string + workspaceId?: string signal?: AbortSignal } diff --git a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts index e37e7351e..1e611a6f6 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/gdrive/list-files.ts @@ -5,6 +5,7 @@ import { } from '@/lib/copilot/tools/server/base-tool' import { createWorkflowPermissionError, + resolveServerWorkspaceId, resolveServerWorkflowScope, } from '@/lib/copilot/tools/server/workflow/workflow-scope' import { getOAuthAccessTokenForUserCredential } from '@/lib/credentials/oauth' @@ -33,6 +34,7 @@ export const listGDriveFilesServerTool: BaseServerTool throw new Error(createWorkflowPermissionError('access Google Drive files in')) } throwIfServerToolAborted(context) + const workspaceId = resolveServerWorkspaceId(context, workflowScope) const accessToken = await getOAuthAccessTokenForUserCredential({ credentialId, userId, requestId: `copilot-gdrive-read-${credentialId}`, - workspaceId: workflowScope?.workspaceId, + workspaceId, }) if (!accessToken) { throw new Error( diff --git a/apps/tradinggoose/lib/copilot/tools/server/knowledge/knowledge-base.ts b/apps/tradinggoose/lib/copilot/tools/server/knowledge/knowledge-base.ts index 935fc693b..1ee18306e 100644 --- a/apps/tradinggoose/lib/copilot/tools/server/knowledge/knowledge-base.ts +++ b/apps/tradinggoose/lib/copilot/tools/server/knowledge/knowledge-base.ts @@ -30,6 +30,7 @@ export const knowledgeBaseServerTool: BaseServerTool { - const normalizedToolCall = { - ...tc, - state: normalizeReloadedToolState(tc?.state, latestTurnStatus), - } - - const instance = ensureClientToolInstance( - normalizedToolCall?.name, - normalizedToolCall?.id - ) - instance?.hydratePersistedToolCall?.(normalizedToolCall) - - return { - ...normalizedToolCall, - display: resolveToolDisplay( - normalizedToolCall?.name, - normalizedToolCall.state, - normalizedToolCall?.id, - normalizedToolCall?.params - ), - ...(normalizedToolCall?.result !== undefined - ? { result: normalizedToolCall.result } - : {}), - } - }) - : (message as any).toolCalls - return { ...message, content: normalizedContent.content, - ...(updatedToolCalls && { toolCalls: updatedToolCalls }), ...(finalBlocks.length > 0 ? { contentBlocks: finalBlocks } : {}), } }) @@ -286,12 +257,6 @@ export function buildPinnedToolCallsById( continue } - if (Array.isArray((message as any).toolCalls)) { - for (const toolCall of (message as any).toolCalls as CopilotToolCall[]) { - pinToolCall(toolCall) - } - } - if (!message.contentBlocks) continue for (const block of message.contentBlocks as any[]) { @@ -315,10 +280,6 @@ function readToolCallsInMessageOrder(message: CopilotMessage): CopilotToolCall[] toolCalls.push(toolCall) } - for (const toolCall of message.toolCalls ?? []) { - appendToolCall(toolCall) - } - for (const block of message.contentBlocks ?? []) { if (block.type === 'tool_call' && block.toolCall) { appendToolCall(block.toolCall) @@ -328,6 +289,14 @@ function readToolCallsInMessageOrder(message: CopilotMessage): CopilotToolCall[] return toolCalls } +function isRecord(value: unknown): value is Record { + return Boolean(value && typeof value === 'object' && !Array.isArray(value)) +} + +function readCanonicalStringId(value: unknown): string | null { + return typeof value === 'string' && value.trim().length > 0 ? value : null +} + export function buildPlanTodosFromMessages(messages: CopilotMessage[]): PlanTodo[] { let todos: PlanTodo[] = [] @@ -337,13 +306,8 @@ export function buildPlanTodosFromMessages(messages: CopilotMessage[]): PlanTodo continue } - const args = ((toolCall as any).params || (toolCall as any).input || {}) as Record< - string, - any - > - const result = ( - toolCall.result && typeof toolCall.result === 'object' ? toolCall.result : {} - ) as Record + const args = toolCall.params ?? {} + const result = isRecord(toolCall.result) ? toolCall.result : {} if (toolCall.name === 'plan') { const todoList = Array.isArray(result.todoList) @@ -353,19 +317,20 @@ export function buildPlanTodosFromMessages(messages: CopilotMessage[]): PlanTodo : null if (todoList) { todos = todoList - .map((item: any, index: number) => { + .map((item: unknown, index: number) => { + const itemRecord = isRecord(item) ? item : null const content = typeof item === 'string' ? item.trim() - : typeof item?.content === 'string' - ? item.content.trim() + : typeof itemRecord?.content === 'string' + ? itemRecord.content.trim() : '' return content ? { - id: String(item?.id || item?.todoId || `todo-${index}`), + id: readCanonicalStringId(itemRecord?.id) ?? `todo-${index}`, content, - completed: item?.completed === true, - executing: item?.executing === true, + completed: itemRecord?.completed === true, + executing: itemRecord?.executing === true, } : null }) @@ -374,18 +339,18 @@ export function buildPlanTodosFromMessages(messages: CopilotMessage[]): PlanTodo continue } - const todoId = args.id || args.todoId || result.id || result.todoId - if (!todoId) { + const todoItemId = readCanonicalStringId(args.id) + if (!todoItemId) { continue } if (toolCall.name === 'mark_todo_in_progress') { todos = todos.map((todo) => - todo.id === todoId ? { ...todo, completed: false, executing: true } : todo + todo.id === todoItemId ? { ...todo, completed: false, executing: true } : todo ) } else if (toolCall.name === 'checkoff_todo') { todos = todos.map((todo) => - todo.id === todoId ? { ...todo, completed: true, executing: false } : todo + todo.id === todoItemId ? { ...todo, completed: true, executing: false } : todo ) } } @@ -439,22 +404,6 @@ export function updateMessagesForToolCallState( }) : message.contentBlocks - const toolCalls = Array.isArray((message as any).toolCalls) - ? (message as any).toolCalls.map((toolCall: any) => { - if (toolCall?.id !== toolCallId) { - return toolCall - } - - blocksChanged = true - return { - ...toolCall, - state: nextState, - display: resolveToolDisplay(toolCall?.name, nextState, toolCallId, toolCall?.params), - ...(options?.result !== undefined ? { result: options.result } : {}), - } - }) - : (message as any).toolCalls - if (!blocksChanged) { result[i] = message continue @@ -464,7 +413,6 @@ export function updateMessagesForToolCallState( result[i] = { ...message, ...(contentBlocks ? { contentBlocks } : {}), - ...(toolCalls ? { toolCalls } : {}), } } return result @@ -538,10 +486,6 @@ export function validateMessagesForLLM(messages: CopilotMessage[]): any[] { role: msg.role, content, timestamp: msg.timestamp, - ...(Array.isArray((msg as any).toolCalls) && - (msg as any).toolCalls.length > 0 && { - toolCalls: (msg as any).toolCalls, - }), ...(Array.isArray(msg.citations) && msg.citations.length > 0 && { citations: msg.citations, @@ -563,10 +507,9 @@ export function validateMessagesForLLM(messages: CopilotMessage[]): any[] { .filter((m) => { if (m.role === 'assistant') { const hasText = typeof m.content === 'string' && m.content.trim().length > 0 - const hasTools = Array.isArray((m as any).toolCalls) && (m as any).toolCalls.length > 0 const hasBlocks = Array.isArray((m as any).contentBlocks) && (m as any).contentBlocks.length > 0 - return hasText || hasTools || hasBlocks + return hasText || hasBlocks } return true }) diff --git a/apps/tradinggoose/stores/copilot/store-provenance.test.ts b/apps/tradinggoose/stores/copilot/store-provenance.test.ts index aae1309ff..5294a69bd 100644 --- a/apps/tradinggoose/stores/copilot/store-provenance.test.ts +++ b/apps/tradinggoose/stores/copilot/store-provenance.test.ts @@ -75,4 +75,17 @@ describe('buildTurnProvenanceFromContexts', () => { contextEntityId: 'workflow-explicit', }) }) + + it('does not synthesize workspace provenance for incomplete non-workflow review targets', () => { + expect( + buildTurnProvenanceFromContexts(undefined, null, null, { + workspaceId: null, + entityKind: 'skill', + entityId: 'skill-review', + draftSessionId: null, + reviewSessionId: 'review-1', + yjsSessionId: 'review-1', + }) + ).toBeUndefined() + }) }) diff --git a/apps/tradinggoose/stores/copilot/store-provenance.ts b/apps/tradinggoose/stores/copilot/store-provenance.ts index 83be1665f..265dcb973 100644 --- a/apps/tradinggoose/stores/copilot/store-provenance.ts +++ b/apps/tradinggoose/stores/copilot/store-provenance.ts @@ -78,10 +78,14 @@ export function buildTurnProvenanceFromContexts( } } - if (reviewTarget && reviewTarget.entityKind !== 'workflow') { + if (reviewTarget && reviewTarget.entityKind !== ENTITY_KIND_WORKFLOW) { const reviewWorkspaceId = normalizeOptionalString(reviewTarget.workspaceId) - if (reviewWorkspaceId) provenance.workspaceId = reviewWorkspaceId - hasContext = Boolean(reviewWorkspaceId) || hasContext + if (!reviewWorkspaceId) { + return hasContext ? provenance : undefined + } + + provenance.workspaceId = reviewWorkspaceId + hasContext = true } return hasContext ? provenance : undefined @@ -114,13 +118,6 @@ export function findAssistantMessageIdForToolCall( const message = messages[index] if (message.role !== 'assistant') continue - if ( - Array.isArray(message.toolCalls) && - message.toolCalls.some((toolCall) => toolCall.id === toolCallId) - ) { - return message.id - } - if ( Array.isArray(message.contentBlocks) && message.contentBlocks.some( diff --git a/apps/tradinggoose/stores/copilot/store.test.ts b/apps/tradinggoose/stores/copilot/store.test.ts index c9a21b778..c3f2d42f2 100644 --- a/apps/tradinggoose/stores/copilot/store.test.ts +++ b/apps/tradinggoose/stores/copilot/store.test.ts @@ -729,7 +729,6 @@ describe('copilot streaming regressions', () => { ], }), toolBlock('checkoff_todo', 'todo-tool-1', { id: 'todo-1' }), - toolBlock('mark_todo_in_progress', 'todo-tool-2', { id: 'todo-2' }), toolBlock( 'mark_todo_in_progress', 'todo-tool-pending-1', @@ -749,16 +748,8 @@ describe('copilot streaming regressions', () => { role: 'assistant', content: '', timestamp: '2026-04-13T00:00:01.000Z', - toolCalls: [ - { - id: 'todo-tool-collision', - name: 'checkoff_todo', - state: ClientToolCallState.success, - params: { id: 'todo-1' }, - }, - ], contentBlocks: [ - toolBlock('mark_todo_in_progress', 'todo-tool-collision', { id: 'todo-1' }), + toolBlock('checkoff_todo', 'todo-tool-collision', { id: 'todo-1' }), ], }, ] as any @@ -796,11 +787,10 @@ describe('copilot streaming regressions', () => { id: 'todo-2', content: 'Apply the workflow edit', completed: false, - executing: true, + executing: false, }, ]) - store.getState().closePlanTodos() store.getState().updatePlanTodoStatus('todo-2', 'completed') expect(store.getState().showPlanTodos).toBe(false) expect(store.getState().planTodos[1]).toMatchObject({ @@ -808,6 +798,9 @@ describe('copilot streaming regressions', () => { completed: true, executing: false, }) + + store.getState().updatePlanTodoStatus('todo-2', 'executing') + expect(store.getState().showPlanTodos).toBe(true) }) it('uses the final output item text when it differs from streamed deltas', async () => { @@ -1034,7 +1027,7 @@ describe('copilot streaming regressions', () => { type: 'function_call', call_id: 'pending-approval-tool', name: 'get_blocks_metadata', - arguments: {}, + arguments: { blockTypes: ['agent'] }, }, }, { type: 'awaiting_tools', data: { pendingToolCallIds: ['pending-approval-tool'] } }, @@ -1047,16 +1040,9 @@ describe('copilot streaming regressions', () => { return url === '/api/copilot/chat/update-messages' }) const updateMessagesBody = parseJsonRequestBody(updateMessageCalls.at(-1)) - expect(updateMessagesBody.latestTurnStatus).toBe('in_progress') - expect(store.getState().currentChat?.latestTurnStatus).toBe('in_progress') - expect(store.getState().toolCallsById['pending-approval-tool']?.state).toBe( - ClientToolCallState.pending - ) - expect(store.getState().isSendingMessage).toBe(true) - expect(store.getState().isAwaitingContinuation).toBe(true) - - await new Promise((resolve) => setTimeout(resolve, 0)) - await Promise.resolve() + const [persistedMessage] = updateMessagesBody.messages as any[] + expect(updateMessagesBody.latestTurnStatus).toBe('completed') + expect(persistedMessage.contentBlocks[0].toolCall.state).toBe(ClientToolCallState.success) expect( fetchMock.mock.calls.some(([input]) => { @@ -1064,9 +1050,12 @@ describe('copilot streaming regressions', () => { return url === '/api/copilot/execute-copilot-server-tool' }) ).toBe(true) + expect(store.getState().currentChat?.latestTurnStatus).toBe('completed') expect(store.getState().toolCallsById['pending-approval-tool']?.state).toBe( ClientToolCallState.success ) + expect(store.getState().isSendingMessage).toBe(false) + expect(store.getState().isAwaitingContinuation).toBe(false) }) it('keeps aborted server tools terminal after late completion', async () => { @@ -1101,7 +1090,7 @@ describe('copilot streaming regressions', () => { id: toolCallId, name: 'get_blocks_metadata', state: ClientToolCallState.pending, - params: { blockIds: ['agent'] }, + params: { blockTypes: ['agent'] }, } as any, }, }) @@ -1627,14 +1616,6 @@ describe('copilot streaming regressions', () => { role: 'assistant', content: '', timestamp: '2026-03-30T00:00:00.000Z', - toolCalls: [ - { - id: 'tool-active-chat', - name: 'edit_indicator', - state: ClientToolCallState.executing, - params: { entityDocument: '{}' }, - }, - ], contentBlocks: [ { type: 'tool_call', @@ -1671,9 +1652,6 @@ describe('copilot streaming regressions', () => { reviewSessionId: 'review-active-chat', latestTurnStatus: 'completed', }) - expect((requestBody.messages as any[])?.[0]?.toolCalls?.[0]?.state).toBe( - ClientToolCallState.aborted - ) expect((requestBody.messages as any[])?.[0]?.contentBlocks?.[0]?.toolCall?.state).toBe( ClientToolCallState.aborted ) @@ -1758,17 +1736,6 @@ describe('copilot streaming regressions', () => { role: 'assistant', content: '', timestamp: '2026-03-30T00:00:00.000Z', - toolCalls: [ - { - id: 'tool-review-chat', - name: 'edit_workflow', - state: ClientToolCallState.review, - params: { - entityDocument: 'workflow: {}', - entityId: 'wf-active-chat-review-tools', - }, - }, - ], contentBlocks: [ { type: 'tool_call', @@ -1811,9 +1778,6 @@ describe('copilot streaming regressions', () => { reviewSessionId: 'review-active-chat-review-tools', latestTurnStatus: 'completed', }) - expect((requestBody.messages as any[])?.[0]?.toolCalls?.[0]?.state).toBe( - ClientToolCallState.review - ) expect((requestBody.messages as any[])?.[0]?.contentBlocks?.[0]?.toolCall?.state).toBe( ClientToolCallState.review ) @@ -1867,14 +1831,6 @@ describe('copilot streaming regressions', () => { role: 'assistant', content: '', timestamp: '2026-03-30T00:00:00.000Z', - toolCalls: [ - { - id: 'tool-new-chat-abort', - name: 'edit_indicator', - state: ClientToolCallState.pending, - params: { entityDocument: '{}' }, - }, - ], contentBlocks: [ { type: 'tool_call', @@ -1911,9 +1867,6 @@ describe('copilot streaming regressions', () => { reviewSessionId: 'review-new-chat-abort', latestTurnStatus: 'completed', }) - expect((requestBody.messages as any[])?.[0]?.toolCalls?.[0]?.state).toBe( - ClientToolCallState.aborted - ) expect((requestBody.messages as any[])?.[0]?.contentBlocks?.[0]?.toolCall?.state).toBe( ClientToolCallState.aborted ) @@ -3101,6 +3054,7 @@ describe('copilot tool user action delegation', () => { context: { contextEntityKind: 'workflow', contextEntityId: 'wf-api-request-access-switch', + workspaceId: 'workspace-1', }, }) expect(store.getState().toolCallsById[toolCallId]?.state).toBe(ClientToolCallState.success) diff --git a/apps/tradinggoose/stores/copilot/store.ts b/apps/tradinggoose/stores/copilot/store.ts index b8992db03..d97835d98 100644 --- a/apps/tradinggoose/stores/copilot/store.ts +++ b/apps/tradinggoose/stores/copilot/store.ts @@ -68,6 +68,7 @@ import { bindClientToolExecutionContext, createExecutionContext, ensureClientToolInstance, + handleCopilotServerToolSuccess, isCopilotTool, isServerManagedCopilotTool, prepareCopilotToolArgs, @@ -86,7 +87,6 @@ import { getCopilotWorkspaceSelection, rememberCopilotWorkspaceSelection, } from '@/stores/copilot/workspace-selection' -import { useEnvironmentStore } from '@/stores/settings/environment/store' const logger = createLogger('CopilotStore') @@ -133,9 +133,8 @@ function resolveFinalStreamTurnState( } } - const toolDrivenStatus = resolveTurnStatusFromToolCalls(toolCallsById) - if (toolDrivenStatus !== ACTIVE_TURN_STATUS) { - latestTurnStatus = toolDrivenStatus + if (!hasUiActiveToolCalls(toolCallsById)) { + latestTurnStatus = COMPLETED_TURN_STATUS isAwaitingContinuation = false } } @@ -336,7 +335,7 @@ const initialState = { function buildPlanTodoStateFromMessages(messages: CopilotMessage[]) { const planTodos = buildPlanTodosFromMessages(messages) - return { planTodos, showPlanTodos: planTodos.length > 0 } + return { planTodos, showPlanTodos: planTodos.some((todo) => !todo.completed) } } const sharedSessionSyncGuards = new WeakSet>() @@ -1125,10 +1124,6 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) resetStreamingQueue() const finalContent = getStreamingAssistantContent(context) - const { latestTurnStatus, isAwaitingContinuation } = resolveFinalStreamTurnState( - context, - get().toolCallsById - ) set((state) => ({ messages: state.messages.map((msg) => msg.id === assistantMessageId @@ -1139,7 +1134,6 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) } : msg ), - abortController: latestTurnStatus === ACTIVE_TURN_STATUS ? state.abortController : null, })) if (context.newReviewSessionId && !get().currentChat) { @@ -1151,10 +1145,15 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) await flushPendingAutoExecutionToolCalls(context, get, logger) + const { latestTurnStatus, isAwaitingContinuation } = resolveFinalStreamTurnState( + context, + get().toolCallsById + ) set((state) => ({ ...buildChatTurnStatusState(state, latestTurnStatus), isSendingMessage: latestTurnStatus === ACTIVE_TURN_STATUS, isAwaitingContinuation, + abortController: latestTurnStatus === ACTIVE_TURN_STATUS ? state.abortController : null, })) // Persist full message state (including contentBlocks) to database @@ -1241,7 +1240,8 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) setInputValue: (value: string) => set({ inputValue: value }), // Todo list (UI only) - setPlanTodos: (todos) => set({ planTodos: todos, showPlanTodos: todos.length > 0 }), + setPlanTodos: (todos) => + set({ planTodos: todos, showPlanTodos: todos.some((todo) => !todo.completed) }), updatePlanTodoStatus: (id, status) => { set((state) => { const planTodos = @@ -1253,7 +1253,10 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) ? { ...t, completed: status === 'completed', executing: status === 'executing' } : t ) - return { planTodos: updated } + return { + planTodos: updated, + showPlanTodos: updated.some((todo) => !todo.completed), + } }) }, closePlanTodos: () => set({ showPlanTodos: false }), @@ -1368,11 +1371,30 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) toolName: name, provenance: provenance ?? {}, }) - const preparedArgs = { - ...prepareCopilotToolArgs(name, params, executionContext), - ...(actionArgs || {}), - } const targetStore = getCopilotStore(storeChannelId) + let preparedArgs: Record + + try { + preparedArgs = prepareCopilotToolArgs( + name, + { + ...(params || {}), + ...(actionArgs || {}), + }, + executionContext + ) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + applyToolStateUpdate(targetStore, id, ClientToolCallState.error) + await postCopilotMarkComplete({ + toolCallId: id, + toolName: name || 'unknown_tool', + status: 400, + message, + }).catch(() => {}) + logger.error('Copilot tool argument validation failed', { id, name, error }) + return + } applyToolStateUpdate(targetStore, id, ClientToolCallState.executing) logger.info('[toolCallsById] pending → executing (copilot tool)', { id, name }) @@ -1386,6 +1408,7 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) contextEntityId: provenance.contextEntityId, } : {}), + ...(provenance?.workspaceId ? { workspaceId: provenance.workspaceId } : {}), } const result = await executeCopilotServerTool({ toolName: name, @@ -1410,14 +1433,8 @@ const createCopilotStoreInstance = (storeChannelId = DEFAULT_COPILOT_CHANNEL_ID) logicalSuccess ? ClientToolCallState.success : ClientToolCallState.error ) - if (logicalSuccess && name === 'set_environment_variables') { - try { - await useEnvironmentStore.getState().loadEnvironmentVariables() - } catch (error) { - logger.warn('Failed to refresh environment store after setting variables', { - error, - }) - } + if (logicalSuccess) { + await handleCopilotServerToolSuccess(name) } const completionMessage = diff --git a/apps/tradinggoose/stores/copilot/streaming.ts b/apps/tradinggoose/stores/copilot/streaming.ts index 2fa3db100..9d08bec1c 100644 --- a/apps/tradinggoose/stores/copilot/streaming.ts +++ b/apps/tradinggoose/stores/copilot/streaming.ts @@ -271,12 +271,12 @@ function applyStreamedFunctionCallItem( context.pendingAutoExecutionToolCallIds.add(id) } -function scheduleAutomaticToolExecution( +async function executeAutomaticToolCall( toolCallId: string, toolName: string, get: () => CopilotStore, logger: StreamingLogger -) { +): Promise { try { const { accessLevel } = get() if (shouldRequireToolApproval(accessLevel, isGatedTool(toolName))) { @@ -293,11 +293,9 @@ function scheduleAutomaticToolExecution( id: toolCallId, name: toolName, }) - setTimeout(() => { - void get().executeCopilotToolCall(toolCallId) - }, 0) + await get().executeCopilotToolCall(toolCallId) } catch (error) { - logger.warn('Tool auto-exec check failed', { + logger.warn('Tool auto-exec failed', { id: toolCallId, name: toolName, error, @@ -324,7 +322,7 @@ export async function flushPendingAutoExecutionToolCalls( continue } - scheduleAutomaticToolExecution(toolCallId, toolCall.name, get, logger) + await executeAutomaticToolCall(toolCallId, toolCall.name, get, logger) } } @@ -448,19 +446,6 @@ export function createSSEHandlers(params: { }, }, }) - - if (targetState === ClientToolCallState.success) { - try { - const result = data?.result || data?.data?.result || {} - const input = (current as any).params || (current as any).input || {} - const todoId = input.id || input.todoId || result.id || result.todoId - if (todoId && current.name === 'checkoff_todo') { - get().updatePlanTodoStatus(todoId, 'completed') - } else if (todoId && current.name === 'mark_todo_in_progress') { - get().updatePlanTodoStatus(todoId, 'executing') - } - } catch {} - } } buildStreamedToolDisplayState(toolCallId, targetState, context, result) @@ -609,6 +594,8 @@ export function createSSEHandlers(params: { context.streamComplete = true }, awaiting_tools: (_data, context) => { + context.latestTurnStatus = ACTIVE_TURN_STATUS + context.awaitingTools = true context.streamComplete = true }, stream_end: (_data, context, _get, set) => { diff --git a/apps/tradinggoose/stores/copilot/tool-registry.test.ts b/apps/tradinggoose/stores/copilot/tool-registry.test.ts index 8177f13a8..41248271d 100644 --- a/apps/tradinggoose/stores/copilot/tool-registry.test.ts +++ b/apps/tradinggoose/stores/copilot/tool-registry.test.ts @@ -73,7 +73,7 @@ describe('tool-registry', () => { expect(getToolInterruptDisplays('edit_workflow_block', toolCallId)).toBeDefined() }) - it('does not inject ambient entity context into server tool args from execution provenance', () => { + it('requires explicit target args instead of injecting ambient entity context', () => { const context = createExecutionContext({ toolCallId, toolName: 'read_workflow_logs', @@ -82,7 +82,10 @@ describe('tool-registry', () => { expect(context.contextEntityKind).toBe('workflow') expect(context.contextEntityId).toBe('wf-current') - expect(prepareCopilotToolArgs('read_workflow_logs', {}, context)).toEqual({}) + expect(() => prepareCopilotToolArgs('read_workflow_logs', {}, context)).toThrow() + expect( + prepareCopilotToolArgs('read_workflow_logs', { entityId: 'wf-explicit' }, context) + ).toEqual({ entityId: 'wf-explicit' }) }) it('preserves only explicit server-routed GDrive args', () => { diff --git a/apps/tradinggoose/stores/copilot/tool-registry.ts b/apps/tradinggoose/stores/copilot/tool-registry.ts index 53ca16e94..b94dedd54 100644 --- a/apps/tradinggoose/stores/copilot/tool-registry.ts +++ b/apps/tradinggoose/stores/copilot/tool-registry.ts @@ -1,4 +1,4 @@ -import { CopilotTool, isToolId, type ToolId } from '@/lib/copilot/registry' +import { CopilotTool, isToolId, ToolArgSchemas, type ToolId } from '@/lib/copilot/registry' import type { BaseClientTool, BaseClientToolMetadata, @@ -54,6 +54,7 @@ import { RenameWorkflowClientTool } from '@/lib/copilot/tools/client/workflow/re import { RunWorkflowClientTool } from '@/lib/copilot/tools/client/workflow/run-workflow' import { SetWorkflowVariablesClientTool } from '@/lib/copilot/tools/client/workflow/set-workflow-variables' import { createLogger } from '@/lib/logs/console/logger' +import { useEnvironmentStore } from '@/stores/settings/environment/store' import type { CopilotToolExecutionProvenance } from '@/stores/copilot/types' const logger = createLogger('CopilotToolRegistry') @@ -260,11 +261,28 @@ export function bindClientToolExecutionContext( } export function prepareCopilotToolArgs( - _toolName: string | undefined, + toolName: string | undefined, args: Record | undefined, _context: ClientToolExecutionContext ): Record { - return cloneArgs(args) + const clonedArgs = cloneArgs(args) + if (!toolName || !isToolId(toolName)) { + return clonedArgs + } + + return ToolArgSchemas[toolName].parse(clonedArgs) as Record +} + +export async function handleCopilotServerToolSuccess(toolName: string | undefined): Promise { + if (toolName !== CopilotTool.set_environment_variables) { + return + } + + try { + await useEnvironmentStore.getState().loadEnvironmentVariables() + } catch (error) { + logger.warn('Failed to refresh environment store after setting variables', { error }) + } } export function getToolInterruptDisplays( diff --git a/apps/tradinggoose/stores/copilot/types.ts b/apps/tradinggoose/stores/copilot/types.ts index 15619b87f..c0306d6ca 100644 --- a/apps/tradinggoose/stores/copilot/types.ts +++ b/apps/tradinggoose/stores/copilot/types.ts @@ -30,7 +30,6 @@ export interface CopilotMessage { content: string timestamp: string citations?: { id: number; title: string; url: string; similarity?: number }[] - toolCalls?: CopilotToolCall[] contentBlocks?: Array< | { type: 'text'; content: string; timestamp: number; itemId?: string } | { diff --git a/apps/tradinggoose/widgets/widgets/copilot/components/copilot-message/copilot-message.tsx b/apps/tradinggoose/widgets/widgets/copilot/components/copilot-message/copilot-message.tsx index 6fbefc260..c540ca885 100644 --- a/apps/tradinggoose/widgets/widgets/copilot/components/copilot-message/copilot-message.tsx +++ b/apps/tradinggoose/widgets/widgets/copilot/components/copilot-message/copilot-message.tsx @@ -790,20 +790,6 @@ const CopilotMessage: FC = memo( return false } - // Check if tool calls changed - const prevToolCalls = prevMessage.toolCalls || [] - const nextToolCalls = nextMessage.toolCalls || [] - - if (prevToolCalls.length !== nextToolCalls.length) { - return false // Tool calls count changed - } - - for (let i = 0; i < nextToolCalls.length; i++) { - if (prevToolCalls[i]?.state !== nextToolCalls[i]?.state) { - return false // Tool call state changed - } - } - return true } @@ -811,21 +797,11 @@ const CopilotMessage: FC = memo( if ( prevMessage.content !== nextMessage.content || prevMessage.role !== nextMessage.role || - (prevMessage.toolCalls?.length || 0) !== (nextMessage.toolCalls?.length || 0) || (prevMessage.contentBlocks?.length || 0) !== (nextMessage.contentBlocks?.length || 0) ) { return false } - // Check tool call states for non-streaming messages too - const prevToolCalls = prevMessage.toolCalls || [] - const nextToolCalls = nextMessage.toolCalls || [] - for (let i = 0; i < nextToolCalls.length; i++) { - if (prevToolCalls[i]?.state !== nextToolCalls[i]?.state) { - return false // Tool call state changed - } - } - // Check contentBlocks tool call states const prevContentBlocks = prevMessage.contentBlocks || [] const nextContentBlocks = nextMessage.contentBlocks || [] diff --git a/changelog/June-06-2026.md b/changelog/June-06-2026.md index f0e00cfb0..cbd32160c 100644 --- a/changelog/June-06-2026.md +++ b/changelog/June-06-2026.md @@ -1,86 +1,73 @@ # June-06-2026 -## fix/copilot-tool @ 54e77de3 vs upstream/staging +## fix/copilot-tool @ f44b3416 vs upstream/staging ### Summary -- Standardizes Copilot tool targeting on `entityId` and document payloads on `entityDocument`, replacing workflow-specific argument names across workflow, entity, monitor, credential, Google Drive, and server tool paths. -- Adds shared entity-target and workflow-scope helpers so client and server tools resolve explicit entity targets and workflow-scoped workspace permissions through one contract. -- Persists staged review results for workflow/entity mutation tools, lets full-access mode auto-accept persisted review tools after hydration, and keeps accepted review previews visible in the inline tool UI. -- Consolidates custom tool schema validation and trims runtime manifest enrichment back to lightweight document-format guards instead of embedding workflow graph contracts in the manifest. +- Removes the legacy persisted `toolCalls` message path and makes `contentBlocks` with `type: 'tool_call'` the persisted Copilot tool-call contract. +- Tightens Copilot tool argument handling by validating execution args through `ToolArgSchemas`, requiring canonical todo `id` fields, and reading inline tool params only from `toolCall.params`. +- Propagates `workspaceId` through Copilot tool provenance and server-tool context so workspace-scoped server tools can resolve resources without workflow-only scope. +- Simplifies runtime workflow document validation by removing the manifest-owned `string_document_contract` validator and leaving graph semantics to Studio validation. ### Branch Scope -- Compared `1f0a581d02fca620de367559409272b0f82c47b0..54e77de33f92bcca44ecefb5c3167966d2917e97`, where `1f0a581d02fca620de367559409272b0f82c47b0` is both the merge base and current `upstream/staging`. +- Compared `2777a382ce1dc53b59b1f7cef921fefe7edf2aa2..f44b34169214a385c917f432ef799c8346cca7e0`, where `2777a382ce1dc53b59b1f7cef921fefe7edf2aa2` is the merge base and current `upstream/staging`. - Ran `git fetch upstream staging` before comparing. This entry intentionally uses `upstream/staging`, not the template default `origin/staging`, because the user requested the upstream base. -- `git status --short` was clean before the branch comparison, so no uncommitted feature work was included in the reviewed merge-base diff. During final validation, local unstaged edits were present in `apps/tradinggoose/stores/copilot/store-messages.ts`, `apps/tradinggoose/stores/copilot/store.test.ts`, and `apps/tradinggoose/stores/copilot/store.ts`; those edits were not included as branch-scope evidence for this changelog. -- Main areas touched: Copilot tool schemas and prompt metadata under `apps/tradinggoose/lib/copilot/*`, client/server tool execution under `apps/tradinggoose/lib/copilot/tools/*`, Copilot store hydration/execution under `apps/tradinggoose/stores/copilot/*`, inline tool review UI, custom tool validation/import-export paths, and focused Copilot tool tests. +- `git status --short` was clean before editing this changelog, so no uncommitted feature work was included in the branch evidence. +- Main areas touched: Copilot chat persistence APIs, review-session message mapping, Copilot store hydration/streaming/provenance/tool registry, server-tool workspace context, todo tools, runtime manifest validators, focused Copilot tests, and `packages/db/schema/copilot.ts`. ### Key Changes -- `apps/tradinggoose/lib/copilot/tools/entity-target.ts` introduces the canonical target helpers: `CopilotEntityTargetArgs`, `CopilotEntityExecutionContext`, `resolveOptionalCopilotEntityId()`, `requireCopilotEntityId()`, and `resolveCopilotContextEntityId()`. Client and server tools now use these helpers instead of trimming `workflowId` or ad hoc `entityId` fields locally. -- `apps/tradinggoose/lib/copilot/registry.ts` changes workflow-facing schemas from `workflowId` to `entityId` and from `workflowDocument` to `entityDocument` for `read_workflow`, `edit_workflow`, `edit_workflow_block`, `rename_workflow`, `run_workflow`, workflow variables, deployment, logs, block outputs, and upstream references. Optional scoped tools such as environment variables, OAuth credentials, Google Drive, and monitors now use optional `entityId`. -- `apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts` updates tool descriptions to teach the runtime the new `entityId`/`entityDocument` contract, adds explicit custom tool document guidance, and keeps `kind`, `entityKind`, and `surfaceKind` metadata in `TOOL_PROMPT_METADATA`. -- `apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts` now builds the manifest directly from `ToolArgSchemas`, `TOOL_PROMPT_METADATA`, and `buildAutomaticSemanticValidators()` without loading workflow embedded document validators. `apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts` detects `entityDocument` for `tg-mermaid-v1` and leaves workflow structure validation to Studio instead of putting the full workflow graph contract into the runtime manifest. -- `apps/tradinggoose/lib/copilot/tools/client/base-tool.ts` adds `StagedReviewClientTool`, which stores staged review results through `stageReviewResult()`, reloads persisted review payloads through `getStagedReviewResult()`, exposes interrupt buttons only while state is `review`, and retries `execute()` before accept if a persisted review result is missing. -- Workflow mutation tools now inherit the staged review behavior. `apps/tradinggoose/lib/copilot/tools/client/workflow/edit-workflow.ts` stages server-generated workflow diffs, accepts persisted staged workflow state, writes accepted state through `setWorkflowState()` with `YJS_ORIGINS.COPILOT_REVIEW_ACCEPT`, and sends server payloads with `entityId` and `entityDocument`. `edit-workflow-block.ts` reuses the same base and sends single-block patches as `entityId`, `blockId`, optional `blockType`, `name`, `enabled`, and `subBlocks`. -- Workflow read/execution helpers enforce explicit targets. `read-workflow.ts`, `run-workflow.ts`, `deploy-workflow.ts`, `rename-workflow.ts`, `read-workflow-variables.ts`, `set-workflow-variables.ts`, `read-block-outputs.ts`, `read-block-upstream-references.ts`, and `check-deployment-status.ts` call `requireCopilotEntityId()` before touching workflow state or APIs. -- `apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts` keeps workflow-specific resolution behind `resolveWorkflowTarget()`, `getReadableWorkflowState()`, `buildWorkflowDocumentToolResult()`, and `buildWorkflowSummary()`. `buildWorkflowDocumentToolResult()` now emits the generic saved-entity payload fields `entityKind`, `entityId`, `entityName`, `entityDocument`, and `documentFormat`. -- Server tool context is now entity-scoped. `apps/tradinggoose/lib/copilot/tools/server/base-tool.ts` keeps only `userId`, `contextEntityKind`, `contextEntityId`, and `signal` in `ServerToolExecutionContext`, while `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` owns `createWorkflowPermissionError()` and `resolveServerWorkflowScope()`. -- Server workflow tools consume the generic target contract. `edit-workflow.ts`, `edit-workflow-block.ts`, and `read-workflow-logs.ts` require `entityId`; `apps/tradinggoose/lib/copilot/tools/server/router.ts` validates payloads with `ServerToolArgSchemas`; and `apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts` accepts `contextEntityKind` and `contextEntityId` in the execution envelope. -- Workflow-scoped server tools now reuse `resolveServerWorkflowScope()`: environment variable tools, OAuth/credential readers, Google Drive file readers/listers, and `get-agent-accessory-catalog.ts` all resolve workspace access from explicit `entityId` or workflow context before listing workspace resources. -- `apps/tradinggoose/stores/copilot/types.ts`, `store-provenance.ts`, and `tool-registry.ts` replace the old provenance shape with `CopilotToolExecutionProvenance` fields `contextEntityKind`, `contextEntityId`, and `workspaceId`. `createExecutionContext()` forwards only those fields to client tools and server tool calls. -- `apps/tradinggoose/stores/copilot/store.ts` auto-executes eligible queued tools through `autoExecuteEligibleToolsForAccessLevel()`: access changes may run `pending` and `review` tools, while hydration under full access only resumes `review` tools. The executor hydrates persisted tool calls before user actions and can immediately accept a staged review result when full access moves a tool from executing to review. -- `apps/tradinggoose/stores/copilot/store-messages.ts` adds `buildPlanTodosFromMessages()` so restored chats rebuild plan todos from persisted `plan`, `mark_todo_in_progress`, and `checkoff_todo` tool results. `stores/copilot/streaming.ts` carries tool `result` data into streamed content blocks when completion events include results. -- `apps/tradinggoose/lib/copilot/inline-tool-call.tsx` uses `shouldRequireStagedReviewApproval()` so full access hides review accept/reject buttons, renders workflow and entity preview payloads in both `review` and `success` states, and labels entity diffs as proposed or applied based on tool state. -- `apps/tradinggoose/lib/custom-tools/schema.ts` centralizes `CustomToolOpenAiSchema`, `CustomToolTransferSchema`, `CustomToolUpsertRequestSchema`, `CustomToolTransferRecord`, and `parseCustomToolSchemaText()`. `app/api/tools/custom/route.ts`, `lib/custom-tools/import-export.ts`, and entity document creation now import that schema instead of keeping parallel validators. -- `apps/tradinggoose/i18n/messages/en.json` refreshes waitlist copy and formatting in the branch alongside the Copilot tool work. +- `apps/tradinggoose/app/api/copilot/chat/route.ts` no longer persists assistant `toolCalls` directly. When the runtime response includes tool calls, the route wraps them into `contentBlocks` entries shaped as `{ type: 'tool_call', timestamp, toolCall }` before saving. +- `apps/tradinggoose/app/api/copilot/chat/update-messages/route.ts`, `apps/tradinggoose/lib/copilot/review-sessions/thread-history.ts`, `apps/tradinggoose/lib/copilot/api.ts`, `apps/tradinggoose/stores/copilot/types.ts`, and `packages/db/schema/copilot.ts` remove the message-level `toolCalls` field and the `copilotReviewItems.toolCalls` / `tool_calls` JSONB column from the current schema contract. +- `apps/tradinggoose/stores/copilot/store-messages.ts` now hydrates, pins, updates, validates, and rebuilds plan todos only from `contentBlocks[*].toolCall`. `buildPlanTodosFromMessages()` also normalizes todo state from canonical `id` values instead of accepting `todoId` fallbacks. +- `apps/tradinggoose/lib/copilot/chat-replay-safety.ts`, `apps/tradinggoose/stores/copilot/store-provenance.ts`, and `apps/tradinggoose/widgets/widgets/copilot/components/copilot-message/copilot-message.tsx` now inspect `contentBlocks` for tool state, accepted live mutations, assistant-message lookup, and memo comparisons. +- `apps/tradinggoose/lib/copilot/registry.ts`, `apps/tradinggoose/lib/copilot/tools/client/other/plan.ts`, `checkoff-todo.ts`, and `mark-todo-in-progress.ts` make todo item IDs canonical as `id`. `ToolArgSchemas` and `ToolResultSchemas` require `id` for `checkoff_todo` and `mark_todo_in_progress`, and the client tools return `{ id }` results. +- `apps/tradinggoose/stores/copilot/tool-registry.ts` changes `prepareCopilotToolArgs()` from a shallow clone to `ToolArgSchemas[toolName].parse(...)`, preserving explicit args while rejecting invalid server-routed tool payloads before execution. It also adds `handleCopilotServerToolSuccess()` as the canonical post-success hook for `set_environment_variables`. +- `apps/tradinggoose/stores/copilot/store.ts` merges action args before schema validation, catches validation failures as tool errors with `postCopilotMarkComplete()`, forwards `workspaceId` into `/api/copilot/execute-copilot-server-tool`, resolves final turn status after pending auto-execution flushes, and delegates successful server-tool side effects to `handleCopilotServerToolSuccess()`. +- `apps/tradinggoose/stores/copilot/streaming.ts` replaces deferred `setTimeout` auto-execution with awaited `executeAutomaticToolCall()`, marks `awaiting_tools` as active/awaiting in stream context, carries tool results into streamed content blocks, and removes direct todo-status updates from the SSE `tool_result` handler. +- `apps/tradinggoose/app/api/copilot/execute-copilot-server-tool/route.ts`, `apps/tradinggoose/lib/copilot/tools/client/server-tool-response.ts`, and `apps/tradinggoose/lib/copilot/tools/server/base-tool.ts` add `workspaceId` to the Copilot server-tool execution envelope. +- `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` adds `resolveServerWorkspaceId(context, workflowScope)` as the shared fallback from verified workflow scope to explicit context workspace. `get-agent-accessory-catalog.ts`, Google Drive tools, credential/OAuth/environment readers, and `tools/server/knowledge/knowledge-base.ts` use it or the context workspace when resolving workspace-scoped resources. +- `apps/tradinggoose/lib/copilot/tools/client/knowledge/knowledge-base.ts` fills missing create/list `args.workspaceId` from the client execution context and forwards the same `workspaceId` to the server-tool context. +- `apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts` removes the `string_document_contract` semantic validator for workflow documents while keeping raw Mermaid newline, `flowchart`, prefix, JSON schema, and forbidden-substring guards. +- `apps/tradinggoose/lib/copilot/inline-tool-call.tsx` reads display params from `toolCall.params ?? {}` only, removing fallback reads from legacy `parameters` or `input` shapes. ### Design Decisions -- The durable Copilot argument contract is now entity-oriented. Tool schemas, prompt metadata, client tools, server tools, and persisted results should speak `entityId` and `entityDocument`; workflow-specific IDs are internal implementation details after the target has been resolved. -- Explicit mutation/execution targets are required even when the chat has a live workflow context. Tests such as `apps/tradinggoose/lib/copilot/tools/client/workflow/run-workflow.test.ts` and `edit-workflow.test.ts` verify that `entityId` is required instead of falling back to `contextEntityId`. -- Workflow context is still useful for scoped reads and workspace permission checks. `resolveServerWorkflowScope()` can fall back from optional `entityId` to `contextEntityKind: 'workflow'` plus `contextEntityId` for server tools that need workspace-scoped credentials, environment variables, Google Drive access, or agent accessory catalogs. -- Staged review ownership moved into `StagedReviewClientTool` so workflow and saved-entity mutation tools share one persisted review-result lifecycle instead of each storing local `lastResult` or custom interrupt behavior. -- Runtime manifest validation is intentionally lighter for workflow documents. The manifest still guards raw Mermaid text and the document format, but full TG_WORKFLOW/TG_BLOCK/TG_EDGE consistency remains owned by Studio parsing and workflow mutation validation. -- Custom tool OpenAI function schema validation is shared between import/export, API upserts, and Copilot entity creation through `apps/tradinggoose/lib/custom-tools/schema.ts` so new paths do not drift on parameter shape or normalization. -- Generic Copilot context usage in `stores/copilot/store.ts` is conversation/user scoped. The branch stops forwarding workflow IDs to usage billing from the generic widget path because workflow context is prompt context, not the billing scope selector for this widget. +- Persisted Copilot messages have one canonical tool-call representation: `contentBlocks` containing `tool_call` blocks. The branch intentionally removes the parallel `message.toolCalls` path instead of keeping dual write/read compatibility. +- Copilot tool args are schema-validated at execution time through `ToolArgSchemas`; tool implementations should not recover legacy aliases after validation rejects them. +- Todo tools now use `id` as the only item identifier. `todoId` is treated as obsolete at the schema, client-tool, result, and plan-rehydration layers. +- Workspace context is part of Copilot execution provenance. Server tools that need workspace-scoped resources should receive `context.workspaceId` or derive it through a verified workflow scope, then call `resolveServerWorkspaceId()`. +- Stream finalization waits for eligible automatic tool execution before computing the final turn state, so a tool that can run immediately does not leave the chat stuck in an awaiting-continuation state. +- Runtime manifest validation stays cheap and format-oriented for workflow documents; canonical workflow graph consistency remains owned by Studio parsing and mutation validation. ### Shared Contracts and Helpers to Reuse -- Reuse `resolveOptionalCopilotEntityId()`, `requireCopilotEntityId()`, and `resolveCopilotContextEntityId()` from `apps/tradinggoose/lib/copilot/tools/entity-target.ts` for all new Copilot tools that accept or derive entity targets. -- Reuse `StagedReviewClientTool` from `apps/tradinggoose/lib/copilot/tools/client/base-tool.ts` for any client tool that creates an accept/reject review result. Implement `hasStagedReviewResult()` and call `stageReviewResult()` instead of adding tool-local review state. -- Reuse `resolveWorkflowTarget()`, `getReadableWorkflowState()`, `buildWorkflowDocumentToolResult()`, and `buildWorkflowSummary()` from `apps/tradinggoose/lib/copilot/tools/client/workflow/workflow-review-tool-utils.ts` for workflow document reads, previews, and mutations. -- Reuse `createWorkflowPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` for server tools that may need workflow-scoped workspace permission checks. -- Reuse `ToolArgSchemas`, `ServerToolArgSchemas`, and `ToolSSESchemas` from `apps/tradinggoose/lib/copilot/registry.ts` as the canonical tool argument surface. Do not introduce a parallel schema that accepts `workflowId` for the same Copilot tool. -- Reuse `TOOL_PROMPT_METADATA` from `apps/tradinggoose/lib/copilot/tool-prompt-metadata.ts` for manifest-visible descriptions, `kind`, `entityKind`, and `surfaceKind` metadata. -- Reuse `shouldAutoExecuteTool()`, `shouldRequireToolApproval()`, and `shouldRequireStagedReviewApproval()` from `apps/tradinggoose/lib/copilot/access-policy.ts` when deciding whether a Copilot tool should show approval controls or execute automatically. -- Reuse `buildTurnProvenanceFromContexts()`, `withPinnedToolExecutionProvenance()`, and `findAssistantMessageIdForToolCall()` from `apps/tradinggoose/stores/copilot/store-provenance.ts` when adding store paths that create, hydrate, or resume tool calls. -- Reuse `buildPlanTodosFromMessages()` from `apps/tradinggoose/stores/copilot/store-messages.ts` when restoring plan UI state from persisted Copilot messages. -- Reuse `CustomToolOpenAiSchema`, `CustomToolTransferSchema`, `CustomToolUpsertRequestSchema`, and `parseCustomToolSchemaText()` from `apps/tradinggoose/lib/custom-tools/schema.ts` for all custom-tool API, import/export, and Copilot document paths. +- Use `contentBlocks` entries shaped as `{ type: 'tool_call', toolCall: CopilotToolCall, timestamp }` as the canonical persisted assistant tool-call format. Reuse the mapping in `apps/tradinggoose/app/api/copilot/chat/route.ts` and the readers in `apps/tradinggoose/stores/copilot/store-messages.ts`. +- Reuse `normalizeMessagesForUI()`, `buildPinnedToolCallsById()`, `buildPlanTodosFromMessages()`, `updateMessagesForToolCallState()`, and `validateMessagesForLLM()` from `apps/tradinggoose/stores/copilot/store-messages.ts` for message hydration, tool-call indexing, plan restoration, and persistence preparation. +- Reuse `ToolArgSchemas` and `ToolResultSchemas` from `apps/tradinggoose/lib/copilot/registry.ts`; route all execution argument preparation through `prepareCopilotToolArgs()` in `apps/tradinggoose/stores/copilot/tool-registry.ts`. +- Reuse `handleCopilotServerToolSuccess()` from `apps/tradinggoose/stores/copilot/tool-registry.ts` when adding post-success server-tool side effects instead of embedding one-off branches in `store.ts`. +- Reuse `ServerToolExecutionContext.workspaceId`, `executeCopilotServerTool({ context: { workspaceId } })`, and `/api/copilot/execute-copilot-server-tool` context parsing for workspace-scoped server tools. +- Reuse `resolveServerWorkspaceId()` from `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` whenever a server tool can use either explicit workspace context or verified workflow scope. +- Reuse `buildTurnProvenanceFromContexts()`, `withPinnedToolExecutionProvenance()`, and `findAssistantMessageIdForToolCall()` from `apps/tradinggoose/stores/copilot/store-provenance.ts` for store paths that create, hydrate, or locate tool calls. ### Removed or Replaced Items -- Replaced Copilot tool arguments named `workflowId` with `entityId` for workflow read, edit, run, deploy, logs, variables, monitor list filters, block output reads, server credentials, and Google Drive scoped tools. Do not reintroduce `workflowId` as the Copilot-facing argument for these tools. -- Replaced `workflowDocument` with `entityDocument` for `edit_workflow` and related manifest validation. Future document mutation tools should use `entityDocument` plus `documentFormat`. -- Removed workflow-specific fields from `ClientToolExecutionContext` and `CopilotToolExecutionProvenance`, including `workflowId`, `contextWorkflowId`, `reviewSessionId`, `entityKind`, `entityId`, and `draftSessionId`. Use `contextEntityKind`, `contextEntityId`, and `workspaceId` instead. -- Removed `createPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/base-tool.ts`. Import `createWorkflowPermissionError()` and `resolveServerWorkflowScope()` from `apps/tradinggoose/lib/copilot/tools/server/workflow/workflow-scope.ts` instead. -- Removed `WORKFLOW_GRAPH_CONTRACT_SPEC`, `buildWorkflowDocumentContract()`, and `WORKFLOW_DOCUMENT_CONTRACT` from `apps/tradinggoose/lib/copilot/runtime-tool-manifest-enrichment.ts`. Do not restore manifest-owned workflow graph validation; use Studio workflow parsing and mutation validation. -- Removed the dynamic import of `buildWorkflowEmbeddedDocumentValidators()` from `apps/tradinggoose/lib/copilot/runtime-tool-manifest.ts`. Manifest generation should stay synchronous over `ToolArgSchemas`, `TOOL_PROMPT_METADATA`, and automatic semantic validators. -- Removed local custom tool schema definitions from `apps/tradinggoose/app/api/tools/custom/route.ts` and `apps/tradinggoose/lib/custom-tools/import-export.ts`. Use `apps/tradinggoose/lib/custom-tools/schema.ts`. -- Removed local `parseCustomToolSchema()` from `apps/tradinggoose/lib/copilot/tools/client/entities/entity-document-tool-utils.ts`. Use `parseCustomToolSchemaText()`. -- Replaced entity mutation tool-local `lastResult` and custom review interrupt handling in `entity-document-tools.ts` with `StagedReviewClientTool`. New mutation tools should not recreate local staged result storage. -- Removed workflow ID forwarding from generic Copilot context-usage requests in `stores/copilot/store.ts`. Do not re-add workflow IDs to generic usage billing unless the usage API contract intentionally changes. +- Removed `copilotReviewItems.toolCalls` / `tool_calls` from `packages/db/schema/copilot.ts`. Do not reintroduce a message-level tool-call column; use `content_blocks` with `tool_call` blocks. +- Removed `toolCalls` from `CopilotMessage`, `ReviewMessageApi`, `ReviewMessageInput`, `UpdateMessagesSchema`, `mapReviewItemToApi()`, `buildReviewItemInsert()`, and update-message persistence. Use `contentBlocks` instead. +- Removed `toolCalls` hydration, pinning, LLM validation, replay-safety scanning, and widget memo checks from the store/UI paths. The replacement is `contentBlocks[*].toolCall`. +- Removed `todoId` as an accepted todo argument/result alias in `plan`, `checkoff_todo`, and `mark_todo_in_progress` flows. Use `id`. +- Removed inline display fallback to `toolCall.parameters` and `toolCall.input` in `apps/tradinggoose/lib/copilot/inline-tool-call.tsx`. Persist and read tool args as `toolCall.params`. +- Removed `string_document_contract` from workflow runtime manifest validators. Do not restore manifest-owned visible-edge versus `TG_EDGE` graph validation; keep that responsibility in Studio workflow validation. +- Removed timer-based `scheduleAutomaticToolExecution()` and direct SSE todo-status updates from `apps/tradinggoose/stores/copilot/streaming.ts`. Use awaited auto-execution and the todo client tools' own store updates. +- Replaced the `set_environment_variables` success special case in `apps/tradinggoose/stores/copilot/store.ts` with `handleCopilotServerToolSuccess()`. ### Future Branch Guardrails -- Keep Copilot tool schemas, prompts, tests, and handlers aligned on `entityId` and `entityDocument`. If a UI or database API still needs `workflowId`, convert after the Copilot boundary by calling the canonical target helpers. -- Do not make workflow mutations or executions infer the target from live workflow context. Require explicit `entityId` for destructive or side-effecting workflow tools. -- Do not add workflow-scope helpers back to generic server `base-tool.ts`. Generic server context should remain entity-kind/entity-id aware, and workflow permission resolution should stay in `tools/server/workflow/workflow-scope.ts`. -- Do not duplicate staged review state in individual tools. Use `StagedReviewClientTool` so review payloads survive persistence/hydration and full-access auto-approval behaves consistently. -- Do not restore full workflow graph contract validators to runtime manifest enrichment. Keep manifest validators cheap and let workflow mutation code validate canonical TG Mermaid state. -- Do not define new custom tool OpenAI-function schemas outside `apps/tradinggoose/lib/custom-tools/schema.ts`. -- When adding server-managed tools that need workspace-scoped credentials or resources, pass `contextEntityKind` and `contextEntityId` through `/api/copilot/execute-copilot-server-tool` and route through `resolveServerWorkflowScope()`. -- Keep accepted review previews renderable from persisted `toolCall.result`; inline UI should not depend on in-memory tool instances to show applied workflow or entity diffs. +- Do not add new code that writes or reads persisted assistant tool calls from `message.toolCalls` or `tool_calls`; all persistence and UI state should flow through `contentBlocks`. +- Do not add legacy `todoId`, `parameters`, or `input` compatibility paths for Copilot tool execution. Keep `id` and `params` as the canonical names and let `ToolArgSchemas` reject invalid shapes. +- Do not inject ambient entity or workspace data directly into tool args. Preserve explicit tool payloads, put workspace/entity context in provenance, and let server tools use `context.workspaceId` or `resolveServerWorkspaceId()`. +- Do not add post-success server-tool side effects inline in `executeCopilotToolCall()`. Add them behind `handleCopilotServerToolSuccess()` so execution flow stays centralized. +- Do not move workflow graph semantic validation back into the runtime manifest. Manifest enrichment should provide lightweight document-format guards only. +- When adding workspace-scoped Copilot server tools, update the client execution context, server-tool request envelope, and `ServerToolExecutionContext` together so persisted tool calls can be resumed with the same workspace context. ### Validation Notes - Used the requested `staging-changelog` skill and followed `changelog/TEMPLATE.md`, with the explicit base override from `origin/staging` to `upstream/staging`. -- Reviewed `git status --short`, `git remote -v`, `git branch --show-current`, `git fetch upstream staging`, `git rev-parse fix/copilot-tool`, `git rev-parse upstream/staging`, `git merge-base upstream/staging fix/copilot-tool`, `git log --oneline 1f0a581d02fca620de367559409272b0f82c47b0..fix/copilot-tool`, `git diff --stat`, `git diff --name-status --find-renames`, `git diff --summary`, and targeted `git diff` output for the branch range. -- Inspected the repo instructions, `changelog/TEMPLATE.md`, existing changelog layout, new shared files `tools/entity-target.ts`, `tools/server/workflow/workflow-scope.ts`, and `custom-tools/schema.ts`, the registry/manifest/prompt metadata paths, workflow client and server tools, credential/environment/Google Drive server tools, Copilot store/provenance/streaming/tool-registry paths, inline tool UI, custom tool API/import-export code, and old merge-base contents for replaced helpers. -- Reviewed focused tests touched by the branch, including Copilot store/provenance/tool-registry tests, runtime manifest tests, inline tool call tests, workflow client/server tool tests, Google Drive server tool tests, entity/monitor tool tests, and server router tests. -- Confirmed the branch delta adds no generated migration files and deletes no files. -- Final `git status --short` showed the new `changelog/June-06-2026.md` file plus preexisting or concurrent unstaged edits in `apps/tradinggoose/stores/copilot/store-messages.ts`, `apps/tradinggoose/stores/copilot/store.test.ts`, and `apps/tradinggoose/stores/copilot/store.ts`. This changelog run did not edit those store files. -- No automated test suite was run for this changelog-only update. Validation focused on merge-base diff review, source inspection, old-content inspection for replaced symbols, and template conformance. +- Reviewed `git status --short --branch`, `git remote -v`, `git fetch upstream staging`, `git rev-parse fix/copilot-tool`, `git rev-parse upstream/staging`, `git merge-base upstream/staging fix/copilot-tool`, `git log --oneline 2777a382ce1dc53b59b1f7cef921fefe7edf2aa2..fix/copilot-tool`, `git diff --stat`, `git diff --name-status --find-renames`, `git diff --summary`, `git diff --dirstat`, and targeted `git diff` output for the branch range. +- Inspected the repository instructions, `changelog/TEMPLATE.md`, existing `changelog/June-06-2026.md`, Copilot chat persistence routes, review-session mapping, DB schema, store message/provenance/streaming/tool-registry paths, server-tool workflow scope and workspace consumers, todo tools, inline tool UI, runtime manifest enrichment, and related Copilot widget/message types. +- Reviewed focused test changes in `review-session-post.test.ts`, `update-messages/route.test.ts`, `store.test.ts`, `store-provenance.test.ts`, `tool-registry.test.ts`, `runtime-tool-manifest.test.ts`, `chat-replay-safety.test.ts`, and `get-agent-accessory-catalog.test.ts`. +- Confirmed the branch delta contains no renamed or deleted files and no generated migration-file edits. +- No automated test suite was run for this changelog-only update. Validation focused on merge-base diff review, related source/test inspection, and template conformance. diff --git a/packages/db/schema/copilot.ts b/packages/db/schema/copilot.ts index fdf14b634..930217168 100644 --- a/packages/db/schema/copilot.ts +++ b/packages/db/schema/copilot.ts @@ -107,7 +107,6 @@ export const copilotReviewItems = pgTable( messageRole: text('message_role').notNull(), content: text('content').notNull().default(''), timestamp: text('timestamp').notNull(), - toolCalls: jsonb('tool_calls').notNull().default(sql`'[]'::jsonb`), contentBlocks: jsonb('content_blocks').notNull().default(sql`'[]'::jsonb`), contexts: jsonb('contexts').notNull().default(sql`'[]'::jsonb`), fileAttachments: jsonb('file_attachments').notNull().default(sql`'[]'::jsonb`),