Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
Configure Firstmate in Pi so the routine sailboat-prefixed supervision updates shown in the supplied screenshot do not appear in chat. Check whether an existing configuration already controls them, check the current remote implementation, update Firstmate, and keep important updates visible.
What Changed
Risk Assessment
✅ Low: Captain, the change is narrowly scoped to presentation, preserves health and captain-facing updates, and introduces no material source-verifiable risk.
Testing
Installed Pi 0.85.1 was driven live in an isolated home. Calm hid live and restored routine notes,
/calmrepainted them reversibly, and current plus legacy health and captain updates stayed visible. Two focused executable regressions passed, including provider-error pause and recovery behavior. After correcting the in-memory export setup, persisted-session export succeeded. macOS Screen Recording permission was unavailable, so the evidence contains exact TUI recordings instead of PNG screenshots. The worktree remained clean.calm-on-tui.typescript: Pi omitted the routine-worker row with Calm on, restored it after/calmoff, and removed it again after/calmon.calm-on-tui.typescript: pause, recovery, legacy captain, and current sequence-keyed captain rows remained visible in Calm.calm-restored-session.typescriptand the persisted JSONL session show Pi suppressing the restored routine row while retaining restored health and captain rows./calmtoggles repainted the existing row without recreating it, while the persisted JSONL retained the routine message. The focused renderer test also passed its stock-export rendering assertio…Evidence: Focused executable regressions
Evidence: Live Pi Calm toggle recording
Source: Live Pi Calm toggle recording
Evidence: Restored Pi session under Calm
Source: Restored Pi session under Calm
Evidence: Persisted Pi session used for restore testing
Source: Persisted Pi session used for restore testing
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
.pi/extensions/fm-branch-supervision.ts:2239- The intent requires “keep important updates visible,” but Calm now hides every restoredfm-branch-mergemessage except the two health strings. A session created by older Firstmate versions can contain a displayed captain outcome such as⚓ task: resultor⎇ branch merged [captain] ...; this line selectsCalmAwareRoutineNote, whoserender()returns no rows. Preserve non-sailboat legacy messages (or explicitly recognize legacy captain formats) and suppress only recognized⛵routine notes.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
calm-on-tui.typescript: Pi omitted the routine-worker row with Calm on, restored it after/calmoff, and removed it again after/calmon.calm-on-tui.typescript: pause, recovery, legacy captain, and current sequence-keyed captain rows remained visible in Calm.calm-restored-session.typescriptand the persisted JSONL session show Pi suppressing the restored routine row while retaining restored health and captain rows./calmtoggles repainted the existing row without recreating it, while the persisted JSONL retained the routine message. The focused renderer test also passed its stock-export rendering assertio…pi --versionLaunched installed Pi with an isolatedFM_HOME, the target Calm and branch-supervision extensions, and a test-only evidence extensionEntered/calmtwice in the live Pi TUI to verify immediate restoration and re-suppressionPersisted the transcript and resumed it usingpi --session ...without the injection extensionRan onlytest_branch_dispatch_two_stage_filter_and_prefix_contractfromtests/fm-pi-branch-extension.test.shRan onlytest_post_construction_provider_error_falls_back_latches_and_recovers_on_cooldownfromtests/fm-pi-branch-extension.test.shAttempted in-memory/export; Pi rejected exporting a--no-sessionsession, so the transcript was persisted andpi --exportwas rerun successfullyRan the macOS screenshot permission preflight; Screen Recording permission was unavailableConfirmed testing left the git worktree clean; no full suite, linter, or formatter was run✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.