refactor: unify instruction file reading with FileReader abstraction

- Created FileReader interface to eliminate duplication between fs and Runtime
- Moved all instruction reading logic to instructionFiles.ts
- Removed 117 lines from systemMessage.ts (-49%)
- Simplified helper function structure with shared implementation
- Removed redundant tests for private helpers (covered by public API)

Net change: -113 lines across 3 files (-28%)
All tests passing (16/16)

Author: ammar-agent

src/services/systemMessage.ts

@@ -1,10 +1,9 @@
 import * as os from "os";
 import * as path from "path";
 import type { WorkspaceMetadata } from "@/types/workspace";
-import { readInstructionSet, INSTRUCTION_FILE_NAMES } from "@/utils/main/instructionFiles";
+import { readInstructionSet, readInstructionSetFromRuntime } from "@/utils/main/instructionFiles";
 import { extractModeSection } from "@/utils/main/markdown";
 import type { Runtime } from "@/runtime/Runtime";
-import { readFileString } from "@/utils/runtime/helpers";
 
 // NOTE: keep this in sync with the docs/models.md file
 
@@ -30,6 +29,9 @@ Use GitHub-style \`<details>/<summary>\` tags to create collapsible sections for
 </prelude>
 `;
 
+/**
+ * Build environment context XML block describing the workspace.
+ */
 function buildEnvironmentContext(workspacePath: string): string {
   return `
 <environment>
@@ -44,121 +46,30 @@ You are in a git worktree at ${workspacePath}
 }
 
 /**
- * The system directory where global cmux configuration lives.
- * This is where users can place global AGENTS.md and .cmux/PLAN.md files
- * that apply to all workspaces.
+ * Get the system directory where global cmux configuration lives.
+ * Users can place global AGENTS.md and .cmux/PLAN.md files here.
  */
 function getSystemDirectory(): string {
   return path.join(os.homedir(), ".cmux");
 }
 
 /**
- * Read the first available file from a list using runtime.
+ * Builds a system message for the AI model by combining instruction sources.
  *
- * @param runtime - Runtime instance (may be local or SSH)
- * @param directory - Directory to search in
- * @param filenames - List of filenames to try, in priority order
- * @returns Content of the first file found, or null if none exist
- */
-async function readFirstAvailableFileFromRuntime(
-  runtime: Runtime,
-  directory: string,
-  filenames: readonly string[]
-): Promise<string | null> {
-  for (const filename of filenames) {
-    try {
-      const filePath = path.join(directory, filename);
-      return await readFileString(runtime, filePath);
-    } catch {
-      // File doesn't exist or can't be read, try next
-      continue;
-    }
-  }
-  return null;
-}
-
-/**
- * Read a file with optional local variant using runtime.
- * Follows the same pattern as readFileWithLocalVariant but uses Runtime.
+ * Instruction layers:
+ * 1. Global: ~/.cmux/AGENTS.md (always included)
+ * 2. Context: workspace/AGENTS.md OR project/AGENTS.md (workspace takes precedence)
+ * 3. Mode: Extracts "Mode: <mode>" section from context then global (if mode provided)
  *
- * @param runtime - Runtime instance (may be local or SSH)
- * @param directory - Directory to search
- * @param baseFilenames - Base filenames to try in priority order
- * @param localFilename - Optional local filename to append if present
- * @returns Combined content or null if no base file exists
- */
-async function readFileWithLocalVariantFromRuntime(
-  runtime: Runtime,
-  directory: string,
-  baseFilenames: readonly string[],
-  localFilename?: string
-): Promise<string | null> {
-  const baseContent = await readFirstAvailableFileFromRuntime(runtime, directory, baseFilenames);
-
-  if (!baseContent) {
-    return null;
-  }
-
-  if (!localFilename) {
-    return baseContent;
-  }
-
-  try {
-    const localFilePath = path.join(directory, localFilename);
-    const localContent = await readFileString(runtime, localFilePath);
-    return `${baseContent}\n\n${localContent}`;
-  } catch {
-    return baseContent;
-  }
-}
-
-/**
- * Read instruction set from a workspace using the runtime abstraction.
- * This supports both local workspaces and remote SSH workspaces.
+ * File search order: AGENTS.md → AGENT.md → CLAUDE.md
+ * Local variants: AGENTS.local.md appended if found (for .gitignored personal preferences)
  *
- * @param runtime - Runtime instance (may be local or SSH)
- * @param workspacePath - Path to workspace directory
- * @returns Combined instruction content, or null if no base file exists
- */
-async function readInstructionSetFromRuntime(
-  runtime: Runtime,
-  workspacePath: string
-): Promise<string | null> {
-  const LOCAL_INSTRUCTION_FILENAME = "AGENTS.local.md";
-  return readFileWithLocalVariantFromRuntime(
-    runtime,
-    workspacePath,
-    INSTRUCTION_FILE_NAMES,
-    LOCAL_INSTRUCTION_FILENAME
-  );
-}
-
-/**
- * Builds a system message for the AI model by combining multiple instruction sources.
- *
- * Instruction sources are layered as follows:
- * 1. Global instructions: ~/.cmux/AGENTS.md (+ AGENTS.local.md) - always included
- * 2. Context instructions: EITHER workspace OR project AGENTS.md (not both)
- *    - Workspace: <workspacePath>/AGENTS.md (+ AGENTS.local.md) - if exists (read via runtime)
- *    - Project: <projectPath>/AGENTS.md (+ AGENTS.local.md) - fallback if workspace doesn't exist
- * 3. Mode-specific context (if mode provided): Extract a section titled "Mode: <mode>"
- *    (case-insensitive) from the instruction file. Priority: context instructions, then global.
- *
- * Each instruction file location is searched for in priority order:
- * - AGENTS.md
- * - AGENT.md
- * - CLAUDE.md
- *
- * If a base instruction file is found, its corresponding .local.md variant is also
- * checked and appended when building the instruction set (useful for personal preferences not committed to git).
- *
- * @param metadata - Workspace metadata (contains projectPath for reading AGENTS.md)
- * @param runtime - Runtime instance for reading workspace files (may be remote)
- * @param workspacePath - Absolute path to the workspace directory (for environment context)
- * @param mode - Optional mode name (e.g., "plan", "exec") - looks for {MODE}.md files if provided
- * @param additionalSystemInstructions - Optional additional system instructions to append at the end
- * @returns System message string with all instruction sources combined
- * @throws Error if metadata is invalid
+ * @param metadata - Workspace metadata (contains projectPath)
+ * @param runtime - Runtime for reading workspace files (supports SSH)
+ * @param workspacePath - Workspace directory path
+ * @param mode - Optional mode name (e.g., "plan", "exec")
+ * @param additionalSystemInstructions - Optional instructions appended last
+ * @throws Error if metadata or workspacePath invalid
  */
 export async function buildSystemMessage(
   metadata: WorkspaceMetadata,
@@ -167,67 +78,40 @@ export async function buildSystemMessage(
   mode?: string,
   additionalSystemInstructions?: string
 ): Promise<string> {
-  // Validate inputs
-  if (!metadata) {
-    throw new Error("Invalid workspace metadata: metadata is required");
-  }
-  if (!workspacePath) {
-    throw new Error("Invalid workspace path: workspacePath is required");
-  }
+  if (!metadata) throw new Error("Invalid workspace metadata: metadata is required");
+  if (!workspacePath) throw new Error("Invalid workspace path: workspacePath is required");
 
-  const systemDir = getSystemDirectory();
-  const projectDir = metadata.projectPath;
+  // Read instruction sets
+  const globalInstructions = await readInstructionSet(getSystemDirectory());
+  const workspaceInstructions = await readInstructionSetFromRuntime(runtime, workspacePath);
+  const contextInstructions = workspaceInstructions ?? (await readInstructionSet(metadata.projectPath));
 
-  // Layer 1: Global instructions (always included)
-  const globalInstructions = await readInstructionSet(systemDir);
+  // Combine: global + context (workspace takes precedence over project)
+  const customInstructions = [globalInstructions, contextInstructions]
+    .filter(Boolean)
+    .join("\n\n");
 
-  // Layer 2: Workspace OR Project instructions (not both)
-  // Try workspace first (via runtime, may be remote for SSH)
-  // Fall back to project if workspace doesn't have AGENTS.md
-  const workspaceInstructions = await readInstructionSetFromRuntime(runtime, workspacePath);
-  const projectInstructions = workspaceInstructions ? null : await readInstructionSet(projectDir);
-
-  // Combine instruction sources
-  // Result: global + (workspace OR project)
-  const instructionSegments = [
-    globalInstructions,
-    workspaceInstructions ?? projectInstructions,
-  ].filter(Boolean);
-  const customInstructions = instructionSegments.join("\n\n");
-
-  // Look for a "Mode: <mode>" section inside instruction sets
-  // Priority: workspace (or project fallback), then global
-  // We only check the workspace OR project instructions, not both
-  // This behavior is documented in docs/instruction-files.md - keep both in sync when changing.
+  // Extract mode-specific section (context first, then global fallback)
   let modeContent: string | null = null;
   if (mode) {
-    const contextInstructions = workspaceInstructions ?? projectInstructions;
-    if (contextInstructions) {
-      modeContent = extractModeSection(contextInstructions, mode);
-    }
-    if (!modeContent && globalInstructions) {
-      modeContent = extractModeSection(globalInstructions, mode);
-    }
+    modeContent =
+      (contextInstructions && extractModeSection(contextInstructions, mode)) ||
+      (globalInstructions && extractModeSection(globalInstructions, mode)) ||
+      null;
   }
 
-  // Build the final system message
-  // Use workspacePath for environment context (where code actually executes)
-  const environmentContext = buildEnvironmentContext(workspacePath);
-  const trimmedPrelude = PRELUDE.trim();
-  let systemMessage = `${trimmedPrelude}\n\n${environmentContext}`;
+  // Build system message
+  let systemMessage = `${PRELUDE.trim()}\n\n${buildEnvironmentContext(workspacePath)}`;
 
-  // Add custom instructions if found
   if (customInstructions) {
     systemMessage += `\n<custom-instructions>\n${customInstructions}\n</custom-instructions>`;
   }
 
-  // Add mode-specific content if found
   if (modeContent) {
     const tag = (mode ?? "mode").toLowerCase().replace(/[^a-z0-9_-]/gi, "-");
     systemMessage += `\n\n<${tag}>\n${modeContent}\n</${tag}>`;
   }
 
-  // Add additional system instructions at the end (highest priority)
   if (additionalSystemInstructions) {
     systemMessage += `\n\n<additional-instructions>\n${additionalSystemInstructions}\n</additional-instructions>`;
   }

src/utils/main/instructionFiles.test.ts

@@ -2,10 +2,8 @@ import * as fs from "fs/promises";
 import * as path from "path";
 import * as os from "os";
 import {
-  readFirstAvailableFile,
   readInstructionSet,
   gatherInstructionSets,
-  INSTRUCTION_FILE_NAMES,
 } from "./instructionFiles";
 
 describe("instructionFiles", () => {
@@ -19,36 +17,6 @@ describe("instructionFiles", () => {
     await fs.rm(tempDir, { recursive: true, force: true });
   });
 
-  describe("readFirstAvailableFile", () => {
-    it("should return null when no files exist", async () => {
-      const result = await readFirstAvailableFile(tempDir, INSTRUCTION_FILE_NAMES);
-      expect(result).toBeNull();
-    });
-
-    it("should return content of first available file in priority order", async () => {
-      await fs.writeFile(path.join(tempDir, "AGENT.md"), "agent content");
-      await fs.writeFile(path.join(tempDir, "CLAUDE.md"), "claude content");
-
-      const result = await readFirstAvailableFile(tempDir, INSTRUCTION_FILE_NAMES);
-      expect(result).toBe("agent content");
-    });
-
-    it("should prefer AGENTS.md over AGENT.md", async () => {
-      await fs.writeFile(path.join(tempDir, "AGENTS.md"), "agents content");
-      await fs.writeFile(path.join(tempDir, "AGENT.md"), "agent content");
-
-      const result = await readFirstAvailableFile(tempDir, INSTRUCTION_FILE_NAMES);
-      expect(result).toBe("agents content");
-    });
-
-    it("should fall back to lower priority files", async () => {
-      await fs.writeFile(path.join(tempDir, "CLAUDE.md"), "claude content");
-
-      const result = await readFirstAvailableFile(tempDir, INSTRUCTION_FILE_NAMES);
-      expect(result).toBe("claude content");
-    });
-  });
-
   describe("readInstructionSet", () => {
     it("should return null when no instruction files exist", async () => {
       const result = await readInstructionSet(tempDir);