Fix persistent mode to use CliUrl instead of spawning child process#118
Merged
Fix persistent mode to use CliUrl instead of spawning child process#118
Conversation
- CopilotService.CreateClient() now branches on Persistent vs Embedded mode: - Persistent: clears CliPath/UseStdio, sets CliUrl for TCP connection to headless server - Embedded: resolves CLI path via bundled/system fallback chain - ServerManager.FindCopilotBinary() falls back to bundled binary when no global install - Removed temporary ResumeTrace debug logging Tests (47 new, 300 total): - PersistentModeTests: SDK mutual exclusivity, option building for all modes - CliPathResolutionTests: bundled binary existence, SDK defaults, fallback chain - InitializationModeTests: mode branching, CliUrl format, fallback scenarios - WsBridgeServerAuthTests: password/token auth contract, backward compat - ConnectionSettingsTests: ServerPassword, DirectSharingEnabled, CliSource fields
PureWeen
added a commit
that referenced
this pull request
Feb 16, 2026
* CopilotService: catch StartAsync failures and set NeedsConfiguration; return early to avoid later 'Service not initialized'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix: dispose failed client and null reference after StartAsync failure When StartAsync fails (e.g., persistent server down + no system copilot binary), the catch blocks now dispose the failed CopilotClient and set _client = null. This prevents a stale broken client reference that would cause NullReferenceException on subsequent operations, and ensures the 'Service not initialized' guard checks work correctly. Also fixes comment indentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add test proving StartAsync throws on unreachable server Validates the premise of the InitializeAsync/ReconnectAsync try/catch fix: CopilotClient.StartAsync() throws when connecting to a non-existent server, confirming the error handling path is defending against a real failure mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract interfaces for CopilotService dependencies and add integration tests - Extract IChatDatabase, IServerManager, IWsBridgeClient, IDemoService interfaces - Concrete classes implement their respective interfaces - CopilotService constructor accepts interfaces (internal ctor for test injection) - DI registration forwards interfaces to concrete singletons - Add TestStubs.cs with stub implementations for all interfaces - Add CopilotServiceInitializationTests.cs with 15 integration tests covering: - Pre-initialization guard checks (CreateSession/ResumeSession throw) - Demo mode initialization and session creation - Embedded mode failure → NeedsConfiguration - Persistent mode failure → NeedsConfiguration - Mode transitions (Demo → Embedded) - OnStateChanged event firing - Session cleanup on reconnect All 316 tests pass. MAUI build succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add missing debug log in ReconnectAsync, suppress CS0067 warnings in stubs - Add Debug log message in ReconnectAsync catch block to match InitializeAsync pattern - Add #pragma warning disable/restore CS0067 around stub classes with unused events Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove stray TestSdk project and null.dll Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix session loss during mode switches (Embedded↔Persistent) Root cause: SaveActiveSessionsToDisk() overwrites active-sessions.json with only the currently in-memory sessions. During mode switches, sessions that fail to restore (or haven't been restored yet) were permanently lost from the file. If the app was killed mid-restore, the same data loss occurred. Fix: SaveActiveSessionsToDisk now merges — it reads the existing file and preserves entries whose session directory still exists on disk, even if they aren't currently loaded in memory. This makes the operation safe to call at any point during startup, restore, or mode switching. Additional changes: - Skip SaveActiveSessionsToDisk during restore (IsRestoring guard) to avoid unnecessary disk I/O per session - Skip SessionStartEvent save during restore for same reason - Track explicitly closed session IDs so merge doesn't re-add them - Add LoadOrganization + ReconcileOrganization in ReconnectAsync (was missing vs InitializeAsync) - Add detailed debug logging in RestorePreviousSessionsAsync - Add 7 mode-switch integration tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix mode persistence, add fallback notice, and bug report button - SetMode() in Settings.razor now calls settings.Save() immediately so the user's mode choice persists even if the app is killed before clicking Save & Reconnect (fixes mode reverting to Embedded on restart) - Add FallbackNotice property to CopilotService — when persistent server can't start and we fall back to Embedded, a dismissable warning banner is shown on the Dashboard with a link to Settings - Add 'Report Bug' button in the sidebar footer that expands inline to collect a description and auto-attach debug info (mode, session count, platform, crash log tail), then submits via gh CLI to PureWeen/PolyPilot - Remove duplicate UiState/ActiveSessionEntry classes from test project (now compiled in via Compile Include from CopilotService.cs) - Add tests for FallbackNotice, ClearFallbackNotice, mode persistence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add CLI source/mode persistence tests, scenarios, and update repo docs - Add 7 new unit tests: CliSource default, switching, independence from mode, serialization round-trip, mode switch preserving CliSource, ResolveBundledCliPath safety, and unique scenario IDs validation - Add 3 new UI scenarios: CLI source switch persistence, mode persists without Save & Reconnect, bug report button visibility - Add scenario cross-reference tests for new scenarios - Update copilot-instructions.md: - Fix default mode (Persistent, not Embedded) for desktop - Document mode switching & merge-based session persistence - Document test safety rules (never call Save/Load, use Demo mode) - Add new test files to coverage section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Split sidebar footer into Report Bug and Fix/Feature buttons Report Bug: creates a GitHub issue via gh CLI with debug info attached. Fix/Feature: creates a worktree on a new branch, writes a prompt file with the bug description + debug info + coding conventions, then launches copilot CLI in a new Terminal.app window pointed at the worktree. The prompt instructs copilot to fix the issue, add tests and scenarios, then create a PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add settings resilience instruction to copilot prompt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix terminal launch: write scripts to temp files to avoid shell quoting issues The osascript -e '...' approach with UseShellExecute mangled the quotes. Now writes both the shell script and AppleScript to temp files, which osascript reads directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix terminal launch: use 'open -a Terminal' to avoid sandbox inheritance osascript-spawned Terminal inherits the MAUI app's sandbox restrictions, causing 'Permission denied' errors for the copilot CLI. Using 'open -a Terminal' launches Terminal.app as a fully independent process with no sandbox. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add --yolo flag to copilot launch for non-interactive permission grants Without --yolo, copilot CLI tries to prompt for directory/tool permissions and fails with 'Permission denied and could not request permission from user' when launched non-interactively in a new worktree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add inline 'Reconnect Now' hint when transport mode changes Shows a yellow hint bar below the mode cards when the user switches between Embedded/Persistent, with a Reconnect Now button. Hint disappears after successful reconnect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make fix-it copilot launch interactive with --resume Adds --resume flag so copilot stays interactive after processing the initial prompt. User can follow up, provide more context, or guide the fix directly in the terminal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use -i (interactive) flag instead of -p for copilot launch -p is non-interactive and exits after completion. -i starts interactive mode, executes the prompt, then keeps the session open for follow-up conversation in the terminal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.
Problem
In Persistent mode,
CreateClient()always setCliPathon the SDK options, causing the SDK to spawn a child copilot process via stdio — identical to Embedded mode. This defeated the purpose of Persistent mode (surviving app restarts via a headless server).The root cause:
CopilotClientOptionsauto-discovers a bundledCliPathand setsUseStdio=truein its constructor. SettingCliUrlwithout first clearing these throwsArgumentException(they're mutually exclusive).Fix
CreateClient()now properly branches:CliPath=null,UseStdio=false,AutoStart=false, then setsCliUrl=http://{host}:{port}to connect to the existing headless server via TCPServerManager.FindCopilotBinary()now falls back to the bundled binary when no global copilot install exists.Compatibility with Redth's PRs
Reviewed PRs #58, #66, #78 — our changes are a deliberate improvement over PR #58's workaround (which made Persistent identical to Embedded due to CliUrl protocol issues). We fixed the root cause by properly clearing SDK constructor defaults. PRs #66 and #78 (direct sharing, auto-start) are unaffected.
Tests (47 new, 300 total)
PersistentModeTests.csCliPathResolutionTests.csInitializationModeTests.csWsBridgeServerAuthTests.csConnectionSettingsTests.csAll 300 tests pass with PolyPilot running (no port conflicts).