feat: L0 T0 #6 PR-OPAQUE-7 — consume static fallback ticket pool (ADR-0002 §3.4) #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/l0-t0-opaque-7-fallback-pool"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Final piece of the L0 Tranche-0 #6 PR-OPAQUE arc. Wires the realtime-service consumer side of the static fallback ticket pool per ADR-0002 §3.4 + the contract documented in
flux-applications/apps/centrifugo/scripts/mint-fallback-tickets.sh.The pool is loaded ONCE at startup into an in-memory
Set<string>; degraded-mode connects look up tickets via O(1) Set membership, atomically delete on consumption, and decrement the new Prometheus gauge.Decision flow
On
/centrifugo/connectwhen identity-service Validate fails withUNAVAILABLE/DEADLINE_EXCEEDED:OutageDetector.notifyFailure(now).(now - lastSuccessMs) <= FALLBACK_DEGRADED_WINDOW_MS→ 503 (today's behaviour).pool.has(ticket)→ 200 +info.degraded=true+ atomically delete.On any Validate response branch (success/disconnect/error),
OutageDetector.notifySuccessruns — a wire response of any kind proves channel reachability.NOT_FOUND/INVALID_ARGUMENTremain pure rejection paths — no fallback bypass for those.Files
Application code (new):
src/fallback/pool.ts— loader (parses one-ticket-per-line file → Set; LF + CRLF; whitespace tolerant; security-sensitive — no ticket logging)src/fallback/outage.ts—OutageDetector(windowMs gate; notifySuccess clears, notifyFailure increments; cold-start safe; lock-free, single-threaded-event-loop atomic)Application code (modified):
src/config.ts—FALLBACK_TICKETS_PATH/FALLBACK_DEGRADED_WINDOW_MS/FALLBACK_LOW_WATERMARKenv varssrc/handlers/connect.ts— fallback decision wired into the catch block; new helpersconsumeFallbackTicket+buildFallbackPoolResult;FALLBACK_USER_PLACEHOLDERsentinelsrc/metrics.ts— new outcomefallback_pool_consumed+ new gaugerealtime_fallback_pool_sizesrc/main.ts— boot-time pool load + gauge initialisation; INFO log carries pool statussrc/server.ts— acceptsfallbackPool+outageDetector+ watermark deps;/healthJSON surface extended withidentity_outage+fallback_pool.sizesrc/types.ts—ConnectOutcomeaddsfallback_pool_consumedTests (new):
tests/fallback.pool.test.ts(14 tests) — parseLines, missing file, empty file, 10k load, low-watermark WARN, whitespace handling, no-ticket-loggingtests/fallback.outage.test.ts(10 tests) — cold-start, window-not-yet-elapsed, degraded after window, notifySuccess clears, consecutive count, boundary exact-vs-past-window, negative windowMs rejectiontests/handlers.connect.fallback.test.ts(9 tests) — happy fallback acceptance, ticket-not-in-pool 503, pre-degraded 503, DEADLINE_EXCEEDED parity, NOT_FOUND no-bypass, single-use second-call rejected, low-watermark WARN once, success clears detector, no ticket loggingTests (modified):
tests/handlers.connect.test.ts— adapted to newConnectHandlerDepsshapetests/metrics.test.ts— gauge + new outcome label coverage (+7 tests)tests/server.test.ts—/healthshape assertionDocs:
README.md— new "Fallback pool consumption (ADR-0002 §3.4)" section + env-var table extensions.env.example—FALLBACK_*knobs documentedOut of scope
centrifugo-fallback-ticketsSecret + setting theFALLBACK_*env vars in the deployment manifest. That is a follow-up PR (PR-OPAQUE-5-FLUX-FOLLOWUP-fallback-mount) in theflux-applicationsrepo. THIS PR ships the application-code side only.Verification
npx tsc --noEmit --noUnusedLocals --noUnusedParameters— cleannpm test— 8 files, 82 tests passing (was 41 → +41)npm run build— cleandocker build .— cleanbq:pr-review:active/:waitingempty before pushTest plan
info.degraded=truedocumented for downstream proxiesOutageDetector.notifySuccessplacement on all Validate response branches soundflux-applications/apps/realtime-serviceoverlay adds Secret mount + env varsWire the realtime-service consumer side of the static fallback ticket pool. Per ADR-0002 §3.4 + the contract documented in flux-applications/apps/ centrifugo/scripts/mint-fallback-tickets.sh: the file is loaded once at startup into an in-memory Set<string>; degraded-mode connects look up the ticket via O(1) Set membership, atomically delete on consumption, and decrement the Prometheus gauge. Decision flow on /centrifugo/connect when identity-service Validate fails with UNAVAILABLE / DEADLINE_EXCEEDED: 1. OutageDetector.notifyFailure(now). 2. If (now - lastSuccessMs) <= FALLBACK_DEGRADED_WINDOW_MS → 503 (today). 3. Else if pool is empty → 503 (today). 4. Else if pool.has(ticket) → 200 + degraded. 5. Else → 503 (today). On any Validate response branch (success / disconnect / error), the OutageDetector resets — a wire response of any kind proves channel reachability. The "error" branch from identity-service is its own transient signal (Redis unreachable inside identity-service) but does NOT mean identity-service is unreachable from our side. NOT_FOUND / INVALID_ARGUMENT remain pure rejection paths — no fallback bypass. Design decisions (load-bearing): * Per-process monotonic depletion. The Set is loaded ONCE at boot and drains until process restart. NO online repopulation. ADR-0002 §3.4 documents this as "Pool drains as connections consume tickets; alert when below 1k remaining" + recovery via quarterly operator rotation (mint-fallback-tickets.sh --force + pod rollout). Online repopulation would defeat the single-use contract. * Fail-closed fallback identity. Fallback-pool connections expose NO real user / family / subscription context — identity-service is the source of truth and it's unreachable. The result body uses a sentinel user ("fallback:degraded") + info.degraded=true. Downstream subscribe / publish / rpc proxies (future PRs) MUST fail-closed on every sensitive operation when info.degraded=true. * Atomic single-use via has+delete. Pool consumption is synchronous has()/delete() with no await between — Node single-threaded semantics make the race-window zero. Double-check via the return of delete(). * Low-watermark crossing. WARN logged exactly once per process when consumption crosses below FALLBACK_LOW_WATERMARK (gauge is the durable signal; log is for triage). Startup WARN if initial size already below. * Local-dev safe. Default FALLBACK_TICKETS_PATH points at the production Secret mount; missing-file is a startup WARN (the local-dev path), not a fatal error. Empty-string disables the path entirely. * Security (rule 01). Ticket VALUES are never logged on any path — startup, consumption, or watermark. Pool size + outage age are the only observable quantities. Test coverage: 41 → 82 (+41 tests across 4 files). Out of scope for this PR: the Flux deployment overlay that mounts the centrifugo-fallback-tickets Secret + sets the FALLBACK_* env vars. That is PR-OPAQUE-5-FLUX-FOLLOWUP-fallback-mount and lives in flux-applications/. Refs ADR-0002 §3.4, mint-fallback-tickets.sh header contract.Show previous round
hib-pr-reviewer review — PR #2 (affinity-intelligence-rework/im2be-realtime-service)
Round 1 — head
5e9d351dbb37, basemain, triggeropenedTL;DR: NEEDS_WORK — kept 3 findings (2 agreed, 1 unique-to-A verified): one major config bug makes the documented disable-by-empty-string path unreachable, one minor JSDoc/implementation mismatch in the fallback user sentinel, one info-level misleading parameter on notifyFailure.
Summary
Arbitration — PR #2 im2be-realtime-service (Round 1)
Prior run history: None — first arbitration for this PR.
No-history submodule context: No stored patterns for this service.
Reconciliation
Finding 1 (F1) — Agreed A + B, verified MAJOR: Both reviewers independently flagged
FALLBACK_TICKETS_PATH=""being swallowed byreadString. Verified atsrc/config.ts:85–94: the helper checksvalue === undefined || value === ""and returns thefallbackargument in both cases. The fallback forFALLBACK_TICKETS_PATHis the production Secret mount path (/var/run/secrets/...), NOT empty string. Consequence confirmed: settingFALLBACK_TICKETS_PATH=in a ConfigMap does NOT produceconfig.fallbackTicketsPath === ""— it produces the production path, so theif (path === "")disabled branch inpool.ts:72is dead code from any env-var configuration. JSDoc, README, and.env.exampleall document this as the disable mechanism. B'smajorseverity is the more accurate assessment: the production safety risk (operator believing fallback is disabled when it isn't because the Secret may be mounted at the default path) is real and verified.Finding 2 (F2) — Agreed A + B, verified MINOR: Both reviewers flagged
buildFallbackPoolResultignoring_ticket. Verified atsrc/handlers/connect.ts:363–374: function signature isbuildFallbackPoolResult(_ticket: string), the_ticketargument is never read, and the returneduseris alwaysFALLBACK_USER_PLACEHOLDER("fallback:degraded"). The JSDoc at lines 352–362 explicitly describes a sha256-derived sentinel ("fallback:<sha256(ticket)[0..12]>"). The discrepancy collapses all concurrent fallback connections into one Centrifugo user bucket. Both reviewers agree on minor severity — kept.Finding 3 (F3) — Unique to A, verified INFO: Reviewer A flagged
notifyFailure(_now)discarding its parameter. Verified atsrc/fallback/outage.ts:94–101: the implementation ignores_nowentirely; onlyhasAnyFailureand the counter are updated. The@param nowJSDoc implies the timestamp is recorded, but it is not. Callers inconnect.ts:177pass a realnow. This is by design (degraded gate depends only onlastSuccessMsset bynotifySuccess), but the JSDoc misleads. Verified correct at info severity — kept.Blast Radius
The diff spans 16 files across config, fallback logic (new outage detector + pool loader), the connect handler, metrics, server wiring, and tests. The fallback pool and outage detector are new modules injected into the existing connect handler. The config bug (Finding 1) affects startup behaviour in every deployment environment; the sentinel mismatch (Finding 2) affects Centrifugo-side observability for any degraded-mode connection.
BLAST_SCORE: 5/10
Risk Indicators
consumeFallbackTicket,buildFallbackPoolResult,loadFallbackPool,OutageDetector.isInDegradedModeCI status (head
5e9d351dbb37)No CI checks reported for this commit.
Findings (3)
[MAJOR]
FALLBACK_TICKETS_PATH=""is silently absorbed byreadString; the documented "disable by empty string" path is unreachable via env varsrc/config.ts:156
readString(lines 85–94) collapses bothundefinedand""to thefallbackargument:For
FALLBACK_TICKETS_PATHthe fallback is"/var/run/secrets/centrifugo-fallback-tickets/fallback-tickets.txt"— the production Secret mount path — not empty string. Therefore:FALLBACK_TICKETS_PATH=""in a Kubernetes ConfigMap givesconfig.fallbackTicketsPath === "/var/run/secrets/..."not"".if (path === "")disabled branch inpool.ts:72is dead code from any real env-var configuration..env.example("Empty value disables the path"),src/config.tsJSDoc ("Empty string DISABLES fallback acceptance"),pool.tsJSDoc status table,pool.ts:62param doc, andREADME.md("SetFALLBACK_TICKETS_PATH=\"\"to disable the path entirely").Production risk: an operator following the runbook who sets
FALLBACK_TICKETS_PATH=""in a ConfigMap will find that if the Secret is mounted at the default path, the pool loads normally and fallback remains active — silently defeating the intended disable.Minimal fix: introduce a helper that treats
""as a valid configured value (meaning disabled) rather than falling through to the default:This makes
FALLBACK_TICKETS_PATH=actually work as documented and removes the confusing ENOENT WARN on dev laptops.[MINOR]
buildFallbackPoolResultaccepts_ticketbut never reads it; JSDoc describes a per-ticket sha256 sentinel that is not implemented — all fallback sessions share one Centrifugo user identitysrc/handlers/connect.ts:363
The function signature is
buildFallbackPoolResult(_ticket: string)and the docblock at lines 347–362 states:But the implementation returns
FALLBACK_USER_PLACEHOLDER("fallback:degraded") unconditionally;_ticketis never read. All fallback connections share a single Centrifugo user identity.Behavioural consequence: Centrifugo uses
userin its presence, connection-limit, and unsubscribe-by-user APIs. With a static sentinel, N simultaneously degraded connections all appear as the same Centrifugo user, collapsing per-connection tracking into one bucket.Note:
FALLBACK_USER_PLACEHOLDERis tested and exported, so any fix to Option A below must update the corresponding test assertions (e.g.tests/handlers.connect.fallback.test.ts:124).Two acceptable resolutions:
Option A — implement the sha256 derivation as documented:
Option B — keep the static sentinel intentionally and fix the comment:
and remove the incorrect sha256 description from the JSDoc.
[INFO]
notifyFailure(_now)accepts and silently discards its timestamp parameter — JSDoc and callers imply it is recordedsrc/fallback/outage.ts:94
The
@param nowJSDoc says "wall-clock at the time the failure was observed", implying the timestamp is stored. It is not — onlyhasAnyFailureand the counter are updated. The degraded gate isnow - this.lastSuccessMs > windowMs, driven entirely bynotifySuccess, not by failure timestamps.In
connect.ts:177, the caller passes a realnow(deps.outageDetector.notifyFailure(now)) under the apparent belief it matters. It does not.Fix (either approach): Remove the
_nowparameter entirely and update callers:Or, at minimum, add a clarifying note to the existing JSDoc and remove the
@param nowline so future maintainers understand the parameter is intentionally absent.Verdict
NEEDS_WORK
hib-pr-reviewer • round 1 • 3 findings (1M/1m/1i) • 2026-05-25T01:31:25.463Z → 2026-05-25T01:33:45.729Z • posted-as: pr-reviewer-bot
Show previous round
hib-pr-reviewer review — PR #2 (affinity-intelligence-rework/im2be-realtime-service)
Round 2 — head
3833938e8294, basemain, triggersynchronizeTL;DR: NEEDS_WORK — kept 4 findings (1 agreed minor, 2 unique-A minor+info, 1 unique-B minor); all 3 R1 findings confirmed resolved.
Summary
Round 2 Arbitration — 4 findings kept (3 minor, 1 info)
Memora persistence
Made best-effort Memora persistence; all tool calls were denied by the permission layer. No prior Memora history found for this PR — treating as first recorded run.
R1 disposition (all three resolved)
All three R1 findings confirmed resolved by both reviewers and independently verified:
readStringAllowEmptyatsrc/config.ts:194;ticketactively hashed atsrc/handlers/connect.ts:382;notifyFailure()parameter removed atsrc/fallback/outage.ts:102. These are closed and NOT re-raised.Reconciliation summary
src/fallback/pool.tslines 94–98): Both reviewers independently flagged the same root cause — the catch block does not distinguishENOENTfromEACCES/EIO/other. Log message says "file missing" andstatusis"missing"regardless of error class. Verified: lines 94–98 confirm a single uniform return path covering all errors. Kept at minor (B's severity, higher than A's info; both agree on the defect).src/handlers/connect.tsline 378):buildFallbackPoolResultreturns{ user, info }with noexpire_at. Verified: lines 378–393 confirm absence.buildSuccessResult(line 425–436) conditionally setsexpire_at; the asymmetry is real. Kept.src/fallback/pool.tsline 90): Comment says "fail open / to an empty Set" but an empty pool means every degraded-mode connect gets 503 — that is fail closed for callers. Verified: line 90 contains exact phrase. Kept.src/config.tsline 198): Guard rejects< 0but silently accepts0. Verified:isInDegradedModeatoutage.ts:121uses>(strict), sowindowMs=0means any ms after boot satisfies the condition — first gRPC fault immediately opens the gate. Constructor comment at line 31–34 explicitly documents cold-start protection, which breaks atwindowMs=0. Kept.Blast Radius
This PR touches 9 production source files spanning config loading, the connect handler (auth-path hot code), the outage detector, pool loader, health endpoint, and metrics. The fallback ticket consumption is a security-sensitive code path — it is the mechanism by which un-authenticated connections are accepted during identity-service outages. Errors in this path affect every connection attempt made while identity-service is unreachable.
BLAST_SCORE: 6/10
Risk Indicators
buildFallbackPoolResult,loadFallbackPool,isInDegradedMode,consumeFallbackTicket,connectHandlerCI status (head
3833938e8294)No CI checks reported for this commit.
Related PRs
Findings (4)
[MINOR] catch block emits wrong log message and
status: "missing"for non-ENOENT read errors — operator cannot distinguish mis-configured RBAC from unprovisioned Secretsrc/fallback/pool.ts:94
Lines 86–98: the
catchblock has a single execution path regardless oferrno. ForENOENT(Secret not yet provisioned) AND forEACCES/EIO/ any other class, the logger emits"fallback ticket pool file missing — degraded-mode fallback DISABLED"and returnsstatus: "missing". An operator whose pod has the Secret mounted at the correct path but lacks read permission (wrongfsGroup/ missingdefaultMode) would seesize: 0in/health, the literal text "file missing" in the WARN log, and waste time verifying Secret provisioning rather than checking RBAC.The
errnofield is present in the structured log payload (line 95), which helps a careful reader, but the human-readable message body actively misleads.Concrete fix — branch on
errnoinside the catch:Also extend
FallbackPoolLoadResult.statusto"loaded" | "disabled" | "missing" | "empty" | "unreadable"and surface the status string in the/healthresponse alongsidesize. Alertmanager / a human reader can then distinguish the two failure classes without grepping logs.[MINOR]
buildFallbackPoolResultemits noexpire_at— fallback-accepted connections persist indefinitely after identity-service recoverssrc/handlers/connect.ts:378
buildFallbackPoolResult(lines 378–393) returns{ user, info }with noexpire_atfield. By contrast,buildSuccessResult(lines 425–436) conditionally setsexpire_atfromsuccess.expiresAt— this is the field Centrifugo uses to schedule re-authentication viarefresh_proxy. Without it, a client accepted during a 20-minute outage remains connected withinfo.degraded=trueindefinitely — even hours after identity-service is back — unless it disconnects voluntarily.Downstream proxies are required to fail-closed on
degraded=true, which limits blast radius. But the stale connection is still undesirable: the user never gets upgraded to a real identity without an explicit reconnect.Concrete fix — add an
expire_atparameter to the call site and function:The call site already has
const now = Date.now()(line 179); passMath.floor(now / 1000).FALLBACK_CONNECT_TTL_Scan be a new config knob (default 300 s) or a file-level constant. The Centrifugorefresh_proxycallback (future PR) would then prompt an inline token refresh, reverting to the real identity once it succeeds.[MINOR]
FALLBACK_DEGRADED_WINDOW_MS=0silently accepted — any single gRPC fault immediately opens the fallback pool gatesrc/config.ts:198
Line 198 guards
fallbackDegradedWindowMs < 0but silently accepts0. Insrc/fallback/outage.ts:121:The
>is strict.lastSuccessMsis initialised to process-start time (constructor line 80). So withwindowMs=0: process boots, some time elapses, the firstUNAVAILABLE/DEADLINE_EXCEEDEDarrives →notifyFailure()setshasAnyFailure=true→isInDegradedMode(now)evaluatesnow − startMs > 0→true(trivially, since any ms has passed) → degraded mode is immediately active → the fallback pool is drained on the very first gRPC blip. The constructor's own JSDoc (line 31–34) explicitly documents that cold-start protection breaks down atwindowMs=0, but config validation doesn't enforce it.Test helpers intentionally inject
windowMs: 0to avoid sleeping, but nothing prevents an operator from settingFALLBACK_DEGRADED_WINDOW_MS=0in a ConfigMap.Concrete fix — add a combined guard after per-field validation, before
return Object.freeze(config):Alternatively emit a loud
WARNand accept0only whenfallbackTicketsPathis empty. ThrowingConfigErroris the safer choice: it fails fast at boot rather than silently misbehaving during a production incident.[INFO]
"fail open"comment uses the phrase in the opposite sense from its standard security meaningsrc/fallback/pool.ts:90
Line 90:
// Other errors (permission denied, etc.) are also tolerated — fail open / to an empty Set rather than crash boot.In security literature, "fail open" means granting access when the guard is unavailable. The actual behaviour here is the opposite: an unreadable file produces an empty pool, which means every degraded-mode connect falls through to 503 — that is fail closed for clients attempting fallback-path connections. A future security auditor scanning the codebase for "fail open" patterns will find this comment and incorrectly conclude that the error path grants access.
Suggested replacement:
Verdict
NEEDS_WORK
hib-pr-reviewer • round 2 • 4 findings (3m/1i) • 2026-05-25T01:45:39.767Z → 2026-05-25T01:48:51.111Z • posted-as: pr-reviewer-bot
Show previous round
hib-pr-reviewer review — PR #2 (affinity-intelligence-rework/im2be-realtime-service)
Round 3 — head
3b18cfe90da5, basemain, triggersynchronizeTL;DR: CONDITIONAL_APPROVE — A=0 findings; kept both unique-to-B minor findings after code verification; all 7 R1+R2 findings confirmed fixed at HEAD.
Summary
Round 3 Reconciliation
Memora recalled no prior run history (first-time persist despite round 3; created new entry). Reviewer A returned 0 findings (all 7 R1+R2 issues confirmed fixed). Reviewer B returned 2 unique-to-B minor findings, both in
src/config.ts. Both were verified against HEAD viaRead.Finding 1 (config.ts:227) — VERIFIED. Lines 227-234 confirm the range check
fallbackConnectTtlS < 60 || > 3600has nofallbackTicketsPath !== ""guard. The sibling window=0 check at line 241 is correctly pool-gated. The asymmetry is real and will reject innocuous staging/CI configs that set a sub-60 s TTL without enabling the pool. Kept.Finding 2 (config.ts:241) — VERIFIED. Line 241 confirms
config.fallbackDegradedWindowMs === 0strict-equality check. For UNAVAILABLE faults (TCP RST, arrives in <1 ms), anywindowMssmaller thanidentityValidateDeadlineMs(2500 ms default) is effectively degenerate — the first blip plus elapsed wall-clock time opens the gate after that many ms with no compounding-failure requirement. The finding's reasoning holds. Kept.No findings from A to reconcile. Verdict: CONDITIONAL_APPROVE (2 minor, 0 blocking).
Blast Radius
The PR touches 17 files across config, fallback pool/outage, connect handler, metrics, server, and test suites — broad surface for a feature addition. The two remaining findings are confined to
src/config.tsvalidation logic, which is an exported boundary consumed bymain.tsat startup only. Runtime blast radius of the specific issues is low (startup-only path); operator blast radius in non-production environments is moderate.BLAST_SCORE: 5/10
Risk Indicators
consumeFallbackTicket,buildFallbackPoolResult,loadFallbackPoolCI status (head
3b18cfe90da5)No CI checks reported for this commit.
Related PRs
Findings (2)
[MINOR]
FALLBACK_CONNECT_TTL_Srange check fires unconditionally — rejects valid non-pool configssrc/config.ts:227
Lines 227-234 validate that
FALLBACK_CONNECT_TTL_Sis in[60..3600]regardless of whether the pool is enabled (fallbackTicketsPath === ""). The sibling invariant at line 241 correctly gates onconfig.fallbackTicketsPath !== ""before rejectingwindowMs=0. The TTL check lacks this guard, so an operator who setsFALLBACK_CONNECT_TTL_S=30in a staging env that deliberately does NOT mount the Secret (pool disabled) will get aConfigErrorat startup even though the TTL is irrelevant whenfallbackPool.size === 0.Suggested fix:
The default 300 s is in-range so no production env is affected today, but the asymmetry with the window guard is a latent footgun for CI harnesses that set a small TTL for fast-reconnect integration tests without mounting the Secret.
[MINOR]
FALLBACK_DEGRADED_WINDOW_MSguard rejects only=0; sub-deadline values are equally degenerate for UNAVAILABLE faultssrc/config.ts:241
Line 241 rejects
FALLBACK_DEGRADED_WINDOW_MS=0when the pool is enabled. However, for gRPC UNAVAILABLE faults (TCP RST/connection refused), the error arrives in <1 ms.OutageDetector.isInDegradedMode()evaluatesnow - lastSuccessMs > windowMs, so withwindowMs=100and steady Validate traffic, a single UNAVAILABLE fault opens the gate as soon as 100 ms has elapsed since the last success — identical in practical effect to=0under normal load.For DEADLINE_EXCEEDED faults the natural minimum gap is
identityValidateDeadlineMs(default 2500 ms), since the call must be in-flight for the full deadline. AwindowMs < identityValidateDeadlineMssetting means one deadline-exceeded call opens the gate immediately.Suggested additional guard inside the same
fallbackTicketsPath !== ""block:The test escape-hatch (inject
OutageDetectordirectly rather than going through config) remains valid for zero/tiny windows in tests.Verdict
CONDITIONAL_APPROVE
hib-pr-reviewer • round 3 • 2 findings (2m) • 2026-05-25T02:04:42.195Z → 2026-05-25T02:06:33.662Z • posted-as: pr-reviewer-bot
hib-pr-reviewer review — PR #2 (affinity-intelligence-rework/im2be-realtime-service)
Round 4 — head
066685e2e63d, basemain, triggersynchronizeTL;DR: NO_NEW_FINDINGS — No new findings this round.
Summary
[quorum-converged] A=0 = B=0. ## Round 4 verdict — CONDITIONAL_APPROVE
All 9 prior-round findings are fixed. Verification against HEAD confirms:
readStringcollapsedFALLBACK_TICKETS_PATH=""to prod defaultreadStringAllowEmptyatconfig.ts:207buildFallbackPoolResultsilently discarded_ticket; all sessions shared one identitysha256(ticket).slice(0,12)atconnect.ts:418notifyFailure(_now)silently discarded timestampnotifyFailure()is now zero-arg atoutage.ts:102status:"missing""unreadable"branch atpool.ts:117-121buildFallbackPoolResultemitted noexpire_atexpire_at: nowS + ttlSatconnect.ts:428FALLBACK_DEGRADED_WINDOW_MS=0silently acceptedconfig.ts:237-243"fail open"comment used in opposite senseFALLBACK_CONNECT_TTL_Srange check fired unconditionallyconfig.ts:271-278fallbackDegradedWindowMs < identityValidateDeadlineMsguard atconfig.ts:255-265One new minor finding:
getHealthStatus()allocates anew Set<ConnectOutcome>([...])on every invocation. The 3-element set is a compile-time constant; hoisting it to module scope eliminates the per-call allocation.CI status (head
066685e2e63d)No CI checks reported for this commit.
Findings
No new findings this round.
Quorum converged on empty findings (A + B both returned 0).
Verdict
NO_NEW_FINDINGS
hib-pr-reviewer • round 4 • 0 findings • 2026-05-25T02:10:34.604Z → 2026-05-25T02:16:50.323Z • posted-as: pr-reviewer-bot • [bookkeeping fallback]