fix: keep Skiller session URLs scoped

Author: skulidropek

docs/integrations/skiller.md

@@ -40,7 +40,7 @@ bun run skiller:check
 
 ## docker-git Web Launch
 
-The docker-git web terminal header includes a `Skiller` button next to `Open browser`. In a project terminal the button opens `/api/ssh/session/:sessionId/skiller/app/` immediately, using the same terminal session id that is present in `/ssh/session/:sessionId`. It also calls `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open`, which launches the pinned submodule Electron app as a separate process and writes launcher output to `~/.docker-git/logs/skiller.log`.
+The docker-git web terminal header includes a `Skiller` button next to `Open browser`. In a project terminal the button calls `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` first, which launches the pinned submodule Electron app as a separate process, registers the terminal session filesystem scope, and writes launcher output to `~/.docker-git/logs/skiller.log`. After that response succeeds, the browser opens the returned `/api/ssh/session/:sessionId/skiller/app/` URL using the same terminal session id that is present in `/ssh/session/:sessionId`.
 
 docker-git serves Skiller's built renderer from the submodule and proxies `/api/ssh/session/:sessionId/skiller/trpc/*` to the running Skiller tRPC backend, so the user sees the actual Skiller UI instead of an invisible background desktop process. The session id is part of the URL so a Skiller tab can be tied back to the terminal container that opened it.
 
@@ -60,6 +60,7 @@ The latest Playwright proof screenshots are checked in under `docs/screenshots/i
 - `pr238-proof-30-skiller-add-project-folder-browser-picker.png` shows the browser-visible project folder picker opened from `Add project folder...` with selected-container paths.
 - `pr238-proof-31-skiller-project-scoped-folder-picker-working.png` shows `/api/skiller/app/` opened from a project-scoped Skiller process with `Add project folder...` resolving to the selected container paths.
 - `pr238-proof-32-docker-git-browser-live-command.png` shows the live `docker-git browser` frontend served through the same controller used for the proof.
+- `pr238-proof-33-skiller-session-folder-picker-mcp.png` shows the terminal session-scoped Skiller URL opened from the real `Skiller` button and the `Add project folder...` picker resolving to `dg-skiller-button-demo:/home/dev/app`.
 
 ## Updating the Pin
 

docs/screenshots/issue-237/proof/pr238-proof-33-skiller-session-folder-picker-mcp.png

@@ -595,16 +595,26 @@ export const resolveSkillerBrowserScopeSelection = (
   route: Extract<SkillerRoute, { readonly _tag: "App" }>,
   currentScope: SkillerContainerScope | null,
   sessionScopeForId: (sessionId: string) => SkillerContainerScope | null | undefined
+): SkillerBrowserScopeSelection | null =>
+  resolveSkillerRouteScopeSelection(route.sessionId, currentScope, sessionScopeForId)
+
+export const resolveSkillerRouteScopeSelection = (
+  sessionId: string | null,
+  currentScope: SkillerContainerScope | null,
+  sessionScopeForId: (sessionId: string) => SkillerContainerScope | null | undefined
 ): SkillerBrowserScopeSelection | null => {
-  if (route.sessionId === null) {
+  if (sessionId === null) {
     return currentScope === null
       ? null
       : { scope: currentScope, sessionId: null }
   }
-  const sessionScope = sessionScopeForId(route.sessionId)
-  return sessionScope === undefined || sessionScope === null
+  const sessionScope = sessionScopeForId(sessionId)
+  if (sessionScope !== undefined && sessionScope !== null) {
+    return { scope: sessionScope, sessionId }
+  }
+  return currentScope === null
     ? null
-    : { scope: sessionScope, sessionId: route.sessionId }
+    : { scope: currentScope, sessionId }
 }
 
 const browserDockerGitScopeBootstrap = (
@@ -719,13 +729,20 @@ const verifySkillerRouteScope = (
   if (route.sessionId === null) {
     return Effect.void
   }
-  const scope = sessionScopes.get(route.sessionId)
-  if (scope === undefined) {
+  const currentScope = currentProcess !== null && isRunning(currentProcess.process)
+    ? currentProcess.scope
+    : null
+  const selection = resolveSkillerRouteScopeSelection(
+    route.sessionId,
+    currentScope,
+    (sessionId) => sessionScopes.get(sessionId)
+  )
+  if (selection === null) {
     return Effect.fail(new ApiConflictError({
       message: `Skiller session is not registered: ${route.sessionId}. Click the terminal Skiller button again.`
     }))
   }
-  if (currentProcess === null || !sameSkillerScope(currentProcess.scope, scope)) {
+  if (currentProcess === null || !sameSkillerScope(currentProcess.scope, selection.scope)) {
     return Effect.fail(new ApiConflictError({
       message: `Skiller is not running for terminal session ${route.sessionId}. Click the terminal Skiller button again.`
     }))

packages/api/src/services/skiller.ts

@@ -3,6 +3,7 @@ import { describe, expect, it } from "@effect/vitest"
 import {
   parseSkillerRoute,
   resolveSkillerBrowserScopeSelection,
+  resolveSkillerRouteScopeSelection,
   type SkillerRoute
 } from "../src/services/skiller.js"
 import type { SkillerContainerScope } from "../src/services/skiller-core.js"
@@ -74,6 +75,32 @@ describe("skiller routes", () => {
     })
   })
 
+  it("falls back to the current project scope for stale session app routes", () => {
+    const currentScope = scope("current-scope")
+
+    expect(resolveSkillerBrowserScopeSelection(
+      appRoute("/api/ssh/session/stale-session/skiller/app/"),
+      currentScope,
+      () => undefined
+    )).toEqual({
+      scope: currentScope,
+      sessionId: "stale-session"
+    })
+  })
+
+  it("falls back to the current project scope for stale session trpc routes", () => {
+    const currentScope = scope("current-scope")
+
+    expect(resolveSkillerRouteScopeSelection(
+      "stale-session",
+      currentScope,
+      () => undefined
+    )).toEqual({
+      scope: currentScope,
+      sessionId: "stale-session"
+    })
+  })
+
   it("does not inject a browser scope for unscoped Skiller app routes", () => {
     expect(resolveSkillerBrowserScopeSelection(
       appRoute("/api/skiller/app/"),

packages/api/tests/skiller-routes.test.ts

@@ -14,9 +14,6 @@ type SkillerLaunch = {
   readonly trpcBasePath: string
 }
 
-const skillerAppPathForSession = (sessionId: string): string =>
-  `/api/ssh/session/${encodeURIComponent(sessionId)}/skiller/app/`
-
 const skillerLaunchMessage = (launch: SkillerLaunch, openedPath: string, opened: boolean): string => {
   const pid = launch.pid === null ? "unknown pid" : `pid ${launch.pid}`
   const state = launch.alreadyRunning
@@ -36,15 +33,13 @@ export const openSkillerApp = (
   sessionId?: string
 ): void => {
   const resolvedProjectKey = projectKey ?? undefined
-  const immediateAppPath = sessionId === undefined ? null : skillerAppPathForSession(sessionId)
-  const immediateOpenResult = immediateAppPath === null ? null : openUrl(immediateAppPath)
   withBusy({
     context,
     effect: openSkiller(resolvedProjectKey, sessionId),
     label: "Opening Skiller",
     onSuccess: (launch) => {
-      const openedPath = immediateAppPath ?? launch.appPath
-      const opened = immediateOpenResult ?? openUrl(openedPath)
+      const openedPath = launch.appPath
+      const opened = openUrl(openedPath)
       context.setMessage(skillerLaunchMessage(launch, openedPath, opened))
     }
   })

packages/app/src/web/actions-skiller.ts

@@ -24,6 +24,7 @@ const proofScope = {
 const skillerLaunch = (
   overrides: {
     readonly alreadyRunning?: boolean
+    readonly appPath?: string
     readonly scope?: null | {
       readonly containerCodexSkillsPath: string
       readonly containerHomePath: string
@@ -36,16 +37,17 @@ const skillerLaunch = (
       readonly projectKey: string
       readonly sshUser: string
     }
+    readonly trpcBasePath?: string
   } = {}
 ) => ({
   alreadyRunning: overrides.alreadyRunning ?? false,
-  appPath: "/api/skiller/app/",
+  appPath: overrides.appPath ?? "/api/skiller/app/",
   logPath: "/home/dev/.docker-git/logs/skiller.log",
   ok: true,
   pid: 1234,
   scope: overrides.scope ?? null,
   startedAtIso: "2026-05-09T17:30:00.000Z",
-  trpcBasePath: "/api/skiller",
+  trpcBasePath: overrides.trpcBasePath ?? "/api/skiller",
   trpcPort: 17_888
 })
 
@@ -110,18 +112,35 @@ describe("web Skiller actions", () => {
       }))
     }))
 
-  it.effect("opens the session-scoped Skiller URL immediately from a terminal session", () =>
+  it.effect("opens the session-scoped Skiller URL after the backend registers the scope", () =>
     Effect.gen(function*(_) {
-      mockScopedSkillerLaunch()
+      let completeLaunch = (_launch: ReturnType<typeof skillerLaunch>): void => {
+        throw new Error("Expected Skiller launch effect to be subscribed.")
+      }
+      openUrlMock.mockReturnValue(true)
+      openSkillerMock.mockImplementation(() =>
+        Effect.async<ReturnType<typeof skillerLaunch>>((resume) => {
+          completeLaunch = (launch) => {
+            resume(Effect.succeed(launch))
+          }
+        })
+      )
       const { context, setMessage } = makeBrowserActionContext()
 
       openSkillerApp(context, "abc123", "terminal-proof")
 
-      expect(openUrlMock).toHaveBeenCalledWith("/api/ssh/session/terminal-proof/skiller/app/")
+      expect(openUrlMock).not.toHaveBeenCalled()
       yield* _(waitForAssertion(() => {
         expect(openSkillerMock).toHaveBeenCalledWith("abc123", "terminal-proof")
       }))
+      completeLaunch(skillerLaunch({
+        alreadyRunning: true,
+        appPath: "/api/ssh/session/terminal-proof/skiller/app/",
+        scope: proofScope,
+        trpcBasePath: "/api/ssh/session/terminal-proof/skiller"
+      }))
       yield* _(waitForAssertion(() => {
+        expect(openUrlMock).toHaveBeenCalledWith("/api/ssh/session/terminal-proof/skiller/app/")
         expect(setMessage).toHaveBeenCalledWith(
           "Skiller is already running (pid 1234). Log: /home/dev/.docker-git/logs/skiller.log. Container FS: dg-project:/home/dev/app. Opened /api/ssh/session/terminal-proof/skiller/app/."
         )