Skip to content

fix(client): read the CRUD data prefix from discovery instead of restating /data - #16676

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14879-client-data-prefix
Sep 7, 2026
Merged

fix(client): read the CRUD data prefix from discovery instead of restating /data#16676
os-sales merged 2 commits into
mainfrom
claude/issue-14879-client-data-prefix

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #14879

crud.dataPrefix moves two things together: REST mounts every CRUD route under ${basePath}${crud.dataPrefix}, and the discovery handler advertises the same value as routes.data = ${realBase}${crud.dataPrefix}. The SDK is the third surface that has to describe those same paths, and its scoped half was not reading the value — it wrote /data into all seventeen of its data methods. On a deployment that moved the prefix, that half called paths the server does not mount.

The sharper framing is the one triage gave, and it survived measurement: the unscoped twin of every one of those methods was already correct. It builds ${baseUrl}${getRoute('data')}, and routes.data already carries the prefix. So this was not "the SDK does not read the key" — it was one SDK disagreeing with itself, the unscoped half reading the advertised value while the scoped half guessed.

What changed

  • _dataPrefix() recovers the prefix from the advertised routes. routes.data is one string carrying two unknowns ({realBase}{dataPrefix}) and no discovery key carries either half alone, so the split is recovered in two steps:
    1. A value that already ends with the conventional /data is the default prefix. Taking this first is what makes the change unable to regress any deployment that works today — every later rule can only run in the branch where the previous single-literal code was already wrong.
    2. Otherwise routes.metadata supplies the missing equation: it is {realBase}{metadata.prefix} over the same realBase (both are substituted from one realBase in the same discovery handler), so the two advertised routes share exactly realBase plus whatever their two prefixes share. Cutting that common run back to its last / lands on the boundary. This also covers a document served from the environment-scoped mount, where both routes carry the same /environments/{id} segment and it simply becomes part of the shared run.
  • It declines rather than guess. No advertised routes, no routes.metadata, or a derived prefix of / or empty, all return the conventional /data — byte-identical to the previous behaviour. This is the rule the neighbouring _apiBase() already sets in this file, which is why an unconnected client is unaffected.
  • dataUrl() on ScopedEnvironmentClient replaces the seventeen hard-coded call sites. The scoped form cannot consume routes.data verbatim the way the unscoped form does — the environment segment goes between the API base and the prefix, and the id is this client's, which need not be the one discovery resolved — so the two halves are taken separately.
  • _apiBase() strips the advertised prefix instead of the literal /data. It previously declined whenever the prefix was non-default, because /data was the only suffix it knew how to strip. Base and prefix now come from one rule and cannot disagree.

Every one of the seventeen rewritten paths was checked against the server: all seventeen resolve to a ${dataPath} mount in rest-server.ts, so all seventeen move with the key.

No new client option, which is the fence this card was graded against: the value is read from the server that already publishes it, so nothing is added to the published SDK surface.

Following the house precedent, not inventing one

The CLI closed this same defect one surface over (#7983) and the checklist records the method: resolve the mount "from the server: one memoised GET {apiBase}/discovery per run, addressing whatever routes.data advertises, with the RestApiConfigSchema + CrudEndpointsConfigSchema convention as the fallback — the @objectstack/client getRoute pattern, copied rather than re-invented". It was measured on three configs, and crud.dataPrefix: '/objects' "went from HTTP Error 404 to a created record". This change is that same pattern applied to the surface that still lacked it, and the fixture below uses the same /objects prefix that measurement used.

That note also records why api.apiPath is a different problem and stays open: apiPath "moves the base that discovery itself is mounted under, so the document that would name the new mount is behind the very prefix we are missing." This change does not attempt that row — it is exactly one of the shapes _dataPrefix() declines on.

Tests

packages/client/src/client.data-prefix.test.ts boots a real Hono server on a non-default prefix and drives the SDK against it. Triage's measured scope limit is that no in-repo caller sets a non-default dataPrefix, so nothing in this tree is broken today and no existing fixture exercises this — the suite therefore creates the condition rather than finding it.

A mock that answers 200 to whatever it is asked would go green against the very bug this pins, so the assertions are on both halves: that the URL the client puts on the wire is the one the server actually mounts, and that the old hard-coded path is genuinely dead on this deployment (serves nothing at /data asserts a 404 there), which is what makes the first assertion mean anything. The recording fetch delegates to the real one, so the recorded URL and the server's answer are the same exchange.

The positive control runs the same drive against a default-prefix server built by the same helper — it is what distinguishes "the SDK follows the advertised prefix" from "the SDK broke and now sends something else". A third control drives an unconnected client, pinning the decline leg.

验收备注

Two findings outside this card's face, both filed rather than fixed here:

One existing pin was re-pinned, deliberately — and it is why the first head was red

client.test.ts's [#6714] ... case B asserted the limitation this change removes: with a custom crud.dataPrefix, _apiBase() could not find the base and the scoped client fell back to the /api/v1 convention. That fallback was never right on such a deployment — it is a 404 — it was only honest, which is why it was pinned rather than fixed.

On the first pushed head (78aef4f6, the fix without this re-pin) that pin failed in CI, exactly as it had failed locally:

AssertionError: expected 'http://localhost:3000/backend/api/v9/…' to be 'http://localhost:3000/api/v1/environm…'
Expected: "http://localhost:3000/api/v1/environments/proj-123/meta"
Received: "http://localhost:3000/backend/api/v9/environments/proj-123/meta"
  ❯ src/client.test.ts:2142:52

The received value is the mounted path on that fixture's deployment, and the expected value is the 404 — so the assertion was inverted by the fix, not broken by it. That was the only genuine failure on that head: Test Files 1 failed | 34 passed (35), Tests 1 failed | 450 passed (451). The other six non-success check-runs were roll-up and fail-fast cascade — Test Core and TypeScript Type Check are aggregators, and the latter's log shows its three real lanes (typecheck-source-gates, typecheck-debt, typecheck-consumers) all concluded success while typecheck-workspace was cancelled, which is what the aggregator refused to call green.

The pin is now three cases:

  • B1 — the same fixture, asserting the path the server actually mounts. The base is derivable because routes.metadata supplies the second equation. Contract-first is intact: this reads a second advertised value, it does not loosen the parse of the first.
  • B2 — no routes.metadata at all, so {realBase}{dataPrefix} stays one string with two unknowns. Still declines to the convention, byte-identical.
  • B3 — a routes.metadata that shares no base with routes.data. The two share nothing but the leading /, which is not a shared base; deriving from it would mistake the whole of routes.data for the prefix. Declines.

B2 and B3 keep the decline leg the old case carried, on the shapes that are genuinely still underivable. The neighbouring case-A, case-C and scoped-discovery pins are untouched and pass.

Verification

Every reading below is named with the head it ran on, and every exit code was captured before any pipe.

reading head exit result
pnpm --filter @objectstack/client test 9778ca8a0f 0 Test Files 35 passed (35), Tests 453 passed (453)
pnpm --filter @objectstack/client exec tsc --noEmit 9778ca8a0f 0 clean
pnpm --filter @objectstack/client run check:test-typecheck 9778ca8a0f 0 test layer compiles, 0 debt entries
gate family reconciliation 9778ca8a0f 0 56 derived · 54 run green · 2 NOT-MEASURED · 0 UNRUN

Ablation. The new pin was ablated by restoring the hard-coded /data on the scoped surface. The mutation was proved to reach disk in both directions before the run was believed (derived=1 literal=0 before, derived=0 literal=1 after, mutated blob hash differing from the HEAD blob), and the restore proved by git diff HEAD empty plus a blob hash equal to HEAD's. The suite went 2 failed / 5 passed — and for the right reason: the client requested /api/v1/environments/proj-alpha/data/task and the server answered 404 ENDPOINT_NOT_FOUND, which is precisely the failure this card describes. The five still-green include all three controls, so the pin is not vacuously red. The test imports the client by relative source path (./index), not through a package exports to dist/, so no build sits between the mutation and the run.

The two NOT-MEASURED families are not gaps — CI measured both on a built tree:

  • pnpm check:dual-build-cjs-loads — locally exit 3, PREREQUISITE NOT MET: it reads built output and 35 packages have no dist/ in this worktree; it prints "⛔ This is NOT a pass: nothing was measured." CI runs it in Build Core, which builds first.
  • pnpm --filter @objectstack/spec run check:skill-examples — locally exit 1 for the same reason in a different shape: it refuses to read packages/client-react/dist rather than judge against a build that does not match src. It is a step of the typecheck-consumers job (lint.yml:5601, "runs after the build step"), whose check-run Type Check · consumer gates is completed/success on this head. Building two packages locally to re-measure what CI already measured on a fully built tree would buy nothing.

One more family is worth naming because its first reading was not a finding: pnpm check:type-check-debt first returned exit 3 — a V8 OOM under contention on this shared box, which is NOT MEASURED, not red. Re-run with --max-old-space-size=8192 it is exit 0, fully measured: "5 ledger entries re-measured, 55 raw tsc errors total, none above its recorded number."

The derived family count is 56, not the 47 derived before the changeset existed — adding .changeset/client-honours-data-prefix.md brought in six changeset families. The list was re-derived after the real changes landed rather than trusted from the first reading. One caveat stated plainly: dispatch-gates reports this branch as a stale tree (3 gate-source files — check-wildcard-fallthrough.mjs, engine-double-contract.pinned.json, import-prerequisite.mjs — changed on origin/main since the branch point), so the local readings used this branch's copies of them. My diff touches none of those; CI runs the merge result, and it is green.

Documentation

No docs change, considered and declined. content/docs/api/environment-routing.mdx names the /api/v1/environments/:environmentId/data/:object anchor this change makes derived rather than literal, but the page is not falsified: the default crud.dataPrefix is still /data, and on a default deployment the SDK's output is byte-identical — which is what the positive control and the untouched case-A/case-C pins assert. The page's subject is the scope segment and projectResolution, not the data prefix, and it never mentions crud.dataPrefix, so it makes no claim to correct. Searched more widely: no doc asserts that the SDK hard-codes or ignores the key; the only prose about a client honouring it is content/docs/deployment/cli.mdx, describing the CLI's own #7983 fix on a different surface, which stays true. Both docs-drift gates (scripts/docs-audit/check-affected-docs.mjs, check-drift-comment.mjs) are exit 0 on this change set. Documenting the non-default-prefix client story is a real but separate change — it adjoins the client-option branch triage fenced off — and content/docs/releases/ is release-owned in any case; this PR's input there is its changeset.


Generated by Claude Code

…ating /data

`crud.dataPrefix` moves the mounted CRUD paths and the advertised discovery
document together, and the SDK is the third surface that has to describe those
same paths. Its scoped half was not reading the value -- it wrote `/data` into
all seventeen of its data methods -- so a deployment on a non-default prefix had
that half calling paths the server does not mount, while the unscoped half of
the SAME SDK called the right ones (it builds `${baseUrl}${getRoute('data')}`,
and `routes.data` already carries the prefix). One SDK disagreed with itself.

`_dataPrefix()` recovers the prefix from the advertised routes: a value already
ending in the conventional `/data` IS the default prefix -- taken first, so no
deployment that works today can regress -- and otherwise `routes.metadata`
supplies the missing equation, being `{realBase}{metadata.prefix}` over the
same base. Where the document does not determine the split it declines to
`/data`, byte-identical to today, following the rule `_apiBase()` already sets
in this file. `_apiBase()` now strips that derived prefix rather than the
literal `/data`, so base and prefix come from one rule and cannot disagree.

No new client option: the value is read from the server that publishes it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/client, touching 8 documentable anchor(s).

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/api/environment-routing.mdx (via ObjectStackClient (symbol, a top-level class), /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/api/wire-format.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/concepts/north-star.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/deployment/publish-and-preview.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/deployment/single-project-mode.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/kernel/runtime-services/data-service.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/kernel/runtime-services/storage-service.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/protocol/kernel/http-protocol.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/protocol/kernel/metadata-service.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/protocol/kernel/realtime-protocol.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/ui/forms.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via /environments/{id} (route, a path literal in ObjectStackClient), /environments/{id}{dataPrefix} (route, a path literal in ScopedEnvironmentClient))
  • content/docs/releases/v17.mdx (via ObjectStackClient (symbol, a top-level class), ScopedEnvironmentClient (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1 (route, 82 pages)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2539f4aec60e327f5a9ff83e6d07f16054d876eepackageMentionDocs.

Which tree this was computed on

This run read content/docs from 22686afcad5e53602287a91b27260b5de4cd1e1a — the merge of head 9778ca8a0ffeacb3a2037bacabf55a3ab72c858f into base 2539f4aec60e327f5a9ff83e6d07f16054d876ee, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 22686afcad5e53602287a91b27260b5de4cd1e1a && git checkout 22686afcad5e53602287a91b27260b5de4cd1e1a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2539f4aec60e327f5a9ff83e6d07f16054d876ee 9778ca8a0ffeacb3a2037bacabf55a3ab72c858f && git checkout -B drift-repro 2539f4aec60e327f5a9ff83e6d07f16054d876ee && git merge --no-ff 9778ca8a0ffeacb3a2037bacabf55a3ab72c858f

node scripts/docs-audit/affected-docs.mjs --json 2539f4aec60e327f5a9ff83e6d07f16054d876ee

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2539f4aec60e327f5a9ff83e6d07f16054d876ee → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…es the base underivable

The case-B pin asserted the limitation this change removes: with a custom
`crud.dataPrefix`, `_apiBase()` could not find the base, so the scoped client
fell back to the `/api/v1` convention. That fallback was never RIGHT on such a
deployment -- it is a 404 -- it was only honest, which is why it was pinned
rather than fixed.

`_dataPrefix()` now recovers the split from a second advertised value rather
than a looser parse of the first, so the base IS derivable here and B1 pins the
path the server actually mounts. The decline leg the old case carried is kept,
on the two shapes that are genuinely still underivable: no `routes.metadata` at
all (B2), and a `routes.metadata` that shares no base with `routes.data` and so
supplies no equation (B3).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
@os-sales
os-sales marked this pull request as ready for review September 7, 2026 19:04
@os-sales
os-sales enabled auto-merge September 7, 2026 19:04
@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit cf74a11 Sep 7, 2026
39 of 41 checks passed
@os-sales
os-sales deleted the claude/issue-14879-client-data-prefix branch September 7, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants