fix(desktop): open sessions without a surface and keep hello ids present

Problem: sending from a draft over an open session created the new session
in the background while the surface stayed on the old one, and clicking a
session row after the last visible session was archived failed. A service
built with version "dev" (pnpm dev:desktop) or with desktop telemetry off
could not complete the shell handshake.

Root cause: OpenSession installed the target into the active tab and
returned "workspace is not ready" when none existed; draft acceptance never
activates the tab it creates, and the removed replacement blank session had
been masking the gap. HelloResult.RunID/IncidentID carried omitempty, so a
service without a lifecycle tracker dropped keys the shell validator
requires to be present (empty allowed).

Fix: surfaceForCanonicalSession resolves the surface for OpenSession (active
tab, else a visible tab already running the session, else an empty surface
created for the session's workspace and bound by the resume path, discarded
if the bind fails). The hello identifiers are always serialized; the
protocol document states they are empty when diagnostics are disabled.

Verification: desktop go test ./..., hostrpc tests, Electron shell tests
and typecheck, and a live dev-version shell run driven over CDP.

desktop/electron/src/main/handshake.test.ts

@@ -94,3 +94,11 @@ test("handshake failures map every documented code and keep the real error text"
   assert.equal(spawn.code, null);
   assert.equal(spawn.detail, "spawn ENOENT");
 });
+
+test("a service without lifecycle diagnostics reports empty run and incident ids", () => {
+  const result = validateHelloResult({ ...goodResult, runId: "", incidentId: "", diagnosticsEnabled: false });
+  assert.equal(result.runId, "");
+  assert.equal(result.incidentId, "");
+  assert.equal(result.diagnosticsEnabled, false);
+  assert.throws(() => validateHelloResult({ ...goodResult, runId: undefined }), /result\.runId/);
+});

desktop/internal/hostrpc/hello.go

@@ -63,16 +63,19 @@ type HelloInstance struct {
 
 // HelloResult is the service's desktop/hello response.
 type HelloResult struct {
-	ProtocolVersion    int             `json:"protocolVersion"`
-	ContractDigest     string          `json:"contractDigest"`
-	Service            ServiceInfo     `json:"service"`
-	RuntimeGeneration  string          `json:"runtimeGeneration"`
-	Resources          Resources       `json:"resources"`
-	Window             *WindowGeometry `json:"window,omitempty"`
-	Instance           *InstanceInfo   `json:"instance,omitempty"`
-	RunID              string          `json:"runId,omitempty"`
-	IncidentID         string          `json:"incidentId,omitempty"`
-	DiagnosticsEnabled bool            `json:"diagnosticsEnabled"`
+	ProtocolVersion   int             `json:"protocolVersion"`
+	ContractDigest    string          `json:"contractDigest"`
+	Service           ServiceInfo     `json:"service"`
+	RuntimeGeneration string          `json:"runtimeGeneration"`
+	Resources         Resources       `json:"resources"`
+	Window            *WindowGeometry `json:"window,omitempty"`
+	Instance          *InstanceInfo   `json:"instance,omitempty"`
+	// The shell validates both identifiers as present strings and accepts
+	// empty values when diagnostics are disabled; omitting the keys is a
+	// handshake failure, so they must never carry omitempty.
+	RunID              string `json:"runId"`
+	IncidentID         string `json:"incidentId"`
+	DiagnosticsEnabled bool   `json:"diagnosticsEnabled"`
 }
 
 // InstanceInfo extends the handshake without invalidating older shells.

desktop/internal/hostrpc/hello_test.go

@@ -0,0 +1,22 @@
+package hostrpc
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// A dev build or a telemetry-off configuration has no lifecycle tracker, so the
+// identifiers are empty. The shell requires the keys to be present; dropping
+// them made every such service fail the handshake.
+func TestHelloResultAlwaysCarriesRunAndIncidentKeys(t *testing.T) {
+	body, err := json.Marshal(HelloResult{DiagnosticsEnabled: false})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, key := range []string{`"runId":""`, `"incidentId":""`} {
+		if !strings.Contains(string(body), key) {
+			t.Fatalf("hello result omitted %s: %s", key, body)
+		}
+	}
+}

desktop/session_open_surface.go

@@ -0,0 +1,102 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"reasonix/desktop/internal/workspacestate"
+	"reasonix/internal/control"
+	"reasonix/internal/session"
+)
+
+// surfaceForCanonicalSession returns the tab OpenSession installs ref into.
+// The active tab wins. Without one (fresh start, or the last visible session
+// was archived), a visible tab that already runs ref is activated, and
+// otherwise an empty surface is created for ref's workspace and bound by the
+// caller. Opening a session must never depend on a replacement blank session
+// having been created for it.
+func (a *App) surfaceForCanonicalSession(ref session.SessionRef, workspace workspacestate.Workspace) (*WorkspaceTab, control.SessionAPI, bool, error) {
+	if tab, ctrl := a.tabAndCtrlByID(""); tab != nil {
+		return tab, ctrl, false, nil
+	}
+	a.mu.Lock()
+	if owner := a.liveRuntimeTabMatchingLocked(nil, sessionRoute(ref.SessionID)); owner != nil && a.tabs[owner.ID] == owner && !owner.removed {
+		a.activeTabID = owner.ID
+		a.saveTabsLocked()
+		ctrl := owner.Ctrl
+		a.mu.Unlock()
+		return owner, ctrl, false, nil
+	}
+	a.mu.Unlock()
+
+	scope := canonicalWorkspaceScope(workspace)
+	workspaceRoot := ""
+	if scope == "project" {
+		workspaceRoot = normalizeProjectRoot(workspace.Root)
+		if workspaceRoot == "" {
+			return nil, nil, false, fmt.Errorf("session workspace root is unavailable")
+		}
+	}
+	actualRoot := desktopWorkspaceRoot(scope, workspaceRoot)
+	if scope != "project" {
+		if err := os.MkdirAll(actualRoot, 0o755); err != nil {
+			return nil, nil, false, fmt.Errorf("create global workspace: %w", err)
+		}
+	}
+	releaseAdmission, err := a.beginProjectRuntimeAdmission(scope, actualRoot)
+	if err != nil {
+		return nil, nil, false, err
+	}
+	defer releaseAdmission()
+	if scope == "project" {
+		saveWorkspace(workspaceRoot)
+		a.registerProjectRoot(workspaceRoot)
+	}
+	model, toolApprovalMode := desktopNewSessionDefaults(scope, actualRoot)
+	tab := &WorkspaceTab{
+		Scope:            scope,
+		WorkspaceRoot:    actualRoot,
+		model:            model,
+		mode:             tabModeFromAxes(false, toolApprovalMode == control.ToolApprovalDangerFullAccess),
+		toolApprovalMode: toolApprovalMode,
+		disabledMCP:      map[string]ServerView{},
+	}
+	a.mu.Lock()
+	if existing := a.activeTabLocked(); existing != nil {
+		// Another navigation published a surface meanwhile; install into it.
+		ctrl := existing.Ctrl
+		a.mu.Unlock()
+		return existing, ctrl, false, nil
+	}
+	tab.ID = a.newUniqueTabIDLocked()
+	tab.sink = &tabEventSink{tabID: tab.ID, app: a}
+	a.tabs[tab.ID] = tab
+	a.tabOrder = append(a.tabOrder, tab.ID)
+	a.activeTabID = tab.ID
+	a.saveTabsLocked()
+	a.mu.Unlock()
+	return tab, nil, true, nil
+}
+
+// discardUnboundSurface removes a surface created for OpenSession whose
+// binding failed, so a failed open leaves no empty tab behind.
+func (a *App) discardUnboundSurface(tab *WorkspaceTab) {
+	if tab == nil {
+		return
+	}
+	a.mu.Lock()
+	defer a.mu.Unlock()
+	if a.tabs[tab.ID] != tab || tab.Ctrl != nil || tab.SessionID != "" {
+		return
+	}
+	a.markTabRemovedLocked(tab)
+	delete(a.tabs, tab.ID)
+	a.removeTabOrderLocked(tab.ID)
+	if a.activeTabID == tab.ID {
+		a.activeTabID = ""
+		if len(a.tabOrder) > 0 {
+			a.activeTabID = a.tabOrder[0]
+		}
+	}
+	a.saveTabsLocked()
+}

desktop/session_open_surface_test.go

@@ -0,0 +1,103 @@
+package main
+
+import (
+	"testing"
+
+	"reasonix/internal/session"
+)
+
+// removeOpenFixtureTab leaves the app with no visible surface, the state after
+// the last session was archived or before the first one was ever opened.
+func removeOpenFixtureTab(t *testing.T, app *App, tab *WorkspaceTab) {
+	t.Helper()
+	app.mu.Lock()
+	app.markTabRemovedLocked(tab)
+	app.releaseSessionRuntimeLocked(tab)
+	delete(app.tabs, tab.ID)
+	app.removeTabOrderLocked(tab.ID)
+	app.activeTabID = ""
+	app.mu.Unlock()
+	if tab.Ctrl != nil {
+		tab.Ctrl.Close()
+		tab.Ctrl = nil
+	}
+}
+
+// Opening a session from the sidebar or from an accepted draft must work when
+// no surface exists; it used to fail with "workspace is not ready" and relied
+// on a replacement blank session being present.
+func TestOpenSessionWithoutSurfaceCreatesOneForTheSession(t *testing.T) {
+	app, tab, target, rootB, workspaceB := canonicalWorkspaceOpenFixture(t)
+	removeOpenFixtureTab(t, app, tab)
+
+	if _, err := app.OpenSession(target.Ref()); err != nil {
+		t.Fatalf("OpenSession without a surface: %v", err)
+	}
+	app.mu.RLock()
+	active := app.tabs[app.activeTabID]
+	count := len(app.tabs)
+	app.mu.RUnlock()
+	if active == nil || count != 1 {
+		t.Fatalf("expected exactly one active surface, got active=%v tabs=%d", active != nil, count)
+	}
+	t.Cleanup(func() {
+		if live := app.controllerForTab(active); live != nil {
+			live.Close()
+		}
+	})
+	if active.SessionID != target.Ref().SessionID || !active.Ready || active.StartupErr != "" || !sameDesktopPath(active.WorkspaceRoot, rootB) || active.SessionWorkspace.ID != workspaceB {
+		t.Fatalf("surface bound wrong session: %+v", persistedDesktopTabEntry(active))
+	}
+	persisted := loadTabsFile()
+	if len(persisted.Tabs) != 1 || persisted.Tabs[0].SessionID != target.Ref().SessionID || persisted.ActiveTab != active.ID {
+		t.Fatalf("surface not persisted as active: %+v", persisted)
+	}
+}
+
+// A session that already runs in a visible but inactive tab (the tab a draft
+// submission created in the background) is activated in place instead of being
+// rebuilt or rejected.
+func TestOpenSessionActivatesVisibleTabAlreadyRunningTheSession(t *testing.T) {
+	app, tab, _, _, _ := canonicalWorkspaceOpenFixture(t)
+	ctrl := tab.Ctrl
+	app.mu.Lock()
+	app.activeTabID = ""
+	app.mu.Unlock()
+
+	if _, err := app.OpenSession(session.SessionRef{HostID: localDesktopHostID, SessionID: tab.SessionID}); err != nil {
+		t.Fatalf("OpenSession: %v", err)
+	}
+	app.mu.RLock()
+	activeID, count := app.activeTabID, len(app.tabs)
+	app.mu.RUnlock()
+	if activeID != tab.ID || count != 1 || tab.Ctrl != ctrl {
+		t.Fatalf("owner tab not activated in place: active=%q tabs=%d sameCtrl=%v", activeID, count, tab.Ctrl == ctrl)
+	}
+}
+
+// A surface created for an open that then fails must not linger as an empty
+// tab; that would be the replacement blank session in another form.
+func TestDiscardUnboundSurfaceRemovesOnlyTheEmptyTab(t *testing.T) {
+	app, tab, target, _, _ := canonicalWorkspaceOpenFixture(t)
+	removeOpenFixtureTab(t, app, tab)
+	workspace, err := app.canonicalSessionWorkspace(t.Context(), target.Ref())
+	if err != nil {
+		t.Fatal(err)
+	}
+	created, ctrl, isNew, err := app.surfaceForCanonicalSession(target.Ref(), workspace)
+	if err != nil || ctrl != nil || !isNew {
+		t.Fatalf("surfaceForCanonicalSession = %+v, %v, new=%v, %v", created, ctrl, isNew, err)
+	}
+	app.mu.RLock()
+	activeID := app.activeTabID
+	app.mu.RUnlock()
+	if activeID != created.ID {
+		t.Fatalf("created surface is not active: %q", activeID)
+	}
+	app.discardUnboundSurface(created)
+	app.mu.RLock()
+	defer app.mu.RUnlock()
+	if len(app.tabs) != 0 || len(app.tabOrder) != 0 || app.activeTabID != "" {
+		t.Fatalf("unbound surface lingered: tabs=%d order=%v active=%q", len(app.tabs), app.tabOrder, app.activeTabID)
+	}
+}

desktop/session_workspace_api.go

@@ -709,11 +709,18 @@ func (a *App) openSessionWithNavigation(ref session.SessionRef, navigationSequen
 	if a.desktopSessions.navigationSeq.Load() != navigationSequence {
 		return HistoryPage{}, errSessionNavigationSuperseded
 	}
-	tab, ctrl := a.tabAndCtrlByID("")
-	if tab == nil {
-		return HistoryPage{}, errors.New("workspace is not ready")
+	workspace, err := a.canonicalSessionWorkspace(a.bootContext(), ref)
+	if err != nil {
+		return HistoryPage{}, err
+	}
+	tab, ctrl, created, err := a.surfaceForCanonicalSession(ref, workspace)
+	if err != nil {
+		return HistoryPage{}, err
 	}
 	if _, err := a.resumeCanonicalSessionForTranscript(tab, ctrl, sessionRoute(ref.SessionID), defaultHistoryPageTurns, false, navigationSequence); err != nil {
+		if created {
+			a.discardUnboundSurface(tab)
+		}
 		return HistoryPage{}, err
 	}
 	// runtime:rebuilt intentionally has no reload semantics. SessionRef opening

docs/DESKTOP_HOST_PROTOCOL.md

@@ -99,8 +99,8 @@ the shell discards anything tagged with an old one.
 `runId` identifies this service run. `incidentId` links service and shell
 lifecycle evidence for the same failure chain. Both are random diagnostic
 identifiers; they do not contain a PID, local path, or user content. When
-diagnostics are disabled, `diagnosticsEnabled` is false and both identifiers
-may be omitted.
+diagnostics are disabled (including a `dev` service build), `diagnosticsEnabled`
+is false and both identifiers are empty strings; the keys are always present.
 
 ## Lifecycle requests (shell → service)