fix(bridge): a refusal is not an effect, so it does not spend the key (Phase 16a)
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<string, Entry> _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;
|
||||
|
||||
/// <summary>
|
||||
/// 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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private static void Release(string key)
|
||||
{
|
||||
if (key != null)
|
||||
_byKey.Remove(key);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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 ----
|
||||
|
||||
Reference in New Issue
Block a user