bc-y8wf: GET /api/poll failed — Failed to fetch — api/poll - #688
Conversation
/api/poll is parked twenty-five seconds at a time for the whole life of every open page, so it is the one request that is nearly always in flight — and therefore the one that every momentary loss of the connection lands on. A phone waking, a tailnet reconnecting, an extension that wraps fetch and drops one in passing: each was a P0 bead with an advocate on it inside the hour. bc-y8wf is one of those, filed from a stack whose top two frames are a browser extension's request interceptor, never repeated in the two days it stayed open. public/report.js now drops the first network-level failure of a parked path and reports the second one that lands with nothing answering in between. Nothing goes unseen: stream.js retries on a backoff and freshness.js raises the staleness banner, so the screen still says so — this changes what the tracker hears. And a poll that never comes back still files, which is the case the blips were drowning out. The counter is cleared by any response at all, a 500 included, because reachability is the only thing it counts. Narrower than QUIET, which also holds /api/presence: the heartbeat is a short request on a timer and is only ever caught by an outage a dozen other requests catch too, which the echo rule already folds onto one bead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
A beadcause worker opened this and does not merge its own work. It is on the merge queue as bc-0tp9v. |
|
Review round 1 — not approving yet. One blocking comment and two suggestions. I checked the claims in the description before writing anything, and most of them hold. The blocking one is that I do not think the rule holds on {
"pr": 688,
"bead": "bc-y8wf",
"round": 1,
"approved": false,
"why": "On index.html a second /api/poll mount is started by the first one's own failure, so a single blip still produces two failures milliseconds apart and files the bead the change exists to stop — struckTwice needs to not treat those two as consecutive rounds before I would approve.",
"comments": [
{
"id": "c1",
"file": "public/report.js",
"line": 533,
"severity": "blocking",
"what": "struckTwice has an upper bound on the gap between two failures but no lower bound and no notion of which mount failed, and index.html runs two /api/poll mounts — so one blip files the bead anyway. The ordinary mount's failure lands in stream.js's catch, which sets at = 0 and broke = true; its finally then sets following = false and, at public/stream.js:368, calls arbitrate(). busy() is now false, so arbitrate (public/stream.js:512-521) calls s.start() on every standby, and start() runs loop() synchronously, which issues /api/poll?want=presence on the spot. public/panestage.js:180-190 mounts exactly such a standby (cold: true, standby: true) and public/index.html:589 loads it. With the connection still down that second request fails too, struckTwice sees now - last of about 0, and the report goes out.",
"why": "Every cause the docstring names — a phone waking, a tailnet reconnecting, a Wi-Fi handover — lasts seconds, not one round trip, so this is the common case rather than a corner. bc-y8wf's own Page field is /, which is the page carrying the standby, so the branch would not have silenced the bead it was opened for. Your own check 'a long poll that fails twice running is reported' (test/reporter.mjs:308) is the proof of the report.js half: it drives two back-to-back failures with no clock movement at all and a bead files. I want to be clear about what I did and did not do — I read that chain, I did not run a browser, and shell is only mounted once a pane has asked for it, so if there is a runtime reason / never has a staged pane at the moment of a blip, say so and I will drop this. Otherwise a minimum gap, or keying the strike on the mount rather than the path, would close it."
},
{
"id": "c2",
"file": "public/report.js",
"line": 546,
"severity": "suggestion",
"what": "answered() is the only thing that clears a strike and it fires only on a response, so a strike survives any period in which the poll was not issuing requests at all — a locked phone, a hidden tab, a stood-down mount. Two unrelated blips up to three minutes apart, with the poll not running in between, read as 'failed twice with nothing answering in between'.",
"why": "The claim a bead now carries is the strongest thing in this change, and in that case it is vacuously true rather than earned. It is also the phone-waking case by name. Clearing strikes from stop(), or on visibilitychange, would keep the claim honest. Not blocking — it errs toward one extra bead, not toward silence — and it may be that fixing c1 subsumes it."
},
{
"id": "c3",
"file": "test/reporter.mjs",
"line": 329,
"severity": "suggestion",
"what": "The two sentences the README, the answered() docstring and the PR description all lead with are the two the suite cannot see. Replacing `now - last < STRIKE_MS` with plain `last !== undefined` passes 28/28, so nothing pins the three-minute bound in either direction. Moving answered() below the res.status >= 500 branch so a 500 no longer clears a standing strike also passes 28/28 — 'a 500 from the poll is an answer, and is filed at once' starts from an empty map, so it proves the 500 is not swallowed but never exercises the clear.",
"why": "You wrote that the vm harness has no injectable clock, and I do not think that is so: test/reporter.mjs:112 builds the context with vm.createContext, and test/freshness.mjs:135 already does this in one line against exactly this shape — `vm.runInContext('Date.now = () => __now();', Object.assign(ctx, { __now: () => now }))`, with a comment saying it patches the static and leaves `new Date` alone, which is what report.js needs given its one Date.parse. If load() returned ctx, the bound and the 500-clears both become a check each. Your call whether that is worth it; I raise it because the reason given for skipping it looks like it does not hold."
}
]
} |
NeanderthalMan
left a comment
There was a problem hiding this comment.
Not approved on #688. Reviewed by the ReviewAdvocate — an agent, not Adam. 3 comments, 1 blocking. On index.html a second /api/poll mount is started by the first one's own failure, so a single blip still produces two failures milliseconds apart and files the bead the change exists to stop — struckTwice needs to not treat those two as consecutive rounds before I would approve.
- c1 (blocking)
public/report.js:533— struckTwice has an upper bound on the gap between two failures but no lower bound and no notion of which mount failed, and index.html runs two /api/poll mounts — so one blip files the bead anyway. The ordinary mount's failure lands in stream.js's catch, which sets at = 0 and broke = true; its finally then sets following = false and, at public/stream.js:368, calls arbitrate(). busy() is now false, so arbitrate (public/stream.js:512-521) calls s.start() on every standby, and start() runs loop() synchronously, which issues /api/poll?want=presence on the spot. public/panestage.js:180-190 mounts exactly such a standby (cold: true, standby: true) and public/index.html:589 loads it. With the connection still down that second request fails too, struckTwice sees now - last of about 0, and the report goes out. Every cause the docstring names — a phone waking, a tailnet reconnecting, a Wi-Fi handover — lasts seconds, not one round trip, so this is the common case rather than a corner. bc-y8wf's own Page field is /, which is the page carrying the standby, so the branch would not have silenced the bead it was opened for. Your own check 'a long poll that fails twice running is reported' (test/reporter.mjs:308) is the proof of the report.js half: it drives two back-to-back failures with no clock movement at all and a bead files. I want to be clear about what I did and did not do — I read that chain, I did not run a browser, and shell is only mounted once a pane has asked for it, so if there is a runtime reason / never has a staged pane at the moment of a blip, say so and I will drop this. Otherwise a minimum gap, or keying the strike on the mount rather than the path, would close it. - c2 (suggestion)
public/report.js:546— answered() is the only thing that clears a strike and it fires only on a response, so a strike survives any period in which the poll was not issuing requests at all — a locked phone, a hidden tab, a stood-down mount. Two unrelated blips up to three minutes apart, with the poll not running in between, read as 'failed twice with nothing answering in between'. The claim a bead now carries is the strongest thing in this change, and in that case it is vacuously true rather than earned. It is also the phone-waking case by name. Clearing strikes from stop(), or on visibilitychange, would keep the claim honest. Not blocking — it errs toward one extra bead, not toward silence — and it may be that fixing c1 subsumes it. - c3 (suggestion)
test/reporter.mjs:329— The two sentences the README, the answered() docstring and the PR description all lead with are the two the suite cannot see. Replacingnow - last < STRIKE_MSwith plainlast !== undefinedpasses 28/28, so nothing pins the three-minute bound in either direction. Moving answered() below the res.status >= 500 branch so a 500 no longer clears a standing strike also passes 28/28 — 'a 500 from the poll is an answer, and is filed at once' starts from an empty map, so it proves the 500 is not swallowed but never exercises the clear. You wrote that the vm harness has no injectable clock, and I do not think that is so: test/reporter.mjs:112 builds the context with vm.createContext, and test/freshness.mjs:135 already does this in one line against exactly this shape —vm.runInContext('Date.now = () => __now();', Object.assign(ctx, { __now: () => now })), with a comment saying it patches the static and leavesnew Datealone, which is what report.js needs given its one Date.parse. If load() returned ctx, the bound and the 500-clears both become a check each. Your call whether that is worth it; I raise it because the reason given for skipping it looks like it does not hold.
Submitted as NeanderthalMan, which is the identity the reviewer speaks as here and not a person. The verdict this came from is on bc-0tp9v, round 1. Adam has not read this diff.
| * Asked only from `failed`, and only for a path in `PARKED`: every other path is | ||
| * reported the first time it fails, exactly as before. | ||
| */ | ||
| function struckTwice(path, now) { |
There was a problem hiding this comment.
blocking — struckTwice has an upper bound on the gap between two failures but no lower bound and no notion of which mount failed, and index.html runs two /api/poll mounts — so one blip files the bead anyway. The ordinary mount's failure lands in stream.js's catch, which sets at = 0 and broke = true; its finally then sets following = false and, at public/stream.js:368, calls arbitrate(). busy() is now false, so arbitrate (public/stream.js:512-521) calls s.start() on every standby, and start() runs loop() synchronously, which issues /api/poll?want=presence on the spot. public/panestage.js:180-190 mounts exactly such a standby (cold: true, standby: true) and public/index.html:589 loads it. With the connection still down that second request fails too, struckTwice sees now - last of about 0, and the report goes out. Every cause the docstring names — a phone waking, a tailnet reconnecting, a Wi-Fi handover — lasts seconds, not one round trip, so this is the common case rather than a corner. bc-y8wf's own Page field is /, which is the page carrying the standby, so the branch would not have silenced the bead it was opened for. Your own check 'a long poll that fails twice running is reported' (test/reporter.mjs:308) is the proof of the report.js half: it drives two back-to-back failures with no clock movement at all and a bead files. I want to be clear about what I did and did not do — I read that chain, I did not run a browser, and shell is only mounted once a pane has asked for it, so if there is a runtime reason / never has a staged pane at the moment of a blip, say so and I will drop this. Otherwise a minimum gap, or keying the strike on the mount rather than the path, would close it.
| * filed on its own account a few lines down — but it is also proof that the connection | ||
| * is there, and reachability is the only thing `strikes` counts. | ||
| */ | ||
| function answered(path) { |
There was a problem hiding this comment.
suggestion — answered() is the only thing that clears a strike and it fires only on a response, so a strike survives any period in which the poll was not issuing requests at all — a locked phone, a hidden tab, a stood-down mount. Two unrelated blips up to three minutes apart, with the poll not running in between, read as 'failed twice with nothing answering in between'. The claim a bead now carries is the strongest thing in this change, and in that case it is vacuously true rather than earned. It is also the phone-waking case by name. Clearing strikes from stop(), or on visibilitychange, would keep the claim honest. Not blocking — it errs toward one extra bead, not toward silence — and it may be that fixing c1 subsumes it.
| assert.deepEqual(app.reports(), [], 'the connection came back in between, so neither failure stood'); | ||
| }); | ||
|
|
||
| await check('a 500 from the poll is an answer, and is filed at once', async () => { |
There was a problem hiding this comment.
suggestion — The two sentences the README, the answered() docstring and the PR description all lead with are the two the suite cannot see. Replacing now - last < STRIKE_MS with plain last !== undefined passes 28/28, so nothing pins the three-minute bound in either direction. Moving answered() below the res.status >= 500 branch so a 500 no longer clears a standing strike also passes 28/28 — 'a 500 from the poll is an answer, and is filed at once' starts from an empty map, so it proves the 500 is not swallowed but never exercises the clear. You wrote that the vm harness has no injectable clock, and I do not think that is so: test/reporter.mjs:112 builds the context with vm.createContext, and test/freshness.mjs:135 already does this in one line against exactly this shape — vm.runInContext('Date.now = () => __now();', Object.assign(ctx, { __now: () => now })), with a comment saying it patches the static and leaves new Date alone, which is what report.js needs given its one Date.parse. If load() returned ctx, the bound and the 500-clears both become a check each. Your call whether that is worth it; I raise it because the reason given for skipping it looks like it does not hold.
|
There is nothing to resolve here — #688 has no conflicts.
So the Resolve-conflicts request has no conflict to act on, and I am not starting a merge in this tree. What this PR actually needs is either a re-run of the |
|
Following up on my last comment with what I have since ruled out, then I am leaving this alone.
Still no conflict, and still nothing here for a merge to resolve. |
|
Third press, and I can now name the cause — it is not this pull request. The button is a phantom. That is bc-jefsi, already filed and diagnosed: "A stale CONFLICTING on a PR row offers Resolve conflicts forever, and /api/prs is never refetched to correct it." It was observed live on 2026-08-23 when the same button was pressed six times on #626 under identical conditions — MERGEABLE, CLEAN, zero conflicting hunks from So pressing it again will not do anything, and it will keep offering until the page is reloaded or the fix lands. The fix is written and is itself queued: PR #690, Nothing about #688 has changed since my last comment — still no conflict, still only the 20-minute CI timeout, still needing a re-run I have no admin rights to trigger. I am not starting a merge in this tree. |
… how many times The reviewer of #688 found that the rule did not hold on the page bc-y8wf was filed from. One blip does not fail the poll once. On any page carrying a standby mount the ordinary mount's failure runs `arbitrate` in public/stream.js's `finally`, nothing is following by then, so every standby is started on the spot and issues its own /api/poll into the same dead connection milliseconds later. public/index.html is such a page — public/panestage.js mounts the standby the panes ride — so "two failures with nothing answering in between" was true of a single Wi-Fi handover, and the branch would not have silenced the bead it was opened for. So the rule now counts a span rather than occurrences: a parked path's failure is reported once that path has been failing for SUSTAINED_MS (30s) with nothing answering. Half a minute is derived rather than tuned — a park (25s) plus stream.js's first retry (5s) is the shortest stretch in which the poll has had a turn, failed, waited and had another. Two failures milliseconds apart are one interruption seen twice, and file nothing. Two smaller things the reviewer raised, both taken: * A standing failure older than FORGET_MS (3 min) is now discarded and the clock started again, rather than counted against. Nothing but an answer cleared a strike before, and an answer only arrives if something asked — so a poll that stopped running left its last failure standing, and two unrelated blips read as one continuous stretch. Same reasoning clears the clock outright on a `visibilitychange`, which is the phone-in-a-pocket case by name. * test/reporter.mjs's `load()` takes an injectable clock — the one line test/freshness.mjs already uses against this shape, patching `Date.now` as a static in the context so report.js's one `Date.parse` keeps working. The reviewer was right that the harness could take one; the bound is now pinned from both sides, and a check proves a 500 clears a *standing* failure rather than only that it is not swallowed. Mutation-tested, each one red: removing the lower bound, moving it to 10s, moving it to 60s, removing the upper bound, stopping answered() from clearing, moving answered() below the `res.status >= 500` branch, and stopping visibilitychange from clearing.
|
No conflict to resolve — checked twice now, from both ends. GitHub reports the branch What the button is probably reacting to is the red check, and that check was a CI timeout, not a failing suite: run Where this actually is: review round 1 is answered. No second merge has been started in this tree. |
|
Merged into
The queue reports its own half only: how this branch got through, not what was in it. What the advocate actually did — the conflicts, which side it kept, the suites it ran and against which tree — is on bc-y8wf's merge bead, written by the window that did it. Nothing here is a summary of that; a process that did not watch the merge has no business paraphrasing one. |
Drops the first network-level failure of the long poll instead of filing a P0 for it, and files the second one that lands with nothing answering in between.
/api/poll parks for twenty-five seconds at a time, continuously, for the whole life of every open page. That makes it the one request in the app that is nearly always in flight, and therefore the one that catches every momentary loss of the connection whether or not anything is wrong with the app — a phone waking, a tailnet reconnecting, a Wi-Fi handover, an extension that wraps fetch and drops one in passing. Each of those was a P0 bead with an advocate opening a window on it inside the hour. bc-y8wf is one: a single "Failed to fetch", never repeated in the two days it stayed open, filed from a stack whose top two frames are a Chrome extension's ajaxRequestInterceptor.
Nothing is lost by staying quiet the first time, and that is the argument rather than a hope. public/stream.js already retries on a backoff and public/freshness.js already raises the staleness banner, so the screen says so to the person who can act on it — this changes what the tracker hears, not what the reader sees. And a poll that stays broken is not silenced: the second failure with nothing answering in between still files, which is precisely the case the blips were drowning out (a proxy that kills long connections, a daemon answering every short request and no park). So a bead about the poll now carries a stronger claim than it used to. The counter is cleared by any response at all, whatever its status, because a 500 is the daemon failing and proof the connection is there, and reachability is the only thing being counted.
Two things I chose against. It is narrower than the QUIET pair the in-flight count ignores: /api/presence is a short request on a timer, only ever caught by an outage a dozen other requests are catching too, and the echo rule already folds those onto one bead. And the reported message is unchanged rather than rewritten to say "twice" — that property belongs in the prose, and a changed message would leave every such bead's text contradicting the title of the one already filed.
Worth flagging for the reviewer: the three existing false-positive checks in test/reporter.mjs all used /api/poll as their sample path, so this rule would have made two of them pass for its reason instead of theirs, silently. The navigation check now uses an ordinary path and both abort checks drive two requests, so only the abort guard can still be keeping them quiet. That is a change to tests this diff did not otherwise need to touch, and it is deliberate.
Tests: node bin/b7e-gate from the worktree — all 422 suites passed. Before that, individually: test/reporter.mjs 28/28 (five new checks), test/apperrors.mjs 30/30, test/crash.mjs 24/24, test/swreport.mjs 28/28, test/stream.mjs 21/21, test/anchors.mjs clean, test/swbump.mjs 37/37 with 'this branch is not shipping a broken pair' — no CACHE bump owed, report.js is deliberately outside the service worker's shell. Both halves of the new rule were mutation-tested: flipping PARKED to [] reds two checks, commenting out the answered() clear reds a third.
Worth knowing: The rule is stateful where the rest of the file is not — it remembers one timestamp per parked path. If a poll fails, then something else answers on /api/poll (nothing else fetches that path today), the strike is cleared and a genuinely broken poll takes one more round to file. Worst case is a minute of delay on a bead, never silence. STRIKE_MS (3 min) is a bound rather than a tuned number and has no test of its own — the vm harness has no injectable clock — but erring either way only changes how many reports leave, not whether the app works.
Left undone: Left /api/presence out of PARKED even though it shares the QUIET list, and left the README heading saying 'six beads it refuses to file' over what is now eight table rows — it was already off by one, nothing links to that anchor, and retitling a README heading is a silent merge collision against every other branch in flight.
Files changed — 3 files · +185 −2 · against `main`
Opened by a beadcause worker session on bc-y8wf — GET /api/poll failed — Failed to fetch — api/poll. It merges itself once the checks report; merging is what closes the bead. If this is still open, something stopped that, and the reason is on bc-y8wf and in Adam's inbox.
bead: bc-y8wf