Repository navigation
mux: release hardening before dev-mux → main (8 fixes) - #512
Merged
Merged
Conversation
MuxDaemonStop.Terminate re-verified the pid by name and start time, but StartTimeMatches answered true when the descriptor had no start time or the process's could not be read. The kill path is now reachable from the GUI restart and the uninstall hook, and the GUI shares the daemon's process name, so a name-only match could kill the wrong ntilde. The kill path now requires a readable, matching start time (requireStartTime: true); the liveness probe keeps its leniency. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
GitHubReleaseMuxAssetSource accepted a download on the served checksum whenever the embedded pin set had no entry for the RID, and MuxAssetPins dropped a malformed pin silently, so a release build missing one pin quietly fell back to trusting the file that sits beside the binary. - With any pins embedded (a release build), a RID without a usable pin is refused with InvalidDataException before the cache or the network is touched. No pins at all (a dev build) keeps the .sha256 fallback. - MuxAssetPins keeps a well-named but unreadable resource as Unusable instead of dropping it. - release.yml checks each checksum is exactly '<64 lowercase hex> ntilde-mux-<rid>' (scripts/ci/check-mux-sha256.sh, self-tested in ci.yml) before embedding, and a workflow test pins that every App build embedding the pins runs it first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
…t is absent The installed app reads the new build's multiplexer protocol range from the staged update's release notes, and a missing marker reads as compatible (R10). Nothing pinned the workflow side. - ReleaseWorkflowTests: every vpk pack passes --releaseNotes artifacts/velopack-release-notes.md, and an earlier step of the same job writes it from scripts/ci/mux-protocol-range.sh. - The runtime stays fail-open, but a release build (one that embeds the ntilde-mux pins) logs a [Warning] line when the staged notes carry no readable marker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
A listing must have no side effects, but ls --all connected through 'ntilde-mux proxy --stdio', which spawns a daemon on demand: every listed host without one got a daemon (idle for 10 minutes). - ntilde-mux: 'proxy --stdio --no-spawn' connects only to a running daemon; with none it starts nothing and exits 5 (MuxProxyExitCodes.NotRunning). - The lister's connector runs that command (ForListing) and reports 'no multiplexer is running' as a fixed-cause row. An ntilde-mux older than the option refuses it with its usage (exit 2), which is reported as 'its ntilde-mux is older than this app; update it to list its sessions' rather than as a failed connection. - The picker's Connect row is a connect and may keep spawning; its text now says so. Reverses R28(a). User manual updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
The proxy points the daemon's stable agent link at its own connection's SSH_AUTH_SOCK on every connect. ls --all connects through it for a few seconds and goes, so with 'ForwardAgent yes' in the user's ssh config every shell in that daemon was left holding a dead agent until a window reconnected. ClearAllForwardings=yes does not clear ForwardAgent. - A listing's OpenSSH exec passes -o ForwardAgent=no ahead of the plan, and drops a -A from the profile's extra arguments (ssh applies -A whatever an earlier -o said); checked against the real ssh -G. The native transport never requests agent forwarding. - The request carries Listing (set by the lister's connector), and the OpenSSH transport exposes NoAgentForwarding. - Belt and braces on the host: a --no-spawn proxy (only listings pass it) leaves an existing agent link untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
… refused A persisted remote tab's SFTP, sidebar listing and path completion read the host's password scope before the vault, but only the mux link's own connect ever removed a refused value, and that link is long-lived and never signs in again. After a server-side rotation every click offered the stale password again, for the host's whole life. - rusty_ssh: a transfer's or listing's refused password is now a structured AuthenticationFailed error, reported with status 'auth-failed' and the same result code as before. - NativeSshInterop turns that status (never the message) into NativeSshAuthenticationRefusedException, an InvalidOperationException as before. - NativeHopPasswords records the host scope the target password came from (not a plain session's own, not the vault, never with jump hops); on a refusal the sidebar listing, path completion and native transfer call RemoveRuntimePassword with the value offered, so a password typed since is kept and a gone host's scope stays gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
ApplySessionPersistenceSetting swaps the factory but leaves the mux panes connected; IsMuxPersistenceActive was then false, so the close neither asked nor ended anything and every open daemon shell was stranded, while the Settings row says 'With Off, a shell ends when its window closes'. When persistence was turned Off during the window's life (the setting is Off, the factory swapped away, the hosts still there), the close now ends the local shells as a remembered 'Close them' does: - shares, and shells another client shows, detach; - shells of tabs not shown yet (pending ids, placeholder trees) end once the daemon says no other client shows them, the close held meanwhile; - a shutdown cannot be held: it ends the live shells only, and PerformAppTeardown ends them on every teardown route before the save. A window Off from the start has no hosts, so nothing changes for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
On macOS Cmd+Q is the normal way to quit, but it reached the window as an ApplicationShutdown close, which applies a remembered answer and otherwise keeps the shells silently: the first-close explanation and 'Don't ask again' were unreachable there (R19). - App subscribes the lifetime's ShutdownRequested on macOS. When the main window would hold a window close (the same decision, run dry), the request is cancelled, the window closes the way its close button does - asked, or a remembered 'Close them' waiting for the daemon - and once that close went through, desktop.Shutdown() quits. Cancel keeps Ntilde running. - A logout, restart or shutdown is never held: Avalonia's IsOSShutdown is internal, so the quit Apple event's 'why?' reason is read through the Objective-C runtime (MacQuitReason). Windows and X11 raise ShutdownRequested only for the session ending, so they are not subscribed. - The held close's posted work is tracked (_heldCloseSettled) so the quit follows it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
…folder When MuxDaemonImage.Resolve cannot stage the daemon's own copy on a Windows Velopack install, the daemon runs from the install folder: every update then stops it and its shells, and every startup update is held. That was only in debug.log. The real resolver now marks the fallback, and the window shows a fixed-text notice once per launch when the local daemon connects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ly when it can be shown - check-mux-sha256.sh pinned only that one line matched, which Git Bash's grep (it drops a CR) and a trailing unterminated line both slipped past: the Windows CI leg failed its CRLF case. The file's byte length is now pinned to the line plus at most one LF. Greptile on PR 512. - The install-folder notice was taken before the posted callback checked the teardown, so a window closing first lost it for the launch. It is taken inside the callback now. Greptile on PR 512. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
…xception The Docker E2E NativeSftp_BadPassword_ReturnsAuthenticationFailed asserted the exact type InvalidOperationException; release hardening item 5 raises its new subclass for the native auth-failed status. The test now pins the specific type, and that it is still an InvalidOperationException to other callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
|
Owner
Author
|
Generated by Claude Code |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
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.
What this changes
Eight defects found reviewing the merged
dev-muxtree (ee75a32), fixed one commit per item. Each has a test that failed first. There are no new settings and no new verbs.MuxProtocolstays 1–2. The rulings are recorded as H1–H8 at the end ofdocs/superpowers/specs/2026-10-08-ntilde-mux-phase5.md.MuxDaemonStop.Terminate) needs a recorded start time that matches the process's readable one. The liveness probe stays lenient.MuxDaemonStopTests.A_descriptor_without_a_start_time_is_not_terminated(failed). The differing-start-time case already held and is now pinned..sha256against^[0-9a-f]{64} ntilde-mux-<rid>$(scripts/ci/check-mux-sha256.sh, self-tested in ci.yml).A_release_build_without_a_pin_for_the_rid_refuses_the_download(replacesA_pin_for_another_rid_changes_nothing),…_refuses_a_self_consistent_cache_too,An_unusable_embedded_pin_refuses_the_download,MuxAssetPinsTests.Foreign_resources_are_ignored_and_a_malformed_pin_is_kept_as_unusable,ReleaseWorkflowTests.Every_app_build_that_embeds_the_ntilde_mux_pins_checks_their_format_first,scripts/tests/mux_sha256_check_tests.py. All failed first.vpk packpasses--releaseNotes artifacts/velopack-release-notes.md, and an earlier step writes it frommux-protocol-range.sh. The runtime stays fail-open, but a release build logs[Warning]when the marker is missing.ReleaseWorkflowTests.Every_vpk_pack_ships_the_multiplexer_protocol_marker(mutation-checked: fails when one--releaseNotesis dropped or the notes step stops running the script),MuxUpdateCompatibilityTests.A_release_build_warns_when_the_marker_is_missingproxy --stdio --no-spawnexits 5 (NotRunning) and starts nothing.ls --alluses it and shows "no multiplexer is running". An older ntilde-mux rejects the option with its usage error (exit 2), which is shown as "update it". The picker's Connect row says it starts the multiplexer. Reverses R28(a).MuxProxyTests.A_no_spawn_proxy_with_no_daemon_exits_not_running_and_starts_nothing,Proxy_parses_stdio_and_an_optional_no_spawn,MuxCommandLsAllTests.A_host_with_no_multiplexer_running_says_so_and_none_is_started,…predates_no_spawn…,MuxSessionPickerTests.A_connect_row_offers_to_connect_to_its_host. All failed first.-o ForwardAgent=noand drops-A. Native never forwards an agent. A--no-spawnproxy also leaves the agent link alone.MuxProxyTests.A_listing_proxy_leaves_an_existing_agent_link_untouched,OpenSshExecCommandLineTests.A_listing_*,RemoteMuxHostFactoryTests.An_OpenSSH_listing_forwards_no_agent…,MuxCommandLsAllTests.Every_listing_transport_is_asked_to_forward_no_agent. All failed first. Plus a realssh -Gcheck (A_listings_ssh_forwards_no_agent_whatever_the_config_and_the_extra_arguments_say).auth-failed(same result code), which becomesNativeSshAuthenticationRefusedException. The sidebar listing, path completion and native transfer then compare-and-remove the target password they took from the host scope.RemoteDirectoryBrowserServiceTests.A_refused_listing_drops_the_scope_password_it_offered(failed),A_late_refusal_of_the_old_password_does_not_clear_a_newly_typed_one,SftpServiceTests.A_refused_transfer_drops_…(false)(failed), Rustclassify_sftp_transfer_error_marks_a_refused_password_auth_failed(failed: did not compile without the variant)MainWindowMuxLifecycleTests.Turning_persistence_off_then_closing_ends_the_open_shells,MainWindowFirstCloseTests.With_persistence_turned_off_*(5 cases). All failed first.ShutdownRequestedis cancelled when the window would hold a close. The close is settled first, thendesktop.Shutdown()runs. Logout, restart and shutdown are told apart by the quit Apple event's'why?', because Avalonia'sIsOSShutdownis internal.MainWindowFirstCloseTests.Cmd_Q_*covers Keep (± Don't ask again), Close them, Cancel, a remembered answer and a remembered close with pending tabs. 5 failed against the stub. AlsoAn_OS_shutdown_request_is_never_heldandMacQuitReasonTests.Optional items:
sessionInfo, andbusyfor a second in-flightreadScreen. Both would add a protocol field or error code, which the brief rules out.Invariant and ownership
Tests
global.jsonwas relaxed locally and not committed.)scripts/build.sh test): each project was run separately instead:Category=ReplayCategory=RenderMetricsCategory=PtySmoketests/Ntilde.App.Tests, run in two lanes:Lane!=PlatformBoot: 6171 passed, 69 skippedLane=PlatformBoot: 59 passed, 4 skippedcargo test --release(133 + 5 passed), and thescripts/tests/*.pyscript tests (pass;aot_smoke_verdict_tests.pyneeds pwsh, which is not installed here).dotnet format whitespace --verify-no-changesflags onlysrc/Ntilde.VT/TerminalBuffer.WritePath.cs, which this PR does not touch. The 10.0.112 formatter may differ from the pinned SDK's.Impact
Does this affect cross-platform behavior? Yes. Tested on Linux only. Item 2 still needs the manual macOS check:
The
objc_msgSendquit-reason read is not exercised on a Mac here; its macOS test only checks that it does not crash.Does this change renderer metrics? No.
Does this change VT coverage? No.
🤖 Generated with Claude Code
https://claude.ai/code/session_019RmDfMhhR8MKG4ejF3h6QQ
Generated by Claude Code