Repository navigation
fix(core)!: apply the service manifest to any plugin list and pick job queues per process - #298
Conversation
zaxovaiko
left a comment
There was a problem hiding this comment.
Looks good to me. Traced every createApp caller and every ctx.jobs.worker registration (core plus the betfeel overlays), all of them go through the new selection, and schedules still land after it. Minor:
create:service accounts identity,mailstill double-handles: the monolith bootsidentitytoo, so its 3 session handlers run twice, and the suggestedauditoverride writes every audit row twice. Maybe worth a warning in the scaffold output until ADR-0045 lands?- Let's give the agent-hub queues one shared prefix, so the split is
JOB_WORKERS=agent-hub*on the worker and!agent-hub*on the API without listing each queue. - Nothing checks across processes, so a dead worker means a queue that silently backs up. We'll need a queue-depth alert next to the deploy change. But that's for later.
- ADR-0044 collides with #295 and #297.
The generated host set SERVICE_MANIFEST but not SERVICE_NAME, which createApp requires with a manifest, so it never booted; its example manifest (identity,wallet) also left out wallet's dependencies. It now bakes the service name as SERVICE_NAME, passes the identity tables as authSchema like the consumer template, uses identity,mail as the example, and drops dependencies its main.ts never imports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
…ssing dependencies SERVICE_MANIFEST was applied only by loadExtensions(), while createApp read it just to require SERVICE_NAME. A host that passes its own plugin list therefore booted every module under the split service's own consumer group, so every broker event was handled once more across the cluster. createApp now selects the manifest's modules from whatever list it is given, and loadExtensions() no longer filters. A manifest that leaves out a module the selected ones depend on - directly or through another left-out module, infra overlays' dependencies included - fails the boot with one error naming them all and a manifest line to paste. The rule that a manifest lists its modules' dependencies and port providers is back in the messaging rule. BREAKING CHANGE: a process that sets SERVICE_MANIFEST and passes its own plugin list now boots that subset, or refuses to boot over missing dependencies, instead of every module. Consumer groups the old full boot created under the split SERVICE_NAME are stale; remove them with XGROUP DESTROY. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
Every process consumed every queue, so a job that runs for minutes and mostly waits on an outside service took a share of every API replica. A manifest service cannot take it over: its own SERVICE_NAME gives it its own copy of every event, and a handler that needs the in-process MCP kernel needs every module that contributes tools. JOB_WORKERS picks the queues a process consumes: names, prefix* patterns and !exclusions; unset is every queue, as before. createApp applies it when it registers workers, so it holds for any queue driver, and logs the queues it consumes and skips. A malformed value, or an inclusion that matches no registered queue, fails the boot; an exclusion that matches nothing is allowed, since the module behind it may be absent. To keep a queue off the API replicas, run one more full instance with the same SERVICE_NAME and JOB_WORKERS=<queues>, and give the replicas JOB_WORKERS=!<queues>. ADR-0044 (Proposed) records the decision. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
A split service loads the modules its own depend on, and those still wire their event handlers, workers, schedules and routers - under the service's own consumer group, so audit's handlers, for one, would run once per service. ADR-0045 (Proposed) records the direction - only the modules a manifest selects consume - for a follow-up. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
The JOB_WORKERS selection copied the registered workers before the router factories ran, so a worker registered from a factory never started, even with JOB_WORKERS unset. The selection now runs at the registration loop, after the factories; an inclusion that matches no queue still fails the boot. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
identity and mail resolve some ports with c.has() and run without them, so a split service of identity and mail skipped the country check on sign-in and sent regulatory mails without an audit record, and nothing at boot said so. A plugin now lists such ports in optionalPorts with what it skips without each, and loadPlugins logs a warning for every one left unbound. identity declares the geo check, player provisioning and MCP token revocation; mail the audit writer and play eligibility. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
A generated service and the monolith both load identity, so identity's handlers ran in both, and the override the scaffold suggested added audit, which would write every audit row twice. The scaffold now says so after it generates the host, points at the boot log's optional-port warnings, and no longer suggests adding audit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
3dedd4c to
beea7f2
Compare
|
Thanks, @zaxovaiko.
|
|
/codex all |
There was a problem hiding this comment.
Blocker: a service manifest can boot money and admin routes without their compliance and permission controls. Blocker: split services can duplicate audit records when they load shared dependency modules.
Reviewed security, compliance, conventions, performance, reliability, rollout, spec, the whole change.
Under SERVICE_MANIFEST a split service could boot without the module behind a money, compliance or authorization control and skip it in silence: wallet without compliance took deposits and bets past the RG limits, and admin routes without iam fell back to the static role table, so an admin whose role was revoked in the database could still approve a withdrawal. A plugin now lists such ports in controlPorts, with the module that binds each. A full boot logs an unbound one, as before. A manifest boot refuses to start while one is unbound and the plugin list has its module, naming the port, what its module skips and a manifest line that adds the binding modules and what they need in turn. AdminGuard holds ADMIN_PERMISSION_RESOLVER (iam) and ADMIN_SECURITY_POLICY (identity) to the same rule once admin routes resolve it. An install whose plugin list lacks the module has the control nowhere and boots as before. Declared: RG_LIMITS (wallet, gaming), GAME_GEO_CHECK (gaming), GEO_CHECK_COMMANDS (identity, bonus, notifications), MCP_TOKEN_REVOCATION (identity) and AUDIT_WRITER (mail). Ports whose absence already fails closed - no KYC policy, no risk tags, no bonus grants, no play eligibility - are left as they are. The container now clears a token's resolving mark when its factory throws, so a refusing factory refuses again with its own error rather than as a circular dependency. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
zaxovaiko
left a comment
There was a problem hiding this comment.
Not quite yet - the scaffold example still doesn't boot, and fixing a manifest takes up to three boots. The AI side looks right to me: core gets no runtime or model code, and JOB_WORKERS is a generic seam the hub can use. Minor:
- The
MCP_TOKEN_REVOCATIONcontrol also refuses a split whileagents.mcp.enabledis off. Admin routes neediamanyway, so that's for later.
A manifest boot found what it missed in three places: dependsOn before loading, control ports after configure(), and AdminGuard's ports only when a router factory resolved the guard. SERVICE_MANIFEST=wallet took three boots to fix. A split that served no admin routes booted without iam and then failed every request that resolved the guard lazily (MCP authorize, for one), since the container does not cache a factory that throws. A manifest boot now checks dependencies before loading and the rest once its modules registered and configure() ran: required ports, the selected modules' controlPorts, and AdminGuard's ports on every manifest boot, admin routes or not. Whatever it finds fails the boot with one error that names each missing module and why, and one "Add them" line that also adds what those modules need in turn - their dependencies, the binders of their required ports and the modules behind their controls - so one edit fixes the manifest. The guard is then built once, at boot. Which module binds a required port is learned only after the boot has failed, by registering each plugin of the list into a container of its own. A plugin whose register() throws there - a module this process does not configure - counts with what it bound before the throw, and the error names it when that leaves a required port without a known binder. loadPlugins() takes requiredPorts: 'defer' so a manifest boot can fold required ports into that error; a full boot asserts them as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
The usage example, identity,mail, did not boot: identity's controls pull in compliance and iam, mail's pulls in audit, and compliance depends on wallet, gaming, player-management and exchange-rate. The example now lives in tools/create/service-example.json, which the scaffold prints, and is the manifest the first boot of SERVICE_MANIFEST=wallet tells to use. An E2E boots it against the real plugin list and checks that the wallet boot still suggests exactly it, so a module that gains a dependency or a control fails the test instead of leaving the example stale. It keeps its app open across its tests, as the other suites do: BullMQ disconnects a connection still initialising when its queue closes, rejecting commands it never awaited, so an app closed straight after boot leaks unhandled rejections. The scaffold's warning now says every split loads iam and identity, and what follows from them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
# Conflicts: # packages/core/src/server/plugin-host/define-plugin.ts # packages/core/src/server/runtime/__tests__/create-app.test.ts # packages/core/src/server/runtime/create-app.ts
jakubfilinger-b
left a comment
There was a problem hiding this comment.
Overall the manifest/JOB_WORKERS logic reads correct to me (parser edge cases look handled). Two notes inline/below: the double register() and the ADR-0044 number clash with #297.
|
Heads-up: this and #297 both claim ADR-0044 (as the description notes), so whichever merges second needs to renumber its ADRs. |
…ose what it opened A failed SERVICE_MANIFEST boot registered every listed plugin a second time, each into a container of its own, to learn which module binds which port, so a register() with an effect outside the registry would act differently on that path. No core plugin's does: each only declares into the registry. mail and identity read a secret and throw, which the boot error already reports. audit, chat and notifications built their logger inside register(); it now sits at module scope, as in the other modules. A downstream plugin can still open a connection or start a timer there, and the contract was written down nowhere. - The boot error no longer registers the plugins the boot already registered: what they bound is in the process's container, which the check reads. Every other plugin registers once, so no register() runs twice in one process. - createApp disposes its container when the boot fails. The caller gets no app to close, so a refused manifest boot left the Redis client createApp opened, and any connection configure() bound, connected and reconnecting, which keeps alive a host that does not exit on the error. - Plugin.register states the contract: declare only, and open a resource in a provide() or router factory that closes it with onDispose(). ADR-0044, the plugin-author rule, the scaffold commands and template say the same, and the error-tracking guide's example, which initialised the vendor SDK in register(), now does it in the factory. - A unit test runs every core plugin through the refused boot's registration pass and fails on a timer, an open handle, a process listener, a global or a process.env change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
|
@jakubfilinger-b on the ADR numbers: agreed. #297 is still open and dev's latest is 0043, so I'm leaving them as they are for now. Whichever merges second renumbers. If #297 lands first, this PR's 0044 and 0045 become 0045 and 0046, and every cross-reference moves with them: the ADRs themselves, the messaging rule and AGENTS.md, the docs, and the plugin-host and scaffold notes that cite ADR-0044. |
…and 0046 dev took ADR-0044 for usage telemetry, so check:refs failed on the duplicate number after the merge. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
…elivery adr dev took ADR-0045 for at-least-once event delivery, so the job worker selection ADR becomes ADR-0046 and the dependency-only modules ADR becomes ADR-0047. References that mean the delivery ADR keep 0045. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
|
Merged |
# Conflicts: # tools/create/create-service.ts
Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com> # Conflicts: # .rulesync/rules/messaging-and-microservices.md # AGENTS.md # packages/core/src/server/plugin-host/load-extensions.ts
…e sportsbook ones dev took ADR numbers 0046 and 0047 for the sportsbook ledger port and provider promotions; this branch's two ADRs become 0048 and 0049. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
dev's SERVICE_MANIFEST entries carry a no-routes flag and a split now needs REALTIME_NAMESPACE: the boot check takes the entries, keeps the flags in the manifest line it suggests, and the tests set the namespace beside the service name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
Dev took ADR-0048 for claimable rakeback, so this branch's ADRs move to ADR-0049 (per-process job worker selection) and ADR-0050 (dependency-only modules), with every cross-reference updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mateusz Paś <mateusz.pas@studiosoftware.com>
|
Heads-up: this PR and #372 both introduce an ADR numbered 0049. Whichever merges second will need to renumber its ADR and update its references. |
jakubfilinger-b
left a comment
There was a problem hiding this comment.
Re-reviewed. The only change of its own since my approval is 8bd1962 (boot check reads the flagged manifest entries, tests set REALTIME_NAMESPACE), which looks right; the rest is merges of dev. The ADR numbers are settled now.
Summary
createAppappliesSERVICE_MANIFESTto whatever plugin list it is given and names everything a manifest leaves out in one error with one manifest line that boots, refuses to boot a split that would skip a money, compliance or authorization control, logs each other optional port it leaves unbound, and a newJOB_WORKERSenv var picks the job queues a process consumes. Also fixes thecreate:servicescaffold and records a follow-up ADR.Why
loadExtensions().createAppreadSERVICE_MANIFESTjust to requireSERVICE_NAME, so a host that passes its own plugin list (the downstream guide shows exactly that) booted every module under the split service's own consumer group: every broker event was handled once more across the cluster. Verified on a real consumer: the "split" process loaded every module and created its group on ~150 streams.SERVICE_MANIFEST=walletgets a line that boots on its first boot.c.has(), sowallet,identity,mail,audittook deposits and bets past the RG limits, signed players in without the country check and sent regulatory mails without an audit record, and its admin routes fell back to static grants without iam - an admin whose role was revoked could still approve a withdrawal. The boot said nothing. Now a manifest boot fails closed: a plugin lists a port that backs a money, compliance, authorization or audit control incontrolPorts, with the module that binds it, and the boot refuses while one is unbound and the plugin list has that module, printing the manifest line to paste.AdminGuardholdsADMIN_PERMISSION_RESOLVER(iam) andADMIN_SECURITY_POLICY(identity) to the same rule on every manifest boot, admin routes or not, and a manifest boot builds the guard before it serves. Declared:RG_LIMITS(wallet, gaming),GAME_GEO_CHECK(gaming),GEO_CHECK_COMMANDS(identity, bonus, notifications),MCP_TOKEN_REVOCATION(identity),AUDIT_WRITER(mail). A full boot, and an install whose list lacks the binding module, only log them, as before. A port whose absence only degrades a feature or already fails closed (no KYC policy, no risk tags) is not a control port.JOB_WORKERS(names,prefix*,!exclusions; unset = all) lets one more full instance with the sameSERVICE_NAMEtake such queues while the replicas skip them. A malformed value or an inclusion that matches no queue fails the boot; the boot log lists consumed and skipped queues. ADR-0046 (Proposed).pnpm create:servicegenerated a host that could not boot: noSERVICE_NAME, and an example manifest that left out what its modules need. Its example is now the line the first boot ofSERVICE_MANIFEST=walletsuggests, which an E2E boots. It now also warns that a module two services load handles each event in both, until ADR-0047 lands.Alternatives considered
loadExtensions(): it rewrites relative paths, so it breaks hosts that list bare package specifiers.SERVICE_ROLE=api|worker): fixes one topology and cannot name queues. Role metadata per worker registration: a module cannot know how it is deployed. A worker-only process without routers: router factories are boot hooks workers rely on (ADR-0038). Details in ADR-0046.Risks
!): a process that setsSERVICE_MANIFESTwith a static plugin list now boots that subset, or refuses to boot over missing dependencies or a skipped control. With the shipped modules every split carries iam, identity, audit, mail, compliance, wallet, gaming, player-management, exchange-rate and chat, and until ADR-0047 lands their handlers run in the split and again in the rest of the install. When a manifest boot fails, each listed plugin the boot had not registered runs itsregister()once, into a container of its own that is then dropped, to learn which module binds which port;register()must only declare (documented onPlugin.register), and the failed boot disposes its container. Consumer groups the old full boot created under a splitSERVICE_NAMEare stale (XGROUP DESTROY). Merges todevpublish a canary immediately.JOB_WORKERSneeds operator care: every queue needs a consumer somewhere, and processes that share aSERVICE_NAMEmust load the same modules; core cannot check either across processes. Unset changes nothing.create-app.ts, ADR-0017 and the messaging rule. Both have merged anddevis merged here; they took ADR-0044 and ADR-0045, so this PR's ADRs are ADR-0046 (job worker selection) and ADR-0047 (dependency-only modules).