fix(flows): stop a recorded sub-flow pinning the device it was recorded on

packages/tool-server/src/tools/flows/flow-device.ts

@@ -15,7 +15,24 @@ import type { WhenPlatform } from "./flow-utils";
 // flow-utils, which this aliases via WhenPlatform.
 export type FlowPlatform = WhenPlatform;
 
-const DEVICE_BIND_KEYS = ["udid", "device_id"] as const;
+/**
+ * Arg names that mean "the device to act on".
+ *
+ * The runner strips these from every recorded step and re-injects the resolved
+ * run device, so a name here must mean a device id on EVERY tool that declares
+ * one — the strip is schema-blind. `udid` covers most tools, `device_id` the
+ * debugger and profiler families, and `device` is `flow-execute`'s own, so a
+ * nested flow inherits the run device instead of pinning the one it was
+ * recorded on (#607).
+ *
+ * `platform` is deliberately absent, for two independent reasons. It is only
+ * ever read when no device was given — `resolveFlowDevice` returns on
+ * `opts.device` before touching it, and the chromium boot spec is gated on
+ * `!params.device` — so once `device` is bound it is inert. And it is not
+ * device-specific on every tool: `react-profiler-analyze` declares its own
+ * `platform`, which a blind strip would silently retarget.
+ */
+const DEVICE_BIND_KEYS = ["udid", "device_id", "device"] as const;
 
 interface RawDevice {
   platform: FlowPlatform;
@@ -94,7 +111,13 @@ export async function resolveFlowDevice(
   );
 }
 
-/** Strip the device-id keys from a set of args (so a flow stores none). */
+/**
+ * Strip the device-id keys from a set of args (so a flow stores none).
+ *
+ * Schema-blind on purpose: `bindDeviceArgs` strips unconditionally and re-injects
+ * only what the target tool declares, so a stale id is never forwarded to a tool
+ * that does not want it.
+ */
 export function stripDeviceKeys(args: Record<string, unknown>): Record<string, unknown> {
   const out = { ...args };
   for (const k of DEVICE_BIND_KEYS) delete out[k];
@@ -108,6 +131,10 @@ export function stripDeviceKeys(args: Record<string, unknown>): Record<string, u
  * a stale baked-in udid can't override the run target. The id is injected only
  * for the device-id keys the tool's input schema declares (so `.strict()`
  * schemas stay valid).
+ *
+ * This covers a nested `tool: flow-execute` step too — its own `device` arg is
+ * rebound, so a composed run inherits the run device rather than driving the one
+ * it was recorded against, matching how `run:` composition already behaves.
  */
 export function bindDeviceArgs(
   registry: Registry,

packages/tool-server/test/flows/flow-composition.test.ts

@@ -10,13 +10,18 @@ import { bindDeviceArgs, stripDeviceKeys } from "../../src/tools/flows/flow-devi
 const DEVICE = "00000000-0000-0000-0000-0000000000ab";
 let tmpDir: string;
 
-function mockRegistry(): Registry {
+/**
+ * `props`, when given, makes `getTool` report that schema for EVERY tool id —
+ * so a fixture using it must be a single step and must pass an explicit device,
+ * otherwise unrelated dispatches (list-devices) would be handed a bogus schema.
+ */
+function mockRegistry(props?: Record<string, unknown>): Registry {
   return {
     invokeTool: vi.fn(async (id: string) => {
       if (id === "list-devices") return { devices: [] };
       return { ok: true };
     }),
-    getTool: vi.fn(() => undefined),
+    getTool: vi.fn(() => (props ? { inputSchema: { properties: props } } : undefined)),
     // iOS launch steps gate on a native-devtools connection: report connected
     // so the run proceeds. No selector directives run in these tests, so the
     // flow tree is never fetched.
@@ -145,6 +150,43 @@ describe("flow composition (run:)", () => {
     expect(result.ok).toBe(true);
   });
 
+  it("runs a nested flow-execute against the run device, not the recorded one (issue #607)", async () => {
+    // The raw `tool: flow-execute` form is what the recorder falls back to when
+    // the target is not a resolvable sibling — and what a remote recording always
+    // produces. Its device parameter is named `device`, which was not a bind key,
+    // so the sub-run drove the id baked in at record time. Here the flow carries
+    // a device that does not exist while the run is given a real one.
+    await writeFlow("main", {
+      executionPrerequisite: "",
+      steps: [
+        {
+          kind: "tool",
+          name: "flow-execute",
+          args: { name: "b-only", project_root: "/elsewhere", device: "STALE-ID" },
+        },
+      ],
+    });
+
+    // Single step + explicit device, per mockRegistry's contract.
+    const registry = mockRegistry({ name: {}, project_root: {}, device: {} });
+    const result = asRun(
+      await createRunFlowTool(registry).execute(
+        {},
+        { name: "main", project_root: tmpDir, device: DEVICE }
+      )
+    );
+
+    expect(registry.invokeTool).toHaveBeenCalledWith(
+      "flow-execute",
+      expect.objectContaining({ device: DEVICE })
+    );
+    expect(registry.invokeTool).not.toHaveBeenCalledWith(
+      "flow-execute",
+      expect.objectContaining({ device: "STALE-ID" })
+    );
+    expect(result.ok).toBe(true);
+  });
+
   it("detects a cyclic run reference", async () => {
     await writeFlow("a", { executionPrerequisite: "", steps: [{ kind: "run", flow: "b" }] });
     await writeFlow("b", { executionPrerequisite: "", steps: [{ kind: "run", flow: "a" }] });
@@ -263,8 +305,30 @@ describe("device binding (portability)", () => {
     expect(out).toEqual({ foo: 1 });
   });
 
-  it("stripDeviceKeys removes udid / device_id", () => {
-    expect(stripDeviceKeys({ udid: "A", device_id: "B", x: 1 })).toEqual({ x: 1 });
+  it("stripDeviceKeys removes udid / device_id / device", () => {
+    expect(stripDeviceKeys({ udid: "A", device_id: "B", device: "C", x: 1 })).toEqual({ x: 1 });
+  });
+
+  it("rebinds a nested flow-execute onto the run device (issue #607)", () => {
+    // `flow-execute`'s own device parameter is named `device`, so before it was
+    // a bind key a recorded nested step kept the id it was recorded on and the
+    // sub-run drove that device instead of the one the replay was given.
+    const out = bindDeviceArgs(
+      reg({ name: {}, project_root: {}, device: {} }),
+      "flow-execute",
+      "RESOLVED",
+      { name: "b", project_root: "/p", device: "STALE" }
+    );
+    expect(out).toEqual({ name: "b", project_root: "/p", device: "RESOLVED" });
+  });
+
+  it("leaves `platform` alone", () => {
+    // Deliberate, and pinned here so a later "symmetry" edit fails loudly. The
+    // strip is schema-blind, and `platform` is not device-specific on every tool
+    // — react-profiler-analyze declares its own — so stripping it would silently
+    // retarget an unrelated recorded step. It is also inert once `device` is
+    // bound, because device resolution returns before it is ever read.
+    expect(stripDeviceKeys({ platform: "android", x: 1 })).toEqual({ platform: "android", x: 1 });
   });
 });
 

packages/tool-server/test/flows/flow-remote-recording.test.ts

@@ -108,6 +108,36 @@ describe("flow recording with a remote client (probe miss)", () => {
     await expect(fs.stat(CLIENT_ROOT)).rejects.toThrow();
   });
 
+  it("does not bake a device id into a remotely recorded flow-execute step (issue #607)", async () => {
+    // A remote recording ALWAYS keeps the raw `tool: flow-execute` step —
+    // `run:` composition is host-resolved, so captureRunTarget bails before it
+    // can rewrite. That makes this path the main real-world producer of a flow
+    // with a record-time device id baked in, which then pinned every replay.
+    const registry = createMockRegistry({
+      "flow-execute": { result: { ok: true, steps: [] } },
+    });
+    const addStep = createFlowAddStepTool(registry);
+
+    await flowStartRecordingTool.execute(
+      {},
+      { name: "remote-flow", project_root: CLIENT_ROOT, executionPrerequisite: "Home" },
+      remoteCtx()
+    );
+
+    const stepResult = await addStep.execute(
+      {},
+      {
+        command: "flow-execute",
+        args: JSON.stringify({ name: "sub", project_root: CLIENT_ROOT, device: "RECORD-TIME-ID" }),
+      }
+    );
+
+    const directive = stepResult.savedTo as { content: string };
+    expect(parseFlow(directive.content).steps).toEqual([
+      { kind: "tool", name: "flow-execute", args: { name: "sub", project_root: CLIENT_ROOT } },
+    ]);
+  });
+
   it("finish-recording summarizes the in-memory flow and clears the session", async () => {
     await flowStartRecordingTool.execute(
       {},

packages/tool-server/test/flows/flow-tools.test.ts

@@ -462,6 +462,31 @@ describe("flow-add-step", () => {
     ]);
   });
 
+  it("strips the device id from a raw flow-execute step (issue #607)", async () => {
+    // Deliberately a target that is NOT a resolvable sibling: a resolvable one
+    // records as `run:`, which carries no args at all and so could never show
+    // this. The raw fallback is the form that kept the record-time device id and
+    // pinned every replay to it.
+    const registry = createMockRegistry({
+      "flow-execute": { result: { ok: true, steps: [] } },
+    });
+    const tool = createFlowAddStepTool(registry);
+
+    await flowStartRecordingTool.execute({}, { name: "compose-pinned", project_root: tmpDir });
+
+    const result = await tool.execute(
+      {},
+      {
+        command: "flow-execute",
+        args: JSON.stringify({ name: "elsewhere", project_root: tmpDir, device: "ABC" }),
+      }
+    );
+
+    expect(parseFlow(result.flowFile).steps).toEqual([
+      { kind: "tool", name: "flow-execute", args: { name: "elsewhere", project_root: tmpDir } },
+    ]);
+  });
+
   it("throws on invalid JSON in args", async () => {
     const registry = createMockRegistry({
       tap: { result: { ok: true } },