fix: stabilize Windows zellij spawns#328
Conversation
Vaibhaav-Tiwari
left a comment
There was a problem hiding this comment.
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...) |
There was a problem hiding this comment.
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> @
09971e7 to
e727565
Compare
Summary
Fixes #327
Tests
Notes