Poll for container log lines in the smoke test instead of grepping once - #10
Merged
Merged
Conversation
Both log assertions sampled `docker logs` exactly once, immediately after
/healthz answered. The app writes those lines before uvicorn binds, so they
have certainly been written by then — but the daemon's log pipeline lags by a
few milliseconds, and a one-shot grep loses that race under load.
It lost in smtp-mcp-wrapper's weekly rebuild on 2026-08-24: the guard grep
missed a line the failure dump printed 12ms later, in a run whose behavioural
checks (foreign Host -> 421) proved the guard was working. This script has the
same two greps, and because fail() exits, a lost race aborts the run and takes
every later phase with it.
Replace them with wait_for_log, which polls for up to 10s. A line that
genuinely never appears still fails, just at the timeout instead of instantly.
The allowlist-value assertion is unchanged — it still greps the full line
including ${ROUTE_HOST}.
Verified by extracting wait_for_log from the script and driving it against a
fake `docker logs`: with the line withheld for 3s the original one-shot grep
returns 1 (reproducing the abort) while wait_for_log returns 0 after ~3s; with
the line permanently absent wait_for_log still fails at its timeout, so the
assertion has not become vacuous. Full smoke test passes all four phases with
the patch applied.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QoUUHzB1EGb9BMjbqG2kqn
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.
Both log assertions in
scripts/smoke_test.shsampleddocker logsexactly once, immediately after/healthzanswered. The app writes those lines before uvicorn binds, so they have certainly been written by then — but the daemon's log pipeline lags by a few milliseconds, and a one-shot grep loses that race under load.It lost in
smtp-mcp-wrapper's weekly rebuild on 2026-08-24: the guard grep missed a line the failure dump printed 12ms later, in a run whose behavioural checks (foreign Host → 421) proved the guard was working. This script has the same two greps, and becausefail()exits, a lost race aborts the run and takes every later phase with it.Change
Replaces both one-shot greps with
wait_for_log, which polls for up to 10s. A line that genuinely never appears still fails, just at the timeout instead of instantly. The allowlist-value assertion is unchanged — it still greps the full line including${ROUTE_HOST}, so a guard that comes up with the wrong allowlist is still caught.Verification
The concern with any fix for flakiness is that it stops asserting anything. Both halves were checked by extracting
wait_for_logfrom the script and driving it against a fakedocker logsthat withholds the line for a set interval:The third row is direct evidence the race can abort a run rather than only a plausible story; the fourth is what keeps the assertion honest.
The full smoke test also passes all four phases with the patch applied, including the 2026-07-28 stateless path and cache-hint phases.
Note for the reviewer — a latent hazard left alone
docker logs | grep -qunderset -o pipefailhas a pre-existing subtlety:grep -qexits at the first match, and if the producer is still writing it takes SIGPIPE, so the pipeline returns 141 even though the match succeeded.This only bites once output exceeds the 64KB pipe buffer. These containers emit roughly 20 lines, so it cannot trigger here — the logs would need to grow by about three orders of magnitude. It is also not introduced by this change: the original assertions and
fail()have the same shape, and insidewait_for_logthe pipeline sits in anifcondition, so the worst case would be a 10s timeout rather than an abort. Deliberately not changed here; noted in case the pattern gets reused somewhere logs are voluminous.Generated by Claude Code