Skip to content

fix(start-os): let effects.action.run carry the form's eventId - #4068

Closed
helix-a wants to merge 1 commit into
masterfrom
fix/effect-run-event-id
Closed

helix-a wants to merge 1 commit into
masterfrom
fix/effect-run-event-id

Conversation

@helix-a

@helix-a helix-a commented Sep 23, 2026

Copy link
Copy Markdown
Member

What changes

The container runtime lets an effect call's own eventId through. rpcRoundFor builds each effect request's params as { ...params, eventId: <calling procedure's id> }, which overwrote the eventId that effects.action.run now carries. The params are built by a new effectParams, where a caller-named eventId wins over the tag.

Why

#4062 runs a service's action under the eventId its form was opened with, so the target's Action.run finds that form. On a VM running #4062's build (CI run 35900355405, b3da16d), Bitcoin opened Tor's Add Onion Service form and ran it. Tor's runtime log shows the two calls under different ids:

execute /actions/add-onion-service/getInput  id E4VNHKM6QZOZ27XRBBECREQSRJP43BTI → eventId E4VNHKM6…
execute /actions/add-onion-service/run       id SVS5F3TCHUS3CX33I6AUC6V4HZEPWC7P
  → Error: getActionInput has not been called for EventID SVS5F3TCHUS3CX33I6AUC6V4HZEPWC7P

Bitcoin's bundled SDK and EffectCreator.run both passed eventId: E4VNHKM6…. rpcRoundFor then replaced it with Bitcoin's own procedure id, which is null during init, so the field arrived empty and run_action fell back to a fresh Guid. #4062's unit test mocked the effects below this layer, which is why it passed.

Nothing in StartOS reads the tag. event_id appears in shared-libs/crates/start-core/src/service/effects/ only on RunActionParams and on prelude.rs's EventId struct, which nothing uses. So letting a named eventId through changes no other effect.

Verification

  • New EffectCreator.test.ts: the tag is added, a named eventId survives, and no id is sent outside a procedure.
  • npm run check and npm test (4 suites, 16 tests) pass in container-runtime, and prettier 3.8.3 passes.
  • End to end: I'll install this PR's CI build on the same VM and repeat Bitcoin → Tor. The result will go in a comment here.

The container runtime tags every effect call with the calling procedure's
event id under `eventId`, overwriting any `eventId` the call itself named.
effects.action.run now names the event id its form was opened under
(#4062), so a service's run reached StartOS with its own procedure's id,
or none, and the target refused it: "getActionInput has not been called
for EventID …". Seen on a VM running #4062's build: Tor's form opened
under E4VNHKM6…, and the run executed under a fresh SVS5F3TC….

An `eventId` the effect names now wins over the tag. Nothing in StartOS
reads the tag.

Helix-Harness: claude-code
Helix-Model: claude-opus-5-5
@helix-a

helix-a commented Sep 23, 2026

Copy link
Copy Markdown
Member Author

Superseded, per dr-bonez's review in Matrix. The runtime already tags every effect call with the calling procedure's eventId, and ConcurrentActor uses that id for re-entrancy (concurrent.rs: a message whose id matches a running handler skips its conflicts). The effect params never read it, because the field was procedure_id and serde expected procedureId. So every effect-initiated get-input and run ran under a fresh random id. The fix is to make the effects read the envelope eventId rather than let a second eventId override it. A getInput and a run from one procedure then share its id, and the explicit eventId on effects.action.run from #4062 goes away. The replacement PR follows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant