Follow-up from the fourth review of #185, raised as non-blocking there and
filed rather than dropped.
Problem
In tests/e2e/network.js, the guard that reports unrecognised POST bodies uses batch.every(...). On an empty array that predicate is vacuously true, so a
POST with body [] is fulfilled with 200 [] instead of being reported as an
unstubbed request. Verified by execution during review - the run stayed green.
This is a hole in the one mechanism whose entire job is to ensure unrecognised
outbound traffic fails the suite rather than passing silently. It is unreachable
in practice today, which is why it did not block the PR, but "unreachable today"
is exactly the qualifier that stops being true later.
Also in scope: a comment that overstates the guard's rationale
tests/e2e/network.js:137-140 claims postData() returns null for
undecodable bodies. That is not quite right. playwright-core@1.56.0 (lib/client/network.js:89) does buffer.toString("utf-8") || null, so a binary body is lossily decoded
rather than yielding null; only an empty decode produces null. The guard is
correct and load-bearing either way - both paths end up reported - but the
stated reason is wrong, and the PR author independently disclosed that they
could not reproduce the sendBeacon/Blob case the comment implies.
Correct the comment to describe the two real paths: an empty or absent body
decodes to null and hits the type guard; a binary body decodes lossily to
invalid JSON and hits the catch.
Definition of done
A POST with body [] is reported as an unstubbed request and fails the suite.
Demonstrated by execution, per the standing rule on this repo: show it green
before and red after. A fix asserted but not demonstrated does not count.
While in there, confirm the sibling cases still report: bodyless POST, scalar
JSON body, [null] batch. These were verified working during review and must
not regress.
The comment at tests/e2e/network.js:137-140 states the actual mechanism.
Note
Keep this small. Do not redesign the guard, and do not touch TRAILING_WATCH_MS
or the canary - both were deliberately ruled out of scope during review and are
documented as they stand.
Follow-up from the fourth review of
https://git.eeqj.de/sneak/AutistMask/pulls/185, raised as non-blocking there and
filed rather than dropped.
## Problem
In `tests/e2e/network.js`, the guard that reports unrecognised POST bodies uses
`batch.every(...)`. On an empty array that predicate is **vacuously true**, so a
POST with body `[]` is fulfilled with `200 []` instead of being reported as an
unstubbed request. Verified by execution during review - the run stayed green.
This is a hole in the one mechanism whose entire job is to ensure unrecognised
outbound traffic fails the suite rather than passing silently. It is unreachable
in practice today, which is why it did not block the PR, but "unreachable today"
is exactly the qualifier that stops being true later.
## Also in scope: a comment that overstates the guard's rationale
`tests/e2e/network.js:137-140` claims `postData()` returns `null` for
undecodable bodies. That is not quite right.
`playwright-core@1.56.0` (`lib/client/network.js:89`) does
`buffer.toString("utf-8") || null`, so a binary body is **lossily decoded**
rather than yielding `null`; only an empty decode produces `null`. The guard is
correct and load-bearing either way - both paths end up reported - but the
stated reason is wrong, and the PR author independently disclosed that they
could not reproduce the `sendBeacon`/Blob case the comment implies.
Correct the comment to describe the two real paths: an empty or absent body
decodes to `null` and hits the type guard; a binary body decodes lossily to
invalid JSON and hits the `catch`.
## Definition of done
- A POST with body `[]` is reported as an unstubbed request and fails the suite.
- Demonstrated by execution, per the standing rule on this repo: show it green
before and red after. A fix asserted but not demonstrated does not count.
- While in there, confirm the sibling cases still report: bodyless POST, scalar
JSON body, `[null]` batch. These were verified working during review and must
not regress.
- The comment at `tests/e2e/network.js:137-140` states the actual mechanism.
## Note
Keep this small. Do not redesign the guard, and do not touch `TRAILING_WATCH_MS`
or the canary - both were deliberately ruled out of scope during review and are
documented as they stand.
clawbot
added this to the 1.0.0 milestone 2026-08-09 18:45:47 +02:00
Done in #267 (branch test/issue-187-empty-batch-guard, base next).
batch.length === 0 added to the guard; comment corrected to the two real paths. Nothing else touched — TRAILING_WATCH_MS and the canary left as they stand, ALLOWED_ERRORS still [].
Verified by execution with a throwaway four-POST probe, added for the two runs and removed before the commit:
before: # probe [] -> fulfilled, ok 28 - PROBE empty-array batch [] — green, the hole
after: # probe [] -> rejected: Failed to fetch, not ok 28 with network: unstubbed request: POST https://e2e-probe.invalid/rpc — red
Siblings reported and failed in both runs: bodyless POST, scalar JSON body, [null] batch. Tests 1-27 stayed green throughout, so the widened ConfirmTx RPC fixtures are not misreported. Full captures are in the PR body.
make check green (576 tests, test-verify-build 18 cases, prettier clean); make test-e2e green at 27/27 with the probe removed.
Done in [#267](https://git.eeqj.de/sneak/AutistMask/pulls/267) (branch `test/issue-187-empty-batch-guard`, base `next`).
`batch.length === 0` added to the guard; comment corrected to the two real paths. Nothing else touched — `TRAILING_WATCH_MS` and the canary left as they stand, `ALLOWED_ERRORS` still `[]`.
Verified by execution with a throwaway four-POST probe, added for the two runs and removed before the commit:
- before: `# probe [] -> fulfilled`, `ok 28 - PROBE empty-array batch []` — green, the hole
- after: `# probe [] -> rejected: Failed to fetch`, `not ok 28` with `network: unstubbed request: POST https://e2e-probe.invalid/rpc` — red
Siblings reported and failed in both runs: bodyless POST, scalar JSON body, `[null]` batch. Tests 1-27 stayed green throughout, so the widened ConfirmTx RPC fixtures are not misreported. Full captures are in the PR body.
`make check` green (576 tests, `test-verify-build` 18 cases, prettier clean); `make test-e2e` green at 27/27 with the probe removed.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Follow-up from the fourth review of
#185, raised as non-blocking there and
filed rather than dropped.
Problem
In
tests/e2e/network.js, the guard that reports unrecognised POST bodies usesbatch.every(...). On an empty array that predicate is vacuously true, so aPOST with body
[]is fulfilled with200 []instead of being reported as anunstubbed request. Verified by execution during review - the run stayed green.
This is a hole in the one mechanism whose entire job is to ensure unrecognised
outbound traffic fails the suite rather than passing silently. It is unreachable
in practice today, which is why it did not block the PR, but "unreachable today"
is exactly the qualifier that stops being true later.
Also in scope: a comment that overstates the guard's rationale
tests/e2e/network.js:137-140claimspostData()returnsnullforundecodable bodies. That is not quite right.
playwright-core@1.56.0(lib/client/network.js:89) doesbuffer.toString("utf-8") || null, so a binary body is lossily decodedrather than yielding
null; only an empty decode producesnull. The guard iscorrect and load-bearing either way - both paths end up reported - but the
stated reason is wrong, and the PR author independently disclosed that they
could not reproduce the
sendBeacon/Blob case the comment implies.Correct the comment to describe the two real paths: an empty or absent body
decodes to
nulland hits the type guard; a binary body decodes lossily toinvalid JSON and hits the
catch.Definition of done
[]is reported as an unstubbed request and fails the suite.before and red after. A fix asserted but not demonstrated does not count.
JSON body,
[null]batch. These were verified working during review and mustnot regress.
tests/e2e/network.js:137-140states the actual mechanism.Note
Keep this small. Do not redesign the guard, and do not touch
TRAILING_WATCH_MSor the canary - both were deliberately ruled out of scope during review and are
documented as they stand.
Done in #267 (branch
test/issue-187-empty-batch-guard, basenext).batch.length === 0added to the guard; comment corrected to the two real paths. Nothing else touched —TRAILING_WATCH_MSand the canary left as they stand,ALLOWED_ERRORSstill[].Verified by execution with a throwaway four-POST probe, added for the two runs and removed before the commit:
# probe [] -> fulfilled,ok 28 - PROBE empty-array batch []— green, the hole# probe [] -> rejected: Failed to fetch,not ok 28withnetwork: unstubbed request: POST https://e2e-probe.invalid/rpc— redSiblings reported and failed in both runs: bodyless POST, scalar JSON body,
[null]batch. Tests 1-27 stayed green throughout, so the widened ConfirmTx RPC fixtures are not misreported. Full captures are in the PR body.make checkgreen (576 tests,test-verify-build18 cases, prettier clean);make test-e2egreen at 27/27 with the probe removed.clawbot referenced this issue2026-08-12 13:26:58 +02:00