Browse Source

fix loop review feedback

Zaradacht Taifour 1 month ago
parent
commit
90d08d9bfe

+ 5 - 4
src/hooks/loop-command/index.test.ts

@@ -46,8 +46,7 @@ describe('loop command hook', () => {
       {
         command: 'loop',
         sessionID: 's1',
-        arguments:
-          'fix typescript errors until typecheck passes, max 3 tries',
+        arguments: 'fix typescript errors until typecheck passes, max 3 tries',
       },
       output,
     );
@@ -55,7 +54,9 @@ describe('loop command hook', () => {
     const text = output.parts[0].text;
     expect(output.parts.length).toBe(1);
     expect(text).toContain('The user ran `/loop`');
-    expect(text).toContain('fix typescript errors until typecheck passes, max 3 tries');
+    expect(text).toContain(
+      'fix typescript errors until typecheck passes, max 3 tries',
+    );
     expect(text).toContain('goal, successCriteria, maxAttempts');
     expect(text).toContain('missing or unclear');
     expect(text).toContain('.opencode/loop-history/');
@@ -74,4 +75,4 @@ describe('loop command hook', () => {
     expect(output.parts.length).toBe(1);
     expect(output.parts[0].text).toBe('original');
   });
-});
+});

+ 2 - 4
src/hooks/loop-command/index.ts

@@ -52,9 +52,7 @@ export function createLoopCommandHook(): {
 } {
   return {
     registerCommand: (opencodeConfig) => {
-      const cfg = opencodeConfig.command as
-        | Record<string, unknown>
-        | undefined;
+      const cfg = opencodeConfig.command as Record<string, unknown> | undefined;
       if (cfg?.[COMMAND_NAME]) return;
       if (!opencodeConfig.command) opencodeConfig.command = {};
       (opencodeConfig.command as Record<string, unknown>)[COMMAND_NAME] = {
@@ -77,4 +75,4 @@ export function createLoopCommandHook(): {
       output.parts.push({ type: 'text', text: activationPrompt(args) });
     },
   };
-}
+}

+ 2 - 2
src/index.ts

@@ -25,12 +25,12 @@ import {
   createDelegateTaskRetryHook,
   createFilterAvailableSkillsHook,
   createJsonErrorRecoveryHook,
+  createLoopCommandHook,
   createPhaseReminderHook,
   createPostFileToolNudgeHook,
   createReflectCommandHook,
   createTaskSessionManagerHook,
   ForegroundFallbackManager,
-  createLoopCommandHook,
 } from './hooks';
 import { processImageAttachments } from './hooks/image-hook';
 import type { MessageWithParts } from './hooks/types';
@@ -263,7 +263,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       multiplexerConfig,
       backgroundJobBoard,
     );
-    backgroundJobBoard.setTerminalStateListener((taskID) => {
+    backgroundJobBoard.addTerminalStateListener((taskID) => {
       void multiplexerSessionManager.retryDeferredIdleClose(taskID);
     });
 

+ 35 - 1
src/loop/loop-session.test.ts

@@ -1,9 +1,11 @@
-import { describe, expect, test } from 'bun:test';
+import { describe, expect, spyOn, test } from 'bun:test';
+import * as fs from 'node:fs';
 import {
   compactAttempt,
   createLoopSession,
   type LoopDefinition,
   loopDirname,
+  writeHistoryFile,
 } from './loop-session';
 
 function testDef(overrides?: Partial<LoopDefinition>): LoopDefinition {
@@ -84,3 +86,35 @@ describe('compactAttempt', () => {
     expect(result).toContain('artifacts: src/output.ts, src/output.test.ts');
   });
 });
+
+describe('writeHistoryFile', () => {
+  test('uses the attempt number for the history filename', () => {
+    const mkdirSpy = spyOn(fs, 'mkdirSync').mockImplementation(() => undefined);
+    const writeSpy = spyOn(fs, 'writeFileSync').mockImplementation(
+      () => undefined,
+    );
+    const session = createLoopSession(testDef(), 'loop-test-1');
+    session.attempts = 99;
+    session.history.push({
+      attemptNumber: 4,
+      executionResult: 'bun test',
+      verificationResult: { passed: true, reason: 'ok' },
+    });
+
+    try {
+      writeHistoryFile(session);
+
+      expect(mkdirSpy).toHaveBeenCalledWith(session.historyDir, {
+        recursive: true,
+      });
+      expect(writeSpy).toHaveBeenCalledWith(
+        expect.stringContaining('history-004.md'),
+        expect.stringContaining('## Attempt 4'),
+        { encoding: 'utf-8' },
+      );
+    } finally {
+      mkdirSpy.mockRestore();
+      writeSpy.mockRestore();
+    }
+  });
+});

+ 1 - 1
src/loop/loop-session.ts

@@ -109,7 +109,7 @@ export function writeHistoryFile(session: LoopSession): void {
   if (!lastAttempt) return;
   const attemptFile = join(
     session.historyDir,
-    `history-${String(session.attempts).padStart(3, '0')}.md`,
+    `history-${String(lastAttempt.attemptNumber).padStart(3, '0')}.md`,
   );
   mkdirSync(session.historyDir, { recursive: true });
   const content = compactAttempt(lastAttempt);

+ 34 - 0
src/utils/background-job-board.test.ts

@@ -73,6 +73,40 @@ describe('BackgroundJobBoard', () => {
     });
   });
 
+  test('resets timeout convergence when a timed out job completes', () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'ses_1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'implement parser',
+    });
+
+    board.updateStatus({
+      taskID: 'ses_1',
+      state: 'running',
+      timedOut: true,
+    });
+    board.updateStatus({
+      taskID: 'ses_1',
+      state: 'running',
+      timedOut: true,
+    });
+
+    const completed = board.updateStatus({
+      taskID: 'ses_1',
+      state: 'completed',
+      timedOut: true,
+    });
+
+    expect(completed).toMatchObject({
+      state: 'completed',
+      timedOut: true,
+      timeoutCount: 0,
+    });
+    expect(board.hasConvergenceSignals('ses_1')).toBe(false);
+  });
+
   test('formats running and terminal unreconciled jobs for prompt', () => {
     const board = new BackgroundJobBoard();
     board.registerLaunch({

+ 1 - 1
src/utils/background-job-board.ts

@@ -210,7 +210,7 @@ export class BackgroundJobBoard {
       updated.totalErrors = (existing.totalErrors ?? 0) + 1;
       updated.lastErrorAt = updated.updatedAt;
     }
-    if (input.timedOut) {
+    if (input.timedOut && input.state !== 'completed') {
       updated.timeoutCount = (existing.timeoutCount ?? 0) + 1;
     }