fix(daemon): move connected-MCP directive out of the cached system prompt (#5336)

* fix(daemon): move connected-MCP directive out of the cached system prompt

The '## External MCP servers — already authenticated' directive was
composed into daemonSystemPrompt, part of the cacheable system prefix
sent upstream. Its membership is driven by live OAuth Bearer validity
(isTokenExpired), so a token expiring/refreshing/reconnecting mid-
conversation changes the directive -> changes the system prompt bytes ->
invalidates the whole downstream prompt-cache prefix, including the
conversation-history cache AMR/Claude-on-Bedrock relies on. Production
telemetry (stable_prompt_cache_miss_reason='stable-prompt-changed')
attributes the bulk of resume-turn prefix drift to this directive, rising
with conversation length as more tokens cross expiry.

Move it from composeSystemPrompt into the per-turn client instruction
slice (alongside runContext/cwd hints): re-sent uncached every turn,
landing after the cached system+history prefix. The model still sees the
current MCP auth state each turn (more accurately -- live state, not the
seed snapshot), while the cacheable prefix stays byte-stable across
resumes. renderConnectedExternalMcpDirective is exported and its leading
separator dropped so it composes cleanly in the joined slice;
composeSystemPrompt no longer takes connectedExternalMcp.

* test(daemon): e2e red-spec for the connected-MCP directive moving to the per-turn slice

Drives a real two-turn codex session (native resume) with a connected
external MCP server (enabled config + live OAuth Bearer) through the
daemon HTTP boundary, capturing the exact prompt each turn reaches the
agent's stdin. Asserts the '## External MCP servers — already
authenticated' directive is present on turn 1 AND re-sent on the turn-2
clean resume — which only holds once the directive rides in the per-turn
instruction slice instead of the cached stable block. Verified red on
origin/main (turn-2 stdin lacks the directive because the cached block is
not re-sent on a clean resume) and green with the fix.

* test(daemon): keep the MCP-directive red-spec hermetic

The new test writes an authenticated github MCP server into the
process-wide OD_DATA_DIR (tests/setup.ts shares one data root for the
whole daemon Vitest run) and the shared afterEach never reset it, so the
connected-MCP state could leak into later test files and make them
suite-order dependent. Reset the MCP config (empty servers) and clear the
github token in afterEach — both idempotent no-ops for the other tests in
this file.

* test(daemon): let MCP teardown failures fail fast

Drop the unconditional .catch(() => {}) around the afterEach MCP reset.
writeMcpConfig always overwrites to an empty server list and clearToken
is a documented no-op when the entry is absent, so neither needs the
catch — and swallowing a real teardown failure would silently let MCP
state leak into later tests, defeating the isolation guarantee.

Author: lefarcen

apps/daemon/src/prompts/system.ts

@@ -520,16 +520,6 @@ export interface ComposeInput {
   // Skill identifier. Required when critique is enabled;
   // ignored when critique is disabled or omitted.
   critiqueSkill?: { id: string } | undefined;
-  // External MCP servers the daemon already holds a valid OAuth Bearer
-  // token for at spawn time. We surface the list to the model so it does
-  // NOT chase Claude Code's synthetic `*_authenticate` /
-  // `*_complete_authentication` tools that get injected when the HTTP
-  // transport's first connect transiently flips a server into
-  // needs-auth state — the Bearer is in `.mcp.json`, the real tools are
-  // available, and burning a turn on a redundant OAuth dance just
-  // confuses the user.
-  connectedExternalMcp?: ReadonlyArray<{ id: string; label?: string | undefined }>
-    | undefined;
   // Optional `## Active plugin` / `## Plugin inputs` block. The daemon's
   // plugin module renders this from an AppliedPluginSnapshot; we splice
   // it in after the active skill so the plugin description sits next to
@@ -595,7 +585,6 @@ export function composeSystemPrompt({
   critique,
   critiqueBrand,
   critiqueSkill,
-  connectedExternalMcp,
   pluginBlock,
   activeStageBlocks,
   streamFormat,
@@ -940,9 +929,6 @@ export function composeSystemPrompt({
     parts.push(ACTIVE_DESIGN_SYSTEM_VISUAL_DIRECTION_OVERRIDE);
   }
 
-  const mcpDirective = renderConnectedExternalMcpDirective(connectedExternalMcp);
-  if (mcpDirective) parts.push(mcpDirective);
-
   if (resolvedExecutionProfile === 'filesystem') {
     parts.push(FILESYSTEM_HANDOFF_OVERRIDE);
   }
@@ -1072,7 +1058,7 @@ If this is a plain API run where filesystem tools are unavailable, output the sa
 // `*_authenticate` / `*_complete_authentication` tool for them. If
 // the real tools really are missing, surface that as a separate
 // failure instead of pivoting to the synthetic flow.
-function renderConnectedExternalMcpDirective(
+export function renderConnectedExternalMcpDirective(
   connectedExternalMcp:
     | ReadonlyArray<{ id: string; label?: string | undefined }>
     | undefined,
@@ -1087,8 +1073,8 @@ function renderConnectedExternalMcpDirective(
     })
     .filter((line): line is string => typeof line === 'string');
   if (lines.length === 0) return '';
+  // No leading separator: callers place this in a `---`-joined slice.
   return [
-    '\n\n---\n\n',
     '## External MCP servers — already authenticated\n\n',
     'The following external MCP servers are already authenticated for this run via an OAuth Bearer token the daemon injected into `.mcp.json`. You can call their real tools directly:\n\n',
     lines.join('\n'),

apps/daemon/src/server.ts

@@ -20,6 +20,7 @@ import net from 'node:net';
 import { executionProfileFromStreamFormat, PLUGIN_SHARE_ACTION_PLUGIN_IDS } from '@open-design/contracts';
 import {
   composeSystemPrompt,
+  renderConnectedExternalMcpDirective,
   resolveExclusiveSurface,
 } from './prompts/system.js';
 import { emittedRenderableQuestionForm } from './question-form-detect.js';
@@ -3373,7 +3374,6 @@ export async function startServer({
     streamFormat,
     locale,
     sessionMode,
-    connectedExternalMcp,
     appliedPluginSnapshotId,
     mediaExecution,
     byokMediaDefaults,
@@ -3889,9 +3889,6 @@ export async function startServer({
       byokMediaDefaults,
       streamFormat,
       executionProfile: executionProfileFromStreamFormat(streamFormat),
-      connectedExternalMcp: Array.isArray(connectedExternalMcp)
-        ? connectedExternalMcp
-        : undefined,
       ...(pluginBlock ? { pluginBlock } : {}),
       ...(activeStageBlocks ? { activeStageBlocks } : {}),
       userInstructions,
@@ -4385,7 +4382,6 @@ export async function startServer({
         streamFormat: def?.streamFormat ?? 'plain',
         locale,
         sessionMode: runSessionMode,
-        connectedExternalMcp,
         mediaExecution: run?.mediaExecution,
         byokMediaDefaults,
         // Plan §3.M2 / §3.V1 — forward the run's snapshot id so the
@@ -4698,9 +4694,16 @@ export async function startServer({
           'Do not mention this title task to the user. Continue with the normal answer after the title marker.',
         ].join('\n')
       : '';
+    // The connected-external-MCP directive reflects live OAuth token state,
+    // which flips mid-conversation as Bearers expire/refresh. Keeping it out of
+    // the cached stable prefix (daemonSystemPrompt) and re-sending it here in
+    // the per-turn slice keeps the upstream prompt-cache prefix byte-stable
+    // across resumes (protecting the conversation-history cache) while still
+    // giving the model the current MCP auth state on every turn.
+    const mcpConnectedDirective = renderConnectedExternalMcpDirective(connectedExternalMcp);
     const clientInstructionParts = includeStableInstructions
-      ? [researchCommandContract, runContextPrompt, browserUsePromptGuard, titleGenerationPrompt, systemPrompt]
-      : [researchCommandContract, runContextPrompt, browserUsePromptGuard, titleGenerationPrompt];
+      ? [researchCommandContract, runContextPrompt, mcpConnectedDirective, browserUsePromptGuard, titleGenerationPrompt, systemPrompt]
+      : [researchCommandContract, runContextPrompt, mcpConnectedDirective, browserUsePromptGuard, titleGenerationPrompt];
     const clientInstructionPrompt = clientInstructionParts
       .map((part) => (typeof part === 'string' ? part.trim() : ''))
       .filter(Boolean)

apps/daemon/tests/codex-session-resume.test.ts

@@ -6,6 +6,8 @@ import path from 'node:path';
 import { afterEach, describe, expect, it } from 'vitest';
 
 import { startServer } from '../src/server.js';
+import { writeMcpConfig } from '../src/mcp-config.js';
+import { clearToken, setToken } from '../src/mcp-tokens.js';
 
 // End-to-end coverage for codex native (capture-style) session resume.
 //
@@ -59,6 +61,19 @@ describe('codex native session resume', () => {
     if (binDir) await removeTempDir(binDir);
     binDir = null;
     restoreEnv(originalEnv);
+    // The MCP-directive red-spec below writes an authenticated `github` server
+    // into the process-wide OD_DATA_DIR (tests/setup.ts shares one root for the
+    // whole daemon Vitest run). Reset it after every test so that state cannot
+    // leak into later test files and make them suite-order dependent.
+    // `writeMcpConfig` unconditionally overwrites to an empty server list and
+    // `clearToken` is a documented no-op when the entry is absent, so neither
+    // needs to tolerate "nothing was written" — let a real teardown failure
+    // surface instead of silently degrading the isolation guarantee.
+    const dataDir = process.env.OD_DATA_DIR;
+    if (dataDir) {
+      await writeMcpConfig(dataDir, { servers: [] });
+      await clearToken(dataDir, 'github');
+    }
   });
 
   it('captures the thread id on turn 1 and resumes it (without resending history) on turn 2', async () => {
@@ -224,6 +239,80 @@ describe('codex native session resume', () => {
     expect(afterIntervening.argv).not.toContain('resume');
     expect(afterIntervening.argv[0]).toBe('exec');
   });
+
+  // Guards the fix that moved the connected-external-MCP directive out of the
+  // cached `daemonSystemPrompt` and into the per-turn instruction slice. The
+  // directive reflects live OAuth Bearer validity, so keeping it in the cached
+  // prefix churned the whole prompt-cache prefix (history included) whenever a
+  // token expired mid-conversation. Now it must ride in the per-turn slice, i.e.
+  // be re-sent on EVERY turn — including a clean resume, which never re-sends the
+  // cached stable block. On origin/main the directive lived in the stable block,
+  // so a clean resume dropped it and the turn-2 assertion below goes red.
+  it('re-sends the connected-MCP directive in the per-turn slice on resume turns', async () => {
+    binDir = await mkdtemp(path.join(os.tmpdir(), 'od-codex-mcp-bin-'));
+    const { bin, logPath } = await writeCapturingCodex(binDir, 'codex-mcp');
+
+    clearTelemetryEnv();
+    started = (await startServer({ port: 0, returnServer: true })) as StartedServer;
+    await putConfig(started.url, {
+      agentId: 'codex',
+      agentCliEnv: { codex: { CODEX_BIN: bin } },
+      telemetry: { metrics: true, content: false, artifactManifest: false },
+      privacyDecisionAt: Date.now(),
+    });
+
+    // A connected external MCP server = enabled config + a live (non-expired)
+    // OAuth Bearer. That is exactly what makes the daemon render the
+    // "already authenticated" directive.
+    const dataDir = process.env.OD_DATA_DIR;
+    if (!dataDir) throw new Error('OD_DATA_DIR is required for the MCP directive test');
+    await writeMcpConfig(dataDir, {
+      servers: [
+        {
+          id: 'github',
+          label: 'GitHub',
+          transport: 'http',
+          url: 'https://mcp.test.invalid/github',
+          enabled: true,
+        },
+      ],
+    });
+    await setToken(dataDir, 'github', {
+      accessToken: 'live-access-token',
+      tokenType: 'Bearer',
+      expiresAt: Date.now() + 3_600_000,
+      savedAt: Date.now(),
+    });
+
+    const conversationId = await createConversation(started.url);
+    const turn1 = await sendRunAndWait(started.url, conversationId, 'first user request');
+    expect(turn1.status).toBe('succeeded');
+    const turn2 = await sendRunAndWait(
+      started.url,
+      conversationId,
+      'second user request please',
+    );
+    expect(turn2.status).toBe('succeeded');
+
+    const runs = await readChatTurnExecs(logPath, conversationId);
+    expect(runs).toHaveLength(2);
+    const [create, resume] = runs as [ExecInvocation, ExecInvocation];
+
+    const MCP_MARKER = 'External MCP servers — already authenticated';
+    // A marker that only appears inside the cached stable block (daemonSystemPrompt).
+    const STABLE_BLOCK_MARKER = '# Identity and workflow charter (background)';
+
+    // Turn 1 (fresh seed) carries the directive.
+    expect(create.stdin).toContain(MCP_MARKER);
+    expect(create.stdin).toContain('`github`');
+
+    // Turn 2 is a clean resume: the cached stable block is NOT re-sent...
+    expect(resume.argv.slice(0, 2)).toEqual(['exec', 'resume']);
+    expect(resume.stdin).not.toContain(STABLE_BLOCK_MARKER);
+    // ...but the MCP directive IS, because it now lives in the per-turn slice.
+    expect(resume.stdin).toContain(MCP_MARKER);
+    expect(resume.stdin).toContain('`github`');
+  });
 });
 
 // Minimal fake Claude CLI (claude-stream-json): emits an init frame + one

apps/daemon/tests/prompts/system.test.ts

@@ -4,7 +4,11 @@ import { fileURLToPath } from 'node:url';
 
 import { describe, expect, it } from 'vitest';
 
-import { composeSystemPrompt, resolveExclusiveSurface } from '../../src/prompts/system.js';
+import {
+  composeSystemPrompt,
+  renderConnectedExternalMcpDirective,
+  resolveExclusiveSurface,
+} from '../../src/prompts/system.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
@@ -415,66 +419,69 @@ describe('composeSystemPrompt', () => {
     });
   });
 
-  describe('connectedExternalMcp directive', () => {
-    it('omits the directive when no servers are passed', () => {
+  // The connected-external-MCP directive reflects live OAuth token state, which
+  // flips mid-conversation as Bearers expire/refresh. It now rides in the
+  // per-turn instruction slice (server.ts), NOT the cached system prompt, so it
+  // no longer churns the cacheable prefix across resumes. composeSystemPrompt
+  // must therefore never emit it; the exported renderer is tested directly.
+  describe('connectedExternalMcp directive is no longer in the system prompt', () => {
+    it('never emits the MCP directive from composeSystemPrompt', () => {
       const prompt = composeSystemPrompt({});
       expect(prompt).not.toContain('External MCP servers — already authenticated');
       expect(prompt).not.toContain('mcp__<server>__authenticate');
     });
 
-    it('omits the directive when an empty array is passed', () => {
-      const prompt = composeSystemPrompt({ connectedExternalMcp: [] });
+    it('keeps the media-execution-disabled block, still with no MCP directive', () => {
+      const prompt = composeSystemPrompt({
+        metadata: { kind: 'image' },
+        mediaExecution: { mode: 'disabled' },
+      });
+      expect(prompt).toContain('Open Design-owned media execution is **disabled for this run**');
+      expect(prompt).not.toContain('## Media generation contract');
       expect(prompt).not.toContain('External MCP servers — already authenticated');
     });
+  });
 
-    it('lists each connected server and forbids the synthetic auth tools', () => {
-      const prompt = composeSystemPrompt({
-        connectedExternalMcp: [
-          { id: 'higgsfield-openclaw', label: 'Higgsfield (OpenClaw)' },
-          { id: 'github' },
-        ],
-      });
+  describe('renderConnectedExternalMcpDirective', () => {
+    it('returns an empty string for no / empty servers', () => {
+      expect(renderConnectedExternalMcpDirective(undefined)).toBe('');
+      expect(renderConnectedExternalMcpDirective([])).toBe('');
+    });
 
-      expect(prompt).toContain('## External MCP servers — already authenticated');
-      expect(prompt).toContain('`higgsfield-openclaw`');
-      expect(prompt).toContain('Higgsfield (OpenClaw)');
-      expect(prompt).toContain('`github`');
-      expect(prompt).toContain(
+    it('lists each connected server and forbids the synthetic auth tools', () => {
+      const directive = renderConnectedExternalMcpDirective([
+        { id: 'higgsfield-openclaw', label: 'Higgsfield (OpenClaw)' },
+        { id: 'github' },
+      ]);
+      expect(directive).toContain('## External MCP servers — already authenticated');
+      expect(directive).toContain('`higgsfield-openclaw`');
+      expect(directive).toContain('Higgsfield (OpenClaw)');
+      expect(directive).toContain('`github`');
+      expect(directive).toContain(
         '**Do NOT call any tool whose name matches `mcp__<server>__authenticate` or `mcp__<server>__complete_authentication`',
       );
-      expect(prompt).toContain('localhost:<random>/callback');
-      expect(prompt).toContain('Settings → External MCP');
+      expect(directive).toContain('localhost:<random>/callback');
+      expect(directive).toContain('Settings → External MCP');
     });
 
-    it('skips entries with blank ids and emits no directive when nothing usable remains', () => {
-      const prompt = composeSystemPrompt({
-        connectedExternalMcp: [
+    it('skips entries with blank ids and emits nothing when none remain', () => {
+      expect(
+        renderConnectedExternalMcpDirective([
           { id: '   ', label: 'blank' },
           { id: '', label: 'empty' },
-        ] as any,
-      });
-      expect(prompt).not.toContain('External MCP servers — already authenticated');
+        ] as any),
+      ).toBe('');
     });
 
     it('does not duplicate the label when it equals the id', () => {
-      const prompt = composeSystemPrompt({
-        connectedExternalMcp: [{ id: 'github', label: 'github' }],
-      });
-      expect(prompt).toContain('- `github`\n');
-      expect(prompt).not.toContain('- `github` (github)');
+      const directive = renderConnectedExternalMcpDirective([{ id: 'github', label: 'github' }]);
+      expect(directive).toContain('- `github`\n');
+      expect(directive).not.toContain('- `github` (github)');
     });
 
-    it('keeps external MCP tools visible when OD-owned media execution is disabled', () => {
-      const prompt = composeSystemPrompt({
-        connectedExternalMcp: [{ id: 'external-media', label: 'External media' }],
-        metadata: { kind: 'image' },
-        mediaExecution: { mode: 'disabled' },
-      });
-
-      expect(prompt).toContain('## External MCP servers — already authenticated');
-      expect(prompt).toContain('`external-media`');
-      expect(prompt).toContain('Open Design-owned media execution is **disabled for this run**');
-      expect(prompt).not.toContain('## Media generation contract');
+    it('has no leading separator so it composes cleanly in a `---`-joined slice', () => {
+      const directive = renderConnectedExternalMcpDirective([{ id: 'github' }]);
+      expect(directive.startsWith('## External MCP servers')).toBe(true);
     });
   });