🤖 fix: offer a legacy plan import only when the migration found the shared plan alone; list only migrated SSH ancestors

Author: ThomasK33

src/node/services/turnContextAssembler.test.ts

@@ -1403,9 +1403,10 @@ describe("buildStreamSystemContext", () => {
     });
   }
 
-  // #5174: SSH ancestors are listed by their installation-scoped path only, even one from an older
-  // build that has not migrated yet: a turn never points the agent at a legacy plan file.
-  test("lists only installation-scoped plan paths for SSH ancestors", async () => {
+  // #5174: SSH ancestors are listed by their installation-scoped path only. One from an older build
+  // that has not migrated yet is left out (its plan is not at that path yet), and a turn never
+  // points the agent at a legacy plan file.
+  test("lists migrated SSH ancestors by their installation-scoped path, and no legacy path", async () => {
     using tempRoot = new DisposableTempDir("stream-system-context");
     const projectPath = path.join(tempRoot.path, "project");
     const xumHome = path.join(tempRoot.path, "mux-home");
@@ -1464,9 +1465,6 @@ describe("buildStreamSystemContext", () => {
       mcpServers: {},
     });
 
-    expect(result.ancestorPlanFilePaths).toEqual([
-      scoped("child-workspace"),
-      scoped("parent-workspace"),
-    ]);
+    expect(result.ancestorPlanFilePaths).toEqual([scoped("child-workspace")]);
   });
 });

src/node/services/turnContextAssembler.ts

@@ -609,6 +609,8 @@ interface WorkspaceConfigLookupEntry {
   projectName: string;
   projectPath: string;
   parentWorkspaceId: string | undefined;
+  /** The row's #5174 migration flag: an SSH row without it may have no plan at its scoped path. */
+  remotePlanMigrated: boolean;
 }
 
 interface AncestorPlanPathEntry {
@@ -639,6 +641,7 @@ function buildWorkspaceConfigLookup(cfg: ProjectsConfig): Map<string, WorkspaceC
           : projectName,
         projectPath: workspace.projects?.[0]?.projectPath ?? projectPath,
         parentWorkspaceId: workspace.parentWorkspaceId,
+        remotePlanMigrated: workspace.remotePlanMigrated === true,
       });
     }
   }
@@ -719,11 +722,14 @@ function resolveAncestorPlanContext(args: {
     }
 
     // Same storage as this workspace's runtime. On SSH that is the installation-scoped tree
-    // (#5174), which needs this installation's identity: without it the ancestor is skipped.
+    // (#5174), which needs this installation's identity: without it the ancestor is skipped. So
+    // is an SSH ancestor from before #5174 that has not migrated yet: its plan is not at its
+    // scoped path until its own next plan access migrates it, and a turn never points the agent
+    // at a legacy file.
     const xumHome = args.runtime.getXumHome();
     const ancestorPlanFilePath = !usesInstallationScopedPlans(args.metadata.runtimeConfig)
       ? getPlanFilePath(currentWorkspace.workspaceName, currentWorkspace.projectName, xumHome)
-      : args.installationId === undefined
+      : args.installationId === undefined || !currentWorkspace.remotePlanMigrated
         ? undefined
         : getInstallationScopedPlanFilePath(
             currentWorkspace.workspaceName,

src/node/services/workspaceService.remotePlanNamespace.test.ts

@@ -236,6 +236,24 @@ describe("SSH plans are installation-scoped (#5174)", () => {
     });
   });
 
+  test("a row left unmarked by a failed flag write is not offered the shared plan", async () => {
+    await withTempMuxRoot(async () => {
+      await addOlderRow();
+      await writeIdPlan();
+      await writeSharedPlan();
+      spyOn(harness.config, "markRemotePlanMigrated").mockRejectedValue(
+        new Error("config write failed")
+      );
+
+      expect(await planContent()).toBe(ID_PLAN);
+      expect(migrated()).toBe(false);
+
+      expect(await offer()).toBeNull();
+      const renamed = await harness.service.rename(id, "renamed");
+      expect(renamed.success ? "" : renamed.error).toBe("");
+    });
+  });
+
   test("an import never replaces a plan already in this installation's path", async () => {
     await withTempMuxRoot(async () => {
       await addOlderRow();

src/node/services/workspaceService.ts

@@ -133,7 +133,7 @@ import { isWorkspaceTrustedForSharedExecution } from "@/node/services/utils/work
 import { mergeMultiProjectSecrets } from "@/node/services/utils/multiProjectSecrets";
 import { getLegacyPlanFilePath, sharesPlanDirectory } from "@/common/utils/planStorage";
 import {
-  getImportableLegacyPlanPath,
+  findImportableLegacyPlanPath,
   importLegacyRemotePlan,
   markRemotePlanMigratedForDeletion,
   resolvePlanFileLocation,
@@ -5605,8 +5605,8 @@ export class WorkspaceService
   /**
    * The shared pre-#5174 plan path the user can import into this SSH workspace (Option B): only
    * for a row from before #5174 whose only legacy plan is that file. Null otherwise, and for
-   * non-SSH workspaces. Resolving the location runs the row's automatic migration first, so an
-   * own plans/<id>.md plan is migrated rather than offered.
+   * non-SSH workspaces. This runs the row's automatic migration first, so an own plans/<id>.md
+   * plan is migrated rather than offered.
    */
   public async getImportableLegacyPlan(workspaceId: string): Promise<Result<string | null>> {
     const metadata = await this.getInfo(workspaceId);
@@ -5615,8 +5615,7 @@ export class WorkspaceService
     try {
       const runtime = createRuntimeForWorkspace(metadata);
       const owner = { ...metadata, id: workspaceId };
-      await this.resolvePlanLocation(owner, runtime);
-      const legacyPath = getImportableLegacyPlanPath(this.config, runtime, owner);
+      const legacyPath = await findImportableLegacyPlanPath(this.config, runtime, owner);
       return Ok(legacyPath === undefined ? null : await runtime.resolvePath(legacyPath));
     } catch (error) {
       return Err(`Failed to check for a plan from an older Xum: ${getErrorMessage(error)}`);
@@ -9964,7 +9963,7 @@ export class WorkspaceService
         // A row whose only legacy plan is the shared basename file stays unmigrated until the
         // user imports it (Option B). That file is named after the workspace, so after a rename
         // the offer would look at another path and the plan would be orphaned: refuse instead.
-        const importable = getImportableLegacyPlanPath(this.config, runtimeForPlanFile, {
+        const importable = await findImportableLegacyPlanPath(this.config, runtimeForPlanFile, {
           ...oldMetadata,
           id: workspaceId,
         });

src/node/utils/runtime/planLocation.ts

@@ -256,19 +256,24 @@ async function migrateRemotePlan(
 }
 
 /**
- * The shared pre-#5174 plan path a user can import into an SSH row (Option B): set only while the
- * row is unmigrated after its automatic migration, which means its only legacy plan is that file.
- * Call after resolvePlanFileLocation. Undefined for other rows.
+ * The shared pre-#5174 plan path a user can import into an SSH row (Option B), or undefined. Runs
+ * the row's automatic migration (as resolvePlanFileLocation does) and offers the path only when
+ * that migration found the shared file to be the row's only legacy plan, so a row left unmarked by
+ * a failed flag write is never offered a file that is not there.
  */
-export function getImportableLegacyPlanPath(
-  config: Pick<PlanLocationConfig, "isRemotePlanMigrated">,
+export async function findImportableLegacyPlanPath(
+  config: PlanLocationConfig,
   runtime: Runtime,
   owner: PlanOwner
-): string | undefined {
+): Promise<string | undefined> {
   if (!usesInstallationScopedPlans(owner.runtimeConfig) || config.isRemotePlanMigrated(owner.id)) {
     return undefined;
   }
-  return getPlanFilePath(owner.name, owner.projectName, runtime.getXumHome());
+  const planPath = await resolvePlanFilePath(config, runtime, owner);
+  const outcome = await migrateRemotePlan(config, runtime, owner, planPath, false);
+  return outcome === "sharedOnly"
+    ? getPlanFilePath(owner.name, owner.projectName, runtime.getXumHome())
+    : undefined;
 }
 
 /** Result of importLegacyRemotePlan. */