Skip to content

fix(plugins): retain selected quotas and support row severity - #4627

Merged
steipete merged 2 commits into
mainfrom
triage/20260921-ui-37
Oct 19, 2026
Merged

steipete merged 2 commits into
mainfrom
triage/20260921-ui-37

Conversation

@steipete

@steipete steipete commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Merged Current selection icons now retain the selected user plugin and show its quota, including an unavailable state before the first snapshot. The selector previously discarded plugin IDs through the first-party conversion. This uses the existing switcher quota policy and shares the built-in icon rendering paths while preserving their animation and cache behavior.

Plugin detail rows also accept optional severity: "normal" | "warning" | "critical". This reuses system colors, native progress bars, and existing translated warning labels; plugins compute their thresholds, and progress remains consumed usage. Absent severity keeps the existing appearance. The mapper validates the field on both engines, and Codable, localization, and privacy transformations preserve it.

Both changes are documented in docs/plugins.md and the 0.77.1 Unreleased changelog. Thanks @soimy for reporting the selection bug and @DamonYoungWork for the severity request.

Verification

All Swift commands ran with the repository test environment scrubbed, under the host-wide build lock, without real-account probes or relaunching the app.

  • Regression on the original main baseline: swift test --build-system native --jobs 2 -Xswiftc -gnone --filter UserPluginQuotaPresentationTests — 6 tests, 5 passed and the new plugin-selection regression failed. The extraction used to expose the selector preserved the original first-party filter.
  • Final focused command and results: swift test --build-system native --jobs 2 -Xswiftc -gnone --filter 'UserPluginQuotaPresentationTests|UserProviderPluginTests|UnifiedIconSourceSettingsTests|FrontmostProviderMonitorTests|MergedIconPresentationTests|StatusItemAnimationSignatureTests|StatusItemBalanceDisplayTests|MenuBarCountdownRefreshTests|ProviderDetailSeverityTests|ProviderDetailSectionsContentTests|ProviderDetailSectionTests|WidgetDetailSnapshotTests|ProviderArchitectureGatekeeperTests' — Test run with 205 tests in 13 suites passed after 53.351 seconds.
  • make check: passed; Done linting! Found 0 violations, 0 serious in 3052 files.
  • Independent Codex autoreview at P2: scoped-clean, no actionable P0–P2 findings.
  • Catalog precheck: no output. git diff --check: clean.
  • Production delta: 10 files changed, 166 insertions(+), 254 deletions(-), net −88. Tests: 6 files changed, 122 insertions(+), 5 deletions(-).

Synthetic UI proof

The images below are native offscreen renders using fictional data. No proof artifacts or screenshot-only tests are committed.

Merged icon before and after, synthetic plugin quota

Detail rows before and after, synthetic severity colors

CI status is unverified: a GitHub REST rate limit was encountered during this lane, so status polling was stopped.

Fixes #4604
Fixes #4602
Refs #4591

Keep user plugin IDs through merged Current selection rendering and observation, sharing the existing first-party renderer paths. Add optional per-row severity using native progress colors and existing warning labels, preserving it through decoding, localization, and privacy filtering.
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-19T06:23:46.117108Z 59cb258 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge.

What this changes

This PR retains selected plugin quotas in merged icons and adds optional severity styling to plugin detail rows.

Example: Select Fixture Meter with 25% remaining while Codex has 80% remaining.

  • Before: The merged icon continues showing Codex’s 80%.
  • After: The merged icon shows Fixture Meter’s 25%.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation is focused and source-consistent, with meaningful regression coverage; supplied visual evidence exercises rendering rather than the shipped selection flow.
Proof confidence 🦐 gold shrimp (3/6) The maintainer-authored PR is outside the ordinary contributor proof gate; inspected screenshots demonstrate synthetic offscreen rendering, while source and compatibility coverage establish an additive optional snapshot field requiring no migration.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Selecting a plugin leaves the merged icon showing another provider’s quota, and pooled-account detail rows lack independent warning styling.
Reason: The concrete reports establish user value, and the owner-authored PR settles the bounded API and icon-scope choices.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep open: this maintainer-authored PR addresses two concrete plugin presentation gaps absent from current main; no actionable code defect was found, but GitHub reports a merge conflict.

Priority: P2
Reviewed head: 95d246f0e460064afa4568ff199feb7f2cec84bb

Before merge

  • Complete next step - Resolve GitHub’s reported merge conflict while preserving the reviewed plugin behavior and current-main changes.

Findings

None.

Tests

  • Missing end-to-end proof: The supplied renders do not exercise selecting and dismissing a plugin menu in the shipped app or severity rows in dark and highlighted presentation.
Agent review details

How this fits together

Plugin snapshots enter the shared mapper and detail model, then feed menu-card rows and the selected merged status icon.

flowchart LR
  A[Plugin engines] --> B[Snapshot mapper]
  B --> C[Usage snapshots]
  C --> D[Detail row renderer]
  C --> E[Merged icon resolver]
  F[Switcher selection] --> E
  E --> G[Status item]
Loading

Technical review

Best possible solution:

Land the scoped instance-ID selection fix and optional host-themed severity through the existing rendering owners.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically discards plugin selections before merged-icon rendering; the PR reports a failing baseline regression, but this read-only review ran no target code.

Is this the best way to solve the issue?

The implementation reuses the switcher quota policy and built-in renderer, and adds semantic severity without inferring thresholds or changing progress meaning.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7e31e74ea662.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Raw commit 00ef23c adds Sources/CodexBarCore/Plugins/ProviderPluginSnapshotMapper.swift:72 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 00ef23cdc9e4; files: Sources/CodexBarCore/Plugins/ProviderPluginSnapshotMapper.swift)
  • Gamitha Samarasingha: Raw commit 4e29944 adds Sources/CodexBar/UnifiedIconProviderResolver.swift:4 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 4e29944ee65e; files: Sources/CodexBar/UnifiedIconProviderResolver.swift)
  • DanyITnerd: Raw commit 2edc439 adds Sources/CodexBar/ProviderDetailSectionsContent.swift:53 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 2edc439c21bb; files: Sources/CodexBar/ProviderDetailSectionsContent.swift)

Review metrics

Metric Value Why it matters
Production and test delta Production +166/−254; tests +122/−5 Sharing the existing built-in renderer removes duplicated production code; added tests cover engine validation, transformations, selection, and visible colors.

Labels

Label changes:

  • add P2: The plugin icon mismatch affects usage at a glance, while the selected menu card remains usable.
  • add merge-risk: 🚨 compatibility: The PR extends the public plugin row API; the owner-authored direction explicitly preserves omitted-field appearance and consumed-progress semantics.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P2: The plugin icon mismatch affects usage at a glance, while the selected menu card remains usable.
  • merge-risk: 🚨 compatibility: The PR extends the public plugin row API; the owner-authored direction explicitly preserves omitted-field appearance and consumed-progress semantics.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Capture the freshly built app retaining a synthetic plugin quota after menu dismissal, alongside severity rows in dark and highlighted presentation.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 19, 2026, 12:16 AM ET / 04:16 UTC.

steipete added a commit that referenced this pull request Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

1 participant