Skip to content

test(relayflows): stop the readiness probe dying on "broker is starting" - #1847

Merged
khaliqgant merged 1 commit into
mainfrom
fix/muse-startup-readiness-probe
Sep 23, 2026
Merged

khaliqgant merged 1 commit into
mainfrom
fix/muse-startup-readiness-probe

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Sep 23, 2026

Copy link
Copy Markdown
Member

Problem

Flows v2 shard 4 fails intermittently on unrelated PRs with:

SyntaxError: Unexpected token 'B', "Broker is "... is not valid JSON
    at async api (tests/relayflows/cases/muse-unattended-startup/run.mjs:182:45)
    at async waitFor (tests/relayflows/cases/muse-unattended-startup/run.mjs:52:19)
RelayFlow case muse-unattended-startup exited 1

The case's api() helper parsed the body before anyone checked the status:

return { status: response.status, body: await response.json() };

and the readiness probe is built on it:

await waitFor(
  () => api('GET', '/api/session').then(({ status }) => status === 200),
  'broker API readiness'
);

While the broker starts it answers Broker is starting, please retry as plain text — exactly the state the probe exists to wait out. response.json() throws on it, and because waitFor treats a rejected probe as fatal rather than falsy, the case fails instead of retrying. It is a pure startup race: nothing about the code under test is wrong.

Fix

Read the body as text and parse it only when it is JSON, keeping the raw text as body so assertion messages still show what came back. broker-local-only survives the same response only because it wraps its probe in try/catch { return false }; fixing the helper addresses the cause, so every call site in the case benefits rather than one probe.

Proof

A local server that answers exactly as the broker does — plain text 503 first, JSON 200 after — run against both helper versions:

OLD helper: THREW -> SyntaxError: Unexpected token 'B', "Broker is "... is not valid JSON
NEW helper: READY

The old helper reproduces the CI error verbatim; the new one retries and reaches ready.

Scope

Test-only, one file. No product behaviour changes, so there is no changelog entry. Four of the other cases that parse unconditionally talk to their own /observations fixture, which is always JSON; broker-local-only is the fifth and is already guarded.

🤖 Generated with Claude Code

Review in cubic


Note

Low Risk
Test harness-only change; no production code or runtime behavior affected.

Overview
Fixes a flaky muse-unattended-startup relayflow where the broker readiness waitFor could fail during startup instead of retrying.

The case’s api() helper no longer calls response.json() unconditionally. It reads the body as text, parses JSON when possible, and otherwise keeps the plain-text body (e.g. Broker is starting, please retry). The probe only checks HTTP status, so non-JSON startup responses no longer throw and abort waitFor.

Test-only change in tests/relayflows/cases/muse-unattended-startup/run.mjs; no product behavior changes.

Reviewed by Cursor Bugbot for commit ecc495f. Bugbot is set up for automated code reviews on this repo. Configure here.

The muse-unattended-startup case builds its `api()` helper around
`await response.json()`, so the body was parsed before any caller looked at
the status. The broker answers `Broker is starting, please retry` as plain
text while it comes up -- precisely what the readiness probe is waiting to
stop seeing -- so the parse threw, the rejection escaped `waitFor`, and the
whole case failed on a startup race rather than retrying.

The helper now reads the body as text and parses it only if it is JSON,
keeping the raw text as the body so an assertion message still shows what
came back. `broker-local-only` already survives this by wrapping its probe
in try/catch; this fixes the cause instead, so every call site benefits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-23T01:58:45.138547Z ecc495f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e32d9e77-fc02-434e-aeec-b50fdef32e92

📥 Commits

Reviewing files that changed from the base of the PR and between 12ce647 and ecc495f.

📒 Files selected for processing (1)
  • tests/relayflows/cases/muse-unattended-startup/run.mjs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The readiness API helper now parses response text as JSON when possible and returns raw text when parsing fails. This prevents plain-text broker startup responses from throwing during readiness probes.

Changes

Startup response handling

Layer / File(s) Summary
API response parsing
tests/relayflows/cases/muse-unattended-startup/run.mjs
The api helper reads response text, attempts JSON.parse, and falls back to the raw response text. It continues to return { status, body }.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ecc49

The readiness probe can retry while the broker returns its temporary plain-text startup response. The test-only change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains the problem, fix, proof, and scope, but it does not use the required template sections. It omits the Test Plan checkboxes and the required RelayFlow Proof fields. Add the required Summary, Test Plan, RelayFlow Proof, and Screenshots sections. Set Change type to non-functional and RelayFlow case to n/a, and mark the applicable test-plan items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: preventing the readiness probe from failing when the broker reports that it is starting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the broker at dawn
Plain words arrive, and probes move on
JSON hops when it can
Text rests safely in hand
Startup races fade before long

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@khaliqgant
khaliqgant merged commit ea7cb75 into main Sep 23, 2026
38 of 41 checks passed
@khaliqgant
khaliqgant deleted the fix/muse-startup-readiness-probe branch September 23, 2026 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants