From 0d7a9313f6e2f6caa189a5e2cd0a44c69cd6671e Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 17 Oct 2026 17:43:05 -0300 Subject: [PATCH 1/3] fix(ci): switch the release e2e back to the web window after the native first launch The first launch of a release opens the native window since #809, and the macOS and Windows end-to-end checks expected the web one on it. --- .github/scripts/mac-e2e.sh | 15 +++++++++++++++ .github/workflows/release.yml | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/.github/scripts/mac-e2e.sh b/.github/scripts/mac-e2e.sh index e5d45031..bb0fe9e4 100755 --- a/.github/scripts/mac-e2e.sh +++ b/.github/scripts/mac-e2e.sh @@ -59,6 +59,21 @@ for _ in $(seq 1 30); do token=$(jq -r .token "$config/runtime.json" 2>/dev/null) && break done test -n "$token" || fail "lich never wrote runtime.json" + +# The first launch of a release opens the native window (#809). What follows +# checks the web one, so lich is switched back the way Settings does it: the +# choice, then the window. +for _ in $(seq 1 30); do sleep 1; pgrep -x lich-native > /dev/null && break; done +pgrep -x lich-native > /dev/null || fail "the first launch of a release did not open the native window" +rpc() { + curl -sf -X POST -H 'Content-Type: application/json' -d "$2" \ + "http://127.0.0.1:$LICH_LISTEN_PORT/rpc/$1?token=$token" > /dev/null || fail "rpc $1 failed" +} +rpc store.SetSetting '["window.kind", "", "web"]' +rpc system.OpenWindow '[{"kind": "web"}]' +for _ in $(seq 1 15); do sleep 1; pgrep -x lich-native > /dev/null || break; done +pgrep -x lich-native > /dev/null && fail "the native window outlived the switch to the web one" +for _ in $(seq 1 30); do sleep 1; grep -q 'the bundled window' "$log" && break; done grep -q 'the bundled window' "$log" || fail "lich did not resolve its own window" for _ in $(seq 1 30); do diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 75487771..37035f36 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -303,6 +303,16 @@ jobs: token=$(jq -r .token "$APPDATA/lich/runtime.json" 2>/dev/null) && break done test -n "$token" || { echo "::error::lich never wrote runtime.json"; cat "$APPDATA/lich/lich.log"; exit 1; } + # The first launch of a release opens the native window (#809); on + # the runner it exits before it attaches and lich falls back to the + # web one. What follows checks the web window, so lich is switched + # back the way Settings does it, and later launches open the web one. + for args in 'store.SetSetting ["window.kind", "", "web"]' 'system.OpenWindow [{"kind": "web"}]'; do + curl -sf -X POST -H 'Content-Type: application/json' -d "${args#* }" \ + "http://127.0.0.1:$LICH_LISTEN_PORT/rpc/${args%% *}?token=$token" > /dev/null \ + || { echo "::error::rpc ${args%% *} failed"; cat "$APPDATA/lich/lich.log"; exit 1; } + done + for _ in $(seq 1 30); do sleep 1; grep -q 'the bundled window' "$APPDATA/lich/lich.log" && break; done grep -q 'the bundled window' "$APPDATA/lich/lich.log" || { echo "::error::lich did not resolve its own window"; cat "$APPDATA/lich/lich.log"; exit 1; } for _ in $(seq 1 30); do sleep 1 From e85a34eea9044f9a8fd85c56dc2e9a7d991b2d2c Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 17 Oct 2026 18:12:53 -0300 Subject: [PATCH 2/3] fix(ci): wait for the native window to attach before the e2e switches back On the slower Intel runner the switch came before the native window had attached, so lich had no window of it to close and both stayed open. lich now logs the attach, and the macOS and Windows checks wait for it. --- .github/scripts/mac-e2e.sh | 8 +++++--- .github/workflows/release.yml | 6 +++++- internal/system/native.go | 2 ++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/scripts/mac-e2e.sh b/.github/scripts/mac-e2e.sh index bb0fe9e4..71c3137c 100755 --- a/.github/scripts/mac-e2e.sh +++ b/.github/scripts/mac-e2e.sh @@ -62,9 +62,11 @@ test -n "$token" || fail "lich never wrote runtime.json" # The first launch of a release opens the native window (#809). What follows # checks the web one, so lich is switched back the way Settings does it: the -# choice, then the window. -for _ in $(seq 1 30); do sleep 1; pgrep -x lich-native > /dev/null && break; done -pgrep -x lich-native > /dev/null || fail "the first launch of a release did not open the native window" +# choice, then the window. Only once the native window has attached, as it has +# for anyone switching from its Settings: before that, lich has no window of +# it to close. +for _ in $(seq 1 60); do sleep 1; grep -q 'native window attached' "$log" && break; done +grep -q 'native window attached' "$log" || fail "the first launch of a release did not open the native window" rpc() { curl -sf -X POST -H 'Content-Type: application/json' -d "$2" \ "http://127.0.0.1:$LICH_LISTEN_PORT/rpc/$1?token=$token" > /dev/null || fail "rpc $1 failed" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 37035f36..57bfdcdd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -306,7 +306,11 @@ jobs: # The first launch of a release opens the native window (#809); on # the runner it exits before it attaches and lich falls back to the # web one. What follows checks the web window, so lich is switched - # back the way Settings does it, and later launches open the web one. + # back the way Settings does it, and later launches open the web one, + # once the native window has attached or exited: before either, lich + # has no window of it to close. + for _ in $(seq 1 60); do sleep 1; grep -q 'native window attached\|native window exited' "$APPDATA/lich/lich.log" && break; done + grep -q 'native window attached\|native window exited' "$APPDATA/lich/lich.log" || { echo "::error::the first launch of a release did not start the native window"; cat "$APPDATA/lich/lich.log"; exit 1; } for args in 'store.SetSetting ["window.kind", "", "web"]' 'system.OpenWindow [{"kind": "web"}]'; do curl -sf -X POST -H 'Content-Type: application/json' -d "${args#* }" \ "http://127.0.0.1:$LICH_LISTEN_PORT/rpc/${args%% *}?token=$token" > /dev/null \ diff --git a/internal/system/native.go b/internal/system/native.go index af6c5c92..8685b57c 100644 --- a/internal/system/native.go +++ b/internal/system/native.go @@ -3,6 +3,7 @@ package system import ( "errors" "fmt" + "log/slog" "sync" "github.com/omartelo/lich/internal/restart" @@ -92,6 +93,7 @@ func (s *Service) AttachNative(opts AttachNativeOptions) { defer s.native.mu.Unlock() s.native.executable = opts.Executable s.native.active, s.native.attached = true, true + slog.Info("native window attached", "executable", opts.Executable) } // OpenWindowOptions is one OpenWindow call. From ece10eafa408f3402891df0791421b4224db872c Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 17 Oct 2026 18:39:04 -0300 Subject: [PATCH 3/3] test(terminal): run the tests that exec a script they wrote, or rebind a freed port, serially #813 ran them in parallel. A test that writes a script and execs it fails with ETXTBSY when another test forks while the script is still open for writing (golang/go#22315), and the port-retry test can lose its freed port to a parallel test's 127.0.0.1:0. Measured on the package: 1 failure in 15 runs before, 0 in 25 after. --- internal/terminal/account_linux_test.go | 1 - internal/terminal/exit_watch_test.go | 1 - internal/terminal/respawn_test.go | 5 ----- internal/terminal/spawn_test.go | 1 - internal/terminal/start_ultracode_test.go | 3 --- internal/terminal/startgate_test.go | 1 - internal/terminal/terminal_test.go | 15 --------------- internal/terminal/transport_restart_test.go | 1 - internal/terminal/usage_cost_test.go | 4 ---- 9 files changed, 32 deletions(-) diff --git a/internal/terminal/account_linux_test.go b/internal/terminal/account_linux_test.go index 2c355e08..8011652a 100644 --- a/internal/terminal/account_linux_test.go +++ b/internal/terminal/account_linux_test.go @@ -17,7 +17,6 @@ import ( // reads back is the environment the provider actually runs with — not the one // lich spawned the wrapper with. func TestSessionAccountReadsWhatTheWrapperExecsInto(t *testing.T) { - t.Parallel() dir := t.TempDir() bin := filepath.Join(dir, "claude-work.sh") script := "#!/bin/sh\nexport CLAUDE_CONFIG_DIR=" + dir + "\nexec sleep 30\n" diff --git a/internal/terminal/exit_watch_test.go b/internal/terminal/exit_watch_test.go index ed92fbc4..8b34b905 100644 --- a/internal/terminal/exit_watch_test.go +++ b/internal/terminal/exit_watch_test.go @@ -15,7 +15,6 @@ import ( // (relay.SessionExited). A close is not: it reaches the relay through the // close watcher instead. func TestAReapedSessionIsReportedToTheExitWatcher(t *testing.T) { - t.Parallel() hub, _ := newProbeHub(t) svc := New(stubBins{bin: exitBin(t, 0)}, nil, hub) var mu sync.Mutex diff --git a/internal/terminal/respawn_test.go b/internal/terminal/respawn_test.go index 5aaea07d..23ddc5ad 100644 --- a/internal/terminal/respawn_test.go +++ b/internal/terminal/respawn_test.go @@ -59,7 +59,6 @@ func respawn(t *testing.T, svc *Service, id string) *session { // so one landing on a session respawned under the same id lies about a live // shell. func TestRespawnedSessionIsNotToldItExited(t *testing.T) { - t.Parallel() hub, rec := newProbeHub(t) svc := New(stubBins{bin: echoBin(t)}, nil, hub) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -79,7 +78,6 @@ func TestRespawnedSessionIsNotToldItExited(t *testing.T) { // evicts only its own PTY, so after Close+Start under one id that id still // resolves to the new PTY and input still reaches it. func TestRespawnedSessionKeepsItsPTY(t *testing.T) { - t.Parallel() svc := New(stubBins{bin: echoBin(t)}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -107,7 +105,6 @@ func TestRespawnedSessionKeepsItsPTY(t *testing.T) { // The card reads it as the reason the session ended, and only a real spawn shows // whether the number survives the trip. func TestExitEventCarriesTheExitCode(t *testing.T) { - t.Parallel() hub, rec := newProbeHub(t) svc := New(stubBins{bin: exitBin(t, 3)}, nil, hub) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -136,7 +133,6 @@ func TestExitEventCarriesTheExitCode(t *testing.T) { // the path that produces it: a child killed by a signal left no exit status, so // the event has to say nothing rather than report the 0 of a clean exit. func TestExitEventOmitsAnUnobservedStatus(t *testing.T) { - t.Parallel() hub, rec := newProbeHub(t) // echoBin execs cat, so the pid below is the process holding the PTY — // a script that forked one would leave the slave open and never reach EOF. @@ -195,7 +191,6 @@ func replayOf(t *testing.T, svc *Service, id string) string { // under it, takes the turn with it: the count that keeps the machine awake // must fall on the reap, not wait for someone to close the card. func TestReapForgetsOpenTurn(t *testing.T) { - t.Parallel() svc := New(stubBins{bin: echoBin(t)}, nil, events.New()) var open atomic.Int64 svc.turns.onOpen = func(n int) { open.Store(int64(n)) } diff --git a/internal/terminal/spawn_test.go b/internal/terminal/spawn_test.go index 42889758..008c0a6d 100644 --- a/internal/terminal/spawn_test.go +++ b/internal/terminal/spawn_test.go @@ -130,7 +130,6 @@ func TestStartReportsAPtyThatCannotSpawn(t *testing.T) { // the effective directory is what the card shows and what the cwd poller // degrades back to. func TestASpawnWithNoDirectoryStartsAtHome(t *testing.T) { - t.Parallel() svc := New(stubBins{bin: stayAliveBin(t)}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) diff --git a/internal/terminal/start_ultracode_test.go b/internal/terminal/start_ultracode_test.go index 558263f6..93449adc 100644 --- a/internal/terminal/start_ultracode_test.go +++ b/internal/terminal/start_ultracode_test.go @@ -15,7 +15,6 @@ import ( // argv ahead of --mcp-config, and that a resume carries it too: Claude Code // forgets ultracode on --resume, unlike the model and effort. func TestStartPassesUltracodeToTheProcess(t *testing.T) { - t.Parallel() tests := []struct { name string store stubBins @@ -53,7 +52,6 @@ func TestStartPassesUltracodeToTheProcess(t *testing.T) { // spawns with it: the fork's row is a fresh insert, and Claude Code drops // ultracode on --resume --fork-session as on a plain resume. func TestStartCarriesUltracodeOntoAFork(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) store := stubBins{ bin: bin, @@ -80,7 +78,6 @@ func TestStartCarriesUltracodeOntoAFork(t *testing.T) { } func TestStartLeavesUltracodeOffByDefault(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) diff --git a/internal/terminal/startgate_test.go b/internal/terminal/startgate_test.go index 75d0260e..70af88ea 100644 --- a/internal/terminal/startgate_test.go +++ b/internal/terminal/startgate_test.go @@ -146,7 +146,6 @@ func TestHoldForStartIgnoresASessionThatWasReplaced(t *testing.T) { // Through the real spawn: the session settles, stays held, and is released by // the provider's report coming through the same path the hook's POST takes. func TestStartHoldsAProviderSessionUntilItsReport(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) svc.SetStartReports(func(string) bool { return true }) diff --git a/internal/terminal/terminal_test.go b/internal/terminal/terminal_test.go index ba912df5..6bf1ee5c 100644 --- a/internal/terminal/terminal_test.go +++ b/internal/terminal/terminal_test.go @@ -569,7 +569,6 @@ func spawnedArgs(t *testing.T, svc *Service, id string) []string { // TestStartPassesResumeToTheProcess proves the resume id reaches the spawned // binary's argv — the wiring resumeArgs' unit test cannot see. func TestStartPassesResumeToTheProcess(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -593,7 +592,6 @@ func TestStartPassesResumeToTheProcess(t *testing.T) { // parent's id — and that the fork is *named*, which is the one place a spawn // carrying a resume id still gets --name (nameArgs). func TestStartForksIntoItsOwnConversation(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -619,7 +617,6 @@ func TestStartForksIntoItsOwnConversation(t *testing.T) { // otherwise reopen the parent's own conversation under a second card, which // is the one outcome a fork promises against. Nothing is spawned. func TestStartRefusesAForkTheProviderCannotSpell(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -640,7 +637,6 @@ func TestStartRefusesAForkTheProviderCannotSpell(t *testing.T) { // it is not a fork — without one resumeArgs would start fresh and the card // would claim a conversation it never copied. func TestStartRefusesAForkWithNothingToBranch(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -656,7 +652,6 @@ func TestStartRefusesAForkWithNothingToBranch(t *testing.T) { // TestStartWithoutResumeSpawnsBare proves a session with no id to resume spawns // the binary alone, with no dangling flag. func TestStartWithoutResumeSpawnsBare(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -681,7 +676,6 @@ func TestStartWithoutResumeSpawnsBare(t *testing.T) { // goes through spawnPins, which drops the briefing before comparing — so // nothing else here would notice it being lost between the two. func TestStartCarriesTheBriefingIntoTheSpawn(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -707,7 +701,6 @@ func TestStartCarriesTheBriefingIntoTheSpawn(t *testing.T) { // through sh -c with the project's script ahead of the exec'd provider — the // wiring wrapSetup's unit test cannot see. func TestStartWithSetupWrapsTheSpawn(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin, projectPath: projectWithSetup(t, "echo setup-ran")}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -733,7 +726,6 @@ func TestStartWithSetupWrapsTheSpawn(t *testing.T) { // TestStartWithSetupButNoScriptSpawnsBare proves the flag is inert for a // project with no setup script configured — no sh indirection sneaks in. func TestStartWithSetupButNoScriptSpawnsBare(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1015,7 +1007,6 @@ func TestSkipPermissionArgs(t *testing.T) { // see, and the one that decides whether a user who ticked the box actually gets // an agent that stops asking. func TestStartPassesSkipPermissionsToTheProcess(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin, skipPerms: true}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1205,7 +1196,6 @@ func TestSupportsEffort(t *testing.T) { // TestStartPassesTheStoredEffortToTheProcess proves the effort on the row reaches // argv, read from the store like the model so a respawn keeps it. func TestStartPassesTheStoredEffortToTheProcess(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin, model: "opus", effort: "high"}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1226,7 +1216,6 @@ func TestStartPassesTheStoredEffortToTheProcess(t *testing.T) { // passed to Start precisely so it survives a respawn, and this is the wiring // that carries it there. func TestStartPassesTheStoredModelToTheProcess(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin, model: "opus"}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1763,7 +1752,6 @@ func TestResizeRecordsTheWindowsSize(t *testing.T) { // TestStartWithoutASizeCopiesTheWindow drives the whole path: a card the window // measured, then a session opened with no terminal of its own. func TestStartWithoutASizeCopiesTheWindow(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1"); _ = svc.Close("s2") }) @@ -1787,7 +1775,6 @@ func TestStartWithoutASizeCopiesTheWindow(t *testing.T) { // the project's setup script is live but has nothing on the other end that can // read a message, until the wrapper's marker comes through its own output. func TestReadyWaitsForTheSetupScript(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin, projectPath: projectWithSetup(t, "echo installing")}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1916,7 +1903,6 @@ func TestReadyRemembersAQuietThatAlreadyPassed(t *testing.T) { } func TestReadyWaitsOnlyTheSettleWithoutASetupScript(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1") }) @@ -1979,7 +1965,6 @@ func TestSetEnvMovesTheSpawnBase(t *testing.T) { // TestLiveCountCountsTheSessionsWithAProcess is what the tray reads: a session // counts while its process runs and stops counting once it is closed. func TestLiveCountCountsTheSessionsWithAProcess(t *testing.T) { - t.Parallel() bin := stayAliveBin(t) svc := New(stubBins{bin: bin}, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s1"); _ = svc.Close("s2") }) diff --git a/internal/terminal/transport_restart_test.go b/internal/terminal/transport_restart_test.go index 48c1346d..361a8b77 100644 --- a/internal/terminal/transport_restart_test.go +++ b/internal/terminal/transport_restart_test.go @@ -132,7 +132,6 @@ func TestListenBailsOutFastOnUnretryableError(t *testing.T) { } func TestListenRetriesUntilPortFreesWithoutWaitMarker(t *testing.T) { - t.Parallel() busy, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("occupy port: %v", err) diff --git a/internal/terminal/usage_cost_test.go b/internal/terminal/usage_cost_test.go index 4b262790..f99a4f80 100644 --- a/internal/terminal/usage_cost_test.go +++ b/internal/terminal/usage_cost_test.go @@ -797,7 +797,6 @@ func TestAPricedSessionCarriesNoMissReason(t *testing.T) { // about to count that stretch a second time, and what it had cost the first time // is what has to come back off. func TestAForkRecordsWhatItsParentHadCost(t *testing.T) { - t.Parallel() store := newCostStore("") store.bin = stayAliveBin(t) store.forkOffsets = map[string]float64{} @@ -819,7 +818,6 @@ func TestAForkRecordsWhatItsParentHadCost(t *testing.T) { // own figure and starts a forked session's at zero, so netting anything off it // would hide what the fork itself spent. func TestAForkOfAProviderThatPricesItselfOffsetsNothing(t *testing.T) { - t.Parallel() store := newCostStore("") store.bin = stayAliveBin(t) store.forkOffsets = map[string]float64{} @@ -842,7 +840,6 @@ func TestAForkOfAProviderThatPricesItselfOffsetsNothing(t *testing.T) { // that only resumes must record no offset — one that did would bill the session // backwards for its own history. func TestAResumeIsNotAFork(t *testing.T) { - t.Parallel() store := newCostStore("") store.bin = stayAliveBin(t) store.forkOffsets = map[string]float64{} @@ -864,7 +861,6 @@ func TestAResumeIsNotAFork(t *testing.T) { // session, and a readout that is too high is not worth losing it, so the // failure is logged and the spawn goes ahead with no offset recorded. func TestAForkWhoseOffsetCannotBeSavedStillStarts(t *testing.T) { - t.Parallel() store := newCostStore("") store.bin = stayAliveBin(t) store.forkOffsets = map[string]float64{}