Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions __tests__/stream/maintenance-drain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,14 @@ describe("unfreezeChannelsAfterMaintenance β€” reverse what was frozen, not what
// no-op costing one rate-limited call; missing one is permanent.
expect(result.unfrozen).toBe(2);
expect(result.source).toBe("derived");

// #1302 review β€” the union path still RETIRES what the ledger held. Gating
// retirement on `source` (now "derived") stranded those ids forever: the
// set never shrank, so every later OFF transition re-unfroze them, burning
// the 300/min budget and reopening channels the #1303 dormancy sweep had
// since frozen on purpose.
const retired = mockSrem.mock.calls.flatMap((c) => c.slice(1));
expect(retired).toContain("webinar-recorded");
});

it("does not union when the ledger is known complete", async () => {
Expand Down
14 changes: 12 additions & 2 deletions __tests__/stream/stream-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,12 @@ describe("Stream Client Module", () => {
it("throws StreamUnavailableError (and reports it) when the breaker is OPEN with no fallback", async () => {
mockWithCircuitBreaker = jest.fn(() =>
Promise.reject(
new Error("Redis circuit breaker is OPEN - service unavailable"),
// #1302 review β€” the exact string `createCircuitBreaker("stream")`
// throws (lib/redis.ts). The old fixture said "Redis", which passed
// only because the guard matches the substring "circuit breaker is
// OPEN" β€” so it was asserting against a message this path cannot
// produce.
new Error("stream circuit breaker is OPEN - service unavailable"),
),
);

Expand All @@ -532,7 +537,12 @@ describe("Stream Client Module", () => {
it("runs the fallback (no throw, no Sentry) when the breaker is OPEN", async () => {
mockWithCircuitBreaker = jest.fn(() =>
Promise.reject(
new Error("Redis circuit breaker is OPEN - service unavailable"),
// #1302 review β€” the exact string `createCircuitBreaker("stream")`
// throws (lib/redis.ts). The old fixture said "Redis", which passed
// only because the guard matches the substring "circuit breaker is
// OPEN" β€” so it was asserting against a message this path cannot
// produce.
new Error("stream circuit breaker is OPEN - service unavailable"),
),
);

Expand Down
16 changes: 15 additions & 1 deletion actions/maintenance/drain-sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,14 @@ export async function unfreezeChannelsAfterMaintenance(): Promise<{
unfrozen: 0,
errors: [] as string[],
source: "none" as UnfreezeSource,
// #1302 review β€” provenance and ledger-retirement are different questions.
// The union path reports "derived" (the set is only best-effort complete)
// while still CONTAINING ledger entries, so gating retirement on `source`
// left those entries in `FROZEN_CHANNELS` forever. Not merely untidy: the
// set is re-unfrozen on every later OFF transition, spending the 300/min
// budget on it and reopening channels that a different subsystem β€” the
// #1303 dormancy sweep β€” froze deliberately in the meantime.
usedLedger: false,
};

try {
Expand Down Expand Up @@ -527,7 +535,7 @@ export async function unfreezeChannelsAfterMaintenance(): Promise<{
// the ledger deliberately, so the next OFF transition tries it again
// instead of leaving it silently frozen forever β€” which is the failure
// this whole ledger exists to make impossible.
if (result.source === "ledger") {
if (result.usedLedger) {
await retireFrozenChannels(unfrozenInBatch, result);
}
}
Expand Down Expand Up @@ -566,6 +574,7 @@ export async function unfreezeChannelsAfterMaintenance(): Promise<{
async function resolveChannelsToUnfreeze(result: {
errors: string[];
source: UnfreezeSource;
usedLedger: boolean;
}): Promise<string[]> {
try {
// Same reasoning as the write: no fallback, because an open breaker read
Expand All @@ -585,6 +594,7 @@ async function resolveChannelsToUnfreeze(result: {

if (ledger.length > 0 && !incomplete) {
result.source = "ledger";
result.usedLedger = true;
return ledger;
}

Expand All @@ -594,6 +604,10 @@ async function resolveChannelsToUnfreeze(result: {
// conversation permanently unwritable. The asymmetry decides it.
const derived = await deriveChannelsToUnfreeze();
result.source = "derived";
// Ledger ids are in the returned set, so they are still retirable β€” and
// `srem` on an id the set never held is a no-op, so passing the derived
// ids through with them costs nothing.
result.usedLedger = true;
return Array.from(new Set([...ledger, ...derived]));
}
} catch (err) {
Expand Down
Loading