chore(release): v0.5.4 - #87
Merged
Merged
Conversation
…, coverage (#82) ## Summary Implements the fixes from a full-codebase review (5 parallel review agents + direct verification). Organized into the tiers from the review plan; each tier is independently reviewable in the two commits. **245 tests pass (+23 new), `just check` clean, build + launch smoke test green.** ## Tier 1 — behavioral bugs (`eaad5f5`) - **Rate limiting:** `403/429` + rate-limit headers → `ClientError.rateLimited(until:)`; the poll loop backs off to the reset time (skipping the request-heavy CI/repo-feed hydration while limited) instead of hammering into GitHub's *secondary* limit. Message surfaced to the UI. - **Legacy-token dead-end:** a `401` on legacy-token migration now drops the dead token so the UI falls back to sign-in instead of a permanent first-load skeleton. - **Device-flow `slow_down`:** replace the poll interval instead of accumulating it. - **Notification 401s:** `markRead`/`markAllRead` route to the reconnect prompt, matching the quick-action path. - **`reviews()` pagination:** walk the `Link` header to the last page so a >100-review PR's latest verdict gates Approve/Merge. ## Tier 2 — URL/scheme hardening (`fbe146b`) - Reject non-`https` base URLs in `GitHubClient.makeRequest` (no bearer token over cleartext); validate the Enterprise host field at submit with an inline hint. - New `WebLink` scheme allowlist gates every URL-open site (`NSWorkspace` in `NotificationService`, `openURL` in menu/notification rows) to `http(s)` only — a hostile self-hosted host can't return a `file:`/`javascript:` deep link. ## Tier 3 — robustness - Close the concurrent `force`-refresh race (coalesce onto a peer's fresh run). - Tolerant ISO8601 decoding (fractional seconds) so one field can't fail a whole page decode. - Nil `mergeReadinessTask` on completion/sign-out; reject `.`/`..` traversal in `normalizedSlug`; reset stale search state when the filter bar unmounts; persist the OAuth client ID only on success and clear the pasted PAT on mode-switch/close. - `justfile`: tee the full build log + surface error lines instead of `tail -30`. ## Tier 4 — accessibility - Expose selected/toggle state to VoiceOver on the tab bar, segmented control, and filter chips; hide the decorative `UnseenDot` when seen. ## Tier 5 — coverage + CI - New tests: reconnect (in-place token swap / reentrancy / failure), `addAccount` invalid + duplicate, maintainer-without-push merge, CI-passed banner, https rejection, `WebLink`, `normalizedSlug` traversal. - Cache resolved SwiftPM/Tuist deps in the build & test CI jobs. ## Deferred (intentionally, want your call) - **Full Dynamic Type support** — anchoring every font token to a text style would materially change the dense, fixed-width popover layout; it's a design decision for you rather than a silent refactor. - **Protocol-level `GitHubAPIError`** — the store still matches `GitHubClient.ClientError`. YAGNI until a second `GitHubAPI` backend exists; low value, non-trivial churn. - **Decoding GitHub error message bodies** into `ClientError.http` — would change its `Equatable` shape and ripple through many test matches for marginal benefit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
… republish (#84) (#85) ## What Fixes #84 — the post-approval **merge-readiness poll**'s gate write being clobbered by a concurrent **CI hydration wave**'s batch republish, which briefly hides (or wrongly shows) the Merge button after an approval. ## Root cause `prGates` (the per-PR Approve/Merge action gate) is written by two `@MainActor` flows that interleave across `await`: - the hydration wave (`drainChecks`/`fold`/`publishChecks`), which refetches many PRs and republishes gates in batches; - `refreshPRState`, a single-PR gate refetch driven by the post-approve poll. A wave that read a PR's gate *before* the poll's single-key write would republish the whole map and clobber it. The `checksGeneration` guard only catches cross-generation clobbers; this is a same-generation interleave. #82's `.checksOnly` path widened the window. ## Approach — issue-time write-recency The naive fixes fail: "poll always wins" shows a Merge button that 405s when the base advanced; a **commit-time** clock still loses because the wave's full fetch reads its gate *early* (from the PR detail) but folds *late* (after its slow checks/reviews legs), so a wave that observed older state commits last and wrongly wins. Fix: a monotonic **issue-time** clock (`gateWriteClock` + `prGateSeq`). Both writers take a tick when they *issue* their fetch — the wave stamps a per-full-fetch issue seq up front; the poll ticks before its detail request and defers to any concurrently-issued newer write. `publishChecks` merges gates onto the live map per key, applying a wave gate only when its issue tick beats the live one. So the observation that read the **newer server state wins in both clobber directions**, regardless of commit order. Checks-only carry-overs and failed fetches don't compete (they keep the live gate rather than blanking it). `prGateSeq` is pruned alongside `prGates` at every mutation/reset site. A documented, bounded, self-healing residual remains (a whole wave shares one issue instant, so a mid-wave poll beats a late-queued key) — noted in code; per-key stamping would need a main-actor hop per fetch and isn't worth it for a transient gate flip. ## Tests Three deterministic `GatedGitHubAPI` regression tests pinning the interleavings by issue order: - `testMergePollGateWriteSurvivesChecksOnlyRepublish` — checks-only carry-over can't clobber the poll. - `testLaterIssuedMergePollGateBeatsEarlierIssuedWaveFullFetch` — the primary #84 trigger; **verified it fails under commit-time stamping**. - `testLaterIssuedWaveFetchBeatsEarlierMergePollGate` — a fresher wave fetch beats a staler poll write (guards "poll always wins"). `just check` clean, full suite (269 tests) green. Reviewed over three rounds (multi-agent + single-agent); final review passed with no blocking findings. Closes #84
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
1 participant
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.
Promotion PR for v0.5.4.
Merging this releases it: release.yml tags
v0.5.4, builds + signs + notarizes the DMG, uploads it to the GitHub release, updates the Homebrew cask, and syncsstageback frommain.