fix(pi): restore Calm /export status and deflake follow-up e2e captures under Pi 0.84 - #5
Merged
Merged
Conversation
Pi 0.84+ showStatus replaces the previous status row in place. Calm post-/export setToolsExpanded redraw overwrote Session exported status with Tool output collapsed, so e2e never saw export complete even though the HTML file was written. Drop that tools-expanded toggle from the export/share input path. Arming stock HTML rendering only needs the stockExportRendering flag; /calm still redraws when presentation actually changes. Unit coverage fails if export/share calls setToolsExpanded. The duplicate captain answer failure was a misnamed assertion on count != 1: session data kept one answer, but plain tmux capture-pane under Pi 0.84+ intermittently missed short assistant rows (join capture always saw exactly one). Wait on joined pane captures and require exactly one on-screen captain answer.
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
2 participants
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 two e2e test failures present on the firstmate default branch since the Pi 0.84.0 upgrade: (1) a duplicated captain answer, and (2) /export never completing when Calm is on. These are pre-existing failures on clean main (first noticed 2026-08-07), so every future branch inherits them. Make main's suite honestly green: either the code regressed against intended behavior (fix the code) or Pi 0.84.0 legitimately changed behavior the tests over-pinned (fix the tests) - decide per failure from evidence, not convenience.
Diagnostic discipline required: reproduce first on clean worktree at default-branch tip before changing anything; separate initiating trigger (what Pi 0.84.0 changed) from any masking condition and the visible symptom; check history of what tests pinned at 0.82.0 vs Pi 0.84 behavior; smallest counterfactual wins; each fixed test must fail on old broken behavior and pass on the fix.
Context: installed Pi is 0.84.x; tests last known green on 0.82.0. Calm is a Pi presentation extension owned by this repo. Do NOT expand into full Pi 0.84.0 smoke re-verification - fix exactly these two failures and any directly-entangled test infrastructure.
Findings and fix that landed: (1) /export: Pi 0.84+ showStatus replaces prior status row in place; Calm post-export setToolsExpanded toggle overwrote 'Session exported to' with 'Tool output: collapsed' even though HTML was written - fixed by dropping setToolsExpanded from export/share path while keeping stockExportRendering flag arming; unit regression fails if export/share calls setToolsExpanded. (2) captain answer: not a product duplicate - session always had one CAPTAIN_ANSWER; plain tmux capture-pane under Pi 0.84+ intermittently missed short assistant rows while join capture always saw exactly one; old assertion treated count!=1 as 'duplicate' including count 0 - fixed by joined pane captures, waiting for on-screen captain answer, and accurate exactly-one assertion.
Acceptance: both failures reproduced with root cause trigger/mask/symptom, fixes committed with regression coverage, touched-surface tests green, no-mistakes pipeline green with a PR on origin (Bloh09/firstmate fork) against main.
What Changed
.pi/extensions/fm-calm.tsno longer cyclessetToolsExpandedafter/exportand/share: under Pi 0.84+showStatusreplaces the prior status row in place, so the post-export toggle overwrote "Session exported to: ..." with a tool-output status. The export pass still arms and resets the stock-rendering flag, and the unit harness now fails if the export/share path callssetToolsExpanded.tests/fm-calm-pi-extension.test.shswitches tmux pane captures tocapture-pane -J(joined soft-wrapped lines), since plain captures under Pi 0.84+ intermittently miss short assistant rows, and replaces the follow-up "duplicate captain answer" check with a poll that waits for the answer on screen and asserts exactly one occurrence (the old check reported count 0 as a duplicate).docs/calm-mode-feasibility.mdrecords the new invariant: only genuine presentation changes such as the/calmtoggle use the tool-expansion redraw, and the export/share pass never cycles tool expansion.Risk Assessment
✅ Low: A minimal, well-documented root-cause fix (removing the status-clobbering toggle) plus behavioral regression coverage that fails on the old code, with test hardening whose remaining plain captures are intentionally geometry-sensitive; all findings are informational.
Testing
Reproduced the pre-fix failure (new export regression fails on base-commit fm-calm.ts), then ran the three targeted tests from tests/fm-calm-pi-extension.test.sh against installed Pi 0.84.1 - all passed - and collected end-user evidence: a tmux pane transcript showing "/export" completing with Calm on ("Session exported to: ..." as the final status row, not clobbered by a tools-expanded status), pane transcripts showing exactly one captain answer in every follow-up e2e case, and a headless-Chrome screenshot of the exported HTML rendering as a complete session viewer. The fail-before counterfactual for the captain-answer test was not run because the old plain-capture miss was intermittent by nature and cannot be reproduced deterministically; the fixed test is strictly stronger (waits for the answer, rejects count 0 and count >1) and passed live.Evidence: Counterfactual: unit regression fails on pre-fix fm-calm.ts
== Counterfactual: tests/fm-calm-pi-extension.test.sh unit regression vs PRE-FIX .pi/extensions/fm-calm.ts (base a2ba0efc) == not ok - Pi calm renderer and lifecycle contract failed: ... Error: /export calm.html export/share path must not call setToolsExpanded (would clobber export status) EXIT=1Evidence: Live TUI pane: /export completes with Calm on, status row intact
Session exported to: /var/folders/pb/.../fm-calm-pi-extension.0igVir/calm-export.html (final status row above the editor; not overwritten by "Tool output: expanded/collapsed")Evidence: Live TUI pane: exactly one captain answer (Calm on case)
CAPTAIN_PROMPT_loaded_on CAPTAIN_ANSWER_loaded_on MONITOR_HANDLED_loaded_on_ONE (joined capture-pane -J; one CAPTAIN_ANSWER per case across all 8 e2e cases)/var/folders/pb/bf5sf79j61bc37mnkt9kwt3h0000gn/T/no-mistakes-evidence/01KZN1J2H6169224Y4QEGP67JY/calm-export-render.png) - Evidence: Exported session HTML produced by /export with Calm on (local file:/var/folders/pb/bf5sf79j61bc37mnkt9kwt3h0000gn/T/no-mistakes-evidence/01KZN1J2H6169224Y4QEGP67JY/calm-export.html)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
.pi/extensions/fm-calm.ts:208- The /calm command handler retains the same setToolsExpanded double-toggle redraw that caused the export bug under Pi 0.84's in-place showStatus replacement. Here it is harmless to the fixed invariant (the handler clears its own status first and no export status exists to clobber), and the new comment in the export path documents this as deliberate, but a lingering 'Tool output: ...' status after /calm toggles is a cosmetic side effect of the same mechanism if Pi renders a status for setToolsExpanded. Out of the stated fix scope; noting for awareness only.tests/fm-calm-pi-extension.test.sh:3404- The e2e export test asserts 'Session exported to:' appears and the exported HTML contains tool data, but has no 'tool rows still hidden on screen' assertion between export completion and the subsequent /calm toggle. The removed toggle previously forced a live redraw after the stock-rendering window closed; the fix relies on the (documented) claim that export never re-renders the live transcript. A post-export assert_not_contains for a calm-hidden marker would pin that counterfactual directly.tests/fm-calm-pi-extension.test.sh:1716- The new captain-answer wait loop caps at 80 iterations (~4s) while sibling wait loops in the same test use 120-240 iterations. The loop only starts after the session file already contains the handled marker, so only render latency remains and 4s is likely enough, but aligning with the 120-iteration convention would give more headroom on slow CI machines if flake recurs.✅ **Test** - passed
✅ No issues found.
Counterfactual: reverted.pi/extensions/fm-calm.tsto base a2ba0ef and rantest_rendering_and_session_lifecyclefromtests/fm-calm-pi-extension.test.sh(via a temporary driver sourcing the test functions) - failed as required with "/export calm.html export/share path must not call setToolsExpanded"test_rendering_and_session_lifecycle(fm-calm renderer/lifecycle unit fixture incl. the new export setToolsExpanded regression) on fixed code - passedtest_operational_followup_turn_e2e(live Pi 0.84.1 in tmux; captain-answer joined-capture wait and exactly-one assertion across Calm on/off/absent/exact-watcher/restart cases) - passedtest_interactive_terminal_e2e(live Pi 0.84.1 TUI incl./exportwith Calm on,Session exported to:status assertion, export HTML content and headless-Chrome DOM checks) - passedManual evidence capture: tmux shim teed full-scrollback pane captures; verified every capture contains exactly one CAPTAIN_ANSWER per label and the export pane's final status row isSession exported to: ...(the earlierTool output: expandedline is the e2e's own deliberate Ctrl+O toggle in scrollback)Rendered the Calm-on exported HTML in headless Chrome and captured a screenshot showing the full session viewerCleanup verified: temporary driver/shim/capture dirs removed, no stray tmux servers,git statusclean✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.