Preserve turn separator across state reset

Appending "\n\n" just before resetAssistantMessageState(...) does not actually separate turns in streaming output, because the reset immediately clears deltaBuffer, blockBuffer, and the blockChunker buffer (src/agents/pi-embedded-subscribe.ts), so the separator is discarded before any later text_delta/drain can emit it. In blockStreaming sessions that span multiple assistant messages (for example around tool calls), this leaves the original concatenation bug unresolved.

Useful? React with 👍 / 👎.

Separator is immediately erased by resetAssistantMessageState

The \n\n is appended to all three buffers (deltaBuffer, blockBuffer/blockChunker) and then immediately cleared by the very next call to ctx.resetAssistantMessageState(...).

Looking at the implementation of resetAssistantMessageState in pi-embedded-subscribe.ts:

const resetAssistantMessageState = (nextAssistantTextBaseline: number) => {
  state.deltaBuffer = "";    // clears the \n\n we just added
  state.blockBuffer = "";    // clears the \n\n we just added
  blockChunker?.reset();     // clears the \n\n we just appended
  // ...
};

And EmbeddedBlockChunker.reset() simply does this.#buffer = "", and append() only buffers without emitting — so blockChunker.append("\n\n") followed immediately by blockChunker.reset() discards the separator entirely.

As a result, the fix is a no-op: the \n\n separator is added and then erased before any streaming output can observe it. The bug (#35308) remains unfixed.

The separator needs to be added after resetAssistantMessageState, preserving the separator in the freshly-cleared buffers so the next turn's text is appended after it:

const hadPreviousTurnContent = ctx.state.deltaBuffer.trim().length > 0;
ctx.resetAssistantMessageState(ctx.state.assistantTexts.length);
if (hadPreviousTurnContent) {
  ctx.state.deltaBuffer = "\n\n";
  if (ctx.blockChunker) {
    ctx.blockChunker.append("\n\n");
  } else {
    ctx.state.blockBuffer = "\n\n";
  }
}
// Use assistant message_start as the earliest "writing" signal for typing.
void ctx.params.onAssistantMessageStart?.();

This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.messages.ts
Line: 76-85

Comment:
**Separator is immediately erased by `resetAssistantMessageState`**

The `\n\n` is appended to all three buffers (`deltaBuffer`, `blockBuffer`/`blockChunker`) and then **immediately cleared** by the very next call to `ctx.resetAssistantMessageState(...)`.

Looking at the implementation of `resetAssistantMessageState` in `pi-embedded-subscribe.ts`:

```typescript
const resetAssistantMessageState = (nextAssistantTextBaseline: number) => {
  state.deltaBuffer = "";    // clears the \n\n we just added
  state.blockBuffer = "";    // clears the \n\n we just added
  blockChunker?.reset();     // clears the \n\n we just appended
  // ...
};
```

And `EmbeddedBlockChunker.reset()` simply does `this.#buffer = ""`, and `append()` only buffers without emitting — so `blockChunker.append("\n\n")` followed immediately by `blockChunker.reset()` discards the separator entirely.

As a result, the fix is a no-op: the `\n\n` separator is added and then erased before any streaming output can observe it. The bug (#35308) remains unfixed.

The separator needs to be added **after** `resetAssistantMessageState`, preserving the separator in the freshly-cleared buffers so the next turn's text is appended after it:

```typescript
const hadPreviousTurnContent = ctx.state.deltaBuffer.trim().length > 0;
ctx.resetAssistantMessageState(ctx.state.assistantTexts.length);
if (hadPreviousTurnContent) {
  ctx.state.deltaBuffer = "\n\n";
  if (ctx.blockChunker) {
    ctx.blockChunker.append("\n\n");
  } else {
    ctx.state.blockBuffer = "\n\n";
  }
}
// Use assistant message_start as the earliest "writing" signal for typing.
void ctx.params.onAssistantMessageStart?.();
```

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

Test doesn't exercise the actual bug it claims to cover

The test uses a quoted YAML name (name: "12306"), which YAML parses as a string — meaning skill.name is already a string when it reaches resolveBundledSkillsContext. The String() coercion on line 35 of bundled-context.ts is therefore a no-op here, and this test would pass identically both before and after the fix.

The actual bug is triggered when YAML parses an unquoted integer (e.g., name: 12306) and the underlying loadSkillsFromDir returns a skill whose .name is a number. The test comments acknowledge this by noting the upstream validateName() function would also throw, but this means the regression test for the actual code path doesn't exist.

Consider mocking loadSkillsFromDir to return a skill with a numeric name directly, so the String() coercion is actually exercised:

vi.mock("@mariozechner/pi-coding-agent", () => ({
  loadSkillsFromDir: () => ({ skills: [{ name: 12306, source: "openclaw-bundled" }] }),
}));

Without this, the fix in bundled-context.ts has no automated regression coverage against the actual numeric-name scenario.

This is a comment left during a code review.
Path: src/agents/skills/bundled-context.test.ts
Line: 22-52

Comment:
**Test doesn't exercise the actual bug it claims to cover**

The test uses a **quoted** YAML name (`name: "12306"`), which YAML parses as a `string` — meaning `skill.name` is already a `string` when it reaches `resolveBundledSkillsContext`. The `String()` coercion on line 35 of `bundled-context.ts` is therefore a no-op here, and this test would pass identically both before and after the fix.

The actual bug is triggered when YAML parses an **unquoted** integer (e.g., `name: 12306`) and the underlying `loadSkillsFromDir` returns a skill whose `.name` is a `number`. The test comments acknowledge this by noting the upstream `validateName()` function would also throw, but this means the regression test for the actual code path doesn't exist.

Consider mocking `loadSkillsFromDir` to return a skill with a numeric name directly, so the `String()` coercion is actually exercised:

```typescript
vi.mock("@mariozechner/pi-coding-agent", () => ({
  loadSkillsFromDir: () => ({ skills: [{ name: 12306, source: "openclaw-bundled" }] }),
}));
```

Without this, the fix in `bundled-context.ts` has no automated regression coverage against the actual numeric-name scenario.

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

Use ASCII quotes for separator string literals

These separator assignments use typographic quotes (“/”) instead of normal string delimiters, which makes the file invalid TypeScript and prevents this handler from compiling/running. Replace them with ASCII quotes ("\n\n") so the stream message path remains executable.

Useful? React with 👍 / 👎.

Preserve separator when paragraph chunk flushing is enabled

Appending "\n\n" as a standalone chunk to blockChunker still loses the turn boundary in chunkMode="newline" flows: resolveBlockStreamingChunking enables flushOnParagraph for that mode (src/auto-reply/reply/block-streaming.ts:186), and EmbeddedBlockChunker.#drainParagraphs drops leading paragraph breaks as empty chunks (src/agents/pi-embedded-block-chunker.ts:192-199). In those environments, assistant text before and after a tool call is still concatenated without the intended separator.

Useful? React with 👍 / 👎.