test(runtime): e2e regression for notifications mark-read on the hono server (#3362)#3388
Merged
Merged
Conversation
… server (#3362) #3362 reported that `POST /api/v1/notifications/read[/all]` and `GET /api/v1/notifications` 404 on the standalone `os dev` / `os serve` hono server, so the unread badge never clears — claiming #3354's route mounts never reached the hono listener. This does NOT reproduce on current main. The dispatcher plugin's `server.<verb>()` registrations DO land on the hono `IHttpServer` (the same path the already-covered `/ready` probe takes), the `notification` service resolves (backed by @objectstack/service-messaging), and mark-read persists — verified end-to-end against the real CRM and showcase `os dev` servers (unread count drops 1 -> 0, receipts flip to `read`). The /mcp 501 the issue cites is likewise the dispatcher's own handler, not a "hono-native" one. The most likely cause of the original report is a stale `@objectstack/runtime` dist built before #3354. The real gap the issue exposes is COVERAGE: the existing guard only asserts route *registration* on a FAKE server (dispatcher-plugin.routes.test.ts), so it cannot catch a real unmounting on hono, a service-resolution break, or a receipt-persistence regression. This adds the e2e test the issue asks for — it boots the actual stack (ObjectQL + messaging + hono + dispatcher), opens a real socket, delivers notifications, and drives mark-read over `fetch`, asserting the `sys_notification_receipt` rows flip to `read` and the unread count drops. Disabling the route mounts makes it fail (404 vs 401/200), so it is a real guard, not a vacuous one. Adds @objectstack/service-messaging as a runtime devDependency (test-only; messaging does not depend on runtime, so no dependency cycle). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QES3gEM1DdBkaK5maR4m57
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 17 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
The e2e regression test in this PR is test-only (plus a test-scoped devDependency) and changes no runtime code, so it releases nothing. Add the sanctioned empty-frontmatter changeset to satisfy the Check Changeset gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QES3gEM1DdBkaK5maR4m57
os-zhuang
marked this pull request as ready for review
July 21, 2026 14:23
os-zhuang
pushed a commit
that referenced
this pull request
Jul 21, 2026
… discovery (#3369) Rebased onto main after the #3361 Server-Timing fix landed independently (#3384) and the notifications mark-read e2e landed (#3388). This keeps only the net-new route-parity work; the redundant #3361 fix is dropped in favour of main's canonical version. - plugin-hono-server: make the STANDALONE discovery honest. The static `registerDiscoveryAndCrudEndpoints` list advertised metadata/packages/ analytics/workflow/automation/ai/notifications/i18n/storage/ui — none of which HonoServerPlugin mounts on its own — so a truly standalone deployment advertised a dozen routes that 404 (the literal `declared !== enforced` gap #3369 cites). It now advertises only what it serves (`/data` + `/auth/me/*`). Shadowed by the dispatcher/REST service-aware discovery under `os serve`, so real deployments are unaffected. - runtime: add the route-parity e2e gate. Boots the real hono app the way `os serve` mounts it (hono server + dispatcher plugin, services provisioned), reads `/api/v1/discovery`, and asserts every advertised/dispatcher route is reachable (never 404/405/501) for anonymous AND admin principals, and that discovery is service-aware in both directions (no dead advertisement). Closes the class of "works in the dispatcher, dead on os serve" bugs (#3361/#3362/ MCP 501) with one boot-the-real-server test. Server-Timing and notifications mark-read specifics are covered by #3384 / #3388; this gate does not duplicate them. - http-dispatcher: document why `routes.mcp` is advertised on the `isMcpServerEnabled()` flag (the #2698 auto-load lockstep) rather than gated on service presence — comment only, keeps the dispatcher symmetric with @objectstack/rest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015si15Q1KWMKpVQWYsEvgcS
os-zhuang
pushed a commit
that referenced
this pull request
Jul 21, 2026
…3369) Adds the "boot-the-real-server" route-parity gate #3369 calls for, rebased onto main after the #3361 Server-Timing fix (#3384) and the notifications mark-read e2e (#3388) landed independently. Keeps only the net-new, non-overlapping work. - runtime: route-parity e2e gate (`route-parity.integration.test.ts`). Boots the real hono app the way `os serve` mounts it (hono server + dispatcher plugin, services provisioned), reads `/api/v1/discovery`, and asserts every advertised / dispatcher-registered route is reachable (never 404/405/501) for an anonymous AND an admin principal, and that discovery is service-aware in both directions (no dead advertisement). Closes the "works in the dispatcher, dead on os serve" class (#3361/#3362/MCP 501) with one test. Does not duplicate the Server-Timing (#3384) / notifications mark-read (#3388) / REST `/data`·`/meta`·`/ui` (@objectstack/client) coverage. - http-dispatcher: document why `routes.mcp` is advertised on the `isMcpServerEnabled()` flag (the #2698 auto-load lockstep) rather than gated on service presence — comment only, keeps the dispatcher symmetric with @objectstack/rest. Test + comment only; no runtime code changes (no-release changeset). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015si15Q1KWMKpVQWYsEvgcS
os-zhuang
added a commit
that referenced
this pull request
Jul 21, 2026
…3369) (#3386) Adds the "boot-the-real-server" route-parity gate #3369 calls for, rebased onto main after the #3361 Server-Timing fix (#3384) and the notifications mark-read e2e (#3388) landed independently. Keeps only the net-new, non-overlapping work. - runtime: route-parity e2e gate (`route-parity.integration.test.ts`). Boots the real hono app the way `os serve` mounts it (hono server + dispatcher plugin, services provisioned), reads `/api/v1/discovery`, and asserts every advertised / dispatcher-registered route is reachable (never 404/405/501) for an anonymous AND an admin principal, and that discovery is service-aware in both directions (no dead advertisement). Closes the "works in the dispatcher, dead on os serve" class (#3361/#3362/MCP 501) with one test. Does not duplicate the Server-Timing (#3384) / notifications mark-read (#3388) / REST `/data`·`/meta`·`/ui` (@objectstack/client) coverage. - http-dispatcher: document why `routes.mcp` is advertised on the `isMcpServerEnabled()` flag (the #2698 auto-load lockstep) rather than gated on service presence — comment only, keeps the dispatcher symmetric with @objectstack/rest. Test + comment only; no runtime code changes (no-release changeset). Claude-Session: https://claude.ai/code/session_015si15Q1KWMKpVQWYsEvgcS 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
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 #3362.
TL;DR
#3362 does not reproduce on current
main. The in-app notifications surface works end-to-end on the standaloneos dev/os serve(hono) server — routes are mounted, the service resolves, and mark-read persists. #3354's fix is effective. What was missing is a test that actually proves it, so this PR adds the end-to-end regression test the issue asked for.What the issue claimed vs. what actually happens
The issue reported that
GET /api/v1/notificationsandPOST /api/v1/notifications/read[/all]return 404 on the hono server, blaming the dispatcher-plugin's route registrations for "not landing on the hono app."I verified empirically by building
mainand booting the real CLI (objectstack dev --fresh) against both the CRM and showcase examples:os dev(hono)GET /api/v1/notifications(unauth)GET /api/v1/notifications(auth){ notifications, unreadCount }POST /api/v1/notifications/read/all(auth){ success, readCount }services.notificationstatus: "available"(declared === enforced)readSupporting evidence that the dispatcher's routes do bind to the hono listener:
/ready(dispatcher-only route) → 200, already covered bydispatcher-plugin.ready.integration.test.ts./keys(dispatcher-only) → 401 (reaches the handler)./mcp→ the dispatcher's ownhandleMcp501 ("MCP server is not available"), not a "hono-native handler" — so the premise that dispatcher registrations don't reach hono is incorrect.The notification routes were added by #3354 (
ee0a499) and have not been touched since, somainreflects post-#3354 behavior. The most likely cause of the original 404 is a stale@objectstack/runtimedist (a common monorepo gotcha:os devloadspackages/runtime/dist, which must be rebuilt after pulling source changes).The real gap: coverage
The only prior guard (
dispatcher-plugin.routes.test.ts) asserts route registration against a fake server. It cannot catch a real unmounting on hono, anotification-service resolution break, or a receipt-persistence regression — exactly the failure modes the issue feared.Change
Adds
packages/runtime/src/notifications.hono.integration.test.ts— a real end-to-end integration test (same style as the existing/readyintegration test):ObjectQLPlugin+MessagingServicePlugin+ a minimalauthservice +HonoServerPlugin+createDispatcherPlugin, on a real socket.fetchexactly like the Console bell.POST /readandPOST /read/allflip thesys_notification_receiptrows toreadand drop the unread count to 0.It is a real guard, not a vacuous one: temporarily disabling the three
server.<verb>()notification mounts in the dispatcher plugin makes it fail (404instead of401/200).Also adds
@objectstack/service-messagingas a devDependency of@objectstack/runtime(test-only; messaging does not depend on runtime, so there is no dependency cycle).Testing
@objectstack/runtimesuite: 40 files / 580 tests pass.dispatcher-plugin.routes,dispatcher-plugin.ready.integration,http-dispatcher): pass.tsc --noEmitand eslint clean;@objectstack/runtimebuilds.No changeset (test-only, no user-facing runtime change).
🤖 Generated with Claude Code
Generated by Claude Code