feat(core): record an effect's intent before it runs, and re-drive it - #320
Conversation
A target body that dies halfway leaves no trace of what it was doing. There was no way to express work where that matters: posting a required status check, publishing a release, telling another system something finished. A process killed mid-post left a run nothing would ever pick up, and nothing that could tell it the post was owed. Add .effect(name, fn). The intent to run it is written to the run record and confirmed before the body runs, so a process killed anywhere inside the effect leaves a pending row, and a resume drives it again. An effect already recorded done is skipped rather than repeated. At-least-once, not exactly-once. A process that dies after the side effect but before recording it repeats the effect, which is the honest cost of never losing one. Bodies must tolerate that, and the context tells them when a previous attempt already committed. The writes are strict, unlike every other write on the writer. Each one throws rather than dropping, because a caller that cannot record its intent must not perform the effect: with no durable intent nothing knows to re-drive it. They also refuse to write to a run that is no longer running, rechecked after every conflicting re-read. Without that, a process still alive on a run some other process already settled would arm its effect and publish a result over the settled one, which for a merge gate is a green check over a red. A strict failure is kept off the writer's shared chain. Every write serialises through it, so leaving a rejection there would make each later best-effort write reject too, and one failed intent would take the run's whole bookkeeping down with it. A store is required and enabled automatically. With state explicitly disabled the run is refused rather than performing an effect nothing recorded as owed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The in-process suite can stage a half-done effect but not produce one, so it cannot show that the intent survives an unclean death or that a different process finds it. Add an e2e that kills a real subprocess inside its effect, reads the record from the parent to confirm the effect is still recorded as owed, and then runs a real resume that drives it again. The fixture appends one line per drive, so the count is observed rather than inferred. One seam is stood in for and labelled: moving an abandoned running run back to suspended is the reaper's job, and the reaper does not exist yet, so the parent performs that single transition by hand.
An adversarial pass over the effect machinery found that the effects were driven from exactly one place in the target lifecycle, and four other paths reach a target's real execution and return before it. A cache hit, a wait gate whose trigger was already satisfied, a service, and a fan-out each skipped the effects entirely — and because the run then reported success with nothing recorded as owed, the effect had not been deferred, it had never existed. That is worse than a failure: it is invisible. A target that declares an effect is no longer cacheable, since a cache hit returns before the effects and the seconds saved are not worth losing one. A satisfied wait gate now drives its effects before returning, because the gate opening is the target's real run. A service and a fan-out run no body of their own, so both refuse the declaration outright rather than accept one they would drop; the fan-out refusal follows the wait gate's existing precedent, for the same reason. A condition that skips the target still skips its effects, which is the one case where not running is correct, and that is now stated where the others are. Two effects declared under one name were also silently one effect: the name is the identity of the record row, so the first to complete made the second skip. Refused at declaration. Four fixes to the recording itself. A refused intent no longer leaves its mutation on the live record for the next write to persist, which had turned a provably-unrun effect into an armed one and made a body's first execution report itself as a re-drive. Giving up after repeated conflicts now marks the record degraded, like every other permanent loss, so a resume demands the operator's authorisation before repeating a non-idempotent step. A settlement no longer writes over an effect another process already completed. And a settle write that fails no longer replaces the effect's real error with a message about bookkeeping, which is the one thing anyone reading the incident needs. The docs and JSDoc claimed a killed process's effect is re-driven by a resume. A resume acts on a suspended run and a killed process leaves its run running, so the claim needed its precondition: the intent is durable either way, but something has to move that run back to suspended first. Stated rather than implied, pending the reaper. Every fix has a regression test. The two that mattered most are mutation-checked: reverting either makes its test fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 Zuke AI review 🔎 security review —
|
| Severity | Finding | Location |
|---|---|---|
| low | Effect error text is persisted after redaction, which can leak sensitive data into state | packages/core/src/state/writer.ts:353 |
Dismiss a false positive
Add a finding's ID to the suppress list to hide it next time:
269qplh4qt4gv— Effect error text is persisted after redaction, which can leak sensitive data into state
The change adds crash-durable effects with state-backed re-drive semantics; the main security concern is a new code path that can preserve or reveal effect errors in state, but no obvious injection, auth, SSRF, or path traversal issues were introduced.
|
🤖 Zuke AI review 🔎 generic review —
|
Review finding
Two reasons, both checkable: The line does not do that. Where the text is handled, the treatment is byte-identical to the path that already existed.
which is the same expression, verbatim, that The residual is real, pre-existing, and documented. Redaction masks the secrets the run knows about — those declared through parameters or secret sources. A third party that echoes an unfamiliar credential back inside an error string would have that string persisted, for an effect and for a target alike. That is why Not adding the id to the suppress list in |
1 participant
Why
A target body that dies halfway leaves no trace of what it was doing. There was no way to express work where that matters — posting a required status check, publishing a release, telling another system something finished. A process killed mid-post left a run nothing would pick up, and nothing that could say the post was owed.
This is R1 of the durable merge-gate work.
What it does
.effect(name, fn)on a target. The intent to run it is written to the run record and confirmed before the body runs, so a process killed anywhere inside the effect leaves apendingrow, and a resume drives it again. An effect already recordeddoneis skipped rather than repeated. A store is required and enabled automatically; with state explicitly disabled the run is refused rather than performing an effect nothing recorded as owed.At-least-once, not exactly-once. A process that dies after the side effect but before recording it repeats the effect. That is the honest cost of never losing one, and it is what the requester asked for.
Two details worth review attention:
The effect writes are strict, unlike every other write on the writer: each throws rather than dropping, because a caller that cannot record its intent must not perform the effect. They also refuse to write to a run that is not
running, rechecked after every conflicting re-read — without that, a process still alive on a run another process already settled would arm its effect and publish a result over the settled one, which for a merge gate is a green check over a red.A strict failure is kept off the writer's shared chain. Every write serialises through it, so leaving a rejection there would make each later best-effort write reject too, and one failed intent would take the run's whole bookkeeping down with it.
Third commit: the adversarial pass, which found a critical defect
Effects were driven from one place in the target lifecycle, and four other paths reach a target's real execution and return before it. A cache hit, a satisfied wait gate, a service, and a fan-out each skipped the effects and the run reported success with nothing recorded as owed — so the effect had not been deferred, it had never existed. Silent, and green.
Fixed: a target declaring an effect is no longer cacheable; a satisfied gate drives its effects before returning; a service and a fan-out refuse the declaration outright, following the wait gate's existing precedent. A condition that skips the target still skips its effects, which is the one case where not running is correct.
Also from that pass: two effects under one name were silently one effect, now refused at declaration; a refused intent no longer leaves its mutation on the live record for a later write to persist; giving up after repeated conflicts now marks the record
degradedlike every other permanent loss; a settlement no longer overwrites an effect another process completed; and a failing settle write no longer replaces the effect's real error with a message about bookkeeping.The docs claimed a killed process's effect is re-driven by a resume. A resume acts on a
suspendedrun and a killed process leaves its runrunning, so that claim needed its precondition stated — the intent is durable either way, but something has to move the run back first. That something is the reaper, in a later PR.One correction to carry forward
A test written to prove that effect inputs are pinned across a re-drive failed, which is how this surfaced:
resumeRunmerges the recorded parameters with any freshly supplied ones, and fresh wins. That is deliberate — it is the only way to re-supply a secret, since secrets are kept out of the record — but it means a parameter is pinned only if nobody re-supplies it. The docs now say that, and three tests cover it, including one that demonstrates the override. A value that must not drift belongs inctx.state.Tests
donerules, the degraded marking, and a record claiming zero attempts being refused.redrivenreporting, all three parameter/state pinning cases, and one regression per confirmed finding.SIGKILLed inside its effect, the record read from the parent showing the effect still owed, and a genuinely separateresumedriving it again — marker file showing exactly two drives,intentAtpreserved across the process boundary. One seam is stood in for and labelled: moving an abandonedrunningrun back tosuspendedis the reaper's job.Full gate green locally, 18 of 18 targets, plus the e2e suite.
🤖 Generated with Claude Code