feat: notification methods on PlatformClient, backend types via submodule - #39
Conversation
Apps can already call the notification API — the iframe hands them the signed-in user's JWT — so this is client methods over endpoints that are already live, not new surface. Adds getNotifications, getUnreadNotificationCount, markNotificationsRead, markAllNotificationsRead, getNotificationSubscriptions and unsubscribeFromAutomation. The types come from the backend repo through a submodule rather than being copied into src/types like every other type here. Those copies are why base.ts has to alias ObjectId to string and hope it stays true; the backend's src/common/dto describes what the API actually sends and imports nothing from mongodb, so it compiles in a browser build unchanged. The submodule is sparse-checked-out to that one directory, so 37 files land on disk instead of the whole backend repo. Sparse config lives in the submodule's .git and does not survive a clone, which is why CI runs types:init after checkout. Moving the pin is `yarn types:update`, a command rather than a postinstall hook: an install that silently moved it would break builds with no commit explaining why. vite-plugin-dts and tsconfig both needed the vendored path adding. Without it the published .d.ts re-exports from ../../vendor/... and that resolves to nothing once installed — the build succeeds and consumers get broken types. Pinned to the backend's feature/notification-dtos branch until that merges.
backend-api#321 is merged, so the DTOs no longer live on a feature branch. Tracking dev rather than main because that is where they are: main gets them with the next release, and repointing is a one-line change here plus yarn types:update. Pin includes the review fixes on that PR — the base.dto helpers are gone and the type union guard is bidirectional.
The backend repo is private, so the default GITHUB_TOKEN cannot clone it and checkout with submodules: true fails outright with "Repository not found". Credential is scoped to the fetch step via url.insteadOf and unset immediately after, rather than handed to actions/checkout. Passing it to checkout would make it the identity for everything else in the job, including the release workflow's version-bump push. Needs a BACKEND_TYPES_TOKEN secret on this repo with read access to contents on platform_backend-api.
The failure was a bare "Repository not found" from git, which says nothing about the cause or the fix. CI now fails with an annotation, a job summary and step-by-step setup instructions. Also accepts a deploy key as an alternative to the token. A read-only deploy key on the backend repo never expires and belongs to the repo rather than to whoever created it, so the yearly token rotation goes away. Both work by rewriting the https URL rather than editing .gitmodules, so cloning over https locally is unaffected. The credential handling moved out of the workflows and into the script, so the two workflows cannot drift and the guidance sits next to the failure.
Replaces the standing token with an App-derived one, scoped to platform_backend-api and valid for an hour, so the only long-lived secret is an App key that grants nothing by itself. A plain token or a deploy key still work if those secrets are set instead. Failure instructions now lead with the App setup.
Three methods the client was missing. subscribeToAutomation and updateAutomationSubscription cover the project routes that have been live since task 04. Without them an app could list and cancel subscriptions but never create one, which is a daft half of the feature. onNotification opens the /notifications socket namespace added in backend-api#325. It is deliberately unlike onExecutionProgress: that one follows a single execution and closes when it ends, while a bell stays connected for the session, so this returns a disconnect function instead of managing its own lifecycle. Events carry ids rather than notifications, so a burst stays cheap and the socket is never the source of a stale render. The token is resolved per connection so a provider-backed client opens with a current one.
| "include": ["src", "test.local.dev/clientus.ts"], | ||
| "include": [ | ||
| "src", | ||
| "vendor/backend-api/src/common/dto", |
There was a problem hiding this comment.
This makes the repo impossible to build without access to the private backend.
On a clean clone:
src/types/notifications.ts(18,8): error TS2307: Cannot find module
'../../vendor/backend-api/src/common/dto/notifications.dto'
and yarn build runs tsc, so the whole thing falls over. Since this repo is public, people outside the org and PRs from forks, which have no secrets can't build it.
What about splitting this part into its own PR? The REST methods look good and could go in on their own already.
| # The backend repo is private, so the default GITHUB_TOKEN cannot clone | ||
| # the submodule. A GitHub App mints a token scoped to that one repo and | ||
| # valid for an hour, so nothing long-lived than the App key is stored. | ||
| - uses: actions/create-github-app-token@v1 |
There was a problem hiding this comment.
I don't think the fallback is reachable: this step has no continue-on-error, so if the secrets are missing the job dies here and the || on line 35 never gets evaluated. And since the same step is in release.yml, publishing would be blocked too.
Adding continue-on-error: true here would make it behave the way the comment above describes.
| // so their declarations have to be emitted too. Without this the | ||
| // published .d.ts points at ../../vendor/... and resolves to nothing on | ||
| // a consumer's machine. | ||
| include: ['src', 'vendor/backend-api/src/common/dto'], |
There was a problem hiding this comment.
Adding vendor/ to the dts include reshuffles the published .d.ts: on main it's dist/{core,types,...}, here it's dist/src/... + dist/vendor/... with index.d.ts as a stub.
The types entrypoint still resolves, but any deep import breaks and the changeset is marked minor.
This repo is public and the backend is private, so vendoring its DTOs made a clean clone unbuildable and left fork PRs, which get no secrets, with no way to run CI. Reaching outside src also moved tsc's root: the published layout became dist/src/... plus dist/vendor/... with index.d.ts as a stub, so every deep import broke. Both are Sergio's findings and both are hard blockers rather than preferences. The types are declared in src/types/notifications.ts instead, the same as every other type in this package, with a note on why and what sharing them properly would take. Sharing needs the backend's wire DTOs published as their own package so they arrive through node_modules rather than through the source tree. The client methods are unaffected and stay. Also removes the App-token CI step, which had no fallback path anyway: without continue-on-error the job died before the || could be evaluated.
Summary
Notification methods on
PlatformClient, and a submodule so the types come from the backend instead of being hand-copied.Nothing new on the API side. Apps already call the platform as the signed-in user —
AppIframeposts them the JWT — so these are client methods over endpoints that are already live.getNotifications({ cursor, limit })getUnreadNotificationCount()markNotificationsRead(ids)markAllNotificationsRead()getNotificationSubscriptions()unsubscribeFromAutomation(hookId)Why a submodule and not another copy
Every file in
src/typesis a hand-maintained copy of a backend type. That is whybase.tshas to declareexport type ObjectId = stringand hope it stays true — and it mostly is true, because JSON has no ObjectId, but nothing enforces it.The backend now has
src/common/dto: wire shapes with string ids and ISO timestamps, importing nothing frommongodb, so they compile in a browser build unchanged.src/types/notifications.tsre-exports from there. If the backend changes the contract, this package stops compiling instead of drifting quietly.Sparse-checked-out to that one directory: 37 files on disk rather than the whole backend repo.
The update command
yarn types:updatemoves the pin to the tip of the tracked branch and re-applies the sparse checkout.yarn types:initjust re-applies the sparse checkout without moving anything, which is what CI runs.Deliberately a command and not a
postinstallhook. An install that silently moved the pin would break a build with no commit explaining why, on someone else's machine, mid-task. The pin moves when someone decides it should.One thing worth reviewing carefully
The build succeeded while producing a broken package, and it took building and installing it to notice.
vite-plugin-dtsonly emits declarations for files it is told to include. The re-export compiled fine,yarn buildwas green, and the publisheddist/src/types/notifications.d.tsre-exported from../../vendor/...— a path that does not exist indist. A consumer would have installed the package and foundNotificationDtounresolvable.Fixed by adding the vendored path to both
tsconfig.jsonand thedtsplugin'sinclude, so declarations emit todist/vendor/...where the relative path resolves. Verified by building the package, copyingdistinto a scratch project and typechecking a consumer against it:Worth knowing because it will apply to anything else vendored this way.
Verification
yarn buildgreen,npx vitest rungreen (5 files, 69 tests, 10 new),eslintclean. The new tests cover each method's path, method and body, that the opaque cursor round-trips unmangled, that the unread count is unwrapped to a number, and that the bearer token is sent on notification routes.CI now checks out with
submodules: trueand runsyarn types:initbefore install, on both the build and release workflows.