Skip to content

feat(shell): app install scopes, consolidated identity menu, multi-account, notification service - #449

Merged
izzywdev merged 14 commits into
masterfrom
claude/fuzefront-app-scopes-user-menu-clplz8
Aug 2, 2026
Merged

feat(shell): app install scopes, consolidated identity menu, multi-account, notification service#449
izzywdev merged 14 commits into
masterfrom
claude/fuzefront-app-scopes-user-menu-clplz8

Conversation

@izzywdev

Copy link
Copy Markdown
Owner

📋 Description

Four changes to the shell's identity surface, plus the inbox behind the notification bell.

  • App install scopes — every registered app declares whether it can live in a personal space, an organization, or either. Installing a both app asks where; installing at org level asks install for me vs install for everyone.
  • The organization switcher moves into the avatar menu.
  • An account switcher joins it — up to MAX_PARALLEL_ACCOUNTS = 5 signed-in accounts on one browser, isolated so no state leaks between them.
  • The language switcher moves into the same menu.
  • The notification bell gets a real backing service — schema, API, delivery, and a shell client.

Plan of record: docs/planning/app-scopes-user-menu-notifications.md
Approval frames: design/frames/app-scopes-user-menu/ (5 frames, stamped)

🔄 Type of Change

  • ✨ New feature (non-breaking change which adds functionality)

🧪 Testing

  • Unit tests
  • Integration tests

Every suite below was run, not assumed. A throwaway Postgres 16 was stood up locally so the DB-backed backend tests actually executed rather than being written and left to CI.

Suite Result
backend/tests/app-installations.test.ts (new) 26/26 against real Postgres — exercises the CHECK constraint and the three partial unique indexes, not just the route logic
backend/tests/notification-proxy.test.ts (new) 10/10
backend/tests/apps.test.ts (pre-existing, touched by the scope_level change) 37/37, no regression
services/notification-service (new) 35/35 — routes, SSE hub, auth middleware
frontend vitest 111/111, including 26 new account-vault tests
vite build clean
tsc --noEmit backend + frontend clean
eslint backend + frontend clean (frontend runs --max-warnings 0)
gate-ds-conformance --changed-only clean — zero raw design values in any changed line
scripts/stamp-frames.mjs --check all 13 features stamped

The notification schema was additionally round-tripped against real Postgres: insert, jsonb read-back, dedupe-index rejection, and re-send-after-archive all verified, plus up()/down().

Test Instructions

npm ci
npm run -w @fuzefront/notification-service test
cd frontend && npm ci && npm test && npx vite build
# backend suites need Postgres (CI provides it):
cd backend && npm test -- tests/app-installations.test.ts tests/notification-proxy.test.ts

🔧 Implementation Details

Changes Made

Backend Changes

  • Migration 015apps.scope_level (personal | organization | both, default both) and the app_installations table. Shape is enforced in the database, not only the route: a CHECK constraint pins which anchor columns each (scope, install_mode) combination must and must not carry, and three partial unique indexes make install idempotent per target. They are scoped to status='active' precisely so uninstall can be a soft revoke and a reinstall does not collide with a stale row.
  • The column is scope_level, never scopeapps.scope already means the Module-Federation remote container name.
  • POST /api/apps/:id/install, DELETE .../install/:installationId, GET .../installations, GET /api/apps/installed. Fail-closed at each step: an app the caller cannot see is 404, never 403 (a stranger must not be able to probe which app ids exist); a scope the app's level forbids is 422; an org install requires an active membership; mode: everyone additionally requires owner/admin, because one member must not be able to push an app into every colleague's launcher.
  • /api/v1/notifications same-origin proxy to the new service.

Frontend Changes

  • lib/accounts.ts — the account vault. Three rules: (1) every per-account value lives under ff.acct.<id>.<key>, and the pre-existing bare authToken/sessionId/user/ff.activeOrganizationId keys are migrated in at boot then deleted, so a stale global can never shadow a namespaced one; (2) getActiveAuthToken() is the only way to obtain a token — the axios interceptor, the federated-app loader, the chat widget, the flags client and the account-security page all read through it; (3) the active account is per-tab (sessionStorage), falling back to a browser default, which is what makes the accounts genuinely parallel.
  • Switching accounts is a full document teardown, not a state update. React state, the app-registry cache, the flag cache, mounted remotes and any open stream die with the document. A soft in-place switch would leave every one of those holding account A's data while account B's token is on the wire — the reload is the feature.
  • At the cap a sixth sign-in is refused, not absorbed by evicting an existing account. Per-account sign-out revokes only that session, using that account's own token rather than the shared axios client's.
  • UserMenu rewritten as the single identity surface (accounts / organization / language / actions). Sections fail independently — a failed org fetch renders in place with a retry while accounts, language and sign-out stay usable, rather than stranding the user with no way to sign out.
  • TopBar sheds the org and language controls.
  • InstallAppDialog, NotificationBell, services/notifications.ts.

Notification service (new)

  • One row per recipient, not per event: fan-out happens at write time, so the read path is a single indexed scan and the badge is a partial-index count.
  • read / seen / archived are separate columns. Opening the panel marks seen — the badge clears because the user looked — and does not silently mark everything read.
  • A per-user dedupe_key, unique among live rows and suffixed per recipient, means a retried producer cannot double-post and one producer key deduplicates each person's copy independently.
  • Every user-facing route derives user_id from the verified JWT. No route accepts a user id, so there is nothing to tamper into another mailbox.
  • SSE for live delivery, with a per-user stream cap and a heartbeat.

Code Quality

  • Code follows the project's coding standards
  • Self-review of code completed
  • Code is commented, particularly in hard-to-understand areas
  • No console.log or debugging statements left in code

Documentation

  • Documentation has been updated — new plan doc, new openapi.yaml, values documented inline

🚨 Breaking Changes

None for API consumers. One internal storage-layout change worth knowing about: browser sessions move from the bare authToken / sessionId / user / ff.activeOrganizationId keys into the account vault's namespace. migrateLegacySession() runs at boot before anything reads a token, so existing signed-in users are carried across without re-authenticating. handleAuthCallback.test.ts was updated to assert the namespaced key — a bare authToken surviving now means the vault was bypassed.

📝 Additional Notes

Deployment Notes

  • Requires database migration — backend migration 015 (runs on backend start), and the notification-service knex migration via the new notification-db-migrate pre-install/pre-upgrade Job
  • Requires environment variable changes — secret.notificationInternalToken is empty by default and that is deliberate: with no token the service disables /internal/publish entirely rather than exposing an unauthenticated write-into-any-inbox endpoint
  • Requires configuration changes — notificationService.enabled ships false; the release workflow's image-bump anchor list and expected-rewrite count were updated alongside the new build step, per the warning in that step

Honest limits, stated rather than glossed

  • Same-origin account isolation is a storage and lifecycle boundary, not a browser security boundary. Anything with script execution on the origin can read every namespace, exactly as it could read a single account's token today. Real cross-account isolation needs separate origins or browser profiles. This design bounds accidental leakage — stale caches, the wrong token on a request, one account's org list rendering under another — which is the actual failure mode.
  • The SSE hub is in-process. With replicas > 1 a live push only reaches clients attached to the same pod. That degrades to a delayed badge, which the client's reconnect + unread-count refetch corrects, and the inbox read is always authoritative from Postgres. Cross-pod fan-out (Redis pub/sub) is an additive fix when scaling past one replica — documented in values.yaml next to replicas.
  • Two surfaces, deliberately not joined. The launcher speaks the app-registry contract, whose App schema is additionalProperties: false and therefore carries no backend app id. The install surface speaks the backend apps API. They are two separate reads rather than a fragile slug-matched join, and rather than my unilaterally widening a frozen contract.

Future Work

  • A delivery worker for the email / sms / push channels. notification_deliveries and the preference matrix already exist, so that is additive rather than a migration on a table holding production data.
  • Producers: billing, provisioning and app-install events calling /internal/publish.
  • Playwright specs against the approved frames' data-* hooks.

Questions for Reviewers

  • apps.scope_level defaults to both. Every app on master was registered under an org-centric model, and nothing about those apps forbids a personal install, so both is the only default that leaves every current flow working. Installation is not the authorization boundary — visibility, membership and Permit still gate what a user may see and do — so a permissive default here does not widen access. Say so if you would rather it defaulted to organization.
  • GET /api/apps/installed returns only personal installs when no organizationId is passed. That is intentional (an org filter the caller is not a member of yields nothing rather than leaking whether that org has installs), but it is a choice worth a second opinion.
  • Deploy window. master is deploy-on-push here, so this should not be bot-merged.

🔄 Backwards Compatibility

  • Fully backwards compatible

Generated by Claude Code

…count, notification service

Four changes to the shell's identity surface plus the inbox behind the bell.
Plan of record: docs/planning/app-scopes-user-menu-notifications.md.
Approval frames: design/frames/app-scopes-user-menu/ (5 frames, stamped).

App install scopes
------------------
`apps` already answered who OWNS an app (organization_id) and who may SEE it
(visibility). Neither answered who it may be INSTALLED for, and there was no
installation record at all.

- migration 015: apps.scope_level (personal|organization|both, default both)
  and app_installations. Shape is enforced in the DATABASE — a CHECK constraint
  pins which anchor columns each (scope, install_mode) combination carries, and
  three partial unique indexes make install idempotent per target. They are
  scoped to status='active' so uninstall can be a soft revoke and a reinstall
  does not collide with a stale row.
- The column is scope_level, never scope: apps.scope already means the
  Module-Federation remote container name.
- POST /api/apps/:id/install, DELETE .../install/:installationId,
  GET .../installations, GET /api/apps/installed. Fail-closed at each step:
  an app the caller cannot see is 404 (never 403 — no id probing), a scope the
  app's level forbids is 422, an org install needs active membership, and
  mode=everyone additionally needs owner/admin. One member must not be able to
  push an app into every colleague's launcher.
- InstallAppDialog asks only what the app and the caller's role leave open.

Consolidated identity menu
--------------------------
The organization switcher and the language selector move out of the top bar and
into the avatar menu, alongside a new account switcher. Six top-bar controls did
not fit a phone, and three of them answered the same question: who am I.

Sections fail INDEPENDENTLY — a failed org fetch renders in place with a retry
while accounts, language and sign-out stay usable, rather than taking the whole
identity surface down and stranding the user with no way to sign out.

Multi-account (MAX_PARALLEL_ACCOUNTS = 5)
-----------------------------------------
lib/accounts.ts is the vault. Three rules:

1. Namespacing — every per-account value lives under ff.acct.<id>.<key>. The
   roster holds no credentials. The pre-existing bare authToken/sessionId/user/
   ff.activeOrganizationId keys are migrated in at boot and then deleted, so a
   stale global can never shadow a namespaced value.
2. One resolver — getActiveAuthToken() is the only way to obtain a token. The
   axios interceptor, the federated-app loader, the chat widget, the flags
   client and the account-security page all read through it, so a token can
   never be read for a non-active account.
3. Per-tab active account — sessionStorage first, localStorage default second.
   That is what makes the accounts parallel: account A in tab 1 and account B in
   tab 2, each pinned to its own identity.

Switching is a full document teardown, not a state update: React state, the
app-registry cache, the flag cache, mounted remotes and any open stream die with
the document. A soft in-place switch would leave all of those holding account
A's data while account B's token is on the wire.

At the cap a sixth sign-in is REFUSED, not absorbed by evicting an existing
account. Signing out one account revokes only that session — using that
account's own token, not the shared axios client's.

Stated limit: same-origin isolation is a storage and lifecycle boundary, not a
browser security boundary. This bounds accidental leakage; it does not claim to
bound a compromised origin.

Notification service
--------------------
services/notification-service — Express + Postgres, reached same-origin through
a new host-backend proxy at /api/v1/notifications.

- One row per RECIPIENT, not per event: fan-out happens at write time, so the
  read path is a single indexed scan and the badge is a partial-index count.
- read / seen / archived are separate columns. Opening the panel marks seen —
  the badge clears because the user looked — and does not silently mark
  everything read.
- A per-user dedupe_key, unique among live rows, means a retried producer cannot
  double-post. Suffixed per recipient so one producer key deduplicates each
  person's copy independently.
- Every user-facing route derives user_id from the verified JWT. No route
  accepts a user id, so there is nothing to tamper into another mailbox.
- /internal/publish is the one privileged route, gated on a shared service
  token, and it FAILS CLOSED (503) when that token is unset. The host-backend
  proxy refuses to forward /internal/* at all, so it is never one header away
  from the browser.
- SSE for live delivery, with a per-user stream cap and a heartbeat. The hub is
  in-process: with replicas > 1 a live push only reaches clients on the same
  pod, which degrades to a delayed badge that the client's reconnect + refetch
  corrects. Documented in values.yaml next to `replicas`.
- The bell degrades quiet — service down means no badge, never a broken shell.

Verification
------------
- backend: 26 app-installation tests + 10 proxy tests pass against real
  Postgres, exercising the CHECK constraint and the partial unique indexes;
  pre-existing apps suite still 37/37.
- notification-service: 35 tests (routes, SSE hub, auth middleware).
- frontend: 111 tests including 26 new vault tests; vite build clean.
- backend + frontend type-check and lint clean; gate-ds-conformance clean on
  changed lines; frames stamped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions
github-actions Bot enabled auto-merge (squash) July 29, 2026 10:58
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

Comment thread backend/src/routes/app-installations.ts Fixed
Comment thread backend/src/routes/app-installations.ts Fixed
Comment thread backend/src/routes/app-installations.ts Fixed
Comment thread backend/src/routes/app-installations.ts Fixed
Comment thread backend/src/routes/notifications.ts Fixed
Comment thread backend/src/routes/notifications.ts Fixed
process.env.JWT_SECRET = JWT_SECRET;

function tokenFor(userId: string): string {
return jwt.sign({ userId }, JWT_SECRET);

it('rejects a token signed with the wrong secret', async () => {
const { app } = build();
const forged = jwt.sign({ userId: 'user-x' }, 'not-the-secret');
});

it('accepts a valid bearer token and exposes its claims', async () => {
const token = jwt.sign({ userId: 'u-1', orgId: 'o-1' }, SECRET);
});

it('accepts the token as a query param, for EventSource', async () => {
const token = jwt.sign({ userId: 'u-1' }, SECRET);
});

it('rejects a token signed with a different secret', async () => {
const forged = jwt.sign({ userId: 'u-1' }, 'wrong-secret');
});

it('rejects an expired token', async () => {
const expired = jwt.sign({ userId: 'u-1' }, SECRET, { expiresIn: -10 });
});

it('rejects a token with no subject claim', async () => {
const anonymous = jwt.sign({ scope: 'nothing' }, SECRET);
Comment thread backend/src/routes/notifications.ts Fixed
CodeQL flagged 7 alerts on this PR (5 high, 2 medium). Triaged individually
rather than blanket-suppressed.

Missing rate limiting — app-installations.ts (4 alerts, REAL)
------------------------------------------------------------
Every route there performs an authorization decision backed by DB lookups (app
visibility + org membership), so an unbounded caller amplifies cheap requests
into repeated database work. Worse, because a not-entitled app or org answers
404 by design, an unbounded caller is exactly the shape of an id-enumeration
probe — the 404 that protects against disclosure is only as good as the rate at
which it can be sampled.

Adds read (60/min) and write (30/min) limiters following the repo's existing
express-rate-limit convention (routes/flags.ts, routes/portal.ts).

Both ceilings are env-overridable, and the limiter is ALWAYS mounted — never
skipped by NODE_ENV. A security control that switches itself off outside
production is not a control. The suite raises the ceiling instead (it makes far
more calls per minute than any real user, all from one loopback IP and so one
bucket), and a new test asserts the limiter actually returns 429 once exceeded.
Silencing the alert without proving the fix works would have been the easy
version of this.

Log injection + externally-controlled format string — notifications.ts (2 alerts, REAL)
--------------------------------------------------------------------------------------
The proxy logged `req.method` and `req.url` interpolated into a template
literal. Both are attacker-controlled, which allowed two things: a CR/LF in the
URL forges an extra, fabricated log entry — the one place an attacker gets to
write the record of their own activity — and a %s/%d is consumed as a console
format specifier, shifting every later argument.

Fixed at both call sites: the format string is now a constant literal with the
values passed as arguments, and each value goes through safeForLog(), which
strips control characters, escapes format specifiers, and bounds the length. A
truncated, flattened URL still identifies the failing route.

Hardcoded JWT secret in tests — Semgrep (6 findings, ACCEPTED)
-------------------------------------------------------------
These are test-only signing keys: they exist so the suites can mint tokens the
middleware under test will verify, they appear nowhere else, and they
authenticate nothing outside the test process. Production reads JWT_SECRET and
NOTIFICATION_INTERNAL_TOKEN from the environment, and both surfaces fail CLOSED
when unset — which the suite itself asserts.

Suppressed with `// nosemgrep: <rule>` at each site plus one justification block
per file, matching the repo's existing pattern (backend/src/routes/auth.ts,
tests/e2e/billing-invoices). Explicit and reviewable rather than a .semgrepignore
entry that would also hide future real findings in those files.

Also merges origin/master (portal epic, PR #424) — no conflicts; migration 012
lands below this branch's 015.

Verification: 37 backend tests (installations + proxy) against real Postgres,
35 notification-service tests, backend type-check + lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

console.error(
'[notification-proxy] upstream error for %s %s: %s',
safeForLog(req.method),
safeForLog(req.url),
…Token key

The Playwright sign-in and OIDC-plumbing suites went red on this branch. Real
breakage caused by this PR, not flaky infra: nine spec files assert on
`localStorage.getItem('authToken')`, and the account vault moved every
per-account value under `ff.acct.<accountId>.<key>`.

The specs were right to check "is there a usable session" — they were just
reading a key that no longer exists. A bare `authToken` surviving would now MEAN
the isolation model had been bypassed, so the fix is to resolve the session the
way the app does, not to re-add the key.

Adds tests/support/account-vault.ts with readActiveAuthToken() /
hasActiveSession() / PROVISIONAL_ACCOUNT_ID, and points every call site at it:
auth-simple, auth, oidc-plumbing, google-signin, google-oauth-e2e,
prod-full-auth-flow, clock-load, pages/login-page, plus the seed path in
federated-apps-register-activate (which now writes into the provisional
namespace, where a real first sign-in parks its token).

The helper deliberately RE-IMPLEMENTS the vault's active-account resolution:
page.evaluate() ships the function source to the browser, so it cannot import
anything and must be self-contained.

That duplication is a drift risk, and the seam had NO guard at all —
frontend/tsconfig.json includes only `src`, so the spec files are not even
type-checked (verified by probe, not assumed). So this also adds
src/__tests__/e2e-account-vault-helper.test.ts, which cross-checks the helper
against the real vault across every state that matters — signed out, one
account, several accounts, per-tab pin, first sign-in in the provisional
namespace, add-account mode, last-account sign-out, stale pin, corrupt roster —
and asserts the helpers stay serializable. When the vault changes, that fails in
milliseconds with an obvious message instead of turning a 7-minute e2e run red
in a way that looks like sign-in broke.

Verification: 122 frontend unit tests (17 files) pass; every touched spec
compiles under `playwright test --list` (both the default and prod configs),
which is what actually type-checks these files; frontend lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

claude added 2 commits July 29, 2026 21:55
Resolves four conflicts and one semantic integration defect the merge exposed.

Conflicts
---------
package.json — master reformatted the file (CRLF -> LF, React 19 / Node 24
upgrade in #450), so the whole file conflicted over my one-line change. Took
master's version and re-applied just the `services/notification-service`
workspace entry; the diff against master is now exactly that one line.

.github/workflows/release.yml — both sides touched the GitOps tag-bump guard.
Master raised the always-built image count 7 -> 9 (#442 added the missing
billing-service and provisioning-service anchors, the third occurrence of that
same silent-freeze bug); this branch added notification-service as a second
continue-on-error build. Combined: EXPECTED=$((9 + CHAT_OK + NOTIF_OK)), with
9 unconditional anchors plus the two conditional ones — verified the anchor
count matches the arithmetic.

frontend/src/components/TopBar.tsx — only the import block actually conflicted.
Master added white-label portal branding and still rendered the organization +
language selectors; this branch removed both (they moved into the avatar menu).
The body auto-merged to exactly the right result — branding kept, selectors gone
— so the resolution keeps master's PortalBrandLockup/usePortalContext import and
drops only LanguageSelector.

package-lock.json — regenerated from the merged manifests.

Integration defect the merge exposed (NOT a conflict)
----------------------------------------------------
Master's WhiteLabelLoginCard wrote the session to the BARE `authToken` key. This
branch moved every per-account value into the account vault
(`ff.acct.<accountId>.<key>`), so nothing reads that key any more.

It happened to still work — the card navigates to /dashboard, and the vault's
one-time legacy migration sweeps the bare key at boot. That is a latent trap, not
a fix: it makes the white-label login path depend on an upgrade shim that exists
to be retired, and the day it is retired portal sign-in breaks with the user
authenticated server-side but booted anonymous.

The card now writes the provisional vault namespace directly, with the contract
documented inline. Its test asserts the vault key AND that no bare `authToken`
is left behind, so a regression fails loudly instead of leaning on the shim.

Also noted: @fuzefront/portal-branding-ui cannot build until
@fuzefront/portal-client is built first, and no CI job builds either. That is
pre-existing on master, not caused by this merge, and is left alone here rather
than fixed silently inside a merge commit.

Verification on the merged tree
-------------------------------
- frontend: 136 tests / 19 files, type-check clean, lint clean (--max-warnings 0),
  vite build clean
- backend: 74 tests (app-installations 27, notification-proxy 10, apps 37)
  against real Postgres; type-check clean, lint 0 errors
- notification-service: 35 tests, type-check clean
- portal-branding-ui: 26 tests
- frames stamped; gate-ds-conformance shows no hard violations in changed files

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

…r merge

Master's #453 landed `015_seed_root_platform_organization.ts` and
`016_provisioning_steps_rebac.ts` while this branch already carried its own
`015_app_scope_levels_and_installations.ts`. Two files sharing ordinal 015 is
not a conflict git can see -- the merge was clean -- but knex sorts migrations
lexically by filename, so the relative order of two 015s is decided by the
rest of the string, not by intent. Renumbered to 017 so the ordering is
explicit and the app-scopes work runs after the root-org/ReBAC provisioning it
now sits behind.

Safe to rename rather than add a no-op shim: this branch has never been
deployed, so no `knex_migrations` row anywhere records the old filename.

Also updated the "migration 015" comment references that pointed at this
file (backend/src/routes/app-installations.ts, backend/src/routes/apps.ts,
backend/tests/app-installations.test.ts, frontend/src/services/api.ts).
Master's own references to its 015 are untouched.

Verified on a fresh Postgres 16 with every migration replayed in the new
order: knex_migrations ends 014_seed_platform_registrar_user,
015_seed_root_platform_organization, 016_provisioning_steps_rebac,
017_app_scope_levels_and_installations; `apps.scope_level` present; all
app_installations indexes and the shape CHECK present. Backend suite
91/91 passing (--runInBand; parallel runs contend on knex's migration lock).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

Copy link
Copy Markdown
Owner Author

Conflicts resolved; branch is up to date with master (9cab21f, #453)

Two merges from master, plus one correction the merges did not surface on their own.

Merge 1 — 753032e (#455). One real conflict, in root package.json: master had reformatted the file CRLF→LF wholesale, so git saw a whole-file conflict where the only semantic change on this branch was the added services/notification-service workspace entry. Took master's version and re-applied that single line.

Also fixed a semantic defect the merge could not see: packages/portal-branding-ui/src/components/WhiteLabelLoginCard.tsx (new on master) wrote the bare authToken key. That happened to work only because the account vault's one-time legacy migration sweeps it at boot — an upgrade path being used as an ongoing write channel, which breaks silently the day that migration is retired. It now writes ff.acct.__provisional__.authToken directly, and its test asserts both the vault key and that the bare key stays null.

Merge 2 — 9cab21f (#453). Clean, no conflicts. But it landed 015_seed_root_platform_organization.ts while this branch already carried its own 015_app_scope_levels_and_installations.ts. Two files sharing ordinal 015 is not a conflict git can detect — knex sorts migrations lexically, so their relative order would have been decided by the rest of the filename rather than by intent. Renumbered to 017_app_scope_levels_and_installations.ts so it explicitly runs after the root-org/ReBAC provisioning it now sits behind. A rename is safe rather than requiring a shim because this branch has never been deployed — no knex_migrations row anywhere records the old name.

The PR body above still says "migration 015" in two places (Changes Made, Deployment Notes) — read those as 017.

Re-verified after the renumber, on a Postgres 16 stood up fresh so every migration replayed in the new order:

  • knex_migrations ends 014_seed_platform_registrar_user015_seed_root_platform_organization016_provisioning_steps_rebac017_app_scope_levels_and_installations
  • apps.scope_level present; the app_installations shape CHECK and all three partial unique indexes present
  • Backend suite 91/91 across app-installations, notification-proxy, apps, provisioning, and master's new root-org-admin — no regression from either merge

One note on running these locally: the suites must go --runInBand. In parallel they contend on knex's migration lock (provisioning.test.ts calls migrate.latest() itself) and fail with "Migration table is already locked" — a harness artifact, not a schema problem.

Deploy window still applies. Merging here dispatches a release and deploys to production, so per CLAUDE.md this is not a bot-merge — the remaining gate is your review approval.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

…24 typings floor

"Chat service (unit)" was the only red check of 59 on this PR. It failed in
`services/chat-service/src/rag/index-docs.ts` with five errors rooted in one:

    Type 'Dirent<string>[]' is not assignable to type 'Dirent<Buffer>[]'

Two separate defects, fixed together.

1. `services/notification-service/package.json` pinned `@types/node` to
   `18.19.0`. That directly violates CLAUDE.md's toolchain floor, which mandates
   `^24.13.3` in every manifest -- my error when the manifest was written, and
   what perturbed the workspace's nested dependency tree in the first place.
   Now `^24.13.3` like every sibling.

2. `index-docs.ts` declared `let entries: Awaited<ReturnType<typeof fs.readdir>>`.
   `readdir` is overloaded, and `ReturnType` silently resolves to the LAST
   overload -- which under @types/node 24 is the buffer-encoding variant
   returning `Dirent<Buffer>[]`, so the `{ withFileTypes: true }` call that
   actually follows (a `Dirent<string>[]`) could not be assigned to it. Replaced
   with an explicit `Dirent[]`: it is what the code means and it is stable across
   typings versions.

Why this surfaced only here, when master is green on the same file: the lockfile
nests a stale `@types/node@18.19.0` under chat-service even though its manifest
asks for `^24.13.3`. npm 10 honours that stale entry, so chat-service compiles
against Node 18 typings where `Dirent` is not generic and the `ReturnType` trick
happens to work. CI's npm 11 correctly ignores a nested entry that cannot satisfy
the declared range and hands chat-service the root 24.13.3 it asked for --
exposing a latent error rather than introducing one. So master is green by
accident, against typings its own floor forbids.

Verified against BOTH typings, since the local and CI resolutions differ:
- pre-fix vs 24.13.3 -> reproduces all five CI errors exactly
- post-fix vs 24.13.3 -> tsc clean
- post-fix vs 18.19.0 -> tsc clean
- chat-service: 131 passed / 2 skipped, 24 suites
- notification-service: `tsc` clean and 35/35, under 24.13.3 as well as 18.19.0
  (checked so this change does not simply relocate the same latent bug)
- `npm ci` exits 0; the only lockfile delta is the mirrored range

Also gitignored `services/notification-service/dist/`, which was untracked build
output. Listed explicitly alongside chat-service rather than as `services/*/dist/`
because email-service's dist is deliberately tracked.

Not fixed here, deliberately: eight other workspaces (shared, billing-client,
portal-client, packages/{chat-client,feature-flags,security,i18n-translate},
services/email-service) carry the same stale nested `@types/node@18` against a
`^24.13.3` manifest. Each is a latent version-sensitive break of exactly this
kind. Reconciling the lockfile repo-wide is its own change, not something to bury
inside this PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

…r-menu-clplz8

Third merge from master. Two conflicts, both resolved in master's favour, plus
one latent defect of my own that the merge brought into view.

**`services/chat-service/src/rag/index-docs.ts`** — master fixed the SAME
`Awaited<ReturnType<typeof fs.readdir>>` bug I had just fixed, with the same
root-cause diagnosis (ReturnType resolves to readdir's Buffer overload under
@types/node 24) and an equivalent remedy: infer from the call rather than
annotate. Took master's version byte-for-byte and dropped my now-unneeded
`type Dirent` import. It is their file and their fix; divergence here buys
nothing.

**`package-lock.json`** — master removed the stale nested
`services/chat-service/node_modules/@types/node@18.19.0`, which is exactly the
lockfile reconciliation my previous commit flagged as needed but out of scope.
Resolved by taking master's lock wholesale and re-running `npm install` to
re-derive this branch's `services/notification-service` subtree, rather than
hand-editing lock JSON. Net effect: there is now exactly ONE `@types/node` in the
tree, the root `24.13.3` — every nested Node-18 copy is gone, so notification-
service resolves the version its manifest asks for and the class of failure that
broke CI cannot recur silently in these workspaces.

**`frontend/e2e/post-prod/live-smoke.spec.ts`** — not a conflict; a spec my
earlier sweep missed because it lives under `e2e/post-prod/` rather than
`tests/`. It seeded the bare `authToken` key before app boot. That still boots
today, but only because the account vault's one-time legacy migration sweeps it
— the same "upgrade path used as a write channel" trap already fixed in
WhiteLabelLoginCard. This one matters more: it is the POST-PRODUCTION synthetic
that verifies the live deployment, so it would have gone quietly red against prod
the day that migration is retired. Now routed through the shared
`seedMockSession` helper, whose docstring is generalised since it now seeds a
real prod token as well as fixtures.

Verified after the merge:
- frontend `tsc --noEmit` clean; vitest 136/136 across 19 files
- backend 91/91 (app-installations, notification-proxy, apps, provisioning,
  root-org-admin) against real Postgres, `--runInBand`
- chat-service 131 passed / 2 skipped across 24 suites; `tsc` clean
- notification-service `tsc` clean, 35/35, now resolving root 24.13.3
- master's new `@fuzefront/auth-ui` 17/17
- no duplicate migration ordinals: 014, 015, 016, 017
- `packages/auth-ui` touches no storage, so #458 introduces no bare-token write

Note for anyone running the backend suites locally: master's Permit ReBAC work
makes `PERMIT_API_KEY` mandatory at import time, so they need
`PERMIT_API_KEY=ci-no-real-permit-calls` (what CI passes) or every suite fails in
setup before a single test runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

claude added 2 commits July 29, 2026 22:43
…t `authToken`

Third and last spec seeding the bare `authToken` key, found by sweeping the whole
tree rather than one directory — which is how the first two were missed. This one
sits at `tests/e2e/billing-invoices/`, outside `frontend/`, so neither the earlier
`frontend/tests/**` pass nor the `frontend/e2e/post-prod/**` pass covered it.

Like `live-smoke.spec.ts`, it boots today only because the account vault's
one-time legacy migration sweeps the bare key at startup. Depending on an upgrade
path as a write channel means this goes quietly red against production the day
that migration is retired — and as a post-production synthetic, quietly red is
the worst failure mode it has.

The vault literal is duplicated rather than imported: this Playwright project has
its own config and must not reach into the shell's source tree, the same reason
`WhiteLabelLoginCard.tsx` duplicates it. `frontend/tests/support/account-vault.ts`
pins the format for the specs that CAN import it.

After this, the only remaining bare-`authToken` write in the repository is
`frontend/src/__tests__/accounts.test.ts`, which writes it deliberately to prove
the legacy migration sweeps it — the one place the bare key SHOULD appear.

Verified: frontend vitest 136/136 across 19 files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

claude added 2 commits July 29, 2026 22:54
`frontend/playwright-report-prod/index.html` is a 516K generated Playwright HTML
report that I committed by accident in f5dd652 while verifying the e2e specs.
Master's #447 added `**/playwright-report-prod/` to .gitignore, but gitignore does
not apply to already-tracked files, so it had to be removed from the index
explicitly. Confirmed ignored afterwards.

Two pre-existing artifacts are deliberately left alone because they are tracked on
master, not introduced here: `frontend/playwright-report/index.html` and
`frontend/test-results/{.last-run.json,auth-simple-success.png}`. Untracking
master's files is not this PR's business.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

@izzywdev
izzywdev merged commit 53b31ca into master Aug 2, 2026
40 of 42 checks passed
@izzywdev
izzywdev deleted the claude/fuzefront-app-scopes-user-menu-clplz8 branch August 2, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants