feat(workflow): materialize certified tool capabilities as nodes - #49
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ 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:
📝 WalkthroughWalkthroughBackend tool capability manifests now project eligible Canvas nodes into workflow catalogs, with runtime recognition and version-pin validation. Frontend and backend regression tests cover catalog lookup, node creation, parameter propagation, runtime resolution, and stale pins. ChangesTool Capability Catalog Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ToolManifest
participant CapabilityProjection
participant WorkflowNodeCatalog
participant WorkflowCompiler
ToolManifest->>CapabilityProjection: expose catalog and toolCapability metadata
CapabilityProjection->>WorkflowNodeCatalog: provide projected catalog entry
WorkflowNodeCatalog->>WorkflowCompiler: create node with toolCapability and versionPin
WorkflowCompiler->>WorkflowCompiler: validate matching capability version
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
✅ Health: 7.3 📋 At a glance 🚨 Change risk: 9.1/10 (high)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-27 19:57 UTC |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@coderabbitai review Independent verifier: PASS. Python 7/7, frontend 12/12, compile/runtime/pin closed loop verified. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/workflow/runtime_registry.py (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated tool-capability/catalogId matching logic across two files.
_is_external_tool_capabilityinruntime_registry.pyand the inline gate in_validate_capability_version_pinincompiler.pyboth independently re-derive whether a node is a catalog-opted or generic external-tool-capability node fromnode.ui.catalogIdandnode.params.toolCapability.id. This PR had to update both in lockstep to add the same rule; the shared root cause is the lack of one common predicate.
backend/workflow/runtime_registry.py#L1136-1142: export/expose_is_external_tool_capability(or an equivalent helper) for reuse.backend/workflow/compiler.py#L388-399: replace the inlinecatalog_id not in {"external.tool.capability", tool_id}check with a call to the shared predicate fromruntime_registry.pyto prevent the two gates from silently diverging on a future change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/workflow/runtime_registry.py` at line 1, Expose the existing `_is_external_tool_capability` predicate from `runtime_registry.py` for reuse, then update `_validate_capability_version_pin` in `compiler.py` to call it instead of independently checking `node.ui.catalogId` and `node.params.toolCapability.id`. Preserve the current catalog-opted and generic external-tool-capability matching behavior while ensuring both validation paths share one predicate.backend/workflow/tool_capabilities.py (1)
307-311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
catalog["id"]duplicates theidkwarg; hardcodedkind/capabilitylimit reuse.Two things worth tightening in
_realtime_tool:
- Both call sites pass
catalog["id"]equal to the already-suppliedid=kwarg (Lines 308, 344)._is_registered_tool_capability_nodeinnode_registry.pyrequirescatalog_id == tool_idto recognize the node asnode_library; if a future call site'scatalog["id"]ever drifts fromid, the tool silently falls back tolegacyorigin with no error surfaced. DerivingnodeCatalog.idfrom theidparameter directly (instead of requiring callers to repeat it) removes this invariant entirely.kind: "action"andcapability: "store"(Lines 426-427) are hardcoded inside the helper rather than taken from thecatalogdict, even thoughcategory/iconare parameterized. Any future catalog-opted tool needing a different kind/capability (e.g."summarize","agent") will require editing the shared helper rather than just its call site.♻️ Suggested tightening
- catalog={ - "id": SITUATION_AWARENESS_TOOL_CAPABILITY_ID, - "category": "processing", - "icon": "Radar", - }, + catalog={ + "category": "processing", + "icon": "Radar", + "kind": "action", + "capability": "store", + },"canvas": {"node": catalog is not None}, **( { "nodeCatalog": { - "id": catalog["id"], + "id": id, "authority": "backend", "origin": "tool-capability", "category": catalog["category"], - "kind": "action", - "capability": "store", + "kind": catalog["kind"], + "capability": catalog["capability"], }, "presentation": {"icon": catalog["icon"]}, }Also applies to: 343-347, 388-433
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/workflow/tool_capabilities.py` around lines 307 - 311, Update _realtime_tool so nodeCatalog.id is derived directly from its id parameter, removing the duplicated catalog["id"] values at both call sites. Read kind and capability from the catalog argument instead of hardcoding them, while preserving the existing category and icon handling. Ensure each catalog-opted call site supplies the required kind and capability values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/workflow/capability_projection.py`:
- Line 1369: Update _tool_catalog_capabilities and its related catalog
capability projections to track emitted catalog_id values across all projected
tools and static WORKFLOW_CATALOG_IDS entries; detect collisions and surface
them through the module’s established error/logging mechanism instead of
allowing duplicate IDs to overwrite entries in callers’ ID maps.
In `@backend/workflow/node_registry.py`:
- Around line 255-269: Update _is_registered_tool_capability_node to require the
same catalog opt-in conditions as _tool_catalog_capabilities, including
manifest.canvas.node and manifest.nodeCatalog.authority == "backend", before
accepting a matching toolCapability.id. Reuse the existing catalog eligibility
logic or symbols rather than treating resolve_workflow_tool_capability(tool_id)
alone as sufficient.
---
Nitpick comments:
In `@backend/workflow/runtime_registry.py`:
- Line 1: Expose the existing `_is_external_tool_capability` predicate from
`runtime_registry.py` for reuse, then update `_validate_capability_version_pin`
in `compiler.py` to call it instead of independently checking
`node.ui.catalogId` and `node.params.toolCapability.id`. Preserve the current
catalog-opted and generic external-tool-capability matching behavior while
ensuring both validation paths share one predicate.
In `@backend/workflow/tool_capabilities.py`:
- Around line 307-311: Update _realtime_tool so nodeCatalog.id is derived
directly from its id parameter, removing the duplicated catalog["id"] values at
both call sites. Read kind and capability from the catalog argument instead of
hardcoding them, while preserving the existing category and icon handling.
Ensure each catalog-opted call site supplies the required kind and capability
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c50a9dce-93dd-4388-aea6-3cae0b4a7098
📒 Files selected for processing (8)
backend/workflow/capability_projection.pybackend/workflow/compiler.pybackend/workflow/node_registry.pybackend/workflow/runtime_registry.pybackend/workflow/tool_capabilities.pyfrontend/package.jsonfrontend/scripts/check-tool-capability-catalog-regressions.mjstests/unit/test_workflow_tool_catalog_projection.py
| return rows | ||
|
|
||
|
|
||
| def _tool_catalog_capabilities() -> list[WorkflowRuntimeCapability]: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
No de-duplication of catalog_id across projected tools.
_tool_catalog_capabilities() emits one row per matching tool without checking whether catalog_id collides with another tool's nodeCatalog.id (or with a static WORKFLOW_CATALOG_IDS entry). Since callers commonly build a {item.id: item} map, a collision means one capability silently disappears from the catalog rather than surfacing a config error.
Consider tracking seen ids and raising/logging on collision (or filtering) so a copy-paste mistake in a future catalog={...} block doesn't silently hide a tool node.
Also applies to: 1457-1457, 1479-1479
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/workflow/capability_projection.py` at line 1369, Update
_tool_catalog_capabilities and its related catalog capability projections to
track emitted catalog_id values across all projected tools and static
WORKFLOW_CATALOG_IDS entries; detect collisions and surface them through the
module’s established error/logging mechanism instead of allowing duplicate IDs
to overwrite entries in callers’ ID maps.
| def _is_registered_tool_capability_node( | ||
| node: WorkflowProjectNode, | ||
| catalog_id: str | None, | ||
| ) -> bool: | ||
| tool = node.params.get("toolCapability") | ||
| if not isinstance(tool, dict): | ||
| return False | ||
| tool_id = _read_string(tool.get("id")) | ||
| if tool_id is None or catalog_id != tool_id: | ||
| return False | ||
|
|
||
| # Local import avoids making the registry depend on this origin guard at import time. | ||
| from backend.workflow.tool_capabilities import resolve_workflow_tool_capability | ||
|
|
||
| return resolve_workflow_tool_capability(tool_id) is not None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how WorkflowNodeOrigin.kind == "node_library" is consumed downstream
rg -n -B2 -A6 'node_library' backend/workflow/compiler.py backend/workflow/runtime_registry.pyRepository: 2233admin/opencli-admin
Length of output: 4707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files 'backend/workflow/*.py' | sed -n '1,120p'
echo "== node_registry origin functions =="
fgrep -n "_is_registered_tool_capability_node\|resolve_node_origin\|_tool_catalog_capabilities\|workflow_tool_capability\|nodeCatalog\|canvas.node" backend/workflow/node_registry.py backend/workflow/capability_projection.py backend/workflow/tool_capabilities.py backend/workflow/runtime_registry.py backend/workflow/compiler.py 2>/dev/null | sed -n '1,240p'
echo "== imports == capability_projection.py =="
sed -n '1,220p' backend/workflow/capability_projection.py
echo "== tool_capabilities.py outline/imports =="
sed -n '1,220p' backend/workflow/tool_capabilities.pyRepository: 2233admin/opencli-admin
Length of output: 2473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant lines node_registry.py =="
sed -n '230,285p' backend/workflow/node_registry.py
echo "== resolve origin usage =="
rg -n -B3 -A8 "resolve_node_origin|WorkflowNodeOrigin|external.tool.capability|tool_capability|node_library" backend/workflow/compiler.py backend/workflow/runtime_registry.py backend/workflow/node_registry.py
echo "== capability_projection.py first 220 =="
sed -n '1,220p' backend/workflow/capability_projection.py
echo "== tool_capabilities.py first 220 =="
sed -n '1,220p' backend/workflow/tool_capabilities.pyRepository: 2233admin/opencli-admin
Length of output: 49698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tool catalog projection lines =="
rg -n -B8 -A18 "_tool_catalog_capabilities|canvas\.node|nodeCatalog|authority|external.tool.capability|tool.intelligence.native" backend/workflow/capability_projection.py backend/workflow/tool_capabilities.py
echo "== resolved external tool capability flow =="
sed -n '133,150p' backend/workflow/runtime_registry.py
sed -n '784,814p' backend/workflow/runtime_registry.py
sed -n '1136,1143p' backend/workflow/runtime_registry.py
echo "== behavioral probe: node origin vs catalog eligibility data =="
python3 - <<'PY'
from pathlib import Path
import ast
node_registry = Path("backend/workflow/node_registry.py").read_text()
catalog = Path("backend/workflow/capability_projection.py").read_text()
node_origin_tree = ast.parse(node_registry)
catalog_tree = ast.parse(catalog)
for name in ("_is_registered_tool_capability_node", "_tool_catalog_capabilities"):
for fn in ast.walk(node_origin_tree if name == "_is_registered_tool_capability_node" else catalog_tree):
if isinstance(fn, ast.FunctionDef) and fn.name == name:
print(f"== {name} ==")
print(Path("backend/workflow/node_registry.py" if name == "_is_registered_tool_capability_node" else "backend/workflow/capability_projection.py").read_text().splitlines()[fn.lineno-1:fn.end_lineno])
print("canvas.node occurrence:", fn.lineno <= next((line for line in ast.get_source_segment(catalog if name == "_tool_catalog_capabilities" else node_registry, fn) or "").splitlines() + ["no match"], ["no match"]))
PYRepository: 2233admin/opencli-admin
Length of output: 25270
Align tool capability origin checks with catalog opt-in eligibility.
resolve_node_origin now treats any matching toolCapability.id as node_library, but _tool_catalog_capabilities() only exposes tools with manifest.canvas.node AND manifest.nodeCatalog.authority == "backend". A non-catalog-node tool can still pass as node_library, while the same id in external-tool runtime handling only goes through workflow.external-tool.capability binding. Use the catalog opt-in gate here if node_library is meant to preserve the existing external.tool.capability behavior for that path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/workflow/node_registry.py` around lines 255 - 269, Update
_is_registered_tool_capability_node to require the same catalog opt-in
conditions as _tool_catalog_capabilities, including manifest.canvas.node and
manifest.nodeCatalog.authority == "backend", before accepting a matching
toolCapability.id. Reuse the existing catalog eligibility logic or symbols
rather than treating resolve_workflow_tool_capability(tool_id) alone as
sufficient.
1 participant
Outcome
Turn explicitly opted-in, machine-runnable Tool Capabilities into backend-authoritative Canvas catalog nodes instead of leaving them only in the resource list.
Changes
nodeCatalog.authority=backendandcanvas.node=trueui.catalogId == params.toolCapability.idand that ID exists in the backend registrySafety
Verification
pytest -q --no-cov tests/unit/test_workflow_tool_catalog_projection.py tests/integration/test_workflow_intelligence_tools.py→ 7 passedpnpm --dir frontend run check:node-capabilities→ 12 passedpy_compile→ passedgit diff --check 1d8b0e8..HEAD→ passedworkflow.external-tool.capabilitytool_capability_version_pin_mismatchIndependent verifier verdict: PASS.