Skip to content

fix(web): report the last probe outcome when detached readiness times out - #2668

Merged
davidfarah2003 merged 5 commits into
Cotal-AI:mainfrom
drakeo338:claude/2653-fix
Oct 10, 2026
Merged

davidfarah2003 merged 5 commits into
Cotal-AI:mainfrom
drakeo338:claude/2653-fix

Conversation

@drakeo338

@drakeo338 drakeo338 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2653

When the detached web dashboard did not become HTTP-ready before the deadline, the timeout error said nothing about why. It now includes the last probe URL and its outcome (status or error), so a failed start can be diagnosed from the message.

Retry cadence and deadline are unchanged. A changeset is included.

Tested: pnpm test in implementations/web (the detach smoke, with the message assertion tightened) passes with the fix. With src/web.ts reverted to upstream/main it fails on that assertion.

The maintainers completed this change on the contributor's branch: it is rebased on main and the review findings are addressed. Two independent reviewers from different model families approved it at the exact head.

Approved-at: 0130b4a

@davidfarah2003

Copy link
Copy Markdown
Contributor

Thanks, the diagnostic is the right idea and the functional claim reproduces at this head. Two things before it can land:

  1. On a 200 /api/meta response that fails the space or pid match, the last-probe text embeds the full JSON.stringify of both fields with no length cap. Whatever answers the bound port can therefore make the timeout error arbitrarily large, and launchDetachedWeb rethrows that whole message to the terminal and into the failure path that also appends the log tail. We reproduced a 200 KB error from a 200 KB space value. Please summarize a mismatched body with bounded fields (truncate the space to a short fixed width, stringify the pid only when it is a short number or string) and cap the whole last-probe string to a few hundred bytes before it is appended.
  2. The operator pages are not updated. docs/watch-a-mesh.md and the cotal web --detach section of docs/cli.md should explain the new suffix: the probe URL, an HTTP refusal status, a transport error, and a mismatched space or pid. The URL is the bound probe address, not the browser launch URL.

We will re-review the next push.

@drakeo338

Copy link
Copy Markdown
Contributor Author

Both points are done. In the last-probe text the space is cut to 40 characters and the pid to 20, and the whole text is capped at 300 UTF-8 bytes. Both operator pages now describe the suffix exactly as summarizeProbeField and capProbeText behave. I also merged the latest main into the branch (one conflict in web.ts, resolved by keeping the probe change on top of the new session header), and typecheck, tests and the docs checks pass.

@davidfarah2003

Copy link
Copy Markdown
Contributor

Thanks for the push. The cap and the operator pages from the first round are in, and the functional claim still reproduces at the merge-base and is fixed at this head for HTTP status, transport refusal, malformed JSON and mismatched metadata. One thing is left:

  1. capProbeText (implementations/web/src/web.ts around line 1372) slices the buffer at PROBE_TEXT_MAX_BYTES (300) and then appends the three-dot marker, so the marker is outside the budget. A local 200 /api/meta answer whose space is U+0001 repeated 40 times and pid U+0001 repeated 20 times drives the JSON escaping past the limit: at this head the text after ; last probe is 303 bytes and the whole suffix 316. That contradicts the hard 300-byte promise in the changeset and in both operator pages (docs/cli.md line 1965, docs/watch-a-mesh.md line 130). The existing detach.smoke.ts only checks the total message stays under 500 bytes, so it does not see this. Reserve the marker's bytes before slicing at a valid UTF-8 boundary, say whether the documented budget covers the prefix, and assert the documented maximum in the existing smoke with an escaped or multibyte boundary input rather than a new suite.

We will re-review the next push.

drakeo338 and others added 4 commits October 10, 2026 01:27
… out

waitForDetachedWeb swallowed every probe failure, so a timeout said only
that the dashboard was not HTTP-ready. The error now ends with the probed
api/meta URL and the fetch error, HTTP status, or space and pid answered.
Retry cadence and deadline are unchanged.

Closes Cotal-AI#2653
A 200 response whose space or pid does not match is now summarized: the
space is cut to 40 characters, the pid is shown only when short, and the
whole last-probe text is capped at 300 bytes, so whatever answers the
port cannot inflate the timeout error. Document the suffix in
docs/watch-a-mesh.md and the cotal web section of docs/cli.md.
capProbeText cut the text at 300 bytes and then appended the three-byte
marker, so a mismatched /api/meta answer whose JSON escaping filled the
budget produced a 303-byte last-probe text. Cut three bytes short so the
marked text stays within the cap, say in the operator pages and the
changeset that the cap covers the URL and outcome after "; last probe "
with the marker counted, and make the existing detach smoke assert the
300-byte maximum with an escaped input.
@davidfarah2003
davidfarah2003 merged commit 261e48c into Cotal-AI:main Oct 10, 2026
21 of 33 checks passed
@davidfarah2003

Copy link
Copy Markdown
Contributor

Thanks for this, @drakeo338. A detached dashboard that misses its readiness deadline now reports the last probe URL and what that probe returned, with the message size capped. Two independent reviews approved the final head, and it typechecked cleanly against current main before we squash-merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage:needs-info A specific missing input prevents a meaningful reproduction attempt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

web: detached readiness probe discards every probe error, so a permanent failure reads as a generic timeout

2 participants