mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-31 03:47:12 +08:00
Vikhyath MondretiandClaude Opus 5
763f3aa1b5
* fix(deployments): stop superseded activations from dead-lettering 29 workflow.deployment.prepare.v2 events dead-lettered with "Webhook registration operation is stale", every one at attempts = max_attempts. A full retry budget means the failure is deterministic, which rules out the preparation path: an attempt superseded while preparing is marked superseded, so its next attempt short-circuits at the top of the handler and completes. The branch a retry re-enters is the other one. isTerminalNonActiveOperation covers failed and superseded but not active, so an attempt that activated and was then superseded by the next deploy keeps its own active status, re-enters post-activation work on every retry, and re-fails the same generation fence until the event dies. The fence it fails is correct — it takes the same workflow row lock the generation bump takes, and compares generations exactly — so nothing about the detection is racy; only the reaction to it was wrong. Reaching it needs a handler timeout, which parks the row for the 10-minute reaper instead of the 2s/4s/8s backoff, opening a window wide enough for a redeploy to land. Gate the resume branch on the operation still owning the current generation, matching the sibling cleanup that already does this, and complete the event as a no-op when it does not. The newer generation adopts the leftover work anyway: it collects every retired registration below its own fence. Also reverse the post-activation order. The audit entry, analytics event, socket notification, and workspace event describe a cutover that is already durable, and each is separately checkpointed, but they ran behind retiring the previous generation's external subscriptions — one provider call per retired row, and by far the most failure-prone step there. A single flaky provider silently cost the deploy its audit trail and left clients on the old version until something else refreshed them. Both call sites now share one helper so the order cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(deployments): pin staging's active-resume tests to the current generation Two tests that staging added alongside durable PostHog delivery drive the `operation.status === 'active'` resume branch this PR now gates, and neither overrides `mockIsDeploymentOperationCurrent` — the suite's `beforeEach` defaults it to `false`. Under the new gate they read as superseded, so the handler short-circuits: the delivery test sees a resolve where it asserts a rejection, and the unconsumed workflow row leaks into the next test. Both are still-current resumes by intent, so they say so explicitly. The checkpoints, not the generation gate, remain what keeps analytics from being captured twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analytics): stop PostHog delivery from failing a deploy `deliverOutboxServerEvent` awaited `client.flush()` before letting the deployment outbox checkpoint advance, so an unreachable PostHog failed the event, retried it, and eventually dead-lettered it — while holding the socket notification, the workspace event, and retired-subscription cleanup behind a third party. `flush()` also drains the whole shared client queue, so an unrelated event's network error surfaced here as a failed deploy. It bought no durability the process did not already have: the outbox handler runs in the long-lived app container, where the client flushes on its own 10s interval and again from the `SIGTERM`/`SIGINT` hook in `instrumentation-node.ts`. The helper had one caller and arrived inside an unrelated squashed PR (#5273) with no rationale, against 161 fire-and-forget `captureServerEvent` call sites. Deleted it and restored `captureServerEvent`, whose contract is already "never throws". `insertId` still collapses retried captures. That contract was untested, which is why this regressed unnoticed, so `server.test.ts` now pins it — spying on the real client, since the lazy `require` defeats `vi.mock` and a disabled client would pass every assertion vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(deployments): fence the cleanup, not the notifications The resume guard sat at the top of the `active` branch, so it skipped every post-activation step whenever `isDeploymentOperationCurrent` went false. That predicate goes false as soon as any newer generation row exists — including one still `preparing` or already `failed` — and in that window this activation is still the live cutover. Nothing newer would ever adopt its audit entry, analytics event, socket notification, or workspace event, so the guard permanently dropped them and completed the outbox event as if they were owed to someone else. Only the two cleanups are generation-fenced. `cleanupInactiveDeploymentsForOperation` already gated itself on that exact predicate and returned quietly; the retired webhook cleanup was the one that instead let the store's `assertCurrentOperation` throw. It now carries the same guard, on the same fence the store asserts — `deploymentVersionId` and `statuses: ['active']` included, so passing the gate actually implies passing the assert. The notifications run unconditionally, still idempotent through their checkpoints. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>