fix: align clipboard-first runtime and DMG notarization#3
Conversation
📝 WalkthroughWalkthroughtransScan's text detection shifts from Accessibility API primary with clipboard fallback to clipboard-only polling via NSPasteboard. Removes PermissionManager, simplifies TextDetector and event model, eliminates permission-gated UI and translation flow, updates all documentation and tests, and adds DMG notarization infrastructure for official releases. ChangesClipboard-first text detection refactor
DMG signing and notarization automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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: 8603aed590
ℹ️ 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".
| .font(.system(size: 13, weight: .medium)) | ||
|
|
||
| Text(permissionDescription) | ||
| Text("텍스트를 복사한 뒤 오버레이를 열면 최근 클립보드 내용을 번역합니다.") |
There was a problem hiding this comment.
This copy now tells users that copying text and then opening the overlay will translate the recent clipboard contents, but the runtime path only subscribes to future pasteboard changes: TextDetector.startMonitoring() seeds lastClipboardChangeCount with the current NSPasteboard.general.changeCount, and TranslationViewModel.onAppear() never calls latestClipboardText(). In the advertised flow, the overlay remains idle until the user copies again, so either translate the current clipboard snapshot when opening or avoid promising copy-before-open behavior.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
transScanTests/TranslationViewModelTests.swift (1)
440-449: ⚡ Quick winAdd the paired
onDisappear()lifecycle assertion test.You now track
stopMonitoringCallCount, but onlyonAppear()is asserted. Add one focused test foronDisappear()to lock the lifecycle contract.Proposed test addition
func testStartsClipboardMonitoringOnAppear() { let vm = makeVM() vm.onAppear() XCTAssertEqual(vm.state, .idle) XCTAssertEqual(mockDetector.startMonitoringCallCount, 1) } + +func testStopsClipboardMonitoringOnDisappear() { + let vm = makeVM() + vm.onAppear() + + vm.onDisappear() + + XCTAssertEqual(mockDetector.stopMonitoringCallCount, 1) +}As per coding guidelines: Add XCTest coverage for new app behavior in
transScanTests/and prefer focused tests around view model state transitions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transScanTests/TranslationViewModelTests.swift` around lines 440 - 449, Add a focused XCTest that verifies the view model calls stop monitoring on lifecycle end: create the VM via makeVM(), invoke vm.onAppear() (to mirror the start path), then call vm.onDisappear(), and assert mockDetector.stopMonitoringCallCount == 1 and vm.state == .idle (and optionally that startMonitoringCallCount == 1 to show the paired lifecycle). Name the test e.g. testStopsClipboardMonitoringOnDisappear and reference vm.onDisappear() and mockDetector.stopMonitoringCallCount in the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@roadmap.progress.json`:
- Line 5: The JSON contains a machine-identifying value for the key
"deviceLabel" (currently "bishoe01s-MacBook-Air-4.local"); replace that value
with a neutral identifier (e.g. "local-device" or "unknown-device") or remove
the "deviceLabel" entry entirely to avoid leaking hostname information, then
amend the commit so the sanitized value is stored; ensure any local
Xcode/machine state files that may regenerate this field are excluded from
version control going forward.
In `@scripts/notarize_dmg.sh`:
- Around line 12-16: The current fallback that sets DMG_PATH uses lexicographic
sort which can pick the wrong file; change the fallback to select the most
recently modified DMG by mtime instead. In the branch that sets DMG_PATH when no
argument is given (the find ... -name 'transScan-*.dmg' | sort | tail -n 1
pipeline), replace the lexicographic sort with a pipeline that sorts by
modification time (e.g., use find to list matches and pipe to ls -t / sort -n on
file mtimes or use find -printf '%T@ %p\n' | sort -n | tail -n1) so the code
that assigns DMG_PATH picks the newest file by modification time.
- Around line 30-43: The notarytool invocation currently discards all output and
treats any failure as a missing profile; change the logic in
scripts/notarize_dmg.sh around the xcrun notarytool history --keychain-profile
"$PROFILE" call to capture stderr/stdout into a variable, test that output (or
the command exit status) for the specific "No Keychain password item found for
profile" message (or exact error code/message returned by notarytool) and only
print the existing "No notarytool Keychain profile found: $PROFILE" help text in
that case; for any other failure, echo the captured notarytool error to stderr
and exit with the original exit code so callers see the real cause (keep the
existing exit 1 for the profile-missing branch and propagate other non-zero
statuses instead).
In `@transScan/TextDetector.swift`:
- Around line 64-68: The clipboard polling Timer created in
startClipboardPolling uses Timer.scheduledTimer(...) which registers in the
default run loop mode and can pause during UI interactions; change
startClipboardPolling to create the Timer (e.g.,
Timer(timeInterval:repeats:block:) or scheduledTimer then remove automatic
scheduling) and explicitly add it to RunLoop.main in .common modes
(RunLoop.main.add(clipboardTimer!, forMode: .common)) so clipboardTimer runs in
RunLoop.Mode.common and won't be paused during menu/drag interactions.
---
Nitpick comments:
In `@transScanTests/TranslationViewModelTests.swift`:
- Around line 440-449: Add a focused XCTest that verifies the view model calls
stop monitoring on lifecycle end: create the VM via makeVM(), invoke
vm.onAppear() (to mirror the start path), then call vm.onDisappear(), and assert
mockDetector.stopMonitoringCallCount == 1 and vm.state == .idle (and optionally
that startMonitoringCallCount == 1 to show the paired lifecycle). Name the test
e.g. testStopsClipboardMonitoringOnDisappear and reference vm.onDisappear() and
mockDetector.stopMonitoringCallCount in the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 029ee371-e709-4315-a209-4c0b712e8859
📒 Files selected for processing (23)
AGENTS.mdCLAUDE.mdREADME.ko.mdREADME.mddocs/ADR.mddocs/PRD.mddocs/UI_GUIDE.mddocs/session-logs/2026-05-28.mdroadmap.mdroadmap.progress.jsonscripts/create_dmg.shscripts/notarize_dmg.shtransScan/AppDelegate.swifttransScan/MenuBarContentView.swifttransScan/OverlayContentView.swifttransScan/PermissionBannerView.swifttransScan/PermissionManager.swifttransScan/SettingsView.swifttransScan/TextDetector.swifttransScan/TranslationResultView.swifttransScan/TranslationViewModel.swifttransScanTests/TextDetectorTests.swifttransScanTests/TranslationViewModelTests.swift
💤 Files with no reviewable changes (5)
- transScan/AppDelegate.swift
- transScan/TranslationResultView.swift
- transScan/PermissionManager.swift
- transScan/PermissionBannerView.swift
- transScan/MenuBarContentView.swift
| "updatedAt": "2026-05-08 23:36:37 JST (+0900)", | ||
| "deviceLabel": "bishoe01s-MacBook-Air-2.local", | ||
| "updatedAt": "2026-05-28 23:47:34 JST (+0900)", | ||
| "deviceLabel": "bishoe01s-MacBook-Air-4.local", |
There was a problem hiding this comment.
Remove machine-identifying hostname from tracked metadata.
Line 5 exposes a local host identifier (bishoe01s-MacBook-Air-4.local) in source control. Replace it with a neutral label to avoid leaking machine-specific information.
Based on learnings: Do not commit local Xcode or machine state files, including machine-specific paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@roadmap.progress.json` at line 5, The JSON contains a machine-identifying
value for the key "deviceLabel" (currently "bishoe01s-MacBook-Air-4.local");
replace that value with a neutral identifier (e.g. "local-device" or
"unknown-device") or remove the "deviceLabel" entry entirely to avoid leaking
hostname information, then amend the commit so the sanitized value is stored;
ensure any local Xcode/machine state files that may regenerate this field are
excluded from version control going forward.
| if [[ $# -eq 1 ]]; then | ||
| DMG_PATH="$1" | ||
| else | ||
| DMG_PATH="$(find "$ROOT_DIR/build" -maxdepth 1 -type f -name 'transScan-*.dmg' -print 2>/dev/null | sort | tail -n 1 || true)" | ||
| fi |
There was a problem hiding this comment.
Pick the newest DMG by modification time, not filename sort.
Line 15 uses lexicographic sorting, so transScan-1.10.dmg sorts before transScan-1.9.dmg. That can notarize an older artifact by default instead of the most recently built one, which breaks the “latest build” behavior described for this flow in roadmap.progress.json:12-13.
💡 Proposed fix
- DMG_PATH="$(find "$ROOT_DIR/build" -maxdepth 1 -type f -name 'transScan-*.dmg' -print 2>/dev/null | sort | tail -n 1 || true)"
+ DMG_PATH="$(
+ find "$ROOT_DIR/build" -maxdepth 1 -type f -name 'transScan-*.dmg' -exec stat -f '%m %N' {} + 2>/dev/null \
+ | sort -n \
+ | tail -n 1 \
+ | cut -d' ' -f2-
+ )"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/notarize_dmg.sh` around lines 12 - 16, The current fallback that sets
DMG_PATH uses lexicographic sort which can pick the wrong file; change the
fallback to select the most recently modified DMG by mtime instead. In the
branch that sets DMG_PATH when no argument is given (the find ... -name
'transScan-*.dmg' | sort | tail -n 1 pipeline), replace the lexicographic sort
with a pipeline that sorts by modification time (e.g., use find to list matches
and pipe to ls -t / sort -n on file mtimes or use find -printf '%T@ %p\n' | sort
-n | tail -n1) so the code that assigns DMG_PATH picks the newest file by
modification time.
| if ! xcrun notarytool history --keychain-profile "$PROFILE" >/dev/null 2>&1; then | ||
| cat >&2 <<MESSAGE | ||
| No notarytool Keychain profile found: $PROFILE | ||
|
|
||
| Create it once with: | ||
|
|
||
| xcrun notarytool store-credentials "$PROFILE" \\ | ||
| --apple-id "YOUR_APPLE_ID_EMAIL" \\ | ||
| --team-id 9A96KXU385 \\ | ||
| --password "APP_SPECIFIC_PASSWORD" | ||
|
|
||
| Use an Apple ID app-specific password, not your normal Apple ID password. | ||
| MESSAGE | ||
| exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Apple xcrun notarytool, what failure modes can notarytool history --keychain-profile return, and is it a reliable way to test whether a keychain profile exists locally?
💡 Result:
What can fail with xcrun notarytool history --keychain-profile <profile>?
history needs two things to succeed: (a) it must be able to read credentials from Keychain for <profile>, and (b) it must be able to authenticate to Apple’s notary service and fetch history.
Common failure modes include:
-
Keychain item for the profile can’t be found/read
Typically surfaces as:Error: No Keychain password item found for profile: <profile>(often reported with exit code 69). This can happen when the profile truly doesn’t exist, or when it exists but isn’t readable (wrong keychain, access issues, corruption, etc.). [2], [3], [4] -
Keychain is locked / requires unlock UI
The man page notes that if the specified keychain is locked,notarytoolwill prompt to unlock it—this can break non-interactive/CI usage and lead to failure. [1] -
No usable credentials were ultimately provided
Ifnotarytoolcan’t resolve usable credentials from the profile (or otherwise), it can fail with errors likeError: Must provide credentials.[5] -
Credentials resolve but authentication is rejected (server-side auth failure)
For example, the man page explicitly mentions cases that yield a401 Unauthorizedresponse (e.g., wrong issuer usage for certain API keys). Any invalid/revoked credential could similarly causehistoryto fail even though the profile exists locally. [1]
Is history --keychain-profile <profile> a reliable “does this profile exist locally?” test?
Not fully.
- A failure does not reliably mean “profile doesn’t exist”, because it can also fail due to locked keychain, unreadable keychain item, bad/expired credentials, or server-side auth issues. [1], [4]
- A success does strongly imply the profile exists and is usable (it was read from Keychain and authenticated).
If your goal is specifically “does the keychain item exist locally?”, prefer directly checking Keychain for the saved-credentials item. Apple’s stored profile naming convention shows up in Keychain with an account attribute like com.apple.gke.notary.tool.saved-creds.<ProfileName>. [6]
Don’t collapse every xcrun notarytool history --keychain-profile failure into “profile missing” (scripts/notarize_dmg.sh, lines 30-43).
- The script discards all
notarytooloutput (>/dev/null 2>&1) and treats any non-zero exit as “No notarytool Keychain profile found”. notarytool history --keychain-profile ...can fail for reasons unrelated to a missing Keychain item (e.g., locked keychain, unreadable/corrupt credential item, missing/invalid credentials, or server-side auth/API errors such as401 Unauthorized).- Show the real
notarytoolerror (stderr) and only display the “profile missing” message for the specific “No Keychain password item found for profile …” failure (or matching exit/error code); otherwise surface the underlying cause.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/notarize_dmg.sh` around lines 30 - 43, The notarytool invocation
currently discards all output and treats any failure as a missing profile;
change the logic in scripts/notarize_dmg.sh around the xcrun notarytool history
--keychain-profile "$PROFILE" call to capture stderr/stdout into a variable,
test that output (or the command exit status) for the specific "No Keychain
password item found for profile" message (or exact error code/message returned
by notarytool) and only print the existing "No notarytool Keychain profile
found: $PROFILE" help text in that case; for any other failure, echo the
captured notarytool error to stderr and exit with the original exit code so
callers see the real cause (keep the existing exit 1 for the profile-missing
branch and propagate other non-zero statuses instead).
| private func startClipboardPolling() { | ||
| clipboardTimer?.invalidate() | ||
| clipboardTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: true) { [weak self] _ in | ||
| self?.checkClipboard() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Timer.scheduledTimer(withTimeInterval:repeats:block:)on macOS add the timer only to the default run loop mode, and should a polling timer use.common modes to keep firing during menu tracking or drag interactions?
💡 Result:
Yes. Apple documents that Timer.scheduledTimer(withTimeInterval:repeats:block:) “schedules it on the current run loop in the default mode”, i.e. only in the default run-loop mode rather than the menu/drag tracking modes.[1][2] For a polling/repeating timer that you need to keep firing during menu tracking or drag interactions, you generally should add the timer to the run loop’s “common modes” (e.g. RunLoop.Mode.common) rather than only the default mode, because common modes share sources/timers across the default/modal/event-tracking modes. Apple’s run-loop documentation explains that timers are associated with specific modes and won’t invoke their handlers unless the run loop is running in a mode that the timer is registered for.[3] Core Foundation further explains that kCFRunLoopCommonModes is a pseudo-mode used so objects added there are monitored by all run loop modes that are in the common set (including the default mode, and typically the tracking modes once they’re part of the common set).[4] Practical Swift pattern (add to common modes instead of relying on the convenience scheduler): let timer = Timer.scheduledTimer(withTimeInterval: interval, repeats: true) { _ in // polling work } RunLoop.current.add(timer, forMode:.common) Or create the timer without scheduling and add it manually to .common (so it’s not limited to the default mode only).[2] In short: the scheduledTimer(withTimeInterval:repeats:block:) convenience API is default-mode-only; use .common for repeating polling timers when you must keep firing during UI tracking/drag/menu interactions.
Citations:
- 1: https://developer.apple.com/documentation/foundation/timer/scheduledtimer(withtimeinterval:repeats:block:)
- 2: https://developer.apple.com/documentation/foundation/timer
- 3: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html
- 4: https://leopard-adc.pepas.com/documentation/CoreFoundation/Reference/CFRunLoopRef/Reference/reference.html
Run clipboard polling timer in .common run loop modes.
Timer.scheduledTimer(withTimeInterval:repeats:block:) schedules the timer on the run loop in the default mode, which can pause during menu tracking/drag interactions; this can delay the clipboard-only ingestion path.
♻️ Proposed fix
private func startClipboardPolling() {
clipboardTimer?.invalidate()
- clipboardTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: true) { [weak self] _ in
- self?.checkClipboard()
- }
+ let timer = Timer(timeInterval: 0.5, repeats: true) { [weak self] _ in
+ self?.checkClipboard()
+ }
+ RunLoop.main.add(timer, forMode: .common)
+ clipboardTimer = timer
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private func startClipboardPolling() { | |
| clipboardTimer?.invalidate() | |
| clipboardTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: true) { [weak self] _ in | |
| self?.checkClipboard() | |
| } | |
| private func startClipboardPolling() { | |
| clipboardTimer?.invalidate() | |
| let timer = Timer(timeInterval: 0.5, repeats: true) { [weak self] _ in | |
| self?.checkClipboard() | |
| } | |
| RunLoop.main.add(timer, forMode: .common) | |
| clipboardTimer = timer | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@transScan/TextDetector.swift` around lines 64 - 68, The clipboard polling
Timer created in startClipboardPolling uses Timer.scheduledTimer(...) which
registers in the default run loop mode and can pause during UI interactions;
change startClipboardPolling to create the Timer (e.g.,
Timer(timeInterval:repeats:block:) or scheduledTimer then remove automatic
scheduling) and explicitly add it to RunLoop.main in .common modes
(RunLoop.main.add(clipboardTimer!, forMode: .common)) so clipboardTimer runs in
RunLoop.Mode.common and won't be paused during menu/drag interactions.
1 participant
Summary
Motivation
The repository documentation now describes clipboard polling as the stable MVP input path, but the app still blocked the overlay on Accessibility permission and kept unused selected-text monitoring code. The distribution scripts also needed a safer signed DMG/notarization handoff before web release.
Changes
transScan/TextDetector.swiftNSPasteboardpolling and remove AX observer state.transScan/TranslationViewModel.swifttransScan/SettingsView.swift,MenuBarContentView.swift,TranslationResultView.swifttransScan/PermissionManager.swift,PermissionBannerView.swiftscripts/create_dmg.sh,scripts/notarize_dmg.shREADME.md,README.ko.md,docs/PRD.md,docs/ADR.md,roadmap*transScanTests/*Test Plan
xcodebuild -project transScan.xcodeproj -scheme transScan test -quietxcodebuild -project transScan.xcodeproj -scheme transScan build -quietbash -n scripts/create_dmg.sh scripts/notarize_dmg.shnode -e "JSON.parse(require('fs').readFileSync('roadmap.progress.json','utf8')); console.log('roadmap.progress.json OK')"git diff HEAD --checkspctlverification not run yet.Summary by CodeRabbit
Release Notes
Documentation
Chores