Merge PR #5190: fix(agent): escape Harmony compaction markers (@roboomp)
This commit is contained in:
@@ -14,6 +14,9 @@
|
||||
### Added
|
||||
|
||||
- Added a process-global pause gate (`agentPauseGate`) to safely pause agent loops before model calls or tool executions, allowing them to be resumed later or aborted cleanly.
|
||||
### Fixed
|
||||
|
||||
- Fixed compaction summary inputs escaping Harmony control tokens so Copilot `gpt-5.6-*` models no longer reject serialized analysis-channel markers. ([#5184](https://github.com/can1357/oh-my-pi/issues/5184))
|
||||
|
||||
## [16.4.3] - 2026-07-11
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ import {
|
||||
extractFileOpsFromMessage,
|
||||
type FileOperations,
|
||||
SUMMARIZATION_SYSTEM_PROMPT,
|
||||
serializeConversation,
|
||||
serializeConversationForSummary,
|
||||
stripReadSelector,
|
||||
truncateToolResultForSummary,
|
||||
upsertFileOperations,
|
||||
@@ -320,7 +320,7 @@ export async function generateBranchSummary(
|
||||
// Transform to LLM-compatible messages, then serialize to text
|
||||
// Serialization prevents the model from treating it as a conversation to continue
|
||||
const llmMessages = (options.convertToLlm ?? defaultConvertToLlm)(messages);
|
||||
const conversationText = serializeConversation(llmMessages, preferredDialect(model.id));
|
||||
const conversationText = serializeConversationForSummary(llmMessages, preferredDialect(model.id));
|
||||
|
||||
// Build prompt
|
||||
const instructions = customInstructions || BRANCH_SUMMARY_PROMPT;
|
||||
|
||||
@@ -66,7 +66,7 @@ import {
|
||||
extractFileOpsFromMessage,
|
||||
type FileOperations,
|
||||
SUMMARIZATION_SYSTEM_PROMPT,
|
||||
serializeConversation,
|
||||
serializeConversationForSummary,
|
||||
stripReadSelector,
|
||||
upsertFileOperations,
|
||||
} from "./utils";
|
||||
@@ -816,7 +816,7 @@ export async function generateSummary(
|
||||
// Serialize conversation to text so model doesn't try to continue it
|
||||
// Convert to LLM messages first (handles custom app messages when caller provides a transformer).
|
||||
const llmMessages = (options?.convertToLlm ?? defaultConvertToLlm)(currentMessages);
|
||||
const conversationText = serializeConversation(llmMessages, preferredDialect(model.id));
|
||||
const conversationText = serializeConversationForSummary(llmMessages, preferredDialect(model.id));
|
||||
|
||||
// Build the prompt with conversation wrapped in tags
|
||||
let promptText = `<conversation>\n${conversationText}\n</conversation>\n\n`;
|
||||
@@ -1030,7 +1030,7 @@ async function generateShortSummary(
|
||||
): Promise<string> {
|
||||
const maxTokens = Math.min(512, Math.floor(0.2 * reserveTokens));
|
||||
const llmMessages = (options?.convertToLlm ?? defaultConvertToLlm)(recentMessages);
|
||||
const conversationText = serializeConversation(llmMessages, preferredDialect(model.id));
|
||||
const conversationText = serializeConversationForSummary(llmMessages, preferredDialect(model.id));
|
||||
|
||||
let promptText = `<conversation>\n${conversationText}\n</conversation>\n\n`;
|
||||
if (historySummary) {
|
||||
@@ -1578,7 +1578,7 @@ async function generateTurnPrefixSummary(
|
||||
const maxTokens = Math.floor(0.5 * reserveTokens); // Smaller budget for turn prefix
|
||||
|
||||
const llmMessages = (options?.convertToLlm ?? defaultConvertToLlm)(messages);
|
||||
const conversationText = serializeConversation(llmMessages, preferredDialect(model.id));
|
||||
const conversationText = serializeConversationForSummary(llmMessages, preferredDialect(model.id));
|
||||
const promptText = `<conversation>\n${conversationText}\n</conversation>\n\n${TURN_PREFIX_SUMMARIZATION_PROMPT}`;
|
||||
const summarizationMessages = [
|
||||
{
|
||||
|
||||
@@ -207,9 +207,19 @@ export function truncateToolResultForSummary(text: string): string {
|
||||
return `${text.slice(0, TOOL_RESULT_MAX_CHARS)}\n\n[... ${truncatedChars} more characters truncated]`;
|
||||
}
|
||||
|
||||
const HARMONY_CONTROL_TOKEN_RE = /<\|(start|end|message|channel|constrain|return|call)\|>/g;
|
||||
|
||||
/**
|
||||
* Serialize LLM messages to text for summarization.
|
||||
* This prevents the model from treating it as a conversation to continue.
|
||||
* Serialize LLM messages as plain summary input without provider control tokens.
|
||||
*/
|
||||
export function serializeConversationForSummary(messages: Message[], dialect?: Dialect): string {
|
||||
const conversation = serializeConversation(messages, dialect);
|
||||
if (dialect !== "harmony") return conversation;
|
||||
return conversation.replace(HARMONY_CONTROL_TOKEN_RE, "<\\|$1\\|>");
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize LLM messages to transcript text.
|
||||
* Call convertToLlm() first to handle custom message types.
|
||||
*/
|
||||
export function serializeConversation(messages: Message[], dialect?: Dialect): string {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { serializeConversation } from "@oh-my-pi/pi-agent-core/compaction";
|
||||
import { serializeConversation, serializeConversationForSummary } from "@oh-my-pi/pi-agent-core/compaction";
|
||||
import type { AssistantMessage, Message, ToolResultMessage, Usage } from "@oh-my-pi/pi-ai";
|
||||
|
||||
const ZERO_USAGE: Usage = {
|
||||
@@ -82,6 +82,41 @@ describe("serializeConversation — useless pairs", () => {
|
||||
expect(out).not.toContain("[Assistant tool calls]:");
|
||||
});
|
||||
|
||||
test("summary serialization escapes Harmony control tokens while preserving assistant thinking", () => {
|
||||
const messages = [
|
||||
assistantMessage([
|
||||
{ type: "thinking", thinking: "Need to inspect the failing compaction path." },
|
||||
{ type: "text", text: "The final answer stays visible." },
|
||||
]),
|
||||
];
|
||||
|
||||
const out = serializeConversationForSummary(messages, "harmony");
|
||||
|
||||
expect(out).not.toContain("<|channel|>analysis");
|
||||
expect(out).not.toContain("<|message|>");
|
||||
expect(out).toContain("<\\|channel\\|>analysis");
|
||||
expect(out).toContain("<\\|channel\\|>final");
|
||||
expect(out).toContain("Need to inspect the failing compaction path.");
|
||||
expect(out).toContain("The final answer stays visible.");
|
||||
});
|
||||
|
||||
test("native Harmony serialization keeps raw transcript markers", () => {
|
||||
const out = serializeConversation(
|
||||
[
|
||||
assistantMessage([
|
||||
{ type: "thinking", thinking: "Native transcript includes analysis." },
|
||||
{ type: "text", text: "Native final text." },
|
||||
]),
|
||||
],
|
||||
"harmony",
|
||||
);
|
||||
|
||||
expect(out).toContain("<|channel|>analysis");
|
||||
expect(out).toContain("<|message|>Native transcript includes analysis.");
|
||||
expect(out).toContain("<|channel|>final");
|
||||
expect(out).toContain("Native final text.");
|
||||
});
|
||||
|
||||
test("native dialect serialization drops empty assistants left by useless calls", () => {
|
||||
const out = serializeConversation(
|
||||
[
|
||||
|
||||
Reference in New Issue
Block a user