Repository navigation
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.
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
|
@greptile |
|
@cursor review |
|
@greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
This branch was previously deployed
1 participant
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