Missing XML entity decoding for parameter values

The regex parser captures parameter content verbatim without decoding XML entities. Well-formed XML requires & to be encoded as &, < as <, > as >, etc. If MiniMax M2.5 emits compliant XML, a command like echo "hello & world" would appear in the raw XML as echo "hello & world", and that encoded string would be forwarded to the tool unchanged.

The test at line 679 of the test file explicitly asserts this pass-through behavior — confirming the issue is known but not resolved. If MiniMax does encode entities (even partially), tool invocations with &, <, or " in parameter values will silently receive the wrong input.

Consider adding a minimal entity decode step:

const HTML_ENTITIES: Record<string, string> = {
  "&amp;": "&",
  "&lt;": "<",
  "&gt;": ">",
  "&quot;": '"',
  "&apos;": "'",
};

function decodeXmlEntities(value: string): string {
  return value.replace(/&(?:amp|lt|gt|quot|apos);/gi, (entity) => HTML_ENTITIES[entity.toLowerCase()] ?? entity);
}

Then apply it when storing the parameter value:

input[paramMatch[1]] = decodeXmlEntities(paramMatch[2]!);

If MiniMax is confirmed to never encode entities, document that assumption explicitly in the JSDoc so future maintainers don't have to rediscover it.

This is a comment left during a code review.
Path: src/agents/pi-embedded-utils.ts
Line: 48-51

Comment:
**Missing XML entity decoding for parameter values**

The regex parser captures parameter content verbatim without decoding XML entities. Well-formed XML requires `&` to be encoded as `&amp;`, `<` as `&lt;`, `>` as `&gt;`, etc. If MiniMax M2.5 emits compliant XML, a command like `echo "hello & world"` would appear in the raw XML as `echo "hello &amp; world"`, and that encoded string would be forwarded to the tool unchanged.

The test at line 679 of the test file explicitly asserts this pass-through behavior — confirming the issue is known but not resolved. If MiniMax does encode entities (even partially), tool invocations with `&`, `<`, or `"` in parameter values will silently receive the wrong input.

Consider adding a minimal entity decode step:

```typescript
const HTML_ENTITIES: Record<string, string> = {
  "&amp;": "&",
  "&lt;": "<",
  "&gt;": ">",
  "&quot;": '"',
  "&apos;": "'",
};

function decodeXmlEntities(value: string): string {
  return value.replace(/&(?:amp|lt|gt|quot|apos);/gi, (entity) => HTML_ENTITIES[entity.toLowerCase()] ?? entity);
}
```

Then apply it when storing the parameter value:

```typescript
input[paramMatch[1]] = decodeXmlEntities(paramMatch[2]!);
```

If MiniMax is confirmed to never encode entities, document that assumption explicitly in the JSDoc so future maintainers don't have to rediscover it.

How can I resolve this? If you propose a fix, please make it concise.

Interleaved prose order is not preserved across multiple tool calls

When a single text block contains prose between multiple <invoke> blocks — e.g.:

Let me read the file.
<invoke name="Read">...</invoke></minimax:tool_call>
Now let me run the command.
<invoke name="Bash">...</invoke></minimax:tool_call>

stripMinimaxToolCallXml collapses all prose into a single string ("Let me read the file.\nNow let me run the command."), which is pushed as one text block before all toolUse blocks. The interleaved ordering ("read → prose → bash") is lost; downstream consumers will see all prose up-front followed by both tool calls.

This matches how promoteThinkingTagsToBlocks works (it preserves order within a single text block), but it is different from what a streaming model would produce. If ordering matters for the tool-dispatch pipeline, the prose segments should be split and interleaved between the toolUse blocks. If it intentionally doesn't matter, a brief comment would help future readers.

This is a comment left during a code review.
Path: src/agents/pi-embedded-utils.ts
Line: 109-123

Comment:
**Interleaved prose order is not preserved across multiple tool calls**

When a single text block contains prose _between_ multiple `<invoke>` blocks — e.g.:

```
Let me read the file.
<invoke name="Read">...</invoke></minimax:tool_call>
Now let me run the command.
<invoke name="Bash">...</invoke></minimax:tool_call>
```

`stripMinimaxToolCallXml` collapses all prose into a single string (`"Let me read the file.\nNow let me run the command."`), which is pushed as one text block _before_ all `toolUse` blocks. The interleaved ordering ("read → prose → bash") is lost; downstream consumers will see all prose up-front followed by both tool calls.

This matches how `promoteThinkingTagsToBlocks` works (it preserves order within a single text block), but it is different from what a streaming model would produce. If ordering matters for the tool-dispatch pipeline, the prose segments should be split and interleaved between the `toolUse` blocks. If it intentionally doesn't matter, a brief comment would help future readers.

How can I resolve this? If you propose a fix, please make it concise.

Module-level mutable counter shared across all tests and all runtime invocations

minimaxToolCallCounter is a module-level variable. In a long-running process it is monotonically increasing, which guarantees uniqueness — but:

1. Concurrent tests: vitest can run test files in parallel worker threads. Each worker gets its own module instance so the counter resets per worker, meaning two workers can produce the same toolu_minimax_1 ID for the same session/message. This won't cause test failures (tests only check uniqueness within a call), but if IDs are stored and later correlated they could collide.
2. Readability: A counter makes IDs predictable/sequential, but crypto.randomUUID() (or even Date.now() + Math.random()) would make IDs truly unique without shared state.

Consider:

id: `toolu_minimax_${Date.now()}_${Math.random().toString(36).slice(2)}`,

or using crypto.randomUUID() if available in the target runtime.

This is a comment left during a code review.
Path: src/agents/pi-embedded-utils.ts
Line: 18

Comment:
**Module-level mutable counter shared across all tests and all runtime invocations**

`minimaxToolCallCounter` is a module-level variable. In a long-running process it is monotonically increasing, which guarantees uniqueness — but:

1. **Concurrent tests**: `vitest` can run test files in parallel worker threads. Each worker gets its own module instance so the counter resets per worker, meaning two workers can produce the same `toolu_minimax_1` ID for the same session/message. This won't cause test failures (tests only check uniqueness _within_ a call), but if IDs are stored and later correlated they could collide.
2. **Readability**: A counter makes IDs predictable/sequential, but `crypto.randomUUID()` (or even `Date.now() + Math.random()`) would make IDs truly unique without shared state.

Consider:
```typescript
id: `toolu_minimax_${Date.now()}_${Math.random().toString(36).slice(2)}`,
```
or using `crypto.randomUUID()` if available in the target runtime.

How can I resolve this? If you propose a fix, please make it concise.

Scope invoke parsing to minimax tool_call segments

This regex parses every <invoke ...> in the entire text as soon as minimax:tool_call appears anywhere, so explanatory/code-sample XML can be promoted into executable toolUse blocks even when it is not an actual tool invocation. In practice, a reply that discusses MiniMax XML format (or echoes user-provided XML) can accidentally trigger real tool execution; parsing should be limited to explicit <minimax:tool_call>...</minimax:tool_call> sections (or another strict envelope) before promoting blocks.

Useful? React with 👍 / 👎.

Decode XML entities before assigning tool parameters

Parameter values are copied verbatim from XML text, so escaped entities remain encoded in tool inputs (for example & instead of &). If MiniMax emits valid XML escaping, tool arguments become semantically wrong (&&, <, > and similar tokens are corrupted), which can break command/tool behavior despite the call being otherwise parsed successfully.

Useful? React with 👍 / 👎.

Strip non-dispatched <invoke> tags from emitted prose

Only <minimax:tool_call> wrappers are removed from prose, so when a text block contains at least one parseable <invoke name="..."> plus any other <invoke> fragment that is not promoted, that leftover XML is emitted as user-visible text. For example, Before<invoke>debug</invoke><minimax:tool_call><invoke name="Bash">...</invoke></minimax:tool_call> will keep <invoke>debug</invoke> after promotion, and extractAssistantText will no longer strip it because stripMinimaxToolCallXml is gated on minimax:tool_call being present. This is a regression from the previous strip-only path that removed all <invoke> blocks in MiniMax-tagged text.

Useful? React with 👍 / 👎.

Parse MiniMax tool XML before turn completion

Calling promoteMinimaxToolCallsToBlocks only in handleMessageEnd means XML is converted after the assistant turn has already completed, so the newly created toolUse blocks are observational only and are not dispatched through the normal tool-execution path; on later turns this can be misread as dangling tool calls and repaired into synthetic missing toolResult errors during history sanitization. This triggers whenever MiniMax emits XML tool-call text, so promotion needs to happen in the pre-dispatch message path (or not emit toolUse blocks at this stage).

Useful? React with 👍 / 👎.

Wire MiniMax tool-call promotion into runtime message handling

This new promotion path is currently dead code: a repo-wide usage check only finds promoteMinimaxToolCallsToBlocks in this file and its tests, so assistant turns never run it in production. As a result, MiniMax XML tool calls are still just text and then get removed by the existing text sanitizer path, meaning the commit does not actually dispatch the tool calls it was meant to preserve.

Useful? React with 👍 / 👎.

Accept MiniMax invocations without an opening tool_call tag

The parser only extracts <invoke> blocks from fully wrapped <minimax:tool_call>...</minimax:tool_call> segments, but this codebase’s MiniMax fixtures (including several in this same test file) commonly use ...<invoke ...></invoke></minimax:tool_call> without the opening tag. In that format parseMinimaxToolCallXml returns no tool calls, so promotion silently falls back and the invocation is stripped instead of executed.

Useful? React with 👍 / 👎.