`worker.rejected` carries `count: 1` on an isolated refusal, which the catalogs say it must never do
I maintainer di solito rispondono entro 1 giorno
Valutazione
Questa issue non è ancora stata valutata.
Descrizione
What is wrong
worker.rejected carries count: 1 on an isolated refusal, which the catalogs say it must never do
Where this came from
Confirmed in the bug bounty sweep tracked on #648 (finding 3). The reproduction was reproduced independently by the orchestrating session, the proposed fix confirmed green against the surrounding suite, and the finding re-confirmed against current main.
Spec
Area: src/gateway/service.ts (uplink rejection coalescing)
Severity: low, audit trail only. No state damage and no protocol change
needed — but count's presence is the documented signal that a flood happened,
so count: 1 tells an operator "one refusal was absorbed by an earlier window"
when there was no flood at all. Any consumer treating count !== undefined as
"there was a flood" — the documented reading — is wrong.
Kind: defect — the emitted payload is one the contract's own documentation
says must be omitted.
Problem
GatewayService#recordRejection coalesces refusals per (reason, claimed id)
into one-second windows. When a window ages out, the next matching refusal
reports that window's total as count.
A window that absorbed exactly one refusal has total 1, and that 1 is passed
straight to registry.rejected, which omits count only when it is undefined.
So two lone refusals an apart — the "overwhelmingly common case of an
isolated refusal" that the code's own comment names — make the second event carry
count: 1.
A misconfigured or revoked worker redialling on its 1 s → 30 s backoff therefore
carries count: 1 on nearly every refusal it ever emits. The backoff puts each
dial outside the previous window by design, so this is the normal path for the
exact scenario the event exists to describe (ADR 0005 §22: "a worker whose
credentials were mistyped or revoked").
The comment sitting four lines above the bug states the opposite of the code:
// src/gateway/service.ts:188-192
// ... so a lone refusal keeps the exact
// pre-P-2 payload shape (no `count` field at all) rather than gaining a
// redundant `count: 1`.
A refusal whose predecessor window held one lone refusal is a lone refusal, and
it does not keep that shape.
Root cause
src/gateway/service.ts:201 — this.#registry.rejected(reason, workerId, label, previousTotal), with previousTotal read at :193 — hands the previous
window's raw count onward without the "above one" rule the comment above it
states. WorkerRegistry#rejected (src/gateway/worker-registry.ts:339) then
omits only count === undefined, so a 1 reaches the bus.
Three independent sources bind the value, all agreeing:
docs/EVENTS.md:108 `count`, present only above one, is how many identical
refusals a coalescing window absorbed
docs/internal/EVENTS.md:165 `count`, present only above one, is how many identical
(reason, claimed id) refusals the *previous* coalescing
window absorbed — GatewayService computes it before
calling into the registry
src/gateway/worker-registry.ts:324 Omitted (not zero or one) outside a flood, so the
payload is unchanged from before this fix in the
overwhelmingly common case of an isolated refusal.
Why the existing P-2 test never caught it: its third assertion uses a different
claimed id (wrk_other), whose previous total is genuinely undefined. The
same-id-after-aging-out path is unexercised.
Evidence
Test file: src/gateway/zz-orchestrator-rejection-count.test.ts
(written independently by the orchestrator from the reported root
cause — the finder's own scratch file was lost when its session
ended, so this is a fresh reproduction, not a re-run)
Test title: worker.rejected payload across aged-out windows > carries no count
when the previous window absorbed a single refusal
Command: pnpm exec vitest run --project unit \
src/gateway/zz-orchestrator-rejection-count.test.ts
AssertionError: expected { reason: 'unauthenticated', …(2) } to not have
property "count"
- Expected: undefined
+ Received: 1
❯ src/gateway/zz-orchestrator-rejection-count.test.ts:49:40
47| await expect(dial()).rejects.toMatchObject({ code: "rejected" });
48| expect(rejected()).toHaveLength(2);
49| expect(rejected()[1]?.payload).not.toHaveProperty("count");
Honesty checks:
- The reproduction asserts the first refusal carries no
counttoo, so the
harness itself cannot be silently mis-wired — that assertion is load-bearing. - Broke it a second way (
previousTotal ?? 0, so a count is never omitted): still
fails at:49, so it keys on the omission rule and not on the literal1. - Broke it a third way (
count: 1on every emission): fails earlier at the first
assertion. - With the fix: this test passes and the entire
src/gatewaymodule is green —
23 files, 695 tests, including the P-2 test'scount: 2assertion at
service.test.ts:1818, which pins that a genuine two-dial window still reports
its tally. Source reverted to HEAD afterwards; only the scratch test remains.
Proposed fix
src/gateway/service.ts:201 — apply the documented bound at the point the count
is handed over, where the coalescing fact lives:
this.#registry.rejected(reason, workerId, label, previousTotal === 1 ? undefined : previousTotal);
previousTotal can only be undefined, 1, or more, so === 1 is exhaustive and
0 is unreachable. No protocol bump: nothing on the wire changes shape, only the
presence of a field already optional and already documented as absent here. No
comment or doc change is needed either — the comment at service.ts:188-192
already describes this behaviour; the fix is what makes it true.
Alternatives rejected
- Drop a
countof one insideWorkerRegistry#rejected— both catalogs state
GatewayServicecomputes the count before calling into the registry, so the
registry would be deciding a coalescing fact that is not its own (architecture
rule: one place enforces a rule). - Reset
#recentRejectionsentries once a window ages out — an aged-out window's
total is exactly the information the next event is supposed to carry; this
would lose the flood tally the mechanism exists for. - Widen the docs to allow
count: 1— the field's whole purpose is that its
presence marks a flood, and two EVENTS.md cuts, theEventMap, and the
registry's own doc comment all currently promise otherwise.
Risk
count: 2must still be reported for a genuine two-dial window —
service.test.ts:1818pins it and stays green.- A reviewer should confirm no consumer keys on
count === 1. None exists in this
repo: the console touches this event's payload only to decide worker
attribution (event-list.ts, deliberately ignoring the claimedworkerId), and
never readscount; the gateway relays envelopes verbatim. - Worth a look in the same PR:
MAX_TRACKED_REJECTION_WINDOWSeviction drops the
oldest window, so an evicted key's next refusal behaves as never-seen and emits
nocount. That is correct in itself, and unchanged by this fix.
Written by an agent.
Written by an agent.
- Lingua principale
- TypeScript
- Stelle
- 20
- Fork
- 2
- Merge medio
- 7h 26m
- PR unite (30g)
- 253
Preparare l'ambiente
Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di callstackincubator/simlock
-
Console lane deletes a failed daemon's home, so a start failure names no causeForse già presa @V3RON l’ha presa oggi. Apertabug:triage
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
callstackincubator/simlock#673 · 1 assegnatario ·
I maintainer di solito rispondono entro 1 giorno
-
bug:needs-info
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
callstackincubator/simlock#657 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
callstackincubator/simlock#639 ·
I maintainer di solito rispondono entro 1 giorno
-
bug:new
Difficoltà 4/5 3-5 giorni Idoneità per principianti 45/100
callstackincubator/simlock#684 ·
I maintainer di solito rispondono entro 1 giorno
-
A preview system image installed makes the newest runtime its codename, not the highest API levelForse già presa @V3RON l’ha presa 1 giorno fa. Apertabug:ready
callstackincubator/simlock#678 · 1 assegnatario ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di callstackincubator/simlock
Issue simili
-
freshness freshness:observed-only
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
thoroc/context-radar#280 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 66/100
-
documentation story
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 74/100
I maintainer di solito rispondono entro 1 giorno
-
Billing: computeBundleEconomicsFrom treats a NULL component cost as 0, inflating bundle marginApertabug category:billing priority:p3 track:fast
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
LanternOps/breeze#9223 ·
I maintainer di solito rispondono entro 1 giorno