fix(test): settle Pi follow-up pane before the duplicate-captain-answer check - #2
Merged
Merged
Conversation
…er check The adjacent-follow-up E2E case captured the tmux pane for its duplicate-captain-answer assertion immediately after the session file confirmed processing, with no settle wait, unlike every other readiness check in this test. Sending two followUp deliveries queues more Calm presentation work (an extra operational-user row plus its hiding invalidation) than a single one, so the already-settled captain answer's redraw could still be in flight at that instant, making the check flaky. Poll the pane the same way the session-file wait already does, and track the peak count seen along the way so a captain answer that is genuinely rendered twice for even one frame still fails even if a later redraw were to self-correct.
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.
Intent
Fix the pre-existing Firstmate main-branch failure where tests/fm-calm-pi-extension.test.sh's Pi follow-up adjacent case (two followUp deliveries queued back-to-back in message_start) intermittently reported a rendered duplicate captain answer. Diagnosis via repeated E2E reproduction and fine-grained pane probing (many runs, up to 150 probes at 10-20ms granularity) showed the underlying session file was always correct (exactly one CAPTAIN_ANSWER, one merged MONITOR_HANDLED_..._ONE_TWO turn) and the rendered pane count never exceeded 1 -- it only ever raced between 0 (not yet redrawn) and 1 (settled), never 2. Git history confirmed the pane capture for this specific assertion has been a single, unpolled snapshot taken immediately after the session-file readiness loop since the check was introduced in commit 65ad47b, unlike every other readiness check in this test which polls. Sending two adjacent followUp deliveries queues more Calm presentation work (an extra operational-user row plus its Calm-hiding invalidation) than a single delivery, so the already-correct captain answer's TUI redraw could still be in flight the instant the session file confirmed processing, making this one assertion flaky specifically in the adjacent case. The fix polls the pane the same way the adjacent session-file wait already does (same idiom, bounded iterations, same sleep granularity) before evaluating the duplicate-answer count, and additionally tracks the peak count observed during that poll so a genuine transient duplicate frame would still fail the assertion even if a later redraw self-corrected -- this strengthens the check rather than weakening it. The original exactly-one assertion and its failure message are unchanged; no product/extension code (fm-calm.ts, fm-calm-assistant-layout.ts, fm-calm-visibility.ts, fm-calm-operational-user-layout.ts, fm-calm-working-ship.ts) was touched, since the defect was entirely in the test's own settle-wait omission. Reviewed pi and pi-signed integration surfaces: pi-signed execs the identical Pi engine/TUI as pi (per harness-adapters skill) and has no separate Calm extension surface, so this test (which already runs against whatever pi binary is installed) covers both; no separate pi-signed test path exists or is needed. Verified the fix with 8+ consecutive clean runs of the full tests/fm-calm-pi-extension.test.sh file (all sub-cases, not just adjacent) and a clean bin/fm-lint.sh pass on the changed file.
What Changed
Risk Assessment
✅ Low: The change is narrowly scoped to test synchronization, preserves the exactly-one pane assertion, records the peak observed captain-answer count to catch sampled transient duplicates, and conforms to the stated durable test-fix intent without changing product code.
Testing
Three consecutive full
fm-calm-pi-extensionE2E runs passed against the installed Pi binary, exercising the real tmux TUI, adjacent back-to-back follow-ups, peak and settled duplicate-answer detection, persisted session semantics, Calm visibility, and restart behavior. Two executable transcripts were retained as evidence. No screenshot was captured because the terminal test automatically tears down its ephemeral tmux session; its pane contents and JSONL session state are asserted directly before teardown.Evidence: Pi Calm E2E run 2 transcript
Source: Pi Calm E2E run 2 transcript
Evidence: Pi Calm E2E run 3 transcript
Source: Pi Calm E2E run 3 transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspectedgit diff 52ff62e8253623b4fafd125c52cdd4c13a3a84b1..f5c983cb389874995f1bc7fd48cf64456784e962 -- tests/fm-calm-pi-extension.test.shRanbash tests/fm-calm-pi-extension.test.shthree consecutive timesCaptured runs 2 and 3 withbash tests/fm-calm-pi-extension.test.sh 2>&1 | tee <evidence-log>Verified cleanup withgit status --short✅ **Document** - passed
✅ No issues found.
⏭️ **Lint** - skipped
🔧 Fix: Verify lint with pinned toolchain
1 warning still open:
✅ **Push** - passed
✅ No issues found.