You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 125ad06
Browse filesBrowse the repository at this point in the historyBrowse files
fix(runtime): an api forward's two action_invocation rows name each other (outer target, inner forwarded_from) (#28389)
<!-- claude-projects-attribution: {"github_login":"os-zhuang"} -->
_Requested by **Sam** · [project
thread](https://claude.ai/code/project/chan_01LjZwNRFiG2fkp8SNf6rHNx?thread=cmsg_01LjZwNRFiG2fkp8SNf6rHNxWFCGbzLH2Q6meq4jHsVCCM)_Fixes#28281
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:`type: 'api'` 动作转发到本服务另一条动作路由时,两条 `action_invocation` 审计行互相点名 ——
外层行记声明的 `target`,内层 REST 行记 `forwarded_from`。
- 为什么改:之前两行只能按时间戳拼接,内层行看起来像人直接调的 REST,管理员分不清是 agent 发起的转发。
- 风险与回滚:只往审计行 `metadata` 里加两个可选键,不改任何门禁或答复;回滚即 revert 本 PR,老行本来就没有这两个键。
-
先看:`packages/runtime/src/action-invocation-audit.ts`(两个读取函数与头注释)、`packages/plugins/plugin-audit/src/action-invocation-audit.ts`(行形状)、`packages/runtime/src/domains/actions.ts`(REST
门写行处)。
- 两分钟验证:`pnpm --filter @objectstack/runtime exec vitest run
src/action-invocation-audit.test.ts`,预期 30 个用例全绿,含 MCP → api → 动作路由那条。
- 需要你做的:无,席位落地。
## What changes
An `api` action whose `target` is another action route on this server
answers through two doors and writes two `sys_audit_log` rows. They now
name each other instead of being joined by time alone:
- **Outer row: `metadata.target`.** On the `api` branch only, the
action's DECLARED `target`. It is the declaration, not the interpolated
path: a path built from the caller's params can carry a param value, and
this row deliberately records no params (the writer's header says why).
`actionInvocationTarget` in
`packages/runtime/src/action-invocation-audit.ts` produces it; both
doors call it.
- **Inner row: `metadata.forwarded_from`.** The forward mark the request
arrived carrying (`OBJECT/ACTION` of the outer action), read from the
same `x-objectstack-action-forward` header the inner REST door already
reads for its loop refusal (`actionInvocationForwardedFrom`). A hint,
not a surface: a forged mark mislabels this key and nothing else, and
the inner row's `surface: 'rest'` stays the door's own.
- `ActionInvocationAuditEvent` gains optional `target` / `forwardedFrom`
in both packages; `buildActionInvocationAuditRow` writes `target` only
on `branch: 'api'`.
**File surface beyond the claim's `packages/runtime/src/**`, and why:**
- `packages/plugins/plugin-audit/src/action-invocation-audit.ts`: the
row's `metadata` is shaped there from an explicit member list (the
runtime hands an event, never a row), so a new key the runtime sends is
dropped unless the writer names it. The card names this package as the
code's home too.
- `packages/runtime/src/action-execution.ts`: ONE added line in
`invokeBusinessAction`'s invocation literal. The MCP `run_action` outer
row is built there, not in `domains/mcp.ts` (which builds only
`resume_run`'s row, a `flow` branch that never forwards). The dispatch's
PM assumption placed the producer in `action-invocation-audit.ts`; that
file is the helper, and the literal lives in the hot file. The line is
an addition beside `branch:`, so it should rebase cleanly over the
released unpushed work on that file.
## Tests
- `packages/runtime/src/action-invocation-audit.test.ts`, new describe
`[#28281]`: the in-process injector routes the forward into the REAL
REST action door with the headers `forwardApiAction` actually sent, so
the inner row is written by the same door reading the same mark as on a
server.
- MCP → api → action route: two events; the outer is `surface: mcp`,
`branch: api`, `target` = the declared path, no `forwardedFrom`; the
inner is `surface: rest`, `branch: script`, `forwardedFrom:
aud_lead/forward_lead`, no `target`.
- REST → api → action route names the same pair; a direct call names
neither.
- The outer row records the declaration, not the interpolated path.
- A forged mark on a direct call is recorded as the hint it is and moves
nothing else on the row (surface stays `rest`).
- Helper pins for both readers.
- `packages/plugins/plugin-audit/src/action-invocation-audit.test.ts`:
the two rows read back through a real engine carry exactly
`forwarded_from` / `target`; a `target` on a non-`api` branch is not
written.
## Acceptance notes
- Only the REST door records `forwardedFrom`, as the card asks. The MCP
door reads no mark into its row; a forward whose target is `/mcp` itself
is not a case this PR measured, and is left as it was.
- The card's "Also noted (S2)" latency item is untouched, as dispatched.
- Dispatch asked for `pnpm lint`; the os-dev contract assigns the
repo-wide lint to CI. Ran eslint with `--no-inline-config` on the six
changed source files instead (exit 0); CI's `Lint & Repo Gates` runs the
full sweep.
## Verification
All readings at head `6e3c12b63`.
<details><summary>Package tests and typecheck</summary>
- `pnpm --filter @objectstack/runtime test`: Test Files 491 passed
(491), Tests 7977 passed, 19 skipped. Lock VERDICT command-exit 0.
- `pnpm --filter @objectstack/plugin-audit test`: Test Files 68 passed
(68), Tests 1099 passed. VERDICT command-exit 0.
- `pnpm --filter @objectstack/runtime --filter @objectstack/plugin-audit
typecheck` (includes `check:test-typecheck`): VERDICT command-exit 0.
The first run caught one TS2345 in the new rig (the in-process request
mock typed with no arguments), fixed in `6e3c12b63`.
- eslint `--no-inline-config` on the six changed source files: exit 0.
</details>
<details><summary>Ablation (each leg through
scripts/ablation-replace.mjs; restored blob equals HEAD, git diff HEAD
empty)</summary>
Tests import `src/` relatively in both packages, so no dist rebuild sits
between mutation and run.
- A: `actionInvocationTarget` returns undefined → runtime file 4 failed
/ 26 passed (the MCP and REST forward pins, the declared-target pin, the
helper pin).
- B: the REST door's `forwardedFrom` forced undefined → runtime file 3
failed / 27 passed (MCP forward, REST forward, forged-mark pins).
- C: plugin-audit's `forwarded_from` line deleted → 1 failed / 8 passed
(the read-back pair).
- D: plugin-audit's api-only guard on `target` removed → 1 failed / 8
passed (the non-api pin).
</details>
<details><summary>Dispatch gates</summary>
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 66 families derived from 7 paths vs merge
base `80b561be1`. Every command run, exit captured to disk before any
pipe; `--ran` reconciliation: 66 accounted, 64 run, 2 NOT MEASURED, 0
unrun.
- NOT MEASURED (exit 3, PREREQUISITE NOT MET: need a full `pnpm build`):
`pnpm check:dual-build-cjs-loads`, `pnpm check:i18n`. Neither reads
anything this diff changes; CI's `Lint & Repo Gates` runs both.
- `node scripts/check-plugin-teardown-shape.mjs --self-test` first
exited 3 on this shallow clone (its pinned fixture commit was
unreachable); after fetching that one commit it exits 0 (48 cases).
- Every other derived command exited 0.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QUs3BvaSHKxMfeL8P6juJm)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(runtime): the two `action_invocation` rows of an `api` action that forwards to another action route name each other
7
+
8
+
Clause-②: yes (widening)
9
+
Metadata-protocol: none
10
+
11
+
A `type: 'api'` action whose `target` is another action route on the same server answers through two doors, so it writes two `sys_audit_log` rows: the outer door's (`branch: 'api'`, `surface: 'mcp'` for an agent's `run_action`, `rest` for a click) and the inner REST door's for the action it reached. The two could only be joined by their timestamps, and the inner row read like a person's direct REST call. Now each row names the other:
12
+
13
+
-**The outer row's `metadata.target`** is the action's declared `target`, on the `api` branch only. It is the declaration, not the interpolated path: a path built from the caller's params can carry a param value, and this row records no params.
14
+
-**The inner row's `metadata.forwarded_from`** is the forward mark the request arrived carrying (`<object>/<action>` of the outer action), the same `x-objectstack-action-forward` header the inner door already reads for its loop refusal. It is a hint, not a surface: any caller can send the mark, so a forged one mislabels this key and nothing else, and the inner row's `surface: 'rest'` still names the door the request came through. Because the header is caller text bound for the ledger, it is recorded only in the forwarding door's own shape: two snake_case machine names joined by one slash, at most 255 characters. Any other value is left out of the row, and the call is answered exactly as it would be without it.
15
+
16
+
**Public surface (widening):**`ActionInvocationAuditEvent`, exported by `@objectstack/plugin-audit`, gains two optional members, `target` and `forwardedFrom`. An event that sets neither writes the same row as before. The runtime's matching event type and helpers are internal to `@objectstack/runtime`. Rows written before this change carry neither key.
0 commit comments