🪑 fix: Rebase Activity Phase Bounds Onto Compacted Content and Unskip the MCPManager Suite (#14782)

* 🧭 fix: Rebase Activity Phase Bounds Onto Compacted Content

`filterMalformedContentParts` compacts the aggregator's content array —
`Array.prototype.filter` skips holes and drops malformed tool calls — but a
parent phase marker's `activity_start_index`/`activity_end_index` still address
the pre-filter positions. The array is routinely sparse: the aggregator writes
parts at provider-source indexes, so a model turn that emits no text before its
tool calls leaves an empty slot.

Every part after a hole therefore shifts left on persistence while the bounds
stay put, so the stored phase claims the wrong range — the final answer is
swallowed into the parent card and the marker's own slot is counted as a child.
The in-run analogue (`rebaseActivityPhaseBounds`) already rebases after
completion-time reshaping; the final compaction had no such step.

Rebase the bounds as part of the compaction, mapping each bound to the number
of retained parts ahead of it. The mapping is monotonic, so `start <= end <=
markerIndex` survives, and an identity mapping leaves untouched arrays — and
their marker objects — exactly as they were. Markers are copied rather than
mutated so the caller's array keeps its own coordinates, which the live stream
and the resume snapshot still address.

Fixes the `activity-phases` e2e failure on dev and the same defect on the two
resume persistence paths.

* 🔌 fix: Stop Replacing the Env Module in the MCPManager Suite

`MCPManager.test.ts` mocked `~/utils/env` with a factory that replaced the whole
module. #14780 then made `~/mcp/utils` read `ALLOWED_BODY_FIELDS` from that
module at module scope, so importing `~/mcp/oauth` -> `handler.ts` ->
`~/mcp/utils` evaluated `undefined.map(...)` and the suite died at import time.
All 111 of its tests have been silently skipped since; the shard has been red on
dev, on this PR, and on release-v0.8.8-rc1.

Spread the real module and keep only the mock that earns its place.
`processMCPEnv` stays a seam: fifteen cases drive it with `mockReturnValue` /
`mockImplementation` to hand the manager a specific processed config, and one
asserts its call count, so making it real would couple these tests to
env-substitution logic. `isPluginSourced` and `MCP_PLUGIN_SOURCE` were dropped —
the factory restated the real implementations verbatim and no test referenced
either, so they were duplication, not a seam.

111 tests now run and pass.

* 🧪 test: Stop Replacing the Env Module in Three More Suites

Same latent trap as the MCPManager suite: a `jest.mock('~/utils/env', ...)`
factory that replaces the whole module. These three pass today only because
their import graphs never reach `~/mcp/utils`, which reads `ALLOWED_BODY_FIELDS`
from that module at module scope — the next module-scope constant added to
`env.ts` would break all three the same silent way.

Each mock is kept only where it earns its place:

- `activityLabels/host.spec.ts` — dropped. `createSafeUser` was never referenced
  and the stub returned `undefined` where the real function returns `{}`, so the
  mock was strictly less faithful than the real, pure implementation.
- `run-codeTools.test.ts` — dropped. Neither `resolveHeaders` nor
  `createSafeUser` was referenced by any case.
- `run-summarization.test.ts` — `resolveHeaders` is now a spy wrapping the real
  implementation rather than an identity stub. One case asserts templated header
  values go through it, which only means something if the real substitution
  actually runs. `createSafeUser` dropped as unreferenced.

103 suites / 2708 tests green across `src/agents`, `src/utils`, and the
MCPManager suite.

* 📝 docs: Describe the Full Contract of filterMalformedContentParts

Per Copilot's review: the public JSDoc still described the function as only
dropping malformed tool calls, while the implementation also compacts empty
slots and rebases parent activity-phase bounds. The detail lived on the private
helper, so callers reading intellisense saw a stale contract.

State what it actually produces, note that compaction is inherent rather than
incidental (the aggregator writes at provider-source indexes, so the array is
frequently sparse), and add an example of a hole moving a phase bound. The
example was verified against the built runtime, not written from memory.

packages/api/src/agents/__tests__/run-codeTools.test.ts

@@ -35,11 +35,6 @@ jest.mock('winston', () => ({
   transports: { Console: jest.fn(), DailyRotateFile: jest.fn(), File: jest.fn() },
 }));
 
-jest.mock('~/utils/env', () => ({
-  resolveHeaders: jest.fn((opts: { headers: unknown }) => opts?.headers ?? {}),
-  createSafeUser: jest.fn(() => ({})),
-}));
-
 jest.mock('@librechat/data-schemas', () => ({
   ...jest.requireActual('@librechat/data-schemas'),
   logger: { debug: jest.fn(), warn: jest.fn(), error: jest.fn(), info: jest.fn() },

packages/api/src/agents/__tests__/run-summarization.test.ts

@@ -40,11 +40,12 @@ jest.mock('winston', () => ({
   },
 }));
 
-// Mock env utilities so header resolution doesn't fail
-jest.mock('~/utils/env', () => ({
-  resolveHeaders: jest.fn((opts: { headers: unknown }) => opts?.headers ?? {}),
-  createSafeUser: jest.fn(() => ({})),
-}));
+/** Spy on the real `resolveHeaders` instead of replacing it — the templated-header
+ *  case below only proves anything if the actual substitution runs. */
+jest.mock('~/utils/env', () => {
+  const actual = jest.requireActual<typeof import('~/utils/env')>('~/utils/env');
+  return { ...actual, resolveHeaders: jest.fn(actual.resolveHeaders) };
+});
 
 jest.mock('@librechat/data-schemas', () => ({
   ...jest.requireActual('@librechat/data-schemas'),

packages/api/src/agents/activityLabels/__tests__/host.spec.ts

@@ -17,7 +17,6 @@ jest.mock('~/endpoints/config/providers', () => ({
   })),
 }));
 jest.mock('~/utils/headers', () => ({ resolveConfigHeaders: jest.fn() }));
-jest.mock('~/utils/env', () => ({ createSafeUser: jest.fn(() => undefined) }));
 
 const appConfig = (endpoints: Record<string, unknown>): AppConfig =>
   ({ endpoints }) as unknown as AppConfig;

packages/api/src/mcp/__tests__/MCPManager.test.ts

@@ -37,10 +37,13 @@ jest.mock('~/mcp/oauth', () => ({
   resolveOboToken: jest.fn(),
 }));
 
+/** Only `processMCPEnv` is a deliberate seam — the cases below drive it to hand
+ *  the manager a specific processed config. Everything else stays real:
+ *  `~/mcp/utils` reads `ALLOWED_BODY_FIELDS` from here at module scope, so a
+ *  replacing factory breaks the suite at import time. */
 jest.mock('~/utils/env', () => ({
+  ...jest.requireActual('~/utils/env'),
   processMCPEnv: jest.fn((params) => params.options),
-  MCP_PLUGIN_SOURCE: 'plugin',
-  isPluginSourced: jest.fn((config) => config?.source === 'plugin'),
 }));
 
 jest.mock('~/auth/domain', () => ({

packages/api/src/utils/content.spec.ts

@@ -197,4 +197,138 @@ describe('filterMalformedContentParts', () => {
       expect(result).toHaveLength(1);
     });
   });
+
+  describe('parent activity phase bounds', () => {
+    const toolPart = (id: string): TMessageContentParts => ({
+      type: ContentTypes.TOOL_CALL,
+      tool_call: {
+        id,
+        name: 'fetch_fact',
+        type: ToolCallTypes.TOOL_CALL,
+        args: '{}',
+        progress: 1,
+        output: 'result',
+      },
+    });
+    const childLabel = (label: string): TMessageContentParts =>
+      ({ type: ContentTypes.ACTIVITY_LABEL, activity_label: label }) as TMessageContentParts;
+    const phaseMarker = (start: number, end: number): TMessageContentParts =>
+      ({
+        type: ContentTypes.ACTIVITY_LABEL,
+        activity_label: 'Gathered both facts',
+        activity_label_type: 'phase',
+        activity_start_index: start,
+        activity_end_index: end,
+        activity_count: 2,
+      }) as TMessageContentParts;
+    const phaseOf = (parts: TMessageContentParts[]) =>
+      parts.find(
+        (part) =>
+          part.type === ContentTypes.ACTIVITY_LABEL &&
+          (part as { activity_label_type?: string }).activity_label_type === 'phase',
+      ) as { activity_start_index?: number; activity_end_index?: number } | undefined;
+
+    /** A model turn that emits no text before its tool calls leaves the source
+     *  slot empty, so the aggregator's array arrives here sparse. */
+    const sparseRun = (): TMessageContentParts[] => {
+      const parts: TMessageContentParts[] = [];
+      parts[1] = toolPart('call_alpha');
+      parts[2] = childLabel('Looked up alpha');
+      parts[4] = toolPart('call_beta');
+      parts[5] = childLabel('Looked up beta');
+      parts[6] = { type: ContentTypes.TEXT, text: 'Final answer' };
+      parts[7] = phaseMarker(0, 6);
+      parts.length = 8;
+      return parts;
+    };
+
+    it('rebases bounds onto compacted coordinates when holes are removed', () => {
+      const result = filterMalformedContentParts(sparseRun());
+
+      expect(result).toHaveLength(6);
+      const finalTextIndex = result.findIndex(
+        (part) => part.type === ContentTypes.TEXT && part.text === 'Final answer',
+      );
+      expect(finalTextIndex).toBe(4);
+      /** The exclusive end must still land on the final answer so the parent
+       *  card groups the two activities and nothing more. */
+      expect(phaseOf(result)?.activity_end_index).toBe(finalTextIndex);
+      expect(phaseOf(result)?.activity_start_index).toBe(0);
+    });
+
+    it('keeps the grouped children exactly the tool and label parts', () => {
+      const result = filterMalformedContentParts(sparseRun());
+      const phase = phaseOf(result);
+      const children = result.slice(phase?.activity_start_index ?? 0, phase?.activity_end_index);
+
+      expect(children).toHaveLength(4);
+      expect(
+        children
+          .map((part) => (part.type === ContentTypes.TOOL_CALL ? part.tool_call?.id : undefined))
+          .filter(Boolean),
+      ).toEqual(['call_alpha', 'call_beta']);
+      expect(children.some((part) => part.type === ContentTypes.TEXT)).toBe(false);
+    });
+
+    it('leaves the source array in its own coordinate space', () => {
+      const parts = sparseRun();
+      filterMalformedContentParts(parts);
+
+      expect(phaseOf(parts.filter(Boolean) as TMessageContentParts[])?.activity_end_index).toBe(6);
+    });
+
+    it('rebases past a dropped malformed tool_call', () => {
+      const parts: TMessageContentParts[] = [
+        toolPart('call_alpha'),
+        { type: ContentTypes.TOOL_CALL } as TMessageContentParts,
+        { type: ContentTypes.TEXT, text: 'Final answer' },
+        phaseMarker(0, 2),
+      ];
+
+      const result = filterMalformedContentParts(parts);
+
+      expect(result).toHaveLength(3);
+      expect(phaseOf(result)?.activity_end_index).toBe(1);
+      expect(result[1]).toMatchObject({ type: ContentTypes.TEXT, text: 'Final answer' });
+    });
+
+    it('maps a bound that lands on a dropped slot to the next retained part', () => {
+      const parts: TMessageContentParts[] = [];
+      parts[0] = toolPart('call_alpha');
+      parts[2] = { type: ContentTypes.TEXT, text: 'Final answer' };
+      parts[3] = phaseMarker(1, 2);
+      parts.length = 4;
+
+      const result = filterMalformedContentParts(parts);
+
+      expect(phaseOf(result)?.activity_start_index).toBe(1);
+      expect(phaseOf(result)?.activity_end_index).toBe(1);
+    });
+
+    it('leaves bounds and part identity untouched when nothing is dropped', () => {
+      const parts: TMessageContentParts[] = [
+        toolPart('call_alpha'),
+        { type: ContentTypes.TEXT, text: 'Final answer' },
+        phaseMarker(0, 1),
+      ];
+
+      const result = filterMalformedContentParts(parts);
+
+      expect(result[2]).toBe(parts[2]);
+      expect(phaseOf(result)?.activity_start_index).toBe(0);
+      expect(phaseOf(result)?.activity_end_index).toBe(1);
+    });
+
+    it('does not touch per-batch activity labels', () => {
+      const parts: TMessageContentParts[] = [];
+      parts[1] = toolPart('call_alpha');
+      parts[2] = childLabel('Looked up alpha');
+      parts.length = 3;
+
+      const result = filterMalformedContentParts(parts);
+
+      expect(result).toHaveLength(2);
+      expect(result[1]).toBe(parts[2]);
+    });
+  });
 });

packages/api/src/utils/content.ts

@@ -1,13 +1,112 @@
 import { ContentTypes } from 'librechat-data-provider';
 import type { TMessageContentParts } from 'librechat-data-provider';
 
+type ActivityLabelPart = Extract<TMessageContentParts, { type: ContentTypes.ACTIVITY_LABEL }>;
+
+function isRetainablePart(
+  part: TMessageContentParts | null | undefined,
+): part is TMessageContentParts {
+  if (!part || typeof part !== 'object') {
+    return false;
+  }
+
+  if (part.type === ContentTypes.TOOL_CALL) {
+    return 'tool_call' in part && part.tool_call != null && typeof part.tool_call === 'object';
+  }
+
+  return true;
+}
+
+function isPhaseMarker(part: TMessageContentParts): part is ActivityLabelPart {
+  return (
+    part.type === ContentTypes.ACTIVITY_LABEL &&
+    (part as ActivityLabelPart).activity_label_type === 'phase'
+  );
+}
+
 /**
- * Filters out malformed tool call content parts that don't have the required tool_call property.
- * This handles edge cases where tool_call content parts may be created with only a type property
- * but missing the actual tool_call data.
+ * Rebase a parent-phase bound from source coordinates onto compacted ones.
+ * `retainedBefore[i]` counts the parts kept ahead of source index `i`, which is
+ * the compacted position of source `i` for an inclusive start and the exclusive
+ * end for a bound that lands on a dropped slot.
+ */
+function rebaseBound(bound: number, retainedBefore: number[], sourceLength: number): number {
+  return retainedBefore[Math.max(0, Math.min(sourceLength, bound))];
+}
+
+/**
+ * Removes malformed tool call parts and any empty slots, then rebases parent
+ * activity-phase bounds onto the compacted coordinates.
  *
- * @param contentParts - Array of content parts to filter
- * @returns Filtered array with malformed tool calls removed
+ * The source array is frequently sparse: the aggregator writes parts at
+ * provider-source indexes, so a model turn that emits no text before its tool
+ * calls leaves holes. Compacting shifts every part after a hole, while a phase
+ * marker's `activity_start_index`/`activity_end_index` still address the source
+ * positions — leaving the persisted bounds pointing at the wrong parts (the
+ * final answer gets swallowed into the parent card, or the marker's own slot is
+ * claimed as a child).
+ *
+ * Markers are copied rather than mutated: the caller's array stays in its own
+ * coordinate space, which the live stream and the resume snapshot still use.
+ */
+function compactContentParts(contentParts: TMessageContentParts[]): TMessageContentParts[] {
+  const retained: TMessageContentParts[] = [];
+  const retainedBefore: number[] = new Array<number>(contentParts.length + 1);
+  let phaseMarkerCount = 0;
+
+  for (let index = 0; index < contentParts.length; index += 1) {
+    retainedBefore[index] = retained.length;
+    const part = contentParts[index];
+    if (!isRetainablePart(part)) {
+      continue;
+    }
+    if (isPhaseMarker(part)) {
+      phaseMarkerCount += 1;
+    }
+    retained.push(part);
+  }
+  retainedBefore[contentParts.length] = retained.length;
+
+  if (phaseMarkerCount === 0 || retained.length === contentParts.length) {
+    return retained;
+  }
+
+  for (let index = 0; index < retained.length; index += 1) {
+    const part = retained[index];
+    if (!isPhaseMarker(part)) {
+      continue;
+    }
+    const { activity_start_index: start, activity_end_index: end } = part;
+    const nextStart =
+      typeof start === 'number' ? rebaseBound(start, retainedBefore, contentParts.length) : start;
+    const nextEnd =
+      typeof end === 'number' ? rebaseBound(end, retainedBefore, contentParts.length) : end;
+    if (nextStart === start && nextEnd === end) {
+      continue;
+    }
+    retained[index] = {
+      ...part,
+      ...(typeof nextStart === 'number' && { activity_start_index: nextStart }),
+      ...(typeof nextEnd === 'number' && { activity_end_index: nextEnd }),
+    };
+  }
+
+  return retained;
+}
+
+/**
+ * Produces the durable content array: drops malformed tool call parts that lack the
+ * required tool_call property, compacts away empty slots, and rebases parent
+ * activity-phase bounds onto the resulting coordinates.
+ *
+ * Compaction is not incidental — the source array is frequently sparse, because the
+ * aggregator writes parts at provider-source indexes. Since that shifts positions, any
+ * index-referencing metadata is rebased to match; see {@link compactContentParts}.
+ *
+ * Non-array input is returned unchanged.
+ *
+ * @param contentParts - Array of content parts to compact
+ * @returns Compacted array with malformed tool calls removed and phase bounds rebased
  *
  * @example
  * // Removes malformed tool_call without the tool_call property
@@ -18,6 +117,16 @@ import type { TMessageContentParts } from 'librechat-data-provider';
  * ];
  * const filtered = filterMalformedContentParts(parts);
  * // Returns all parts except the malformed tool_call
+ *
+ * @example
+ * // A hole shifts later parts, so a phase marker's bounds move with them
+ * const parts = []; // parts[0] never materialized
+ * parts[1] = { type: 'tool_call', tool_call: { id: 'a' } };
+ * parts[2] = { type: 'text', text: 'Final answer' };
+ * parts[3] = { type: 'activity_label', activity_label_type: 'phase',
+ *              activity_start_index: 0, activity_end_index: 2 };
+ * const filtered = filterMalformedContentParts(parts);
+ * // Final answer is now at index 1, and activity_end_index is rebased to 1
  */
 export function filterMalformedContentParts(
   contentParts: TMessageContentParts[],
@@ -30,17 +139,5 @@ export function filterMalformedContentParts<T>(
     return contentParts;
   }
 
-  return contentParts.filter((part) => {
-    if (!part || typeof part !== 'object') {
-      return false;
-    }
-
-    const { type } = part;
-
-    if (type === ContentTypes.TOOL_CALL) {
-      return 'tool_call' in part && part.tool_call != null && typeof part.tool_call === 'object';
-    }
-
-    return true;
-  });
+  return compactContentParts(contentParts);
 }