feat(agents): native schema for codex/claude agent({ schema })#99
feat(agents): native schema for codex/claude agent({ schema })#99Waishnav wants to merge 5 commits into
Conversation
Wire LocalAgentRunInput.schema through CodexSdkLocalAgentRuntime to thread.run turn options, and surface parsed structured output.
Pass JSON Schema via outputFormat on query options and prefer structured_output from result messages. OpenCode stays prompt-path only.
Hardcode NATIVE_SCHEMA_PROVIDERS; attempt 0 uses adapter schema without prompt bloat, then prompt-repair retries with Ajv. Wire schema through runProvider / CLI worker. Document in skill.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository 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:
✨ 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 |
| ...providerBase, | ||
| prompt: p, | ||
| // Keep schema on adapter for codex/claude native+repair attempts. | ||
| schema: agentOpts.schema, |
There was a problem hiding this comment.
P1 — this is not yet a real native-to-prompt fallback.
enforceAgentSchema() calls run(prompt, { mode }), but this closure ignores mode and always forwards schema. As a result, a prompt repair attempt still invokes the provider's native structured-output API. If Codex or Claude rejects the schema itself, the exception also escapes before any fallback attempt occurs.
Please accept the mode here and omit schema for prompt-only attempts, and treat native capability/schema-rejection errors as a transition to the prompt path. A test should cover a native adapter throwing on attempt 0 and succeeding through prompt+Ajv on the next attempt.
There was a problem hiding this comment.
Addressed in this PR by a8b8e5a. The enforcement callback now consumes mode, forwards schema only for the native attempt, and transitions recognized native-schema capability failures to prompt+Ajv fallback. Regression coverage makes the native attempt throw and verifies that the prompt-only attempt succeeds.
| model: input.model, | ||
| effort: input.effort, | ||
| writeMode: "allowed", | ||
| schema: input.schema, |
There was a problem hiding this comment.
P1 — schema repair attempts still lose the provider session.
No providerSessionId is forwarded here, so each retry starts a new Codex thread or Claude session. That means the repair prompt refers to a previous validation failure while the provider cannot see the previous response, and it creates extra sessions/cost.
Please thread the latest session ID through WorkflowProviderRunInput / EnforceSchemaInput and pass it into the next adapter call. Add a test asserting that attempt 2 receives the session ID returned by attempt 1.
There was a problem hiding this comment.
Addressed in this PR by a8b8e5a. The latest providerSessionId is threaded through WorkflowProviderRunInput, schema enforcement, the worker, and provider adapters. A regression test asserts that the second attempt receives the session ID returned by the first.
| lastSession = result.providerSessionId ?? lastSession; | ||
| const extracted = | ||
| result.structured !== undefined | ||
| ? result.structured |
There was a problem hiding this comment.
P1 — valid structured JSON strings are rejected.
The Claude adapter permits structured_output to be a string, but any defined result.structured is passed directly to Ajv here. For example, structured: '{"n":1}' is validated as a string and fails an object schema instead of being parsed.
Please normalize native structured output first: parse strings as JSON, accept JSON values, and only then validate. This deserves a regression test with string-valued structured_output.
There was a problem hiding this comment.
Addressed in this PR by a8b8e5a. Native structured output is normalized into JSON candidates before Ajv validation; string-valued JSON is parsed and validated as the represented object/array/value rather than as a raw string. Regression coverage includes structured: "{\"n\":4}".
| /** Provider-native effort / reasoning level (was thinking). */ | ||
| effort?: string; | ||
| /** JSON Schema for native structured output (codex/claude). */ | ||
| schema?: object; |
There was a problem hiding this comment.
P2 — this feature carries a schema through the stack but discards its type.
schema?: object accepts values that are not JSON schemas, while the corresponding result remains structured?: unknown. This repeats across the local-agent, workflow-provider, and enforcement contracts, so workflow authors get no compile-time benefit from supplying a schema.
Please introduce shared JsonValue / JsonSchema types and make these contracts generic. Ideally the public agent() overload should derive its return type from a const schema (FromSchema<S>, TypeBox Static<S>, or equivalent). Even before that larger change, replacing object with a JSON-safe schema type would materially tighten this boundary.
There was a problem hiding this comment.
Addressed by stacked PR #100 (4cb4007). The stack introduces shared JsonValue, JsonObject, and JsonSchema contracts, and the public WorkflowAgent schema overload derives its result with FromSchema<S>. Compile-time tests cover schema-derived results and tuple-preserving orchestration types. This comment becomes obsolete once #100 is merged on top of #99.
| const require = createRequire(import.meta.url); | ||
|
|
||
| /** Providers with a real structured-output API (hardcoded — no capability probe). */ | ||
| export const NATIVE_SCHEMA_PROVIDERS = new Set(["codex", "claude"]); |
There was a problem hiding this comment.
P2 — provider capabilities should be exhaustive and typed, not a mutable Set<string>.
This duplicates adapter knowledge, allows mutation by importers, and adding a new LocalAgentProvider does not force a structured-output capability decision.
Please use an immutable capability map such as satisfies Record<LocalAgentProvider, ProviderCapabilities>. That gives TypeScript an exhaustiveness check whenever providers are added and leaves room for related capabilities such as session resume.
There was a problem hiding this comment.
Addressed by stacked PR #100 (4cb4007). Provider capabilities now live in an immutable LOCAL_AGENT_PROVIDER_CAPABILITIES object declared with satisfies Record<LocalAgentProvider, LocalAgentProviderCapabilities>, so adding a provider requires an explicit capability decision. Native structured-output checks use that map.
| run(input: LocalAgentRunInput): Promise<LocalAgentRunResult>; | ||
| } | ||
|
|
||
| interface CodexTurnOptions { |
There was a problem hiding this comment.
P2 — prefer the SDK's exported types over a local structural copy.
@openai/codex-sdk already exports TurnOptions; re-declaring it here can silently drift when the dependency changes. The Claude SDK likewise exports OutputFormat and SDKResultMessage, which would remove casts and make result narrowing safer.
Please import the upstream types directly so SDK incompatibilities surface during typechecking rather than at runtime.
| if (extracted === undefined) { | ||
| lastErrors = "Response was not valid JSON"; | ||
| input.onRetry?.({ attempt: attempt + 1, errors: lastErrors }); | ||
| input.onRetry?.({ attempt: attempt + 1, errors: lastErrors, mode }); |
There was a problem hiding this comment.
P2 — schema_retry is emitted even when no retry will happen.
On the final allowed attempt, this callback still fires and records a retry event before the loop exits and throws. Also, mode describes the failed attempt rather than the upcoming retry mode, which is easy for consumers to misread.
Please only call onRetry when attempt < maxRetries, or rename the event to schema_validation_failed and include explicit willRetry and nextMode fields. Apply the same handling to the Ajv-validation failure branch below.
There was a problem hiding this comment.
Addressed in this PR by a8b8e5a. Retry callbacks are now gated by attempt < maxRetries, including invalid-JSON, Ajv-validation, and native-capability fallback paths. The exhausted final attempt no longer records a retry that will not occur.
Summary
LocalAgentRunInput.schemathrough Codexthread.run({ outputSchema })and ClaudeoutputFormat: { type: 'json_schema', schema }, surfacestructured/structured_output.enforceAgentSchemais native-first for hardcodedcodex+claude(attempt 0 raw prompt + adapter schema), then prompt-repair + Ajv; other providers keep prompt+Ajv. Schema stays on adapter during repair.Stack
pr/dw-5-mcp-skill(PR feat(workflow): MCP tools, skill, agentProviders #98)pr/dw-6-native-schemaTest plan
tsx src/local-agent-runtime.test.ts(outputSchema on/off)tsx src/local-agent-adapters.test.ts(outputFormat / structured_output helpers)tsx src/workflow-schema.test.ts(native 1-shot, native→prompt repair, non-native never native)tsx src/workflow-engine.test.tsnpm run typecheckagent({ schema })against real codex/claude