소스 검색

fix(herdr): align spec with code, restore parse log, test fallback path

- Update main-vertical design spec to match shipped implementation
  (drop wasFirstChild, fix readonly modifiers, correct closePane/applyLayout
  pseudocode placement)
- Restore raw stdout in spawnPane parse-failure log via runSplit return payload
- Add test for implicit agent-area split-failure fallback path

Closes #668
Michael Henke 4 주 전
부모
커밋
d33d56f33a
3개의 변경된 파일134개의 추가작업 그리고 33개의 파일을 삭제
  1. 35 26
      docs/superpowers/specs/2026-07-07-herdr-main-vertical-design.md
  2. 82 0
      src/multiplexer/herdr/index.test.ts
  3. 17 7
      src/multiplexer/herdr/index.ts

+ 35 - 26
docs/superpowers/specs/2026-07-07-herdr-main-vertical-design.md

@@ -21,8 +21,8 @@ Track a single `agentAreaPaneId` — the first child pane created in the right c
 ### State Changes (`HerdrMultiplexer`)
 
 ```typescript
-private readonly layout: MultiplexerLayout;        // stored from constructor
-private readonly paneDirection: HerdrPaneDirection; // kept for non-main layouts
+private layout: MultiplexerLayout;                  // mutable — applyLayout writes it
+private paneDirection: HerdrPaneDirection;          // mutable — applyLayout writes it
 private agentAreaPaneId: string | null = null;      // tracks first child in right column
 ```
 
@@ -37,59 +37,68 @@ spawnPane(sessionId, description, serverUrl, directory):
   1. Get herdr binary (existing)
 
   2. Determine split target and direction:
-     let target, direction, wasFirstChild = false
+     let paneId = null
 
      if layout === 'main-vertical' AND agentAreaPaneId is set:
-       target = [agentAreaPaneId]
-       direction = 'down'
-       result = split(target, direction)
-       if result failed:
-         log('[herdr] agent area split failed, falling back to parent', {stderr})
+       paneId = runSplit(target=[agentAreaPaneId], direction='down')
+       if paneId is null:
+         log('[herdr] agent area split failed, falling back to parent', {...})
          agentAreaPaneId = null
 
-     if agentAreaPaneId is null:  // first child OR fallback
-       target = targetPaneArg()   // parentPaneId or --current
-       direction = paneDirection  // 'right' for main-vertical
-       wasFirstChild = true
+     if agentAreaPaneId is null:  // first child OR fallback from failed split
+       paneId = runSplit(target=targetPaneArg(), direction=paneDirection)
 
-   3. If wasFirstChild: split(target, direction)  // 1st child or fallback path
-   4. Parse pane_id from output
-   5. If wasFirstChild AND layout === 'main-vertical':
-       agentAreaPaneId = newPaneId
-   6. Rename pane, run opencode attach (existing)
+     if paneId is null:
+       log('[herdr] spawnPane: could not parse pane_id from output', {stdout})
+       return { success: false }
+
+    3. Rename pane, run opencode attach (existing)
+    4. If layout === 'main-vertical' AND agentAreaPaneId is null:
+        agentAreaPaneId = paneId
 ```
 
 Key properties:
-- **First child** splits parent → right, creating the agent area
-- **Subsequent children** split agent area → down, stacking vertically
+- **First child**: agentAreaPaneId is null → splits parent → right, creating the agent area
+- **Subsequent children**: agentAreaPaneId is set → splits agent area → down, stacking vertically
 - **Stale reference** (agent area pane closed externally): split fails → log → clear → fall through to parent split (re-creates agent area)
-- **Explicit `wasFirstChild` flag** avoids fragile implicit detection
+- **Implicit `!agentAreaPaneId` gate** replaces the `wasFirstChild` flag — the same `null` check handles both first-child and fallback
 - **Gated on `main-vertical` only** — other layouts unchanged
 
 ### `closePane` Change
 
-Placement: after the existing `if (!paneId || paneId === 'unknown') return true;` guard (after guard), before `getBinary()`.
+`agentAreaPaneId` is cleared **inside** the success branch, only after the close command confirms success (exit code 0 or 1).
 
 ```typescript
 async closePane(paneId: string): Promise<boolean> {
   if (!paneId || paneId === 'unknown') return true;
 
-  if (paneId === this.agentAreaPaneId) {
-    this.agentAreaPaneId = null;  // next spawn re-creates from parent
+  const herdr = await this.getBinary();
+  // ... send Ctrl+C, wait, run close ...
+
+  const exitCode = await proc.exited;
+  const stderr = await proc.stderr();
+
+  // Inside the success branch only
+  if (exitCode === 0 || exitCode === 1) {
+    if (paneId === this.agentAreaPaneId) {
+      this.agentAreaPaneId = null;  // next spawn re-creates from parent
+    }
+    return true;
   }
 
-  const herdr = await this.getBinary();
-  // ... existing logic unchanged
+  return false;
 }
 ```
 
 ### `applyLayout` Change
 
 ```typescript
-async applyLayout(_layout: MultiplexerLayout, _mainPaneSize: number): Promise<void> {
+async applyLayout(layout: MultiplexerLayout, _mainPaneSize: number): Promise<void> {
   // ponytail: herdr has no rebalancing API; clear agent area so a layout
   // switch starts fresh from the parent pane.
   this.agentAreaPaneId = null;
+  this.layout = layout;
+  this.paneDirection = getPaneDirection(layout);
 }
 ```
 

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

@@ -592,6 +592,88 @@ describe('HerdrMultiplexer', () => {
     ]);
   });
 
+  test('main-vertical: implicit fallback when agent area split fails', async () => {
+    const { HerdrMultiplexer } = await importFreshHerdr();
+    const herdr = new HerdrMultiplexer('main-vertical', 60);
+
+    // First spawn creates agent area (w1:p2)
+    await herdr.spawnPane('s1', 'A1', 'http://localhost:4096', '/repo');
+
+    // Mock so split targeting w1:p2 (agent area, direction=down) fails
+    // but split targeting w1:p1 (parent, direction=right) succeeds
+    crossSpawnMock.mockImplementation((command: string[]) => {
+      if (command[0] === 'which') {
+        return createSpawnResult(0, '/usr/bin/herdr\n');
+      }
+      if (
+        command.includes('split') &&
+        command.includes('w1:p2') &&
+        command.includes('down')
+      ) {
+        return createSpawnResult(1, '', 'agent area pane gone');
+      }
+      if (command.includes('split')) {
+        return createSpawnResult(0, `${createSplitResponse('w1:p3')}\n`);
+      }
+      return createSpawnResult();
+    });
+
+    // Second spawn: agent area split fails → falls back to parent split
+    const result = await herdr.spawnPane(
+      's2',
+      'A2',
+      'http://localhost:4096',
+      '/repo',
+    );
+
+    expect(result).toEqual({ success: true, paneId: 'w1:p3' });
+
+    const splitCommands = commands().filter((c) => c.includes('split'));
+
+    // 1st: parent split → right (first spawn)
+    expect(splitCommands[0]).toEqual([
+      '/usr/bin/herdr',
+      'pane',
+      'split',
+      'w1:p1',
+      '--direction',
+      'right',
+      '--cwd',
+      '/repo',
+      '--no-focus',
+    ]);
+
+    // 2nd: agent area split → down (second spawn) — fails
+    expect(splitCommands[1]).toEqual([
+      '/usr/bin/herdr',
+      'pane',
+      'split',
+      'w1:p2',
+      '--direction',
+      'down',
+      '--cwd',
+      '/repo',
+      '--no-focus',
+    ]);
+
+    // 3rd: parent split → right (fallback from failed agent area split)
+    expect(splitCommands[2]).toEqual([
+      '/usr/bin/herdr',
+      'pane',
+      'split',
+      'w1:p1',
+      '--direction',
+      'right',
+      '--cwd',
+      '/repo',
+      '--no-focus',
+    ]);
+
+    // agentAreaPaneId was updated to the new pane from the parent split
+    // @ts-expect-error - accessing private for test
+    expect(herdr.agentAreaPaneId).toBe('w1:p3');
+  });
+
   test('closePane clears agentAreaPaneId when agent area closed', async () => {
     const { HerdrMultiplexer } = await importFreshHerdr();
     const herdr = new HerdrMultiplexer('main-vertical', 60);

+ 17 - 7
src/multiplexer/herdr/index.ts

@@ -74,9 +74,15 @@ export class HerdrMultiplexer implements Multiplexer {
 
     try {
       let paneId: string | null = null;
+      let lastRawOutput = '';
 
       if (this.layout === 'main-vertical' && this.agentAreaPaneId) {
-        paneId = await this.runSplit([this.agentAreaPaneId], 'down', directory);
+        const result = await this.runSplit(
+          [this.agentAreaPaneId],
+          'down',
+          directory,
+        );
+        paneId = result.paneId;
         if (!paneId) {
           log('[herdr] agent area split failed, falling back to parent', {
             agentAreaPaneId: this.agentAreaPaneId,
@@ -86,15 +92,19 @@ export class HerdrMultiplexer implements Multiplexer {
       }
 
       if (!this.agentAreaPaneId) {
-        paneId = await this.runSplit(
+        const result = await this.runSplit(
           this.targetPaneArg(),
           this.paneDirection,
           directory,
         );
+        paneId = result.paneId;
+        lastRawOutput = result.rawOutput;
       }
 
       if (!paneId) {
-        log('[herdr] spawnPane: could not parse pane_id from output');
+        log('[herdr] spawnPane: could not parse pane_id from output', {
+          stdout: lastRawOutput,
+        });
         return { success: false };
       }
 
@@ -204,9 +214,9 @@ export class HerdrMultiplexer implements Multiplexer {
     target: string[],
     direction: HerdrPaneDirection,
     directory: string,
-  ): Promise<string | null> {
+  ): Promise<{ paneId: string | null; rawOutput: string }> {
     const herdr = await this.getBinary();
-    if (!herdr) return null;
+    if (!herdr) return { paneId: null, rawOutput: '' };
 
     const splitArgs = [
       herdr,
@@ -236,10 +246,10 @@ export class HerdrMultiplexer implements Multiplexer {
         exitCode: splitExitCode,
         stderr: splitStderr.trim(),
       });
-      return null;
+      return { paneId: null, rawOutput: splitStdout.trim() };
     }
 
-    return parsePaneId(splitStdout);
+    return { paneId: parsePaneId(splitStdout), rawOutput: splitStdout.trim() };
   }
 
   private targetPaneArg(): string[] {