fix(tools): bind schema-enrichment reads to executor delegations - #6593
Conversation
Two schema-enrichment callers still sent the deprecated legacy internal JWT to routes that moved onto delegation-only auth, so both 401'd and swallowed the failure: - tools/params.ts fetched a child workflow's input fields with an unsubjected buildAuthHeaders(), leaving the Agent block's workflow_executor inputMapping untyped so the model guessed the child's field names. - tools/schema-enrichers.ts fetched KB tag definitions the same way, which dropped the tags/tagFilters parameter from the knowledge tools entirely. Extract the executionId binding rule into executionScopeForTarget so the three enrichment call sites share one definition.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Extracts shared Reviewed by Cursor Bugbot for commit a939738. Configure here. |
Enrichment degrades silently by design, so the log line is the only signal a credential break leaves. Drain the body on the failure path too.
Greptile SummaryThe PR replaces legacy internal authentication on workflow and knowledge schema-enrichment reads with executor delegations bound to the acting subject. It also centralizes execution-scope selection and adds regression coverage for same-workflow, child-workflow, missing-context, and rejected-read behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/utils/delegation.ts | Introduces a focused helper that prevents a parent run ID from being asserted against a child-workflow delegation. |
| apps/sim/providers/utils.ts | Reuses the centralized execution-scope rule when fetching workflow metadata. |
| apps/sim/tools/params.ts | Uses the acting execution subject for delegated workflow reads and enriches workflow-executor input mappings with child input fields. |
| apps/sim/tools/schema-enrichers.ts | Migrates knowledge tag-definition enrichment to workflow-scoped executor delegation while retaining graceful fallback behavior. |
| apps/sim/executor/utils/delegation.test.ts | Covers same-workflow, child-workflow, inactive-run, and missing-workflow delegation scopes. |
| apps/sim/tools/params.test.ts | Verifies delegation claims and workflow input schema enrichment, including missing-subject and authorization-failure fallbacks. |
| apps/sim/tools/schema-enrichers.test.ts | Verifies knowledge tag enrichment uses the acting subject and appropriate workflow execution binding. |
Sequence Diagram
sequenceDiagram
participant Exec as Workflow Executor
participant Tool as Tool Schema Enricher
participant Auth as Delegation Builder
participant API as Internal API
Exec->>Tool: Build schema with execution context
Tool->>Auth: subjectUserId + target workflow
alt Target is running workflow
Tool->>Auth: Include executionId
else Target is child workflow
Tool->>Auth: Omit parent executionId
end
Auth-->>Tool: Scoped delegation headers
Tool->>API: Read workflow or tag definitions
API-->>Tool: Authorized enrichment data
Tool-->>Exec: Enriched tool schema
Reviews (2): Last reviewed commit: "improvement(tools): surface tag-definiti..." | Re-trigger Greptile
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a939738. Configure here.
Summary
tools/params.tsstill called the deprecatedbuildAuthHeaders()(no subject) to read a child workflow, so/api/workflows/[id]— now oninternalWorkflowReadAuth— rejected the legacytype: 'internal'token with a 401. The Agent block'sworkflow_executorinputMappingwas left as an untyped{ type: 'object' }, so the model guessed the child's field names andlazyCleanupInputMappingsilently dropped the wrong keys.tools/schema-enrichers.tshad the same break against/api/knowledge/[id]/tag-definitions. Worse blast radius: when an enricher returns null the parameter is dropped from the schema entirely, soknowledge_search/create_document/upsert_documentlost tag filtering altogether.buildExecutorDelegationHeaders, bound to the acting execution subject.executionIdbinding rule intoexecutor/utils/delegation.ts— it was hand-rolled in three places. A child workflow binds on its own id, so forwarding the parent'sexecutionIdwould assert a run that doesn't cover it.Regression from #5273, which migrated the routes and the sibling call sites but missed these two.
git diff origin/main -- apps/sim/tools/params.tswas empty. Audited every remainingbuildAuthHeaderscaller — the rest target routes still on legacy auth, so no other mismatch remains.Type of Change
Testing
properties/requiredschema. Verified they fail against the pre-fix code (2 failures inparams.test.ts, 3 inschema-enrichers.test.ts) — the existinginputMapping always includedtest sat on this path but asserted onlytype === 'object', which holds in the broken state, so it could not fail.bunx vitest run tools/ providers/ executor/— 5,981 passing.lint:checkandtype-check(23/23 each), pluscheck:api-validation, monorepo-boundary and realtime-prune gates.Checklist