fix(calm): share the Pi working-ship widget slot with standalone Calm - #1854
Open
ironerumi wants to merge 2 commits into
Open
fix(calm): share the Pi working-ship widget slot with standalone Calm#1854ironerumi wants to merge 2 commits into
ironerumi wants to merge 2 commits into
Conversation
Firstmate Calm and the user-global standalone Pi Calm both install an animated working-ship widget during agent runs. Each claimed its own Pi widget key, so a session loading both (the main Firstmate home) rendered two boats. Pi replaces widgets under one key, so claiming the shared "calm-working-ship" slot keeps dual-install sessions to a single boat while a Firstmate-only session is unchanged. Pins the shared slot contract in the working-ship module test so the key cannot silently diverge again.
…al-install The key-parity assertion inside the Pi fixture only runs where the @earendil-works/pi-coding-agent package is installed, so CI never exercised it. Add a source-level twin that needs nothing but the tracked file, and note in docs/calm.md that the boat shares the standalone Pi Calm working-row widget slot.
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
The developer wanted the already-validated Firstmate Calm widget de-duplication fix contributed upstream to kunchenguid/firstmate:main as a narrow pull request, under strict process constraints: read the current upstream contribution standards (AGENTS.md, CONTRIBUTING.md, the no-mistakes-required workflow, the coding-guidelines skill, and two scout reports) before touching anything; branch from freshly fetched upstream/main rather than the fork's history; and apply only the exact two-file patch from fork PR #17 (pointing Calm's widget key at the standalone Calm's shared calm-working-ship slot with a contract comment, plus a focused regression assertion in tests/fm-calm-pi-extension.test.sh), importing no unrelated fork commits or fork-private files. Delivery had to go through upstream's required no-mistakes pipeline with ironerumi/firstmate as push-only fork and kunchenguid/firstmate:main as PR base, without --yes and without merging. A hard constraint was commit identity: only a provably captain-owned, GitHub-attributable identity (ironerumi 39033099+ironerumi@users.noreply.github.com) was acceptable, with no agent co-author, and the unexplained sim sim@example.com pipeline identity was forbidden — the developer wanted the agent to stop and escalate rather than invent or rewrite identity. Mid-run the developer approved captain decisions to fix two review findings (add the smallest ungated constant-parity regression so CI enforces the shared key, and one concise docs/calm.md sentence on dual-install single-boat behavior) while keeping the shared-slot midrun-toggle finding as informational, to drop the scope-widening .gitignore entry, and to strip sim-authored commits — culminating in a read-only investigation of whether worktree-scoped git config could expose the captain identity to the pipeline, and then approval of a temporary repository identity switch for one authorized replacement validation run.
What Changed
CALM_WORKING_SHIP_WIDGET_KEYmoves from the privatefirstmate-calm-working-shipslot tocalm-working-ship, the key the standalone Pi Calm extension already uses. Pi replaces widgets per key, so a session that loads both Calms now renders one boat instead of two; either extension alone renders as before. A contract comment records that the slot must be renamed in both implementations together.tests/fm-calm-pi-extension.test.shgainstest_working_ship_widget_key_parity, a source-level assertion on the literal key that runs with no Pi installed, alongside the same check inside the working-ship fixture (which skips where Pi is absent, including CI). Review confirmed the ungated half is all this repo can enforce, since the standalone implementation lives outside this tree.docs/calm.mdstates the dual-install consequence: one boat, not two.Test reported a pre-existing
test_interactive_terminal_e2efailure in the same file that reproduces on base commit fb368dc and is untouched here.Risk Assessment
✅ Low: A one-line widget-key change with no stale references anywhere in the repo, backed by an ungated regression assertion that actually executes in CI and a single accurate docs sentence; the only residual concern is the deliberately accepted cross-extension slot-sharing tradeoff, which cannot regress single-install behavior.
Testing
Ran the single targeted suite tests/fm-calm-pi-extension.test.sh (twice) plus a base-commit baseline of the same suite, and a CI-shaped run with Pi, node and tmux removed from the environment to prove the new ungated parity assertion still executes where every Pi fixture skips; a mutation of the widget key made that assertion fail with its intended contract message, confirming it is a real guard. For product-level evidence I built a dual-install harness that launches real Pi 0.83.0 sessions in tmux with Calm on, the Firstmate Calm extension, and a stand-in for the standalone Pi Calm extension that claims the same working-row slot, and captured the live working row mid-run: the tracked code shows exactly one boat, the pre-fix private key in the same harness shows two stacked boats, and a Firstmate-Calm-only session is unchanged. Those captures and the new docs sentence are rendered into an HTML page and a screenshot. Everything related to the intent passes; the only failure, the live interactive /export e2e, reproduces identically on the base commit and traces to stale test expectations against the newer locally installed Pi rather than to this change.
/var/folders/l8/tyd57dc53v3938ttkwmw5kgr0000gp/T/no-mistakes-evidence/01KZCWGBSGYZRX5CJRXTJY6EQ9/dual-calm-evidence.png)Evidence: Same evidence as a rendered HTML page (color-faithful tmux panes)
Evidence: Dual-install working row — pre-fix private key vs shared key (live Pi 0.83.0 tmux panes)
Evidence: CI-shaped run: every Pi fixture skips, the new key guard still enforces the contract
Evidence: Dual-install e2e harness used to produce the captures
Evidence: Raw tmux pane captures (plain + ANSI)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
.pi/extensions/lib/fm-calm-working-ship.ts:56- Sharing the slot makes widget removal cross-extension, not just widget installation. In .pi/extensions/fm-calm.ts:145-153 a true->false transition calls ui.setWidget("calm-working-ship", undefined) and ui.setWorkingVisible(true). In a dual-install session that clears whatever occupies the shared slot, so toggling Firstmate Calm off mid-run (registerCommand handler at fm-calm.ts:452) or agent_settled (line 436) removes the standalone Calm's boat and restores Pi's stock working row until the other extension's next transition. The mirror case is symmetric: if the standalone clears the slot, Firstmate's cached workingShipShown stays true, so it will not re-install its boat until the next agent_start. Both windows are bounded by the remaining run and neither regresses single-install behavior, which is the tradeoff the shared-slot contract comment deliberately accepts; recording it only so the accepted scope is explicit.tests/fm-calm-pi-extension.test.sh:1954- The new parity assertion (and its fixture twin at line 2021) can only pin this repo's half of a two-implementation contract; the standalone Pi Calm extension is not present in this tree (no in-repo calm implementation outside .pi/extensions/), so a rename on that side silently restores the duplicate-boat defect with this suite still green. The source comment's "rename the slot in both implementations together" is the only available enforcement, which is the correct honest bound for a repo that does not own the other artifact. No in-repo action possible.tests/fm-calm-pi-extension.test.sh:3261- Pre-existing, unrelated failure in the same test file:test_interactive_terminal_e2efails at/export did not complete while calm mode was on. It fails identically on the base commit fb368dc (verified by checking out the base versions of the three touched files and re-running), so this change did not introduce it. Direct probe of the same flow against the locally installed Pi 0.83.0 shows the product is fine:/exportunder Calm does write its HTML artifact (279KB), butM-sno longer submits the composer (Enter does) and theSession exported to: <path>status text never appears in the pane. The suite documents verification against Pi 0.81.1/0.82.0, so this reads as test staleness against the newer locally installed Pi rather than a Calm defect; it also self-skips wherever Pi/tmux are absent, so it does not gate CI. I left it untouched because fixing it would widen this deliberately narrow two-file upstream patch.bin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh(run twice; the newtest_working_ship_widget_key_parityand the working-ship fixture with its in-fixture key check both pass)Baseline comparison:git checkout fb368dc -- .pi/extensions/lib/fm-calm-working-ship.ts tests/fm-calm-pi-extension.test.sh docs/calm.md && bin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh(then restored to 7dcdd89) — same/exportfailure at baseCI-shaped run with no Pi/node/tmux:env -i PATH=/usr/bin:/bin:/usr/sbin:/sbin FM_PI_PACKAGE_DIR=/nonexistent bash bin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh— every Pi-dependent fixture skips, the widget-key parity check still runs and passesMutation check of the new guard: sed the key tofirstmate-calm-working-ship, re-run the same CI-shaped command — fails with the intended contract message, then restored the fileDual-install end-to-end in real Pi 0.83.0 + tmux:dual-calm-e2e.sh shared(Firstmate Calm + standalone-Calm stand-in, tracked source) → 1 boat rowdual-calm-e2e.sh private(same harness, key reverted to the pre-fixfirstmate-calm-working-shipin the fixture copy only) → 2 boat rowsdual-calm-e2e.sh solo(Firstmate Calm alone, shared key) → 1 boat, single-install rendering unchangedManual probe of the pre-existing failure: drove/export <path>in a live Calm session withM-sand withEnter, confirmed the exported HTML file is written on Pi 0.83.0 while the pane status text never appearsRendered the three ANSI pane captures plus the new docs/calm.md sentence to HTML and screenshotted with headless Chrome.pi/extensions/lib/fm-calm-working-ship.ts:51- Judgment call, left as-is: the cross-repo rename contract ("rename the slot in both Calm implementations together or dual-install duplicates the boat") is owned solely by the code comment at .pi/extensions/lib/fm-calm-working-ship.ts:51-55 and enforced by two assertions in tests/fm-calm-pi-extension.test.sh (ungated source-level parity check plus the Pi-gated fixture check). The maintainer-architecture section docs/calm-mode-feasibility.md 'Calm working presentation' does not restate it. Per the placement policy, code comments own external constraints and safety invariants, so adding a prose copy there would create a second location to keep in sync; docs/calm.md:7 already carries the user-visible consequence. No edit made.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.