Add actionable Iroh connection checks - #9750
Closed
azooz2003-bit wants to merge 14 commits into
Closed
azooz2003-bit wants to merge 14 commits into
azooz2003-bit wants to merge 14 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 staged Iroh connection-check reports with transport, relay, discovery, and session results. Adds relay reachability APIs, mobile and Mac runtime integration, settings UI, localization, and automated tests. ChangesIroh connection check
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant SettingsModel
participant RuntimeComposition
participant IrohRuntime
participant CmxIrohConnectionCheckReport
SettingsUI->>SettingsModel: start connection check
SettingsModel->>RuntimeComposition: runIrohConnectionCheck()
RuntimeComposition->>RuntimeComposition: refresh settings and diagnostics
RuntimeComposition->>IrohRuntime: probe configured relay URLs
IrohRuntime-->>RuntimeComposition: relay reachability
RuntimeComposition->>CmxIrohConnectionCheckReport: evaluate snapshot and diagnostics
CmxIrohConnectionCheckReport-->>RuntimeComposition: staged report
RuntimeComposition-->>SettingsModel: publish report
SettingsModel-->>SettingsUI: render stages and recommendation
Possibly related issues
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 1 warning)
✅ Passed checks (19 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 `@cmuxUITests/SettingsNetworkingBehaviorUITests.swift`:
- Around line 21-33: Update the staged report assertion in the relevant UI test
to poll until Encrypted Transport, Relay Policy, and Relay Reachability are all
present, rather than polling only transportStage and checking the other sections
immediately. Keep the existing timeout and failure messaging where appropriate,
and preserve validation that each required staged section is published.
- Around line 6-19: Update the Settings networking test around makeLaunchedApp
and the Iroh connection-check flow to create and configure a test-scoped fake
relay or ephemeral endpoint before runButton.click(). Ensure the launched app’s
Iroh networking configuration points exclusively to this local fixture, then
retain assertions against the resulting staged connection-check state without
contacting production relays or endpoints.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift`:
- Around line 47-50: Update the runtime status update loop in
MobileIrohSettingsModel to compare the previous and incoming statuses, invoking
the shared runConnectionCheckAndWait path when the status transitions into
.degraded and diagnosticReport.lastFailureKind is non-nil. Preserve the existing
initial-snapshot behavior, and add a regression test covering a later transition
to .degraded with a diagnostic failure.
- Around line 109-113: Update MobileIrohSettingsModel.runConnectionCheck() to
launch runConnectionCheckAndWait() through the model’s stored,
view-lifecycle-managed task rather than an unretained unstructured Task.
Preserve the existing isRunningConnectionCheck guard and shared execution path,
and ensure dismissing Settings can cancel the task.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swift`:
- Around line 28-31: Update the runtime observation logic in IrohSettingsModel
so the diagnosed degraded-state check is evaluated after every status update,
not only for the initial snapshot. Track the previous runtime status and invoke
runConnectionCheckAndWait(using:) only when transitioning into .degraded while
diagnosticReport.lastFailureKind is non-nil; leave unchanged degraded updates
without starting duplicate checks.
- Around line 88-90: Update runConnectionCheck() to retain the created Task in a
lifecycle-owned property or expose the check as an async operation owned by its
caller, rather than discarding it. Cancel that task when the Settings flow ends,
propagate cancellation through runConnectionCheckAndWait() into
runIrohConnectionCheck(), and prevent cancelled operations from publishing
connectionCheck, snapshot, or diagnostics state.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohNetworkingSection.swift`:
- Around line 254-258: Add catalog entries for settings.networking.check.title
and settings.networking.check.subtitle in Resources/Localizable.xcstrings for
every supported locale, not only en and ja. Preserve the existing English and
Japanese values and follow the catalog’s established locale and translation
structure.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohConnectionCheckReport.swift`:
- Around line 93-94: Update the status logic in CmxIrohConnectionCheckReport to
use an authoritative live-Mac discovery result from
routeCatalog.liveMacCandidates or an equivalent dedicated snapshot field, not
snapshot.privateNetworkMacs, which includes configured paths. Fail closed or
report unknown when the live discovery result is unavailable, while preserving
the selectedTransportPath status logic.
- Around line 81-85: Separate relay configuration absence from unavailable probe
data across the connection-check flow. In
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohConnectionCheckReport.swift
lines 81-85, add a distinct not-configured state and map unavailable
reachability to unknown or failed so readiness fails closed. In
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohSettingsControlling.swift
lines 57-64, return the explicit unsupported or unavailable state from the
default implementation. In
ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift lines
2486-2494 and Sources/Mobile/MobileHostIrohRuntime+SettingsControl.swift lines
171-179, map a configured relay profile with a nil probe result to unavailable
rather than not-configured.
- Around line 40-42: Update the StageKind declaration to conform to Hashable so
it can satisfy the Identifiable.ID requirement used by Stage.id. Preserve its
existing conformances and behavior.
In `@Resources/Localizable.xcstrings`:
- Around line 265137-265216: Add localization entries for every supported
catalog locale to all 20 settings.networking.check keys, including their title,
status, note, and action strings. Preserve the existing en and ja translations,
and follow the surrounding Localizable.xcstrings structure and established
translations for each locale.
🪄 Autofix
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: ff2e3974-0a4b-4dd6-9c88-5fda0bf5501b
📒 Files selected for processing (16)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohConnectionCheckReport.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohSettingsControlling.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/CmxIrohConnectionCheckReportTests.swiftPackages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime+RelayReachability.swiftPackages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime+RelayReachability.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohNetworkingSection.swiftResources/Localizable.xcstringsSources/Mobile/MobileHostIrohRuntime+SettingsControl.swiftcmuxUITests/SettingsNetworkingBehaviorUITests.swiftios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift
Comment on lines
+6
to
+19
| let app = makeLaunchedApp() | ||
| let window = openSettings(app) | ||
| defer { closeSettings(app, window) } | ||
|
|
||
| navigate(window, to: "Networking") | ||
| let runButton = requireElement( | ||
| candidates: [ | ||
| window.buttons["SettingsIrohRunConnectionCheck"], | ||
| window.descendants(matching: .any)["SettingsIrohRunConnectionCheck"], | ||
| ], | ||
| timeout: 5, | ||
| description: "Iroh connection check button" | ||
| ) | ||
| runButton.click() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 --glob '*.swift' \
'\bmakeLaunchedApp\s*\(|\bSettingsUITestCase\b|Iroh|relay|endpoint|URLSession|NWConnection' .Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg 'TestsNetworkingBehaviorUITests|makeLaunchedApp|SettingsUITestCase|LaunchArguments|Launch.*Helper|Launch' | head -200
echo
echo "== target file =="
if [ -f cmuxUITests/SettingsNetworkingBehaviorUITests.swift ]; then
nl -ba cmuxUITests/SettingsNetworkingBehaviorUITests.swift | sed -n '1,120p'
fi
echo
echo "== focused references =="
rg -n --glob '*.swift' -C 6 '\bfunc makeLaunchedApp\b|\bbinding\s*=|CMUCLaunch|launchArguments|launchEnvironment|Iroh|SettingsIrohRunConnectionCheck|relayPort|relayToken|endpoint' cmuxUITests .github Packages cmuxTests tests 2>/dev/null | head -250Repository: manaflow-ai/cmux
Length of output: 11687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
if [ -f cmuxUITests/SettingsNetworkingBehaviorUITests.swift ]; then
cat -n cmuxUITests/SettingsNetworkingBehaviorUITests.swift | sed -n '1,220p'
else
echo "missing cmuxUITests/SettingsNetworkingBehaviorUITests.swift"
fi
echo
echo "== search target references =="
rg -n --glob '*.swift' -C 8 '\bmakeLaunchedApp\b|\bSettingsUITestCase\b|\bopenSettings\b|\bcloseSettings\b|\bnavigate\b|\brequireElement\b|\bSettingsIrohRunConnectionCheck\b|\bIroh\b|\brelay(?:Port|ID|Token)?\b|\beb\.' cmuxUITests || true
echo
echo "== focused launch/network fake references =="
rg -n --glob '*.swift' -C 6 '\bbinding\s*=|launchedApp|launchArguments|launchEnvironment|fake.*network|mock.*endpoint|local.*server|ephemeral.*port|Iroh|relayPort|relayToken|cmuxIroh|CMUX_IROH|Irohub|relayer' cmuxUITests .github Packages cmuxTests tests 2>/dev/null | head -300Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact helper and fixture references =="
rg -n --glob '*.swift' -C 8 '\bbinding\s*=[\s]*"CMUX_UI_TEST_BROWSER_IMPORT_FIXTURE"|CMUX_UI_TEST_BROWSER_IMPORT_FIXTURE|cmux.*launch|makeLaunchedApp\s*\(\)|func makeLaunchedApp|func makeLaunchedAppWithImportFixture|class SettingsUITestCase|protocol SettingsUITestCase|let settings.*base|launchArguments:|launchEnvironment:' cmuxUITests Packages Sources Tests cmuxTests 2>/dev/null || true
echo
echo "== SettingsUITestCase files =="
git ls-files | rg 'cmuxUITests/.*Settings.*Test|Settings.*TestCase|SettingsNetworkingBehaviorUITests' | sort | while read -r f; do
echo "--- $f"
wc -l "$f"
done
echo
echo "== base SettingsUITestCase content candidates =="
for f in $(git ls-files 'cmuxUITests/*Settings*UI*.swift'); do
if rg -q 'class SettingsUITestCase|protocol SettingsUITestCase|extension SettingsUITestCase' "$f"; then
echo "--- $f"
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SettingsUITestCase declarations =="
rg -n --glob '*.swift' 'typealias SettingsUITestCase|class SettingsUITestCase|protocol SettingsUITestCase|extension SettingsUITestCase' .
echo
echo "== likely SettingsUITestCase definition files =="
for f in cmuxUITests/SettingsAppBehaviorUITests.swift cmuxUITests/SettingsAutomationBehaviorUITests.swift cmuxUITests/SettingsSidebarBetaBehaviorUITests.swift cmuxUITests/SettingsBrowserBehaviorUITests.swift cmuxUITests/SettingsTerminalBehaviorUITests.swift cmuxUITests/SettingsAccountJSONResetBehaviorUITests.swift cmuxUITests/SettingsNetworkingBehaviorUITests.swift; do
if [ -f "$f" ]; then
lines=$(wc -l "$f" | awk '{print $1}')
echo "--- $f ($lines lines)"
cat -n "$f" | sed -n '1,130p'
fi
done
echo
echo "== launch/environment helpers in cmuxUITests =="
rg -n --glob '*.swift' -C 10 'cmuxTestApplication\(\)|launchAndActivate|settingsLaunchArguments|func makeLaunchedApp|let settingsLaunchArguments|override func setUp|override func tearDown' cmuxUITests | sed -n '1,260p'Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SettingsUITestSupport.swift =="
cat -n cmuxUITests/SettingsUITestSupport.swift | sed -n '1,220p'
echo
echo "== all SettingsUITestSupport declarations/usages =="
rg -n --glob '*.swift' 'func makeLaunchedApp|func launchAndActivate|let settingsLaunchArguments|var settingsLaunchArguments|launchArguments|launchEnvironment|wait|activate' cmuxUITests/SettingsUITestSupport.swift cmuxUITests/Settings*BehaviorUITests.swift cmuxUITests/SettingsNetworkingBehaviorUITests.swift | sed -n '1,240p'
echo
echo "== exact environment/fixture references excluding generated/search noise =="
rg -n --glob '*.swift' -C 8 'CMUX_UI_TEST_(MODE|FETCH_PROFILE|BROWSER_IMPORT|IMPORT_.*FIXTURE|JUMP|SKIP|SKIP_|SETUP).*|Iroh|iroh|relayer|relayID|relayToken|relayPort' cmuxUITests Packages Sources cmuxTests .github | sed -n '1,260p'Repository: manaflow-ai/cmux
Length of output: 36184
Use a local Iroh networking fixture for the Settings connection-check test.
makeLaunchedApp() only sets CMUX_UI_TEST_MODE and English/transient launch args, so runButton.click() exercises the production Iroh path against any configured live relay/endpoint. Add a test-scoped fake relay/ephemeral endpoint and wire the Settings/UI test to it before asserting staged results.
🤖 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 `@cmuxUITests/SettingsNetworkingBehaviorUITests.swift` around lines 6 - 19,
Update the Settings networking test around makeLaunchedApp and the Iroh
connection-check flow to create and configure a test-scoped fake relay or
ephemeral endpoint before runButton.click(). Ensure the launched app’s Iroh
networking configuration points exclusively to this local fixture, then retain
assertions against the resulting staged connection-check state without
contacting production relays or endpoints.
Source: Path instructions
Comment on lines
+254
to
+258
| String(localized: "settings.networking.check.title", defaultValue: "Connection Check"), | ||
| subtitle: String( | ||
| localized: "settings.networking.check.subtitle", | ||
| defaultValue: "Checks encrypted transport, relay policy, and relay reachability from this Mac." | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
catalog_path = Path("Resources/Localizable.xcstrings")
catalog = json.loads(catalog_path.read_text())
strings = catalog["strings"]
keys = [
"settings.networking.check.title",
"settings.networking.check.subtitle",
"settings.networking.check.run",
"settings.networking.check.note",
"settings.networking.check.transport",
"settings.networking.check.policy",
"settings.networking.check.relay",
"settings.networking.check.mac",
"settings.networking.check.session",
"settings.networking.check.passed",
"settings.networking.check.warning",
"settings.networking.check.failed",
"settings.networking.check.notApplicable",
"settings.networking.check.action.retry",
"settings.networking.check.action.internet",
"settings.networking.check.action.mac",
"settings.networking.check.action.relay",
"settings.networking.check.action.account",
"settings.networking.check.action.settings",
"settings.networking.check.action.repair",
]
supported = set()
for entry in strings.values():
supported.update(entry.get("localizations", {}))
errors = []
for key in keys:
entry = strings.get(key)
if entry is None:
errors.append(f"missing key: {key}")
continue
missing = sorted(supported - set(entry.get("localizations", {})))
if missing:
errors.append(f"{key} missing locales: {', '.join(missing)}")
if errors:
raise SystemExit("\n".join(errors))
PYRepository: manaflow-ai/cmux
Length of output: 2887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Localizable.xcstrings path =="
git ls-files | grep -E '(^|/)(Resources/Localizable\.xcstrings|.*\.xcstrings)$' | head -50
python3 - <<'PY'
import json
from pathlib import Path
catalog_path = Path("Resources/Localizable.xcstrings")
missing = []
for p in catalog_path.parents:
if p.name == "Packages":
missing.append("not_found_at_repo_root")
break
if catalog_path.exists():
catalog = json.loads(catalog_path.read_text())
strings = catalog["strings"]
connected_keys = [k for k in strings if k.startswith("settings.networking.check")]
print(f"catalog_exists={catalog_path}")
print(f"connected_keys_count={len(connected_keys)}")
for k in connected_keys:
print(f"{k}: locales={sorted(strings[k].get('localizations', {}))}")
all_localizations = {}
for entry in strings.values():
for locale in entry.get("localizations", {}):
all_localizations.setdefault(locale, set()).add((entry.get("comments", {}), entry.get("revision", {}), entry.get("stringUnit", {}).get("state")))
print("keys_without_all_locale_values:", sorted(set(k for k,v in strings.items() if len(v.get("localizations", {})) < len(all_localizations))))
else:
print("Resources/Localizable.xcstrings not found at repo root")
PYRepository: manaflow-ai/cmux
Length of output: 2690
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files under Packages/macOS/CmuxSettingsUI containing Localizable/xcstrings =="
git ls-files 'Packages/macOS/CmuxSettingsUI/**' | grep -E '(^|/)(Resources/Localizable\.xcstrings|.*\.xcstrings)$' || true
for f in $(git ls-files 'Packages/macOS/CmuxSettingsUI/**' | grep -E 'Localizable\.xcstrings$'); do
echo "--- $f"
python3 - <<'PY' "$f"
import json, sys
p = sys.argv[1]
catalog = json.loads(open(p, encoding="utf-8").read())
keys = {s for s in catalog.get("strings", {}) if s.startswith("settings.networking.check")}
print(f"keys_count={len(keys)}")
locales = set()
for e in catalog.get("strings", {}).values():
locales.update(e.get("localizations", {}).keys())
missing = []
for k in keys:
miss = sorted(locales - set(catalog["strings"].get(k, {}).get("localizations", {})))
if miss:
missing.append((k, miss))
print(f"supported_locales_count={len(locales)}")
print(f"missing_locale_count={len(missing)}")
for k, miss in missing[:20]:
print(f"{k}: missing_locales={len(miss)}")
PY
doneRepository: manaflow-ai/cmux
Length of output: 235
Add catalog values for every supported locale before introducing these keys.
Resources/Localizable.xcstrings defines settings.networking.check.*, but all 20 keys currently only have en and ja. Add all supported locale values for these keys so the shipped catalog covers internationalization requirements.
🤖 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/IrohNetworkingSection.swift`
around lines 254 - 258, Add catalog entries for settings.networking.check.title
and settings.networking.check.subtitle in Resources/Localizable.xcstrings for
every supported locale, not only en and ja. Preserve the existing English and
Japanese values and follow the catalog’s established locale and translation
structure.
Sources: Coding guidelines, Path instructions
Comment on lines
+265137
to
+265216
| "settings.networking.check.title": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Connection Check" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "接続チェック" } } | ||
| } }, | ||
| "settings.networking.check.subtitle": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Checks encrypted transport, relay policy, and relay reachability from this Mac." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "この Mac から暗号化トランスポート、リレーポリシー、リレーへの到達性を確認します。" } } | ||
| } }, | ||
| "settings.networking.check.run": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Run Check" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "チェックを実行" } } | ||
| } }, | ||
| "settings.networking.check.note": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "cmux automatically uses direct internet, LAN, or any VPN route available to macOS, then falls back to an allowed relay. Every route remains end-to-end encrypted." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "cmux は macOS で利用可能なインターネット、LAN、または任意の VPN 経路を自動的に使用し、必要に応じて許可されたリレーへ切り替えます。すべての経路でエンドツーエンド暗号化が維持されます。" } } | ||
| } }, | ||
| "settings.networking.check.transport": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Encrypted Transport" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "暗号化トランスポート" } } | ||
| } }, | ||
| "settings.networking.check.policy": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Relay Policy" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "リレーポリシー" } } | ||
| } }, | ||
| "settings.networking.check.relay": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Relay Reachability" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "リレーへの到達性" } } | ||
| } }, | ||
| "settings.networking.check.mac": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Mac Available" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "Mac の利用可否" } } | ||
| } }, | ||
| "settings.networking.check.session": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Secure Session" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "安全なセッション" } } | ||
| } }, | ||
| "settings.networking.check.passed": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Passed" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "正常" } } | ||
| } }, | ||
| "settings.networking.check.warning": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Needs Attention" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "確認が必要" } } | ||
| } }, | ||
| "settings.networking.check.failed": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Failed" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "失敗" } } | ||
| } }, | ||
| "settings.networking.check.notApplicable": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Not Needed" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "不要" } } | ||
| } }, | ||
| "settings.networking.check.action.retry": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Retry. If this continues, share the safe report with cmux support." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "再試行してください。問題が続く場合は、安全なレポートを cmux サポートと共有してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.internet": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Connect this Mac to the internet, then run the check again." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "この Mac をインターネットに接続してから、もう一度確認してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.mac": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Keep cmux open and confirm both apps use the same account." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "cmux を開いたままにし、両方のアプリが同じアカウントを使用していることを確認してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.relay": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Your network may block relay traffic. Ask IT to allow HTTPS and WebSocket access to your configured cmux relay domains, or add an approved custom relay." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "ネットワークがリレートラフィックを遮断している可能性があります。設定済みの cmux リレードメインへの HTTPS と WebSocket アクセスを許可するよう IT 部門に依頼するか、承認済みのカスタムリレーを追加してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.account": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Confirm you are signed in, then reopen cmux and run the check again." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "サインインを確認し、cmux を開き直してからもう一度確認してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.settings": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Choose Automatic relay selection, or fix the selected custom relay and its device secret." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "リレー選択を「自動」にするか、選択したカスタムリレーとデバイスシークレットを修正してください。" } } | ||
| } }, | ||
| "settings.networking.check.action.repair": { "extractionState": "manual", "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Update cmux on both devices. If needed, pair them again." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "両方のデバイスで cmux を更新してください。必要に応じて再度ペアリングしてください。" } } | ||
| } }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add translations for every supported locale.
These 20 new keys contain only en and ja. Resources/Localizable.xcstrings must include translated entries for every locale already supported by the catalog. Add the missing locale entries before merging this settings flow.
🤖 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 265137 - 265216, Add
localization entries for every supported catalog locale to all 20
settings.networking.check keys, including their title, status, note, and action
strings. Preserve the existing en and ja translations, and follow the
surrounding Localizable.xcstrings structure and established translations for
each locale.
Sources: Coding guidelines, Path instructions, Learnings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swift`:
- Around line 73-78: Canonicalize relay URLs before constructing the allowlist
text in relayAllowlistText (iOS,
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swift#L73-L78)
and the corresponding macOS allowlist implementation
(Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohConnectionCheckCard.swift#L90-L95).
Include only valid root origins with no user info, query, fragment, or non-root
path, and reject or fail closed for invalid relay values so raw credentials or
URL components never enter the ShareLink payload.
🪄 Autofix
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: 41976dad-b61c-4245-a97e-d05ce24cf626
📒 Files selected for processing (6)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohConnectionCheckCard.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohNetworkingSection.swiftResources/Localizable.xcstrings
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swift`:
- Around line 101-121: Update supportReportText(_:) to remove the
safeRelayOrigins check and relayAllowlistText append, ensuring Share Connection
Report never includes relay origins. Preserve relayAllowlistText exclusively in
the separate Share IT Allowlist action.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift`:
- Around line 104-110: Update runConnectionCheck to reserve ownership of the
connection-check operation before launching a new Task, preventing rapid calls
from creating competing tasks. When the task completes, clear
connectionCheckTask only if it still references that task, preserving
cancellation of the active request. Add coverage for rapid starts and
cancel-then-restart behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift`:
- Around line 130-133: Replace the fixed Task.yield loop in the duplicate
degraded snapshot test with a deterministic completion signal or consumed-update
counter exposed by the controller test double. Await confirmation that the
second snapshot was processed by acceptSnapshot, then assert
controller.connectionCheckRunCount == 1.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swift`:
- Around line 89-93: Update the connection-check task flow around
connectionCheckTask and runConnectionCheckAndWait(using:) to reserve ownership
before starting execution: only create a new task when connectionCheckTask is
nil, and ensure cleanup clears the handle only for that task. In
runConnectionCheckAndWait(using:), check !Task.isCancelled before invoking
runIrohConnectionCheck() so cancelled tasks never start the probe.
- Around line 163-168: Update acceptSnapshot() so its automatic-check guard also
requires the refreshed diagnostics to report a failure, in addition to the
existing cancellation, previous-status, and degraded runtime checks. Use the
diagnostic failure signal populated by reloadDiagnostics(using:) and keep
runConnectionCheckAndWait(using:) limited to diagnosed degraded states.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohConnectionCheckReport.swift`:
- Around line 89-93: Update the report flow around relayStatus and
recommendation to pass the RelayReachability value through instead of collapsing
.unavailable into .failed. Return .allowRelayTraffic only for .unreachable, and
return .retry for .unavailable after higher-priority transport and policy
failures. Use authoritative structured runtime state with fail-closed behavior
when live data is unavailable, and add coverage for an inactive runtime
producing an unavailable probe.
🪄 Autofix
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: 08fa3d55-1040-49d5-a0ce-4659f72ee154
📒 Files selected for processing (18)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohConnectionCheckReport.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohRelayOrigin.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohSettingsControlling.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/CmxIrohConnectionCheckReportTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohPrivateNetworksSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohConnectionCheckCard.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohNetworkingSection.swiftResources/Localizable.xcstringsSources/Mobile/MobileHostIrohRuntime+SettingsControl.swiftcmux.xcodeproj/project.pbxprojcmuxUITests/SettingsNetworkingBehaviorUITests.swiftios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift
- recommendation distinguishes an unavailable relay probe (indeterminate: inactive runtime, unreadable path hints) from a probed-and-blocked relay; corporate allowlist advice now requires .unreachable, .unavailable retries - Share Connection Report no longer embeds relay origins on either platform; the IT allowlist stays in its dedicated share action - connection-check tasks reserve ownership before starting, clear only their own handle, and never start a probe after cancellation - automatic checks require a diagnosed degraded entry (diagnostic failure or relay-configuration failureDescription) per the PR contract - model tests wait on deterministic diagnostics-read counts instead of yield loops, and cover rapid starts, cancel-then-restart, and undiagnosed degraded transitions on both platforms Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Review findings addressed in 4ebb0ba: Unavailable relay probe vs blocked relay (Bugbot on Relay origins in Share Connection Report (CodeRabbit on Connection-check task ownership (CodeRabbit on Diagnosed degraded gate (CodeRabbit on Scheduler-dependent test waits (CodeRabbit on Not changed, with reasons:
|
cmux settings open networking (aliases: network, iroh) deep-links the Settings section that hosts the connection check, matching the app-side SettingsNavigationTarget that already existed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Label used as the LabeledContent value inflates the row to several hundred points on iOS 26 Forms (the Active Route row with a plain text value renders compact in the same section). Render the status as a plain HStack of Image + Text instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
setIrohPathPreference and setIrohDebugTransportVerificationMode persist the preference and publish the new snapshot immediately, then restart the Iroh runtime before returning; a cold restart takes 30s+ and can run much longer. Holding isMutating (which disables every Networking control, and on iOS the whole sheet) across that await froze Settings for the full restart. Run restart-flavored mutations without the isMutating gate and reconcile from the settings update stream, surfacing only failures. Regression tests hold the controller call mid-restart and assert the models never report isMutating. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With Transport Mode set to No Relay (Direct Only), relays are excluded by the user's own setting, but the connection check still probed them, failed the relay stage, and told users to ask IT to allowlist relay domains. Both call sites now report an administratively excluded relay as not-configured, so the stage reads Not Needed and a dead direct path recommends retrying instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- The settings snapshot read persisted private paths only through activeAccountID, which is nil while a transport-mode change restarts the runtime, so mid-restart snapshots dropped the configured private address rows from Settings. Fall back to observedAccountID, matching the settings mutations. - Add Private Addresses was permanently disabled once every known Mac had a configuration, with no explanation. It now opens the edit sheet for the existing configuration in that case (adding an address to a configured Mac is an edit), and only disables when no Mac is known. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bfdd481. Configure here.
| if let accountID = observedAccountID ?? activeAccountID { | ||
| privatePathSnapshot = await customPrivatePaths.availableSnapshot( | ||
| accountID: activeAccountID | ||
| accountID: accountID |
There was a problem hiding this comment.
Private path edits fail mid-restart
Medium Severity
irohSettingsSnapshot() now resolves private paths via observedAccountID so configs stay visible during Iroh restarts, but upsertIrohCustomPrivatePath and removeIrohCustomPrivatePath still require activeAccountID, which is cleared for the whole restart. Networking stays interactive in that window, so toggles and edits surface false save failures.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bfdd481. Configure here.
This was referenced Aug 11, 2026
…tion-checker-claude # Conflicts: # Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/CmxIrohRelayOrigin.swift # Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/CmxIrohConnectionCheckReportTests.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohConnectionCheckSection.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohPrivateNetworksSection.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstrings # Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift # Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/IrohConnectionCheckCard.swift # Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/IrohSettingsModelTests.swift # cmuxUITests/SettingsNetworkingBehaviorUITests.swift
This was referenced Aug 17, 2026
lawrencecchen
added
the
stale-revisit
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
3 participants
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.
Summary
Addresses the encrypted Iroh replacement for the topology in #7230 without enabling plaintext LAN fallback.
Verification
swift test --filter CmxIrohConnectionCheckReportTestsswift test --filter CmxIrohCustomRelayProbeTestsswift buildinPackages/macOS/CmuxSettingsUISettingsNetworkingBehaviorUITestsdispatched with videoNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Medium Risk
Touches live Iroh runtime read paths, relay policy interpretation, and networking settings UX on both platforms; mistakes could mis-route users (e.g. false IT allowlist advice) though logic explicitly distinguishes unreachable vs unavailable probes.
Overview
Adds a privacy-safe connection checker to iOS and macOS Iroh/Networking settings so users can see encrypted transport, relay policy, reachability, Mac discovery (mobile), and session readiness with concrete recovery steps—not raw diagnostics.
Core: New
CmxIrohConnectionCheckReportmaps snapshots, diagnostics, and relay probe results into staged statuses and prioritized recommendations (retry, IT allowlist only when a relay was probed and blocked, not when probes are indeterminate).CmxIrohRelayOriginbuilds credential-free HTTPS origins for IT allowlists. Client/host runtimes exposehasReachableRelay(in:)from live endpoint path hints.UI: Manual Check Connection / Run Check, shareable safe reports (relay URLs omitted from reports; allowlist via separate share), and auto-run once when the runtime newly enters a diagnosed degraded state. Path-preference changes no longer freeze the whole settings sheet during long Iroh restarts.
Also: CLI
settings open networkingtarget; iOS private addresses collapsed under Advanced Private Addresses; mobile snapshot fix so private paths survive mid-restart account gaps; localized strings and broad unit/UI tests.Reviewed by Cursor Bugbot for commit bfdd481. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a staged, privacy‑safe Iroh connection checker to iOS and macOS Settings to diagnose encrypted transport and relay readiness with clear recovery steps. Keeps Networking settings responsive during Iroh restarts and stabilizes iOS Private Addresses and Add behavior.
MobileIrohConnectionCheckSectionand macOSIrohConnectionCheckCardshow the active route, stage results, a recommendation, and provide “Share Connection Report” plus “Share IT Allowlist” when relay traffic may be blocked; allowlists use canonical HTTPS origins viaCmxIrohRelayOrigin.CmxIrohConnectionCheckReportmaps snapshots, diagnostics, and relay probes into staged statuses with prioritized recommendations; relay reachability reads live path hints viahasReachableRelay(in:). The shared Connection Report omits relay origins; the IT allowlist remains a separate share.cmux settings open networking(aliasesnetwork,iroh) deep‑links to the checker. Localized (en/ja) with expanded unit, UI, and model tests covering auto‑run gating, coalesced starts, cancellation, and restart behavior.Written for commit 028840f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests