Open the usage share preview from desktop widgets - #3678
Chipagosfinest wants to merge 41 commits into
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. |
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. |
![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: 45f3f6dc25
ℹ️ 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 let balance = extraUsageBalanceLine(for: entry) { | ||
| balance | ||
| } | ||
| WidgetShareOverviewFooter(launchesSharePreview: false) |
There was a problem hiding this comment.
When the small Switcher displays a normal Codex entry with two quota rows plus the code-review row, this footer pushes the nested content beyond the tile height after the provider-chip row and WidgetKit content margins are included, so the final usage row or the new label is compressed or clipped. The native proof only checks the outer hosting-view bounds without real WidgetKit margins, so it cannot detect this; omit the redundant footer for small tiles—which already use widgetURL—or reduce the small row budget, then verify the actual desktop widget.
AGENTS.md reference: AGENTS.md:L41-L41
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
P3
|
Codex review: blocked before merge. Reviewed September 17, 2026, 1:25 AM ET / 05:25 UTC (Revision 19). ClawSweeper reviewWhat this changesAdds desktop-widget links to the local usage-share preview, queued cold-launch delivery, and the prerequisite menu-sharing changes with tests and native screenshots. Merge readiness⛔ Blocked before merge - 3 items remain Keep open: current main lacks this widget entry point, and substantial native evidence supports retaining the contribution. No blocking code defect was found. Priority: P3 Review scores
Verification
How this fits togetherCodexBar widgets display usage snapshots published by the app. The new link asks the app to rebuild its selected usage overview and present a local preview; copying or saving still requires a separate action. flowchart LR
A[Desktop widget click] --> B[Validate data-free URL]
B --> C[Queue until app is ready]
C --> D[Build selected usage overview]
D --> E[Local share preview]
E --> F[User chooses copy or save]
Decision needed
Why: The evidence proves working fresh-install interactions and a macOS 15 replacement, but cannot decide the acceptable release boundary for macOS 26 without maintainer intent. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep one shared preview and filtered payload path, with an explicitly accepted or resolved macOS 26 retained-widget adoption boundary. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this adds a widget entry point. Supplied native runs demonstrate actual widget clicks opening populated previews. Is this the best way to solve the issue? Yes: reusing the existing filtered overview and preview is a narrow implementation, with a data-free route and separate user actions for export. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 1a57ef11cbe6. 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 (18 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
proof: 📸 screenshot
…ix/widget-share-stats
…ix/widget-share-stats
…ix/widget-share-stats
clawsweeper
Bot
removed
the
proof: 📸 screenshot
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
Localization catalogs conflicted because both sides appended new keys in the same region; all 23 keep both sets. The gatekeeper allowlist entry main added for PreferencesSpendDashboardPane.swift:519 is dropped: that construct moved into ShareStatsPayload.swift on this branch and already has its own entry, whose anchor shifts 361 -> 372 under the merged file.
…r task A failed image copy left "Could not copy image" on the button and on its accessibility label until the window was closed and reopened: only the success path scheduled a reset, and the preceding cancel() had killed any pending one. Both outcomes now share makeCopyFeedbackResetTask(). The presentation task's defer cleared whichever handle was stored rather than its own, so a task resuming from its 50ms sleep after being superseded nil'd its successor's handle and let a second presentation start concurrently. A generation token means a task only clears the handle it installed.
…andoff parse() short-circuited on an exact whole-string compare, so every component check below it was unreachable and the 13-case rejection test proved nothing about them. The comparisons run now, which also makes the case-insensitive scheme and host tolerance real instead of advertised. The query is pinned as raw percent-encoded text because URLComponents decodes queryItems, and "version=%31" would otherwise read as "version=1". A queued route had no expiry, so one that failed to deliver was retained indefinitely and could pop an unrequested share window much later. It now expires after 60 seconds: a queued route answers a click, and past that window it no longer represents the user's intent.
… value StatusItemController was already at exactly the 800-line type_body_length ceiling, so adding the generation counter as a second stored property tipped it to 801. Both now live in an OverviewSharePresentation value declared outside the class body, which restores the count to 800 and keeps the two fields together where their invariant lives.
|
ClawSweeper's remaining blocker on this one is a disposition only you can give, so here it is as a single question with the evidence attached. Question: existing macOS 26 widgets may keep their previous view after the app is replaced, and so may not expose the new Share action until the widget is removed and re-added. Is that acceptable to ship? What is demonstrated: the macOS 15 retained-widget upgrade preserves existing widget data and behavior, and installed small and medium Switcher interactions exercise the production URL, the app handoff, and the overview preview on both warm and cold launch. What is not: prompt adoption of the new action by an already-installed macOS 26 widget across an app update. For what it's worth, retained widgets not picking up changes until they are re-added is documented WidgetKit behavior rather than something specific to this change — Apple's own guidance for a changed widget configuration is to instruct people to remove and re-add it. Nothing here regresses an existing widget; the action is simply absent until the widget refreshes. Happy to go either way:
Also worth noting, since the diff looks big: 13 of the 74 files are source. The rest are 23 localization catalogs, 33 proof artifacts, and 5 test files — and the source count drops further once #3677 lands, since this branch currently carries it. |
1 participant
Desktop widgets currently have no visible path to the local Usage & Spend share image. This adds Share overview with the standard Share symbol to Usage, Compact, Switcher, and History widgets. Small widgets use the tile URL and an existing header/chip row; larger widgets use an explicit link. Provider-switch buttons remain available, with single-line compact labels and full accessibility names.
The data-free
codexbar://share-stats?version=1route opens the existing preview for the selected overview scope. Cold-launch delivery queues until the status controller is ready and coalesces duplicate requests. The URL carries no account, provider, spend, usage, callback, or file data. Unsupported parameters and destinations are rejected.Depends on #3677. This is a separate stacked feature PR because GitHub cannot use a contributor fork branch as an upstream PR base. The prerequisite ancestry is tracked, so the concern-scoped comparison contains only widget changes. Remove the prerequisite diff after #3677 merges.
Validation:
e880687f9: three route/handoff tests and one native rendering XCTest, zero failures.d223726ffd8d9d60c3eab54af55e730b7291669c: debug app contains the Xcode-built WidgetKit extension; bundle IDs, URL scheme, extension point, and strict deep signatures were verified. This run did not launch or install either bundle. Current-head full CI remains the merge gate.d223726ffd8d9d60c3eab54af55e730b7291669c; subsequent PR commits add evidence only.All nine native captures and proof boundaries.
Small Usage/Compact headers use the relative-timestamp space for Share. Medium/large widgets retain labelled links. Burn Down remains outside this aggregate overview entry point.
References: Apple widget links, widget interactivity.