Standardize tool invocation contracts and fail-open tooling metadata (Vibe Kanban)#215
Standardize tool invocation contracts and fail-open tooling metadata (Vibe Kanban)#215
Conversation
📝 WalkthroughWalkthroughThe PR introduces the "Tool Invocation Contract v1," a standardized framework for capturing tool-related behavior across three lifecycle stages: planner intent ( Changes
Sequence Diagram(s)sequenceDiagram
participant Planner
participant Orchestrator
participant Runtime as Runtime/Provider
participant Metadata as Response Metadata
Planner->>Orchestrator: ToolInvocationIntent<br/>(toolName, requested, input?)
Orchestrator->>Orchestrator: Evaluate eligibility<br/>based on provider support
Orchestrator->>Runtime: ToolInvocationRequest<br/>(toolName, requested,<br/>eligible, reasonCode?)
Runtime->>Runtime: Execute or skip<br/>based on eligibility
Runtime-->>Orchestrator: ToolExecutionContext<br/>(toolName, status,<br/>reasonCode?, durationMs?)
Orchestrator->>Metadata: Record execution<br/>in ResponseMetadata.execution[]<br/>with kind="tool"
Metadata-->>Planner: Final metadata with<br/>tool outcome + reasonCode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
# Conflicts: # packages/backend/src/services/chatOrchestrator.ts # packages/contracts/src/ethics-core/index.ts # packages/contracts/src/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/services/chatOrchestrator.ts (1)
291-305: Clarify the semantic ofeligiblewhenrequested=false.When
toolIntent.requested=false, the code setseligible: falsewithreasonCode: 'tool_not_requested'. While semantically reasonable, consider whethereligiblehas meaning when the tool was never requested.This creates a fully-defined state for all paths, which aids analytics consistency. If this is intentional defensive design, a brief inline comment explaining the rationale would help future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/chatOrchestrator.ts` around lines 291 - 305, The code sets toolRequestContext (type ToolInvocationRequest) with eligible: false when buildWebSearchToolIntent(...).requested is false; add a short inline comment next to the branch that explains the semantic choice (i.e., when requested === false we intentionally set eligible to false and provide reasonCode 'tool_not_requested' so analytics/consumers always see a fully-defined invocation state), or alternatively make eligible undefined if you prefer to express "not applicable" — update the branch around toolRequestContext (the object literal for web_search when requested is false) to include that explanatory comment referencing the reasonCode and why eligible is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/services/chatOrchestrator.ts`:
- Around line 291-305: The code sets toolRequestContext (type
ToolInvocationRequest) with eligible: false when
buildWebSearchToolIntent(...).requested is false; add a short inline comment
next to the branch that explains the semantic choice (i.e., when requested ===
false we intentionally set eligible to false and provide reasonCode
'tool_not_requested' so analytics/consumers always see a fully-defined
invocation state), or alternatively make eligible undefined if you prefer to
express "not applicable" — update the branch around toolRequestContext (the
object literal for web_search when requested is false) to include that
explanatory comment referencing the reasonCode and why eligible is set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b136d619-9cc7-48ce-8068-2e9b2224908d
📒 Files selected for processing (14)
docs/architecture/tool-invocation-contract-v1.mdpackages/agent-runtime/src/index.tspackages/agent-runtime/src/voltagentRuntime.tspackages/agent-runtime/test/voltagentRuntime.test.tspackages/api-client/src/chat.tspackages/api-client/src/index.tspackages/backend/src/services/chatOrchestrator.tspackages/backend/src/services/chatService.tspackages/backend/src/services/openaiService.tspackages/backend/test/openaiService.metadata.test.tspackages/contracts/src/ethics-core/index.tspackages/contracts/src/ethics-core/types.tspackages/contracts/src/index.tspackages/contracts/src/web/schemas.ts
Summary
This PR establishes a single Tool Invocation Contract v1 across planner, orchestration, and runtime boundaries so tool request, eligibility, and outcome states are serialized consistently.
What Changed
Why
Task context required one stable, framework-agnostic, serializable contract for tool behavior across planner -> orchestrator -> runtime, with predictable fail-open handling and clear operator/UI-visible outcomes (�xecuted / skipped / ailed +
easonCode).
This implementation aligns tool semantics across packages, reduces contract drift risk, and preserves backend authority over provenance, trace metadata, and cost ownership.
Important Implementation Details
This PR was written using Vibe Kanban
Summary by CodeRabbit
New Features
Documentation
Tests