refactor(ts-sdk): remove duplicated code across modules

- RunModeType/RunMode: re-export from common/constants.ts in sandbox/types.ts
- extractNohupPid: re-export from utils/http.ts in sandbox/utils.ts
- sleep: import from utils/retry.ts in model/client.ts instead of private method

Addresses code duplication feedback from PR alibaba#492 review.

Author: Timandes

rock/ts-sdk/src/model/client.test.ts

@@ -9,6 +9,46 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'fs';
 import { join } from 'path';
 import { tmpdir } from 'os';
 import { ModelClient, PollOptions } from './client.js';
+import * as retryUtils from '../utils/retry.js';
+
+// Test that ModelClient uses the shared sleep function
+describe('sleep function', () => {
+  test('ModelClient should use shared sleep from utils/retry.ts', async () => {
+    // This test verifies that ModelClient uses the shared sleep function
+    // by spying on the retry module's sleep function
+    
+    const testDir = join(tmpdir(), `model-client-sleep-test-${Date.now()}`);
+    mkdirSync(testDir, { recursive: true });
+    const logFile = join(testDir, 'model.log');
+    
+    try {
+      // Create log file with request that has index 0
+      const REQUEST_START_MARKER = '__REQUEST_START__';
+      const REQUEST_END_MARKER = '__REQUEST_END__';
+      writeFileSync(logFile, `${REQUEST_START_MARKER}test${REQUEST_END_MARKER}{"index":0}\n`);
+      
+      const client = new ModelClient({ logFileName: logFile });
+      
+      // Spy on the sleep function from retry module
+      const sleepSpy = jest.spyOn(retryUtils, 'sleep').mockResolvedValue();
+      
+      // popRequest for index 1 should poll and call sleep
+      // before timing out (we set a very short timeout)
+      try {
+        await client.popRequest(1, { timeout: 0.1 });
+      } catch {
+        // Expected to timeout
+      }
+      
+      // If ModelClient uses shared sleep, the spy should have been called
+      expect(sleepSpy).toHaveBeenCalled();
+      
+      sleepSpy.mockRestore();
+    } finally {
+      rmSync(testDir, { recursive: true, force: true });
+    }
+  });
+});
 
 // Test markers (must match client.ts)
 const REQUEST_START_MARKER = '__REQUEST_START__';

rock/ts-sdk/src/model/client.ts

@@ -5,6 +5,7 @@
 import { existsSync, readFileSync, appendFileSync } from 'fs';
 import { initLogger } from '../logger.js';
 import { envVars } from '../env_vars.js';
+import { sleep } from '../utils/retry.js';
 
 const logger = initLogger('rock.model.client');
 
@@ -136,15 +137,15 @@ export class ModelClient {
         }
 
         logger.debug(`Last request is not the index ${index} we want, waiting...`);
-        await this.sleep(1000);
+        await sleep(1000);
       } catch (e) {
         // Re-throw abort errors
         if (e instanceof Error && e.message.includes('aborted')) {
           throw e;
         }
         // For other errors (like file not found), wait and retry
         logger.debug(`Error reading request: ${e}, waiting...`);
-        await this.sleep(1000);
+        await sleep(1000);
       }
     }
   }
@@ -172,7 +173,7 @@ export class ModelClient {
 
       if (!existsSync(this.logFile)) {
         logger.debug(`Log file ${this.logFile} not found, waiting...`);
-        await this.sleep(1000);
+        await sleep(1000);
         continue;
       }
 
@@ -181,7 +182,7 @@ export class ModelClient {
 
       if (lines.length === 0) {
         logger.debug(`Log file ${this.logFile} is empty, waiting for the first request...`);
-        await this.sleep(1000);
+        await sleep(1000);
         continue;
       }
 
@@ -251,8 +252,4 @@ export class ModelClient {
     const metaJson = JSON.stringify(meta);
     return `${RESPONSE_START_MARKER}${lastResponse}${RESPONSE_END_MARKER}${metaJson}\n`;
   }
-
-  private sleep(ms: number): Promise<void> {
-    return new Promise((resolve) => setTimeout(resolve, ms));
-  }
 }

rock/ts-sdk/src/sandbox/types.test.ts

@@ -0,0 +1,39 @@
+/**
+ * Tests for Sandbox Types
+ * 
+ * This test verifies that RunModeType and RunMode are re-exported from
+ * common/constants.ts rather than duplicated.
+ */
+
+import { RunMode, RunModeType } from './types.js';
+import { RunMode as ConstantsRunMode, RunModeType as ConstantsRunModeType } from '../common/constants.js';
+
+describe('RunModeType', () => {
+  test('should be re-exported from constants', () => {
+    // This verifies that RunModeType in types.ts is the same as in constants.ts
+    // After refactoring, they should be the same reference (not just equal values)
+    expect<RunModeType>('normal').toBe('normal');
+    expect<RunModeType>('nohup').toBe('nohup');
+  });
+
+  test('should be the same reference as constants RunModeType', () => {
+    // This test will pass once we refactor to re-export from constants
+    // Currently it may fail if they are different type definitions
+    const normalMode: RunModeType = 'normal';
+    const constNormalMode: ConstantsRunModeType = normalMode;
+    expect(constNormalMode).toBe('normal');
+  });
+});
+
+describe('RunMode', () => {
+  test('should have correct values', () => {
+    expect(RunMode.NORMAL).toBe('normal');
+    expect(RunMode.NOHUP).toBe('nohup');
+  });
+
+  test('should be the same reference as constants RunMode', () => {
+    // After refactoring, RunMode should be imported from constants
+    // and re-exported, so they should be the same object
+    expect(RunMode).toBe(ConstantsRunMode);
+  });
+});

rock/ts-sdk/src/sandbox/types.ts

@@ -2,18 +2,8 @@
  * Sandbox types
  */
 
-/**
- * Run mode type
- */
-export type RunModeType = 'normal' | 'nohup';
-
-/**
- * Run mode enum
- */
-export const RunMode = {
-  NORMAL: 'normal' as const,
-  NOHUP: 'nohup' as const,
-};
+// Re-export RunModeType and RunMode from constants to avoid duplication
+export { RunModeType, RunMode } from '../common/constants.js';
 
 /**
  * Speedup type enum

rock/ts-sdk/src/sandbox/utils.test.ts

@@ -3,9 +3,16 @@
  */
 
 import { extractNohupPid } from './utils.js';
+import { extractNohupPid as httpExtractNohupPid } from '../utils/http.js';
 import { PID_PREFIX, PID_SUFFIX } from '../common/constants.js';
 
 describe('extractNohupPid', () => {
+  test('should be the same function as in utils/http.ts', () => {
+    // After refactoring, extractNohupPid should be imported from utils/http.ts
+    // and re-exported, so they should be the same function reference
+    expect(extractNohupPid).toBe(httpExtractNohupPid);
+  });
+
   test('should extract PID from valid output', () => {
     const output = `some output\n${PID_PREFIX}12345${PID_SUFFIX}\nmore output`;
     expect(extractNohupPid(output)).toBe(12345);

rock/ts-sdk/src/sandbox/utils.ts

@@ -120,15 +120,6 @@ export async function arunWithRetry(
 }
 
 /**
- * Extract nohup PID from output
+ * Re-export extractNohupPid from utils/http.ts to avoid duplication
  */
-export function extractNohupPid(output: string): number | null {
-  const PID_PREFIX = '__ROCK_PID_START__';
-  const PID_SUFFIX = '__ROCK_PID_END__';
-  const pattern = new RegExp(`${PID_PREFIX}(\\d+)${PID_SUFFIX}`);
-  const match = output.match(pattern);
-  if (match?.[1]) {
-    return parseInt(match[1], 10);
-  }
-  return null;
-}
+export { extractNohupPid } from '../utils/http.js';