Browse Source

Merge pull request #764 from mhenke/fix/762-concurrent-spawn-race

Alvin 2 weeks ago
parent
commit
9543fd08f0
2 changed files with 65 additions and 0 deletions
  1. 44 0
      src/multiplexer/herdr/index.test.ts
  2. 21 0
      src/multiplexer/herdr/index.ts

+ 44 - 0
src/multiplexer/herdr/index.test.ts

@@ -757,4 +757,48 @@ describe('HerdrMultiplexer', () => {
     // No CLI commands should be issued
     expect(commands()).toHaveLength(0);
   });
+
+  // Regression: concurrent spawns must not race on agentAreaPaneId (#762)
+  test('main-vertical: concurrent spawns produce one parent split + down splits', async () => {
+    let splitCount = 0;
+    crossSpawnMock.mockImplementation((command: string[]) => {
+      if (command[0] === 'which') {
+        return createSpawnResult(0, '/usr/bin/herdr\n');
+      }
+      if (command.includes('split')) {
+        splitCount++;
+        // Return sequential pane IDs so we can track which split is which
+        return createSpawnResult(
+          0,
+          `${createSplitResponse(`w1:p${splitCount + 1}`)}\n`,
+        );
+      }
+      return createSpawnResult();
+    });
+
+    const { HerdrMultiplexer } = await importFreshHerdr();
+    const herdr = new HerdrMultiplexer('main-vertical', 60);
+
+    // Fire both concurrently — before the fix, both would see
+    // agentAreaPaneId === null and split from the parent (w1:p1).
+    const [r1, r2] = await Promise.all([
+      herdr.spawnPane('s1', 'Agent 1', 'http://localhost:4096', '/repo'),
+      herdr.spawnPane('s2', 'Agent 2', 'http://localhost:4096', '/repo'),
+    ]);
+
+    expect(r1.success).toBe(true);
+    expect(r2.success).toBe(true);
+
+    const splitCommands = commands().filter((c) => c.includes('split'));
+    // Exactly 2 splits total
+    expect(splitCommands.length).toBe(2);
+
+    // First split: parent (w1:p1) → right
+    expect(splitCommands[0][3]).toBe('w1:p1');
+    expect(splitCommands[0]).toContain('right');
+
+    // Second split: agent area (w1:p2) → down
+    expect(splitCommands[1][3]).toBe('w1:p2');
+    expect(splitCommands[1]).toContain('down');
+  });
 });

+ 21 - 0
src/multiplexer/herdr/index.ts

@@ -42,6 +42,8 @@ export class HerdrMultiplexer implements Multiplexer {
   private layout: MultiplexerLayout;
   private paneDirection: HerdrPaneDirection;
   private agentAreaPaneId: string | null = null;
+  // ponytail: serialize spawnPane to prevent concurrent races on agentAreaPaneId
+  private spawnMutex: Promise<unknown> = Promise.resolve();
 
   constructor(layout: MultiplexerLayout = 'main-vertical', mainPaneSize = 60) {
     // Herdr does not support exact main pane sizing like tmux.
@@ -70,6 +72,25 @@ export class HerdrMultiplexer implements Multiplexer {
     description: string,
     serverUrl: string,
     directory: string,
+  ): Promise<PaneResult> {
+    // ponytail: serialize concurrent spawns to prevent races on agentAreaPaneId
+    const prev = this.spawnMutex;
+    let release!: () => void;
+    this.spawnMutex = new Promise<void>((r) => (release = r));
+    await prev;
+
+    try {
+      return await this.doSpawn(sessionId, description, serverUrl, directory);
+    } finally {
+      release();
+    }
+  }
+
+  private async doSpawn(
+    sessionId: string,
+    description: string,
+    serverUrl: string,
+    directory: string,
   ): Promise<PaneResult> {
     const herdr = await this.getBinary();
     if (!herdr) {