improve: let Code Mode use declared tool result shapes#109813
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b928540ce7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b928540 to
40a5f61
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 9:36 AM ET / 13:36 UTC. Summary PR surface: Source +418, Tests +789, Docs +212, Generated 0, Other +48. Total +1467 across 33 files. Reproducibility: not applicable. This PR proposes a new tool-result contract and performance capability rather than repairing a defined current-main failure. Its benefit is demonstrated through the supplied before/after live matrix. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Approve one documented optional Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new tool-result contract and performance capability rather than repairing a defined current-main failure. Its benefit is demonstrated through the supplied before/after live matrix. Is this the best way to solve the issue? Yes technically: one optional schema attached to the existing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b48c8016237c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +418, Tests +789, Docs +212, Generated 0, Other +48. Total +1467 across 33 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (6 earlier review cycles)
|
5ccb36c to
70531c6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70531c6ecb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7fbf9da to
7fa86fc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fa86fcb88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Human-readable label for UI display. */ | ||
| label: string; | ||
| /** Optional schema for the structured `AgentToolResult.details` value. */ | ||
| outputSchema?: TSchema; |
There was a problem hiding this comment.
Preserve output schemas across tool normalization
In provider/model runs where a provider runtime or runtimePlan normalizer returns cloned tool objects (a supported path handled by normalizeAgentRuntimeTools / copyRuntimeToolMetadata), this new field is not copied with the existing plugin/channel/catalog metadata before applyCodeModeCatalog or applyToolSearchCatalog builds the catalog. The cloned tool therefore loses outputSchema, so Code Mode shows -> ? and skips the validation for exactly the providers that normalize schemas; copy outputSchema in that metadata-preservation step.
Useful? React with 👍 / 👎.
7fa86fc to
9b04bcb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b04bcb1a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| // Hooks may replace details after execution, so validate the final catalog | ||
| // value at the host boundary before Code Mode trusts the declared shape. | ||
| await assertCatalogOutputMatchesSchema(entry, result); |
There was a problem hiding this comment.
Do not record schema-rejected tool results
When this post-execution validation fails in a normal embedded run, the hidden target tool has already been recorded as a successful Tool Search projection: the production executor in src/agents/embedded-agent-runner/run/attempt-stream-prepare.ts pushes result before returning to ToolSearchRuntime, and src/agents/embedded-agent-runner/run/attempt-stream-settle.ts later projects every queued projection into the transcript. So a tool that declares outputSchema but returns mismatching details still has its rejected result content written into the conversation before the bridge/control tool reports the schema error, leaking exactly the value this gate is meant to suppress; validate before queuing the projection or defer projection until after this schema check succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1e5c82f. The embedded executor now awaits an acceptance gate before queuing any hidden-tool projection. That gate creates a deep-frozen JSON snapshot, validates the snapshot, and returns the same accepted object for projection; the host repeats acceptance so custom executors cannot return a later-mutated value. Rejected results produce only the sanitized error projection. Added regressions for rejection-before-projection, delayed-settlement mutation, mutation after executor acceptance, and framework policy-block identity. Exact focused proof: 275/275. Final Testbox changed gate: core/test types passed, lint 0/0, SDK/boundary/import-cycle/architecture guards passed.
9b04bcb to
bcb8096
Compare
c164a6a to
f65d1f2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f65d1f2719
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!variants || variants.length === 0 || variants.length > MAX_COMPACT_UNION_TYPES) { | ||
| return undefined; |
There was a problem hiding this comment.
Do not fall through after unrenderable output unions
When a trusted outputSchema has anyOf/oneOf alongside a base type/properties but the variants are empty or exceed the compact limit, this returns undefined, and compactSchemaType then continues to render the base schema as a complete hint. In that case Code Mode can advertise an output shape that omits required union variants, so the model may skip describe and process missing fields in the first exec; return an incomplete/unknown hint whenever a union keyword is present but cannot be compacted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 85df5e7. Union presence is now fail-closed: malformed, empty, oversized, dual anyOf/oneOf, or base-structural intersections return an incomplete hint instead of falling through to type/properties/const/enum rendering. Added base-object regressions for empty oneOf, a five-variant anyOf, and both union keywords. Focused proof: 6/6; autoreview clean at 0.96 confidence. Fresh exact-head CI is running.
65d8afc to
2cc947d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17f0cb71be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| // Settlement persists every queued projection. Validate the final result | ||
| // first so a rejected hidden-tool value never enters session history. | ||
| const acceptedResult = await toolParams.acceptResultBeforeProjection(result); |
There was a problem hiding this comment.
Validate hidden outputs before lifecycle success
When a hidden Code Mode/Tool Search target declares outputSchema and returns mismatching details, this acceptance gate runs only after subscription.runToolLifecycle has already completed. I checked src/agents/embedded-agent-subscribe.ts:1413-1421: that lifecycle helper passes the raw result to handleToolExecutionEnd, and src/agents/embedded-agent-subscribe.handlers.tools.ts:1510-1519 emits it on the tool event stream/onAgentToolResult before validation rejects it, so rejected values still leak as successful tool results outside the transcript projection. Fresh evidence beyond the earlier transcript finding is that the current head still emits the raw lifecycle result before this line; move acceptance into the lifecycle execute closure or otherwise emit only the accepted/error result.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
* feat(agents): add Code Mode output contracts * perf(agents): defer untrusted tool schemas * chore(plugin-sdk): refresh API baseline * fix(agents): preserve deferred schema markers * fix(agents): preserve policy-blocked tool results * fix(agents): preserve nullable output contracts * fix(agents): reject policy blocks from success contracts * fix(agents): preserve output contracts through normalization * test(agents): type normalized tool fixtures * fix(agents): validate catalog results before projection * fix(agents): keep result snapshots internal * fix(agents): fail closed on unrenderable unions * fix(agents): preserve empty result markers * test(agents): keep result fixture discriminator literal * test(agents): use native own-property check
Related: #109288
Follow-up to #109651 and #109290
AI-assisted: Yes (Codex)
What Problem This Solves
Code Mode learned exact tool ids and compact input signatures from #109651, but every tool result still appeared as unknown. Even when a tool had a stable structured result, models often spent one exec returning the raw value and another exec processing fields they could have known up front. This left avoidable model turns, tokens, and latency in common multi-operation workflows.
Why This Change Was Made
Trusted OpenClaw core and plugin tools can now declare an optional
outputSchemaforAgentToolResult.details. Code Mode and Tool Search expose a bounded TypeScript-style output hint only when that schema is complete and closed; open, oversized, unsupported, MCP, and client schemas remain unknown and raw-first.OpenClaw compiles trusted output schemas before execution, then validates final details after normal hooks. The embedded executor converts accepted results into deep-frozen JSON-safe snapshots before queuing transcript projections, preserves the result envelope's
detailspresence marker, validates that exact snapshot, and returns it for a second host-boundary acceptance pass; rejected or later-mutated values therefore cannot enter session history as successful hidden-tool results, and an explicitdetails: undefinedcannot redirect unwrapping or validation to the whole envelope. Framework-owned pre-execution policy blocks remain outside the success schema and are recognized by object identity, so calls reject with the denial reason before generated code can treat a block as a successful value; tool-authored blocked lookalikes still fail validation. Runtime/provider normalization preserves the result contract when it clones tool definitions. The compact renderer has strict depth/property/character/union bounds and normalizes AJV-style nullable schemas so every validator-acceptednullremains visible to generated code. Remote and client schemas stay deferred asinput: "unknown"and use object identity for catalog reuse, preventing attacker-sized metadata walks while preserving schema-replacement invalidation.agents_listadopts the contract as the first built-in example.The change also promotes Code Mode to a dedicated
/tools/code-modedocumentation page with Quickstart, runtime semantics, guest API, output-contract authoring, trust/validation rules, fallback behavior, debugging, and validation guidance. The old/reference/code-moderoute redirects to it.Maintainer Decision
Approve
outputSchemaas the supported optional plugin API for describing and validatingAgentToolResult.detailsacross Code Mode and Tool Search. Tools that omit it retain existing behavior. Opted-in tools get pre-execution schema compilation, final-details validation, and framework-owned policy-result preservation.User Impact
Code Mode can call and process tools with declared result shapes in the first exec instead of paying a raw-inspection round trip. Unknown outputs keep the safe existing raw-first behavior. Plugin authors get one documented result contract shared by Tool Search and Code Mode, without trusting remote MCP or client schema claims.
Evidence
Live multi-provider matrix: four validated Code Mode tasks per provider over the real Agent runtime and a 72-tool catalog with adversarial decoys. Baseline is merged #109651; after is this branch.
Stress: three complete Code Mode matrices produced 34/36 validated tasks, 78 turns, 42 exec calls, 57 underlying service calls, and zero decoy calls. Anthropic and Google were 12/12; OpenAI was 10/12. Both misses were the model filtering singular fixture value
plumwith the plural wordplumsafter selecting the correct real tool, not a discovery, transport, schema, or decoy failure. A final OpenAI/Google pass kept every completed task at two turns and one exec. The final Anthropic retry hit provider overload before tokens or tools; earlier Anthropic stress was 12/12.Validation:
agents_list, plugin SDK, descriptor cache, and optional tool metadata.input: "unknown"sentinel.