feat: macOS GUI Phase 10+11 — diagnostics, panic, install/uninstall, VPN config panel#16
Merged
Conversation
Closes the CLI↔GUI gap identified after Phase 8: the menubar app only exposed Start/Stop/Block/Unblock, leaving Panic, install/uninstall, diagnostics, and VPN-guard config editing CLI-only. Phase 9 (diagnostics, output capture, safety controls): - runPrivileged now returns captured stdout/stderr instead of a bare Bool, so failures show real output instead of silence - Panic (confirmation-gated), Install/Uninstall service, About panel, and a View logs submenu filtered to the unified log - One shared OutputPanel window reused by every diagnostic/action Phase 10 (VPN guard config panel): - Closes the Phase 8 scope note deferring in-app VPN-mode toggling - Drives the CLI's existing config get/set/validate per dotted key — no new Go command, no second config schema mirrored in Swift - Orders field writes (other fields before enabling, vpn.enabled first when disabling) since config set validates the whole config on every write, not just the touched field - Discloses the stop/start restart's fail-open window explicitly via a warning modal rather than hiding it; polls status --json after restart instead of assuming success Zero Go changes; go build/vet/test and swift build (debug + release) all pass. Planned via independent draft + adversarial review before implementation (see docs/plans/phase-9-gui-diagnostics.md and phase-10-gui-vpn-config.md for full design rationale and the still-open manual/root verification checklist). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017o9YaEjft1DgS62KQN4UaS
Behnam-RK
force-pushed
the
feat/macos-gui-phase-9-10
branch
from
d4f9115 to
5b1e5c8
Compare
Behnam-RK
changed the title
There was a problem hiding this comment.
Pull request overview
Extends the macOS menubar GUI to cover additional CLI functionality (diagnostics, panic, install/uninstall, about, scoped logs) and adds an in-app VPN guard configuration panel that reads/writes the existing dotted vpn.* config keys via the CLI, including an explicit restart warning and bounded post-restart status polling.
Changes:
- Added a shared reusable output window (
OutputPanel) and streaming process support forlog stream. - Enhanced CLI invocation plumbing (
DezhbanCLI) to return captured output and added log helpers + service-installed detection. - Added a Phase 11 VPN config panel that seeds from
config getand applies changes via ordered privilegedconfig set+ validate + restart flow.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| macos-gui/Sources/DezhbanMenu/VPNConfigPanel.swift | New VPN guard config UI that seeds via config get and applies via ordered config set + validate + restart + status polling. |
| macos-gui/Sources/DezhbanMenu/OutputPanel.swift | New shared output window for run-to-completion output and streaming logs with a Stop button. |
| macos-gui/Sources/DezhbanMenu/DezhbanCLI.swift | Introduces CommandResult, improves privileged output capture, adds unprivileged run, service detection, and log command helpers + streaming process. |
| macos-gui/Sources/DezhbanMenu/AppDelegate.swift | Wires new menu items (diagnostics/panic/install/uninstall/about/logs) and opens the VPN config panel; improves failure alerts with captured output. |
| docs/plans/readme.md | Updates phase index/status text and adds Phase 10/11 entries. |
| docs/plans/phase-10-gui-diagnostics.md | Adds the Phase 10 plan document describing diagnostics/output capture/logs/etc. |
| docs/plans/phase-11-gui-vpn-config.md | Adds the Phase 11 plan document describing the VPN config panel approach and restart-window disclosure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- runPrivileged: fold stderr into stdout via 2>&1 so a command that writes to stderr while exiting 0 still surfaces its output - OutputPanel.append: append into textStorage instead of `string +=` (O(n^2) over long log streams); carry the view font to keep styling - VPNConfigPanel.seedFields: capture each `config get` result, short-circuit on failure, keep Apply disabled, surface the error instead of seeding error text back into the config - docs/plans/readme.md: mark phases 10-11 as complete Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- exec: drain stdout/stderr concurrently via a DispatchGroup, so a child that fills its stderr pipe buffer can't deadlock the GUI background thread - VPNConfigPanel apply: resolve the config path once and pass --config to every `config set` (matching the final `validate`), so all writes and the validation provably target the same file - AppDelegate log menu: stop gating "Show last hour"/"Stream live…" on the dezhban binary — they run /usr/bin/log and stay useful when the CLI is uninstalled/mislocated - phase-11 doc: correct the seeding description to per-key `config get` (not `config show`), matching the implementation Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- VPNConfigPanel.seedFields: read via the same resolved --config path Apply writes/validates with, so a nonstandard path can't seed one file and apply to another - exec: drop the two-writer concurrent drain for a single background stderr reader ordered by a semaphore — no shared mutable state, still deadlock-free - AppDelegate: cache serviceInstalled() and refresh it off the main thread (launch, post install/uninstall, each menu open) so opening the menu never blocks on `status --json` - streamLogs: on start failure, revert the panel to non-streaming so the Stop button and onStop handler don't linger with no process Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- VPNConfigPanel: keep Apply disabled through the restart (only re-enabled on cancel or when the restart finishes), so a second config-write/restart can't run concurrently; re-enable Apply in the restart-failure and completion paths so the panel is never left stuck - About panel: split the misleading "Service status" (which showed the enforcement posture) into distinct "Posture" and "Service" (installed?) lines - docs/plans/readme.md: stop claiming phases 10-11 are merged to main — they're complete on this branch, pending merge - phase-11 doc: reference AppDelegate.openConfig / "Open config file…" by name instead of a fragile line number Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- Add DezhbanCLI.reportedPosture(): parse `status --json` from stdout only (via exec, like serviceInstalled) instead of run's combined stdout+stderr, so a warning on stderr with a 0 exit can't corrupt the JSON parse. Use it in the post-restart poll. - performRestart: document why stop/start take no --config — they act on the already-installed service unit whose config path is baked at install time (cmdService reads --config only for `install`); the service manager ignores --config on start/stop, and the GUI installs with resolvedConfigPath(), the same path the panel writes to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- resolvedConfigPath: stop splitting `config path` output on spaces, which truncated valid paths containing spaces (e.g. ~/Library/Application Support/…). Take the single line and strip only the optional trailing " (not present …)" parenthetical note. - VPNConfigPanel.seedFields: reuse the already-resolved cfgPath in the "Seeded from …" status text instead of calling resolvedConfigPath() a second time (extra subprocess; could display a path different from the one actually seeded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- runPrivileged: fix a regression from the earlier `2>&1` change. `do shell script` reports a non-zero exit as an AppleScript error whose message is the command's stderr, so folding stderr into stdout erased failure diagnostics (a bare "error code N" for `config set` validation failures). Now capture combined output in a shell var and print it to stdout on success / stderr + re-exit non-zero on failure, so both outcomes surface real output. - StreamingProcess.start: decode with String(decoding:as:) (never nil) instead of String(data:encoding:), which dropped whole chunks when a multi-byte UTF-8 sequence split across availableData reads; clear the readabilityHandler on EOF so it doesn't dangle. - VPNConfigPanel apply transcript: quote value/cfgPath via String(reflecting:) so the log stays unambiguous and copy/paste-runnable when they contain spaces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- resolvedConfigPath: strip only the exact fixed Go note suffix
" (not present — using built-in defaults)" instead of cutting at the last
" (", so a real config path that itself ends in ")" (a directory with
parentheses) is no longer truncated.
- looksLikeGoDuration: make the superficial pre-check permissive enough to
accept everything time.ParseDuration does — optional sign and the bare "0"
were previously rejected (e.g. "0", "-1s", "+5m"). config set stays the
authority; the pre-check only needs to avoid rejecting valid input.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
- looksLikeGoDuration: require a numeric prefix per chunk (a digit before or after the dot) so bare units like "s"/"ms" are rejected again, while still accepting every valid form (signed, bare "0", ".5s", "5.s"). Balances the previous round's over-permissive fix without regressing valid input. - VPNConfigPanel.seedFields: clear all controls before loading, so a failed seed can't leave stale values from a previous successful open on screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6ZjJH6UqiJQAYre5tNSuC
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
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
Closes the CLI↔GUI gap identified after Phase 8: the menubar app previously only exposed Start/Stop/Block/Unblock, leaving several essential CLI features reachable only from the terminal.
Phase 10 — diagnostics, output capture, safety controls (docs/plans/phase-10-gui-diagnostics.md):
runPrivilegednow captures and returns real stdout/stderr instead of a bareBool— failures show what actually happened instead of silencestatus --json), an About panel, and a View-logs submenu scoped to the unified log (process == "dezhban")OutputPanelwindow reused by every diagnostic/actionPhase 11 — VPN guard config panel (docs/plans/phase-11-gui-vpn-config.md):
config get/set/validateper dotted key — no new Go command, no second config schema mirrored in Swiftvpn.enabledlast when enabling; reversed when disabling), sinceconfig setvalidates the entire config on every write, not just the touched fieldstop/startrestart's fail-open window explicitly via a warning modal rather than hiding it, and pollsstatus --jsonafter restart instead of assuming successZero Go changes —
go build/go vet/go test(93 tests, 11 packages) all pass unmodified.swift build,swift build -c release, andmake gui-macosall succeed and producedist/Dezhban.app.This was planned via an independent draft + adversarial review pass before implementation (see the two phase docs for full design rationale, rejected alternatives, and the risks/open-questions sections).
Test plan
Automated (done):
go build ./... && go vet ./... && go test ./...swift buildandswift build -c releasemake gui-macosproducesdist/Dezhban.appManual (needs a real macOS host with root — not exercisable in CI/sandbox, see each phase doc's
## Acceptance / verificationsection):status --json's registered flag correctlydoctor/validate/print-rules/monitor --oncedezhban config show; enabling with valid endpoints round-trips through restart (icon ⚪ during the gap, then 🟢/🔴); a deliberately-bad endpoint is refused before any restart; a simulatedstartfailure is reported as failure, not false successlogcommands; no orphanedlog streamprocess after closing the window🤖 Generated with Claude Code
https://claude.ai/code/session_017o9YaEjft1DgS62KQN4UaS