The duplicate Forge event that deleted our spam filter

Key takeaways

• Forge product events carry no documented delivery guarantee — the at-least-once promise belongs to the Async Events API, and the two get conflated constantly.
• The duplicate doesn't double-post. It wipes the 24h dedup marker, so the NEXT occurrence speaks when it should have stayed quiet.
• The fix is one clause: a run that ever saw a violation may not later declare the page clean.
• A version claim doesn't close this — and not for the reason we assumed. @forge/kvs DOES have atomic set-if-absent; the two guards are simply orthogonal.
• Where you store the run-scoped flag decides whether it works. Inside the retry loop it resets on the one attempt that matters.

Sentinel Vault watches Confluence pages and puts them back when someone edits a part they shouldn't have. When it does that it leaves a comment saying what it reverted. One comment, because being told nine times that your resize was undone is worse than not being told at all.

In July the logs showed something that shouldn't happen: two invocations of the same trigger, four hundred milliseconds apart, carrying the same page and the same version number. Confluence had delivered one event twice.

I assumed I knew what that meant, and wrote it up. Almost all of it was wrong, including a test I built specifically to prove it, which produced clean confident numbers for a mechanism the code doesn't have. That part is the more useful half of this post, so it's at the end rather than in a footnote.

Forge product events don't promise anything

Ask around and you'll be told Forge events are delivered at least once. It's repeated often enough to feel like documentation. It isn't. Or at least, not for the events most apps actually use.

The product events reference states no delivery guarantee. What it describes is retry machinery: your function can request a retry by returning an InvocationError object, and the platform retries automatically on four named conditions — FUNCTION_OUT_OF_MEMORY, FUNCTION_TIME_OUT, FUNCTION_PLATFORM_RATE_LIMITED, and a catch-all FUNCTION_PLATFORM_UNKNOWN_ERROR. There's a cap: "You can only retry an event for a maximum of four times."

The only thing that page says about delivery is negative. Events larger than 200 kB are not delivered at all, and the limit "may change without notice."

The at-least-once language belongs somewhere else. It's the Async Events API, where the wording is explicit: enqueued events "are guaranteed to be delivered at least once within a defined retention window." That's a queue you push to deliberately, with @forge/events. It isn't the product-event trigger firing because somebody saved a page.

The practical difference is small and the rhetorical difference is not. Product events give you no guarantee of duplicates and none against them. There's retry machinery underneath, so duplicates happen. You just can't point at a contract and say the platform told you to expect it — which is what I did in the first draft of this piece, for about four hundred words, until somebody opened the page.

Write handlers as if a duplicate will arrive. Not because a contract says so. Because one did.

The race

Call the two deliveries A and B.

Both start. Both read the page, and at that moment it's genuinely tampered — the bad edit is sitting right there. Both build a list of violations. Both intend to restore the original content and leave a comment.

A gets to the write first. It restores the body, claims a dedup marker so nothing else comments about this incident, and posts. One comment, correct behaviour.

B submits its write a fraction of a second later against a version number that's no longer current, and Confluence answers 409. The retry loop does what it should: back off and try again. The backoff is Math.pow(2, attempt) * 500, so 500ms, then a second, then two.

B retries, and re-reads the page. The page it reads is the one A already fixed.

So B looks at a clean page. Its reading of the world is accurate. It just isn't B's world; it's A's outcome, observed from the losing seat.

Here's the clause that mattered. A clean save is supposed to clear the dedup markers, because a page tampered with today and again next month deserves a fresh comment rather than silent suppression forever. B has just observed a clean page. So B clears the marker A claimed half a second ago.

What I got wrong

I wrote that up as "one user action, two comments." It's the obvious symptom, it's the one the feature exists to prevent, and it's wrong.

The obvious symptom was wrong. The losing duplicate never comments — it removes the guard, and a later delivery speaks instead.

Trace what B actually does. It re-reads the page, finds no violations, clears the markers on the clean branch, and drops out of the retry loop. It never modified the document, so anyChange stays false and the seal-restore dispatch that would have sent a comment is gated on it.

There are a couple of direct-notify paths in the same handler that don't ride a page write and so aren't gated — I checked, because "it can't post" is exactly the kind of absolute this article is about not asserting. B's clean branch returns before reaching any of them. So B posts nothing about this incident, which is a narrower claim than the one I first wrote and the only one the code supports.

The damage is quieter and lives longer. Notice markers carry a 24-hour TTL today, though a TTL bug meant older installs hold permanent copies for which the clean-save clear is the only reaper. Their job is absorbing the repeat traffic one incident generates: a stale editor draft republishing every few minutes, the same tamper re-saved, an attachment delete that legitimately fires two different events for one physical action. B deleted that window. The next occurrence inside those 24 hours, which should have been swallowed, gets a comment.

So the user-visible result is eventually a second comment on the same incident. Not from B, and not in that run. It arrives later, from a different delivery, and by then nothing in the logs connects it to the duplicate that removed the guard.

That distinction cost me time. If you go looking for a bug that double-posts within one invocation, you'll read the dispatch code, find it correctly gated, and conclude the system is fine.

How it became visible at all

Not by reasoning about it. The Forge developer console separates output by invocation on the platform side, which is the only reason the twin deliveries were distinguishable at all — two separate invocations, same page, same version, four hundred milliseconds apart.

That is worth knowing precisely because our own code contributes nothing to it. I checked while writing this: there is not a single invocation id, trace id or request id logged anywhere in the app. Every line the conflict path emits looks like this, and it identifies neither the invocation nor the page:

[PAGE-PROTECT] Version conflict, retrying in 500ms (attempt 1/3)

Two of those in a log tell you almost nothing. Without the platform grouping them for you, they could be one event delivered twice, a person who hit save twice, or an editor draft republishing — three problems, three different fixes.

It sat there for weeks before anyone noticed. The app had been running happily, and the duplicate had presumably arrived before, harmlessly, whenever the two deliveries didn't happen to straddle a write. Concurrency bugs are rare alignments more than rare events, and this one needed a duplicate and a conflict in the same second.

We're adding the page and version to that line. It should have been there from the first day the handler existed.

A 409 is the normal case

The conflict itself is the part people get wrong, and treating it as an error is what makes this class of bug unfixable.

When two writers touch the same Confluence page, one supplies a version number that's no longer current and gets a 409. That means somebody else got there first, which is information you asked for by supplying a version at all. The loser's job is to back off, re-read, and decide again.

We used to log the 409 and move on. That builds an app which abandons its work exactly when concurrency proves it was needed. Retrying without re-reading is the other failure: you resubmit the same stale version and burn all three attempts. Both look fine in a test suite where nothing else is writing.

The backoff wants to be short. Ours totals three and a half seconds worst case, which is deliberate rather than generous. This handler is already close to its ceiling, and there's a separate cap in the same file limiting how many purges one invocation will confirm, with a comment noting that a multi-purge sweep blows the budget mid-triage. Back off long enough to exhaust the function timeout and a recoverable conflict becomes a hard failure. The symptom is a handler that sometimes does nothing.

There's a second-order effect that took us a while to see. The retry isn't just a second attempt at the same work. It's a second observation of a world other writers have been changing. Every conclusion the first pass reached is potentially stale, and the interesting bugs live in the conclusions carried across that boundary without anyone noticing they were carried.

One physical action can be two events

Deleting an embedded attachment through the Confluence UI is one action to the person doing it. To Forge it's two events: the attachment is trashed, and the page is updated. Both fire, both are legitimate, and neither is a duplicate in the at-least-once sense. They're genuinely different events describing one human intention.

Dedup keyed on the event comments twice and is technically correct both times. The user doesn't care that the reasoning was sound. We aliased the two notice classes onto one physical class so a single delete can't produce two comments regardless of which event arrives first, or whether one arrives at all.

Which means several different things all present as "that happened twice", and they need different answers: a genuine duplicate delivery, two events describing one action, and a retry of your own handler re-observing a changed world. Dedup that only handles the first will still embarrass you.

The fix is one clause

The clean-save clear now requires two things:

if (violations.length > 0) probeCache.set("__saw-violations", true);

if (violations.length === 0) {
  if (attrViolations === 0 && !probeCache.get("__saw-violations")) {
    // …clear the markers…
  }
  return;
}

probeCache is per-invocation. The flag is set on the pass where violations were found and survives into the retry. A run that ever saw a violation isn't allowed to later declare the page clean.

Only a run that never saw trouble may say there was none.

It reads like a tautology once written down. It wasn't obvious in advance, because the natural thing to inspect is the page, and the page really has been fixed by the time the loser gets there. Every fact B has is true. The error is that B answers a question about the incident using evidence from after the incident was handled.

Deciding explicitly which runs are entitled to conclude "nothing is wrong here" is the whole of the fix, and it's the part with no obvious home in most codebases.

There was a third place with the same shape, and I found it by fact-checking this article rather than by reviewing the code. The sealed-section pass had its own clean-save clear, with a comment describing it as a "mirror of the media clean-save clear" — and it was missing the guard that makes it a mirror. Same retry loop, same re-read, same race, still open on that surface while the media surface was fixed. It's closed now, threaded with the same run-scoped flag, and the two passes share the key deliberately: if either saw a violation this run, neither may declare the page clean.

The attrViolations === 0 clause beside it closes a sibling hole that does double-post inside one run. A save carrying only an attribute violation isn't a clean save. If it clears the layout marker, two rapid resizes produce two comments for what the user experienced as one action, which is how that one was found — in the test suite, not in production. Any check shaped like "no violations of type X, therefore clear the markers for types Y and Z" has this bug in it.

The version claim doesn't save you

The obvious objection: dedup the delivery itself. Claim the version before doing anything, and the duplicate loses at the door.

We do that. Our claim reads the key, then writes it — two operations with a gap in the middle, so two genuinely concurrent deliveries can both pass the check before either write lands. A comment in our own source explains why that's acceptable:

KVS has no CAS — the tiny concurrent double-claim window is the same one T6 already accepts.

That comment is wrong, and I only found out because someone fact-checked this article against the installed package rather than against the comment. @forge/kvs has had an atomic conditional write for a while. It's in the type definitions we have on disk:

export interface PolicySetOptions extends SetOptions {
    keyPolicy: 'OVERRIDE' | 'FAIL_IF_EXISTS';
}

FAIL_IF_EXISTS is set-if-absent, which is exactly the primitive a claim wants. So our claim could be atomic and isn't, on the strength of a note somebody wrote when it was true.

Here's the part that matters more, and that I also had backwards: making the claim atomic would not have prevented this bug. The two guards operate on different keys. The version claim writes a validation-checked key. The clean-save clear deletes violation-noticed keys. Nothing in the media pass reads the claim. Make the claim perfectly atomic and B still enters the retry loop, still re-reads A's fixed page, still reaches the clear, and still deletes a marker it doesn't own — because that clear is a delete on a different key, not a second claim.

So the flag isn't covering what the claim can't close. The two are orthogonal, and no delivery-level dedup of any strength stops a losing retry from clearing someone else's marker.

They're also in the same handler, which I had wrong in a draft and which is worth saying because the mistake is instructive. There's no separate validation trigger — the manifest declares three product-event triggers and the validation phase is called from inside the content-protection one. Assuming two guards live in two handlers is how you end up believing one of them covers the other.

Where the flag lives decides whether it works

One implementation detail that's easy to get backwards, and I got it backwards in a draft.

The cache has to be created outside the retry loop:

const mediaProbeCache = new Map();
let anyChange = false;

for (let attempt = 0; hasBodyWork && attempt < MAX_RETRIES; attempt++) {
  let ctx;
  const { pageData, adfDoc } = await readDocBody(pageId);
  // …
}

The context object ctx is declared inside the loop and rebuilt on every attempt, because it holds the page data that was just re-read. Hang a run-scoped flag off ctx and it dies on the retry, which is the one attempt where you need it. The guard does nothing.

Module scope has the opposite problem. Whether a Forge isolate gets reused between invocations isn't something your app controls, and a module-level Map that never resets can carry a true from one invocation into the next. A flag stuck true means you never clear a marker again — quieter than the bug you were fixing, and harder to notice.

The read being inside the loop is the other half. That re-read is the entire mechanism; it's why the retry sees a different world than the first attempt did. Hoist it out for efficiency and you've removed both the bug and the reason the guard exists.

How we test it now

A guard that only matters across a retry is a guard you've never exercised if your first write always succeeds. That's the whole reason this survived a test suite for weeks.

Three things changed in how we test this class of bug.

The suite now replays the same event twice rather than performing two edits. Two different edits legitimately deserve two comments and prove nothing; the identical payload dispatched twice with a small offset is what production actually does, and almost no suite does it by default.

Conflicts get forced rather than waited for. Making the first write attempt collide on purpose is a couple of lines, and it converts "this path is exercised when we're unlucky" into "this path is exercised every run". The flags that reset on retry are invisible until you do it.

And the assertions moved off the comment count and onto the marker. The loud symptom here was going to be a second comment; the actual symptom was a marker deleted when it should have been held. Assert only on what the user sees and you'll watch this bug pass, then meet it three weeks later with nothing in the logs connecting it to anything.

That last one generalises past Forge. When a guard's job is to prevent something, the test that matters asserts on the guard's state, not on the absence of the thing. Absence is cheap to fake and easy to achieve accidentally.

The part that should bother you

I built a test to prove the double-comment story. Forty-odd lines of Node, no Forge app needed, modelling A and B and the marker between them. Run it one way and it printed two comments for one user action. Run it with the guard and it printed one.

It ran. It printed numbers. The mechanism was wrong.

The harness contained a line deciding whether B commented, based on whether the marker was still there. Nothing in the codebase works like that. B's comment is gated on B having made a change, which on the retry it never does. I'd written a gate with no counterpart in the source, and then treated its output as evidence.

The code comment sitting directly above the fix was precise. It attributes the same-run double comment to the attribute-violation clause, and says of the duplicate-delivery case only that it "would clear the marker the winner just claimed." Two adjacent bugs, carefully distinguished by whoever fixed them. I read it, fused them, and gave the duplicate the other one's symptom.

I'd even written, in that same draft, that a wrong model producing a number is easier to disbelieve than one that stays in your head. That's false when the number comes out of the model itself. A harness doesn't test the code. It tests your reading of the code, and it will agree with you enthusiastically and repeatably.

The check I use now: for every branch in the harness, name the line in the real source it corresponds to. A gate with no counterpart is a fabrication. Then trace the real path to the statement that actually dispatches, writes or returns, because the endpoint is where a plausible story stops matching the code.

What caught it wasn't re-reading the draft, and it wasn't anyone spotting the flaw in the argument — the argument was internally consistent, which is the problem with a well-built wrong model. It was running the file and diffing the output against what the draft claimed the output was.

The cost was a day and an article that had to be rewritten from the premise up. Nothing shipped, nobody was misled. It would have been a considerably worse week if the review had happened after publication instead of before.