Typed, discoverable resources — code-first defineTable + per-method request contract (RFC 0001) - #1767
Merged
Merged
Conversation
…le + per-method request contract Implements RFC 0001 (design PR #1503): the mergeable implementation of both authoring front-ends, integrated onto current main. Pillar 1/2b — code-first schema (resources/defineTable.ts): `defineTable(name, shape, opts)` + `types` author a table in TypeScript and eagerly register through the same `table()` factory GraphQL drives — the return IS the live class, with per-verb shapes inferred as `$record/$insert/$upsert/ $patch/$query` projections. Relations via lazy thunks (+ relationOf/hasManyOf escape hatch for mutual pairs). Pillar 2 — per-method request contract (resources/withSchema.ts): `defineResource(contract, impl)` (function form) + `Resource.withSchema(contract)` (class form). Handler types are derived from a runtime contract; a handler gets the SAME RequestTarget, structurally narrowed (subset, not fork). Each declared verb validates/coerces query/body before dispatch and throws a structured 400 (ValidationError, per-field {path,code,message}[]). Built-in `t`/`schemaOf` reduce to JsonSchemaFragment — one vocabulary across table fields, query, and bodies; a defineTable projection slots into a contract body via schemaOf({ table, projection }). Nullability: non-nullable by default, `.nullable` opts into null (table-derived bodies mirror Table.validate). Cross-cutting: - ValidationError (extends ClientError, 400); Table.validate refactored to the same structured shape (HTTP-title message preserved). - OpenAPI emits declared query/body/response for parameterised routes. - MCP drives tool input/output off the contract and binds arbitrary path params + query (applyContractInputs), lifting the generated-verb binding restriction for contract resources. - Shared attributeToFragment hardened with a nested-object branch; derive.ts Object/Array projection bugfix. Integration with main (the RFC branch was ~1007 lines behind on these files): merged with main's newer MCP paramroutes work (paramBinding gating, isSimpleIdRoute, mcpResources) and the liveResource authz fix — a request contract now exempts a resource from the generated-handler binding restriction. Design summary in resources/DESIGN.md; full RFC + type spikes remain in #1503. Type contract verified against built exports in docs/rfcs/spikes/0001/*-real.check.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements code-first schema authoring via defineTable and per-method request contracts using defineResource and Resource.withSchema (RFC 0001), enabling runtime validation, coercion, and integration with OpenAPI and MCP tools. The review feedback highlights several important issues: a type validation bypass where Date instances can bypass checks for non-date fields, potential crashes when accessing properties on untrusted error objects in wrapError, an inconsistency where target.getAll is not overridden to return coerced values, and a silent coercion bug where empty or whitespace-only query strings are parsed as 0 instead of triggering validation errors.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
Reviewed; no blockers found. |
kriszyp
requested review from
dawsontoth
and removed request for
cb1kenobi and
heskew
July 12, 2026 23:35
kriszyp
marked this pull request as draft
July 12, 2026 23:36
… test import
- scope the Date type-check exception to string/date-time fields (a Date must not
pass validation for number/boolean/array/object schemas)
- override target.getAll alongside get so multi-value query params read coerced
- reject empty/whitespace numeric query params instead of Number('')→0
- harden MCP wrapError: read the untrusted err's props inside a try/catch (revoked
Proxy / throwing getters must not crash the error path)
- application-contract.test.js: require('assert') + strict methods (node:assert/strict
is oxlint-banned via no-restricted-imports)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dawsontoth
approved these changes
Jul 13, 2026
dawsontoth
left a comment
dawsontoth
left a comment
Contributor
There was a problem hiding this comment.
I like the shape of it a lot.
…tifacts - rename resources/withSchema.ts → resources/defineResource.ts (defineResource is the primary API; Resource.withSchema stays the class-form name) + the test file - remove docs/rfcs/ (the *-real.check.ts type-contract proofs + tsconfig) — a real PR shouldn't carry spike/RFC scaffolding; those live in the design PR (#1503) - strip references to the spikes and the RFC doc (which are not in this PR) from code/test comments and resources/DESIGN.md; keep the #1503 pointer for the record No behavior change. 100 unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
marked this pull request as ready for review
July 14, 2026 18:45
Standalone type-tests under unitTests/types/ (no spike/RFC framing): assert the SHIPPED public types (imported from the built dist) against the contract — defineTable projections + relations, and the defineResource/withSchema handler inference, narrowed target, subset property, and negative (@ts-expect-error) cases. - unitTests/types/{defineResource,defineTable}.type-test.ts + tsconfig.json (strict, noEmit, skipLibCheck; isolated from the main build/typecheck, which don't include unitTests/, and from mocha, which only loads js/mjs) - `npm run test:types` (tsc --project unitTests/types/tsconfig.json) - CI: a "Type contract tests" step in unit-test.yml (after Build, gated to one Node version) so a regression in the public type surface fails CI Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements RFC 0001 — the mergeable implementation of typed, discoverable resources, integrated onto current
main. The full RFC + type-level spikes stay in the design PR #1503; this PR is the code intended for merge.What this adds
Pillar 1 / 2b — code-first schema (
resources/defineTable.ts)defineTable(name, shape, opts)+typesauthor a table in TypeScript and eagerly register it through the sametable()factory GraphQL drives — the returned value is the live table class, with per-verb shapes inferred as$record/$insert/$upsert/$patch/$queryprojections. Relations via lazy thunks (+relationOf/hasManyOffor mutual pairs).Pillar 2 — per-method request contract (
resources/withSchema.ts)defineResource(contract, impl)— function form.Resource.withSchema(contract)— class form (extendsit).A contract is
{ path, record?, get/post/put/patch/delete: { query?, body?, response? } }. Handler types are derived from it; a handler receives the SAMERequestTarget, structurally narrowed (target.id: string,target.get('expand')typed by the query schema) — a subset, not a fork. The narrowed types are justified by runtime enforcement: each declared verb validates/coercesquery/bodybefore dispatch and returns a structured 400 (ValidationError, per-field{ path, code, message }[]).One vocabulary. Built-in
t/schemaOfreduce toJsonSchemaFragment(the IRdefineTable/GraphQL fields already reduce to viaattributeToFragment), so one declaration feeds validation + OpenAPI + MCP. AdefineTableprojection slots into a contract body viaschemaOf({ table: Track, projection: 'insert' }).Nullability: non-nullable by default (a bare
t.stringrejectsnull);.optionalallows absence,.nullableallows explicitnull; table-derived bodies mirrorTable.validate's policy.Cross-cutting
ValidationError(extendsClientError, 400);Table.validaterefactored to the same structured shape (HTTP-title message byte-preserved).applyContractInputs).attributeToFragmenthardened with a nested-object branch;derive.tsObject/Arrayprojection bugfix.Integration notes (for reviewers)
The RFC branch was ~1007 lines behind
mainon the exact hot files this touches. Resolved into main's newer work:paramBindinggating (isSimpleIdRoute— drops generated verbs it can't bind) andmcpResources. A request contract now exempts a resource from that restriction (the contract binds arbitrary path params + query), which is the "richer binding rides on the contract" that main's own comments defer to.liveResourceauthz fix (Consolidate row-level allowRead enforcement — apply it on all read paths, not just the single-record point-read #1487): the contract handler wiring composes with main's per-call live-resource resolution.Review provenance
Cross-model reviewed (Codex + Harper-domain adjudicator) on the feature branch; the Gemini leg timed out (diff at
agy's hang threshold). Three "blockers" resolved to one false positive (arg-order —loadAsInstancepin) + two fixed (MCP flatten-poisons-body, output verb-name mismatch). No confirmed blockers.Tests
95 unit tests green on the merged tree, incl.
withSchema.test.js,defineTable-registration.test.js,openApi-contract.test.js,application-contract.test.js, and main'sapplication-paramroutes.test.js. Type contract verified against built exports viadocs/rfcs/spikes/0001/*-real.check.ts(tsc --project).🤖 Generated with Claude Code