Skip to content

PATCH contract mismatch: spec defines content-only merge patch, APIAdapter sends record-field envelopes #52

Description

@cuibonobo

Problem

The spec and the only existing client disagree about what the body of PATCH /records/:id is.

The spec says (Wire Format → Records): PATCH accepts a partial content object — RFC 7396 JSON Merge Patch semantics over content only. Omitted fields are retained, null deletes a field, and "associations and permissions are managed via their own endpoints."

The adapter sends (APIAdapter.updateRecord): the raw Partial<StackRecord> changes object that Stack hands to the adapter contract — e.g. { typeId, content: <fully merged content>, updatedAt, version }.

A server implementing the spec as written would merge typeId, version, and updatedAt into the record's content as ordinary fields. Concrete failure modes:

  1. Record fields become content fields. Every Stack.update() over the API adapter corrupts content on a spec-conforming server (content.typeId, content.version, content.updatedAt appear as user data).
  2. Field deletion silently fails. Stack.update() resolves null-deletions client-side, so deleted fields are simply absent from the merged content it sends. A merge-patch server retains absent fields — update(id, { title: null }) deletes locally, does nothing remotely. Same API, silently different data.
  3. setPermissions() goes to the wrong endpoint. Stack.setPermissions() funnels through adapter.updateRecord(id, { permissions }) → a PATCH body { permissions }, which a conforming server merges into content. The spec's PUT /records/:id/permissions endpoint is never called by the adapter.
  4. restoreVersion() bypasses its endpoint. The spec defines POST /records/:id/restore/:version; the adapter never uses it — Stack.restoreVersion() goes through the generic updateRecord PATCH instead. Restore works only by accident of the same non-conforming body shape.
  5. Migration commit has no wire representation at all. migrateAll() writes typeId changes through updateRecord, but a content-only PATCH cannot express "this record is now note@2." There is currently no spec-conforming way to commit a migration over the API adapter.

Root cause

StackRecordAdapter.updateRecord(id, changes: Partial<StackRecord>) is a grab-bag: Stack smuggles four different intents through it — content patch, permission replacement, version restore, migration commit. The wire format, correctly, has intent-specific endpoints. The API adapter sits between a one-method contract and a many-endpoint protocol and can't map faithfully. Fixing the body shape without fixing the contract just moves the mismatch around.

Proposed resolution

Affirm the spec's contract — PATCH stays a content-only RFC 7396 merge patch — and restructure the adapter contract to carry intent. The spec's version is the better wire contract: standard semantics, minimal payloads, working null-deletion, field-level composition of concurrent updates, and it pairs naturally with If-Match (#48). The alternative (respec PATCH as a full record envelope) would codify client-side read-modify-write and forfeit all of that.

1. Split updateRecord into intent-revealing operations

interface StackRecordAdapter {
  // replaces the Partial<StackRecord> grab-bag:
  patchContent(id, patch: Record<string, unknown | null>, opts?: { expectedVersion?: number }): Promise<StackRecord>;
  setPermissions(id, permissions: Permission[]): Promise<void>;
  restoreVersion(id, version: number): Promise<StackRecord>;   // was client-composed
  commitMigration(id, toTypeId: TypeId, content: Record<string, unknown>): Promise<StackRecord>; // was smuggled through updateRecord
  // deleteRecord, associate/dissociate etc. unchanged — already intent-shaped
}

Wire mapping becomes 1:1: patchContentPATCH /records/:id (raw patch travels, nulls intact); setPermissionsPUT /records/:id/permissions; restoreVersionPOST /records/:id/restore/:version; commitMigration → new endpoint (below).

2. The patch travels; merge happens where the write is serialized

Stack.update() still fetches + merges for client-side validation (it needs the merged result to validate against the schema), but hands the adapter the raw patch, not the merged content. Local adapters merge via a shared helper in core; the API adapter forwards the patch and the server merges against its own current state. Combined with server-assigned version/updatedAt (#48 §5), the response body is authoritative. This shrinks the lost-update window (a concurrent change to an untouched field survives) and makes null-deletion work identically everywhere.

3. New wire operation: migration commit

POST /records/:id/migrate with { toTypeId, content } (or equivalent) — the missing primitive that migrateAll() needs to work over the API adapter at all, and which #47's eager owner-driven migration makes load-bearing. Server snapshots before applying, same as PATCH.

4. Conformance fixtures so this can't drift again

This mismatch existed because nothing ever executes both sides of the wire contract. Add a shared conformance suite — request/response fixtures for every endpoint — run in this repo against APIAdapter (mocked transport) and importable by haverstack/server against its handlers. The spec stops being the only witness.

Impact

  • @haverstack/core: StackRecordAdapter contract change (breaking for adapter authors; Stack/StackClient public API unchanged). Shared merge helper.
  • adapter-api: rewrite of the write paths to the 1:1 mapping.
  • record-adapter-sqljs / future record-adapter-sqlite (Split SQLite support: native record-adapter-sqlite for Node, browser-only record-adapter-sqljs, and a StackTokenStore interface in core #46): implement the split operations (mostly mechanical — the SQL already special-cases each field).
  • Spec: PATCH section gains explicit "top-level record fields never appear in a PATCH body"; migrate endpoint added; permissions/restore endpoints noted as the only route for those intents.
  • haverstack/server: consumes the conformance fixtures; implements /migrate.

Open questions

  • Should commitMigration take the full migrated content (client-computed, server-validated) or the migration name (server runs a registered migration)? Client-computed matches RFC: replace lazy migration with explicit owner-driven migration + additive cross-app evolution #47's model where migration code lives in the owning app; server-side validation against the target schema is the backstop either way.
  • Batch/transactional variant for migrateAll() over HTTP (N records = N round trips otherwise) — possibly POST /records/migrate with a typeId filter, executed server-side. Can be deferred; single-record commit unblocks correctness first.

Refs #48 (expectedVersion lands on patchContent; server-authoritative counters), #47 (migration commit becomes a first-class operation), #46 (adapter-contract changes should land alongside the adapter split), #45 (the server topology that makes the wire contract the hot path).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions