refactor(agent-manager): simplify codex matching flow

codeaholicguy · codeaholicguy · commit 35b2e7083799 · 2026-02-26T16:57:55.000+01:00
diff --git a/docs/ai/design/feature-codex-adapter-agent-manager-package.md b/docs/ai/design/feature-codex-adapter-agent-manager-package.md
@@ -91,6 +91,8 @@ Responsibilities:
   - Rationale: preserves cross-agent behavior consistency and avoids adapter-specific drift.
 - Decision: Use `codex-<session-id-prefix>` fallback naming when `cwd` is unavailable.
   - Rationale: keeps identifiers deterministic and short while remaining user-readable.
+- Decision: Keep matching orchestration in explicit phases (`cwd`, `missing-cwd`, `any`) with extracted helper methods and PID/session tracking sets.
+  - Rationale: preserves behavior while reducing branching complexity and repeated scans in `detectAgents`.
 
 ## Non-Functional Requirements
 
diff --git a/docs/ai/implementation/feature-codex-adapter-agent-manager-package.md b/docs/ai/implementation/feature-codex-adapter-agent-manager-package.md
@@ -40,6 +40,7 @@ description: Implementation notes for Codex adapter support in package agent man
 - Follow `ClaudeCodeAdapter` structure for consistency.
 - Keep adapter-specific parsing in adapter module; keep formatting in CLI.
 - Fail soft on malformed/partial entries; avoid throwing across adapter boundary.
+- Keep `detectAgents` orchestration readable via small private helpers for each matching stage.
 
 ## Integration Points
 
@@ -78,3 +79,13 @@ description: Implementation notes for Codex adapter support in package agent man
     - `PROCESS_SESSION_TIME_TOLERANCE_MS`
     - `PROCESS_START_DAY_WINDOW_DAYS`
     - session-scan bound constants (`MIN/MAX/SCAN_MULTIPLIER`)
+  - Simplification refactor (no behavior change):
+    - extracted orchestration helpers:
+      - `listCodexProcesses`
+      - `calculateSessionScanLimit`
+      - `assignSessionsForMode`
+      - `addMappedSessionAgent`
+      - `addProcessOnlyAgent`
+      - `filterCandidateSessions`
+      - `rankCandidatesByStartTime`
+    - replaced repeated `agents.some(...)` PID checks with `assignedPids` set tracking
diff --git a/docs/ai/planning/feature-codex-adapter-agent-manager-package.md b/docs/ai/planning/feature-codex-adapter-agent-manager-package.md
@@ -40,6 +40,9 @@ description: Task plan for adding Codex adapter support and integrating it into
 - [x] Task 3.2: Validate and document
   - Run lint/build/tests for affected projects
   - Record implementation + testing outcomes in docs/ai
+- [x] Task 3.3: Simplify implementation structure without behavior changes
+  - Extract matching orchestration and ranking helpers for readability
+  - Replace repeated PID lookup scans with set-based tracking
 
 ## Dependencies
 
@@ -71,4 +74,4 @@ description: Task plan for adding Codex adapter support and integrating it into
 
 ## Progress Summary
 
-Implementation scope is complete. `CodexAdapter` was added to `@ai-devkit/agent-manager`, exported through package entry points, and registered in CLI agent command flows. Follow-up fixes addressed three runtime issues: false-positive process matching, missing long-lived session links, and list latency from broad session scans. Matching now uses `etime`-based process start time with configurable tolerance and process-start day-window session inclusion, while keeping a bounded recent-file scan for performance. Focused adapter and CLI agent-command tests pass, and package lint/build/test checks pass for affected areas. Full `cli:test` includes unrelated pre-existing module-resolution failures (`@ai-devkit/memory` and workspace package mocking) and is tracked as external validation noise rather than a feature regression.
+Implementation scope is complete. `CodexAdapter` was added to `@ai-devkit/agent-manager`, exported through package entry points, and registered in CLI agent command flows. Follow-up fixes addressed false-positive process matching, missing long-lived session links, and list latency from broad session scans. Matching now uses `etime`-based process start time with configurable tolerance and process-start day-window session inclusion, while keeping a bounded recent-file scan for performance. A final simplification pass extracted helper methods for match phases/ranking and introduced set-based PID assignment tracking; behavior is unchanged with focused tests still passing.
diff --git a/docs/ai/requirements/feature-codex-adapter-agent-manager-package.md b/docs/ai/requirements/feature-codex-adapter-agent-manager-package.md
@@ -28,6 +28,7 @@ Who is affected:
 - Reuse shared process/file utilities and adapter contract patterns
 - Add tests for Codex adapter discovery, status mapping, and command metadata
 - Establish a clean extension path for future adapters (Gemini/others)
+- Keep Codex adapter internals maintainable via small helper functions without changing runtime behavior
 
 ### Non-Goals
 - Reworking overall `ai-devkit agent` UX
@@ -48,6 +49,7 @@ Who is affected:
 - `packages/cli/src/commands/agent.ts` registers `CodexAdapter` from package exports
 - Unit tests cover Codex adapter happy path, empty/no-session path, and invalid data handling
 - Existing agent command tests continue to pass without regressions
+- Implementation remains readable enough for future adapter extension work (clear matching phases/helpers)
 
 ## Constraints & Assumptions
 
diff --git a/docs/ai/testing/feature-codex-adapter-agent-manager-package.md b/docs/ai/testing/feature-codex-adapter-agent-manager-package.md
@@ -59,6 +59,7 @@ description: Test strategy and coverage plan for Codex adapter integration
 
 Coverage and residual gaps:
 - New Codex adapter unit suite (`CodexAdapter.test.ts`) is passing with coverage on detection, filtering, status mapping, fallback naming, and time-based matching.
+- Post-simplification verification: focused Codex adapter tests and lint still pass after helper extraction/set-based PID tracking refactor.
 - Full `npx nx run cli:test` currently fails due to unrelated pre-existing module-resolution issues in `memory.test.ts` and baseline `agent.test.ts` mocking behavior when running the entire suite without focused filtering.
 - Runtime validation confirmed targeted mapping: PID `81442` maps to session `019c7024-89e6-7880-81eb-1417bd2177b5` after time-based matching + process-day window logic.
 
diff --git a/packages/agent-manager/src/adapters/CodexAdapter.ts b/packages/agent-manager/src/adapters/CodexAdapter.ts
@@ -43,6 +43,8 @@ interface CodexSession {
     lastPayloadType?: string;
 }
 
+type SessionMatchMode = 'cwd' | 'missing-cwd' | 'any';
+
 export class CodexAdapter implements AgentAdapter {
     readonly type = 'codex' as const;
 
@@ -69,23 +71,15 @@ export class CodexAdapter implements AgentAdapter {
     }
 
     async detectAgents(): Promise<AgentInfo[]> {
-        const codexProcesses = listProcesses({ namePattern: 'codex' }).filter((processInfo) =>
-            this.canHandle(processInfo),
-        );
+        const codexProcesses = this.listCodexProcesses();
 
         if (codexProcesses.length === 0) {
             return [];
         }
 
         const processStartByPid = this.getProcessStartTimes(codexProcesses.map((processInfo) => processInfo.pid));
 
-        const sessionScanLimit = Math.min(
-            Math.max(
-                codexProcesses.length * CodexAdapter.SESSION_SCAN_MULTIPLIER,
-                CodexAdapter.MIN_SESSION_SCAN,
-            ),
-            CodexAdapter.MAX_SESSION_SCAN,
-        );
+        const sessionScanLimit = this.calculateSessionScanLimit(codexProcesses.length);
         const sessions = this.readSessions(sessionScanLimit, processStartByPid);
         if (sessions.length === 0) {
             return codexProcesses.map((processInfo) =>
@@ -97,72 +91,114 @@ export class CodexAdapter implements AgentAdapter {
             (a, b) => b.lastActive.getTime() - a.lastActive.getTime(),
         );
         const usedSessionIds = new Set<string>();
+        const assignedPids = new Set<number>();
         const agents: AgentInfo[] = [];
 
-        // First pass: match sessions by exact cwd, then closest start time.
-        for (const processInfo of codexProcesses) {
-            const match = this.selectBestSession(
-                processInfo,
-                sortedSessions,
-                usedSessionIds,
-                processStartByPid,
-                'cwd',
-            );
+        // Match exact cwd first, then missing-cwd sessions, then any available session.
+        this.assignSessionsForMode(
+            'cwd',
+            codexProcesses,
+            sortedSessions,
+            usedSessionIds,
+            assignedPids,
+            processStartByPid,
+            agents,
+        );
+        this.assignSessionsForMode(
+            'missing-cwd',
+            codexProcesses,
+            sortedSessions,
+            usedSessionIds,
+            assignedPids,
+            processStartByPid,
+            agents,
+        );
+        this.assignSessionsForMode(
+            'any',
+            codexProcesses,
+            sortedSessions,
+            usedSessionIds,
+            assignedPids,
+            processStartByPid,
+            agents,
+        );
 
-            if (!match) {
+        // Every running codex process should still be listed.
+        for (const processInfo of codexProcesses) {
+            if (assignedPids.has(processInfo.pid)) {
                 continue;
             }
 
-            usedSessionIds.add(match.sessionId);
-            agents.push(this.mapSessionToAgent(match, processInfo, agents));
+            this.addProcessOnlyAgent(processInfo, assignedPids, agents);
         }
 
-        // Second pass: allow missing-cwd sessions to attach to unmatched codex processes.
-        for (const processInfo of codexProcesses) {
-            if (agents.some((agent) => agent.pid === processInfo.pid)) {
-                continue;
-            }
-
-            const match = this.selectBestSession(
-                processInfo,
-                sortedSessions,
-                usedSessionIds,
-                processStartByPid,
-                'missing-cwd',
-            );
+        return agents;
+    }
 
-            if (!match) {
-                continue;
-            }
+    private listCodexProcesses(): ProcessInfo[] {
+        return listProcesses({ namePattern: 'codex' }).filter((processInfo) =>
+            this.canHandle(processInfo),
+        );
+    }
 
-            usedSessionIds.add(match.sessionId);
-            agents.push(this.mapSessionToAgent(match, processInfo, agents));
-        }
+    private calculateSessionScanLimit(processCount: number): number {
+        return Math.min(
+            Math.max(
+                processCount * CodexAdapter.SESSION_SCAN_MULTIPLIER,
+                CodexAdapter.MIN_SESSION_SCAN,
+            ),
+            CodexAdapter.MAX_SESSION_SCAN,
+        );
+    }
 
-        // Third pass: every running codex process should still be listed.
+    private assignSessionsForMode(
+        mode: SessionMatchMode,
+        codexProcesses: ProcessInfo[],
+        sessions: CodexSession[],
+        usedSessionIds: Set<string>,
+        assignedPids: Set<number>,
+        processStartByPid: Map<number, Date>,
+        agents: AgentInfo[],
+    ): void {
         for (const processInfo of codexProcesses) {
-            if (agents.some((agent) => agent.pid === processInfo.pid)) {
+            if (assignedPids.has(processInfo.pid)) {
                 continue;
             }
 
-            const fallbackSession = this.selectBestSession(
+            const session = this.selectBestSession(
                 processInfo,
-                sortedSessions,
+                sessions,
                 usedSessionIds,
                 processStartByPid,
-                'any',
+                mode,
             );
-
-            if (fallbackSession) {
-                usedSessionIds.add(fallbackSession.sessionId);
-                agents.push(this.mapSessionToAgent(fallbackSession, processInfo, agents));
+            if (!session) {
                 continue;
             }
 
-            agents.push(this.mapProcessOnlyAgent(processInfo, agents));
+            this.addMappedSessionAgent(session, processInfo, usedSessionIds, assignedPids, agents);
         }
+    }
 
-        return agents;
+    private addMappedSessionAgent(
+        session: CodexSession,
+        processInfo: ProcessInfo,
+        usedSessionIds: Set<string>,
+        assignedPids: Set<number>,
+        agents: AgentInfo[],
+    ): void {
+        usedSessionIds.add(session.sessionId);
+        assignedPids.add(processInfo.pid);
+        agents.push(this.mapSessionToAgent(session, processInfo, agents));
+    }
+
+    private addProcessOnlyAgent(
+        processInfo: ProcessInfo,
+        assignedPids: Set<number>,
+        agents: AgentInfo[],
+    ): void {
+        assignedPids.add(processInfo.pid);
+        agents.push(this.mapProcessOnlyAgent(processInfo, agents));
     }
 
     private mapSessionToAgent(
@@ -260,14 +296,13 @@ export class CodexAdapter implements AgentAdapter {
             }
         }
 
-        const selectedPaths = new Set(
-            files
+        const recentFiles = files
             .sort((a, b) => b.mtimeMs - a.mtimeMs)
             .slice(0, limit)
-            .map((file) => file.path),
-        );
-
+            .map((file) => file.path);
         const processDayFiles = this.findProcessDaySessionFiles(processStartByPid);
+
+        const selectedPaths = new Set(recentFiles);
         for (const processDayFile of processDayFiles) {
             selectedPaths.add(processDayFile);
         }
@@ -385,9 +420,29 @@ export class CodexAdapter implements AgentAdapter {
         sessions: CodexSession[],
         usedSessionIds: Set<string>,
         processStartByPid: Map<number, Date>,
-        mode: 'cwd' | 'missing-cwd' | 'any',
+        mode: SessionMatchMode,
     ): CodexSession | undefined {
-        const candidates = sessions.filter((session) => {
+        const candidates = this.filterCandidateSessions(processInfo, sessions, usedSessionIds, mode);
+
+        if (candidates.length === 0) {
+            return undefined;
+        }
+
+        const processStart = processStartByPid.get(processInfo.pid);
+        if (!processStart) {
+            return candidates.sort((a, b) => b.lastActive.getTime() - a.lastActive.getTime())[0];
+        }
+
+        return this.rankCandidatesByStartTime(candidates, processStart)[0];
+    }
+
+    private filterCandidateSessions(
+        processInfo: ProcessInfo,
+        sessions: CodexSession[],
+        usedSessionIds: Set<string>,
+        mode: SessionMatchMode,
+    ): CodexSession[] {
+        return sessions.filter((session) => {
             if (usedSessionIds.has(session.sessionId)) {
                 return false;
             }
@@ -402,17 +457,11 @@ export class CodexAdapter implements AgentAdapter {
 
             return true;
         });
+    }
 
-        if (candidates.length === 0) {
-            return undefined;
-        }
-
-        const processStart = processStartByPid.get(processInfo.pid);
-        if (!processStart) {
-            return candidates.sort((a, b) => b.lastActive.getTime() - a.lastActive.getTime())[0];
-        }
-
+    private rankCandidatesByStartTime(candidates: CodexSession[], processStart: Date): CodexSession[] {
         const toleranceMs = CodexAdapter.PROCESS_SESSION_TIME_TOLERANCE_MS;
+
         return candidates
             .map((session) => {
                 const diffMs = Math.abs(session.sessionStart.getTime() - processStart.getTime());
@@ -428,7 +477,8 @@ export class CodexAdapter implements AgentAdapter {
                 if (a.rank !== b.rank) return a.rank - b.rank;
                 if (a.diffMs !== b.diffMs) return a.diffMs - b.diffMs;
                 return b.recency - a.recency;
-            })[0]?.session;
+            })
+            .map((ranked) => ranked.session);
     }
 
     private getProcessStartTimes(pids: number[]): Map<number, Date> {
