[feat] Reference a workflow as an agent tool (type:"reference")#4860
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:XL
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a ChangesWorkflow Reference Tools
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Review ask (for JP / CTO review — do not merge): this is the reference half of the lgtm'd embedref-tools two-syntax design (#4837): an agent tool can now point at a workflow via
No wire change (golden fixtures untouched), no new runner kind. Live end-to-end QA is deferred to the dedicated embedref live QA after the gate. The FE client-tool execution path is #4850 (composes with this). |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== PR metadata =="
gh pr view 4860 --json number,title,headRefName,baseRefName,files --jq '{number,title,head:.headRefName,base:.baseRefName,files:[.files[].path]}'
echo
echo "== Changed files matching likely review areas =="
gh pr diff 4860 --name-only | rg -n 'ResolverMiddleware|compat|tools|workflow|embed|skill_config|callback|router|service|models|parsing|utils' || true
echo
echo "== Full changed file list =="
gh pr diff 4860 --name-only
echo
echo "== Locate candidate files in checkout =="
fd -i 'utils.py|compat.py|router.py|service.py|resolver.py|test_.*\.py|models.py|parsing.py' api sdks/python || true
echo
echo "== Outline: api/oss/src/oss/api/tools/router.py =="
ast-grep outline api/oss/src/oss/api/tools/router.py --view expanded || true
echo
echo "== Outline: api/oss/src/oss/core/embeds/utils.py =="
ast-grep outline api/oss/src/oss/core/embeds/utils.py --view expanded || true
echo
echo "== Outline: sdks/python/oss/src/agenta_sdk/agents/compat.py =="
ast-grep outline sdks/python/oss/src/agenta_sdk/agents/compat.py --view expanded || true
echo
echo "== Outline: sdks/python/oss/src/agenta_sdk/agents/tools/resolver.py =="
ast-grep outline sdks/python/oss/src/agenta_sdk/agents/tools/resolver.py --view expanded || true
echo
echo "== Outline: sdks/python/oss/src/agenta_sdk/agents/platform/workflow_resolver.py =="
ast-grep outline sdks/python/oss/src/agenta_sdk/agents/platform/workflow_resolver.py --view expanded || trueLength of output: 32094 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Outline: sdk resolver middleware =="
ast-grep outline sdks/python/agenta/sdk/middlewares/running/resolver.py --view expanded || true
echo
echo "== Outline: sdk tools compat =="
ast-grep outline sdks/python/agenta/sdk/agents/tools/compat.py --view expanded || true
echo
echo "== Outline: sdk tools models =="
ast-grep outline sdks/python/agenta/sdk/agents/tools/models.py --view expanded || true
echo
echo "== Outline: sdk tools resolver =="
ast-grep outline sdks/python/agenta/sdk/agents/tools/resolver.py --view expanded || true
echo
echo "== Outline: sdk platform workflow =="
ast-grep outline sdks/python/agenta/sdk/agents/platform/workflow.py --view expanded || true
echo
echo "== Outline: sdk platform resolve =="
ast-grep outline sdks/python/agenta/sdk/agents/platform/resolve.py --view expanded || true
echo
echo "== Outline: api tools router =="
ast-grep outline api/oss/src/apis/fastapi/tools/router.py --view expanded || true
echo
echo "== Outline: api core embeds utils =="
ast-grep outline api/oss/src/core/embeds/utils.py --view expanded || true
echo
echo "== Outline: api core tools service =="
ast-grep outline api/oss/src/core/tools/service.py --view expanded || true
echo
echo "== Outline: api core workflows service =="
ast-grep outline api/oss/src/core/workflows/service.py --view expanded || true
echo
echo "== Search relevant symbols =="
rg -n -C2 '\bAG_REFERENCE_KEY\b|\b_has_embed_markers\b|\bresolve_tools\b|\bReferenceToolConfig\b|\bWorkflowToolResolver\b|\bAgentaWorkflowToolResolver\b|\bcall_ref\b|\b_call_workflow_tool\b|\binvoke_workflow\b|\bToolCallback\b' \
api sdks/python
echo
echo "== PR diff hunks for key files =="
for f in \
api/entrypoints/routers.py \
api/oss/src/apis/fastapi/tools/router.py \
api/oss/src/core/embeds/utils.py \
sdks/python/agenta/sdk/agents/platform/resolve.py \
sdks/python/agenta/sdk/agents/platform/workflow.py \
sdks/python/agenta/sdk/agents/tools/compat.py \
sdks/python/agenta/sdk/agents/tools/interfaces.py \
sdks/python/agenta/sdk/agents/tools/models.py \
sdks/python/agenta/sdk/agents/tools/resolver.py \
sdks/python/agenta/sdk/utils/types.py \
api/oss/tests/pytest/unit/tools/test_workflow_tool_call.py \
api/oss/tests/pytest/unit/embeds/test_utils.py \
sdks/python/oss/tests/pytest/unit/agents/platform/test_workflow_resolver.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_models.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_parsing.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py \
sdks/python/oss/tests/pytest/unit/test_skill_config_catalog.py
do
echo
echo "---- $f ----"
git diff --unified=80 origin/big-agents...HEAD -- "$f" || true
doneLength of output: 113974 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
local file="$1"
local start="$2"
local end="$3"
echo
echo "==== ${file}:${start}-${end} ===="
sed -n "${start},${end}p" "$file" || true
}
echo "== Fallback file diffs =="
for f in \
api/entrypoints/routers.py \
api/oss/src/apis/fastapi/tools/router.py \
api/oss/src/core/embeds/utils.py \
sdks/python/agenta/sdk/agents/platform/resolve.py \
sdks/python/agenta/sdk/agents/platform/workflow.py \
sdks/python/agenta/sdk/agents/tools/compat.py \
sdks/python/agenta/sdk/agents/tools/interfaces.py \
sdks/python/agenta/sdk/agents/tools/models.py \
sdks/python/agenta/sdk/agents/tools/resolver.py \
sdks/python/agenta/sdk/utils/types.py \
api/oss/tests/pytest/unit/tools/test_workflow_tool_call.py \
api/oss/tests/pytest/unit/embeds/test_utils.py \
sdks/python/oss/tests/pytest/unit/agents/platform/test_workflow_resolver.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_models.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_parsing.py \
sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py \
sdks/python/oss/tests/pytest/unit/test_skill_config_catalog.py
do
echo
echo "---- git diff -- $f ----"
git diff -- "$f" || true
done
show sdks/python/agenta/sdk/middlewares/running/resolver.py 102 126
show api/oss/src/core/embeds/utils.py 37 49
show api/oss/src/core/embeds/utils.py 1062 1078
show api/oss/src/core/embeds/utils.py 1164 1176
show api/oss/src/core/embeds/utils.py 1632 1644
show sdks/python/agenta/sdk/agents/tools/compat.py 72 140
show sdks/python/agenta/sdk/agents/tools/models.py 85 126
show sdks/python/agenta/sdk/agents/tools/interfaces.py 1 60
show sdks/python/agenta/sdk/agents/tools/resolver.py 91 240
show sdks/python/agenta/sdk/agents/platform/resolve.py 56 74
show sdks/python/agenta/sdk/agents/platform/workflow.py 37 90
show api/oss/src/apis/fastapi/tools/router.py 111 130
show api/oss/src/apis/fastapi/tools/router.py 965 1078
show api/oss/src/apis/fastapi/tools/router.py 1078 1168
show api/entrypoints/routers.py 1 220
show api/oss/tests/pytest/unit/tools/test_workflow_tool_call.py 1 260
show api/oss/tests/pytest/unit/embeds/test_utils.py 150 240
show sdks/python/oss/tests/pytest/unit/agents/platform/test_workflow_resolver.py 1 220
show sdks/python/oss/tests/pytest/unit/agents/tools/test_models.py 1 140
show sdks/python/oss/tests/pytest/unit/agents/tools/test_parsing.py 140 240
show sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py 1 320
show sdks/python/oss/tests/pytest/unit/test_skill_config_catalog.py 1 220Length of output: 74111
So: architecturally this looks consistent with the two-syntax design, and the split between generic embed resolution vs tool-specific reference resolution looks right to me. No merge-blocking concerns from my side. (•ㅅ•)💭 ✅ Action performedReview finished.
|
Railway Preview Environment
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
api/oss/src/core/embeds/utils.py (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim this comment to the invariant only.
This block explains the what in detail; the guideline here asks for one terse
whycomment instead.Proposed simplification
-# The reference-syntax marker (sibling of `@ag.embed`). The generic resolver is tool-agnostic and -# only ever INLINES `@ag.embed`; it must LEAVE an `@ag.reference` in place so resolve_tools can build -# the callback spec from the kept reference. A node carrying `@ag.reference` is opaque to the -# resolver: it is not an embed, and the walker does not descend into it (so a nested `@ag.embed` -# inside a kept reference is not accidentally inlined). See the embedref-tools design. +# Kept `@ag.reference` payloads stay opaque so tool resolution can handle them later.As per coding guidelines, "Keep AI-generated in-code comments minimal; comment only the non-obvious why ... Prefer one terse line over multi-line blocks."
Source: Coding guidelines
sdks/python/agenta/sdk/agents/tools/resolver.py (1)
185-192: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert callback equivalence before collapsing workflow and gateway results.
WorkflowToolResolverandGatewayToolResolverare injected independently, but this branch silently keeps the gateway callback whenever both are present. That works for the current Agenta adapters, yet a custom resolver pair with different endpoint/auth values would leave one set of specs posting to the wrong callback. Please fail fast if the two callbacks differ instead of discarding one.api/oss/tests/pytest/unit/embeds/test_utils.py (1)
161-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim these comments down to the invariant only.
The test names already describe the behavior. These blocks mostly restate what the code is doing, which adds noise in
api/**/*.pyinstead of documenting a non-obvious why. As per coding guidelines, "Keep AI-generated in-code comments minimal; comment only the non-obvious why (a subtle invariant, a workaround, a pool/concurrency hazard), never the what."Also applies to: 183-184, 213-214
Source: Coding guidelines
api/oss/tests/pytest/unit/tools/test_workflow_tool_call.py (1)
50-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake these test helpers keyword-only.
These are new
api/**/*.pyhelper signatures, but they still take positional arguments. Using keyword-only parameters keeps the test API aligned with the repo convention and makes future call-site edits safer. As per coding guidelines, "Prefer keyword-only parameters using*in function signatures."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4629eeee-3fba-4274-82fb-31e07960b769
📒 Files selected for processing (24)
api/entrypoints/routers.pyapi/oss/src/apis/fastapi/tools/router.pyapi/oss/src/core/embeds/utils.pyapi/oss/tests/pytest/unit/embeds/test_utils.pyapi/oss/tests/pytest/unit/tools/test_workflow_tool_call.pydocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.mddocs/design/agent-workflows/interfaces/in-service/tool-models-and-resolution.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mdsdks/python/agenta/sdk/agents/__init__.pysdks/python/agenta/sdk/agents/platform/__init__.pysdks/python/agenta/sdk/agents/platform/resolve.pysdks/python/agenta/sdk/agents/platform/workflow.pysdks/python/agenta/sdk/agents/tools/__init__.pysdks/python/agenta/sdk/agents/tools/compat.pysdks/python/agenta/sdk/agents/tools/interfaces.pysdks/python/agenta/sdk/agents/tools/models.pysdks/python/agenta/sdk/agents/tools/resolver.pysdks/python/agenta/sdk/utils/types.pysdks/python/oss/tests/pytest/unit/agents/platform/test_workflow_resolver.pysdks/python/oss/tests/pytest/unit/agents/tools/test_models.pysdks/python/oss/tests/pytest/unit/agents/tools/test_parsing.pysdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.pysdks/python/oss/tests/pytest/unit/test_skill_config_catalog.py
| if AG_REFERENCE_KEY in config and AG_EMBED_KEY not in config: | ||
| # A kept @ag.reference node is opaque to the generic resolver: leave it in place and | ||
| # do not descend (the reference-syntax "leave it" branch). resolve_tools handles it. | ||
| return embeds |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't short-circuit sibling fields on reference-tool objects.
These returns make the entire dict opaque as soon as @ag.reference is present. That also skips valid embeds in sibling fields like name, description, input_schema, or render, even though the contract keeps those beside the reference marker. Skip only the @ag.reference value itself and keep traversing the other keys.
Suggested direction
- if AG_REFERENCE_KEY in config and AG_EMBED_KEY not in config:
- return embeds
+ child_items = config.items()
+ if AG_REFERENCE_KEY in config and AG_EMBED_KEY not in config:
+ child_items = (
+ (key, value)
+ for key, value in config.items()
+ if key != AG_REFERENCE_KEY
+ )
if AG_EMBED_KEY in config:
...
else:
- for key, value in config.items():
+ for key, value in child_items:
...Apply the same pattern in find_string_embeds and find_snippet_embeds.
Also applies to: 1170-1172, 1638-1640
| return await ToolResolver( | ||
| secret_provider=secret_provider or AgentaNamedSecretProvider(), | ||
| gateway_resolver=gateway_resolver or AgentaGatewayToolResolver(), | ||
| workflow_resolver=workflow_resolver or AgentaWorkflowToolResolver(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Don't silently pair a custom gateway resolver with the default workflow resolver.
If a caller passes gateway_resolver but leaves workflow_resolver=None, this path still creates a fresh AgentaWorkflowToolResolver(). ToolResolver then collapses gateway and workflow resolutions onto one shared tool_callback, so mixed tool sets can resolve against different callback/auth sources and execute with whichever callback wins last. Either require both resolvers to be supplied together or assert callback equality before merging.
| type: Literal["reference"] = "reference" | ||
| slug: str = Field(min_length=1) | ||
| version: Optional[str] = None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate slug and version against the /tools/call segment grammar.
call_ref uses . as a structural separator, but this model currently accepts any non-empty slug and any version. Values like slug="my.wf" or version="1.2" will parse here, resolve into a callback spec, and then fail when the API router splits workflow.{slug}[.{version}]. Reject invalid segment characters and empty versions at config-parse time so the SDK and API contracts stay aligned.
Also applies to: 115-124
| class _ToolReferenceSchema(BaseModel): | ||
| """An ``@ag.reference`` marker standing in for one ``tools`` entry (the reference syntax). | ||
|
|
||
| The new top-level marker (sibling of ``@ag.embed``): unlike embed, the generic resolver | ||
| LEAVES this reference in the config, and ``resolve_tools`` turns the kept reference into a | ||
| server-side ``callback`` tool that runs the referenced workflow revision. The marker body | ||
| stays permissive (``Dict[str, Any]``) — its inner ``@ag.references`` / ``@ag.selector`` keys | ||
| name the workflow target, the same way ``@ag.embed`` does. ``extra="allow"`` so the | ||
| model-facing surface (``name`` / ``description`` / ``input_schema``) and the tool axes | ||
| (``needs_approval`` / ``render`` / ``permission``) ride as sibling keys of the marker.""" | ||
|
|
||
| model_config = ConfigDict(extra="allow", populate_by_name=True) | ||
|
|
||
| reference: Dict[str, Any] = Field( | ||
| alias="@ag.reference", | ||
| title="Workflow reference", | ||
| description=( | ||
| "An @ag.reference marker kept in the config; resolve_tools runs the referenced " | ||
| "workflow revision server-side as a callback tool." | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Model the supported @ag.reference sibling fields explicitly.
extra="allow" makes this public schema accept arbitrary keys, but the coercion path only preserves name, description, input_schema, needs_approval, render, and permission. That means typos validate here and then disappear during coercion, and the playground loses typed editors for the fields this PR actually supports. Please declare those sibling fields on _ToolReferenceSchema and forbid unknown extras again.
|
FE is up: #4877 (draft, based on this branch). Adds the playground UI to reference a workflow as a tool: a "Reference a workflow" picker in the tool selector, plus inline |
mmabrouk
force-pushed
the
feat/agent-embedref-tools
branch
from
7162a4d to
4bb6f16
Compare
|
Changes made — This pushes #4860 to the direct-call-tools plan's Workstream B spec ( Marker machinery removed
Kept working (NOT touched, per the plan — removing the routing belongs to Workstream A)
Env/variant targeting added
Tests + docs
What I need reviewed: a code review of the backend/SDK changes — in particular (1) the |
mmabrouk
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
api/oss/src/apis/fastapi/tools/router.py (1)
1144-1147: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject an empty pinned workflow version.
workflow.variant.foo.currently setsversion=""and skips version validation because the segment list only includes truthy versions. Validate the second operand when it is present, even if empty.Suggested fix
slug = operands[0] version = operands[1] if len(operands) == 2 else None - segments = [slug] + ([version] if version else []) + segments = [slug] + ([version] if version is not None else []) self._validate_slug_segments(segments=segments)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ef4b5331-ed70-49b7-be12-311c90ac6fee
📒 Files selected for processing (9)
api/entrypoints/routers.pyapi/oss/src/apis/fastapi/tools/router.pyapi/oss/tests/pytest/unit/embeds/test_utils.pyapi/oss/tests/pytest/unit/tools/test_workflow_tool_call.pydocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.mddocs/design/agent-workflows/interfaces/in-service/tool-models-and-resolution.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md
💤 Files with no reviewable changes (7)
- docs/design/agent-workflows/interfaces/README.md
- api/oss/tests/pytest/unit/embeds/test_utils.py
- docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md
- docs/design/agent-workflows/documentation/tools.md
- api/oss/tests/pytest/unit/tools/test_workflow_tool_call.py
- docs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.md
- docs/design/agent-workflows/interfaces/in-service/tool-models-and-resolution.md
🚧 Files skipped from review as they are similar to previous changes (1)
- api/entrypoints/routers.py
| call_ref = body.data.function.name.replace("__", ".") | ||
| if call_ref.startswith(_WORKFLOW_CALL_REF_PREFIX): | ||
| return await self._call_workflow_tool(request=request, body=body) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow/environment/version slug validators reject consecutive underscores
# wherever referenced workflow tool call_refs are created or accepted.
rg -n -C3 '(__|consecutive underscores|workflow.*slug|slug.*workflow|ReferenceToolConfig|call_ref)' \
api/oss/src sdks/python/agenta/sdkRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact router code around the cited lines.
sed -n '980,1010p' api/oss/src/apis/fastapi/tools/router.py
printf '\n---\n'
sed -n '1125,1145p' api/oss/src/apis/fastapi/tools/router.py
printf '\n=== slug/call_ref search ===\n'
rg -n -C2 'replace\("__", "\."\)|call_ref|WORKFLOW_CALL_REF_PREFIX|slug|variant_slug|environment_slug|version_slug|consecutive underscores|__' \
api/oss/src/apis/fastapi/tools/router.py \
api/oss/src/apis/fastapi \
sdks/python/agenta/sdk
printf '\n=== workflow/application/evaluator constructors ===\n'
sed -n '118,230p' sdks/python/agenta/sdk/decorators/running.py
printf '\n---\n'
sed -n '660,810p' sdks/python/agenta/sdk/decorators/running.py
printf '\n=== any explicit slug validation helpers ===\n'
rg -n -C3 'def .*slug|slug.*validate|validate.*slug|Regex|consecutive.*underscore|__' \
api/oss/src sdks/python/agenta/sdkRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== router lines around the two call_ref conversions ===\n'
sed -n '990,1005p' api/oss/src/apis/fastapi/tools/router.py
printf '\n---\n'
sed -n '1128,1140p' api/oss/src/apis/fastapi/tools/router.py
printf '\n=== workflow/tool call_ref construction and parsing ===\n'
rg -n -C3 'call_ref|replace\("__", "\."\)|_WORKFLOW_CALL_REF_PREFIX|function\.name|workflow.*call_ref|tool call_ref' \
api/oss/src/apis/fastapi/tools/router.py \
api/oss/src sdks/python/agenta/sdk
printf '\n=== slug validation / model constraints ===\n'
rg -n -C3 'slug.*(regex|pattern|consecutive|underscore|__)|variant_slug|environment_slug|version_slug|Field\(.*slug|constr\(.*slug|StringConstraints|validator.*slug' \
api/oss/src sdks/python/agenta/sdk
printf '\n=== workflow/application/evaluator definitions ===\n'
sed -n '118,235p' sdks/python/agenta/sdk/decorators/running.py
printf '\n---\n'
sed -n '660,805p' sdks/python/agenta/sdk/decorators/running.py
printf '\n=== any helper that normalizes "__" elsewhere ===\n'
rg -n -C2 'replace\("__", "\."\)|replace\("\.", "__"\)|"__"' api/oss/src sdks/python/agenta/sdkRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== workflow/environment slug field definitions ===\n'
sed -n '90,130p' api/oss/src/core/workflows/dtos.py
printf '\n---\n'
sed -n '70,120p' api/oss/src/core/environments/dtos.py
printf '\n---\n'
sed -n '150,220p' api/oss/src/core/applications/dtos.py
printf '\n---\n'
sed -n '70,120p' api/oss/src/core/queries/dtos.py
printf '\n---\n'
sed -n '90,150p' api/oss/src/core/evaluators/dtos.py
printf '\n=== where workflow/environment slugs are generated ===\n'
rg -n -C2 'uuid4\(\)\.hex|slug=|variant_slug|environment_slug|workflow_slug|query_variant_slug|evaluator_variant_slug' \
api/oss/src/core/workflows/service.py \
api/oss/src/core/environments/service.py \
api/oss/src/core/applications/service.py \
api/oss/src/core/queries/service.py \
api/oss/src/core/evaluators/service.py
printf '\n=== any explicit slug validation regexes in core workflows/environments/applications ===\n'
rg -n -C3 'Field\(.*pattern|constr\(|StringConstraints|regex=|pattern=' \
api/oss/src/core/workflows \
api/oss/src/core/environments \
api/oss/src/core/applications \
api/oss/src/core/queries \
api/oss/src/core/evaluatorsRepository: Agenta-AI/agenta
Length of output: 39752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== workflow base model slug fields ===\n'
rg -n -C3 'class (WorkflowCreate|WorkflowEdit|WorkflowQuery|WorkflowVariantCreate|WorkflowVariantEdit|WorkflowRevisionCommit|Reference)' \
api/oss/src/core/workflows/dtos.py \
api/oss/src/core/shared/dtos.py \
sdks/python/agenta/sdk/agents/tools/models.py
printf '\n=== workflow base model slices ===\n'
sed -n '1,90p' api/oss/src/core/workflows/dtos.py
printf '\n---\n'
sed -n '130,220p' api/oss/src/core/workflows/dtos.py
printf '\n---\n'
sed -n '1,120p' api/oss/src/core/shared/dtos.py
printf '\n---\n'
sed -n '140,220p' sdks/python/agenta/sdk/agents/tools/models.py
printf '\n=== any normalize/encode helper for workflow refs ===\n'
rg -n -C2 'replace\("__", "\."\)|normalize.*reference|call_ref|callRef|workflow\.\{axis\}' \
sdks/python/agenta/sdk/agents \
api/oss/src/apis/fastapi/tools/router.py \
api/oss/src/core/tools/service.pyRepository: Agenta-AI/agenta
Length of output: 28051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== shared slug / identifier model definitions ===\n'
rg -n -C4 'class Slug|Slug\(|pattern=|regex=|constr\(|StringConstraints|Field\(.*slug|def .*slug' \
api/oss/src/core/shared/dtos.py \
api/oss/src/core/git/dtos.py \
sdks/python/agenta/sdk/models/shared.py \
sdks/python/agenta/sdk/models/workflows.py
printf '\n=== relevant slug slices ===\n'
sed -n '1,120p' api/oss/src/core/shared/dtos.py
printf '\n---\n'
sed -n '1,120p' api/oss/src/core/git/dtos.py
printf '\n---\n'
sed -n '1,160p' sdks/python/agenta/sdk/models/shared.py
printf '\n---\n'
sed -n '1,220p' sdks/python/agenta/sdk/models/workflows.py
printf '\n=== workflow tool reference construction sites ===\n'
rg -n -C2 'ReferenceToolConfig|call_ref = f"workflow\.|workflow\.variant|workflow\.environment|replace\("__", "\."\)' \
sdks/python/agenta/sdk \
api/oss/src/apis/fastapi/tools/router.pyRepository: Agenta-AI/agenta
Length of output: 25671
Avoid lossy __ → . normalization in workflow call refs. Slug.check_url_safety allows consecutive underscores (and dots), so body.data.function.name.replace("__", ".") can turn a valid slug like foo__bar into foo.bar before dispatch. Use a delimiter-safe parse or reject __ in workflow refs.
| # Normalise arguments — the LLM may send them as a JSON string. | ||
| arguments = body.data.function.arguments | ||
| if isinstance(arguments, str): | ||
| try: | ||
| arguments = json.loads(arguments) | ||
| except json.JSONDecodeError as e: | ||
| log.warning("Failed to parse workflow tool arguments as JSON: %s", e) | ||
| arguments = {} | ||
| elif not isinstance(arguments, dict): | ||
| arguments = {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject malformed or non-object workflow arguments before invocation.
Invalid JSON falls back to {}, and JSON arrays/scalars parsed from strings bypass the dict check. That can run workflows with unintended inputs or fail later as a 500; return a 400 before invoking.
Suggested fix
arguments = body.data.function.arguments
if isinstance(arguments, str):
try:
arguments = json.loads(arguments)
except json.JSONDecodeError as e:
log.warning("Failed to parse workflow tool arguments as JSON: %s", e)
- arguments = {}
- elif not isinstance(arguments, dict):
- arguments = {}
+ raise HTTPException(
+ status_code=400,
+ detail="Workflow tool arguments must be a valid JSON object.",
+ ) from e
+
+ if not isinstance(arguments, dict):
+ raise HTTPException(
+ status_code=400,
+ detail="Workflow tool arguments must be a JSON object.",
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Normalise arguments — the LLM may send them as a JSON string. | |
| arguments = body.data.function.arguments | |
| if isinstance(arguments, str): | |
| try: | |
| arguments = json.loads(arguments) | |
| except json.JSONDecodeError as e: | |
| log.warning("Failed to parse workflow tool arguments as JSON: %s", e) | |
| arguments = {} | |
| elif not isinstance(arguments, dict): | |
| arguments = {} | |
| # Normalise arguments — the LLM may send them as a JSON string. | |
| arguments = body.data.function.arguments | |
| if isinstance(arguments, str): | |
| try: | |
| arguments = json.loads(arguments) | |
| except json.JSONDecodeError as e: | |
| log.warning("Failed to parse workflow tool arguments as JSON: %s", e) | |
| raise HTTPException( | |
| status_code=400, | |
| detail="Workflow tool arguments must be a valid JSON object.", | |
| ) from e | |
| if not isinstance(arguments, dict): | |
| raise HTTPException( | |
| status_code=400, | |
| detail="Workflow tool arguments must be a JSON object.", | |
| ) |
Add the @ag.reference syntax so an agent config tools[] entry can point at a workflow that runs server-side as a tool, the reference half of the embedref-tools two-syntax model (#4837): - @ag.reference (new): keep the reference. coerce_tool_config parses the kept marker into a ReferenceToolConfig (type "reference"); resolve_tools maps it to a CallbackToolSpec (call_ref "workflow.{slug}[.{version}]") + the shared ToolCallback. - @ag.embed (existing): inline the referenced value into a concrete client tool. The generic resolver stays tool-agnostic: it only inlines @ag.embed and leaves @ag.reference in place (AG_REFERENCE_KEY guard in the API embed finders). All tool-specific mapping lives in resolve_tools, via a new WorkflowToolResolver port + AgentaWorkflowToolResolver platform adapter. /tools/call routes a workflow.* call_ref to WorkflowsService.invoke_workflow (server-side execute; the workflow's secrets stay server-side, the gateway tool safety shape). No new runner kind, no wire change. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
…th env/variant targeting Remove the @ag.reference marker machinery (AG_REFERENCE_MARKER + _coerce_reference_tool in the SDK, _ToolReferenceSchema in the catalog, the AG_REFERENCE_KEY 'leave it' guards in the embed resolver). A workflow referenced as a tool is now the plain type:"reference" ToolConfig arm. Add ref_by (variant/environment) + slug/version/environment targeting, wired through _call_workflow_tool to WorkflowServiceRequest.references. Keeps the /tools/call workflow.* routing and its slug .fullmatch validation intact. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
mmabrouk
force-pushed
the
feat/agent-embedref-tools
branch
from
af95f27 to
63318ac
Compare
2 participants
Context
The agent config's
toolsandskillsfields were not symmetric.skillscould point at a stored workflow via@ag.embedand the backend would inline it.toolsaccepted only the four concrete variants (builtin/gateway/code/client), so a tool could not be authored as a workflow and reused by pointing at it. PR #4821 review comment 3469653315 asked to close that gap.This is the reference half of the lgtm'd two-syntax design (#4837): an author picks one of two syntaxes inside
tools[i], and the syntax decides the behavior.@ag.embed(existing): inline the referenced value. For a tool this inlines a concreteclientconfig; it rides the existingclientpath (fulfilled in the browser).@ag.reference(new): keep the reference.resolve_toolsturns it into a server-sidecallbacktool. When the model calls it, Agenta runs the referenced workflow revision, exactly like a gateway tool. The workflow's connections and secrets stay server-side.Changes
The marker, kept generic. The generic resolver stays tool-agnostic: it only INLINES
@ag.embedand LEAVES@ag.referencein place. The API embed finders now treat a kept@ag.referencenode as opaque (AG_REFERENCE_KEYincore/embeds/utils.py); the SDK_has_embed_markersalready ignores it, so an embed-free reference simply passes through toresolve_toolsuntouched. All tool-specific mapping lives inresolve_tools.The config + resolution (SDK).
compat.pycoerces a committed@ag.referencemarker into a newReferenceToolConfig(type: "reference", carrying the workflowslug/versionplus the model-facingname/description/input_schema). A newWorkflowToolResolverport +AgentaWorkflowToolResolverplatform adapter map it to aCallbackToolSpec:Gateway and workflow callbacks both point at
{api}/tools/callwith the same per-request auth, so they share the singleToolCallback.The server-side execute (API).
POST /tools/callroutes by thecall_refprefix: aworkflow.{slug}[.{version}]ref goes to a new_call_workflow_tool, which builds aWorkflowServiceRequest(references={"workflow": Reference(slug, version)},data.inputs = <the model's arguments>) and callsWorkflowsService.invoke_workflow(project_id, user_id, request). Auth is minted server-side from the caller's project + user, so the workflow's own secrets never leave the service. The workflow'sresponse.data.outputsbecomes the tool result.The schema.
AgentConfigSchema.toolsis now a union of the concreteToolConfigvariants plus an@ag.embedarm and an@ag.referencearm, so both validate in the playground.Scope / risk
callbackspec, an embed as aclientspec; only thecall_refcontent (workflow.*) is new. The golden/runfixtures are untouched and the wire-contract tests stay green.spec.callRefopaquely on thecallbackpath (direct or Daytona relay), soservices/agentneeds no change. Confirmed it has notools.*prefix assumption.is_toolFE display hint (deferred, display-only). No platform-tools-as-workflows (platform tools stay in the existing tools endpoints, per the [docs] Design EmbedRef tools (tools-as-workflows) #4837 review). NoWorkflowToolConfigdiscriminated variant in the public union beyond thereferencearm.Tests
All green. New coverage:
@ag.referencemarker coercion +call_refgrammar (test_parsing.py,test_models.py); reference resolves to aCallbackToolSpecand shares one callback with gateway (test_resolver.py); the platform adapter builds the spec + callback (test_workflow_resolver.py); thetoolsschema validates both arms (test_skill_config_catalog.py).@ag.referenceand still resolves sibling embeds (test_utils.py::TestAgReferenceLeftInPlace); the/tools/callworkflow-execute branch (test_workflow_tool_call.py).How to QA
Prerequisites: a local
pytestrun (no live stack needed for the unit path). For a live exercise, the agent stack with a committed workflow to reference.Steps (unit):
cd sdks/python && uv run python -m pytest oss/tests/pytest/unit/agents/.cd api && uv run python -m pytest oss/tests/pytest/unit/embeds/ oss/tests/pytest/unit/tools/.Expected result: green. The new tests assert: a
@ag.referencetool coerces toReferenceToolConfigwithcall_ref == "workflow.<slug>", resolves to acallbackspec, the generic resolver leaves the marker in place, and/tools/callwith aworkflow.*ref invokes the workflow and returns its outputs.Automated tests:
Edge cases: A config that mixes a gateway tool and a
@ag.referencetool must end with a single sharedToolCallback. A@ag.referenceblock with no@ag.embedanywhere must skip the SDK resolve pass entirely yet still reachresolve_tools. A malformedworkflow./workflow.a.b.c/ invalid-char call_ref must 400 before invoking.Note for JP's review
One judgment call worth a look: I wired
WorkflowsServiceintoToolsRouter(the API boundary) rather than intoToolsService(the relayed research suggested the latter). The router-level wiring keeps the ComposioToolsServicefrom depending onWorkflowsService(lower cross-domain coupling); the workflow-invoke orchestration sits in_call_workflow_toolnext to where the Composio branch callstools_service.execute_tool. Happy to move it if you prefer the service-level seam.https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc