Quality Pass 1: RBAC live-resolution + Traffic triage fixes + seed evidence - #38
Merged
Conversation
…apshot Permissions were snapshotted into user_permissions only at create/role-change, but login + the per-request middleware + socket auth READ that snapshot — so a permission added to a role never reached existing users (the recurring collections:*/datasources:* bug that needed a re-seed). Meanwhile UserService.getPermissions already resolved live: two divergent truths. All four readers (auth.middleware authenticate + optionalAuthenticate, auth.service login, socket.ts) now use permissionNamesForRole(user.role); the dead syncPermissionsForRole write path is removed. user_permissions is now vestigial (drop-migration deferred to RBAC Round 2). Tests updated to the live-resolution contract. Server suite 995 green. DECISIONS D-180. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1ad6jb5mCYxzFVXHbksqi
The triage feed passed onReprocess unconditionally, so a viewer saw an actionable Reprocess button the server rejects. Gate it with usePermissions()/CHANNELS_DEPLOY, matching the per-channel message browser. (The metaDataId=0 destination-error blind spot in cross-channel-search is the other half of this P0 — landing next with a real-DB integration test.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1ad6jb5mCYxzFVXHbksqi
cross-channel-search joined connector_messages on metaDataId=0 (source only), so destination ERRORs — the dominant healthcare failure mode — never appeared or counted. Rewrote: status filter is now EXISTS over any connector message; the displayed status/connector is the one matching the filter (else the errored one, else source) via scalar subqueries, keeping one row per message. Replaced the vacuous mock-the-ORM unit test with a real-Postgres integration test proving a destination-only ERROR surfaces with the failed connector's name, plus status/channelIds branch coverage. Completes the Traffic P0 (with the earlier Reprocess RBAC gate). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1ad6jb5mCYxzFVXHbksqi
Adds an idempotent seed module (invoked from run-seed after seedExampleChannels) that leaves human-reviewable evidence in the dev DB for the pages that render empty on a fresh install: 2 resources, an example-orders collection + 3 records, a code-template library + FUNCTION, PREPROCESSOR/DEPLOY global scripts, 2 global- map + 3 config-map entries, a reporting-db data source (gated on CONTENT_ENCRYPTION_KEY), a CHANNEL_ERROR alert, and a headline "Example: Full Pipeline" channel whose destination carries a filter + transformer + response transformer. Existence-checked by natural key (re-running is a clean no-op). Verified: db:seed populates all tables; the Full Pipeline dest has all three message-flow scripts; a second run is a no-op. seed-examples.ts untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1ad6jb5mCYxzFVXHbksqi
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple P0 correctness gaps surfaced by the testing-gaps audit: it unifies RBAC permission enforcement to resolve live from roles (removing stale per-user snapshots), fixes cross-channel traffic triage to include destination connector failures, and adds an idempotent showcase seed to populate otherwise-empty feature pages in dev.
Changes:
- RBAC: switch login, HTTP middleware, and Socket.IO auth to
permissionNamesForRole(user.role); removeuser_permissionssnapshot write path. - Traffic triage: update cross-channel search to match status across any connector (source or destination) and return the most relevant connector/status without fan-out; add a real-Postgres integration test.
- Dev experience: gate “Reprocess” UI on
channels:deployand addseed-showcase.tsinvoked bydb:seed.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/pages/TrafficPage.tsx | Gates reprocess action in the Traffic UI on channels:deploy. |
| packages/server/test/integration/cross-channel-search.itest.ts | Adds real-Postgres integration coverage proving destination-only errors surface in triage. |
| packages/server/test/integration/_setup.ts | Exposes CrossChannelSearchService to integration tests. |
| packages/server/src/services/user.service.ts | Removes per-user permission snapshot sync; relies on live role resolution. |
| packages/server/src/services/cross-channel-search.service.ts | Fixes status filtering to match any connector and picks the most relevant connector/status per message. |
| packages/server/src/services/auth.service.ts | Resolves login permissions from role instead of user_permissions. |
| packages/server/src/services/tests/user.service.test.ts | Updates tests to reflect removal of permission snapshot writes. |
| packages/server/src/services/tests/cross-channel-search.service.test.ts | Removes brittle ORM-mocking unit test suite for cross-channel search. |
| packages/server/src/middleware/auth.middleware.ts | Resolves request permissions from role in both authenticate and optional auth paths. |
| packages/server/src/lib/socket.ts | Resolves Socket.IO permissions from role (and selects role from users). |
| packages/server/src/lib/tests/socket.test.ts | Updates Socket.IO auth tests for live role-derived permissions. |
| packages/server/src/db/seeds/seed-showcase.ts | Adds idempotent showcase seed data across multiple feature pages. |
| packages/server/src/db/seeds/run-seed.ts | Runs the new showcase seed as part of db:seed. |
| docs/progress/DECISIONS.md | Documents D-180 RBAC live permission resolution decision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+154
to
+156
| // Seed showcase example data for otherwise-empty feature pages | ||
| const { seedShowcase } = await import('./seed-showcase.js'); | ||
| await seedShowcase(exampleDb); |
Comment on lines
+176
to
+179
| it('stores user data and permissions resolved live from the role after successful auth', async () => { | ||
| // Permissions are resolved from the user's role (single source of truth), | ||
| // not a per-user snapshot — a viewer gets exactly the viewer role's set. | ||
| userRow = { id: 'user-42', enabled: true, role: 'viewer' }; |
Comment on lines
+22
to
+25
| const { db, schema, eq } = mods; | ||
| for (const id of channelIds.splice(0)) { | ||
| await db.delete(schema.messages).where(eq(schema.messages.channelId, id)); | ||
| } |
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.
Quality Pass 1 — P0 fixes + seed evidence
Foundation pass from the testing-gaps audit: three correctness bugs the audit surfaced (not just missing tests) plus the "leave evidence in dev" seed.
RBAC: live permission resolution (D-180)
The recurring bug (new permissions like
collections:*/datasources:*never reaching existing users, needing a re-seed) was a stale per-useruser_permissionssnapshot read by login + the request middleware + socket auth, whilegetPermissionsresolved live — two divergent truths. Now all four paths resolve permissions live from the user's role (permissionNamesForRole); the dead snapshot write path is removed. Adding a permission to a role reaches every existing user on their next request — no re-seed, no re-login.user_permissionsis now vestigial (drop-migration deferred to RBAC Round 2).Traffic triage correctness (P0)
connector_messagesonmetaDataId=0(source only), so destination send failures (the dominant healthcare failure mode) never appeared or counted. Rewrote: status filter isEXISTSover any connector; display shows the matching/errored connector via scalar subqueries (one row per message). A real-Postgres integration test proves a destination-only ERROR surfaces with the failed connector's name.channels:deploy— viewers no longer see a button the server rejects.Seed-showcase — evidence in dev
New idempotent
seed-showcase.ts(runs onpnpm db:seed) populates every previously-empty feature page: resources, anexample-orderscollection + records, a code-template library + FUNCTION, PREPROCESSOR/DEPLOY global scripts, global-map + config-map entries, areporting-dbdata source (gated onCONTENT_ENCRYPTION_KEY), aCHANNEL_ERRORalert, and a headline "Example: Full Pipeline" channel whose destination carries a filter + transformer + response transformer. Existence-checked by natural key — re-running is a clean no-op. (SetCONTENT_ENCRYPTION_KEYin.envto seed the data-source example; it's skipped gracefully otherwise.)Verification
Full build +
lint --max-warnings 0clean. Server unit 990, web 90, integration 25 (incl. the new cross-channel proof).pnpm db:seedverified to populate all showcase tables and be idempotent; the Full Pipeline dest confirmed to carry all three message-flow scripts.Deferred (next passes)
P1: Certificates coverage, destination filter/transformer/response-transformer harness, backend test patches, admin component tests, Code Templates TS/JS, HTTP→HTTPS+certs, channel-script tests. P2 + RBAC Round 2 (role CRUD, sub-resource ACLs) remain plan-only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01F1ad6jb5mCYxzFVXHbksqi