Skip to content

fix: stabilize Windows zellij spawns#328

Open
Vaibhaav-Tiwari wants to merge 3 commits into
mainfrom
ao/ao-good-8/fix-windows-zellij-spawn
Open

fix: stabilize Windows zellij spawns#328
Vaibhaav-Tiwari wants to merge 3 commits into
mainfrom
ao/ao-good-8/fix-windows-zellij-spawn

Conversation

@Vaibhaav-Tiwari

Copy link
Copy Markdown
Collaborator

Summary

  • start Windows zellij session creation without the PowerShell Start-Process wrapper and clean up failed create attempts
  • delete pre-runtime seed rows on spawn failures so the reaper does not repeatedly warn about missing runtime handles
  • show daemon error-envelope messages when Electron orchestrator spawn fails
  • update docs to reflect that the Go rewrite currently requires zellij 0.44.3+ and does not wire runtime process yet

Fixes #327

Tests

  • go test ./internal/adapters/runtime/zellij -run TestCreate
  • go test ./internal/session_manager -run TestSpawn

Notes

  • Focused backend tests passed locally. Full zellij Windows integration no longer hangs in create, but still fails locally with pane exited before ready; left as a remaining integration/environment check rather than blocking this focused first-run race cleanup.
  • Frontend vitest could not run in this worktree because frontend/node_modules is missing config dependencies such as vitest/config and @vitejs/plugin-react.

@Vaibhaav-Tiwari Vaibhaav-Tiwari left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: APPROVE (posted as a comment because GitHub does not allow approving your own PR).

Focused, correct, and well-tested Windows zellij spawn fix. The failed-spawn rollback refactor (rollbackSpawnSeedRow) is sound: it deletes only seed-state rows and falls back to parking terminated when the row has progressed or the delete fails, and the post-MarkSpawned path correctly keeps markSpawnFailedTerminated. The added Destroy on the createSession failure path is idempotent and uses context.Background(), matching the sibling paths. Frontend apiErrorMessage refactor and tests look good; docs are updated comprehensively.

One non-blocking cleanup note inline (dead helper functions). Full review in review.md.

func startBackgroundProcess(env []string, name string, args ...string) error {
script := "Start-Process -FilePath " + psQuote(name) + " -ArgumentList " + psQuote(windowsCommandLine(args)) + " -WindowStyle Hidden"
cmd := exec.Command("powershell.exe", "-NoLogo", "-NoProfile", "-EncodedCommand", powerShellEncodedCommand(script))
cmd := exec.Command(name, args...)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the PowerShell Start-Process wrapper is gone, windowsCommandLine (line 27) and windowsQuoteArg (line 35) are no longer referenced by anything except each other — they're dead code. psQuote/powerShellEncodedCommand are still used by commands.go, but these two should be removed. The unused linter is enabled in .golangci.yml; it won't fire on Linux CI because of the //go:build windows tag, but it would fail a GOOS=windows lint. Non-blocking, but worth deleting in this PR.

@
refactor(zellij): drop dead Windows command-line helpers

windowsCommandLine and windowsQuoteArg were only used by the PowerShell
Start-Process wrapper that the spawn fix removed; they are now
unreachable. Delete both (and the now-unused strings import) so a
GOOS=windows lint with the unused linter stays clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@
@Vaibhaav-Tiwari Vaibhaav-Tiwari force-pushed the ao/ao-good-8/fix-windows-zellij-spawn branch from 09971e7 to e727565 Compare June 19, 2026 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows Electron orchestrator spawn has zellij first-run failure and docs drift

1 participant