feat(sessions): make failed-host hiding opt-in - #3547
Conversation
Add "Hide unreachable hosts" setting (on by default) for Agent Sessions, filtering Tailscale-discovered and manually added SSH hosts whose session fetch failed out of the menu instead of showing an unreachable row. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
I have compiled and tested this and shown working. There might need to be some more fidly filtering, but it does reduce a list of 20 tail nodes that don't have codexbar or claude on them from being displayed. |
|
Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 8:49 PM ET / September 12, 2026, 00:49 UTC (Revision 4). ClawSweeper reviewWhat this changesAdds a saved, off-by-default setting that hides failed remote-session hosts from the menu, with localized controls, documentation, and regression coverage. Merge readiness✅ Ready for maintainer review The PR remains useful: current main and v0.59.0 lack this preference. The maintainer-approved opt-in behavior preserves existing defaults, has convincing native proof, and has no identified blocking defect. Priority: P3 Review scores
Verification
How this fits togetherCodexBar combines local agent sessions and SSH host results into its menu. This preference filters failed-host diagnostic rows during rendering without changing discovery, fetching, or authentication. flowchart TD
A[Local agent sessions] --> D[Session menu builder]
B[SSH host results] --> C[Apply saved visibility preference]
E[Settings toggle] --> C
C --> D
D --> F[Session rows and visible diagnostics]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep failed-host diagnostics visible by default and offer reversible, persisted hiding solely at the menu-rendering boundary. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this adds an optional display preference, and the inspected native screenshots demonstrate its visible effect. Is this the best way to solve the issue? Yes. Filtering during menu construction reuses the existing error classification, preserves fetching and defaults, and matches the owner's approved direction. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 830cdd86c4b6. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
steipete
changed the title
|
Maintainer native proof passed with the repaired opt-in behavior. Built the current CodexBar sources and launched a Developer ID–signed, isolated XCTest host using the production settings section and the same controller descriptor path used by the live menu. An injected session store supplied only
The interactive XCTest passed with zero failures. The 19 descriptor/settings tests and 41 architecture-gate tests pass; default and explicit true/false reloads have isolated regression coverage. Locale coverage, make check, and independent P0–P2 review pass. The attached captures contain synthetic data only. The first proof attempt left a native popup tracking; the verified run closes it before changing the setting and yields during the interactive wait. The final branch also reanchors existing architecture checks without changing their provider memberships or fingerprints. The changelog line and contributor credit are in #3567. Exact-head CI remains the final readiness gate.
|
clawsweeper
Bot
added
P3
|
Final CI passed on The full local suite also passed 1,074 selections in 90 groups with zero failures or retries. Signed native visible → hidden → restored proof and inspected synthetic screenshots are recorded above. Make check, all 41 architecture-gate tests, and final independent P0–P2 review pass. Ready for squash with @warthurton's credit; keep #3567 last for the centralized notes. |
Collect the ordered 0.59.1 Unreleased notes and contributor credits after #3539, #3566, #3547, #3525 and #3544. Preserve all released sections. Separate process-fixture startup from measured cleanup deadlines and use the existing task-local authentication hook in Claude retry fixtures. Retain retry, timeout, ownership, and dedicated authentication tests. Dependency pins remain current within their existing policies; larger migrations remain separate.
2 participants
Add an opt-in Settings → Menu → Agent sessions → “Hide unreachable hosts” control. It is off by default for both new and existing installations, so connection, authentication, incompatible-CLI and decoding failures remain visible unless the user explicitly chooses to hide them. Fetching and retries continue unchanged; turning the option off restores the diagnostic rows.
Reconciled with current main, preserved explicit saved choices, completed the two strings in all 22 required locales, and added isolated default/persistence/menu-observation coverage. The native proof uses the production settings section and live controller descriptor path, fed only by an injected synthetic session store. Thanks @warthurton for the original contribution. The release note is centralized in #3567.
The focused descriptor/settings suites pass 19 tests, locale validation passes, and independent P0–P2 review of the current implementation is clean. Signed interactive proof passed visible → hidden → restored through the actual control and production controller path. The full local suite passed 1,074 selections in 90 groups, with zero failures or retries. Make check and 41 architecture tests pass. Exact-head CI is green: https://github.com/steipete/CodexBar/actions/runs/34658787278. Native screenshots and the detailed proof are in #3547 (comment). This is a menu visibility preference; it does not change SSH discovery or authentication policy.