Merge pull request #12205 from KHG420/codex/studio-failed-switch-sidecar-rollback

fix(studio): restore sidecars after model selection fails

internal/assembly/boot/build.go

@@ -123,12 +123,12 @@ func build(ctx context.Context, opts Options) (*BuildResult, error) {
 	return b.freeze(ctrl)
 }
 
-// retireUnownedSidecars closes the preflighted sidecars when the build fails
-// before the extension snapshot takes ownership: no process outlives a failed build.
+// retireUnownedSidecars restores adopted clients and closes fresh sidecars when
+// the build fails before the extension snapshot takes ownership.
 func (b *builder) retireUnownedSidecars() {
 	if b.pendingMgr != nil {
 		close(b.ext.failed)
-		_ = b.pendingMgr.Close()
+		b.pendingMgr.RollbackPlanStart(b.opts.Extensions)
 	}
 }
 
@@ -595,6 +595,7 @@ func (b *builder) freeze(ctrl *control.Controller) (*BuildResult, error) {
 		// The failed assembly already retired the sidecars; bind neither hub nor manager.
 		extensionMgr = nil
 	}
+	installSidecarStreamRouters(extensionMgr, b.providers.extension)
 	providerResolver := b.providers.base
 	if b.providers.extension != nil {
 		providerResolver = b.providers.extension

internal/assembly/boot/effect_failed_switch_sidecar_test.go

@@ -0,0 +1,89 @@
+package boot
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"reasonix/internal/contract/config"
+	"reasonix/internal/contract/event"
+	"reasonix/internal/contract/provider"
+	"reasonix/internal/ext/extension/protocol"
+)
+
+func TestEffectFailedSwitchRestoresSidecars(t *testing.T) {
+	for _, addPackage := range []bool{false, true} {
+		name := "unchanged-graph"
+		if addPackage {
+			name = "added-package"
+		}
+		t.Run(name, func(t *testing.T) {
+			isolateConfigHome(t)
+			workspace := robustTempDir(t)
+			t.Chdir(workspace)
+			writePluginDefaultFixture(t, workspace, "plugin/stable/fake/x")
+			home := config.ReasonixHomeDir()
+			installProviderFake(t, home, "stable", nil)
+			serving, err := BuildRuntime(t.Context(), Options{Sink: event.Discard})
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(serving.Controller.Close)
+			client := serving.Extensions.Client("stable")
+			if client == nil {
+				t.Fatal("serving runtime has no provider sidecar")
+			}
+			generation := serving.Controller.RuntimeGeneration()
+			pidFile := filepath.Join(workspace, "added.pid")
+			if addPackage {
+				installBootFakePlugin(t, home, "added", map[string]any{
+					"env": map[string]string{bootFakeEnvPIDFile: pidFile},
+				})
+			}
+
+			// Mirror serve.reuseFromLastBuild, including the published generation.
+			opts := Options{Model: "plugin/missing/fake/x", Sink: event.Discard}
+			opts.RuntimeReload = RuntimeReload{
+				ForceFullRebuild:   true,
+				Extensions:         serving.Extensions,
+				Graph:              serving.Plan.Graph,
+				Generation:         serving.Snapshot.Generation(),
+				Owner:              serving.Owner,
+				PreviousSnapshot:   serving.Snapshot,
+				PreviousDispatcher: serving.Dispatcher,
+				PreviousPlan:       serving.Plan,
+				ReuseAssembly:      serving.Assembly,
+			}
+			_, err = BuildRuntime(t.Context(), opts)
+			if !errors.Is(err, ErrUnknownModel) {
+				t.Fatalf("failed switch = %v, want ErrUnknownModel", err)
+			}
+			if addPackage {
+				pid := readFakePID(t, pidFile)
+				waitForCond(t, "failed build's new sidecar exit", 10*time.Second, func() bool { return !pidAlive(pid) })
+			}
+			if serving.Extensions.Client("stable") != client || client.Exited() {
+				t.Fatal("failed switch did not restore the serving sidecar")
+			}
+			if serving.Runtime.Closed() || serving.Controller.RuntimeGeneration() != generation {
+				t.Fatal("failed switch changed the serving runtime")
+			}
+			ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
+			defer cancel()
+			result, err := client.Intercept(ctx, protocol.EventSessionStart, json.RawMessage(`{}`), time.Second)
+			if err != nil || result.Decision != protocol.DecisionContinue {
+				t.Fatalf("serving sidecar after failed switch = %+v, %v", result, err)
+			}
+			if err := serving.Controller.RunTurn(ctx, "reply after failed switch"); err != nil {
+				t.Fatalf("provider turn after failed switch: %v", err)
+			}
+			history := serving.Controller.History()
+			if last := history[len(history)-1]; last.Role != provider.RoleAssistant || last.Content != "fake-hello fake-world" {
+				t.Fatalf("provider reply after failed switch = %+v", last)
+			}
+		})
+	}
+}

internal/assembly/boot/extension_stage.go

@@ -126,7 +126,6 @@ func resolveProviders(opts Options, cfg *config.Config, proxySpec netclient.Prox
 	if err != nil {
 		return ps, fmt.Errorf("boot: %w", err)
 	}
-	installSidecarStreamRouters(mgr, merged)
 	ps.effective, ps.extension = merged, merged
 	return ps, nil
 }

internal/ext/extension/sidecar/plan.go

@@ -132,7 +132,7 @@ func StartPackagesWithPlan(ctx context.Context, home string, sessionCtx protocol
 }
 
 func adoptAll(previous *Manager) *Manager {
-	m := &Manager{clients: make(map[string]*Client)}
+	m := &Manager{clients: make(map[string]*Client), planAdopted: make(map[string]*Client)}
 	if previous == nil {
 		return m
 	}
@@ -140,6 +140,7 @@ func adoptAll(previous *Manager) *Manager {
 		id := client.PluginID()
 		if c := previous.Detach(id); c != nil {
 			m.clients[id] = c
+			m.planAdopted[id] = c
 		}
 	}
 	return m