Merge pull request #2772 from nanocoai/fix/codex-per-thread-archive

fix(codex): per-thread conversation archive (CDX-004)

container/Dockerfile

@@ -20,7 +20,6 @@ ARG INSTALL_CJK_FONTS=false
 # mean every rebuild silently picks up the latest and can break in lockstep
 # across all users.
 ARG CLAUDE_CODE_VERSION=2.1.116
-ARG CODEX_VERSION=0.138.0
 ARG AGENT_BROWSER_VERSION=latest
 ARG VERCEL_VERSION=latest
 ARG BUN_VERSION=1.3.12
@@ -102,9 +101,6 @@ RUN --mount=type=cache,target=/root/.cache/pnpm \
 RUN --mount=type=cache,target=/root/.cache/pnpm \
     pnpm install -g "agent-browser@${AGENT_BROWSER_VERSION}"
 
-RUN --mount=type=cache,target=/root/.cache/pnpm \
-    pnpm install -g "@openai/codex@${CODEX_VERSION}"
-
 RUN --mount=type=cache,target=/root/.cache/pnpm \
     pnpm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"
 

container/agent-runner/src/providers/codex-cli-tools.test.ts

@@ -0,0 +1,39 @@
+// Structural guard for the Codex CLI install in container/cli-tools.json.
+//
+// @openai/codex is a CLI *binary* installed from the global-CLI manifest (a
+// json-merge seam), not an importable package, so the barrel-driven
+// registration tests cannot see it. This test reads the real cli-tools.json
+// and asserts the @openai/codex entry is present and pinned to an exact
+// version. It goes red if the manifest entry is dropped or unpins.
+//
+// Runs under bun (same suite as the container registration test):
+//   cd container/agent-runner && bun test src/providers/codex-cli-tools.test.ts
+
+import { existsSync, readFileSync } from 'fs';
+import path from 'path';
+
+import { describe, it, expect } from 'bun:test';
+
+// container/agent-runner/src/providers/ -> container/cli-tools.json
+const MANIFEST = path.join(import.meta.dir, '..', '..', '..', 'cli-tools.json');
+const manifestPresent = existsSync(MANIFEST);
+
+// Read lazily — `describe.skipIf` still runs the body to register tests, so the
+// read has to be guarded for the bare-branch (no manifest) case.
+const tools: Array<{ name: string; version: string }> = manifestPresent
+  ? JSON.parse(readFileSync(MANIFEST, 'utf8'))
+  : [];
+const codex = tools.find((t) => t.name === '@openai/codex');
+
+// cli-tools.json is a trunk file; on the bare providers branch it isn't present,
+// so skip there. In an installed tree (trunk + this payload) it must carry the
+// pinned @openai/codex entry.
+describe.skipIf(!manifestPresent)('container/cli-tools.json codex CLI install', () => {
+  it('includes the @openai/codex entry', () => {
+    expect(codex).toBeDefined();
+  });
+
+  it('pins it to an exact semver (no latest, no ranges)', () => {
+    expect(codex?.version).toMatch(/^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/);
+  });
+});

container/agent-runner/src/providers/codex-dockerfile.test.ts

@@ -1,30 +0,0 @@
-// Structural guard for the Codex CLI install in container/Dockerfile.
-//
-// @openai/codex is a CLI *binary* installed via the Dockerfile, not an
-// importable package, so the barrel-driven registration tests cannot see it.
-// This test reads the real Dockerfile and asserts the version ARG and the
-// `pnpm install -g` line for @openai/codex are both present. It goes red if
-// either Dockerfile edit is dropped or drifts.
-//
-// Runs under bun (same suite as the container registration test):
-//   cd container/agent-runner && bun test src/providers/codex-dockerfile.test.ts
-
-import { readFileSync } from 'fs';
-import path from 'path';
-
-import { describe, it, expect } from 'bun:test';
-
-// container/agent-runner/src/providers/ -> container/Dockerfile
-const DOCKERFILE = path.join(import.meta.dir, '..', '..', '..', 'Dockerfile');
-
-describe('container/Dockerfile codex CLI install', () => {
-  const dockerfile = readFileSync(DOCKERFILE, 'utf8');
-
-  it('declares the CODEX_VERSION ARG', () => {
-    expect(dockerfile).toMatch(/ARG\s+CODEX_VERSION=/);
-  });
-
-  it('installs the @openai/codex CLI pinned to that ARG', () => {
-    expect(dockerfile).toMatch(/pnpm install -g\s+"@openai\/codex@\$\{CODEX_VERSION\}"/);
-  });
-});

container/agent-runner/src/providers/exchange-archive.test.ts

@@ -20,7 +20,7 @@ function makeTmpDir(): string {
 }
 
 describe('provider exchange archive', () => {
-  it('writes unique exchange-level archives with provider metadata', () => {
+  it('appends same-thread exchanges into one file with a single header', () => {
     const conversationsDir = makeTmpDir();
     const timestamp = new Date('2026-06-03T12:34:56.789Z');
 
@@ -43,17 +43,80 @@ describe('provider exchange archive', () => {
       timestamp,
     });
 
-    expect(first).not.toBeNull();
-    expect(second).not.toBeNull();
-    expect(first).not.toBe(second);
+    // Same thread → same date-prefixed, thread-stable file, not one per exchange.
+    expect(first).toBe('2026-06-03-codex-thread-123.md');
+    expect(second).toBe(first);
+    expect(fs.readdirSync(conversationsDir)).toHaveLength(1);
 
     const content = fs.readFileSync(path.join(conversationsDir, first!), 'utf-8');
-    expect(content).toContain('# Codex Exchange');
+    // Header (thread-level metadata) written exactly once.
+    expect(content.match(/# Codex Conversation/g)).toHaveLength(1);
     expect(content).toContain('Provider: codex');
     expect(content).toContain('Continuation/thread id: thread-123');
-    expect(content).toContain('Status: completed');
+    // Both exchanges present, each with its own status line.
     expect(content).toContain('**User**: hello');
     expect(content).toContain('**Assistant**: world');
+    expect(content).toContain('**User**: hello again');
+    expect(content).toContain('**Assistant**: world again');
+    expect(content.match(/Status: completed/g)).toHaveLength(2);
+  });
+
+  it('writes a separate file per thread', () => {
+    const conversationsDir = makeTmpDir();
+    const timestamp = new Date('2026-06-03T12:34:56.789Z');
+
+    const a = archiveProviderExchange({
+      conversationsDir,
+      provider: 'codex',
+      prompt: 'p',
+      result: 'r',
+      continuation: 'thread-a',
+      status: 'completed',
+      timestamp,
+    });
+    const b = archiveProviderExchange({
+      conversationsDir,
+      provider: 'codex',
+      prompt: 'p',
+      result: 'r',
+      continuation: 'thread-b',
+      status: 'completed',
+      timestamp,
+    });
+
+    expect(a).toBe('2026-06-03-codex-thread-a.md');
+    expect(b).toBe('2026-06-03-codex-thread-b.md');
+    expect(fs.readdirSync(conversationsDir)).toHaveLength(2);
+  });
+
+  it('keeps the creation-date prefix stable when later exchanges land on another day', () => {
+    const conversationsDir = makeTmpDir();
+
+    const first = archiveProviderExchange({
+      conversationsDir,
+      provider: 'codex',
+      prompt: 'a',
+      result: 'b',
+      continuation: 'thread-x',
+      status: 'completed',
+      timestamp: new Date('2026-06-03T10:00:00.000Z'),
+    });
+    // A later exchange on a different day must append to the same file, not
+    // mint a new 2026-06-05-* one (the bug a naive date-from-timestamp scheme
+    // would introduce).
+    const second = archiveProviderExchange({
+      conversationsDir,
+      provider: 'codex',
+      prompt: 'c',
+      result: 'd',
+      continuation: 'thread-x',
+      status: 'completed',
+      timestamp: new Date('2026-06-05T10:00:00.000Z'),
+    });
+
+    expect(first).toBe('2026-06-03-codex-thread-x.md');
+    expect(second).toBe(first);
+    expect(fs.readdirSync(conversationsDir)).toHaveLength(1);
   });
 
   it('skips empty result text', () => {

container/agent-runner/src/providers/exchange-archive.ts

@@ -2,10 +2,17 @@ import fs from 'fs';
 import path from 'path';
 
 /**
- * Per-exchange markdown archive for providers with no on-disk transcript —
+ * Per-thread conversation archive for providers with no on-disk transcript —
  * payload code, shipped with the provider that needs it. The provider's
  * `onExchangeComplete` hook (see types.ts) calls this with each completed
  * exchange; the runner never archives on a provider's behalf.
+ *
+ * One file per thread (keyed on the continuation id), named
+ * `<date>-<provider>-<thread>.md` and appended to as exchanges complete —
+ * mirroring the Claude path's one-file-per-session granularity and its
+ * date-prefixed, name-sortable filenames, since the Codex app-server keeps
+ * history server-side with no transcript to roll up at a compaction boundary.
+ * The date is the thread's creation day and stays stable across later appends.
  */
 
 const DEFAULT_CONVERSATIONS_DIR = '/workspace/agent/conversations';
@@ -21,8 +28,10 @@ export interface ProviderExchangeArchiveOptions {
 }
 
 /**
- * Archive a single prompt/result exchange. Returns the written filename, or
- * null when there is nothing to archive (empty result).
+ * Append a single prompt/result exchange to its thread's conversation file,
+ * writing the thread-level header once when the file is first created. Returns
+ * the (thread-stable) filename, or null when there is nothing to archive
+ * (empty result).
  */
 export function archiveProviderExchange(options: ProviderExchangeArchiveOptions): string | null {
   const result = options.result?.trim();
@@ -33,43 +42,51 @@ export function archiveProviderExchange(options: ProviderExchangeArchiveOptions)
     options.conversationsDir || process.env.NANOCLAW_CONVERSATIONS_DIR || DEFAULT_CONVERSATIONS_DIR;
   fs.mkdirSync(conversationsDir, { recursive: true });
 
-  const filename = uniqueArchiveFilename(conversationsDir, options.provider, options.continuation, timestamp);
-  const lines = [
-    `# ${titleCase(options.provider)} Exchange`,
-    '',
-    `Archived: ${timestamp.toISOString()}`,
-    `Provider: ${options.provider}`,
-    `Continuation/thread id: ${options.continuation || '(none)'}`,
-    `Status: ${options.status}`,
+  const filename = threadArchiveFilename(conversationsDir, options.provider, options.continuation, timestamp);
+  const filePath = path.join(conversationsDir, filename);
+
+  // Thread-level metadata (provider, thread id) belongs in the header, written
+  // once. Per-exchange metadata (timestamp, status) rides in each appended
+  // block. Each block leads with a blank line + `---` so the separator renders
+  // as a thematic break, not a setext heading underline on the prior line.
+  const parts: string[] = [];
+  if (!fs.existsSync(filePath)) {
+    parts.push(
+      `# ${titleCase(options.provider)} Conversation`,
+      '',
+      `Provider: ${options.provider}`,
+      `Continuation/thread id: ${options.continuation || '(none)'}`,
+    );
+  }
+  parts.push(
     '',
     '---',
     '',
+    `Archived: ${timestamp.toISOString()} · Status: ${options.status}`,
+    '',
     `**User**: ${truncate(options.prompt)}`,
     '',
     `**Assistant**: ${truncate(result)}`,
     '',
-  ];
-  fs.writeFileSync(path.join(conversationsDir, filename), lines.join('\n'));
+  );
+  fs.appendFileSync(filePath, parts.join('\n'));
   return filename;
 }
 
-function uniqueArchiveFilename(
+function threadArchiveFilename(
   dir: string,
   provider: string,
   continuation: string | undefined,
   timestamp: Date,
 ): string {
-  const date = timestamp.toISOString().split('T')[0];
-  const time = timestamp.toISOString().replace(/[-:.TZ]/g, '').slice(8, 17);
-  const thread = sanitizeSlug(continuation || 'no-thread').slice(0, 24) || 'no-thread';
-  const base = `${date}-${sanitizeSlug(provider)}-${time}-${thread}`;
-  let filename = `${base}.md`;
-  let counter = 2;
-  while (fs.existsSync(path.join(dir, filename))) {
-    filename = `${base}-${counter}.md`;
-    counter += 1;
-  }
-  return filename;
+  const thread = sanitizeSlug(continuation || 'no-thread').slice(0, 48) || 'no-thread';
+  const suffix = `${sanitizeSlug(provider)}-${thread}.md`;
+  // Reuse this thread's existing file whatever day it was created; only stamp a
+  // new date when none exists. Match on the suffix after the date prefix.
+  const dated = /^\d{4}-\d{2}-\d{2}-/;
+  const existing = fs.readdirSync(dir).find((f) => dated.test(f) && f.replace(dated, '') === suffix);
+  if (existing) return existing;
+  return `${timestamp.toISOString().split('T')[0]}-${suffix}`;
 }
 
 function sanitizeSlug(value: string): string {

setup/providers/codex.ts

@@ -400,10 +400,18 @@ export function verifyCodexInstall(): { ok: boolean; problems: string[] } {
     }
   }
 
-  const dockerfilePath = path.join(root, 'container', 'Dockerfile');
-  const dockerfile = fs.existsSync(dockerfilePath) ? fs.readFileSync(dockerfilePath, 'utf-8') : '';
-  if (!/ARG CODEX_VERSION=/.test(dockerfile) || !dockerfile.includes('@openai/codex@${CODEX_VERSION}')) {
-    problems.push('container/Dockerfile missing the pinned @openai/codex install');
+  const manifestPath = path.join(root, 'container', 'cli-tools.json');
+  let hasCodexCli = false;
+  if (fs.existsSync(manifestPath)) {
+    try {
+      const tools = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')) as Array<{ name?: string }>;
+      hasCodexCli = Array.isArray(tools) && tools.some((t) => t.name === '@openai/codex');
+    } catch {
+      hasCodexCli = false;
+    }
+  }
+  if (!hasCodexCli) {
+    problems.push('container/cli-tools.json missing the @openai/codex CLI entry');
   }
 
   return { ok: problems.length === 0, problems };