fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate link/links beside data (#4038) - #4049
Merged
Conversation
…plicate link/links beside data (#4038) The producer-side other half of #3983. That PR moved the sharing plugin's routes onto the declared envelope; this removes the compatibility shim the dispatcher twin had been carrying BECAUSE that surface answered bare. Create and list answered with the payload under two keys — `{ success: true, data: link, link }` and `{ success: true, data: links, links }`. The duplicate existed so readers predating the envelope kept working, which is why objectui's ShareDialog reads `body.links ?? body.data`. Once #3983 made both surfaces answer `data`, that first branch had no producer left and the duplicate had no reader in any repo: framework no consumer of these routes at all objectui ShareDialog already falls through to body.data cloud swept — it only REGISTERS SharingServicePlugin into per-environment kernels with registerShareLinkRoutes:false so this dispatcher serves the paths; it never calls them and never reads a body That cloud sweep is what #4038 was waiting on, and it came back clean. `data` is unchanged on both routes — only the duplicate key is gone, so anything reading `body.data` or going through `unwrapResponse` sees no difference. The list route now routes through `deps.success(...)` like the domain's other three. Create stays hand-built because `deps.success` hardcodes status 200 and this route is a 201 — the same reason `/keys` hand-builds its own 201, and the same shape it uses. (My own issue text said to route BOTH through the helper; that was wrong about the status.) `scripts/check-route-envelope.mjs` does not and cannot cover this file — it scans route modules that write via `res.json(...)`, while dispatcher domains return `{ status, body }` for a central sender, so the drift was invisible to it by construction. Three tests in domain-handler-registry.test.ts cover it instead: two per-route, plus a general one asserting no success body carries a top-level key outside success/data/meta. Restoring the duplicates fails all three. Not touching objectui's `body.links ?? body.data`: with both producers converged that is version skew, not drift, and dropping it would turn "old framework" from working into a silently empty share list. Deleting it needs a version gate, which is a larger call than this issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Contributor
Author
|
Two notes from CI on this PR. The Check Changeset failure is infrastructure, not the changeset. The job died before it ran anything: The docs-drift advisory (18 files) needs no doc change, but one of them corroborates the premise. I checked the ones that could plausibly document these bodies — const link = await client.shareLinks.create('crm_account', recordId, { … });
await client.shareLinks.revoke(link.token);That documented So the doc needed no edit — but it was independent evidence that Generated by Claude Code |
os-zhuang
marked this pull request as ready for review
July 30, 2026 07:09
This was referenced Jul 30, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…ns (#4038 follow-up) (#4056) The platform answers REST from two kinds of file. Route modules (*-routes.ts) call `res.json(...)`, which this scan counted. Dispatcher domains (runtime/src/domains/*.ts) RETURN `{ status, body }` for a central sender, so they never call `res.json` and were invisible to it by construction. That gap cost something real: /share-links emitted its payload under both `data` and a legacy `link`/`links` for as long as nothing looked, and it surfaced only because #3983's consumer sweep happened to walk past it (#4038, fixed in #4049). I noted the blind spot there; this closes it. The new table counts hand-built `response: { … }` literals per domain. A domain answering only through the deps.* helpers hand-builds zero and cannot drift; any hand-built one now needs a declared count plus a `note` saying why. Hand-building is not automatically wrong — four kinds show up and only the last is drift: 1. enveloped, but the helper cannot express it — deps.success hardcodes 200 so a 201 is assembled by hand, and deps.error carries no headers so a 405 with `Allow:` is too (keys.ts, share-links.ts, one in mcp.ts) 2. passthrough of a body this dispatcher does not own (mcp.ts, ai.ts) 3. a foreign wire format a client library requires — /auth answers better-auth's shapes because better-auth's client parses them (auth.ts x4) 4. drift 16 domains audited: 11 helper-only, 5 declaring hand-built responses, 1 ratcheted. The ratchet is ai.ts -> #4053, where GET /ai/agents is SDK-addressable but unenveloped and the SDK compensates by reading `.agents` off the raw body — converting it without changing cloud and the SDK together makes `client.ai.agents.list()` return an empty list silently, which is indistinguishable from the legitimate "no AI service configured" state. That warning now fails CI for whoever tries it, instead of sitting in an issue nobody reads. Five self-test assertions cover the new scan. One caught a real false positive while it was being written: matching the `response` key alone counted a PAYLOAD field named response (`deps.success({ response: … })`) as a hand-built response. It now requires `response` to be a sibling of `handled`, which is what makes the pair an HttpDispatcherResult rather than just an object with that key. Verified the guard actually bites: adding a hand-built response to a helper-only domain fails with the site and the four-kind classification prompt. Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z Co-authored-by: Claude <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…hema cannot express (#4090) * test(spec): envelopeViolations — the conformance check BaseResponseSchema cannot express Every conformance suite from the #3843 line leads with `BaseResponseSchema.safeParse(body)` under a comment calling it "the contract itself, imported — not a restatement of it". That overclaimed, and the gap is demonstrable: BaseResponseSchema.safeParse({ success: true }) // passes BaseResponseSchema.safeParse({ success: true, data: link, link }) // passes The schema declares no `data` (each response type adds its own via `.extend({ data })`) and a plain z.object strips unknown keys rather than rejecting them. So it catches the one drift it was added for — a missing or non-boolean `success`, the flag `unwrapResponse` keys on — and nothing else. The second body is exactly the duplicate-payload drift /share-links shipped until #4038/#4049 removed it, and safeParse passed it the whole time. What was actually catching that drift was the assertions each suite hand-wrote beside it, which works only for as long as whoever writes the next suite remembers to. `envelopeViolations(body)` returns every departure as readable reasons, empty when conformant: `success` must be a boolean; a success body must carry `data` (`undefined` only — null/[]/{}/0/'' are payloads, not absences); a failure body must carry `error` with a string code and message; and no top-level key outside success/data/error/meta, which is the general form of the duplicate-payload drift. It deliberately does NOT check the shape of `data` — that is each route's own payload schema, and conflating the two is what let SettingsNamespacePayload describe a whole body before #3843 and only `data` after it. Ten suites now assert it beside safeParse, and their comments say what each of the two actually proves. Verified the pairing is load-bearing: reintroducing the /share-links duplicate key fails the new assertion and still passes the old one. Verified the rules are not too strict by running every existing suite unchanged — spec 6951, rest 505, runtime 914, storage 220, settings 189, datasource 154, i18n 62, sharing 225, client 200, all green, nothing flagged. Placed in contract.zod.ts beside the schema it completes, alongside the other plain predicates spec/api already exports (standardErrorCodeForHttpStatus, readServiceSelfInfo). No new package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z * chore(spec): regenerate the API-surface snapshot for envelopeViolations `check:api-surface` is a ratchet on `@objectstack/spec`'s public exports, and the new predicate is an addition to it: ./api + envelopeViolations (function) 0 breaking (removed/narrowed), 1 added. One line, alphabetically beside the other predicates that module already exports. Two things this caught that are worth writing down. The gate lives in the TypeScript Type Check job, which runs TEN steps — I had found four of them by grepping a window of lint.yml that cut off before the rest, and ran only those. The full list is: spec tsc, check:docs, check:skill-refs, check:react-blocks, a workspace build, examples typecheck, downstream-contract typecheck, check:api-surface, check:skill-examples, and the two i18n checks. All ten pass locally now. And `gen:api-surface` reads `dist/index.d.ts`, so it fails outright under `OS_SKIP_DTS=1` — the flag I had been using to keep local builds fast. Rebuilding spec with declarations is a prerequisite for regenerating the snapshot, not an optional speed-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z --------- Co-authored-by: Claude <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.
Closes #4038. The producer-side other half of #3983 — and the cloud sweep that issue was waiting on came back clean, so this is smaller than filed.
What the shim was
Create and list answered with the payload under two keys:
That duplicate existed so readers predating the envelope kept working — which is exactly why objectui's
ShareDialogreadsbody.links ?? body.data. It stayed alive because the other surface for these same paths (the sharing plugin's routes) answered bare, so consumers genuinely needed both.#3983 moved that surface onto
data. The first branch of every??then had no producer left, and the duplicate had no reader anywhere.The sweep #4038 was blocked on
ShareDialogalready falls through tobody.data; nothing else reads themcloudwas the blocker, and I could reach it after all. It touches share-links in exactly two places (objectos-runtime/src/artifact-kernel-factory.ts,service-cloud/src/default-environment-plugins.ts), and both only registerSharingServicePlugininto a per-environment kernel withregisterShareLinkRoutes: false— precisely so this dispatcher serves the paths. Zero calls to the endpoints, zero body reads, zerobody.link/body.linksanywhere in the repo.Shape
POST /share-links{ success, data: link, link }{ success, data: link }GET /share-links{ success, data: links, links }{ success, data: links }datais unchanged on both — only the duplicate key is gone. Anything readingbody.data, or going throughObjectStackClient.unwrapResponse, sees no difference at all. Only a raw reader of top-levelbody.link/body.linkswould move, and there isn't one.The list route now goes through
deps.success(...)like the domain's other three. Create stays hand-built —deps.successhardcodes status 200 and this route is a 201, which is the same reason/keyshand-builds its own 201, and the same shape it uses. My own issue text said to route both through the helper; that was wrong about the status, and I've corrected it on the issue.Guard
scripts/check-route-envelope.mjsdoes not and cannot cover this file — it scans route modules that write viares.json(...), while dispatcher domains return{ status, body }for a central sender. So this drift was invisible to that guard by construction, which is worth stating plainly: the guard's 7/0/1 is a claim about route modules, not about every REST body the platform emits.Three tests in
domain-handler-registry.test.tscover it instead — two per-route, plus a general one asserting no success body carries a top-level key outsidesuccess/data/meta, so a new duplicate spelling fails too. Restoring the duplicates fails all three.What I deliberately did not do
#4038's step 3 said to delete objectui's
body.links ?? body.datatolerance, and I'm declining that — correcting my own recommendation.With both producers converged, that
??is no longer drift (two live producers disagreeing, which was the actual bug); it's version skew (one producer changed, the consumer still supports the older one).@object-ui/componentspublishes standalone against peer deps, so dropping it turns "new console, older framework" from working into a silently empty share list — a bad failure mode to trade for removing one??. Deleting it wants a version gate, which is a larger call than this issue. Noted on #4038 rather than left implicit.Verification
@objectstack/runtime@objectstack/clientcheck:route-envelope+--self-testtsc --noEmit·check:docs·check:skill-refs·check:react-blockscheck:error-code-casing·check:console-sha·check:nul-bytes·check:role-word·check:org-identifiereslint --no-inline-configon changed filesNo
.objectui-shabump needed — this touches no frontend, so the console pin stays where #4037 left it (check:console-shaconfirms it is still in sync).Generated by Claude Code