Skip to content

fix(agent-manager): make tool requests strict - #13197

Merged
marius-kilocode merged 2 commits into
mainfrom
fix-agent-manager-tool-schema
Aug 18, 2026
Merged

fix(agent-manager): make tool requests strict#13197
marius-kilocode merged 2 commits into
mainfrom
fix-agent-manager-tool-schema

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Agent Manager tool requests currently flatten start and management operations into one object with optional fields. That makes it easy for a model to send a mixed payload and rely on action precedence, which can dispatch an unintended operation or fail provider-side schema validation.

Use a strict discriminated union for start, list, prompt, stop, and move requests. Each branch advertises only its operation-specific fields, rejects unknown or mixed fields at runtime, and preserves null as the explicit move target for unassigning a worktree. The provider-facing schema omits the incompatible prefix-only session ID pattern while retaining runtime validation.

Add focused coverage for generated branch schemas, valid operations, invalid session IDs, mixed payload rejection, and dispatch protection.

Comment thread packages/opencode/src/tool/json-schema.ts Outdated
Comment thread packages/opencode/src/kilocode/tool/agent-manager.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Both previous findings were addressed in the latest commit: packages/opencode/src/tool/json-schema.ts is back to byte-identical upstream (strictness now applied via structuredClone + walk in the Kilo-owned wireSchema(), which also protects the shared fromSchema cache from mutation), and the sessionID descriptions were restored on the prompt/stop/move branches with test coverage asserting them. The strict discriminated-union approach remains sound, mixed payloads are rejected at decode time before dispatch, and no memory leaks or error-handling gaps were introduced.

Files Reviewed (3 files)
  • .changeset/strict-agent-manager-tool-requests.md
  • packages/opencode/src/kilocode/tool/agent-manager.ts
  • packages/opencode/test/kilocode/agent-manager-tool.test.ts
Previous Review Summary (commit db0784b)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit db0784b)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/tool/json-schema.ts 9 Fork hygiene: the additionalProperties option could be applied in the Kilo-owned wireSchema() walk instead of modifying this shared upstream file
packages/opencode/src/kilocode/tool/agent-manager.ts 128 sessionID lost its model-facing description for the prompt/stop branches after WireParams was removed

The strict discriminated-union approach is sound: strict() correctly rejects mixed payloads at decode time before dispatch, the fromSchema cache bypass prevents wireSchema()'s strip mutation from polluting cached schemas, and no memory leaks or error-handling gaps were introduced. Tests cover branch schemas, mixed-payload rejection, and dispatch protection against the real implementation. Changeset is present and user-facing.

Files Reviewed (4 files)
  • .changeset/strict-agent-manager-tool-requests.md - 0 issues
  • packages/opencode/src/kilocode/tool/agent-manager.ts - 1 issue
  • packages/opencode/src/tool/json-schema.ts - 1 issue
  • packages/opencode/test/kilocode/agent-manager-tool.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by kimi-k3 · Input: 134.5K · Output: 6.5K · Cached: 214.6K

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants