Skip to content

fix(native-tray): guard oversized quota percentages - #6099

Merged
lidge-jun merged 1 commit into
devfrom
codex/t4-bug-hardening-native-quota
Sep 27, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/t4-bug-hardening-native-quota

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

The macOS menu bar companion could crash while formatting a quota bar. NativeTrayFormat.number accepts any finite, nonnegative Double, and percentText/percentDescription then converted it with Int(percent.rounded(.down)). A finite value at or above Double(Int.max) (for example 1e20 from a malformed quota payload) trapped that conversion. Both formatters now return their existing unavailable strings ("—" visibly, "Unavailable" for VoiceOver) before converting, and ordinary flooring (89.9% reads "89%") is unchanged. number itself is untouched, because other consumers apply their own clamping (the bar fill is capped at 100 in Models.swift).

This carries #6082 by @luvs01, with the same two-line guard and additional boundary assertions at Double(Int.max) and its next-lower representable value.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Verification

  • swift run --package-path app NativeTrayTests: PASS, 59 assertions (53 on dev before this change, plus 6 new).
  • Red-green: with the new assertions and the old formatter, the same executable trapped on 1e20 (exit 133). With the guard it passes.
  • git diff --check clean. Swift files are outside the file-size ratchet, and no TypeScript changed, so typecheck/test:changed have nothing to exercise here. The local full Bun suite was not run because this is Swift-only and seven release lanes share the machine. The macOS widget CI job is the hosted proof.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (structure/companion.md was reviewed and does not describe which raw values render, so it needed no change.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Input-validation hardening only; no auth, credential, or account-selection behavior changes.)

Summary by CodeRabbit

  • Bug Fixes
    • Percentage values beyond the safely supported range now display the existing unavailable message in both numeric and spoken formats, instead of producing an invalid result.
    • Values within the supported range continue to display as before, including the largest value below the limit.

A finite percentage at or above Double(Int.max) passed NativeTrayFormat.number
and trapped when percentText or percentDescription converted it to Int. Both
formatters now return their unavailable strings before the conversion, and
the NativeTray test executable covers 1e20 and the Int upper boundary.

Carries #6082.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 27, 2026 15:44
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 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 ✅ Completed 2026-09-27T15:46:08.806988Z 8812bb5 PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a654e973-3e26-4fce-b73c-888303146a96

📥 Commits

Reviewing files that changed from the base of the PR and between 24b2f39 and 8812bb5.

📒 Files selected for processing (2)
  • app/Sources/NativeTray/Models.swift
  • app/Sources/NativeTrayTests/main.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

Percentage formatting now returns unavailable strings for nonnegative values at or above Double(Int.max). Tests cover oversized values, the boundary, and the largest representable value below it.

Changes

Percentage formatting bounds

Layer / File(s) Summary
Reject out-of-range percentages
app/Sources/NativeTray/Models.swift, app/Sources/NativeTrayTests/main.swift
At lines 163–168, percentText and percentDescription reject values at or above Double(Int.max). Tests at lines 102–108 verify unavailable output for 1e20 and the boundary, and visible output for Double(Int.max).nextDown.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 8812b

The menu-bar formatters now return their unavailable text instead of trapping on oversized finite percentages, while preserving ordinary formatting. No material merge risk is evident; merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: guarding NativeTray quota percentage formatters against oversized values that could cause an Int conversion trap.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 27, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 44 / 80

이 PR은 맥 상단 막대(트레이)가 사용량 퍼센트를 글자로 만들 때, 너무 큰 숫자 때문에 프로그램이 꺼지지 않게 해요. 베이스는 dev예요. #6082를 이어서 가져온 것이고, 가드 두 줄은 같고 경계 테스트만 더 있어요.

사용량 숫자는 소수점을 버린 뒤 정수로 바꿔 "89%"처럼 적어요. 1e20처럼 정수에 안 들어가는 큰 값이 오면, 그 변환에서 트레이 프로세스가 죽어요. 이제는 화면에 "—"를 쓰고, 읽어 주는 말은 "Unavailable"이에요. Double(Int.max)도 같은 빈 표시예요. 그 바로 아래 수(nextDown)는 그대로 숫자로 보여요. 89.9처럼 평소 숫자는 그대로예요. types.ts와 config.ts를 나누는 일과는 겹치지 않아요.

라인 - app/Sources/NativeTray/Models.swift 126행. Window.value는 number만 거쳐요. number는 유한하고 0 이상이면 1e20도 통과시켜요.

라인 - app/Sources/NativeTray/ProviderMark.swift 67행. 막대 색은 severity예요. 1e20은 90보다 커서 빨간색이에요. 80행의 fill은 100을 넘지 못하게 잘라서 막대가 끝까지 차요. 163행 글자는 "—"인데, 막대는 가득 찬 위험 상태예요. 87행 읽어 주는 말은 "Unavailable"이에요.

라인 - app/Sources/NativeTrayTests/main.swift 102행. 1e20, Double(Int.max), 그 바로 아래 수의 글자만 확인해요. 막대 색과 Window.value가 비는지는 확인하지 않아요.

메인테이너의 판단이 필요한 지점

정수로 못 바꾸는 퍼센트를 창 전체에서 "없는 값"으로 볼지예요. 글자만 숨기면 막대는 가득 찬 빨간색으로 남아요.

#6082와 이 PR은 고친 코드가 같아요. 어느 쪽을 남길지 정해 주세요.

너의 추천

창의 값에서 거르세요. Window.value가 이 한계 이상이면 nil로 두면, 색 막대는 그리지 않고 글자와 읽어 주는 말도 없는 값이 돼요. 163행과 168행의 가드는 그대로 두세요. 이 가드를 number에는 넣지 마세요. number는 토큰 수에도 쓰여서, 큰 토큰 수까지 대시가 되면 안 돼요. 테스트에 큰 퍼센트의 Window.value가 비는지 한 줄 더하면 좋아요.

이 PR을 남기고 #6082는 닫으세요. 경계 테스트는 이쪽에만 있어요. 막대까지 맞춘 다음 머지하세요.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md, dev-only exception), recorded before merge. This is not an approval or an independent review.

  • Head 8812bb5dfcfd6f933959e24abd6a1961515f8960 is based on current origin/dev 24b2f39b77.
  • Exact-head Cross-platform CI run 36330651498 (pull_request): every executed job succeeded, including test 1-4/4, gates, macos 1/2 and 2/2, macos widget + bundle, desktop shell, docker smoke and the keyrings. Skipped jobs are path-filtered, and the cancelled label/enforce-target entries are superseded runs whose reruns passed on this head.
  • scripts/ci/assert-mergeable-review.sh --maintainer-integration 6099: OK for this head.
  • Codex and CodeRabbit left no findings. Local proof: swift run --package-path app NativeTrayTests 59 assertions pass, red on the old formatter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant