Repository navigation
Preview terminal links in the embedded browser - #9374
lawrencecchen wants to merge 13 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds configurable terminal-link hover previews. The change validates targets, delays presentation, reuses prewarmed web views, renders preview cards, propagates browser profiles, and exposes the delay through settings and configuration files. ChangesTerminal link preview
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalLinkHoverIndicatorView
participant TerminalLinkPreviewController
participant TerminalLinkOpenCoordinator
participant BrowserPrewarmedWebViewPool
participant BrowserPanel
TerminalLinkHoverIndicatorView->>TerminalLinkPreviewController: submit hovered URL and anchor
TerminalLinkPreviewController->>TerminalLinkOpenCoordinator: resolve eligible preview target
TerminalLinkPreviewController->>BrowserPrewarmedWebViewPool: prewarm and attach matching webview
BrowserPrewarmedWebViewPool-->>TerminalLinkHoverIndicatorView: provide webview and load state
BrowserPanel->>BrowserPrewarmedWebViewPool: claim attached webview
BrowserPrewarmedWebViewPool-->>BrowserPanel: return webview and load state
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (7 errors, 1 warning, 1 inconclusive)
✅ Passed checks (16 passed)
✨ Finishing Touches 💡 1📝 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 |
…eview # Conflicts: # docs/ghostty-fork.md # ghostty # scripts/ghosttykit-checksums.txt
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@cmuxTests/BrowserPrewarmedWebViewPoolTests.swift`:
- Around line 228-348: Replace the two fixed async waits in
delayedPreviewStartsAfterDwellAndLeavingFirstCancelsIt and
movingWithinTheSameURLDoesNotRestartDwell with bounded polling of the real
completion predicates prewarmedURLs == [url] and prewarmCount == 1. Follow the
existing sleepCounter polling pattern, yielding until each predicate is
satisfied or a bounded retry/deadline expires, while preserving the current
assertions and cancellation behavior.
In `@ghostty`:
- Line 1: Update the Ghostty prebuilt pin in scripts/ghosttykit-checksums.txt
from commit 88357634c4dbadc87981e2ebb64eb599c53aa012 to
f0af83afe3c806b756f4aa0847507969f873b9ae, and add the corresponding downloaded
GhosttyKit archive checksum when required by the documented checksum flow.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swift`:
- Line 62: Update the _terminalLinkPreviewDelay accessor in BrowserSection so it
uses a single validated value constrained to the same 200...2,000 range enforced
by BrowserPanel and JSONConfigStore. Ensure both the initial UserDefaults value
and subsequent writes are normalized or rejected through that accessor,
preventing DefaultsValueModel<Int> from exposing or persisting out-of-range
values.
In `@Resources/Localizable.xcstrings`:
- Around line 170592-170642: Add localization entries for ar, bs, da, de, es,
fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant under each
terminalLinkPreviewDelay key shown, including the base, milliseconds, and
subtitle entries. Preserve the existing en and ja translations and match the
surrounding Localizable.xcstrings stringUnit structure.
In `@scripts/ghosttykit-checksums.txt`:
- Line 113: Update the checksum entry in scripts/ghosttykit-checksums.txt to
match the published GhosttyKit.xcframework.tar.gz release for the checked-in
Ghostty commit, ensuring ensure-ghosttykit.sh accepts it and does not fall back
to a local build.
In `@Sources/GhosttyTerminalView.swift`:
- Line 8237: Change terminalLinkPreviewController from an internal mutable
optional to a private non-optional let owned by the scroll view, initialize it
exactly once after super.init at the existing initialization site, and remove
optional chaining from the setLinkHoverURL update and deinit invalidate call
sites.
In `@Sources/KeyboardShortcutSettingsFileStore.swift`:
- Around line 911-919: Update the terminalLinkPreviewHoverDelayMilliseconds
handling in the browser-section parser to log out-of-range values and continue
instead of returning, so subsequent applyNormalizedStringArraySettings
processing still runs. Also log when the setting is present but not an integer,
while preserving assignment only for valid in-range integers.
In `@Sources/SettingsSearchAliases.swift`:
- Line 168: Add the key
settings.search.alias.setting.browser.terminal-link-preview-delay to
Resources/Localizable.xcstrings for every supported locale, using the source
locale’s alias text and matching the catalog’s existing localization structure.
In `@Sources/TerminalLinkPreviewController.swift`:
- Around line 91-98: Update TerminalLinkPreviewController’s update flow to check
for an unchanged currentRawURL with an already cached target before calling
targetResolver(request), returning through the existing visibility/anchor
handling when applicable. Store a single TerminalLinkOpenCoordinator property
and reuse it from the default resolver instead of constructing a new coordinator
on each hover callback.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72b4a53f-a1b5-4be9-a8ec-b6fbda616f54
📒 Files selected for processing (32)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/CmuxSettingsJSONPathSupport.swiftSources/DockSplitStore+TerminalLinkOpening.swiftSources/GhosttyTerminalView.swiftSources/GhosttyTerminalViewSupport.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/BrowserPanel+PrewarmedWebViewAdoption.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPrewarmedWebViewPool.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/TerminalLinkOpenContainer.swiftSources/TerminalLinkOpenCoordinator.swiftSources/TerminalLinkPreviewController.swiftSources/Workspace+TerminalLinkOpening.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserPrewarmedWebViewPoolTests.swiftcmuxTests/TerminalLinkOpenCoordinatorTests.swiftcmuxTests/TerminalLinkPreviewSettingsTests.swiftdocs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txtskills/cmux-settings/references/all-keys.mdweb/data/cmux.schema.jsonweb/messages/en.jsonweb/messages/ja.json
| _discardDelay = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.browser.hiddenWebViewDiscardDelaySeconds)) | ||
| _askWhereToSaveDownloads = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.browser.askWhereToSaveDownloads)) | ||
| _openTermLinks = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.browser.openTerminalLinksInCmuxBrowser)) | ||
| _terminalLinkPreviewDelay = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.browser.terminalLinkPreviewHoverDelayMilliseconds)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect the settings model or JSON loader to normalize this key to 200...2_000.
rg -n -C 8 \
'DefaultsValueModel|terminalLinkPreviewHoverDelayMillisecondsRange|browser\.terminalLinkPreviewHoverDelayMilliseconds' \
--glob '*.{swift,json}' Packages Sources cmuxTests webRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
fd -a 'BrowserSection.swift|DefaultsValueModel.swift|UserDefaultsSettingsStore.swift|JSONConfigStore.swift|KeyboardShortcutSettingsFileStore.swift|BrowserPanel.swift|BrowserCatalogSection.swift' . | sed 's#^\./##' | sort
echo
echo "== BrowserSection.swift relevant sections =="
file="$(fd 'BrowserSection.swift$' . | head -n1)"
if [ -n "$file" ]; then
wc -l "$file"
sed -n '1,120p' "$file" | cat -n
sed -n '220,290p' "$file" | cat -n
fi
echo
echo "== DefaultsValueModel definitions/usages around model =="
rg -n -C 6 'struct DefaultsValueModel|class DefaultsValueModel|actor DefaultsValueModel|typealias DefaultsValueModel|extension DefaultsValueModel|DefaultsValueModel<' --glob '*.swift' . | head -n 220
echo
echo "== DefaultsValueModel files =="
for f in $(rg -l 'struct DefaultsValueModel|class DefaultsValueModel|actor DefaultsValueModel' --glob '*.swift' . | head -n 10); do
echo "--- $f ($(wc -l < "$f") lines) ---"
sed -n '1,240p' "$f" | cat -n
done
echo
echo "== BrowserPanel runtime accessor =="
bpf="$(fd 'BrowserPanel.swift$' Sources/Panels | head -n1)"
if [ -n "$bpf" ]; then
sed -n '450,490p' "$bpf" | cat -n
fi
echo
echo "== Store initialization around _terminalLinkPreviewDelay line =="
if [ -n "$file" ]; then
rg -n -C 10 'terminalLinkPreviewDelay|terminalLinkPreviewHoverDelayMilliseconds' "$file"
fiRepository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DefaultsValueModel.swift relevant implementation =="
file="$(fd 'DefaultsValueModel.swift$' . | head -n1)"
sed -n '1,220p' "$file" | cat -n
sed -n '220,307p' "$file" | cat -n
echo
echo "== UserDefaultsSettingsStore initialization/validation/key read paths =="
ust="$(fd 'UserDefaultsSettingsStore.swift$' Packages/macOS/CmuxSettings/Sources | head -n1)"
sed -n '1,260p' "$ust" | cat -n
echo "matches in UserDefaultsSettingsStore.swift for setting keys / reads"
rg -n -C 4 'initialValue|set\(|reset\(|reconcile|terminalLinkPreviewHoverDelayMilliseconds|DefaultsKey|class DefaultsKey|struct DefaultsKey' "$ust"
echo
echo "== JSONConfigStore snippets for relevant validation path =="
jcs="$(fd 'JSONConfigStore.swift$' Packages/macOS/CmuxSettings/Sources | head -n1)"
rg -n -C 6 'terminalLinkPreviewHoverDelayMilliseconds|defaultTerminalLinkPreviewHoverDelayMilliseconds|logInvalid|managedUserDefaults|jsonInt' "$jcs"
echo
echo "== Static verifier: bounds used by runtime/import paths =="
python3 - <<'PY'
from pathlib import Path
files = list(Path('.').rglob('*'))
for name in [Path('Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Bindings/DefaultsValueModel.swift'),
Path('Packages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/UserDefaultsSettingsStore.swift'),
Path('Packages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/JSONConfigStore.swift'),
Path('Sources/Panels/BrowserPanel.swift')]:
print(f"\n--- {name} ---")
text = name.read_text()
for needle in ['terminalLinkPreviewHoverDelayMillisecondsRange', 'terminalLinkPreviewHoverDelayMilliseconds',
'defaultTerminalLinkPreviewHoverDelayMilliseconds', '200...2_000', 'maximum', 'minimum']:
print(f"{needle}: {text.count(needle)}")
print("\n--- Behavioral invariant from source text ---")
checks = [
('BrowserSection reads unbounded DefaultsValueModel<Int>', Path('Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swift').read_text().count('DefaultsValueModel<Int>') >= 2),
('JSON import rejects out-of-range integer and does not persist it', 'guard BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) else {\n logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath)\n return' in Path('Sources/KeyboardShortcutSettingsFileStore.swift').read_text()),
]
for label, ok in checks:
print(f"{label}: {ok}")
PYRepository: manaflow-ai/cmux
Length of output: 33739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd 'DefaultsValueModel.swift$' . | head -n1)"
ust="$(fd 'UserDefaultsSettingsStore.swift$' Packages/macOS/CmuxSettings/Sources | head -n1)"
jcs="$(fd 'JSONConfigStore.swift$' Packages/macOS/CmuxSettings/Sources | head -n1)"
echo "== DefaultsValueModel remaining implementation =="
sed -n '220,307p' "$file" | cat -n
echo
echo "== UserDefaultsSettingsStorage class references =="
rg -n -C 5 'class|struct|enum UserDefaultsSettingsStorage|func value\\(|func set\\(' "$ust" "$(fd 'UserDefaultsSettingsStorage.swift$' Packages/macOS/CmuxSettings/Sources | head -n1)" || true
echo
echo "== JSONConfigStore validation imports for terminalLinkPreviewHoverDelayMilliseconds =="
rg -n -C 8 'terminalLinkPreviewHoverDelayMilliseconds|defaultTerminalLinkPreviewHoverDelayMilliseconds|jsonInt\\(|logInvalid\\(|managedUserDefaults' "$jcs" "Sources/KeyboardShortcutSettingsFileStore.swift"
echo
echo "== Static checks =="
python3 - <<'PY'
from pathlib import Path
models = [
'Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Bindings/DefaultsValueModel.swift',
'Packages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/UserDefaultsSettingsStore.swift',
'Packages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/UserDefaultsSettingsStorage.swift',
'Sources/KeyboardShortcutSettingsFileStore.swift',
'Sources/Panels/BrowserPanel.swift',
]
for path in models:
text = Path(path).read_text()
print(f"\n--- {path} ({len(text.splitlines())} lines) ---")
for needle in [
'set(_ value: Value, for key: DefaultsKey<Value>'),
'storage.value(for: key)',
'storage.set(value, for: key)',
'terminalLinkPreviewHoverDelayMillisecondsRange',
'terminalLinkPreviewHoverDelayMilliseconds',
'maximum',
'minimum',
'default',
]:
lines = [i for i, line in enumerate(text.splitlines(), 1) if needle in line]
if lines:
print(f"{needle}: {lines[:10]}")
else:
print(f"{needle}: missing")
print("\n--- Relevant source assertions ---")
json_path = Path('Sources/KeyboardShortcutSettingsFileStore.swift')
json_text = json_path.read_text()
print("JSON import path rejects unbounded terminalLinkPreviewHoverDelayMilliseconds before persisting:",
'guard BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) else {\n logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath)\n return\n }' in json_text)
browserpath = 'Sources/Panels/BrowserPanel.swift'
print("Preview runtime clamps values outside range to 650:",
'let value = stored.intValue\n return BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value)\n ? value\n : defaultTerminalLinkPreviewHoverDelayMilliseconds' in Path(browserpath).read_text())
PYRepository: manaflow-ai/cmux
Length of output: 4878
Use one validated accessor for the preview delay.
JSONConfigStore rejects out-of-range browser.terminalLinkPreviewHoverDelayMilliseconds, but DefaultsValueModel<Int> seeds current from the raw UserDefaults value and BrowserSection writes it back through an unbounded store path. A stale or manually inserted value can appear in the settings UI, while BrowserPanel.swift rejects values outside 200...2_000. Add the shared range clamp/rejection to the settings binding or normalize it at a single validated accessor.
[low_effort_and_medium_reward]
🤖 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
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swift`
at line 62, Update the _terminalLinkPreviewDelay accessor in BrowserSection so
it uses a single validated value constrained to the same 200...2,000 range
enforced by BrowserPanel and JSONConfigStore. Ensure both the initial
UserDefaults value and subsequent writes are normalized or rejected through that
accessor, preventing DefaultsValueModel<Int> from exposing or persisting
out-of-range values.
| "settings.browser.terminalLinkPreviewDelay": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Terminal Link Preview Delay" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ターミナルリンクのプレビュー遅延" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.browser.terminalLinkPreviewDelay.milliseconds": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%lld ms" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%lldミリ秒" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.browser.terminalLinkPreviewDelay.subtitle": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "How long the pointer must rest on a terminal link before its live page preview appears." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ターミナルリンクにポインタを置いてからライブページのプレビューが表示されるまでの時間です。" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
path = Path("Resources/Localizable.xcstrings")
catalog = json.loads(path.read_text())
strings = catalog["strings"]
keys = [
"settings.browser.terminalLinkPreviewDelay",
"settings.browser.terminalLinkPreviewDelay.milliseconds",
"settings.browser.terminalLinkPreviewDelay.subtitle",
"settings.search.alias.setting.browser.terminal-link-preview-delay",
]
catalog_locales = sorted({
locale
for entry in strings.values()
for locale in entry.get("localizations", {})
})
for key in keys:
missing = [
locale
for locale in catalog_locales
if locale not in strings[key].get("localizations", {})
]
if missing:
print(f"{key}: missing {', '.join(missing)}")
PYRepository: manaflow-ai/cmux
Length of output: 740
Add missing localization entries for the new terminal preview keys.
Resources/Localizable.xcstrings contains additional supported locales beyond en and ja, but these keys still lack entries for ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant. Add stringUnit translations for each changed key family.
🤖 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 `@Resources/Localizable.xcstrings` around lines 170592 - 170642, Add
localization entries for ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru,
th, tr, uk, zh-Hans, and zh-Hant under each terminalLinkPreviewDelay key shown,
including the base, milliseconds, and subtitle entries. Preserve the existing en
and ja translations and match the surrounding Localizable.xcstrings stringUnit
structure.
Source: Path instructions
| private let keyboardCopyModeBadgeIconView: NSImageView | ||
| private let keyboardCopyModeBadgeLabel: NSTextField | ||
| let linkHoverIndicatorView: TerminalLinkHoverIndicatorView | ||
| var terminalLinkPreviewController: TerminalLinkPreviewController? |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Narrow the ownership of terminalLinkPreviewController.
The property is an internal mutable optional, so any file in the module can replace it or set it to nil. The scroll view is the sole owner of this controller and initializes it once at Line 8503.
linkHoverIndicatorView is assigned before super.init, so you can declare the controller as a non-optional stored let and assign it after super.init. That removes the optional chaining at both the setLinkHoverURL and deinit call sites and keeps a single owner for the preview lifecycle.
♻️ Proposed refactor
- var terminalLinkPreviewController: TerminalLinkPreviewController?
+ let terminalLinkPreviewController: TerminalLinkPreviewController super.init(frame: .zero)
- terminalLinkPreviewController = TerminalLinkPreviewController(view: linkHoverIndicatorView)
+ terminalLinkPreviewController = TerminalLinkPreviewController(view: linkHoverIndicatorView)Then drop the ? at the update(...) and invalidate() call sites.
Also applies to: 8503-8503
🤖 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 `@Sources/GhosttyTerminalView.swift` at line 8237, Change
terminalLinkPreviewController from an internal mutable optional to a private
non-optional let owned by the scroll view, initialize it exactly once after
super.init at the existing initialization site, and remove optional chaining
from the setLinkHoverURL update and deinit invalidate call sites.
| if let value = jsonInt(section["terminalLinkPreviewHoverDelayMilliseconds"]) { | ||
| guard BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) else { | ||
| logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath) | ||
| return | ||
| } | ||
| snapshot.managedUserDefaults[ | ||
| BrowserCatalogSection().terminalLinkPreviewHoverDelayMilliseconds.userDefaultsKey | ||
| ] = .int(value) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not abort browser-section parsing for an invalid delay.
When the delay is out of range, Line 914 returns before applyNormalizedStringArraySettings runs. Valid browser.urlsToAlwaysOpenExternally, browser.hostsToOpenInEmbeddedBrowser, and browser.insecureHttpHostsAllowedInEmbeddedBrowser values in the same file are then ignored.
Log the invalid value and continue parsing. Also log a present but non-integer value.
Proposed fix
if let value = jsonInt(section["terminalLinkPreviewHoverDelayMilliseconds"]) {
- guard BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) else {
- logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath)
- return
+ if BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) {
+ snapshot.managedUserDefaults[
+ BrowserCatalogSection().terminalLinkPreviewHoverDelayMilliseconds.userDefaultsKey
+ ] = .int(value)
+ } else {
+ logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath)
}
- snapshot.managedUserDefaults[
- BrowserCatalogSection().terminalLinkPreviewHoverDelayMilliseconds.userDefaultsKey
- ] = .int(value)
+ } else if section.keys.contains("terminalLinkPreviewHoverDelayMilliseconds") {
+ logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath)
}📝 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.
| if let value = jsonInt(section["terminalLinkPreviewHoverDelayMilliseconds"]) { | |
| guard BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) else { | |
| logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath) | |
| return | |
| } | |
| snapshot.managedUserDefaults[ | |
| BrowserCatalogSection().terminalLinkPreviewHoverDelayMilliseconds.userDefaultsKey | |
| ] = .int(value) | |
| } | |
| if let value = jsonInt(section["terminalLinkPreviewHoverDelayMilliseconds"]) { | |
| if BrowserCatalogSection.terminalLinkPreviewHoverDelayMillisecondsRange.contains(value) { | |
| snapshot.managedUserDefaults[ | |
| BrowserCatalogSection().terminalLinkPreviewHoverDelayMilliseconds.userDefaultsKey | |
| ] = .int(value) | |
| } else { | |
| logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath) | |
| } | |
| } else if section.keys.contains("terminalLinkPreviewHoverDelayMilliseconds") { | |
| logInvalid("browser.terminalLinkPreviewHoverDelayMilliseconds", sourcePath: sourcePath) | |
| } |
🤖 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 `@Sources/KeyboardShortcutSettingsFileStore.swift` around lines 911 - 919,
Update the terminalLinkPreviewHoverDelayMilliseconds handling in the
browser-section parser to log out-of-range values and continue instead of
returning, so subsequent applyNormalizedStringArraySettings processing still
runs. Also log when the setting is present but not an integer, while preserving
assignment only for valid in-range integers.
| "browser:hidden-webview-discard-delay": localized("settings.search.alias.setting.browser.hidden-webview-discard-delay", defaultValue: "browser.hiddenWebViewDiscardDelaySeconds memory hidden tabs delay seconds discard unload"), | ||
| "browser:ask-where-to-save-downloads": localized("settings.search.alias.setting.browser.ask-where-to-save-downloads", defaultValue: "browser.askWhereToSaveDownloads downloads save panel folder attachments files pdf gmail"), | ||
| "browser:terminal-links": localized("settings.search.alias.setting.browser.terminal-links", defaultValue: "browser.openTerminalLinksInCmuxBrowser click url terminal links open in browser href"), | ||
| "browser:terminal-link-preview-delay": localized("settings.search.alias.setting.browser.terminal-link-preview-delay", defaultValue: "browser.terminalLinkPreviewHoverDelayMilliseconds terminal url link hover preview dwell delay milliseconds thumbnail web page"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
path = Path("Resources/Localizable.xcstrings")
catalog = json.loads(path.read_text())
key = "settings.search.alias.setting.browser.terminal-link-preview-delay"
entry = catalog.get("strings", {}).get(key)
assert entry is not None, f"Missing catalog key: {key}"
source_locale = catalog.get("sourceLanguage", "en")
supported = {source_locale}
for value in catalog.get("strings", {}).values():
supported.update(value.get("localizations", {}).keys())
present = set(entry.get("localizations", {}).keys())
missing = sorted(supported - {source_locale} - present)
assert not missing, f"Missing locales for {key}: {missing}"
PY
rg -n -C 3 \
'terminalLinkPreviewHoverDelayMilliseconds|terminal-link-preview-delay' \
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swift \
Sources/SettingsSearchAliases.swiftRepository: manaflow-ai/cmux
Length of output: 453
Add catalog translations for the new search alias key.
settings.search.alias.setting.browser.terminal-link-preview-delay is present in the source locale, but it is missing across the supported translations. Add the matching key locallyizations to Resources/Localizable.xcstrings.
[full-internationalization]
🤖 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 `@Sources/SettingsSearchAliases.swift` at line 168, Add the key
settings.search.alias.setting.browser.terminal-link-preview-delay to
Resources/Localizable.xcstrings for every supported locale, using the source
locale’s alias text and matching the catalog’s existing localization structure.
Source: Path instructions
| let target = targetResolver(request) | ||
|
|
||
| if currentRawURL == normalizedRawURL, currentTarget == target { | ||
| if view?.isPreviewVisible != true { | ||
| currentAnchorPoint = anchorPoint | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid resolving the preview target on every repeat hover callback.
targetResolver(request) runs at Line 91 before the unchanged-URL check at Line 93. The default resolver at Lines 59-61 allocates a new TerminalLinkOpenCoordinator on each call, and previewTarget then reads several UserDefaults keys and compiles the external-open regex patterns. update runs on each Ghostty mouse-over-link callback, so an unchanged hover repeats that work.
Store one coordinator in a property, and skip resolution when the raw URL is unchanged and a target is already cached.
♻️ Proposed refactor
+ if currentRawURL == normalizedRawURL {
+ if view?.isPreviewVisible != true {
+ currentAnchorPoint = anchorPoint
+ }
+ return
+ }
+
let request = TerminalLinkOpenRequest(
rawValue: normalizedRawURL,
sourceWorkspaceId: sourceWorkspaceId,
sourcePanelId: sourcePanelId,
workingDirectory: nil
)
let target = targetResolver(request)
-
- if currentRawURL == normalizedRawURL, currentTarget == target {
- if view?.isPreviewVisible != true {
- currentAnchorPoint = anchorPoint
- }
- return
- }Also hoist the coordinator out of the default closure:
// Store once instead of constructing per hover callback.
private let coordinator: TerminalLinkOpenCoordinator🤖 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 `@Sources/TerminalLinkPreviewController.swift` around lines 91 - 98, Update
TerminalLinkPreviewController’s update flow to check for an unchanged
currentRawURL with an already cached target before calling
targetResolver(request), returning through the existing visibility/anchor
handling when applicable. Store a single TerminalLinkOpenCoordinator property
and reuse it from the default resolver instead of constructing a new coordinator
on each hover callback.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/BrowserPrewarmedWebViewPoolTests.swift (1)
262-263: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a controllable virtual sleeper for the dwell tests.
Both closures return immediately and ignore the requested duration. The tests can pass if
TerminalLinkPreviewControllerskips the 650 ms dwell or uses the wrong duration. The cancellation case does not exercise cancellation while a dwell is pending.Use a test-controlled sleeper that records the requested duration and completes only when the test advances it. Keep it pending in the cancellation path before calling
update(nil).As per coding guidelines, time-driven tests must use injected virtual clocks and explicit completion signals.
Also applies to: 297-298
🤖 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 `@cmuxTests/BrowserPrewarmedWebViewPoolTests.swift` around lines 262 - 263, Update the dwell tests’ delayMilliseconds and sleep closures to use a controllable virtual sleeper that records requested durations and completes only when explicitly advanced by the test. Ensure the normal path advances the expected 650 ms dwell, while the cancellation path leaves the sleeper pending before calling update(nil) so cancellation is exercised during the dwell.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@cmuxTests/BrowserPrewarmedWebViewPoolTests.swift`:
- Around line 262-263: Update the dwell tests’ delayMilliseconds and sleep
closures to use a controllable virtual sleeper that records requested durations
and completes only when explicitly advanced by the test. Ensure the normal path
advances the expected 650 ms dwell, while the cancellation path leaves the
sleeper pending before calling update(nil) so cancellation is exercised during
the dwell.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a475a1ea-72cc-4617-b378-a6ae7449df84
📒 Files selected for processing (1)
cmuxTests/BrowserPrewarmedWebViewPoolTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Panels/BrowserPrewarmedWebViewPool.swift (1)
137-150: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winInvoke the previous preview's
didDismissafter committing the new entry state.Line 144 calls the previous
previewDidDismisswhileself.entrystill holds the old attachment id and the old callbacks. If that callback re-enters the pool (for exampledetachPreviewor a nestedattachPreview), its mutation ofself.entryis then overwritten by the write of the stale local copy at Line 150.claimanddiscarduse the opposite order: they clear the stored entry first, then invokepreviewDidDismiss. AlignattachPreviewwith that order so the reentrant path cannot lose an update.♻️ Proposed reorder
- entry.previewDidDismiss?() + let previousDidDismiss = entry.previewDidDismiss let attachmentID = UUID() entry.previewAttachmentID = attachmentID entry.previewStateDidChange = stateDidChange entry.previewDidDismiss = didDismiss Self.host(entry.webView, in: hostView) self.entry = entry + previousDidDismiss?() scheduleExpiry()🤖 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 `@Sources/Panels/BrowserPrewarmedWebViewPool.swift` around lines 137 - 150, Update attachPreview around the local entry assignment so the new entry state is committed to self.entry before invoking the previous previewDidDismiss callback. Preserve the existing callback replacement and web view hosting behavior, ensuring reentrant detachPreview or attachPreview mutations are not overwritten by stale local state.
🤖 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 `@Sources/GhosttyTerminalViewSupport.swift`:
- Around line 300-315: Change the pointer-monitor lifecycle so
installPointerDownMonitor is no longer called during initialization; invoke it
when preparePreview makes the preview visible, and add removePointerDownMonitor
to unregister and clear the monitor during finishPreviewDismissal. Preserve the
existing event handling while ensuring no monitor remains active when the
preview is hidden.
- Around line 49-54: Update TerminalLinkHoverIndicatorView.hitTest(_:) to pass
the incoming point unchanged to previewShadowView.hitTest, since the child
expects its superview’s coordinate space; if this view may be inset, normalize
it with the superview conversion fallback before the hit test.
---
Outside diff comments:
In `@Sources/Panels/BrowserPrewarmedWebViewPool.swift`:
- Around line 137-150: Update attachPreview around the local entry assignment so
the new entry state is committed to self.entry before invoking the previous
previewDidDismiss callback. Preserve the existing callback replacement and web
view hosting behavior, ensuring reentrant detachPreview or attachPreview
mutations are not overwritten by stale local state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e85c686a-a8b9-48a1-ba8e-eab75893e1bb
📒 Files selected for processing (4)
Sources/GhosttyTerminalViewSupport.swiftSources/Panels/BrowserPrewarmedWebViewPool.swiftSources/TerminalLinkPreviewController.swiftcmuxTests/BrowserPrewarmedWebViewPoolTests.swift
| override func hitTest(_ point: NSPoint) -> NSView? { | ||
| nil | ||
| guard !isHidden, | ||
| !previewShadowView.isHidden, | ||
| previewShadowView.frame.contains(point) else { return nil } | ||
| return previewShadowView.hitTest(convert(point, to: previewShadowView)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the indicator view creation and layout to determine its frame origin.
rg -nP -C 6 '\bTerminalLinkHoverIndicatorView\b' --type=swiftRepository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files named GhosttyTerminalViewSupport.swift:"
fd -a 'GhosttyTerminalViewSupport\.swift$' . || true
echo
echo "Relevant symbols in Swift files:"
rg -n -C 4 'previewShadowView|override func hitTest|hitTest\(_ point|TerminalLinkHoverIndicatorView|class .*View|struct .*View' --type=swift . || trueRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate previewShadowView definitions/usages:"
rg -n -C 4 'previewShadowView' . || true
echo
echo "Locate GhosttyTerminalViewSupport.swift candidates:"
git ls-files | rg '^(Sources/)?GhosttyTerminalViewSupport\.swift$|GhosttyTerminalViewSupport\.swift$' || true
echo
echo "Relevant files with focused line ranges from output log path:"
if [ -f /tmp/coderabbit-shell-logs/shell-output-oPwJHP ]; then
# This is not useful; skip and just show exact candidate matching hits via less likely exact file names.
true
fi
echo
echo "Show TerminalLinkPreviewController outline/section:"
wc -l Sources/TerminalLinkPreviewController.swift
ast-grep outline Sources/TerminalLinkPreviewController.swift || true
sed -n '1,220p' Sources/TerminalLinkPreviewController.swiftRepository: manaflow-ai/cmux
Length of output: 22880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Sources/GhosttyTerminalViewSupport.swift outline:"
ast-grep outline Sources/GhosttyTerminalViewSupport.swift --view expanded || true
echo
echo "Relevant Sources/GhosttyTerminalViewSupport.swift section:"
wc -l Sources/GhosttyTerminalViewSupport.swift
sed -n '1,220p' Sources/GhosttyTerminalViewSupport.swift
echo
echo "Focused search for class and addSubview ordering:"
rg -n -C 3 'class TerminalLinkHoverIndicatorView|init\(frame|setURL|showPreview|layoutPreviewCard|addSubview\(previewShadowView\)|override func hitTest' Sources/GhosttyTerminalViewSupport.swiftRepository: manaflow-ai/cmux
Length of output: 12994
🌐 Web query:
Apple NSView hitTest point coordinate superview convert(to:) documentation
💡 Result:
The NSView method hitTest(:) expects a point in the coordinate system of the view's superview [1][2]. If you have a point in another coordinate system (such as the window's coordinate system or a different view's coordinate system), you must convert it before passing it to hitTest(:). You can use the NSView method convert(:to:) or convert(:from:) to perform these translations [3][4]. Key technical details regarding these coordinate conversions include: 1. Hit-Test Coordinate Expectation: The hitTest(:) method takes a point parameter that must be in the coordinate system of the receiver's superview [1][2]. 2. Converting to a Superview: To convert a point from the view's own coordinate system to its superview's coordinate system, you can use: view.convert(point, to: view.superview) [5] 3. Converting from Other Sources: - If you have a point in the window's coordinate system (often provided by NSEvent), you can convert it to the superview's coordinate system by passing nil as the view parameter in the conversion method [3][6]. For example: view.superview?.convert(pointInWindow, from: nil) [3][4] - Alternatively, if you are converting from the window directly to the local view, you would use: view.convert(pointInWindow, from: nil) [7][6] Because hitTest(:) is typically used by NSWindow to determine the target view for mouse events [1][7], you should ensure that the coordinate conversion correctly bridges the gap between the event's source (usually the window) and the superview of the view hierarchy being queried [3].
Citations:
- 1: https://developer.apple.com/documentation/appkit/nsview/hittest(_:)
- 2: https://developer.apple.com/documentation/appkit/nsview/hittest(_:)?language=objc
- 3: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CocoaViewsGuide/WorkingWithAViewHierarchy/WorkingWithAViewHierarchy.html
- 4: https://developer.apple.com/library/archive/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/APIs/APIs.html
- 5: https://developer.apple.com/documentation/appkit/nsview/convert(_:to:)-6u9ir?language=objc
- 6: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CocoaDrawingGuide/Transforms/Transforms.html
- 7: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CocoaViewsGuide/SubclassingNSView/SubclassingNSView.html
Use a superview-normalized point for the preview hit test.
NSView.hitTest(_:) expects a point in the receiver’s superview coordinate space. Here, hitTest(_:) is on TerminalLinkHoverIndicatorView, and previewShadowView is a direct subview, so pass the incoming point unchanged; passing the already-converted point to previewShadowView shifts the hit target and can make web content under the preview card unclickable. Use superview.map { convert(point, from: $0) } ?? point if this view can be inset.
🤖 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 `@Sources/GhosttyTerminalViewSupport.swift` around lines 49 - 54, Update
TerminalLinkHoverIndicatorView.hitTest(_:) to pass the incoming point unchanged
to previewShadowView.hitTest, since the child expects its superview’s coordinate
space; if this view may be inset, normalize it with the superview conversion
fallback before the hit test.
| private func installPointerDownMonitor() { | ||
| pointerDownMonitor = NSEvent.addLocalMonitorForEvents( | ||
| matching: [.leftMouseDown, .rightMouseDown, .otherMouseDown] | ||
| ) { [weak self] event in | ||
| guard let self, self.isPreviewVisible else { return event } | ||
| let isInsidePreview: Bool | ||
| if event.window === self.window { | ||
| let point = self.convert(event.locationInWindow, from: nil) | ||
| isInsidePreview = self.previewShadowView.frame.contains(point) | ||
| } else { | ||
| isInsidePreview = false | ||
| } | ||
| self.onPreviewPointerDown?(isInsidePreview) | ||
| return event | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Install the pointer-down monitor only while a preview is visible.
installPointerDownMonitor runs in init, so every TerminalLinkHoverIndicatorView keeps a process-wide local event monitor for its whole lifetime. Each terminal surface owns one indicator view, so the app runs one monitor per surface and each mouse-down walks all of them, even when no preview exists. Add the monitor in preparePreview and remove it in finishPreviewDismissal, so at most one monitor is active.
♻️ Proposed lifecycle change
- private func installPointerDownMonitor() {
+ private func installPointerDownMonitorIfNeeded() {
+ guard pointerDownMonitor == nil else { return }
pointerDownMonitor = NSEvent.addLocalMonitorForEvents(
matching: [.leftMouseDown, .rightMouseDown, .otherMouseDown]
) { [weak self] event in
guard let self, self.isPreviewVisible else { return event }private func removePointerDownMonitor() {
guard let pointerDownMonitor else { return }
NSEvent.removeMonitor(pointerDownMonitor)
self.pointerDownMonitor = nil
}🤖 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 `@Sources/GhosttyTerminalViewSupport.swift` around lines 300 - 315, Change the
pointer-monitor lifecycle so installPointerDownMonitor is no longer called
during initialization; invoke it when preparePreview makes the preview visible,
and add removePointerDownMonitor to unregister and clear the monitor during
finishPreviewDismissal. Preserve the existing event handling while ensuring no
monitor remains active when the preview is hidden.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalViewSupport.swift (1)
341-344: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRoute size-driven dismissal through
TerminalLinkPreviewController.When the view shrinks below the minimum size, Line 343 calls
dismissPreview(animated: false)without a completion.Sources/TerminalLinkPreviewController.swiftdetaches its pooled attachment and clears dismissal state only from the completion passed by its dismissal path in Lines 269-297. A resize can therefore hide the card while retaining the attachment. During an active fade, the view can suppress the original completion and leaveisDismissalAnimatingset.Report the size failure to the controller and use the existing dismissal path with its completion. Do not finish preview lifecycle state locally from
layoutPreviewCard.As per path instructions, correctness-critical state must use one reliable source of truth, and Swift architecture changes must not split UI lifecycle ownership.
🤖 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 `@Sources/GhosttyTerminalViewSupport.swift` around lines 341 - 344, Update layoutPreviewCard(at:) to report unavailable preview size through TerminalLinkPreviewController’s existing dismissal path, including its required completion, instead of directly calling dismissPreview(animated: false). Ensure attachment detachment and dismissal-animation state are finalized by the controller, with no local preview lifecycle cleanup in layoutPreviewCard.Source: Path instructions
🤖 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.
Outside diff comments:
In `@Sources/GhosttyTerminalViewSupport.swift`:
- Around line 341-344: Update layoutPreviewCard(at:) to report unavailable
preview size through TerminalLinkPreviewController’s existing dismissal path,
including its required completion, instead of directly calling
dismissPreview(animated: false). Ensure attachment detachment and
dismissal-animation state are finalized by the controller, with no local preview
lifecycle cleanup in layoutPreviewCard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 082394ce-f951-413e-9b79-a8dd1b76a54d
📒 Files selected for processing (1)
Sources/GhosttyTerminalViewSupport.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
lawrencecchen
added
the
stale-revisit
2 participants
Summary
Verification
CmuxWebViewfirst responder, then the next key's terminal repair moved first responder toGhosttyNSViewurlpvinputred2-7e0a61d4bc-74374e1f1550; 8 existing policy tests passed and the new WebView responder test failed at the expected assertionurlpvinputgreen-d1146e6db5-bdf0bea27b91; 10/10 tests passed, including interactive WebView retention and background-WebView reclamationurlpvpreviewgreen-d1146e6db5-0070da2fd262; 23/23 tests passed, including focus pinning, outside-click release, hover bridge, pointer anchoring, and exact WebView reuseurlpv; final UI input dogfood was blocked because the pinned Ghostty test override did not spawn a PTY after restart, so no fresh terminal URL could be renderedurlpvred-44503a9bc8c8; 17 existing tests passed and the new interactive hit-routing test failed at the expectednilhitbd053bc640; old geometry deterministically violates the 8 pt distance and outer tracking-halo assertions. Its cloud red run could not lease a test builder, so this is not reported as a completed red actionurlpvpointergreen-7a03c07cc80c; 23 tests inBrowserPrewarmedWebViewPoolTestspassed, including pointer placement, passthrough hover bridge, and terminal source-event coordinate conversionurlpv; final visual interaction was blocked by the macOS lock screen. Exact final geometry and hover-halo behavior are covered by the focused tests aboveurlpv2-c18ef61df43aSummary by CodeRabbit