fix(hono,plugin-hono-server,runtime): one CORS source and one registry key — the last derivable copies from the #3786 sweep - #4282
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
os-zhuang
force-pushed
the
claude/spec-checklist-drift-investigation-ewgd6l
branch
from
July 31, 2026 03:07
0409afb to
f456ee6
Compare
github-actions
Bot
added
documentation
os-zhuang
changed the title
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…y key — the last derivable copies from the #3786 sweep Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`, one level deep, so it missed everything under `packages/plugins/` and `packages/adapters/` — "the sweep is basically clean" was based on an incomplete scan. A STALE CORS DEFAULT, on the one description callers actually read. `HonoCorsOptions.allowHeaders`' TSDoc promised `['Content-Type', 'Authorization', 'X-Requested-With']` "sufficient for cookie and bearer-token auth". The real default carries three more: `X-Tenant-ID`, `X-Environment-Id` (multi-tenant routing) and `If-Match` (OCC token on record PATCHes, objectui#2572). Sizing a custom allowHeaders against that sentence drops all three and breaks every cross-origin save. Three Hono CORS sites each kept their own copy under "keep in sync" comments. The copies agreed; the DOC — the only description with no counterpart to be diffed against, and the only one a caller reads — is what drifted. Both defaults are now single constants exported from plugin-hono-server and imported by the adapter (which already depends on it). The TSDoc links them instead of restating, and documents the asymmetry it never mentioned: allowHeaders REPLACES the default, exposeHeaders MERGES. hono-plugin.test.ts stopped stubbing ./adapter wholesale and keeps the real constants via importOriginal — it asserts exact header lists, so a mocked copy would make the test agree with itself rather than with what ships. Verified: removing `If-Match` fails `should allow If-Match by default`, by name. A HAND-COPIED REGISTRY KEY. runtime's share-links domain resolved 'shareLinks' as a literal, copied from SHARE_LINK_SERVICE — whose own comment says "keep in sync with the SharingPlugin registration". Now imports the constant. A drifted copy resolves nothing, so every share link 501s "Sharing is not configured for this environment" on an environment where it is. PLUS A DUPLICATE LEDGER ENTRY, the same defect one level up: check-generated.ts carried two NO_GENERATOR entries for check:strictness-ledger, because #4203 and #4252 each added one without seeing the other. Harmless (the ledger is read into a Set) but two comments told overlapping versions of one story. #4203's is kept — the fuller account, from the PR that fixed the underlying problem. NOT included, because main already has it: wiring the reconciliation into CI. #4203 landed it as `check:generated --reconcile-only` in lint.yml's unfiltered job, which is strictly better than what this branch first proposed — it runs only the reconciliation rather than re-running all eight artifact gates that already have their own steps, and it sits in an unfiltered required job so it cannot go dormant behind a paths filter. Checked and left alone: ApprovalStatus (5 values) and ApprovalActionKind (12 values) vs their plugin-approvals selects — diffed verbatim, no drift today, still hand-copied across a package boundary. Verified: 28/28 turbo test tasks green (plugin-hono-server 149, hono 73, runtime 67 files), affected builds clean, check:generated --reconcile-only PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
os-zhuang
force-pushed
the
claude/spec-checklist-drift-investigation-ewgd6l
branch
from
July 31, 2026 03:10
f456ee6 to
5889189
Compare
os-zhuang
marked this pull request as ready for review
July 31, 2026 03:30
os-zhuang
pushed a commit
that referenced
this pull request
Jul 31, 2026
The docs-drift check turned up a FOURTH copy of the status set, in the highest- visibility place: `automation/approvals.mdx`'s "Statuses in full" line. Unlike the CORS case in #4282 this one is correct today — all five values, same order — so there is nothing to fix, only something unguarded. Points at APPROVAL_STATUSES and says plainly that this line is a copy nothing updates automatically. That is weaker than derivation and I am not pretending otherwise: prose docs legitimately restate values, and a path-coupled test asserting this exact sentence would be brittle. Recorded as a follow-up instead — `check:variant-docs` already solves this shape for schema variants and is the right place to extend if the set starts moving. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…, not hand-matched (#3786) (#4295) * fix(spec,plugin-approvals): the two approval vocabularies are derived, not hand-matched (#3786) `sys_approval_request.status` and `sys_approval_action.action` spelled their option lists out — five values and twelve — each under a "Keep in sync with `ApprovalStatus` / `ApprovalActionKind` (spec/contracts)" comment, while the contract held the same sets as bare type unions. Seventeen strings matched by hand across a package boundary with nothing checking them. They did all still agree — verified verbatim before changing anything. But agreeing is not being held, and both directions of drift are quiet: a value the COLUMN accepts and the contract omits is invisible to every consumer typed against the contract (the row exists and nothing can narrow it), while a value the CONTRACT declares and the column rejects surfaces only at write time, on whichever tenant first reaches that transition. An audit vocabulary is a bad place for either. The contract now publishes the lists as values — APPROVAL_STATUSES and APPROVAL_ACTION_KINDS — with ApprovalStatus / ApprovalActionKind derived via `(typeof X)[number]`, and both columns spread the constants. The per-entry rationale (which action kinds move the flow, which are thread-only, why `returned` differs from `recalled`) moved onto the constants, where the values live. New exports, no behaviour change: the emitted option lists are byte-identical, checked against the built artifact before and after. Existing imports of the two types are unaffected — they resolve to the same unions. approval-vocabularies.test.ts pins the qualifier derivation alone cannot: the columns agree with the contract WHILE THE SPREAD IS THERE, so the test fails if either is re-inlined as a literal that drifted. It guards the guard (an unresolvable import would compare two empty lists and pass) and asserts the two vocabularies stay distinct — a copy-paste pointing one column at the other constant would satisfy "derived from the contract" while being the wrong vocabulary entirely. Mutation-tested in both directions: adding a value to APPROVAL_STATUSES propagates into the BUILT sys_approval_request.status options (the derivation is live, not a stale build), and re-inlining a drifted literal fails `sys_approval_request.status offers exactly the contract statuses, in order`. Verified: plugin-approvals 330 tests green, spec 7169 green, api-surface regenerated (0 breaking, 2 added), check:generated / exported-any / liveness / docs all PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb * docs(automation): name the source of the approval status list (#3786) The docs-drift check turned up a FOURTH copy of the status set, in the highest- visibility place: `automation/approvals.mdx`'s "Statuses in full" line. Unlike the CORS case in #4282 this one is correct today — all five values, same order — so there is nothing to fix, only something unguarded. Points at APPROVAL_STATUSES and says plainly that this line is a copy nothing updates automatically. That is weaker than derivation and I am not pretending otherwise: prose docs legitimately restate values, and a path-coupled test asserting this exact sentence would be brittle. Recorded as a follow-up instead — `check:variant-docs` already solves this shape for schema variants and is the right place to extend if the set starts moving. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb --------- Co-authored-by: Claude <support@objectstack.ai> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…ugin is a transport adapter (#4073) (#4296) Completes the retirement. HonoServerPlugin now owns the socket, the middleware and the three current-user endpoints, and nothing else. The data and discovery APIs have one owner each — @objectstack/rest and the runtime dispatcher (ADR-0076 D11). Net −678 lines. Deleted: raw C+R `/api/v1/data/:object` (+ `/:id`), this plugin's third `/api/v1/discovery` + `/.well-known/objectstack` payload, and the `registerStandardEndpoints` flag itself — gone, not defaulted off, with a test pinning that passing the removed option resurrects nothing. Unaffected: `/auth/me/*` + `/me/apps` (sole supply, unconditional since #4144), and every composed host — `os serve`, `objectstack dev`, cloud's objectos — which mount an owner that already answered these routes byte-identically either way (#4260). A host with NO owner keeps the boot warn, reworded to name both remedies now that opting in is not one of them. Tests decided on what each was actually pinning: `hono-discovery` and `hono-anonymous-deny` deleted (wholly about the retired routes); `server-timing-e2e` KEPT and repointed at `/auth/me/permissions` (#3361's value is "perf middleware + a handler that resolves the principal", not the route); `client.hono` KEPT and repointed at a REST-composed host (it is the only coverage of `client.connect()` and the discovery handshake); the default-pin suite rewritten as `hono-transport-only`. Three gates caught bookkeeping the deletion owed, each fixed on its own terms: - the #2567 anonymous-deny ledger flagged STALE covers. The row went with its surface, and its id came out of HIGH_RISK — but the ratchet's own negative test used that row as its FIXTURE, so it was repointed at a live row rather than left to pass vacuously and disarm the check that just caught this. - the docs drift check surfaced `authorization.mdx`, which documented the deleted surface twice as part of a SECURITY invariant. Both now read as history, matching how the same sentence already handles the retired GraphQL endpoint: the surface LEFT the matrix; the invariant is unchanged and simply has one fewer door to hold shut. - a merge conflict with #4282's CORS work: both intents kept. AGENTS.md gains the four rules this thread paid for — one route one owner, explicit composition over default magic, absence must be loud, machine-readable surfaces must not lie — plus the verification note that "who serves this path" is a question about the composed, PROVISIONED runtime. #4073 was answered wrongly three times, once per shortcut around that. Verified: plugin-hono-server 135/135, dogfood 430 passed, client 12/12, runtime 19/19, http-conformance 46/46, full build 71/71, CI 16/16. (Temporal Conformance timed out once on a live-service step touching no package in this diff; it passed on re-run.)
This was referenced Jul 31, 2026
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
1 participant
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.
First, two corrections
The sweep report was based on an incomplete scan. I said the #3786 sweep was "basically clean". The scan globbed
packages/*/src, which is one level deep, so it missed everything underpackages/plugins/andpackages/adapters/. Re-running across all 72 packages found the instances below.And the CI wiring I proposed is already on
main, in a better form. #4203 landedcheck:generated --reconcile-onlyinlint.yml's unfiltered job. That is strictly better than what this branch first pushed: it runs only the reconciliation instead of re-running all eight artifact gates that already have their own steps, and it sits in an unfiltered required job so it cannot go dormant behind a paths filter (#4203's comment notesci.yml'sgeneratedfilter doesn't watchpackage.json— the one file every offending PR must touch). That change has been dropped from this PR.The drift: a CORS default, on the one description callers actually read
HonoCorsOptions.allowHeaders' TSDoc promised:The real default carries three more:
X-Tenant-IDandX-Environment-Id(multi-tenant routing) andIf-Match(the OCC token on record PATCHes, objectui#2572). Anyone sizing a customallowHeadersagainst that sentence drops all three, and every cross-origin save fails in the browser with "Failed to fetch".The instructive part: three Hono CORS sites each carried their own copy of the defaults under "keep in sync" comments, and the copies all agreed. What drifted was the doc — the only description with no counterpart to be diffed against, and the only one a caller reads.
Both defaults are now single constants,
DEFAULT_CORS_ALLOW_HEADERS/DEFAULT_CORS_EXPOSE_HEADERS, exported from@objectstack/plugin-hono-serverand imported by the adapter (which already depends on it — no new edge, no cycle). The TSDoc links them rather than restating them, and documents an asymmetry it never mentioned:allowHeadersreplaces the default,exposeHeadersmerges with it.hono-plugin.test.tsstopped stubbing./adapterwholesale and keeps the real constants viaimportOriginal. It asserts exact header lists, so a mocked copy would have made the test agree with itself instead of with what ships.A hand-copied service-registry key
runtime's share-links domain resolved'shareLinks'as a string literal — copied fromSHARE_LINK_SERVICE, whose own doc-comment says "keep in sync with the SharingPlugin registration". It now imports the constant. A drifted copy resolves nothing, so every share link answers501 Sharing is not configured for this environmenton an environment where it is configured perfectly well.A duplicate ledger entry — the same defect one level up
check-generated.tscarried twoNO_GENERATORentries forcheck:strictness-ledger, because #4203 and #4252 each added one without seeing the other. Functionally harmless (the ledger is read into aSet), but two comments told overlapping versions of one story. #4203's is kept — the fuller account, from the PR that fixed the underlying problem.Checked and left alone
ApprovalStatus(5 values) andApprovalActionKind(12 values) versus theirplugin-approvalsselects: diffed verbatim, no drift. Both remain hand-copied across a package boundary under "keep in sync" comments — real derivation candidates, but correct today and a larger change than this PR's scope.Verification
If-MatchfromDEFAULT_CORS_ALLOW_HEADERSshould allow If-Match by defaultfails, by nametest --forceacross the touched packages — 28/28 tasks green (plugin-hono-server 155 tests, hono 73, runtime 67 files)check:generated --reconcile-only— PASS (16check:+ 10gen:scripts, all classified)Note for reviewers: two build failures hit while verifying (
service-messaging→@objectstack/platform-objects/audit,service-i18n→@objectstack/types) were both stale worktree installs aftermainadvanced, fixed bypnpm install. Neither is a code issue.