Repository navigation
Commit d79dc48
fix(runtime)!: a param-less declarative update refuses undeclared wire params at both action doors (#24392)
Fixes #24268
Clause-②: no (narrowing). Basis: the two action doors stop accepting an
undeclared wire param on a param-less `operation: 'update'` action, and
nothing newly accepted or exported is added. The envelope changes at the
MCP door (the ADR-0104 D2 refusal, and the declarative executor's own
refusals, now carry `code` + `status`) add members to refusals that
already refused; they accept nothing new. Owes one
`CONTRACT_REVIEW_TIER` review before the queue.
Dispatched by the `domain:cli` seat 1
(`session_01SYEJqDL3NXxrJRBVqaMu9T`, claim comment 6254028639); this dev
ran as a subagent of that session. Branched from `origin/main`
`b2e78701f` (one commit past the claim's read `d65826fa7`: #24373, which
touches neither runtime file nor `.objectui-sha`).
## Measured before writing
- **P1, the defect.** On `b2e78701f`, `enforceActionParams` returned
`null` at once when `action.params` was empty or absent, and
`declarativeUpdateWrite` merged `{ ...patch, ...params }`. Measured with
this PR's conformance cases run against the unchanged source: 3 of 33
red. The REST door answered **200** (expected 400) for `params: { note:
... }` and for `params: { status: 'reopened' }` on the param-less
`close_case` (`patch: { status: 'closed' }`). The MCP `run_action` door
**resolved** instead of refusing.
- **P2, the console half.** In objectui, `git merge-base --is-ancestor
fb501bc894 0707b4ae37` exits **0** (ancestor). Exit 0 proves it even
though that checkout is shallow (`--is-shallow-repository` answers
`true`). `.objectui-sha` on this branch's base is `0707b4ae37`. At that
sha, `ActionRunner.executeUpdateOperation` posts `{ ...collected }` and
never `patch`. `serverActionHandler` strips `_rowRecord` and POSTs `{
recordId, params }`. So the vendored console does not post the patch as
params, and this change does not break its declarative-update buttons.
## What changed
- **`enforceActionParams`**
(`packages/runtime/src/action-execution.ts:1678`). The early return now
applies only to a param-less action that is **not** a declarative
update. For `isDeclarativeUpdateAction(action)`, the declared list
resolves to `[]`, and the existing validator then refuses every key that
is not a dispatcher built-in (`recordId`, `objectName`, `_selectedIds`).
The route is the existing D2 path: the same message (`Invalid action
params: Unknown action param "KEY" ...`), the same 400 at the REST door,
and the same `OS_ALLOW_LAX_ACTION_PARAMS=1` escape hatch, which warns
and accepts. No new switch, permission, capability or config key. The
docblock sentence "Actions that declare no `params` keep the
pass-through" is replaced by a section saying which param-less actions
keep it (those with a handler) and why a declarative update does not.
- **MCP door envelope** (`action-execution.ts:2542`, in
`invokeBusinessAction`). The D2 throw was a bare `Error`, which
`@objectstack/mcp`'s `thrownRefusalEnvelope` renders message-only,
because it reads a string `code`. It is now thrown with `code:
standardErrorCodeForHttpStatus(400)` (`VALIDATION_ERROR`, the code the
REST door's `deps.error(paramError, 400)` derives) and `status: 400`.
This applies to every D2 refusal at that door, not only the new one (see
Acceptance notes).
- **The declarative executor's own refusals**
(`declarativeUpdateRefusal`, `action-execution.ts:2221`: no `recordId`,
nothing to write, an object-less action key). They threw `status` only,
so over the real `/mcp` bridge they rendered message-only (measured; see
Tests). They now carry `code: standardErrorCodeForHttpStatus(status)`
beside the status, as the D2 throw does. The REST door already derived
the same code from the status, so its answers are unchanged.
- **Docblocks that became false or incomplete:**
`declarativeUpdateWrite`; the `invokeBusinessAction` input comment
("silently accepted on one that declares none"); and
`declarativeUpdateRefusal`'s ("No `code` literal is stamped here ...
agree by construction"), which the measurement disproved.
- **Published prose this PR made false**, edited only at the sentence
(other lanes' files, listed for the seat's cross-lane declaration):
- `content/docs/automation/flows.mdx`: "the same way an action with no
`params` is untouched". It now names the declarative-update exception.
- `packages/spec/src/contracts/ai-service.ts`: the TSDoc on
`AI_ACTION_CONFIRMATION_MEMBER`, "On an action declaring no params that
check is a pass-through". This is a comment only; there is no schema,
`.describe` or generated artifact.
- Checked and **not** false: the spec `patch` `.describe` (the
precedence is unchanged), and `liveness/action.json` `/props/patch`
("the wire cannot widen the bag (an undeclared param is refused by
ADR-0104 D2)"), which this PR makes true for the param-less case too.
`content/docs/protocol/objectui/actions.mdx` and
`content/docs/getting-started/build-with-claude-code.mdx` describe
param-less updates invoked with no params; those still write.
- **Changeset:**
`.changeset/24268-declarative-update-undeclared-params-refused.md`,
`@objectstack/runtime: minor`, `fix(runtime)!:`, line-initial `Clause-②:
no (narrowing)`, ADR-0087 `not-required (no-migration-prescription)`,
and a `**BREAKING**` sentence naming the consumers:
objectstack-ai/hotcrm's `complete_task`, consoles vendored before
objectui `fb501bc894`, and page action rows that forward static object
`params` to a param-less declarative update (none in this repo; the row
surface is #24394).
## Family: every door that executes a declarative update
`git grep` over `packages/**` for `executeDeclarativeUpdateAction`,
`declarativeUpdateWrite`, `isDeclarativeUpdateAction`,
`invokeBusinessAction` and `runAction` (non-test sources).
| Door | Path to the write | Verdict |
|:---|:---|:---|
| REST `POST /actions/OBJECT/ACTION[/ID]` (all three Hono mounts and the
fallback seam reach `handleActionsRequest`) | `domains/actions.ts:676`
`enforceActionParams`, then `:728` `executeDeclarativeUpdateAction` |
**Refused**: 400 `VALIDATION_ERROR`, nothing written (pinned) |
| MCP `run_action`: the runtime bridge `domains/mcp.ts` `runAction`, and
`@objectstack/mcp` `mcp-http-tools.ts` `bridge.runAction` |
`invokeBusinessAction`, then `enforceActionParams`, then
`executeDeclarativeUpdateAction` | **Refused**: thrown with `code`
`VALIDATION_ERROR` + `status` 400, nothing written (pinned) |
| MCP `resume_run` (`domains/mcp.ts` `resumeActionRun`) | none | Not
reachable: its candidate filter is `action.type === 'flow' &&
!isDeclarativeUpdateAction(action)` |
| List view `bulkActionDefs` `operation: 'update'` (the def
`declarativeUpdateWrite`'s docblock names) | none: objectui
`useBulkExecutor` writes through `dataSource.update` / `bulkUpdate` (the
data API) at `0707b4ae37` | Not reachable: it is not an action door and
never calls `declarativeUpdateWrite`. Its collected values come from the
def's own strict `params[]`. |
| Anything else | `executeDeclarativeUpdateAction` has exactly the two
callers above. `objectql`'s `action-governance.ts` and `lint`'s
`validate-ai-tool-references.ts` only read `operation` for inventory and
linting. | Not reachable |
## Consumers, measured
`git grep -E "operation: ?['\"]update['\"]"` over `examples/**`,
`packages/apps/**`, `packages/qa/**` and every test, at `ee0888855`:
- **Param-less declarative update actions in this repo:** test fixtures
only. `packages/runtime/src/action-declarative-update.test.ts`
(`close_case`, standalone `escalate_case`, `flag_case`,
`global_update`),
`packages/runtime/src/dispatcher-unique-violation-wire-code.test.ts`
(`take_email`, posted with a `{}` body), and
`packages/mcp/src/mcp-action-refusal-details.test.ts` (`close_case`,
behind a fake bridge, no params). Two docs examples are invoked with no
params: `resolve_ticket` (getting-started) and `task_complete`
(protocol/objectui/actions). There are **zero** in `examples/**`,
`packages/apps/**` and `packages/qa/**`: every hit there is an RLS
policy, a `bulkActionDefs` entry or a batch op.
- **Callers that send `params` to one:** zero in this repo before this
PR. Every fixture invocation sends no `params` or an empty bag. No test
or dogfood pin broke: the full runtime suite is green (below), so no
fixture needed a fix.
- **Out of repo:** objectstack-ai/hotcrm's `complete_task`, called with
params, and any console vendored before objectui `fb501bc894` (both from
the card); and page action rows (`action:button` / `action:icon` with
`operation: 'update'`) whose static object `params` the console forwards
to a param-less declarative update action, zero of them in this repo
(the row surface is #24394). All three are named in the changeset.
## Tests
New cases, all on the real doors: `HttpDispatcher.handleActions` for
REST, and `invokeBusinessAction` wired to the real `callData` for MCP.
The rig is the file's existing row-scoped, identity-honest engine
double.
- `packages/runtime/src/action-declarative-update.test.ts`, describe
`#24268`:
- REST: an undeclared key on param-less `close_case` gives 400,
`error.code` `VALIDATION_ERROR`, a message naming `"note"`, **zero
updates**, and the row is equal to `freshRow()`.
- REST: a `patch` key sent as a param (`status`, the console shape
before `fb501bc894`) is refused the same way, with no write.
- MCP: the same key is thrown with `code` `VALIDATION_ERROR`, `status`
400, the key named, zero updates, and the row unchanged.
- An absent or empty bag (`{}` and `{ params: {} }` at REST; `{ recordId
}` and `{ recordId, params: {} }` at MCP) still writes exactly `{
status: 'closed' }`.
- A declared param still writes and wins over `patch` at both doors: `{
status: 'triaged', priority: 'urgent' }`.
- Control: a param-less handler action (`nudge_case`) with the same
undeclared key still reaches the registry (404 not-found,
`executeAction` called). The pass-through is kept.
- `packages/runtime/src/action-declarative-update.test.ts`, describe
`#15079 point 7` (patch round 1): the MCP door answers the no-`recordId`
refusal (`close_case`) and the nothing-to-write refusal (`note_case`, a
new params-only fixture action) with `code` `VALIDATION_ERROR` +
`status` 400, and writes nothing. Those are the two members
`thrownRefusalEnvelope` reads.
- `packages/runtime/src/action-params-enforcement.test.ts`: the gate
itself. A param-less update, with `params` absent or `[]`, refuses an
undeclared key and names it. It accepts `{}` and the built-ins (the
positive controls). `OS_ALLOW_LAX_ACTION_PARAMS=1` accepts the key and
warns once. The existing handler pass-through case was retitled and
kept.
Real-bridge measurement (patch round 1; a throwaway dogfood file, not
committed): `bootStack` + `MCPServerPlugin`, `POST /mcp` `tools/call
run_action` on a fixture object, so the runtime's `buildMcpBridge` throw
is rendered by `@objectstack/mcp`. On `ee0888855`, no `recordId` and
nothing-to-write each answered `isError: true` with the bare message and
no JSON envelope, while the D2 control answered
`{"error":{"code":"VALIDATION_ERROR",...,"status":400}}`. On
`6466a6c05`, with runtime rebuilt, all three answered
`{"error":{"code":"VALIDATION_ERROR","message":...,"status":400}}`, and
the success control wrote.
Runs at `6466a6c05`, each through `scripts/pm/os-verify-lock.sh`:
- The two files: `Test Files 2 passed (2)`, `Tests 42 passed (42)`,
`VERDICT command-exit 0`.
- The full runtime suite (`pnpm --filter @objectstack/runtime exec
vitest run --project local --maxWorkers=2`): `Test Files 421 passed
(421)`, `Tests 6625 passed | 19 skipped (6644)`, `VERDICT command-exit
0`.
## Reverse verification
The fix was committed first (`ee0888855` for legs A and B, `6466a6c05`
for leg C). Each leg went through `node scripts/ablation-replace.mjs` in
WRAP mode, so the restore is armed on exit, INT and TERM against the
absolute path.
- **Leg A, the gate reverted** (`if (!declaresParams &&
!isDeclarativeUpdateAction(action)) return null;` replaced by `if
(!declaresParams) return null;`). The anchor went from 1 hit to 0, and
the blob from `afc25f537838` to `23bc0e53137a`. **Exactly 5 red, 36
green.** Red were the three door refusals (REST undeclared, REST
patch-key, MCP) and two gate cases (refuses an undeclared key; the lax
switch's warn). Green were the empty-bag, declared-param and
handler-control cases, as predicted. Restore: blob `afc25f537838` equals
HEAD, and `git diff HEAD` is empty.
- **Leg B, the MCP envelope reverted** (the throw went back to `throw
new Error(paramError);`). The blob went from `afc25f537838` to
`cf8007e82ea1`. **Exactly 1 red**: the MCP refusal case, with `expected
undefined to be 'VALIDATION_ERROR'`. Restore: blob equals HEAD, and `git
diff HEAD` is empty.
- **Leg C, the executor refusal's code dropped** (patch round 1:
`declarativeUpdateRefusal` back to `Object.assign(new Error(message), {
status })`). The blob went from `f379ccfb9e51` to `257cb3d3a474`.
**Exactly 1 red, 41 green**: the new MCP point-7 case, with `expected
undefined to be 'VALIDATION_ERROR'`. The REST point-7 cases stayed
green, because the REST door derives the code from the status. Restore:
blob `f379ccfb9e51` equals HEAD, and `git diff HEAD` is empty.
- Observed direction: red, the expected direction in all three legs.
`git status --porcelain` was empty after each.
## Gates (local, at `6466a6c05`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 110 commands, the same set as at
`ee0888855`. After `turbo run build --filter='!@objectstack/docs'` (76
tasks, 63 cached), all 110 exited 0 on the first pass. (At `ee0888855`,
`check:skill-examples` and `check:dual-build-cjs-loads` first exited 3,
`PREREQUISITE NOT MET`, and exited 0 after the same build.)
- `dispatch-gates --ran`: `110 derived, 110 run, 0 NOT-MEASURED, 0
UNRUN` (a derived zero: every line carries its exit code).
- The 11 declared-wide families (`init-service-contract`,
`live-db-isolation`, `meta-type-normalized`, `optional-error-sink`,
`resume-authority-declared`, `route-envelope`, `runner-env-posture`,
`settings-bind-window`, `startup-registry-verdict`, `verify-stand-in`,
`wildcard-fallthrough`): all exit 0.
- The roster gates whose roster sits under a touched directory
(`check-changeset-fixed`, `check:meta-url-spelling`,
`check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`, `check:route-ledger-census`): all exit 0.
- `pnpm --filter @objectstack/runtime run typecheck` exited 0 at
`6466a6c05`. `pnpm --filter @objectstack/spec run typecheck` exited 0 at
`ee0888855`; round 1 does not touch `packages/spec`. Both new test files
are in `tsconfig.test.json`'s program (`--listFiles`), and
`check:test-typecheck` is OK.
- `eslint --no-inline-config` over the 4 changed TypeScript files exited
0; in round 1, over the 2 it touches, exit 0.
- Changeset gates: `check-changeset-no-major` (no major) and
`check-adr-0087-registration` (`1 declared-breaking changeset ...
not-required (no-migration-prescription)`) both exit 0.
- Not run locally, declared to CI: `test:repo`, the full workspace
typecheck lanes, and `pnpm lint` (the repo-wide scans). The spec
package's own test suite was also not run: the spec diff is one TSDoc
sentence, its typecheck and every derived spec family ran, and CI's
`Test Core` runs the suite.
- `origin/main` has since moved to `584b4225e` (6 commits past the
base). None touches this diff's files or behaviour, and `git merge-tree`
is clean. Not merged, per the claim: a merge happens only for a
conflict, a main-caused red or a dequeue.
- CI on `ee0888855` showed no red. Its six `Test Core` shards were
cancelled when `6466a6c05` was pushed, and they rerun on that head.
## Acceptance notes
1. **ADR-0104 D2's text states the boundary this PR narrows.** Its lines
195-196 say "Actions declaring no `params` keep today's pass-through
(there is nothing to check against), so existing param-less actions are
untouched". Lines 941-942 say "Actions declaring no `params` keep their
pass-through — that boundary is unchanged from D2's landing". Both were
written before declarative updates existed (ruling #14092 came later),
when every action had a handler. This PR keeps that boundary for handler
actions and narrows it for `operation: 'update'`. Prime Directive #13
asks for an amended status line or an addendum on the ADR when a
recorded decision changes. `docs/adr/**` is Tier H and outside this
claim's surface, so it is untouched here. The seat filed the ADR record
as #24393 (Tier H, its own PR, `Blocked-by: #24268`).
2. **Page action rows.** An `action:button` / `action:icon` row with
`operation: 'update'` may carry an OBJECT `params` of static values. The
spec's row judge counts those as values to write
(`action-declarative-update.ts`, `staticParams`; the row `operation`
describe says "a `patch` or `params` gives the values to write"). The
console forwards them as wire params. After this PR, they are written
only when the action the route resolves by name declares them; a
param-less declaration refuses them. The row's own `patch` has not
reached the route since objectui `fb501bc894`: the route writes its
declaration's `patch`. There are zero page rows with `operation:
'update'` in this repo. The seat filed the row surface for the spec lane
as #24394. This PR only names the rows in the changeset.
3. **Built-ins** (`recordId`, `objectName`, `_selectedIds`) stay
allowlisted by the D2 validator and still reach
`declarativeUpdateWrite`'s bag. The engine's undeclared-field write door
then refuses them as `INVALID_FIELD` unless the object declares such a
field. This is unchanged here.
4. **MCP envelope scope.** The `code` + `status` stamp covers every D2
refusal at `run_action`, including declared-params handler actions,
which used to render message-only. The declarative executor's own
refusals (`declarativeUpdateRefusal`: no `recordId`, nothing to write,
an object-less key) were measured message-only over the real `/mcp`
bridge and now carry the derived code too (patch round 1; see What
changed).
5. **Stale analogies in internal comments, outside this surface.**
`packages/services/service-automation/src/engine.ts` (about line 8601)
and `screen-input-contract.ts` (about line 64) cite
`enforceActionParams`' param-less pass-through as their model. They
remain accurate for handler actions and were not edited.
6. **Handler actions** that declare no `params` keep the pass-through.
They are out of this card's scope.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 62e2b6d commit d79dc48
6 files changed
Lines changed: 265 additions & 22 deletions
File tree
- .changeset
- content/docs/automation
- packages
- runtime/src
- spec/src/contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1193 | 1193 | | |
1194 | 1194 | | |
1195 | 1195 | | |
1196 | | - | |
| 1196 | + | |
| 1197 | + | |
1197 | 1198 | | |
1198 | 1199 | | |
1199 | 1200 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
90 | 99 | | |
91 | 100 | | |
92 | 101 | | |
| |||
103 | 112 | | |
104 | 113 | | |
105 | 114 | | |
106 | | - | |
| 115 | + | |
107 | 116 | | |
108 | 117 | | |
109 | 118 | | |
| |||
323 | 332 | | |
324 | 333 | | |
325 | 334 | | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
326 | 445 | | |
327 | 446 | | |
328 | 447 | | |
| |||
500 | 619 | | |
501 | 620 | | |
502 | 621 | | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
503 | 649 | | |
504 | 650 | | |
505 | 651 | | |
| |||
0 commit comments