Repository navigation
feat(core): add the mcp kernel with agent tool and action-type registries - #264
Merged
Merged
Conversation
added 5 commits
September 28, 2026 07:47
…ries Modules register agent tools with ctx.mcp.tool(defineMcpTool(...), factory) and action types with ctx.actions.register(defineActionType(...), factory). Consumers reach both only through MCP_KERNEL, which validates input shape at registration, checks IAM per call through AdminGuard.assertUser, returns only allow-listed output, maps exceptions to declared codes and audits every call. - RunContext, PlatformConfig.agents, trigger catalog, agent IAM resources - first tools: player.summary, wallet.activity, kyc.status, ggr.summary - first action types: add_tag, add_note, hold_withdrawal, send_to_manual_review, request_enhanced_kyc - withdrawals can be held; admins approve or reject a held withdrawal - catalog and @openora/mcp list the agent surface; ADR-0041
Conflicts resolved by keeping both sides: the new promo audit actions next to the mcp ones, the moved KYC enum schemas next to PositiveMoneyAmountSchema, the impact tool next to list-agent-tools, and dev's kycPolicy and grantLedger wallet dependencies inside the shared WalletService factory. Adapted to dev: - wallet.withdrawal.held is filed under the player with after.transactionId, like the other withdrawal lifecycle rows since the player-scoped audit change - the mcp kernel ADR is renumbered to 0042; dev took 0041
The agent hub adds behaviour and leaves existing flows alone, so the daily cap reads the auto-approved marker by the reason alone again. A held withdrawal whose reason reads "auto-approved" counts towards the cap, which fails closed.
A replace loses the race only when its first read precedes the winner's commit. The test left that to timing, so a slow runner started the second replace after the first committed and both succeeded. A third transaction now holds the player row until both replaces wait on it.
mp-blurify
marked this pull request as ready for review
October 6, 2026 06:51
mp-blurify
requested review from
damianrzepka,
jakubfilinger-b,
klaudia-blazyczek-blurify,
marek-chmielowski-blurify,
okapitula and
zaxovaiko
as code owners
October 6, 2026 06:51
zaxovaiko
requested changes
Oct 6, 2026
zaxovaiko
left a comment
Member
There was a problem hiding this comment.
Not quite yet - the precondition IAM gap, the rebindable MCP_KERNEL and the shallow output allow-list. Minor:
recordExecutionAuditreturnsaudit_unavailableafter the effect is already committed, so the caller sees a failure for an applied action.hold_withdrawalwrites the proposed reason intoreviewReason, so approving a held row keeps the hold text, and a reason ofauto-approvedcounts toward the cap.- The changeset says breaking types but bumps
minor. - No retention job for the audit row every kernel call writes. But that's for later.
added 8 commits
October 7, 2026 14:33
…ide its handler The canonical JSON behind every input and output hash recursed without a limit, so an input nested a few hundred thousand levels deep threw a RangeError while the kernel hashed it, outside any catch, and the call rejected without its audit record. Nesting past 64 levels now serializes as a fixed marker, and a tool call that throws anywhere outside its handler answers internal_error and still writes its one mcp.tool.failed record. The change matches the one already on the stacked transport branch, so the two merge without rework.
…dition checkPrecondition answered any well-formed run actor without the IAM check a tool call and an execution make, so a caller without the action type's grant could learn from add_tag whether a player is self-excluded or on a break, from hold_withdrawal whether a withdrawal exists, and from request_enhanced_kyc where a player's verification stands. The precondition now resolves the actor's admin and checks the action type's grant through AdminGuard first, answering forbidden, or internal_error when the check itself fails, before the payload is parsed or the precondition runs. AdminGuard records the denial as it does for every route; the kernel still writes no record of its own for a precondition.
…r boot createApp binds MCP_KERNEL after every plugin and configure(), but it returns the container, and a later register() of the token dropped the cached kernel and served the replacement, without the IAM check or the audit record the kernel enforces. The container gains registerSealed(), which binds a token for good: every later registration of it throws. createApp binds the kernel that way, so a binding made in configure() is still overridden and a rebind after boot is refused.
…rough The output allow-list picks top-level keys only, and registration checked the output schema's top level only, so a tool allowing "player" with a z.looseObject, a catchall, a record or an untyped field under it booted and returned whatever the handler put there, personal data included. Registration now runs the input walker's record, loose, catchall, untyped and recursive rules over the whole output schema, at every depth, without the bounds an input needs. The parse then strips every key a schema does not declare at each level below an allowed key.
…thdrawal hold_withdrawal writes the proposed reason into the withdrawal's review reason, and approval rewrote the reviewer and review time but kept the reason, so an approved withdrawal showed its approver next to the text it had been held for. An admin approval now clears the reason, as a rejection replaces it with its own; the hold's reason stays on the wallet.withdrawal.held audit record. A pending withdrawal carries no reason, so approving one is unchanged.
wallet.activity sums a wallet's completed deposits, withdrawals, bets and wins over the last 1 to 365 days, and a model may call it as often as its run allows. No index leads on (wallet_id, status, created_at), so the planner read every completed row of those types the wallet ever had through (wallet_id, type, status) and dropped the ones outside the window in the heap. The new index bounds the scan by the window. The generated migration is a plain CREATE INDEX, because the migrator applies each file inside a transaction. It holds a SHARE lock on wallet_transaction while it builds, which blocks every ledger write but no read, so a large deployment applies it in a quiet window.
The changeset lists breaking type changes for core but asked for a minor bump, so the release notes would file them under minor changes. Core now bumps major, as a breaking change does elsewhere; @openora/mcp only adds tools and stays minor. The summary also covers what the review changed: the sealed MCP_KERNEL binding, the output schema rule, the precondition's IAM check, the cleared hold reason on approval, and the wallet index the migration builds while it blocks ledger writes.
add_note recognised a replay by a note with the same player, author and text, so an approver who approved two proposals saying "reviewed, no issues" got one note and an already_applied for the second, and a note the same admin had typed in the Backoffice suppressed a proposal's note too. The note now keeps the proposal id in a nullable proposal_id column under a partial unique index, and the executor inserts with ON CONFLICT DO NOTHING on it: nothing inserted means a replay. The unique index also serialises two executions of one proposal, so the per-proposal advisory lock and the content lookup are gone. Backoffice notes keep proposal_id null and are never matched, and the note routes still do not return it. ADR-0042's trade-off, the executor contract wording and the changeset describe the new key.
Collaborator
Author
|
On the minor points:
|
mp-blurify
enabled auto-merge
October 8, 2026 05:50
…ules ADR-0042 still said binding MCP_KERNEL after every plugin keeps it from being replaced, which the returned container disproved. It now names the sealed binding, the output-schema rule at every depth, the IAM check before a precondition, and total input hashing. It also records how a tool references a player (its id, no pseudonym format in core) and a money amount (a bounded decimal string, value checked in superRefine), and that a money-moving executor keys durably on the proposal id.
# Conflicts: # packages/core/src/pam/identity/__tests__/identity-reader.service.int.test.ts # packages/core/src/wallet/drizzle/migrations/meta/0028_snapshot.json # packages/core/src/wallet/drizzle/migrations/meta/_journal.json
zaxovaiko
added a commit
that referenced
this pull request
Oct 9, 2026
## Summary Exposes the in-process MCP kernel from #264 to an admin's own MCP client (Claude Code and similar) over MCP Streamable HTTP. Every call is authenticated by an admin-issued bearer token stored only as a hash in the iam module, checked per tool by `AdminGuard`, and audited. The endpoint is off by default. This PR is stacked on #264. ## Why The kernel serves only in-process callers such as the agent hub. Admins and analysts need to query platform data from their own MCP clients, under their own grants, with the same redaction and audit as the Backoffice, and without anything appearing on the player domain. A static bearer token with a mandatory lifetime replaces an OAuth authorization server; every current MCP client supports a static header (ADR-0043). What ships: - `agents.mcp` platform config: off by default, a bound host required while enabled, per-address and per-token rate limits, a per-admin token cap and hourly issuance throttle, and a personal-field policy (dropped by default). - iam: an `mcp_token` table (hash, prefix, mandatory expiry, revocation reason, last use, call count), routes for an admin's own tokens and for oversight (list, revoke, revoke all), and two permissions: `mcp-access` (use MCP, own tokens) and `mcp-token` (oversee and revoke any token). - Transport: a stateless SDK endpoint mounted ahead of CORS, sessions and oRPC; only read-class tools, narrowed to the admin's grants; every call goes through the kernel, which audits it with the token id and the client's address; refused tool names are audited too. - Revocation: manual, and automatic on deactivation, demotion, all-sessions revocation, password reset or change, and loss of MCP access through IAM. Each automatic revocation commits in the same transaction as the change that causes it, except a password change or reset, which better-auth commits first. For those, the authenticator refuses every token issued before the admin's latest password change, so a failed revocation write fails closed. - Issuance: takes the revocation lock and re-reads the admin's MCP access and the issuing session under it, so a grant change, a forced logout or a revoke-all that commits while the request waits leaves no token behind. - Audit: the audit writer gains a batch append (one chain-head read, one sequence reservation, the hash chain unchanged), and a bulk revocation writes its per-token rows with it. - `AdminGuard.assertUser` refuses a deactivated account, which also stops hub runs acting for that admin. - ADR-0043 and `docs/guides/mcp-transport.md`. Verification: the transport was exercised end to end from Claude Code against a local platform: tool listing and calls, dropped personal fields, one audit record per call, refusals (origin, method, host, token, batch, rate limit, expiry), automatic revocation on deactivation, and Claude Code's handling of a revoked token (the header is rejected, no OAuth attempt). The review hardening that followed is covered by the E2E suite. ## Alternatives considered - **better-auth API-key plugin:** deletes expired keys, which loses the history the token list shows, and a misconfigured session hook turns a key into a session for every route. - **better-auth MCP and OAuth-provider plugins:** require OAuth with login and consent. OAuth stays the next step when a client cannot send a static header, a third party needs access, an agent runtime moves out of process, or MCP gains writes; it plugs in behind the same authenticator port. - **better-auth agent-auth plugin:** experimental, signs a fresh short-lived JWT per request, and reaches Claude Code only through a local proxy that exposes its own tools. - **Stateful MCP sessions, serving MCP through the oRPC handler, the SDK's high-level tool registration:** rejected for the reasons in ADR-0043. ## Risks - **Public API changes:** - `createIamRouter` gains a required argument. - `IamService` and `McpTokenService` take a deps object. - The iam plugin now depends on the audit module. - `AdminGuard.assertUser` refuses deactivated accounts. - Kernel descriptors publish `outputJsonSchema` projected to `redact.allow`. - `RunContext` gains optional `clientMeta` and `dropPersonal`. - `RateLimitResult` gains `unavailable`. - `AuditWritePort` gains a required `recordManyInTransaction`; a hand-written implementation or test double needs it. - `McpTokenAuthentication` gains the `credentials_changed` reason. - **Enabling the endpoint** requires installing `@modelcontextprotocol/sdk@1.29.0` (an optional peer dependency) and listing `agents.mcp.allowedHosts`. Every proxy or load balancer that can reach the app must set the `Host` header itself and reject absolute-form request targets. - **Migrations:** iam `0001` (enum, table, indexes) and `0002` (status index, consistency CHECK), both additive. - **Seed:** the predefined admin role gains both MCP permissions; roles created earlier need them granted in the role matrix. - **Locking:** IAM grant changes, bulk token revocations and token issuance take a table lock on `mcp_token` and run one at a time. - **Residual risks:** - On a password reset, better-auth deletes the sessions a few milliseconds after the token revocation commits; a token issued from an old session in that gap survives. - After a failed revocation write, the tokens the authenticator refuses still show as active and count towards the cap until revoked or expired. - The password-change check compares application clocks, so clock skew between replicas can put a token issued at the moment of a change on the wrong side of it. - **Deferred:** issuing a token does not require a fresh second factor yet; it needs a new identity port and a Backoffice step. - **Stacked PR:** the base moves to `dev` after #264 merges, by merging `dev` into this branch, with no force push. - **No changeset in this PR**, per the delivery standard; the release PR should list the API changes above. --------- Co-authored-by: Mateusz Paś <mateusz.pas@studiosoftware.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Volodymyr Zakhovaiko <zaxovaiko@gmail.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.
Summary
Adds the MCP kernel to
@openora/core: modules register agent read tools (ctx.mcp.tool) and proposable action types (ctx.actions.register) in their ownplugin.ts, and consumers reach bothonly through
MCP_KERNEL. First registrations: toolsplayer.summary,wallet.activity,kyc.status,ggr.summary; action typesadd_tag,add_note,hold_withdrawal,send_to_manual_review,request_enhanced_kyc.Why
An agent runtime and an MCP transport need to call module capabilities without importing modules, under the same IAM and audit rules as the Backoffice. A spike on real models showed what no
runtime enforces: a top-level
oneOfmade models send amounts as strings, tool output and thrown errors (including SQL text) reached the model verbatim, a restart executed an action twice, anda model proposed a bonus to a player with harm signals.
So the kernel enforces it:
AdminGuard.assertUser;Also included:
RunContext;PlatformConfig.agents: limits, retention, and a model allow-list of<gateway>/<vendor>/<model>, with the gateway list defaulting toopenrouter;agent,agent-proposalandagent-configIAM resources;on_hold);@openora/mcp(list-agent-tools);Container.registerSealed, which bindsMCP_KERNELso nothing can replace it after boot.See ADR-0042.
Alternatives considered
createAppbinds it sealed after every plugin, and a later registration throws.redact.personal) instead.Risks
ModuleRegistryneedsactions,mcp.getToolsand the two-argumentmcp.tool.PlatformConfigliteral needsagents; config files parse unchanged.ctx.mcp.toolstill registers, but the kernel does not serve it.on_holdwithdrawal, and anon_holdwithdrawal blocks deleting its wallet asset. Onlyhold_withdrawalsetson_hold. Approving a held withdrawal clears the hold's reason; the hold's audit record keeps it.checkPreconditionchecks the caller's grant like an execution, so a hub calls it with an actor that holds the action type's permission.0028adds an index onwallet_transaction (wallet_id, status, created_at)with a plainCREATE INDEX, which blocks ledger writes while it builds, so apply it in a quiet window on a large ledger; player-note0001adds a nullableproposal_idunder a partial unique index, which is instant.adminrole get the new IAM resources. Grant them to other roles before gating a screen on them.add_notekeeps the proposal id, on its note under a partial unique index, so it adds one note per approved proposal. The hub must not re-execute a proposal it recorded as executed.anonymous.
IDENTITY_READERwithout the new optionalgetUserIdByPlayerIdgets a boot warning, and four registrations answerinternal_error.