Repository navigation
cmux-next Cloud Tunnel: re-land with the NWPathMonitor path watch and an embed that only ships with the tunnel - #34262
lawrencecchen wants to merge 13 commits into
Conversation
…-red revert #34198) The twelve commits #34198 reverted, unchanged (original work by lawrencecchen, direct push ad2c767..cc33e08). The next commits fix the two defects that made the revert necessary: the packet tunnel's path watch (defaultPath is not in the macOS 26.5 SDK) and a Release build that embedded the extension and then deleted it before Xcode validated it.
CloudTunnelPathChangeGate holds the rule the extension applied inline: the first path is the current one, so only a later path that differs from the one before it is a real change. Generic over the path, so the tests need no tunnel.
… (defaultPath is not in the macOS 26.5 SDK) The full Release app build (fleet job a565ad1d55c788476e604c39, feat 7fd3931, Xcode 26.6, MacOSX26.5 SDK) fails: CloudTunnelExtension/PacketTunnelProvider.swift:50:35: error: value of type 'PacketTunnelProvider' has no member 'defaultPath' 010ffb9 observed NEProvider's defaultPath through KVO. This SDK does not offer it to Swift; Network.framework's NWPathMonitor is the documented replacement. The provider now runs one NWPathMonitor on its own serial utility queue (not the main thread), started where the KVO observation was and cancelled on stop. The same rule holds through CloudTunnelPathChangeGate: the monitor's first path is the current one, so only a real change after it calls engine.networkChanged(). Fixes the build break from 010ffb9 (lawrencecchen).
…g call in its closure does not compile); the extension names Network.NWPath
…t in a Copy Files phase (tests/test_embed_cloud_tunnel.sh) Red at the re-land: the project's Embed System Extensions phase copies the extension into every build and embed-cloud-tunnel.sh then deletes it in a Release build without CMUX_CLOUD_TUNNEL=YES, so Xcode's ValidateEmbeddedBinary fails (fleet job 10b8f087c165db1fb4ad92df: "Couldn't load Info dictionary for ... cmux-cloud-tunnel.systemextension"). The test also needs a shipping build to embed the built extension, which only the Copy Files phase did so far.
…loud-tunnel.sh, only when it ships The app target had an unconditional Embed System Extensions copy phase, and embed-cloud-tunnel.sh deleted its result in every Release build without CMUX_CLOUD_TUNNEL=YES. Xcode validates each bundle a Copy Files phase embeds after the script phases, so every such Release build failed in ValidateEmbeddedBinary. The copy phase is gone. The script phase (now "Embed, stamp and check Cloud Tunnel", with the built extension as an input) copies $BUILT_PRODUCTS_DIR/cmux-cloud-tunnel.systemextension into the app only in a Debug build or a Release build with CMUX_CLOUD_TUNNEL=YES, then stamps, signs and checks it as before. It first removes a previous build's copy. The target dependency stays, so the extension still builds (and compiles) in every build. tests/test_embed_cloud_tunnel.sh: red before, green now.
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Passes: CI passes on CI passes on Written by web and size checks: green Default changesThis pull request changes 1 default at
Every change is a new setting that defaults to off, the behavior from before it existed. Regenerate the snapshot with |
|
Found 3 test failures on Blacksmith runners: Failures
|
|
For awareness: #34271 removes the stale |
|
Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over. This re-land also repairs a current base build failure affecting #27490: #34198 removed |
|
Fleet evidence at the head 7a93ec1:
|
… menu exemption, the copy-files section) The independent review of #34262 found three reds that the re-land causes, two of them already in the original push: - ActionContractTests.everyActionHasAUniqueNounVerbCLIName: `cloud tunnel on|off|status` had three words. The verbs take the house style the other tunnel verbs use (`cloud tunnel-attach`): `cloud tunnel-on`, `cloud tunnel-off`, `cloud tunnel-status`, in the catalog, the cmux-tui Cloud help (and its tests), the doc comments, the refusal string (the command token only, in every language) and the design doc. The three-word help branch in app_scopes.rs is gone; nothing needs it now. The tunnel never shipped, so no command a person used changes. - ActionSurfaceParityTests.everyActionDeclaresEverySurface: the three tunnel actions had no context-menu decision. They are app-wide, so they are .noObject. - tests/test_ci_nightly_push_throttle.py: nightly_build_inputs.py needs the project's PBXCopyFilesBuildPhase section (empty before the push), and it cannot read a phase name with a comma. The empty section is back, and the phase is "Embed and check Cloud Tunnel". The generated action contracts follow from the regeneration step.
…nnel-reland-v2 # Conflicts: # cmux-tui/crates/cmux-tui/src/cli/cloud_help.rs # cmux-tui/crates/cmux-tui/src/cli/cloud_help/tests.rs
…o longer offers) (#34505) 3d978ec (lawrencecchen, direct push 00:29Z) added cloud help rows for `tunnel on`, `tunnel off` and `tunnel status`. The Cloud Tunnel actions were reverted in #34198, and the base autofix ca66a4c (22:54Z) had already removed them from plans/cmux-next/action-surfaces.json. So at the tip cli::cloud_help::tests::help_lists_exactly_the_offered_cloud_verbs fails: "cloud help lists verbs the catalog no longer offers: [tunnel off, tunnel on, tunnel status]" (fleet step fcedf354570a0ebf13c21dcd at 4f13a23). The re-land #34262 brings the tunnel verbs back as `tunnel-on`, `tunnel-off` and `tunnel-status`, with their help rows, at the same time as the actions.
…or output, applied by pr-autofix-apply.sh) The PR autofix run 41750611565 regenerated these on a Mac for 785d6cb but did not push them (its push job was skipped). This commit is that run's pr-autofix-patch, applied through scripts/cmux-next/pr-autofix-apply.sh with --changed. The script dropped schemas/defaults/cmux-next.json (bot-only) and refused the three web/data/next-docs files (not PR autofix outputs; the base autofix writes them). No hand edit. action-surfaces.json and actions.md now offer cloud tunnel-on, tunnel-off and tunnel-status, which fixes the cloud help test at this branch. The settings schema and docs/mdm list cloud.tunnel.enabled again. The share invite row's context_menu (familyMember to offered) is the generator's view of the current catalog, not a change of this PR.
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
…-tunnel-reland-v2 # Conflicts: # cmux-tui/crates/cmux-tui/src/cli/cloud_help.rs # cmux-tui/crates/cmux-tui/src/cli/cloud_help/tests.rs
|
Fleet evidence at the exact head db6efa9 (after merging feat at 6767426 with scripts/merge-main.sh):
|
…d strings, nightly step coverage) The independent review of #34262 at db6efa9 found three reds this PR made: - generated drift: 14ac915 committed Packages/macOS/CmuxNext/ci-target-graph.json, which only the base autofix writes (scripts/cmux-next/generated_outputs.py). It is back to feat's copy; the base autofix regenerates it after the merge. - localization debt ratchet: cloudTunnel.title, settings.cloud.tunnel.enabled and settings.group.cloudTunnel are "Cloud Tunnel" in every language, the product name. They are recorded as brand literals in scripts/localization-allowed-omissions.json, as DEV, Claude and Codex are. - nightly step coverage: the re-landed nightly step "Prepare the Cloud Tunnel extension" gets back its .github/nightly-step-coverage.json entry, which #34271 removed after the revert ("A re-land of the step re-adds the entry"). Checked here: localization_catalog.py check against the feat base is within the debt baseline, tests/test_nightly_step_coverage.py is OK.
…-tunnel-reland-v2
…ia pr-autofix-apply.sh) PR autofix run 41766647218 regenerated plans/cmux-next/action-surfaces.json and actions.md on a Mac for d3f32d0 and did not push (its push job skipped). Applied through scripts/cmux-next/pr-autofix-apply.sh --changed; it dropped the bot-only defaults snapshot and refused web/data/next-docs, and ci-target-graph.json stays the base copy (bot-only). Against feat a4f11cd the exports now only add the three Cloud Tunnel actions (cloud tunnel-on|tunnel-off|tunnel-status). No hand edit.
|
Review (claude-opus-reviewer): SHIP at a8e2d43 Independent review. The three earlier reds are fixed:
The action exports against a4f11cd only add the 3 tunnel actions (1346 to 1349 ids, none lost), and they are byte-identical to PR autofix run 41766647218's output. The engine parses wg-quick only through cmux-wg's WgConfig. The PR adds no new Swift parser; CloudTunnelConfiguration.swift is unchanged from cc33e08, which is a known follow-up. Also checked: cloud help lists exactly the 62 offered cloud verbs; there are no conflict markers; check-pbxproj passes; the throttle, embed and sign-gate shell tests pass; the prepush gate finds no new violations. Base finding, not from this PR: tooling-tests Follow-ups, not reds: get the network-settings fields from WgConfig over FFI and delete the Swift subset parser; move the action.cloudSystemTunnel* titles out of the action.agent.share object in ActionCatalog.xcstrings (this was already in the original push). |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
This re-lands the Cloud Tunnel that the base-red revert #34198 took out, with the defects that made the revert necessary fixed. It replaces #34204: that branch was cut before #34198 merged, and moving it onto the new base needs a force push.
Takes back #34198: the revert was the base-red step; this is the re-land with the fixes, as the team rule asks (revert first, re-land with the fix second).
Commits:
435c9fe475d: the twelve commits base red: revert 010ffb95c155 (Cloud Tunnel extension does not compile on the macOS 26.5 SDK) #34198 reverted, unchanged. Original work by lawrencecchen (direct pushad2c76731f28..cc33e089376e).0edd2750e1f,0c7640fade4,5858add9b28: the packet tunnel observes the network path withNWPathMonitoron its own utility queue (not the main thread), becauseNEProvider.defaultPathis not in the macOS 26.5 SDK. Same rule as before: the monitor's first path is the current one, so only a real change after it callsengine.networkChanged(). The rule isCloudTunnelPathChangeGatewithCloudTunnelPathChangeGateTests. The extension writesNetwork.NWPath(NetworkExtension has a legacyNWPath).e50417298c6(red), then the fix: the extension enters the app only fromembed-cloud-tunnel.sh, and only when it ships. Before, an unconditional Embed System Extensions copy phase embedded it in every build, the script deleted it in a Release build withoutCMUX_CLOUD_TUNNEL=YES, and Xcode'sValidateEmbeddedBinarythen failed ("Couldn't load Info dictionary for ... cmux-cloud-tunnel.systemextension", fleet job10b8f087c165db1fb4ad92df). The copy phase is gone. The script phase copies$BUILT_PRODUCTS_DIR/cmux-cloud-tunnel.systemextensionin a Debug build or a Release build withCMUX_CLOUD_TUNNEL=YES, then stamps, signs and checks it as before. The target dependency stays, so the extension still compiles in every build.The third defect of the push (4
indexing_slicingsites incmux-wg-tun) was fixed in #34220, already on the base. Thecmux-wg-tunpid-dependent utun test is #34248.Changelog
none
Testing
tests/test_embed_cloud_tunnel.sh: red ate50417298c6with 3 failures (shipping and Debug builds embed nothing, and the project still has the Copy Files embed); green at the head. Run here (shell only, no build).python3 scripts/cmux-next/prepush-gate.py: no new violations.Observed / Not run
CMUX_CLOUD_TUNNEL=YESand a tunnel profile (that is the nightly channel's signing path), and a live tunnel on a Mac.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Re-lands the Cloud Tunnel system extension reverted in #34198, fixing the two defects that caused the revert: the packet tunnel now watches network path changes with
NWPathMonitorinstead ofNEProvider.defaultPath(missing from the macOS 26.5 SDK), and the extension is embedded only when a build actually ships it, removing the unconditional Copy Files phase that brokeValidateEmbeddedBinary.The independent reviews' reds are fixed as well: the CLI verbs take the house style (
cloud tunnel-on,cloud tunnel-off,cloud tunnel-status) instead of the three-wordcloud tunnel on|off|status, the three tunnel actions got their context-menu exemption, the project's copy-files section is back so the nightly build parser works, the generatedci-target-graph.jsondrift is reverted, and the "Cloud Tunnel" brand strings are recorded in the localization omission file. The nightly step coverage entry for the tunnel prep step is restored, and the action catalog, settings schema, and MDM docs regenerated from these changes.Bug Fixes
PacketTunnelProviderrunsNWPathMonitoron its own serial queue; the first path is the current one, so only real changes callengine.networkChanged().embed-cloud-tunnel.sh: it copies it into Debug builds and Release builds withCMUX_CLOUD_TUNNEL=YES, and other Release builds never embed it.Written for commit a8e2d43. Summary will update on new commits.