Skip to content

fix: project-wide audit — dormant bugs, silent failures, missing safety - #90

Merged
jaylann merged 5 commits into
stagefrom
fix/audit-hardening
Jul 10, 2026
Merged

fix: project-wide audit — dormant bugs, silent failures, missing safety#90
jaylann merged 5 commits into
stagefrom
fix/audit-hardening

Conversation

@jaylann

@jaylann jaylann commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Full-project audit (auth/networking, store/notifications, UI, CI/scripts) for dormant bugs, missing safety, and silent-failure paths. No stubs/TODOs/force-unwraps found; ~20 defects fixed below. All fixes have new or extended unit tests where testable.

Auth / networking

  • Device-flow polling no longer aborts sign-in on a transient 5xx/429 mid-authorization; it keeps polling.
  • Host-supplied poll interval is clamped before nanosecond conversion (overflow trap on hostile input).
  • 403 secondary/abuse rate limits are classified as rate-limited (via X-RateLimit-Reset or body hint) even without X-RateLimit-Remaining: 0 / Retry-After, so the store backs off instead of re-polling into a longer lockout.
  • Literal + in saved search queries is percent-encoded (previously reached /search/issues as a space).
  • Reviews-pagination truncation at the page cap is now logged instead of silent.
  • Specific user-facing copy for merge failures 405 (not mergeable), 409 (head changed), 422 — previously a misleading generic "Try again".

Store / notifications

  • addAccount forces a refresh so an in-flight poll built before the account existed can't starve it (previously the new account could stay empty indefinitely with polling off).
  • removeAccount cancels the merge-readiness poll, which could linger holding removed-account state.
  • Batched GraphQL check hydration publishes per account, so a superseded wave can't fire CI banners / advance baselines without the matching display update.
  • When rate-limited, hydration still prunes stale CI dots/gates and unblocks the Actions/Releases skeleton on a first-poll rate limit.
  • All-neutral check runs (everything skipped/cancelled) roll up to neutral instead of success — no green dot or spurious "CI passed" banner.
  • Persisted accounts decode leniently: one corrupt/incompatible element no longer silently signs out all accounts and orphans Keychain tokens.
  • rateLimitedUntil cleared on sign-out; markAllRead surfaces both expired-session and generic failures instead of dropping the latter.

UI / mapping

  • GraphQL hydration normalizes MERGED to the REST representation (closed + merged flag); null check-run status maps to completed so conclusions classify correctly. The two hydration paths now produce identical values.
  • Notification web URLs rewrite only the resource path segment (a repo/owner literally named pulls no longer corrupts the link).
  • Search field focus is deferred until the field is mounted, so opening search actually focuses it.
  • Verified (no change needed): the popover .task refresh re-fires per open — NSPopover sends full appearance transitions on each show.

CI / scripts / hooks

  • require-milestone re-runs when a milestone is added (previously only demilestoned was a trigger, so a blocked PR could never clear).
  • Release cask push no longer swallows real commit failures as "no cask change".
  • cut-release validates marketingVersion is 3-part semver before bumping.
  • commit-msg hook help text corrected to match the enforced rule (history predates a stricter total-length cap, so the text was fixed rather than the rule tightened).

Audited and deliberately unchanged

Defensive-but-unreachable DeviceFlowError arms, Link-header substring parse, 50-item search window (by design), keychain delete-status ignoring (by design), RelativeAge math, badge-count projections, reorder logic, event-monitor lifecycle — all verified correct.

Verification: just check clean, full just test suite green locally.

jaylann added 4 commits July 10, 2026 12:47
- keep polling through transient 5xx/429 during device-flow sign-in
- clamp host-supplied poll interval before nanosecond conversion
- classify 403 secondary/abuse rate limits (reset header or body hint)
- percent-encode literal '+' in search query values
- log silent reviews-pagination truncation
- specific error copy for merge 405/409/422 responses
- force refresh on addAccount so an in-flight poll can't starve the new account
- cancel mergeReadinessTask on removeAccount
- publish batched check folds per account so superseded waves can't strand banners
- prune CI/gate maps and unblock repo-feed skeleton when rate-limited
- all-neutral check runs roll up to neutral, not success
- decode persisted accounts leniently so one bad element can't sign everyone out
- clear rateLimitedUntil on sign-out; surface both errors in markAllRead
- GraphQL MERGED state normalizes to closed + merged flag (matches REST path)
- null GraphQL check-run status maps to completed so conclusions classify
- notification web URL rewrites only the resource path segment
- defer search focus until the field is mounted
- require-milestone re-runs when a milestone is added
- cask push no longer swallows real commit failures
- cut-release validates marketingVersion is 3-part semver before bumping
- commit-msg help text matches the enforced subject-length rule
@jaylann jaylann added this to the v0.6.1 milestone Jul 10, 2026
A wave from the previous poll could re-publish its pre-prune snapshot,
re-adding the stale CI dots/gates the rate-limited skip path just pruned.
@jaylann
jaylann merged commit cc49125 into stage Jul 10, 2026
7 checks passed
@jaylann
jaylann deleted the fix/audit-hardening branch July 10, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant