Add top-level plugin switcher tabs - #3516
Conversation
|
🦞👀 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. |
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 9, 2026, 3:59 PM ET / 19:59 UTC (Revision 10). ClawSweeper reviewWhat this changesAdds opt-in provider-plugin tabs to the merged menu switcher, preserves legacy plugin cards, and routes standard Refresh controls to the selected plugin. Merge readiness⛔ Blocked before merge - 3 items remain This remains a useful, owner-approved enhancement absent from current main and the latest release. Earlier concrete findings are addressed, but real native behavior and upgrade compatibility remain unproven. Priority: P2 Review scores
Verification
How this fits togetherCodexBar loads local provider plugins and displays their usage snapshots in its menu bar interface. The switcher combines enabled providers and plugin metadata to select which cards appear and which provider receives Refresh actions. flowchart TD
A[Plugin manifests and settings] --> B[Enabled provider identities]
B --> C[Menu switcher selection]
C --> D[Selected plugin and legacy cards]
C --> E[Refresh action]
E --> F[Existing plugin approval and runtime]
F --> G[Provider usage snapshot]
G --> D
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the approved opt-in design, with unchanged legacy placement and preferences demonstrated on fresh and upgraded installations. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this adds an approved capability, and current-main and release source confirm that plugin tabs are absent. Is this the best way to solve the issue? Yes: the opt-in flag reuses existing provider identities, settings, rendering, and approval checks; native compatibility validation is still needed. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 5e5a9003efb2. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (9 earlier review cycles; latest 8 shown)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 681c63098f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if case let .provider(instanceID) = switcherSelection, | ||
| UserProviderPluginRegistry.plugin(for: instanceID) != nil | ||
| { | ||
| self.addUserPluginMenuCards( | ||
| to: menu, | ||
| width: context.menuWidth, | ||
| selectedPluginID: instanceID) |
There was a problem hiding this comment.
When a top-level plugin tab is selected, this branch renders only that plugin's card, but the persistent Refresh row and ⌘R remain in the actionable sections. manualRefreshProvider(for:) resolves merged menus through resolvedMenuProvider(), which discards plugin IDs and returns a first-party provider, so those refresh controls refresh Codex or another fallback rather than the visible plugin. Preserve the selected plugin ID and route this action through refreshUserPlugin(_:) so the tab's standard refresh controls target the content being shown.
Useful? React with 👍 / 👎.
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. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
1 participant
Summary
topLevel: trueprovider-plugin manifest fieldProduct boundary
This changes placement only. Plugins keep the existing sandbox, first-network approval, generic snapshot/details rendering, settings enablement, and provider-data isolation. Plugins without
topLevel: trueretain appended-card behavior, including when a top-level plugin tab is selected.Edge-case behavior
refreshUserPlugin, not Codex/global refreshValidation
Exact head:
b4cd88a019da01001895caf622123c6c8925ad3dswift build --skip-update --target CodexBarCore— passed.build/lint-tools/bin/swiftformat <changed Swift files>— 0 files formattedgit diff --check— passedmake checkphases through repository Python tests — passedad77955ad8f48b30a7617d1ac5bb08bac99420c51b44a96bfa7ecfd1035401df); one advisory plugin-only status-bar styling enhancement was deferred because it broadens the provider-instance icon pipeline and is not a regressionThe focused regressions now exercise the visible attached singleton menu, real selected-plugin refresh dispatch, plugin-only width measurement, cache-signature collisions, and plugin-owned menu-open behavior. The refresh test produces the selected plugin snapshot through an explicitly approved temporary binding and approval store. The local Command Line Tools-only environment lacks Swift
Testing, SwiftUI macro implementations,sourcekitdInProc.framework, and Pythonwaitid(WNOWAIT), so the focused Swift test target, full app build, SwiftLint, and sharded suite could not complete locally; GitHub macOS CI is authoritative. No user approval file, Keychain/cookie import, or live account probe was used.Native app proof was explicitly approved and attempted with
./Scripts/compile_and_run.sh. Release compilation reached the widget target, but this host has no full Xcode installation and cannot loadSwiftUIMacros.EntryMacro, so it could not package or launch a fresh bundle. The failure was toolchain-only; no provider probes, browser-cookie imports, Keychain reads, or Keychain changes occurred.AI assistance: Codex implemented and tested the change. Independent review findings covering singleton status-item reachability, attached-menu identity, stale Overview state, and architecture-gate anchors were reconciled and fixed; the final exact head has a READY receipt.
Fixes #2988