fix(agent): infer tool context types from contextSchema#65
Conversation
tool() now preserves the concrete contextSchema type through its overloads: execute's ctx.local is typed from the schema, and callModel's context map slots accept/reject the real per-tool shape. No more 'ctx.local as X' or 'context: map as any' in consumers. Type-mechanics notes: - BaseToolFunction.contextSchema is readonly so TCtx stays covariant; concrete tools remain assignable to the wide Tool union. - execute/onToolCalled/onResponseReceived use method syntax, and ToModelOutputFunction/NextTurnParamsFunctions/ToolApprovalCheck use the bivarianceHack pattern: params positions check bivariantly, which the fully-erased types previously got implicitly. - InferToolContext distinguishes the wide default schema type from concrete schemas, so no-context tools keep Record<string, never>. - pnpm typecheck now includes the new type test (other .test-d.ts files have pre-existing SDK-version drift; excluded with a note). Types-only: zero runtime behavior change (one sanctioned construction- time write cast for the readonly field). 315 tests green; type gates verified red/green.
There was a problem hiding this comment.
Perry's Review
Verdict: 💬 Comments / questions
Risk: 🟢 Low
Types-only change that makes contextSchema inference work end-to-end. CI is green across all checks (lint, typecheck, unit, e2e). The bivariance hack pattern is the standard TypeScript technique for restoring assignability after preserving concrete generic types, and the type test file pins all the important consumer scenarios.
Findings
Suggestion: WithConcreteContextSchema is a no-op intersection
WithConcreteContextSchema<TCtx> always resolves to {}, so the & WithConcreteContextSchema<TCtx> intersections on ToolWithExecute, ToolWithGenerator, ManualTool, and HITLTool add no constraint. The concrete schema type is already preserved through BaseToolFunction<TInput, TCtx>.readonly contextSchema. The type is marked @deprecated, but the 4 intersection sites still reference it — they could be removed to reduce surface area. Low priority; not blocking.
What's good
- The
TCtxgeneric threading throughtool()overloads, config types, and return types is clean. The concrete schema type survives the overload boundary as intended. ContextFromSchema(forctx.localin execute) vs.InferToolContext(forToolContextMapslots) correctly diverge on the wide-default case: execute context getsRecord<string, unknown>(read/write any key), map getsRecord<string, never>(no context values accepted). This matches the semantic difference between "the tool's runtime context" and "what callers must provide."- Method-syntax /
bivarianceHackpattern is applied consistently across all parametric function types that need it (execute,onToolCalled,onResponseReceived,ToModelOutputFunction,NextTurnParamsFunctions,ToolApprovalCheck). - The
tsconfig.typecheck.jsoncorrectly extends the base config and adds only the new test file, with an honest note about the pre-existing drift in other type-level test files. - Type test coverage is excellent: inference acceptance, wrong-shape rejection, missing-key rejection, no-schema unchanged, mixed arrays, manual tools, and wide-union assignability.
| * `InferToolContext` distinguishes the wide default from concrete schemas. | ||
| * Kept as an alias for source compatibility. | ||
| */ | ||
| export type WithConcreteContextSchema<_TCtx extends $ZodObject<$ZodShape>> = |
There was a problem hiding this comment.
💡 Suggestion: WithConcreteContextSchema<TCtx> always resolves to {}, so the & WithConcreteContextSchema<TCtx> intersections on ToolWithExecute (line 389), ToolWithGenerator (line 405), ManualTool (line 418), and HITLTool (line 433) are no-ops. The concrete TCtx is already preserved through BaseToolFunction<TInput, TCtx>.readonly contextSchema.
Consider removing the type and the 4 intersection sites to reduce dead surface area. Low priority — not blocking.
▶ Prompt for agents
In packages/agent/src/lib/tool-types.ts, WithConcreteContextSchema<TCtx> (line 72) always resolves to {}. Remove the type definition and the 4 `& WithConcreteContextSchema<TCtx>` intersections on ToolWithExecute, ToolWithGenerator, ManualTool, and HITLTool. Run `pnpm --filter @openrouter/agent typecheck` and `pnpm --filter @openrouter/agent test` to verify the types still resolve correctly.
| "test:e2e": "vitest --run --project e2e", | ||
| "test:watch": "vitest --watch --project unit", | ||
| "typecheck": "tsc --noEmit", | ||
| "typecheck": "tsc --noEmit -p tsconfig.typecheck.json", |
There was a problem hiding this comment.
🔍 typecheck now covers only one test-d file; others still undrifted
The typecheck script now points at tsconfig.typecheck.json (packages/agent/package.json:119), which includes src/**/*.ts plus only tests/unit/context-schema-inference.test-d.ts. Previously tsc --noEmit used tsconfig.json which includes only src, so tests were never typechecked at all — the new config strictly increases coverage for the one file. However, the other .test-d.ts files remain excluded from tsc and (per the PR description) fail against the current @openrouter/sdk. This is acknowledged as out of scope, but reviewers should track that these type tests are still not gated, so future type regressions in them will go unnoticed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Problem
contextSchema's Zod type never reachedexecute's context orcallModel'scontextmap:ctxtyped asToolExecuteContext<Name, Record<string, unknown>>, andInferToolContextsaw only the erased$ZodObject<$ZodShape>→ map slots resolved tonever→ every consumer neededctx.local as Xandcontext: map as any. Typed tool context is a headline feature; the types didn't help. (agent-monorepo pr-review FRICTION #2 — 2 casts; wikillm — 1.)Fix (types-only)
tool()overloads carry aTCtxgeneric so the concrete schema type survives to the returned tool'sfunction.contextSchemactx.localis nowzodInfer<TCtx>insideexecute;ToolContextMapslots resolve to the real per-tool shape (and reject wrong shapes)contextSchemakeepRecord<string, never>exactly as beforeType-mechanics worth reviewing
Preserving concrete types exposed strict-function-types contravariance against the wide
Toolunion (previously masked by full erasure). Restored assignability with the standard patterns:contextSchemaisreadonly(covariantTCtx)execute/onToolCalled/onResponseReceiveduse method syntax;ToModelOutputFunction/NextTurnParamsFunctions/ToolApprovalCheckuse thebivarianceHackpattern — params positions check bivariantly, which erased types implicitly got beforetool()(runtime unchanged)Typecheck coverage note
pnpm typechecknow includes the new type test file. Discovered while wiring: the existing.test-d.tsfiles were never actually typechecked (vitest typecheck config gap), and several failtscagainst the current@openrouter/sdk(pre-existing drift, e.g.OutputServerToolItemrenamed). Only the new file is included, with a tsconfig note; fixing the others is out of scope here.Tests
tests/unit/context-schema-inference.test-d.ts— ctx.local inference, map accept/reject (incl. missing-key + per-slot in mixed arrays), no-schema unchanged, manual tools, wide-union assignability. Verified the gate goes red on a deliberate type error. Suite: 30 files / 315 tests, build + biome green.Changeset: patch. Plan: agent-monorepo
planning/sdk-fix-plan.mdPR-3.