feat(webapp): mollifier trigger-time decisions — mollify, claim, read fallback#3753
Draft
d-cs wants to merge 8 commits into
Draft
feat(webapp): mollifier trigger-time decisions — mollify, claim, read fallback#3753d-cs wants to merge 8 commits into
d-cs wants to merge 8 commits into
Conversation
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d-cs
commented
May 26, 2026
d-cs
force-pushed
the
mollifier-phase-3-trigger
branch
2 times, most recently
from
5a7bc19 to
baa6f17
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
d-cs
force-pushed
the
mollifier-phase-3-trigger
branch
from
01f3958 to
449a0bc
Compare
d-cs
force-pushed
the
mollifier-phase-3-buffer
branch
from
e85e771 to
9298706
Compare
d-cs
force-pushed
the
mollifier-phase-3-trigger
branch
from
449a0bc to
ffe51b8
Compare
d-cs
force-pushed
the
mollifier-phase-3-buffer
branch
from
9298706 to
f4c5b21
Compare
d-cs
force-pushed
the
mollifier-phase-3-trigger
branch
5 times, most recently
from
e56b937 to
cae33fa
Compare
… fallback The trigger hot path's mollifier integration: - `mollifyTrigger`: when the gate trips, write the engine.trigger snapshot to the buffer and return a synthesised QUEUED response. - Pre-gate idempotency-key claim: same-key triggers serialise through Redis so a burst lands in PG / buffer exactly once. - Read-fallback extensions: `findRunByIdWithMollifierFallback` for the trigger-time idempotency lookup that must see buffered runs. - Gate bypasses: debounce, oneTimeUseToken, parentTaskRun (triggerAndWait) skip the mollify path entirely. - triggerTask + IdempotencyKeyConcern wired to the above. Stacked on buffer extensions PR. All behaviour gated by the master `TRIGGER_MOLLIFIER_ENABLED` switch; off-state hot path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`new Date("not-a-date")` returns a truthy Invalid Date object, which would
mis-classify the run as CANCELED in the read-fallback synthesised shape.
Add an `asDate` helper that rejects NaN-valued parses and use it for
`cancelledAt` and `delayUntil`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The buffer's claim API now requires a caller-supplied ownership token so compare-and-act protects the slot against a stale predecessor. Wires the token end-to-end: - `claimOrAwait` generates the token (UUID) up front and reuses it across the retry path; returns it on the `claimed` outcome. - `publishClaim` and `releaseClaim` wrappers accept and forward the token to the buffer. - `ClaimedIdempotency` carries the token so the trigger pipeline can publish or release with the same token it claimed under. - `triggerTask.server.ts` threads the token into the publish call. Tests pin token round-trip: claimOrAwait → claimIdempotency, plus the publish and release pass-through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t leak fix)
Devin review flagged a security + correctness bug: the mollify path
returned \`MollifySyntheticResult\` with run shape
\`{ friendlyId, spanId }\` and the call site cast it to
\`TriggerTaskServiceResult\` (which expects \`run: TaskRun\` with an
\`id: string\`). Downstream, the trigger route calls
\`saveRequestIdempotency(requestKey, "trigger", result.run.id)\` — so
the cache stored \`undefined\` as the entity id. On SDK retry the
request-idempotency flow then ran
\`prisma.taskRun.findFirst({ where: { id: undefined } })\`. Prisma
strips \`undefined\` from where clauses, so the query degenerates to an
unfiltered \`findFirst\` and returns an arbitrary TaskRun row —
potentially from another env / user.
Fix:
- Add \`id: string\` to \`MollifySyntheticResult.run\`.
- Compute it via \`RunId.fromFriendlyId(...)\` on both branches:
the happy-accept path (id from \`args.runFriendlyId\`) and the
\`duplicate_idempotency\` race-loser path (id from
\`result.existingRunId\` so the response carries the WINNER's id).
- Add regression tests pinning both branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uard - readFallback: read snapshot.taskVersion (the key buildEngineTriggerInput writes) instead of the nonexistent snapshot.lockToVersion, so buffered version-locked runs report their locked version; test now uses the real key as a regression guard. - env: TRIGGER_MOLLIFIER_TRIP_THRESHOLD back to positive() (matching sibling mollifier numerics) to forbid threshold=0 silently mollifying every trigger. - idempotencyKeys: document why the resolved-but-unfindable fall-through is safe (PG-unique + accept SETNX dedup + ~30s claim TTL self-heal); add regression test pinning the fall-through and the resolved-and-findable cached-hit path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove plan-tracking shorthand (Q#, C#, F#, Phase N, _plans/) from trigger-layer mollifier comments and test names; reword to plain English. Comment/test-name only; no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d-cs
force-pushed
the
mollifier-phase-3-trigger
branch
from
cae33fa to
16bfff0
Compare
…f-state token alloc triggerTask.server.ts: mollifier.buffered logs at debug, not info — it fires per mollified trigger during a burst, exactly when info-level would flood aggregation. idempotencyClaim.server.ts: move the buffer-null fall-open check above token generation so the mollifier-OFF path skips the default randomUUID() for idempotency-keyed triggers (triggerTask is the highest-throughput path). Injected test generators still honoured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
The trigger hot path's mollifier integration:
mollifyTrigger: when the gate trips, write the engine.trigger snapshot to the buffer and return a synthesised QUEUED response. Postgres write is deferred to drainer-replay (next PR in the stack).findRunByIdWithMollifierFallbackfor the trigger-time idempotency lookup that must see buffered runs.debounce,oneTimeUseToken,parentTaskRunId/triggerAndWaitskip the mollify path entirely.triggerTask+IdempotencyKeyConcernwired to the above.All behaviour gated by the master
TRIGGER_MOLLIFIER_ENABLEDswitch; off-state hot path is unchanged (the gate is not even consulted).Stacked on the buffer extensions PR.
Test plan