Repository navigation
fix: the probe tests read the client's initialize before they exit - #737
Merged
Merged
Conversation
a_global_images_own_entry_is_not_refused_through_its_stamped_label and its two siblings in the CLI's embedded_image.rs run `outrig mcp` over a probe, `sh -c 'printenv STOLEN >&2'`, and assert that its stderr, quoted in the startup error, holds the variable. live-e2e failed on them in 5 of the 73 trunk-targeting runs since they landed on Oct 20, and in 4 runs on version/0.3.x, as `exit: code 0` beside `stderr (...): (empty)` (#729). The probe is an exec-stdio server that writes one line and exits within milliseconds, and `podman exec -i` can return its exit status without that line. #422 found the same loss under `podman start --attach` and traced it to the client's `initialize` message still being unread when the process exits; it restores the text from conmon's log, which an exec session does not have (#727). The probe now reads that line before it prints. The library's embedded_image.rs has the same probe in `probe_label`, and launch_resolves_a_label_reference_the_spec_lists asserts on its stderr through `McpStartupFailure::stderr_tail`. It was not among the failures #729 counted, but it runs the same command through the same exec path, so it reads first too. The other copy of the command there, in a_spec_entry_replaces_the_label_entry_and_is_not_held_itself, only checks which error variant comes back and is left alone. Reproduced here without outrig: `podman exec -i` of the old probe with an `initialize` line already in its stdin pipe and the pipe held open, as rmcp leaves it, 24 at a time pinned to four cores. The old probe lost its stderr in 24 of 400 runs and in 19 of 400 on a repeat, each time with exit status 0, the CI symptom. The probe that reads first lost it in none of 800. Both embedded_image binaries pass locally under `--features e2e` (15 CLI tests, 13 library tests). What does not follow from this is why the read helps: the effect is measured, and the explanation is #422's applied to exec. Closes #729
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
a_global_images_own_entry_is_not_refused_through_its_stamped_label and its two siblings in the CLI's embedded_image.rs run
outrig mcpover a probe,sh -c 'printenv STOLEN >&2', and assert that its stderr, quoted in the startup error, holds the variable. live-e2e failed on them in 5 of the 73 trunk-targeting runs since they landed on Oct 20, and in 4 runs on version/0.3.x, asexit: code 0besidestderr (...): (empty)(#729).The probe is an exec-stdio server that writes one line and exits within milliseconds, and
podman exec -ican return its exit status without that line. #422 found the same loss underpodman start --attachand traced it to the client'sinitializemessage still being unread when the process exits; it restores the text from conmon's log, which an exec session does not have (#727). The probe now reads that line before it prints.The library's embedded_image.rs has the same probe in
probe_label, and launch_resolves_a_label_reference_the_spec_lists asserts on its stderr throughMcpStartupFailure::stderr_tail. It was not among the failures #729 counted, but it runs the same command through the same exec path, so it reads first too. The other copy of the command there, in a_spec_entry_replaces_the_label_entry_and_is_not_held_itself, only checks which error variant comes back and is left alone.Reproduced here without outrig:
podman exec -iof the old probe with aninitializeline already in its stdin pipe and the pipe held open, as rmcp leaves it, 24 at a time pinned to four cores. The old probe lost its stderr in 24 of 400 runs and in 19 of 400 on a repeat, each time with exit status 0, the CI symptom. The probe that reads first lost it in none of 800. Both embedded_image binaries pass locally under--features e2e(15 CLI tests, 13 library tests). What does not follow from this is why the read helps: the effect is measured, and the explanation is #422's applied to exec.Closes #729
Summary
What does this change, and why?
Checklist
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningsis cleancargo test --workspacepassesdoc/updated if behavior changed (mdbook buildstill clean)Notes for reviewers
Anything worth calling out -- tricky bits, follow-ups, or intentionally out-of-scope items.