Repository navigation
feat(mcp): native business-action execution (list_actions / run_action) - #2307
Merged
Merged
Conversation
Make the open-source MCP server (`@objectstack/mcp`) natively expose business
action execution, so a self-host / Community-Edition runtime (open framework +
@objectstack/mcp, no cloud studio) can *operate* an app over MCP — run its
business actions/flows — not just CRUD records.
Action execution previously rode on `@objectstack/service-ai`'s tool registry,
which the MCP server only ever optionally duck-typed. With the in-UI `ask`
agent moved out to the closed cloud package, that registry is empty in a
headless CE runtime, so BYO-AI via MCP could read/write rows but never trigger
business logic. Action execution is an open *mechanism*, so it now lives in the
open framework.
@objectstack/mcp:
- `registerActionTools()` registers two native tools alongside the object-CRUD
tools, via a `McpActionBridge` seam (mirrors `registerObjectTools` exactly):
- `list_actions` — enumerate invokable business actions the caller may run.
- `run_action` — invoke an action by name with { recordId, params }.
- Wired into `handleHttpRequest` by capability (only when the runtime bridge can
resolve the action mechanism — graceful degradation, like record resources
needing a dataEngine). No dependency on `@objectstack/service-ai`.
@objectstack/runtime:
- `buildMcpBridge` now resolves + dispatches actions through the framework's own
mechanism — `IDataEngine.executeAction` (script/body) / automation flow runner
(flow) — bound to the caller's ExecutionContext: the same permission + RLS
path the REST `/actions/...` route uses. `list_actions` is permission- and
visibility-filtered; `sys_*`-object actions are held back fail-closed.
- The ADR-0066 D4 `requiredPermissions` capability gate is single-sourced
(`actionPermissionError`) and enforced for both the REST route and run_action.
Tests:
- mcp: `mcp-action-tools.test.ts` — tool shapes, delegation, capability gating,
fail-closed sys_* guard, tool-error surfacing.
- runtime: `http-dispatcher.test.ts` — real `buildMcpBridge` listActions filtering,
run_action executeAction dispatch (via the action's target), ADR-0066 D4 gate.
- examples/app-todo: `mcp-actions.test.ts` — CE E2E booting the real engine (no
service-ai) and driving the MCP runtime over JSON-RPC: list + execute a real
action end-to-end, permission-enforced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
size/xl
documentation
Contributor
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 17 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 6, 2026
fix(app-shell): give freshly-created records a back link to their origin (#2307) objectui@5eea2017aa42103a0f84b493aa5b3cb716361b1a
This was referenced Jul 9, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 9, 2026
…2714 Phase 0) (#2715) The skill's tool list covered 7 of the 9 native tools — the business-action pair from #2307 was missing, so skill-installed agents never learned they can run app-registered actions directly. Adds the two tools with their real signatures, weaves actions into the intro / when-to-use / discover / workflow sections, and teaches action preference over hand-editing records. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Sep 17, 2026
…of silently skipping it (objectstack-ai#17620) (objectstack-ai#17802) Fixes objectstack-ai#17620 ## 1. The history read — run FIRST, and it clears the fork Triage ruled REMOVE on a **stated premise** — that the guard was never a recorded decision — and told the claimant to verify that premise before writing any diff, with an explicit fork: deliberate-with-a-stated-reason means stop, no diff, decision box. The verification is below. **The premise holds, so the fork does not trigger.** The clone this started in was **SHALLOW** (`git rev-parse --is-shallow-repository` = `true`; 3,265 commits; the oldest commit touching `action-execution.ts` inside that window dated 2026-09-08). A pickaxe answered from truncated history would have attributed the guard to the 2026-07-27 extraction commit — confidently, at exit 0. The clone was unshallowed first (`git fetch --unshallow`: 13,713 commits, `--is-shallow-repository` = `false`), and every reading below is taken on that full history. ### Where it came from **Introduced:** `7d7fee71cc24daf9857543ed0375750ff21c0dc2` — 2026-06-25, os-zhuang, `feat(mcp): native business-action execution (list_actions / run_action) (objectstack-ai#2307)`. Found with `git log --all -S 'if (id != null) await ql.delete'`, which returns exactly two commits: this one and the relocation below. **Control on the same corpus:** `git log --all -S 'buildActionEngineFacade'` returns 13 commits — so the two-commit answer is a reading, not a broken search. **What that commit says about the guard: nothing.** It is an 11-file, 1,193-insertion feature commit. Its message describes the MCP action-execution surface — `registerActionTools`, the `McpActionBridge` seam, `IDataEngine.executeAction` dispatch, the ADR-0066 D4 permission gate — and never mentions ids, nullish values, skipping, or tolerance of any kind. The guard arrives as one unremarked line inside the facade the commit introduces. **The one comment beside it explains the OTHER tolerance** — the array one, the one PR objectstack-ai#17608 made declared. Quoted from that commit's own diff: ```ts + // Tolerant of both the single-id and array conventions handler suites + // use (CRM handlers pass one id; todo handlers pass an id array). + async delete(object: string, idOrIds: string | string[]): Promise<void> { + const ids = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; + for (const id of ids) { + if (id != null) await ql.delete(object, { where: { id } }); + } + }, ``` That comment is about `Array.isArray(idOrIds)`. It gives a reason for accepting two ARGUMENT SHAPES. It gives no reason for skipping a nullish ELEMENT, and nothing else in the commit does either. The distinction matters here more than usual: the stated reason attaches to the tolerance that has since been **declared**, and the undeclared one it sits next to inherited none of it. **Relocated, not decided:** `3216344dfa1377109ed8d70dd3181a444157cc9b` — 2026-07-27, `feat(runtime): extract action-execution subsystem — ADR-0076 D11 step ③ PR-8 (objectstack-ai#2462)`. A mechanical extraction of 16 helpers; occurrence counts either side of it are **1 before** (`http-dispatcher.ts`) and **1 after** (`action-execution.ts`), so it moved the line rather than duplicating or authoring it. Its message describes the extraction and says nothing about the guard. **Untouched by the card that made its neighbour declared:** `ea2940d1` (objectstack-ai#17608) edited only the comment two lines above. The guard line itself is byte-identical across it. ### Four further searches, each reported with what makes its result a reading | search, full history, all refs | result | why that result is a measurement | |---|---|---| | `git log --grep='nullish' -i` | 15 commits, none about this guard | the search returns a populated set, so the absence of this guard from it is a reading | | `git log --grep='null id' -i` | 1 commit, about seed-time `os.user.id` resolution | as above | | `git log --grep='skip.*null' -iE` | 8 commits, none about this guard | lit control on the same corpus: `--grep='ActionEngineFacade' -i` returns 3 | | `git grep 'ActionEngineFacade' -- docs/adr` | ADR-0096 only, and only about identity/context binding (`buildActionExecutionContext`) — nothing about ids | the grep finds the symbol, so the absence of an id ruling is a reading | And one absence measured in the working tree rather than in history: **no test anywhere pins the skip.** The only pin on this value says the opposite — `packages/spec/src/ui/action-params.test.ts` carries `// @ts-expect-error — "delete nothing" is the EMPTY ARRAY, never a null id`. ⇒ **No commit message, no code comment, no ADR and no test states a reason for the nullish skip.** The premise triage named is confirmed rather than assumed, the fork stays shut, and the ruled direction is what this PR implements. ## 2. The diff `packages/runtime/src/action-execution.ts` — located from the **symbol**, `buildActionEngineFacade`, not from the card's line number. As it happens `:1474` was still the guard when this claim opened, and the report says so rather than leaving it implied. The loop's `if (id != null)` is gone. Every id now reaches `ql.delete(object, { where: { id }, context })` as written. **Why removing it is enough to make the value loud, and why that is not an accident of some particular engine.** `ObjectQL.delete` dispatches on one question, and the answer is a shared, gated predicate rather than a local `if`: `resolveEngineDeleteDispatch` in `@objectstack/metadata-core` routes `by-id` only for a **truthy scalar** `where.id`, falls to `multi` only when `options.multi` is truthy, and otherwise **rejects**. The facade sets no `multi`, so a nullish id lands on the reject arm and the call throws `ENGINE_DELETE_REJECT_MESSAGE` — `Delete requires an ID or options.multi=true`. There is no path on which the nullish id could instead widen into a predicate delete: `multi` is absent, so the unscoped-multi arm is unreachable from here. No new refusal vocabulary was invented for this. Adding a facade-level ADR-0112 code would have been a new declaration, which is the half of this card that is explicitly above a seat. ## 3. Acceptance — driven readings, red before green Both runs are the same command on the same tree, with only the guard between them. **Before (guard present), `src/action-engine-facade-nullish-id.test.ts`: 3 failed | 3 passed.** ``` × refuses a nullish ELEMENT of the array form instead of skipping it AssertionError: expected undefined to be an instance of Error × refuses a nullish SINGLE id (the non-array spelling) the same way AssertionError: expected undefined to be an instance of Error × stops AT the nullish element — ids before it are deleted, ids after it untouched TypeError: Cannot read properties of undefined (reading 'message') ``` `expected undefined to be an instance of Error` IS the defect, stated by the instrument: the call resolved, carrying nothing, having deleted nothing. **After (guard removed): 6 passed (6).** The three that were red are the refusal readings; the three that were green throughout are the controls, and they are the half that proves the removal took nothing else with it: - a well-formed single id still deletes, with the elevated caller envelope still on the call; - the **declared array form** still issues one `ql.delete` per id, in order — `['case_1','case_2','case_3']` arrives as exactly that; - an **empty array** still deletes nothing and resolves, which is the member doc's own sentence. Those three passing BEFORE the fix is what makes them controls rather than decoration: they were already true, and the diff had to leave them true. **The refusal is pinned by name, never by `toThrow()`.** The assertion compares `err.message` **exactly** against the imported `ENGINE_DELETE_REJECT_MESSAGE`, and asserts `err instanceof Error` beside it. A bare `toThrow()` would stay green against any unnamed `Error` — including whatever an unfixed arm might throw for an unrelated reason — which is precisely the shape this card is about. This particular refusal is a plain `Error` carrying no ADR-0112 `code`/`status`, so the exported constant is the whole of its named surface and the pin says so in the test's own docblock. **The double is bound to the producer, not to a copy of it.** The fake engine's `delete` opens with `assertEngineDeleteDispatch(options)` — the same predicate `ObjectQL.delete` dispatches on. A hand-rolled id check in the double would have let this file stay green against a facade that still swallowed the value, which is the `check:engine-double-contract` failure mode exactly. The new double is recorded in `scripts/engine-double-contract.pinned.json` (regenerated with `--write`: **1 added or grown, 0 lost**). ## 4. Verification — every reading below taken at `3d2026622`, the final commit | what | result | |---|---| | `pnpm --filter '@objectstack/runtime^...' build` (dependency closure) | exit 0 | | `vitest run --project local src/action-engine-facade-nullish-id.test.ts` | **before: 3 failed / 3 passed** · **after: 6 passed (6)** | | `pnpm --filter @objectstack/runtime test` (whole package) | exit 0 — **260 files / 3,619 tests passed** | | `pnpm --filter @objectstack/runtime typecheck` | exit 0 — `tsc --noEmit` clean; test layer compiles, debt ledger unmoved at 27 files / 191 errors / 69 pinned signatures | | `pnpm lint` (repo-wide, `eslint . --no-inline-config`) | exit 0 | | derived gate families — `scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` | **68 commands, all exit 0** | | `scripts/pm/dispatch-gates.mjs --ran <record>` reconciliation | exit 0 — *68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN* | | `pnpm check:engine-double-contract` | exit 0 — *862 pinned*, after `--write` recorded **1 added or grown, 0 lost** | | `scripts/pm/check-clause2-carriers.mjs --pair 17802` | exit 0 — declaration readable, both carriers agree, no widening tell | Two of the 68 first answered **exit 3** — `check:dual-build-cjs-loads` and `check:type-check-debt`, both `PREREQUISITE NOT MET`, which is those gates' own distinct code for *nothing was measured* and is neither a pass nor a finding. Both read built output that a fresh worktree does not have. After `turbo run build --filter='./packages/*' --filter='./packages/*/*'` (72/72 tasks successful) both were re-run and both exit 0; that is the number in the table. What is **outside** this account, stated rather than implied: the 48 artifact-roster families, the 11 declared wide-population families, the 5 path-scheduled CI jobs and the always-runs tail are each outside the derived 68 and are CI's. The card's integration-shaped consumers — the dogfood suites that drive `examples/app-todo`'s `deleteCompletedTasks`, the one first-party caller of the array form — are in that set and are declared to CI here rather than claimed. Clause-②: no This PR puts no new key on any published payload, so the level axis stands down and the changeset is a `patch`. The reasoning is worth writing out because the direction is the interesting part: what changes is an **undeclared runtime tolerance being withdrawn**, which SHRINKS the accepted set toward the type that is already published — `string | string[]`, which has never admitted nullish. Nothing is added to a schema, an index, an export or an accepted-value set; no declaration moves in either direction. It is still an observable change, and for exactly one population: **untyped hosts**, who used to get a silent resolve and now get a named rejection. The changeset is written for them, with the one-line fix, and does not read as a correction of PR objectstack-ai#17608 — because it is not one. That PR's contract sentence is true: under the declared type no typed caller ever reached this line. ## Acceptance notes - `packages/spec` was **not touched**. The `never a null id` comment at `action-params.test.ts` is cited as evidence throughout and edited nowhere. - **No sibling sweep.** The history read surfaced no family — the pickaxe over the full history returns this one site, and the sweep the filing seat declined to run is still not run here. Nothing was filed, because nothing was found to file. - Noted, not filed: `buildActionEngineFacade`'s `insert` arm still reads `(res as any).id ?? (data as any).id`, and `find` still unwraps `rows.value`. Both are pre-existing, both are outside this card's one-guard fence, and neither is a defect anyone has demonstrated — an observation, and it stays one. Successor: whoever next opens this facade under a card of its own. --- Authored by the `domain:cli` execution seat on the dispatch at objectstack-ai#17620 (comment) — session https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c. Durable attribution is stated here in prose because a PR body's trailing footer block on this surface is the platform's to write, not the author's. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This branch was successfully deployed
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
1 participant
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.
Why
The open-source MCP server (
@objectstack/mcp, Apache-2.0) let a BYO-AI client (Claude Code, Cursor, …) read/write records but could not operate the app — run its business actions/flows. Action execution rode on@objectstack/service-ai'stoolRegistry, which MCP only ever optionally duck-typed (it must not hard-depend on service-ai). Now that the in-UIaskagent moved out to the closed cloud@objectstack/service-ai-studio, the framework AI runtime is headless and that bridged registry is empty in a CE runtime → MCP could CRUD rows but never trigger business logic.An action is an open mechanism, not closed "intelligence", so this moves action execution into the open framework — bridged directly from the framework's action mechanism, with no dependency on
@objectstack/service-ai.What
Two native MCP tools, registered exactly like the existing object-CRUD tools (
registerObjectTools→registerActionTools), bound to the caller's principal:list_actionsrun_action{ recordId, params }.@objectstack/mcp(mcp-http-tools.ts)registerActionTools()+McpActionBridgeseam (McpActionSummary,RegisterActionToolsOptions). Owns the tool shape; delegates execution + security to the runtime bridge — the same split asregisterObjectTools.handleHttpRequestby capability: only registered when the runtime bridge can resolve the action mechanism (graceful degradation, mirroring how record resources need adataEngine).package.jsonunchanged — no@objectstack/service-ai.run_actionmarkeddestructiveHint(clients confirm);sys_*-object actions rejected fail-closed at the tool layer.@objectstack/runtime(http-dispatcher.ts)buildMcpBridgenow implementslistActions+runAction, resolving + dispatching through the framework's own mechanism —IDataEngine.executeAction(script/body) / automation flow runner (flow) — bound to the request'sExecutionContext: the same permission + RLS path the REST/actions/...route uses.list_actionsenumerates headless-invokable actions (script/flow; UI-onlyurl/modal/formexcluded), filtered to what the caller may run;sys_*-object actions held back fail-closed.requiredPermissionscapability gate is now single-sourced (actionPermissionError) and enforced for both the REST action route andrun_action(no drift).Referenced the now-moved
service-ai/src/tools/action-tools.tsfor the resolution + invocation contract (resolve by name → dispatch via the action'starget→executeAction), and reproduced it directly in the runtime bridge without importing service-ai.Permissions / safety
requiredPermissionsenforced before dispatch;list_actionshides what the caller can't run.sys_*-object actions excluded by default (fail-closed) at both the tool layer and the bridge.Tests
packages/mcp/src/mcp-action-tools.test.ts— tool shapes, capability-gated registration, delegation, fail-closedsys_*guard, tool-error surfacing (mirrors the object-tools http test).packages/runtime/src/http-dispatcher.test.ts— realbuildMcpBridge:list_actionsfiltering,run_actiondispatching via the action'stargethandler key, ADR-0066 D4 gate deny/allow, modal/unknown rejection.examples/app-todo/test/mcp-actions.test.ts— CE end-to-end: boots the real ObjectQL engine + app-todo handlers (NO service-ai) and drives the realMCPServerRuntimeover JSON-RPC.run_action complete_taskflows throughengine.executeAction→ the registered handler → the driver; verifies the record becomescompleted, plus the fail-closedsys_*guard and the requiredPermissions gate (deny → allow) end-to-end. (pnpm --filter @objectstack/example-todo test:mcp)Full suites green:
@objectstack/mcp53/53,@objectstack/runtime441/441. Both packages typecheck clean.Acceptance
@objectstack/mcp, no cloud studio) lists and executes the app's business actions, permission-enforced.@objectstack/mcphas no@objectstack/service-aidependency; existing object tools unchanged.packages/mcp/src/andpackages/runtime/src/.🤖 Generated with Claude Code