fix: AISandbox vm start routes through GUI app + per-VM spawn logging#6
Merged
Conversation
Two bugs surfaced from ai-mon ("Issue C and D" punch list):
Issue C — foreground subprocess died silently
- vm start (background mode) spawned `secvf-cli vm start <name> --foreground`
with stdio sent to /dev/null, then waited 1s and asked
VMProcessManager.isVMRunning. When VZVirtualMachine.start() failed
(entitlements gap, missing IPSW, malformed bundle), the user only saw
"VM failed to start" with no diagnostic — the subprocess's error was
in /dev/null.
- Fix: redirect the foreground subprocess's stdout+stderr to a per-VM
log under ~/.avf/logs/<name>-start-<timestamp>.log, opened via
open(2) with O_WRONLY|O_CREAT|O_APPEND and 0600 perms. Path is
printed on success and on failure so the user has somewhere to look.
Also extended the post-spawn check from a single 1s wait to a
5s/0.5s polling loop so slow VZ bringup isn't reported as failure.
Issue D — CLI's VMRunner can't produce the exec-bridge socket
- VMRunner.createAISandboxConfiguration adds the vsock socket DEVICE on
the guest side, but the host-side LISTENER (VsockExecBridgeManager)
lives in the GUI app and is only started by AppDelegate.bootAISandboxSession().
A CLI-spawned VM has no /tmp/secvf-exec-<id>.sock, so --wait always
times out for AISandbox VMs.
- Fix: detect osType == "AISandbox" before the spawn branch and route
via DistributedNotificationCenter to com.secvf.cli.start. The GUI
app's existing handleCLIStartVM observer already calls
bootAISandboxSession() for AISandbox VMs, which sets up the bridge.
The CLI subprocess is skipped entirely on this path.
- If SecVF.app isn't running, fail clearly:
"SecVF.app is not running. Open the app first, then re-run …"
- Detection uses NSWorkspace.shared.runningApplications against
{com.ItzDaxxy.SecVF, com.DaxxSec.SecVF} (same legacy/intended ID
pair ISOCacheManager already accepts).
Plumbing:
- Added isSecVFAppRunning() and vmStartLogPath(for:) helpers at file
scope in VMCommand.swift.
- Imported AppKit (NSWorkspace) — available on macOS 14+; no Package.swift
change needed.
Build: clean. CLI swift build + Xcode build both succeed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DaxxSec
pushed a commit
that referenced
this pull request
May 13, 2026
Works through every item from the review's suggestions table. #1 SecVFError: extract `tokenizeHomePath(_:home:)` static helper from logToAudit so the home-redaction logic is independently testable. Test now exercises the production helper directly (was previously re-implementing the replacement and asserting on stdlib API). Added 2 extra cases: leaves non-home paths alone, replaces every occurrence in a single line. #2 VMLibraryWindowController.deleteVM: remove the deleted VM's UUID from `runningFilterIDs` so the focus filter doesn't ghost a phantom entry. Reset to nil if the set goes empty. #3 TacticalTableRowView.drawSeparator: comment claimed a "skip last row" behavior that wasn't actually implemented. Updated the comment to reflect reality (the host NSTableView controls whether drawSeparator is called for the last row via gridStyleMask / intercellSpacing). #4 PacketFilterPresets: add `populateMenu(_:target:action:)` overload that appends preset items onto an existing menu. buildMenu now delegates to it. PacketAnalysisWindowController now calls populateMenu directly on the popup's existing menu, preserving the title item without the allocate-then-copy-every-item dance. #5 VMConnectionOverlayView.draw: documented the latent horizontal- scroller caveat. `bounds.maxX` tracks content width (overlay is a table subview); when the table has no horizontal scroller (today) visible == content so this is fine. Comment now points future maintainers at `clipView.visibleRect.maxX` if/when that changes. #6 AppDelegate.handleFocusVMConsole: switched from Swift string interpolation inside NSLog's format string to printf-style %@ formatting matching the rest of the file (and avoiding format- string concerns). #7 VMManager.networkPeers: added a perf comment documenting the O(routers × n) cost in refreshConnectionOverlay's loop and the `[routerVMId: [guests]]` index hint if the library ever grows into the hundreds of VMs. #8 AppColorsTests.testCyanAliasesPointToODGreen: replaced NSColor instance equality with a component-by-component comparison via a new `assertSameColor` helper. Test now still passes if a future refactor expresses the alias as two literal NSColors with matching components — what matters is the *visible* contract, not Swift-level reference equality. Tolerance: 0.001 per channel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Fixes the two CLI bugs surfaced by ai-mon (the "Issue C and D" punch list).
Issue C — foreground subprocess died silently
Before:
secvf-cli vm start <name>(background mode) spawned a child runningsecvf-cli vm start <name> --foregroundwith stdio sent to/dev/null, then waited 1s and askedVMProcessManager.isVMRunning. WhenVZVirtualMachine.start()failed (entitlements gap, missing IPSW, malformed bundle, etc.), the user saw only `"VM failed to start"` with no diagnostic — the actual error was discarded into/dev/null.After:
stdout+stderrredirected to~/.avf/logs/<name>-start-<timestamp>.log(mode 0600, append-only). Log path is printed on success and on failure so the user has somewhere to look.Issue D — CLI's VMRunner can't produce the exec-bridge socket
Before:
VMRunner.createAISandboxConfigurationadds the vsock socket device on the guest side, but the host-side listener (VsockExecBridgeManager) lives in the GUI app and is only started byAppDelegate.bootAISandboxSession(). So a CLI-spawned AISandbox VM never produces/tmp/secvf-exec-<id>.sock, and--waitalways times out.After: detect
osType == \"AISandbox\"before the spawn branch and route viaDistributedNotificationCentertocom.secvf.cli.start. The GUI app's existinghandleCLIStartVMobserver already callsbootAISandboxSession()for AISandbox VMs, which sets up the bridge. The CLI subprocess is skipped entirely on this path.If
SecVF.appisn't running, the CLI fails clearly with:App-running detection uses
NSWorkspace.shared.runningApplicationsagainst{com.ItzDaxxy.SecVF, com.DaxxSec.SecVF}(same legacy/intended bundle-id pairISOCacheManager.verifyCallerIsMainApphistorically accepted).Plumbing
isSecVFAppRunning()andvmStartLogPath(for:)at file scope inVMCommand.swift.AppKitforNSWorkspace— available on macOS 14+; noPackage.swiftchange required.--waitpolling loop is unchanged; it already checksisExecBridgeAvailablefor AISandbox VMs (with the S12 connect probe from PR fix: PR #4 review — supersession races, BridgeState lifetime, CLI self-spawn + 11 follow-ups #5), which is the right readiness signal oncebootAISandboxSession()has had time to run.Test plan
swift buildinSecVF/cli/— clean (only pre-existing Swift 6 warning in VMRunner.swift)xcodebuild buildfor the main app — cleansecvf-cli vm start <linux-vm>with intentionally broken bundle — confirm log path is reported and contains the actual VZ errorsecvf-cli vm start <ai-sandbox-base> --waitwithSecVF.apprunning — confirm the bridge socket appears and--waitreturns successsecvf-cli vm start <ai-sandbox-base>withSecVF.appNOT running — confirm clear error message🤖 Generated with Claude Code