From 6061fe39ce39948274fec3240e9e7a887ce3d02b Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 9 Sep 2026 08:28:50 -0500 Subject: [PATCH] fix(bridge): a refusal is not an effect, so it does not spend the key (Phase 16a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol 7, amended in place. The at-most-once store had two cases and needed a third. A handler that THREW keeps its key and stores the failure — correct, because it may have applied half of itself, and that is the failure this file exists to prevent. A handler that RETURNED had its reply stored and replayed for ever. There was no case for a handler that ran to completion and deliberately REFUSED, and every refusal on this plane is a guard: a missing runId, an unknown item, a cap, a rate limit, a write that failed and left the value alone. Nothing happened — and remembering the refusal froze it as the key's answer, so a refusal that WAITING FIXES could never be retried past. `uo.world.save` is the case the acceptance walk found it with, and the module says in as many words what it expected: "429 is the shard's save rate limit, and it is the one refusal on this plane that waiting fixes … which is what makes a phase boundary retried rather than abandoned." It was not achievable. Six attempts over four minutes — three automatic, an operator retry, three more — every one answering the same frozen sentence, "the last save was 227 seconds ago". The number could not age because it was the first reply being replayed, and a step's idempotency key is one value for the life of the step, so the operator's retry control could not escape it either. So a refusal releases the key. The refusal is still emitted to the caller, which is what ends the attempt; it is simply not remembered as this key's answer. A refusal is recognised by its `kind` ending in `.error`, matched on the suffix so a handler family added later is covered without extending a list here. `bridge.error` is excluded deliberately — that is the reply this file writes when a handler threw, which is exactly the case whose key must be kept. Both completion paths are covered, including a deferred handler that refuses after its timer arms. The safety argument is that "nothing happened" holds for every `*.error` reply here — audited across BridgeWorld, BridgeOneShots, BridgeLeases and BridgeParticipation, where every refusal is a pre-mutation guard or follows a `Write` that failed. It is a property this file cannot verify, so it is written down as a rule handlers must keep: do not answer `*.error` after changing the world. Report a partial change in an `ok` reply, as the item grant does with `granted`/`missed` and the despawn with `removed`/`gone`/`refused`. Verified on a real shard: the same key 25 seconds apart now answers "the last save was 15 seconds ago" then "40 seconds ago" — a number that moves, with no `replayed` marker — while a SUCCESSFUL reply is still replayed unchanged, so the at-most-once guarantee is intact where it matters. `Status()` gains a `refused=` counter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4 --- .../Custom/Bridge/BridgeIdempotency.cs | 96 +++++++++++++++++-- 1 file changed, 89 insertions(+), 7 deletions(-) diff --git a/overlay/Scripts/Custom/Bridge/BridgeIdempotency.cs b/overlay/Scripts/Custom/Bridge/BridgeIdempotency.cs index f0dc9ea..d44cf68 100644 --- a/overlay/Scripts/Custom/Bridge/BridgeIdempotency.cs +++ b/overlay/Scripts/Custom/Bridge/BridgeIdempotency.cs @@ -28,13 +28,31 @@ namespace Server.Custom.Bridge /// tells the caller to come back. `bridge.busy` is deliberately not an error — the work is /// happening, and the module classifies it retryable. /// - /// ── A key that has begun is never released ──────────────────────────────────────────── + /// ── A key that has begun is never released — EXCEPT on a refusal ────────────────────── /// - /// Not even when the handler throws. Releasing it would let a retry re-run a command that may - /// have applied half of itself, which is precisely the failure this file exists to prevent. - /// A handler that throws stores a `bridge.error` reply instead, so the retry gets a definite + /// Not when the handler throws. Releasing it would let a retry re-run a command that may have + /// applied half of itself, which is precisely the failure this file exists to prevent. A + /// handler that throws stores a `bridge.error` reply instead, so the retry gets a definite /// answer and the step fails once rather than looping. /// + /// A REFUSAL is the third case, and it was missing until the Phase 16 acceptance walk. A + /// handler that ran to completion and answered `*.error` did not do anything: every refusal + /// on this plane is a guard — a missing runId, an unknown item, a cap, a rate limit, a write + /// that failed and left the value alone. Remembering it froze the answer for ever, so a + /// refusal that WAITING FIXES could never be retried past. `uo.world.save` is the case that + /// found it: the shard saves at most every 300 seconds, the module says in as many words that + /// this is "the one refusal on this plane that waiting fixes", and six attempts over four + /// minutes all replayed one frozen sentence — "the last save was 227 seconds ago" — because + /// the number was the first reply's, not the clock's. + /// + /// So a refusal releases the key: nothing happened, and the caller is free to ask again. The + /// refusal is still EMITTED to the caller, which is what ends the attempt; it is simply not + /// remembered as this key's answer. The safety argument is that "nothing happened" is a + /// property of every `*.error` reply here, and it is a property this file cannot verify — so + /// it is a rule handlers must keep: **do not answer `*.error` after changing the world.** + /// Report a partial change in an `ok` reply, as the item grant does with `granted`/`missed` + /// and the despawn with `removed`/`gone`/`refused`. + /// /// ── The one hole, and why it is loud ────────────────────────────────────────────────── /// /// The set is bounded, so an evicted key's repeat WOULD be applied a second time. The bounds @@ -75,6 +93,36 @@ namespace Server.Custom.Bridge public string Corr; // the correlation value the original reply carries public string CorrField; // which of CorrFields that value sits in public string Kind; // for diagnostics only + public bool Refused; // the reply was a `*.error`: nothing happened, so do not keep the key + } + + /// + /// Is this reply a refusal — a handler that ran and deliberately did nothing? + /// + /// Every refusal on this plane is emitted as a `kind` ending in `.error` + /// (`world.error`, `lease.error`, `oneshot.error`, `participation.error`, …). Matched on + /// the suffix rather than a list, so a handler family added later is covered without + /// anyone remembering to extend an enumeration here. + /// + /// `bridge.error` is deliberately EXCLUDED: that is the reply this file writes itself + /// when a handler THREW, and a throw is exactly the case whose key must be kept. + /// + private static bool IsRefusal(string replyLine) + { + if (replyLine == null) + return false; + + var parsed = BridgeJson.Parse(replyLine); + + if (parsed == null) + return false; + + var kind = BridgeJson.GetString(parsed, "kind"); + + if (kind == null || String.Equals(kind, "bridge.error", StringComparison.Ordinal)) + return false; + + return kind.EndsWith(".error", StringComparison.Ordinal); } private static readonly Dictionary _byKey = @@ -89,7 +137,7 @@ namespace Server.Custom.Bridge private static string _openCorr; private static string _openCorrField; - private static long _seen, _replayed, _busy, _evicted, _uncorrelated; + private static long _seen, _replayed, _busy, _evicted, _uncorrelated, _refusals; /// /// True while a keyed command's handler is running. BridgeLink.Emit checks this on every @@ -103,8 +151,8 @@ namespace Server.Custom.Bridge public static string Status() { return String.Format( - "idem(keys={0} seen={1} replayed={2} busy={3} evicted={4} uncorrelated={5})", - _byKey.Count, _seen, _replayed, _busy, _evicted, _uncorrelated); + "idem(keys={0} seen={1} replayed={2} busy={3} evicted={4} uncorrelated={5} refused={6})", + _byKey.Count, _seen, _replayed, _busy, _evicted, _uncorrelated, _refusals); } /// @@ -230,6 +278,16 @@ namespace Server.Custom.Bridge Console.WriteLine("[Bridge] idempotency: {0} under key {1} emitted no reply the sidecar could correlate", entry.Kind, key); } + else if (entry.Refused) + { + // The handler ran and refused, so nothing happened and this key is not spent. The + // refusal has already gone out to the caller; it just is not remembered as the + // answer. Without this, a refusal that waiting fixes could never be retried past — + // see the class header. + Release(key); + _refusals++; + return; + } entry.Done = true; } @@ -294,10 +352,33 @@ namespace Server.Custom.Bridge } } + // A deferred handler can refuse too — a lease whose target vanished while the timer was + // armed answers `lease.error` here rather than from inside the inbound call. Same rule: + // nothing happened, so the key is not spent. + if (IsRefusal(replyLine)) + { + Release(key); + _refusals++; + return; + } + entry.Reply = replyLine; entry.Done = true; } + /// + /// Give a key back, as though it had never been seen. + /// + /// Only ever called for a refusal — see the class header. It removes the entry from the + /// lookup; the stale key left in `_order` is harmless, because eviction re-reads + /// `_byKey` and skips what is no longer there. + /// + private static void Release(string key) + { + if (key != null) + _byKey.Remove(key); + } + /// /// Every line a keyed handler emits passes through here. Only the one the sidecar would /// correlate with THIS command is kept: an `admin.audit` broadcast that happens to be @@ -327,6 +408,7 @@ namespace Server.Custom.Bridge entry.Reply = line; entry.Corr = _openCorr; entry.CorrField = _openCorrField; + entry.Refused = IsRefusal(line); } // ---- internals ----