Merge remote-tracking branch 'upstream/main' into feat/secret-friendly-names
This commit is contained in:
@@ -694,4 +694,81 @@ describe("AgentSession auto-compaction progress guard", () => {
|
||||
expect(noProgress.length).toBe(1);
|
||||
expect(noProgress[0].level).toBe("warning");
|
||||
});
|
||||
|
||||
it("auto-continues (no warning) when a shake rescue frees the oversized tail", async () => {
|
||||
// The escalation contract: compaction cut at the only turn boundary but the
|
||||
// kept tail (e.g. a huge tool result) still sits over the recovery band. The
|
||||
// guard now runs an elide shake INSIDE that tail; once it frees enough, the
|
||||
// auto-continue proceeds instead of pausing with the no-progress warning.
|
||||
const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(session.agent, "continue").mockResolvedValue();
|
||||
// Residual is over the band until the rescue elides the tail, then drops.
|
||||
let shaken = false;
|
||||
vi.spyOn(session, "getContextUsage").mockImplementation(() =>
|
||||
shaken
|
||||
? { tokens: 1000, contextWindow: 200000, percent: 0.5 }
|
||||
: { tokens: 190000, contextWindow: 200000, percent: 95 },
|
||||
);
|
||||
const shakeSpy = vi.spyOn(session, "shake").mockImplementation(async () => {
|
||||
shaken = true;
|
||||
return { mode: "elide", toolResultsDropped: 1, blocksDropped: 0, tokensFreed: 160000, artifactId: "art-1" };
|
||||
});
|
||||
|
||||
const notices = collectNotices();
|
||||
|
||||
const { promise: compactionDone, resolve: onCompactionDone } = Promise.withResolvers<void>();
|
||||
session.subscribe(event => {
|
||||
if (event.type === "auto_compaction_end") onCompactionDone();
|
||||
});
|
||||
|
||||
const assistantMsg = highUsageAssistant();
|
||||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMsg });
|
||||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMsg] });
|
||||
|
||||
await compactionDone;
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything());
|
||||
expect(promptSpy).toHaveBeenCalledTimes(1);
|
||||
const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT));
|
||||
expect(noProgress.length).toBe(0);
|
||||
const recovery = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes("dead-end recovery"));
|
||||
expect(recovery.length).toBe(1);
|
||||
expect(recovery[0].level).toBe("info");
|
||||
});
|
||||
|
||||
it("still warns when a shake rescue cannot free the irreducible tail", async () => {
|
||||
// When the oversized tail has nothing elide-eligible (image-only or plain
|
||||
// prose), the rescue frees nothing, the residual stays over the band, and
|
||||
// the guard MUST still pause with the single no-progress warning.
|
||||
const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(session.agent, "continue").mockResolvedValue();
|
||||
vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 190000, contextWindow: 200000, percent: 95 });
|
||||
// Nothing eligible: shake reports zero dropped, so residual is unchanged.
|
||||
const shakeSpy = vi
|
||||
.spyOn(session, "shake")
|
||||
.mockResolvedValue({ mode: "elide", toolResultsDropped: 0, blocksDropped: 0, tokensFreed: 0 });
|
||||
|
||||
const notices = collectNotices();
|
||||
|
||||
const { promise: compactionDone, resolve: onCompactionDone } = Promise.withResolvers<void>();
|
||||
session.subscribe(event => {
|
||||
if (event.type === "auto_compaction_end") onCompactionDone();
|
||||
});
|
||||
|
||||
const assistantMsg = highUsageAssistant();
|
||||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMsg });
|
||||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMsg] });
|
||||
|
||||
await compactionDone;
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything());
|
||||
expect(promptSpy).not.toHaveBeenCalled();
|
||||
const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT));
|
||||
expect(noProgress.length).toBe(1);
|
||||
expect(noProgress[0].level).toBe("warning");
|
||||
const recovery = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes("dead-end recovery"));
|
||||
expect(recovery.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,12 +6,14 @@ import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import type { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions";
|
||||
import { ExtensionRuntime, loadExtensionFromFactory } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/loader";
|
||||
import { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/runner";
|
||||
import type { GoalModeState } from "@oh-my-pi/pi-coding-agent/goals/state";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { type } from "arktype";
|
||||
|
||||
@@ -265,6 +267,43 @@ describe("AgentSession mid-run threshold compaction", () => {
|
||||
expect(persistedToolTurnRoles).toEqual(["assistant", "toolResult"]);
|
||||
});
|
||||
|
||||
it("treats same-key assistant content variants as persisted before mid-run compaction", async () => {
|
||||
const extensionRuntime = new ExtensionRuntime();
|
||||
const extension = await loadExtensionFromFactory(
|
||||
pi => {
|
||||
pi.on("message_end", event => {
|
||||
if (event.message.role !== "assistant" || event.message.stopReason !== "toolUse") return;
|
||||
const [block] = event.message.content;
|
||||
if (block?.type !== "toolCall") return;
|
||||
event.message.content = [{ ...block, arguments: { cmd: "display-variant" } }];
|
||||
});
|
||||
},
|
||||
tempDir.path(),
|
||||
new EventBus(),
|
||||
extensionRuntime,
|
||||
"assistant-display-variant",
|
||||
);
|
||||
const extensionAuthStorage = await AuthStorage.create(path.join(tempDir.path(), "extension-auth-variant.db"));
|
||||
cleanups.push(async () => {
|
||||
extensionAuthStorage.close();
|
||||
});
|
||||
const extensionRunner = new ExtensionRunner(
|
||||
[extension],
|
||||
extensionRuntime,
|
||||
tempDir.path(),
|
||||
SessionManager.inMemory(),
|
||||
new ModelRegistry(extensionAuthStorage, path.join(tempDir.path(), "extension-models-variant.yml")),
|
||||
);
|
||||
const { session, observedContexts } = await createHarness({}, { extensionRunner });
|
||||
const compactSpy = mockCompaction("MID-RUN-COMPACTED-WITH-CONTENT-VARIANT");
|
||||
|
||||
await session.prompt("work on the release");
|
||||
|
||||
expect(compactSpy).toHaveBeenCalledTimes(1);
|
||||
expect(observedContexts.length).toBeGreaterThanOrEqual(2);
|
||||
expect(observedContexts[1].join("\n")).toContain("MID-RUN-COMPACTED-WITH-CONTENT-VARIANT");
|
||||
});
|
||||
|
||||
it("does not compact mid-run outside goal mode when disabled", async () => {
|
||||
const { session } = await createHarness({ "compaction.midTurnEnabled": false });
|
||||
const compactSpy = mockCompaction("SHOULD-NOT-RUN");
|
||||
|
||||
@@ -17,6 +17,7 @@ import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
import * as snapcompact from "@oh-my-pi/snapcompact";
|
||||
|
||||
const HANDOFF_SECRET = "HANDOFF_SECRET_TOKEN_12345";
|
||||
const UNRENDERABLE_SNAPCOMPACT_TEXT = "\uE000\uE001\uE002\uE003\uE004\uE005\uE006\uE007\uE008\uE009";
|
||||
|
||||
describe("AgentSession handoff", () => {
|
||||
// Immutable across the whole file: the model registry's synchronous bundled-model
|
||||
@@ -451,7 +452,11 @@ describe("AgentSession handoff", () => {
|
||||
const fixedPreparation: compactionModule.CompactionPreparation = {
|
||||
firstKeptEntryId: lastEntryId,
|
||||
messagesToSummarize: [
|
||||
{ role: "user", content: [{ type: "text", text: "中文内容".repeat(100) }], timestamp: 1 },
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "text", text: UNRENDERABLE_SNAPCOMPACT_TEXT.repeat(100) }],
|
||||
timestamp: 1,
|
||||
},
|
||||
],
|
||||
turnPrefixMessages: [],
|
||||
recentMessages: [],
|
||||
@@ -478,7 +483,11 @@ describe("AgentSession handoff", () => {
|
||||
const fixedPreparation: compactionModule.CompactionPreparation = {
|
||||
firstKeptEntryId: lastEntryId,
|
||||
messagesToSummarize: [
|
||||
{ role: "user", content: [{ type: "text", text: "中文内容".repeat(100) }], timestamp: 1 },
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "text", text: UNRENDERABLE_SNAPCOMPACT_TEXT.repeat(100) }],
|
||||
timestamp: 1,
|
||||
},
|
||||
],
|
||||
turnPrefixMessages: [],
|
||||
recentMessages: [],
|
||||
|
||||
@@ -774,7 +774,7 @@ describe("AgentSession MCP discovery", () => {
|
||||
settings: Settings.isolated({
|
||||
"mcp.discoveryMode": true,
|
||||
defaultThinkingLevel: "high",
|
||||
serviceTier: "priority",
|
||||
"tier.openai": "priority",
|
||||
}),
|
||||
modelRegistry: {} as never,
|
||||
toolRegistry,
|
||||
@@ -789,10 +789,10 @@ describe("AgentSession MCP discovery", () => {
|
||||
|
||||
expect(session.getSelectedMCPToolNames()).toEqual(["mcp__docs_search"]);
|
||||
sessionManager.appendThinkingLevelChange(ThinkingLevel.High);
|
||||
sessionManager.appendServiceTierChange("flex");
|
||||
sessionManager.appendServiceTierChange({ openai: "flex" });
|
||||
sessionManager.appendMCPToolSelection(["mcp__docs_search"]);
|
||||
expect(sessionManager.buildSessionContext().thinkingLevel).toBe(ThinkingLevel.High);
|
||||
expect(sessionManager.buildSessionContext().serviceTier).toBe("flex");
|
||||
expect(sessionManager.buildSessionContext().serviceTier).toEqual({ openai: "flex" });
|
||||
expect(sessionManager.buildSessionContext().selectedMCPToolNames).toEqual(["mcp__docs_search"]);
|
||||
expect(sessionManager.buildSessionContext().hasPersistedMCPToolSelection).toBe(true);
|
||||
await sessionManager.rewriteEntries();
|
||||
@@ -803,7 +803,7 @@ describe("AgentSession MCP discovery", () => {
|
||||
await session.switchSession(olderSessionFile!);
|
||||
expect(session.sessionFile).toBe(olderSessionFile);
|
||||
expect(session.thinkingLevel).toBe(ThinkingLevel.Medium);
|
||||
expect(session.serviceTier).toBe("priority");
|
||||
expect(session.serviceTierByFamily).toEqual({ openai: "priority" });
|
||||
expect(session.getSelectedMCPToolNames()).toEqual([]);
|
||||
expect(session.getActiveToolNames()).toEqual(["read"]);
|
||||
expect(session.systemPrompt).toEqual(["tools:read"]);
|
||||
@@ -813,7 +813,7 @@ describe("AgentSession MCP discovery", () => {
|
||||
await session.switchSession(originalSessionFile!);
|
||||
expect(session.sessionFile).toBe(originalSessionFile);
|
||||
expect(session.thinkingLevel).toBe(ThinkingLevel.Medium);
|
||||
expect(session.serviceTier).toBe("flex");
|
||||
expect(session.serviceTierByFamily).toEqual({ openai: "flex" });
|
||||
expect(session.getSelectedMCPToolNames()).toEqual(["mcp__docs_search"]);
|
||||
expect(session.getActiveToolNames()).toEqual(["read", "mcp__docs_search"]);
|
||||
expect(session.systemPrompt).toEqual(["tools:read,mcp__docs_search"]);
|
||||
|
||||
@@ -4,6 +4,7 @@ import { scheduler } from "node:timers/promises";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import type { ApiKeyResolveContext, AssistantMessage, ToolCall } from "@oh-my-pi/pi-ai";
|
||||
import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock";
|
||||
import * as aiStream from "@oh-my-pi/pi-ai/stream";
|
||||
import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
@@ -54,6 +55,9 @@ describe("AgentSession retry delay cap", () => {
|
||||
beforeEach(async () => {
|
||||
tempDir = TempDir.createSync("@pi-retry-cap-");
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
// A live env var now overrides a stored static api_key; these tests rotate stored Anthropic
|
||||
// credentials, so neutralize env resolution (ignores every provider's ambient env key).
|
||||
vi.spyOn(aiStream, "getEnvApiKey").mockReturnValue(undefined);
|
||||
authStorage.setRuntimeApiKey("anthropic", "anthropic-test-key");
|
||||
modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
|
||||
});
|
||||
|
||||
@@ -11,6 +11,8 @@ import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
const UNRENDERABLE_SNAPCOMPACT_TEXT = "\uE000\uE001\uE002\uE003\uE004\uE005\uE006\uE007\uE008\uE009";
|
||||
|
||||
interface Harness {
|
||||
session: AgentSession;
|
||||
sessionManager: SessionManager;
|
||||
@@ -47,7 +49,7 @@ async function createHarness(tempDir: TempDir, authStorage: AuthStorage, options
|
||||
const settings = Settings.isolated({
|
||||
"compaction.strategy": "snapcompact",
|
||||
// Force a 1-token recent window so the post-turn cut always splits off the
|
||||
// last turn and summarizes the seeded (CJK) history. With the default
|
||||
// last turn and summarizes the seeded unrenderable history. With the default
|
||||
// 20k window the cut keeps both tiny messages, leaving nothing for
|
||||
// snapcompact's renderability preflight to scan.
|
||||
"compaction.keepRecentTokens": 1,
|
||||
@@ -164,7 +166,7 @@ describe("AgentSession auto-snapcompact local-blocker fallback", () => {
|
||||
seedMessages: [
|
||||
{
|
||||
role: "user",
|
||||
content: "你好,请帮我审查这段代码。它的逻辑似乎有问题,我无法理解为何返回空结果。",
|
||||
content: UNRENDERABLE_SNAPCOMPACT_TEXT.repeat(10),
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
],
|
||||
|
||||
@@ -163,6 +163,7 @@ describe("AgentSession snapcompact frame-budget sizing", () => {
|
||||
const maxFrames = opts?.maxFrames;
|
||||
expect(maxFrames).toBeDefined();
|
||||
expect(maxFrames).toBeLessThan(snapcompact.MAX_FRAMES_DEFAULT);
|
||||
expect(maxFrames).toBeLessThanOrEqual(snapcompact.maxFramesForDataBudget());
|
||||
expect(maxFrames).toBeGreaterThan(0);
|
||||
|
||||
// Verify the FULL projection — base (non-message + kept-recent) +
|
||||
@@ -241,4 +242,41 @@ describe("AgentSession snapcompact frame-budget sizing", () => {
|
||||
// text-only `planArchive` path makes this case recoverable.
|
||||
expect(opts?.maxFrames).toBe(1);
|
||||
});
|
||||
|
||||
it("applies the frame byte cap when the model context window is unknown", async () => {
|
||||
const model = session.model;
|
||||
if (!model) throw new Error("Expected model");
|
||||
await session.dispose();
|
||||
const unknownWindowModel = { ...model, contextWindow: 0 };
|
||||
session = new AgentSession({
|
||||
agent: new Agent({
|
||||
initialState: { model: unknownWindowModel, systemPrompt: ["Test"], tools: [], messages: [] },
|
||||
}),
|
||||
sessionManager,
|
||||
settings: Settings.isolated({
|
||||
"compaction.strategy": "snapcompact",
|
||||
"compaction.autoContinue": false,
|
||||
"compaction.keepRecentTokens": 4000,
|
||||
}),
|
||||
modelRegistry,
|
||||
});
|
||||
|
||||
const branchEntries = sessionManager.getBranch();
|
||||
const lastEntry = branchEntries[branchEntries.length - 1];
|
||||
if (!lastEntry?.id) throw new Error("Expected branch entry with id");
|
||||
const compactSpy = vi.spyOn(snapcompact, "compact").mockResolvedValue({
|
||||
summary: "stubbed snapcompact",
|
||||
shortSummary: "stub",
|
||||
firstKeptEntryId: lastEntry.id,
|
||||
tokensBefore: 100_000,
|
||||
details: { readFiles: [], modifiedFiles: [] },
|
||||
preserveData: {
|
||||
snapcompact: { frames: [], totalChars: 0, truncatedChars: 0 },
|
||||
},
|
||||
});
|
||||
|
||||
await session.compact(undefined, { mode: "snapcompact" });
|
||||
|
||||
expect(compactSpy.mock.calls[0]?.[1]?.maxFrames).toBe(snapcompact.maxFramesForDataBudget());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import type { Model } from "@oh-my-pi/pi-ai";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import type { BuildSessionContextOptions, SessionContext } from "@oh-my-pi/pi-coding-agent/session/session-context";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/**
|
||||
* Regression for issue #3846: in-TUI `/resume` rebuilt the *previous*
|
||||
* session's display context before switching files. That call expands persisted
|
||||
* snapcompact archives and `openaiRemoteCompaction.replacementHistory` payloads
|
||||
* into messages, which can OOM on huge pre-fix sessions even though the loader
|
||||
* itself streams. The previous context is only needed for same-session reloads
|
||||
* (where `#didSessionMessagesChange` compares against the freshly rebuilt one);
|
||||
* different-session switches MUST skip that work.
|
||||
*/
|
||||
describe("AgentSession.switchSession previous-context build", () => {
|
||||
let sharedDir: TempDir;
|
||||
let authStorage: AuthStorage;
|
||||
let modelRegistry: ModelRegistry;
|
||||
let model: Model;
|
||||
const tempDirs: TempDir[] = [];
|
||||
const sessions: AgentSession[] = [];
|
||||
|
||||
beforeAll(async () => {
|
||||
sharedDir = TempDir.createSync("@pi-switch-prev-ctx-shared-");
|
||||
authStorage = await AuthStorage.create(path.join(sharedDir.path(), "testauth.db"));
|
||||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||||
modelRegistry = new ModelRegistry(authStorage);
|
||||
const bundled = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||||
if (!bundled) throw new Error("Expected built-in anthropic model to exist");
|
||||
model = bundled;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
authStorage.close();
|
||||
try {
|
||||
await sharedDir.remove();
|
||||
} catch {}
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
while (sessions.length > 0) {
|
||||
await sessions.pop()?.dispose();
|
||||
}
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
try {
|
||||
await dir.remove();
|
||||
} catch {}
|
||||
}
|
||||
});
|
||||
|
||||
function buildSession(tempDir: TempDir): { session: AgentSession; sessionManager: SessionManager } {
|
||||
const sessionManager = SessionManager.create(tempDir.path(), tempDir.path());
|
||||
const agent = new Agent({
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
});
|
||||
const session = new AgentSession({
|
||||
agent,
|
||||
sessionManager,
|
||||
settings: Settings.isolated({ "compaction.enabled": false }),
|
||||
modelRegistry,
|
||||
});
|
||||
sessions.push(session);
|
||||
return { session, sessionManager };
|
||||
}
|
||||
|
||||
/** Wrap `sessionManager.buildSessionContext` so each call's caller-visible
|
||||
* state (the manager's currently-loaded session file) is recorded in
|
||||
* invocation order. The constructor itself calls `buildSessionContext`
|
||||
* once; spying *after* construction means only switchSession-driven calls
|
||||
* are observed. */
|
||||
function instrumentBuildSessionContext(sessionManager: SessionManager): {
|
||||
calls: Array<{ sessionFile: string | undefined; transcript: boolean | undefined }>;
|
||||
restore: () => void;
|
||||
} {
|
||||
const calls: Array<{ sessionFile: string | undefined; transcript: boolean | undefined }> = [];
|
||||
const original = sessionManager.buildSessionContext.bind(sessionManager);
|
||||
const patched = (options?: BuildSessionContextOptions): SessionContext => {
|
||||
calls.push({ sessionFile: sessionManager.getSessionFile(), transcript: options?.transcript });
|
||||
return original(options);
|
||||
};
|
||||
sessionManager.buildSessionContext = patched as SessionManager["buildSessionContext"];
|
||||
return {
|
||||
calls,
|
||||
restore: () => {
|
||||
sessionManager.buildSessionContext = original;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("skips building the previous display context when switching to a different session", async () => {
|
||||
const tempDir = TempDir.createSync("@pi-switch-prev-ctx-different-");
|
||||
tempDirs.push(tempDir);
|
||||
|
||||
const { session, sessionManager } = buildSession(tempDir);
|
||||
sessionManager.appendMessage({ role: "user", content: "previous", timestamp: 1 });
|
||||
await sessionManager.flush();
|
||||
const previousSessionFile = sessionManager.getSessionFile();
|
||||
expect(previousSessionFile).toBeString();
|
||||
|
||||
const otherManager = SessionManager.create(tempDir.path(), tempDir.path());
|
||||
otherManager.appendMessage({ role: "user", content: "target", timestamp: 2 });
|
||||
await otherManager.flush();
|
||||
const targetSessionFile = otherManager.getSessionFile();
|
||||
expect(targetSessionFile).toBeString();
|
||||
expect(targetSessionFile).not.toBe(previousSessionFile);
|
||||
await otherManager.close();
|
||||
|
||||
const { calls, restore } = instrumentBuildSessionContext(sessionManager);
|
||||
try {
|
||||
const switched = await session.switchSession(targetSessionFile!);
|
||||
expect(switched).toBe(true);
|
||||
expect(session.sessionFile).toBe(targetSessionFile);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
|
||||
// The previous session's display context MUST NOT be materialized. Only
|
||||
// the new target context (post-`setSessionFile`) should be built.
|
||||
expect(calls).toEqual([{ sessionFile: targetSessionFile!, transcript: undefined }]);
|
||||
});
|
||||
|
||||
it("builds the previous display context for same-session reloads", async () => {
|
||||
const tempDir = TempDir.createSync("@pi-switch-prev-ctx-reload-");
|
||||
tempDirs.push(tempDir);
|
||||
|
||||
const { session, sessionManager } = buildSession(tempDir);
|
||||
sessionManager.appendMessage({ role: "user", content: "current", timestamp: 1 });
|
||||
await sessionManager.flush();
|
||||
const sessionFile = sessionManager.getSessionFile();
|
||||
expect(sessionFile).toBeString();
|
||||
|
||||
const { calls, restore } = instrumentBuildSessionContext(sessionManager);
|
||||
try {
|
||||
const switched = await session.switchSession(sessionFile!);
|
||||
expect(switched).toBe(true);
|
||||
expect(session.sessionFile).toBe(sessionFile);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
|
||||
// Same-session reload must snapshot the pre-reload context so
|
||||
// `#didSessionMessagesChange` can detect rollback edits.
|
||||
expect(calls).toEqual([
|
||||
{ sessionFile: sessionFile!, transcript: undefined },
|
||||
{ sessionFile: sessionFile!, transcript: undefined },
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -19,6 +19,7 @@ import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentSession, type AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { type CustomMessage, convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
@@ -241,4 +242,136 @@ describe("AgentSession thinking-loop retry", () => {
|
||||
expect(assistants).toHaveLength(1);
|
||||
expect(assistants[0].content).toEqual([{ type: "text", text: "Recovered after retry." }]);
|
||||
});
|
||||
|
||||
it("injects a redirect notice into the retried turn after a thinking loop", async () => {
|
||||
const model = createMockModel({ provider: "openrouter", id: "google/gemini-3.5-flash" }).model;
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
const calls: string[] = [];
|
||||
const contexts: Context[] = [];
|
||||
const agent = new Agent({
|
||||
getApiKey: requestedModel => `${requestedModel.provider}-test-key`,
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
convertToLlm,
|
||||
streamFn: (requestedModel, context, _options?: SimpleStreamOptions) => {
|
||||
calls.push(`${requestedModel.provider}/${requestedModel.id}`);
|
||||
contexts.push(context);
|
||||
return calls.length === 1 ? errorIdOnlyThinkingLoopStream(requestedModel) : successStream(requestedModel);
|
||||
},
|
||||
});
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": false,
|
||||
"retry.enabled": true,
|
||||
"retry.baseDelayMs": 0,
|
||||
"retry.maxDelayMs": 5_000,
|
||||
"retry.maxRetries": 1,
|
||||
"retry.modelFallback": false,
|
||||
"todo.enabled": false,
|
||||
"model.loopGuard.enabled": true,
|
||||
});
|
||||
settings.setModelRole("default", `${model.provider}/${model.id}`);
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings,
|
||||
modelRegistry,
|
||||
});
|
||||
vi.spyOn(scheduler, "wait").mockResolvedValue(undefined);
|
||||
|
||||
await session.prompt("Trigger redirect injection after thinking loop");
|
||||
await session.waitForIdle();
|
||||
const retryContext = contexts[1];
|
||||
const extractText = (content: string | Array<{ type: string; text?: string }>): string =>
|
||||
typeof content === "string"
|
||||
? content
|
||||
: content.map(part => (part.type === "text" ? (part.text ?? "") : "")).join("");
|
||||
const redirectDevMsgs = retryContext.messages.filter(
|
||||
message => message.role === "developer" && extractText(message.content).includes("thinking_loop_detected"),
|
||||
);
|
||||
expect(redirectDevMsgs).toHaveLength(1);
|
||||
|
||||
const redirects = session.agent.state.messages.filter(
|
||||
(message): message is CustomMessage =>
|
||||
message.role === "custom" && message.customType === "thinking-loop-redirect",
|
||||
);
|
||||
expect(redirects).toHaveLength(1);
|
||||
expect(redirects[0].display).toBe(false);
|
||||
expect(redirects[0].attribution).toBe("agent");
|
||||
expect(typeof redirects[0].content).toBe("string");
|
||||
expect(redirects[0].content).toContain("thinking_loop_detected");
|
||||
|
||||
const assistants = session.agent.state.messages.filter(
|
||||
(message): message is AssistantMessage => message.role === "assistant",
|
||||
);
|
||||
expect(assistants).toHaveLength(1);
|
||||
expect(assistants[0].content).toEqual([{ type: "text", text: "Recovered after retry." }]);
|
||||
});
|
||||
|
||||
it("injects a redirect notice on each consecutive thinking-loop retry", async () => {
|
||||
const model = createMockModel({ provider: "openrouter", id: "google/gemini-3.5-flash" }).model;
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
const calls: string[] = [];
|
||||
const contexts: Context[] = [];
|
||||
const agent = new Agent({
|
||||
getApiKey: requestedModel => `${requestedModel.provider}-test-key`,
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
convertToLlm,
|
||||
streamFn: (requestedModel, context, _options?: SimpleStreamOptions) => {
|
||||
calls.push(`${requestedModel.provider}/${requestedModel.id}`);
|
||||
contexts.push(context);
|
||||
return calls.length <= 2 ? errorIdOnlyThinkingLoopStream(requestedModel) : successStream(requestedModel);
|
||||
},
|
||||
});
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": false,
|
||||
"retry.enabled": true,
|
||||
"retry.baseDelayMs": 0,
|
||||
"retry.maxDelayMs": 5_000,
|
||||
"retry.maxRetries": 2,
|
||||
"retry.modelFallback": false,
|
||||
"todo.enabled": false,
|
||||
"model.loopGuard.enabled": true,
|
||||
});
|
||||
settings.setModelRole("default", `${model.provider}/${model.id}`);
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings,
|
||||
modelRegistry,
|
||||
});
|
||||
vi.spyOn(scheduler, "wait").mockResolvedValue(undefined);
|
||||
|
||||
await session.prompt("Trigger redirect injection after two thinking loops");
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(calls).toHaveLength(3);
|
||||
const redirects = session.agent.state.messages.filter(
|
||||
(message): message is CustomMessage =>
|
||||
message.role === "custom" && message.customType === "thinking-loop-redirect",
|
||||
);
|
||||
expect(redirects).toHaveLength(2);
|
||||
const extractText = (content: string | Array<{ type: string; text?: string }>): string =>
|
||||
typeof content === "string"
|
||||
? content
|
||||
: content.map(part => (part.type === "text" ? (part.text ?? "") : "")).join("");
|
||||
const thirdAttemptRedirectDevMsgs = contexts[2].messages.filter(
|
||||
message => message.role === "developer" && extractText(message.content).includes("thinking_loop_detected"),
|
||||
);
|
||||
expect(thirdAttemptRedirectDevMsgs).toHaveLength(2);
|
||||
|
||||
const assistants = session.agent.state.messages.filter(
|
||||
(message): message is AssistantMessage => message.role === "assistant",
|
||||
);
|
||||
expect(assistants).toHaveLength(1);
|
||||
expect(assistants[0].content).toEqual([{ type: "text", text: "Recovered after retry." }]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -173,13 +173,16 @@ describe("bench empty-output guard", () => {
|
||||
|
||||
function settingsStub(serviceTier: string | undefined): Settings | undefined {
|
||||
if (serviceTier === undefined) return undefined;
|
||||
return { get: (key: string) => (key === "serviceTier" ? serviceTier : undefined) } as unknown as Settings;
|
||||
return {
|
||||
get: (key: string) =>
|
||||
key === "tier.openai" ? serviceTier : key === "tier.anthropic" || key === "tier.google" ? "none" : undefined,
|
||||
} as unknown as Settings;
|
||||
}
|
||||
|
||||
async function captureServiceTier(opts: {
|
||||
flag?: string;
|
||||
setting?: string;
|
||||
}): Promise<{ wire: SimpleStreamOptions["serviceTier"]; summary: BenchSummary["serviceTier"] }> {
|
||||
}): Promise<{ wire: SimpleStreamOptions["serviceTier"]; summary: BenchSummary["serviceTierByFamily"] }> {
|
||||
const registry = fakeRegistry({ models: [fakeModel("openai-codex", "gpt-5.5")], authedProviders: ["openai-codex"] });
|
||||
let captured: SimpleStreamOptions | undefined;
|
||||
const summary = await runBenchCommand(
|
||||
@@ -205,7 +208,7 @@ async function captureServiceTier(opts: {
|
||||
stdoutIsTTY: false,
|
||||
},
|
||||
);
|
||||
return { wire: captured?.serviceTier, summary: summary.serviceTier };
|
||||
return { wire: captured?.serviceTier, summary: summary.serviceTierByFamily };
|
||||
}
|
||||
|
||||
describe("bench provider session state and websocket preference", () => {
|
||||
@@ -245,24 +248,24 @@ describe("bench service tier", () => {
|
||||
it("sends the configured serviceTier setting when no flag is passed", async () => {
|
||||
const { wire, summary } = await captureServiceTier({ setting: "flex" });
|
||||
expect(wire).toBe("flex");
|
||||
expect(summary).toBe("flex");
|
||||
expect(summary).toEqual({ openai: "flex" });
|
||||
});
|
||||
|
||||
it("lets an explicit --service-tier override the configured setting", async () => {
|
||||
const { wire, summary } = await captureServiceTier({ flag: "priority", setting: "flex" });
|
||||
expect(wire).toBe("priority");
|
||||
expect(summary).toBe("priority");
|
||||
expect(summary).toEqual({ openai: "priority", anthropic: "priority", google: "priority" });
|
||||
});
|
||||
|
||||
it("omits service_tier when the setting is none and no flag is passed", async () => {
|
||||
const { wire, summary } = await captureServiceTier({ setting: "none" });
|
||||
expect(wire).toBeUndefined();
|
||||
expect(summary).toBeUndefined();
|
||||
expect(summary).toEqual({});
|
||||
});
|
||||
|
||||
it("omits service_tier when neither flag nor settings are present", async () => {
|
||||
const { wire, summary } = await captureServiceTier({});
|
||||
expect(wire).toBeUndefined();
|
||||
expect(summary).toBeUndefined();
|
||||
expect(summary).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -111,6 +111,56 @@ describe("hashline executor", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves UTF-8 BOM bytes when hashline edits decoded text", async () => {
|
||||
await withTempDir(async tempDir => {
|
||||
const filePath = path.join(tempDir, "Program.cs");
|
||||
const source = "using A;\n";
|
||||
await Bun.write(filePath, new Uint8Array([0xef, 0xbb, 0xbf, ...new TextEncoder().encode(source)]));
|
||||
const session = makeHashlineSession(tempDir);
|
||||
const sourceTag = recordFullSnapshot(getFileReadCache(session), filePath, source);
|
||||
const input = `${header("Program.cs", sourceTag)}\n${sameLineRange(tag(1, source))}\n${repl("using B;")}\n`;
|
||||
|
||||
await executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session));
|
||||
|
||||
const bytes = await fs.readFile(filePath);
|
||||
expect(Array.from(bytes.subarray(0, 3))).toEqual([0xef, 0xbb, 0xbf]);
|
||||
expect(new TextDecoder().decode(bytes.subarray(3))).toBe("using B;\n");
|
||||
});
|
||||
});
|
||||
|
||||
it("edits BOM-prefixed notebooks through the virtual cell text", async () => {
|
||||
await withTempDir(async tempDir => {
|
||||
const filePath = path.join(tempDir, "notebook.ipynb");
|
||||
const notebook = {
|
||||
cells: [
|
||||
{
|
||||
cell_type: "markdown",
|
||||
metadata: { keep: true },
|
||||
source: ["# Title\n"],
|
||||
},
|
||||
],
|
||||
metadata: {},
|
||||
nbformat: 4,
|
||||
nbformat_minor: 5,
|
||||
};
|
||||
await Bun.write(
|
||||
filePath,
|
||||
new Uint8Array([0xef, 0xbb, 0xbf, ...new TextEncoder().encode(JSON.stringify(notebook))]),
|
||||
);
|
||||
const session = makeHashlineSession(tempDir);
|
||||
const editableText = "# %% [markdown] cell:0\n# Title\n";
|
||||
const sourceTag = recordFullSnapshot(getFileReadCache(session), filePath, editableText);
|
||||
const input = `${header("notebook.ipynb", sourceTag)}\n${sameLineRange(tag(2, "# Title"))}\n${repl("# Updated")}\n`;
|
||||
|
||||
await executeHashlineSingle(hashlineExecuteOptions(tempDir, input, undefined, session));
|
||||
|
||||
const updated = await Bun.file(filePath).json();
|
||||
expect(updated.cells).toHaveLength(1);
|
||||
expect(updated.cells[0].source).toEqual(["# Updated\n"]);
|
||||
expect(updated.cells[0].metadata).toEqual({ keep: true });
|
||||
});
|
||||
});
|
||||
|
||||
it("emits an actionable no-op diagnostic when the payload matches the file byte-for-byte", async () => {
|
||||
await withTempDir(async tempDir => {
|
||||
const filePath = path.join(tempDir, "a.ts");
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import { CustomEditor } from "@oh-my-pi/pi-coding-agent/modes/components/custom-editor";
|
||||
import { getEditorTheme, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import { StdinBuffer } from "@oh-my-pi/pi-tui/stdin-buffer";
|
||||
|
||||
/**
|
||||
* Regression for #3857.
|
||||
*
|
||||
* A fast double-Esc lands as one `"\x1b\x1b"` chunk on stdin. Before the fix,
|
||||
* `StdinBuffer` held it as the buffered remainder, then timer-flushed it as
|
||||
* one sequence. `parseKey("\x1b\x1b")` returns `undefined`, so
|
||||
* `CustomEditor.handleInput` fell through to the base editor and never fired
|
||||
* the configured `onEscape` — breaking the double-escape gesture.
|
||||
*
|
||||
* The fix splits a bare `"\x1b\x1b"` into two ESC events only when no follower
|
||||
* arrives in the disambiguation window. If a follower arrives, the second ESC
|
||||
* remains attached to that follower so legacy Alt chords survive.
|
||||
*/
|
||||
describe("buffered double-Esc reaches CustomEditor.onEscape", () => {
|
||||
beforeAll(async () => {
|
||||
await initTheme();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("fires onEscape twice when a fast double-Esc arrives as one buffered chunk", () => {
|
||||
const editor = new CustomEditor(getEditorTheme());
|
||||
const onEscape = vi.fn();
|
||||
editor.onEscape = onEscape;
|
||||
|
||||
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
|
||||
buf.on("data", chunk => editor.handleInput(chunk));
|
||||
|
||||
buf.process("\x1b\x1b");
|
||||
// Drain the flush timer chain (main timeout + zero-delay deferral).
|
||||
vi.runAllTimers();
|
||||
|
||||
expect(onEscape).toHaveBeenCalledTimes(2);
|
||||
buf.destroy();
|
||||
});
|
||||
|
||||
it("preserves a legacy Alt chord batched after a bare ESC", () => {
|
||||
const editor = new CustomEditor(getEditorTheme());
|
||||
const onEscape = vi.fn();
|
||||
editor.onEscape = onEscape;
|
||||
editor.setText("foo bar");
|
||||
|
||||
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
|
||||
buf.on("data", chunk => editor.handleInput(chunk));
|
||||
|
||||
buf.process("\x1b\x1b\x7f");
|
||||
vi.runAllTimers();
|
||||
|
||||
expect(onEscape).toHaveBeenCalledTimes(1);
|
||||
expect(editor.getText()).toBe("foo ");
|
||||
buf.destroy();
|
||||
});
|
||||
|
||||
it("does not split a meta-CSI arrow into two ESC events", () => {
|
||||
const editor = new CustomEditor(getEditorTheme());
|
||||
const onEscape = vi.fn();
|
||||
editor.onEscape = onEscape;
|
||||
|
||||
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
|
||||
buf.on("data", chunk => editor.handleInput(chunk));
|
||||
|
||||
buf.process("\x1b\x1b[A");
|
||||
vi.runAllTimers();
|
||||
|
||||
// alt+up is its own keypress and must never look like two ESC keys.
|
||||
expect(onEscape).not.toHaveBeenCalled();
|
||||
buf.destroy();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,24 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { startCpuProfile } from "@oh-my-pi/pi-coding-agent/debug/profiler";
|
||||
|
||||
describe("startCpuProfile", () => {
|
||||
// Regression: `node:v8` `setFlagsFromString` throws on Bun
|
||||
// (oven-sh/bun#1702). The profiler used to call it unconditionally and
|
||||
// crash before connecting the inspector session. Running this test under
|
||||
// Bun guarantees the guard is in place — without it the call below would
|
||||
// reject with "node:v8 setFlagsFromString is not yet implemented in Bun".
|
||||
it("starts and stops successfully even when v8.setFlagsFromString is unavailable", async () => {
|
||||
const session = await startCpuProfile();
|
||||
// Run a tiny bit of work so the profile has at least one sample.
|
||||
let acc = 0;
|
||||
for (let i = 0; i < 10_000; i++) acc += i;
|
||||
expect(acc).toBeGreaterThan(0);
|
||||
|
||||
const profile = await session.stop();
|
||||
const parsed = JSON.parse(profile.data) as { nodes: unknown[] };
|
||||
expect(Array.isArray(parsed.nodes)).toBe(true);
|
||||
expect(parsed.nodes.length).toBeGreaterThan(0);
|
||||
expect(typeof profile.markdown).toBe("string");
|
||||
expect(profile.markdown.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
@@ -146,6 +146,83 @@ describe("builtin-defaults rule provider", () => {
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
it("go-new-expr matches value→pointer helpers (named + generic) but not real functions, only on *.go", async () => {
|
||||
const rules = await loadBuiltinRules();
|
||||
const rule = rules.find(r => r.name === "go-new-expr");
|
||||
if (!rule) throw new Error("go-new-expr rule missing");
|
||||
const manager = new TtsrManager();
|
||||
expect(manager.addRule(rule)).toBe(true);
|
||||
const ctx: TtsrMatchContext = { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] };
|
||||
|
||||
const hits = [
|
||||
"package p\nfunc boolPtr(v bool) *bool { return &v }",
|
||||
"package p\nfunc Ptr[T any](v T) *T { return &v }",
|
||||
];
|
||||
for (const snippet of hits) {
|
||||
manager.resetBuffer();
|
||||
expect(
|
||||
(await manager.checkAstSnapshot(snippet, ctx)).map(m => m.name),
|
||||
snippet,
|
||||
).toEqual(["go-new-expr"]);
|
||||
}
|
||||
|
||||
const misses = [
|
||||
"package p\nfunc add(a int, b int) *int { return &a }",
|
||||
"package p\nfunc (s *S) Get() *int { return &s.x }",
|
||||
];
|
||||
for (const snippet of misses) {
|
||||
manager.resetBuffer();
|
||||
expect(await manager.checkAstSnapshot(snippet, ctx), snippet).toEqual([]);
|
||||
}
|
||||
|
||||
// AST conditions never reach a non-go path.
|
||||
manager.resetBuffer();
|
||||
expect(
|
||||
await manager.checkAstSnapshot(hits[0], { source: "tool", toolName: "edit", filePaths: ["pkg/foo.ts"] }),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("go-bench-loop fires on a *testing.B b.N loop but not an ordinary .N counter", async () => {
|
||||
const rules = await loadBuiltinRules();
|
||||
const rule = rules.find(r => r.name === "go-bench-loop");
|
||||
if (!rule) throw new Error("go-bench-loop rule missing");
|
||||
const manager = new TtsrManager();
|
||||
expect(manager.addRule(rule)).toBe(true);
|
||||
const ctx: TtsrMatchContext = { source: "tool", toolName: "edit", filePaths: ["pkg/foo_test.go"] };
|
||||
|
||||
const bench =
|
||||
"package p\nfunc BenchmarkX(b *testing.B) {\n\tsetup()\n\tfor i := 0; i < b.N; i++ {\n\t\twork()\n\t}\n}";
|
||||
manager.resetBuffer();
|
||||
expect((await manager.checkAstSnapshot(bench, ctx)).map(m => m.name)).toEqual(["go-bench-loop"]);
|
||||
|
||||
// A `.N` selector on something that is not the benchmark receiver must not fire.
|
||||
const helper =
|
||||
"package p\nfunc TestThing(t *testing.T) {\n\treq := build()\n\tfor i := 0; i < req.N; i++ {\n\t\twork()\n\t}\n}";
|
||||
manager.resetBuffer();
|
||||
expect(await manager.checkAstSnapshot(helper, ctx)).toEqual([]);
|
||||
});
|
||||
|
||||
it("go-range-int fires only on *.go, never on a same-named non-go path", async () => {
|
||||
const rules = await loadBuiltinRules();
|
||||
const rule = rules.find(r => r.name === "go-range-int");
|
||||
if (!rule) throw new Error("go-range-int rule missing");
|
||||
const manager = new TtsrManager();
|
||||
expect(manager.addRule(rule)).toBe(true);
|
||||
|
||||
const loop = "package p\nfunc f(n int) {\n\tfor i := 0; i < n; i++ {\n\t\tuse(i)\n\t}\n}";
|
||||
manager.resetBuffer();
|
||||
expect(
|
||||
(await manager.checkAstSnapshot(loop, { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] })).map(
|
||||
m => m.name,
|
||||
),
|
||||
).toEqual(["go-range-int"]);
|
||||
// A step-2 loop is not equivalent to range-over-int and must not fire.
|
||||
const step2 = "package p\nfunc f(n int) {\n\tfor i := 0; i < n; i += 2 {\n\t\tuse(i)\n\t}\n}";
|
||||
manager.resetBuffer();
|
||||
expect(
|
||||
await manager.checkAstSnapshot(step2, { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] }),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("is the lowest-priority rule provider so user/project rules override defaults", () => {
|
||||
const { cap, provider } = ruleProvider();
|
||||
|
||||
@@ -299,6 +299,47 @@ describe("EventController working loader reconciliation", () => {
|
||||
expect(ctx.ensureLoadingAnimation).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("self-heals missing working loader when a task subagent finishes mid-turn (#3858)", async () => {
|
||||
// `task` subagents run inside the parent's streaming turn. While the task is
|
||||
// running a transient overlay (auto-compaction / auto-retry) can drop the
|
||||
// working loader by clearing the status container, and the overlay's end
|
||||
// handler is the only restorer keyed off the missing loader. If the task
|
||||
// finishes between the overlay's start and end (or any other branch where
|
||||
// the loader was nulled without a follow-up overlay-end), `tool_execution_end`
|
||||
// is the next streaming event that lands and must heal the loader, mirroring
|
||||
// the `tool_execution_update` reconciler. Without this the spinner stays
|
||||
// gone for the remainder of the parent turn even though the agent keeps
|
||||
// streaming (the user-visible regression in #3858).
|
||||
const { controller, ctx } = createFixture();
|
||||
(ctx.viewSession as unknown as { isStreaming: boolean }).isStreaming = true;
|
||||
|
||||
await controller.handleEvent({
|
||||
type: "tool_execution_end",
|
||||
toolCallId: "task-1",
|
||||
toolName: "task",
|
||||
isError: false,
|
||||
result: { content: [{ type: "text", text: "ok" }], details: {} },
|
||||
} as Extract<AgentSessionEvent, { type: "tool_execution_end" }>);
|
||||
|
||||
expect(ctx.ensureLoadingAnimation).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not restore the working loader while an overlay loader (auto-retry) owns the status container at tool_execution_end", async () => {
|
||||
const { controller, ctx } = createFixture();
|
||||
ctx.retryLoader = { stop: vi.fn() } as unknown as InteractiveModeContext["retryLoader"];
|
||||
(ctx.viewSession as unknown as { isStreaming: boolean }).isStreaming = true;
|
||||
|
||||
await controller.handleEvent({
|
||||
type: "tool_execution_end",
|
||||
toolCallId: "task-2",
|
||||
toolName: "task",
|
||||
isError: false,
|
||||
result: { content: [{ type: "text", text: "ok" }], details: {} },
|
||||
} as Extract<AgentSessionEvent, { type: "tool_execution_end" }>);
|
||||
|
||||
expect(ctx.ensureLoadingAnimation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps transient retry status exclusive while a retry loader is visible", async () => {
|
||||
const { controller, ctx } = createFixture();
|
||||
ctx.retryLoader = { stop: vi.fn() } as unknown as InteractiveModeContext["retryLoader"];
|
||||
|
||||
@@ -21,6 +21,11 @@ function createContext() {
|
||||
updateEditorTopBorder: vi.fn(),
|
||||
clearPinnedError: vi.fn(),
|
||||
ensureLoadingAnimation: vi.fn(),
|
||||
// `viewSession.isStreaming` is read by `#ensureWorkingLoaderWhileStreaming`,
|
||||
// which runs at the top of `tool_execution_end` (and other streaming-event
|
||||
// handlers). Leaving it false matches the implicit assumption in this
|
||||
// fixture: the todo HUD lifecycle is independent of the working loader.
|
||||
viewSession: { isStreaming: false },
|
||||
todoReminderContainer,
|
||||
setTodos: vi.fn(),
|
||||
present,
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
/**
|
||||
* Regression guard for issue #3827.
|
||||
*
|
||||
* `/mcp list` and the `/extensions` dashboard MUST agree on whether a given MCP
|
||||
* server is enabled or disabled. The two read paths historically diverged: the
|
||||
* dashboard's `loadAllExtensions` only consulted the dashboard-private
|
||||
* `disabledExtensions` settings array, while `/mcp list` (and the MCP runtime
|
||||
* itself) honored both the per-server `enabled` flag in `mcp.json` and the
|
||||
* user-level `disabledServers` denylist.
|
||||
*
|
||||
* The fixtures below cover both inputs and the round-trip helper the
|
||||
* dashboard's MCP toggle uses.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { initializeWithSettings, reset as resetDiscoveryCache } from "@oh-my-pi/pi-coding-agent/discovery";
|
||||
import { readMCPConfigFile, setMcpServerEnabled, setServerDisabled } from "@oh-my-pi/pi-coding-agent/mcp/config-writer";
|
||||
import { loadAllExtensions } from "@oh-my-pi/pi-coding-agent/modes/components/extensions/state-manager";
|
||||
import { __resetDirsFromEnvForTests, getMCPConfigPath, removeWithRetries, setAgentDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
describe("loadAllExtensions MCP parity with /mcp list (issue #3827)", () => {
|
||||
let projectDir = "";
|
||||
let userAgentDir = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
resetSettingsForTest();
|
||||
projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-3827-project-"));
|
||||
userAgentDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-3827-user-"));
|
||||
|
||||
// Redirect user-scoped mcp.json (resolved via getAgentDir() at the call
|
||||
// site) into the per-test temp directory so neither the discovery loader
|
||||
// nor the denylist reader touches the real user profile.
|
||||
setAgentDir(userAgentDir);
|
||||
|
||||
await fs.mkdir(path.join(projectDir, ".omp"), { recursive: true });
|
||||
await fs.writeFile(
|
||||
path.join(projectDir, ".omp", "mcp.json"),
|
||||
JSON.stringify({
|
||||
mcpServers: {
|
||||
"denylisted-server": { command: "echo", args: ["denylisted"] },
|
||||
"flag-disabled-server": { command: "echo", args: ["flag"], enabled: false },
|
||||
"active-server": { command: "echo", args: ["active"] },
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
// User-level mcp.json carries the denylist; this is what `/mcp disable`
|
||||
// writes through setServerDisabled().
|
||||
await fs.writeFile(
|
||||
path.join(userAgentDir, "mcp.json"),
|
||||
JSON.stringify({
|
||||
mcpServers: {},
|
||||
disabledServers: ["denylisted-server"],
|
||||
}),
|
||||
);
|
||||
|
||||
const settings = await Settings.init({ inMemory: true, cwd: projectDir });
|
||||
initializeWithSettings(settings);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
resetSettingsForTest();
|
||||
__resetDirsFromEnvForTests();
|
||||
await removeWithRetries(projectDir);
|
||||
await removeWithRetries(userAgentDir);
|
||||
});
|
||||
|
||||
test("treats a server in user-level disabledServers as disabled (matches /mcp list)", async () => {
|
||||
const extensions = await loadAllExtensions(projectDir, []);
|
||||
const denylisted = extensions.find(e => e.id === "mcp:denylisted-server");
|
||||
expect(denylisted).toBeDefined();
|
||||
expect(denylisted!.state).toBe("disabled");
|
||||
expect(denylisted!.disabledReason).toBe("item-disabled");
|
||||
});
|
||||
|
||||
test("treats a server with enabled:false as disabled (matches /mcp list)", async () => {
|
||||
const extensions = await loadAllExtensions(projectDir, []);
|
||||
const flagDisabled = extensions.find(e => e.id === "mcp:flag-disabled-server");
|
||||
expect(flagDisabled).toBeDefined();
|
||||
expect(flagDisabled!.state).toBe("disabled");
|
||||
expect(flagDisabled!.disabledReason).toBe("item-disabled");
|
||||
});
|
||||
|
||||
test("leaves untouched servers active", async () => {
|
||||
const extensions = await loadAllExtensions(projectDir, []);
|
||||
const active = extensions.find(e => e.id === "mcp:active-server");
|
||||
expect(active).toBeDefined();
|
||||
expect(active!.state).toBe("active");
|
||||
expect(active!.disabledReason).toBeUndefined();
|
||||
});
|
||||
|
||||
test("setServerDisabled round-trips through the dashboard view", async () => {
|
||||
// Re-enable `denylisted-server` through the canonical writer the
|
||||
// dashboard's MCP toggle now calls. The dashboard view MUST flip to
|
||||
// active on the next load.
|
||||
await setServerDisabled(getMCPConfigPath("user", projectDir), "denylisted-server", false);
|
||||
const reenabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:denylisted-server");
|
||||
expect(reenabled).toBeDefined();
|
||||
expect(reenabled!.state).toBe("active");
|
||||
|
||||
// The inverse path: disabling `active-server` via the writer flips the
|
||||
// dashboard view to disabled.
|
||||
await setServerDisabled(getMCPConfigPath("user", projectDir), "active-server", true);
|
||||
const disabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:active-server");
|
||||
expect(disabled).toBeDefined();
|
||||
expect(disabled!.state).toBe("disabled");
|
||||
expect(disabled!.disabledReason).toBe("item-disabled");
|
||||
});
|
||||
|
||||
test("dashboard re-enable flips enabled:false in mcp.json (PR #3829 review)", async () => {
|
||||
// The bug: when a server has `enabled: false` in mcp.json, the dashboard
|
||||
// toggle previously only removed it from the user-level denylist, so
|
||||
// state-manager's `server.enabled === false` check kept it disabled.
|
||||
// setMcpServerEnabled MUST overwrite the per-server flag.
|
||||
const projectMcpPath = path.join(projectDir, ".omp", "mcp.json");
|
||||
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "flag-disabled-server",
|
||||
enabled: true,
|
||||
});
|
||||
|
||||
const projectConfig = await readMCPConfigFile(projectMcpPath);
|
||||
expect(projectConfig.mcpServers?.["flag-disabled-server"]?.enabled).toBe(true);
|
||||
|
||||
const reenabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:flag-disabled-server");
|
||||
expect(reenabled).toBeDefined();
|
||||
expect(reenabled!.state).toBe("active");
|
||||
});
|
||||
|
||||
test("dashboard re-enable also clears a stale denylist entry on a config-resident server", async () => {
|
||||
// Manually disable `active-server` via BOTH the per-server flag and the
|
||||
// denylist, simulating a server that's been toggled off multiple ways.
|
||||
const projectMcpPath = path.join(projectDir, ".omp", "mcp.json");
|
||||
const initial = await readMCPConfigFile(projectMcpPath);
|
||||
await Bun.write(
|
||||
projectMcpPath,
|
||||
JSON.stringify({
|
||||
...initial,
|
||||
mcpServers: {
|
||||
...initial.mcpServers,
|
||||
"active-server": { ...initial.mcpServers!["active-server"], enabled: false },
|
||||
},
|
||||
}),
|
||||
);
|
||||
await setServerDisabled(getMCPConfigPath("user", projectDir), "active-server", true);
|
||||
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "active-server",
|
||||
enabled: true,
|
||||
});
|
||||
|
||||
const userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.disabledServers ?? []).not.toContain("active-server");
|
||||
|
||||
const reenabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:active-server");
|
||||
expect(reenabled).toBeDefined();
|
||||
expect(reenabled!.state).toBe("active");
|
||||
});
|
||||
|
||||
test("dashboard disable on a config-resident server writes enabled:false (not denylist)", async () => {
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "active-server",
|
||||
enabled: false,
|
||||
});
|
||||
|
||||
const projectConfig = await readMCPConfigFile(path.join(projectDir, ".omp", "mcp.json"));
|
||||
expect(projectConfig.mcpServers?.["active-server"]?.enabled).toBe(false);
|
||||
|
||||
// The denylist is reserved for discovered (config-less) servers; a
|
||||
// config-resident server's `enabled: false` flag is the canonical signal.
|
||||
const userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.disabledServers ?? []).not.toContain("active-server");
|
||||
|
||||
const disabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:active-server");
|
||||
expect(disabled).toBeDefined();
|
||||
expect(disabled!.state).toBe("disabled");
|
||||
});
|
||||
|
||||
test("dashboard re-enable updates the row's non-primary source mcp.json before denylisting", async () => {
|
||||
const alternatePath = path.join(projectDir, ".omp", ".mcp.json");
|
||||
await Bun.write(
|
||||
alternatePath,
|
||||
JSON.stringify({
|
||||
mcpServers: {
|
||||
"alternate-server": { command: "echo", args: ["alternate"], enabled: false },
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const disabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:alternate-server");
|
||||
expect(disabled).toBeDefined();
|
||||
expect(disabled!.state).toBe("disabled");
|
||||
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
sourcePath: alternatePath,
|
||||
name: "alternate-server",
|
||||
enabled: true,
|
||||
});
|
||||
|
||||
const alternateConfig = await readMCPConfigFile(alternatePath);
|
||||
expect(alternateConfig.mcpServers?.["alternate-server"]?.enabled).toBe(true);
|
||||
|
||||
const userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.disabledServers ?? []).not.toContain("alternate-server");
|
||||
|
||||
const reenabled = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:alternate-server");
|
||||
expect(reenabled).toBeDefined();
|
||||
expect(reenabled!.state).toBe("active");
|
||||
});
|
||||
test("dashboard re-enable force-enables a tool-owned source (opencode.json) via enabledServers", async () => {
|
||||
// OpenCode is a non-writable source: the dashboard must NOT mutate
|
||||
// opencode.json, but the user-level enabledServers allowlist still has
|
||||
// to flip the row active. Modeled after the codex review on PR #3829.
|
||||
const opencodePath = path.join(projectDir, "opencode.json");
|
||||
await Bun.write(
|
||||
opencodePath,
|
||||
JSON.stringify({
|
||||
mcp: {
|
||||
"opencode-server": {
|
||||
type: "local",
|
||||
command: ["echo", "opencode"],
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
// beforeEach's Settings.init() already cached an absent opencode.json
|
||||
// for this projectDir, so drop the capability fs cache before the first
|
||||
// dashboard load picks the file up.
|
||||
resetDiscoveryCache();
|
||||
|
||||
const before = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:opencode-server");
|
||||
expect(before).toBeDefined();
|
||||
expect(before!.source.provider).toBe("opencode");
|
||||
expect(before!.state).toBe("disabled");
|
||||
|
||||
// The dashboard withholds sourcePath for tool-owned sources, mirroring
|
||||
// the #writableMcpSourcePath gate.
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "opencode-server",
|
||||
enabled: true,
|
||||
});
|
||||
|
||||
// opencode.json MUST stay untouched.
|
||||
const opencodeRaw = JSON.parse(await Bun.file(opencodePath).text()) as {
|
||||
mcp: { "opencode-server": { enabled: boolean } };
|
||||
};
|
||||
expect(opencodeRaw.mcp["opencode-server"].enabled).toBe(false);
|
||||
|
||||
// The override lands in the user mcp.json's enabledServers list.
|
||||
const userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.enabledServers ?? []).toContain("opencode-server");
|
||||
|
||||
const after = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:opencode-server");
|
||||
expect(after).toBeDefined();
|
||||
expect(after!.state).toBe("active");
|
||||
|
||||
// Disabling again clears the override.
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "opencode-server",
|
||||
enabled: false,
|
||||
});
|
||||
|
||||
const userConfigAfter = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfigAfter.enabledServers ?? []).not.toContain("opencode-server");
|
||||
expect(userConfigAfter.disabledServers ?? []).toContain("opencode-server");
|
||||
|
||||
const offAgain = (await loadAllExtensions(projectDir, [])).find(e => e.id === "mcp:opencode-server");
|
||||
expect(offAgain).toBeDefined();
|
||||
expect(offAgain!.state).toBe("disabled");
|
||||
});
|
||||
|
||||
test("dashboard toggles on a discovered (config-less) server use the denylist", async () => {
|
||||
// `phantom-server` is not in any config; only the denylist can suppress it.
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "phantom-server",
|
||||
enabled: false,
|
||||
});
|
||||
|
||||
let userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.disabledServers ?? []).toContain("phantom-server");
|
||||
|
||||
await setMcpServerEnabled({
|
||||
userPath: getMCPConfigPath("user", projectDir),
|
||||
projectPath: getMCPConfigPath("project", projectDir),
|
||||
name: "phantom-server",
|
||||
enabled: true,
|
||||
});
|
||||
|
||||
userConfig = await readMCPConfigFile(getMCPConfigPath("user", projectDir));
|
||||
expect(userConfig.disabledServers ?? []).not.toContain("phantom-server");
|
||||
});
|
||||
});
|
||||
@@ -9,9 +9,7 @@ import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
type FastModeScope = "both" | "openai" | "claude";
|
||||
|
||||
describe("fast mode scope", () => {
|
||||
describe("/fast targets the current model's service-tier family", () => {
|
||||
let tempDir: TempDir;
|
||||
let authStorage: AuthStorage;
|
||||
let session: AgentSession;
|
||||
@@ -29,77 +27,54 @@ describe("fast mode scope", () => {
|
||||
tempDir.removeSync();
|
||||
});
|
||||
|
||||
async function createSession(fastModeScope?: FastModeScope): Promise<AgentSession> {
|
||||
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||||
async function createSession(provider: "anthropic" | "openai", modelId: string): Promise<AgentSession> {
|
||||
const model = getBundledModel(provider, modelId);
|
||||
if (!model) {
|
||||
throw new Error("Expected bundled test model to exist");
|
||||
throw new Error(`Expected bundled test model ${provider}/${modelId} to exist`);
|
||||
}
|
||||
|
||||
const settings = fastModeScope === undefined ? Settings.isolated() : Settings.isolated({ fastModeScope });
|
||||
const agent = new Agent({
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
initialState: { model, systemPrompt: ["Test"], tools: [], messages: [] },
|
||||
});
|
||||
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
authStorage.setRuntimeApiKey(model.provider, "anthropic-token");
|
||||
authStorage.setRuntimeApiKey(model.provider, "token");
|
||||
modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
|
||||
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings,
|
||||
settings: Settings.isolated(),
|
||||
modelRegistry,
|
||||
});
|
||||
session.subscribe(() => {});
|
||||
return session;
|
||||
}
|
||||
|
||||
it("scopes enabled fast mode to OpenAI when configured", async () => {
|
||||
const session = await createSession("openai");
|
||||
|
||||
it("enables priority on the Anthropic family for a Claude model", async () => {
|
||||
const session = await createSession("anthropic", "claude-sonnet-4-5");
|
||||
session.setFastMode(true);
|
||||
|
||||
expect(session.serviceTier).toBe("openai-only");
|
||||
expect(session.serviceTierByFamily).toEqual({ anthropic: "priority" });
|
||||
expect(session.isFastModeEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
it("scopes enabled fast mode to Claude when configured", async () => {
|
||||
const session = await createSession("claude");
|
||||
|
||||
it("enables priority on the OpenAI family for an OpenAI model", async () => {
|
||||
const session = await createSession("openai", "gpt-5.2");
|
||||
session.setFastMode(true);
|
||||
|
||||
expect(session.serviceTier).toBe("claude-only");
|
||||
expect(session.serviceTierByFamily).toEqual({ openai: "priority" });
|
||||
expect(session.isFastModeEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
it("defaults enabled fast mode to priority for both providers", async () => {
|
||||
const session = await createSession();
|
||||
|
||||
it("clears only the current model's family when disabled", async () => {
|
||||
const session = await createSession("anthropic", "claude-sonnet-4-5");
|
||||
session.setFastMode(true);
|
||||
|
||||
expect(session.serviceTier).toBe("priority");
|
||||
});
|
||||
|
||||
it("clears the service tier when disabled", async () => {
|
||||
const session = await createSession("openai");
|
||||
session.setFastMode(true);
|
||||
|
||||
session.setFastMode(false);
|
||||
|
||||
expect(session.serviceTier).toBeUndefined();
|
||||
expect(session.serviceTierByFamily).toEqual({});
|
||||
expect(session.isFastModeEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
it("does not broaden an already enabled scoped tier", async () => {
|
||||
const session = await createSession("claude");
|
||||
session.setFastMode(true);
|
||||
expect(session.serviceTier).toBe("claude-only");
|
||||
session.settings.set("fastModeScope", "both");
|
||||
|
||||
session.setFastMode(true);
|
||||
|
||||
expect(session.serviceTier).toBe("claude-only");
|
||||
it("toggle reports the resulting state", async () => {
|
||||
const session = await createSession("anthropic", "claude-sonnet-4-5");
|
||||
expect(session.toggleFastMode()).toBe(true);
|
||||
expect(session.serviceTierByFamily.anthropic).toBe("priority");
|
||||
expect(session.toggleFastMode()).toBe(false);
|
||||
expect(session.serviceTierByFamily.anthropic).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -98,4 +98,26 @@ describe("generateFileMentionMessages path resolution", () => {
|
||||
expect(message.files).toHaveLength(1);
|
||||
expect(message.files[0]?.path).toBe("My Folder/my file.png");
|
||||
});
|
||||
|
||||
test("skips auto-reading a binary file instead of injecting raw bytes", async () => {
|
||||
const cwd = await createTempDir();
|
||||
// TTF header begins with a NUL run; auto-reading it as text would leak
|
||||
// control bytes into the conversation (the reported bug).
|
||||
await Bun.write(path.join(cwd, "Silver.ttf"), Buffer.from([0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x4f, 0x53]));
|
||||
// A non-NUL invalid-UTF8 blob must be refused too, not just NUL-bearing files.
|
||||
await Bun.write(path.join(cwd, "blob.bin"), Buffer.from([0x4d, 0x5a, 0xff, 0xfe, 0xc0, 0xc0]));
|
||||
|
||||
const messages = await generateFileMentionMessages(["Silver.ttf", "blob.bin"], cwd);
|
||||
expect(messages).toHaveLength(1);
|
||||
const message = messages[0];
|
||||
if (message?.role !== "fileMention") {
|
||||
throw new Error("expected file mention message");
|
||||
}
|
||||
expect(message.files).toHaveLength(2);
|
||||
for (const file of message.files) {
|
||||
expect(file.skippedReason).toBe("binary");
|
||||
expect(file.content).toContain("binary file");
|
||||
expect(file.content).not.toContain("\u0000");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,6 +17,7 @@ import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SILENT_ABORT_MARKER, USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { AUTO_THINKING } from "@oh-my-pi/pi-coding-agent/thinking";
|
||||
import { setKeybindings } from "@oh-my-pi/pi-tui";
|
||||
import { formatNumber, TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
@@ -836,6 +837,46 @@ describe("InteractiveMode plan review rendering", () => {
|
||||
expect(defaultApply?.[0]?.explicitThinkingLevel).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves DEFAULT(auto) when plan approval restores the default tier", async () => {
|
||||
const sonnet = session.modelRegistry.find("anthropic", "claude-sonnet-4-5");
|
||||
const opus = session.modelRegistry.find("anthropic", "claude-opus-4-5");
|
||||
if (!sonnet || !opus) throw new Error("Expected sonnet + opus to exist in registry");
|
||||
|
||||
session.settings.setModelRole("default", "anthropic/claude-sonnet-4-5");
|
||||
session.settings.setModelRole("slow", "anthropic/claude-opus-4-5");
|
||||
session.settings.setModelRole("plan", "anthropic/claude-opus-4-5");
|
||||
session.setThinkingLevel(AUTO_THINKING, true);
|
||||
|
||||
const planFilePath = "local://PLAN.md";
|
||||
const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, {
|
||||
getArtifactsDir: () => session.sessionManager.getArtifactsDir(),
|
||||
getSessionId: () => session.sessionManager.getSessionId(),
|
||||
});
|
||||
await Bun.write(resolvedPlanPath, "# Plan\n\nPreserve the configured auto selector.");
|
||||
|
||||
await mode.handlePlanModeCommand();
|
||||
expect(session.model?.id).toBe(opus.id);
|
||||
|
||||
vi.spyOn(session, "getContextUsage").mockReturnValue(undefined);
|
||||
vi.spyOn(session, "prompt").mockResolvedValue(undefined as never);
|
||||
|
||||
vi.spyOn(mode, "showPlanReview").mockImplementation(
|
||||
async (_planContent, _title, _options, _dialogOptions, extra?: { slider?: HookSelectorSlider }) => {
|
||||
const slider = extra?.slider;
|
||||
expect(slider).toBeDefined();
|
||||
const defaultIndex = slider!.segments.findIndex(segment => segment.label === "default");
|
||||
expect(defaultIndex).toBeGreaterThanOrEqual(0);
|
||||
slider!.onChange?.(defaultIndex);
|
||||
return "Approve and keep context";
|
||||
},
|
||||
);
|
||||
|
||||
await mode.handlePlanApproval({ planFilePath, planExists: true, title: "PLAN" });
|
||||
|
||||
expect(session.model?.id).toBe(sonnet.id);
|
||||
expect(session.configuredThinkingLevel()).toBe(AUTO_THINKING);
|
||||
});
|
||||
|
||||
it("falls back to the pre-plan model when only plan is configured and the slider is hidden", async () => {
|
||||
const sonnet = session.modelRegistry.find("anthropic", "claude-sonnet-4-5");
|
||||
const opus = session.modelRegistry.find("anthropic", "claude-opus-4-5");
|
||||
|
||||
@@ -171,10 +171,30 @@ describe("issue #3291 — tiny-model downloads keep the worker referenced", () =
|
||||
|
||||
worker.emit({ type: "downloaded", id: downloadRequestId });
|
||||
|
||||
expect(await download).toBe(true);
|
||||
expect(await download).toEqual({ ok: true });
|
||||
expect(worker.unrefCalls).toBe(1);
|
||||
} finally {
|
||||
await client.terminate();
|
||||
}
|
||||
});
|
||||
|
||||
it("returns the worker error for failed download requests", async () => {
|
||||
let downloadRequestId = "";
|
||||
const worker = new FakeTinyWorker(message => {
|
||||
if (message.type === "download") downloadRequestId = message.id;
|
||||
});
|
||||
const client = new TinyTitleClient(() => worker);
|
||||
|
||||
try {
|
||||
const download = client.downloadModel("lfm2-700m");
|
||||
|
||||
expect(downloadRequestId).not.toBe("");
|
||||
worker.emit({ type: "error", id: downloadRequestId, error: "Error: runtime install failed" });
|
||||
|
||||
expect(await download).toEqual({ ok: false, error: "Error: runtime install failed" });
|
||||
expect(worker.terminated).toBe(true);
|
||||
} finally {
|
||||
await client.terminate();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,7 +48,7 @@ describe("job renderer task-result preview", () => {
|
||||
|
||||
it("previews the envelope body, not the wrapper markup", () => {
|
||||
const summary = prompt.render(taskSummaryTemplate, {
|
||||
agentName: "quick_task",
|
||||
agentName: "sonic",
|
||||
id: "SpawnProbe",
|
||||
status: "completed",
|
||||
duration: "8.7s",
|
||||
@@ -83,7 +83,7 @@ describe("job renderer task-result preview", () => {
|
||||
|
||||
it("flattens a pretty-printed JSON body instead of previewing a lone brace", () => {
|
||||
const summary = prompt.render(taskSummaryTemplate, {
|
||||
agentName: "quick_task",
|
||||
agentName: "sonic",
|
||||
id: "EchoAlpha",
|
||||
status: "completed",
|
||||
duration: "11.6s",
|
||||
|
||||
@@ -43,6 +43,9 @@ function restoreEnvValue(name: string, value: string | undefined): void {
|
||||
}
|
||||
function createController(authStorage: AuthStorage, mcpManagerOverrides: Record<string, unknown> = {}) {
|
||||
const showError = vi.fn();
|
||||
const showStatus = vi.fn();
|
||||
const present = vi.fn();
|
||||
const editor: { onEscape?: () => void } = {};
|
||||
const prepareConfig = vi.fn(async (config: MCPServerConfig) => config);
|
||||
const mcpManager = {
|
||||
prepareConfig,
|
||||
@@ -55,11 +58,11 @@ function createController(authStorage: AuthStorage, mcpManagerOverrides: Record<
|
||||
};
|
||||
const controller = new MCPCommandController({
|
||||
chatContainer: { addChild: vi.fn() },
|
||||
present: vi.fn(),
|
||||
present,
|
||||
ui: { requestRender: vi.fn() },
|
||||
editor: {},
|
||||
editor,
|
||||
showError,
|
||||
showStatus: vi.fn(),
|
||||
showStatus,
|
||||
oauthManualInput: {
|
||||
hasPending: vi.fn(() => false),
|
||||
pendingProviderId: undefined,
|
||||
@@ -72,7 +75,7 @@ function createController(authStorage: AuthStorage, mcpManagerOverrides: Record<
|
||||
mcpManager,
|
||||
} as never);
|
||||
|
||||
return { controller, showError, prepareConfig, mcpManager };
|
||||
return { controller, showError, showStatus, present, editor, prepareConfig, mcpManager };
|
||||
}
|
||||
|
||||
describe("/mcp auth commands", () => {
|
||||
@@ -219,6 +222,109 @@ describe("/mcp auth commands", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("Esc aborts the OAuth flow during /mcp reauth", async () => {
|
||||
const authStorage = freshAuthStorage();
|
||||
await authStorage.reload();
|
||||
vi.spyOn(mcpClient, "connectToServer").mockRejectedValue(AUTH_ERROR);
|
||||
|
||||
// Simulate the real flow: login hangs waiting for the OAuth callback and
|
||||
// only resolves when the controller's signal aborts. Mirrors what
|
||||
// OAuthCallbackFlow.#waitForCallback does in production.
|
||||
vi.spyOn(oauthFlow.MCPOAuthFlow.prototype, "login").mockImplementation(function (this: oauthFlow.MCPOAuthFlow) {
|
||||
const pending = Promise.withResolvers<never>();
|
||||
this.ctrl.signal?.addEventListener("abort", () => {
|
||||
pending.reject(new Error(`OAuth callback cancelled: ${String(this.ctrl.signal?.reason ?? "aborted")}`));
|
||||
});
|
||||
return pending.promise;
|
||||
});
|
||||
|
||||
const { controller, showError, showStatus, editor } = createController(authStorage);
|
||||
|
||||
const reauthPromise = controller.handle("/mcp reauth envserver");
|
||||
|
||||
// Wait for #handleOAuthFlow to install its editor.onEscape hook.
|
||||
const deadline = Date.now() + 1_000;
|
||||
while (typeof editor.onEscape !== "function" && Date.now() < deadline) {
|
||||
await Bun.sleep(10);
|
||||
}
|
||||
expect(typeof editor.onEscape).toBe("function");
|
||||
|
||||
const installedEscape = editor.onEscape;
|
||||
editor.onEscape?.();
|
||||
|
||||
// Cancellation must resolve the reauth promise promptly (well under the
|
||||
// 5-minute production timeout); a 2s race exposes a hung flow as a test
|
||||
// failure rather than a suite hang.
|
||||
await Promise.race([
|
||||
reauthPromise,
|
||||
Bun.sleep(2_000).then(() => {
|
||||
throw new Error("reauth did not resolve within 2s of Esc");
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(showError).not.toHaveBeenCalled();
|
||||
expect(showStatus).toHaveBeenCalledWith(expect.stringMatching(/cancel/i));
|
||||
// onEscape must be restored to its previous value so subsequent user
|
||||
// input does not keep aborting the (now-finished) flow.
|
||||
expect(editor.onEscape).not.toBe(installedEscape);
|
||||
});
|
||||
|
||||
test("Esc cancels even when OAuth login has not registered its signal listener yet", async () => {
|
||||
const authStorage = freshAuthStorage();
|
||||
await authStorage.reload();
|
||||
vi.spyOn(mcpClient, "connectToServer").mockRejectedValue(AUTH_ERROR);
|
||||
|
||||
// Simulates the review race: Esc aborts oauthTimeout before
|
||||
// OAuthCallbackFlow.#waitForCallback has registered its abort listener
|
||||
// (e.g. during dynamic client registration or metadata discovery).
|
||||
// The login promise itself never observes ctrl.signal; #handleOAuthFlow
|
||||
// must race it against oauthTimeout.signal.
|
||||
vi.spyOn(oauthFlow.MCPOAuthFlow.prototype, "login").mockReturnValue(Promise.withResolvers<never>().promise);
|
||||
const { controller, showError, showStatus, editor } = createController(authStorage);
|
||||
|
||||
const reauthPromise = controller.handle("/mcp reauth envserver");
|
||||
const deadline = Date.now() + 1_000;
|
||||
while (typeof editor.onEscape !== "function" && Date.now() < deadline) {
|
||||
await Bun.sleep(10);
|
||||
}
|
||||
expect(typeof editor.onEscape).toBe("function");
|
||||
editor.onEscape?.();
|
||||
|
||||
await Promise.race([
|
||||
reauthPromise,
|
||||
Bun.sleep(2_000).then(() => {
|
||||
throw new Error("reauth did not resolve within 2s of pre-wait Esc");
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(showError).not.toHaveBeenCalled();
|
||||
expect(showStatus).toHaveBeenCalledWith(expect.stringMatching(/cancel/i));
|
||||
});
|
||||
|
||||
test("OAuth deadline still surfaces as a reauthorization error, not a cancellation", async () => {
|
||||
const authStorage = freshAuthStorage();
|
||||
await authStorage.reload();
|
||||
vi.spyOn(mcpClient, "connectToServer").mockRejectedValue(AUTH_ERROR);
|
||||
|
||||
// Deadline path bypasses both the editor's Esc hook and any external
|
||||
// signal: withTimeout aborts the controller with reason "MCP OAuth flow
|
||||
// timed out" and the login promise rejects with a "timed out" message.
|
||||
// Mirror that here. Keeping the surface distinct from the user-cancel
|
||||
// flag in #handleOAuthFlow is the whole point of this regression test.
|
||||
vi.spyOn(oauthFlow.MCPOAuthFlow.prototype, "login").mockRejectedValue(
|
||||
new Error("OAuth flow timed out after 5 minutes"),
|
||||
);
|
||||
const { controller, showError, showStatus } = createController(authStorage);
|
||||
|
||||
await controller.handle("/mcp reauth envserver");
|
||||
|
||||
// Deadline must read as "failed", not "cancelled" — they have different
|
||||
// surfaces (error banner vs status line) and the user expects a clear
|
||||
// timeout message rather than thinking they pressed Esc.
|
||||
expect(showStatus).not.toHaveBeenCalledWith(expect.stringMatching(/cancel/i));
|
||||
expect(showError).toHaveBeenCalledWith(expect.stringMatching(/timed out/i));
|
||||
});
|
||||
|
||||
test("clears both expanded and stale raw URL-keyed credentials on unauth", async () => {
|
||||
const authStorage = freshAuthStorage();
|
||||
await authStorage.reload();
|
||||
|
||||
@@ -7,7 +7,7 @@ import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-regis
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { ModelSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/model-selector";
|
||||
import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import type { ConfiguredThinkingLevel } from "@oh-my-pi/pi-coding-agent/thinking";
|
||||
import { AUTO_THINKING, type ConfiguredThinkingLevel } from "@oh-my-pi/pi-coding-agent/thinking";
|
||||
import type { TUI } from "@oh-my-pi/pi-tui";
|
||||
|
||||
function normalizeRenderedText(text: string): string {
|
||||
@@ -156,6 +156,26 @@ describe("ModelSelector role badge thinking display", () => {
|
||||
expect(rendered).not.toContain("low medium high max");
|
||||
});
|
||||
|
||||
test("reloads DEFAULT(auto) from defaultThinkingLevel", async () => {
|
||||
installTestTheme();
|
||||
const model = getBundledModel("openai", "gpt-5.5");
|
||||
if (!model) throw new Error("Expected bundled model openai/gpt-5.5");
|
||||
|
||||
const settings = Settings.isolated({
|
||||
defaultThinkingLevel: AUTO_THINKING,
|
||||
modelRoles: {
|
||||
default: `${model.provider}/${model.id}`,
|
||||
},
|
||||
});
|
||||
|
||||
const selector = createSelector(model, settings);
|
||||
await Bun.sleep(0);
|
||||
installTestTheme();
|
||||
|
||||
const rendered = normalizeRenderedText(selector.render(220).join("\n"));
|
||||
expect(rendered).toContain("DEFAULT (auto)");
|
||||
});
|
||||
|
||||
test("shows compact auto badges for unconfigured role defaults", async () => {
|
||||
installTestTheme();
|
||||
const settings = Settings.isolated({});
|
||||
|
||||
@@ -91,26 +91,45 @@ describe("EventController paces streamed tool args", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("reveals partialJson prefixes per frame, then snaps to final args when the JSON closes", async () => {
|
||||
it("reveals the initial slice immediately, then paces growth across message_updates", async () => {
|
||||
await Settings.init({ inMemory: true, cwd: process.cwd() });
|
||||
vi.useFakeTimers();
|
||||
const updateArgsSpy = vi.spyOn(ToolExecutionComponent.prototype, "updateArgs");
|
||||
const content = "x".repeat(400);
|
||||
const target = `{"path":"/tmp/a.ts","content":"${content}"}`;
|
||||
const streaming = makeStreamingMessage([
|
||||
// Seed includes the complete `path` field (closing quote at byte 20) plus
|
||||
// the opening of `content`, so the rendered preview must show the real
|
||||
// path on the very first dispatch.
|
||||
const seed = target.slice(0, 35);
|
||||
|
||||
// First message_update: only a small slice has arrived. The reveal
|
||||
// MUST surface it as-is (no empty initial frame).
|
||||
const seedStreaming = makeStreamingMessage([
|
||||
{ type: "toolCall", id: "tc-1", name: "write", arguments: {}, [kStreamingPartialJson]: seed },
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(seedStreaming);
|
||||
await dispatch(controller, seedStreaming);
|
||||
expect(pendingTools.size).toBe(1);
|
||||
|
||||
// Component constructor consumes the initial render args directly; no
|
||||
// updateArgs has been invoked yet, but the seeded prefix is already on
|
||||
// the pending preview.
|
||||
const componentRender = pendingTools.get("tc-1")!.render(80).join("\n");
|
||||
expect(Bun.stripANSI(componentRender)).toContain("/tmp/a.ts");
|
||||
|
||||
// Second message_update: the rest of the payload arrives. The controller
|
||||
// paces the new backlog through reveal ticks.
|
||||
const fullStreaming = makeStreamingMessage([
|
||||
{ type: "toolCall", id: "tc-1", name: "write", arguments: {}, [kStreamingPartialJson]: target },
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(streaming);
|
||||
|
||||
await dispatch(controller, streaming);
|
||||
expect(pendingTools.size).toBe(1);
|
||||
await dispatch(controller, fullStreaming);
|
||||
|
||||
for (let i = 0; i < 3; i++) {
|
||||
vi.advanceTimersByTime(STREAMING_REVEAL_FRAME_MS);
|
||||
}
|
||||
const pacedFrames = updateArgsSpy.mock.calls.map(call => call[0] as Record<string, unknown>);
|
||||
expect(pacedFrames.length).toBeGreaterThan(0);
|
||||
let previousLength = 0;
|
||||
let previousLength = seed.length;
|
||||
for (const frame of pacedFrames) {
|
||||
const prefix = frame.__partialJson;
|
||||
if (typeof prefix !== "string") throw new Error("Expected __partialJson string on paced frame");
|
||||
@@ -171,12 +190,13 @@ describe("EventController paces streamed tool args", () => {
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(streaming);
|
||||
|
||||
// Args still streaming: the reveal seeds the preview at an empty prefix, so
|
||||
// the write head shows its `…` path placeholder rather than the real path.
|
||||
// Args still streaming, but the reveal now seeds the preview with the
|
||||
// full available partialJson on the very first message_update — so the
|
||||
// path is already visible before the tool starts executing.
|
||||
await dispatch(controller, streaming);
|
||||
const component = pendingTools.get("tc-1");
|
||||
if (!component) throw new Error("expected a pending write component");
|
||||
expect(Bun.stripANSI(component.render(80).join("\n"))).not.toContain("/tmp/exec.ts");
|
||||
expect(Bun.stripANSI(component.render(80).join("\n"))).toContain("/tmp/exec.ts");
|
||||
|
||||
// The closing full-args message_update never arrives (throttled `arguments`
|
||||
// with smoothing off, an owned-dialect projector, or a superseded turn that
|
||||
|
||||
+58
-1
@@ -70,7 +70,13 @@ function createContext(options: { terminalProgress?: boolean } = {}) {
|
||||
editor: { getText: () => "" },
|
||||
sessionManager: { getSessionName: () => "test-session" },
|
||||
ui: { requestRender: vi.fn(), requestComponentRender: vi.fn(), terminal: { setProgress } },
|
||||
viewSession: { isCompacting: false, getLastAssistantMessage: () => undefined },
|
||||
viewSession: {
|
||||
isCompacting: false,
|
||||
getLastAssistantMessage: () => undefined,
|
||||
get isStreaming() {
|
||||
return streamState.isStreaming;
|
||||
},
|
||||
},
|
||||
session: {
|
||||
get isStreaming() {
|
||||
return streamState.isStreaming;
|
||||
@@ -109,6 +115,14 @@ const RETRY_START = {
|
||||
delayMs: 1000,
|
||||
errorMessage: "overloaded",
|
||||
} as unknown as AgentSessionEvent;
|
||||
const TASK_TOOL_EXECUTION_END = {
|
||||
type: "tool_execution_end",
|
||||
toolCallId: "call-task-1",
|
||||
toolName: "task",
|
||||
args: {},
|
||||
result: { content: [], details: {} },
|
||||
isError: false,
|
||||
} as unknown as AgentSessionEvent;
|
||||
|
||||
describe("EventController loader recovery after overflow maintenance", () => {
|
||||
beforeAll(async () => {
|
||||
@@ -179,6 +193,49 @@ describe("EventController loader recovery after overflow maintenance", () => {
|
||||
expect(statusContainer.children).toContain(ctx.loadingAnimation);
|
||||
});
|
||||
|
||||
it("re-shows the Working… loader after a subagent task completes while the session keeps streaming", async () => {
|
||||
const { ctx, streamState, statusContainer, workingLoaders } = createContext();
|
||||
const controller = new EventController(ctx);
|
||||
|
||||
// Turn begins: the working loader is created and attached.
|
||||
await controller.handleEvent(AGENT_START);
|
||||
const firstWorking = workingLoaders[0];
|
||||
expect(firstWorking).toBeDefined();
|
||||
|
||||
// A transient overlay (auto-retry / auto-compaction) tore the loader down
|
||||
// mid-tool; the session is still streaming when the subagent's task
|
||||
// completes. Before the fix, `tool_execution_end` (unlike `_update`) did
|
||||
// not re-arm the loader, so the UI looked idle while the agent kept going.
|
||||
streamState.isStreaming = true;
|
||||
ctx.loadingAnimation?.stop();
|
||||
ctx.loadingAnimation = undefined;
|
||||
statusContainer.clear();
|
||||
|
||||
await controller.handleEvent(TASK_TOOL_EXECUTION_END);
|
||||
|
||||
expect(ctx.loadingAnimation).toBeDefined();
|
||||
expect(statusContainer.children).toContain(ctx.loadingAnimation);
|
||||
expect(workingLoaders).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("does not re-arm the Working… loader on tool_execution_end once the session has stopped streaming", async () => {
|
||||
const { ctx, streamState, statusContainer } = createContext();
|
||||
const controller = new EventController(ctx);
|
||||
|
||||
await controller.handleEvent(AGENT_START);
|
||||
ctx.loadingAnimation?.stop();
|
||||
ctx.loadingAnimation = undefined;
|
||||
statusContainer.clear();
|
||||
streamState.isStreaming = false;
|
||||
|
||||
await controller.handleEvent(TASK_TOOL_EXECUTION_END);
|
||||
|
||||
// No streaming → reconciler must stay a no-op; the spinner is not the
|
||||
// post-turn idle state.
|
||||
expect(ctx.loadingAnimation).toBeUndefined();
|
||||
expect(statusContainer.children).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("mirrors agent and auto-compaction activity to OSC 9;4 when enabled", async () => {
|
||||
const { ctx, setProgress } = createContext({ terminalProgress: true });
|
||||
const controller = new EventController(ctx);
|
||||
|
||||
@@ -426,7 +426,7 @@ describe("mcp oauth flow", () => {
|
||||
);
|
||||
|
||||
await expect(flow.login()).rejects.toThrow(
|
||||
"OAuth callback port 80 unavailable; cannot fall back to a random port when oauth.redirectUri is set",
|
||||
"OAuth callback port 80 is in use, but oauth.redirectUri (http://localhost/callback) requires this exact port",
|
||||
);
|
||||
expect(serveSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -447,7 +447,7 @@ describe("mcp oauth flow", () => {
|
||||
);
|
||||
|
||||
await expect(flow.login()).rejects.toThrow(
|
||||
"OAuth callback port 3000 unavailable; cannot fall back to a random port when oauth.redirectUri is set",
|
||||
"OAuth callback port 3000 is in use, but oauth.redirectUri (http://localhost:3000/callback) requires this exact port",
|
||||
);
|
||||
expect(serveSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -468,7 +468,123 @@ describe("mcp oauth flow", () => {
|
||||
{ signal: AbortSignal.timeout(1_000) },
|
||||
);
|
||||
|
||||
await expect(flow.login()).rejects.toThrow("cannot fall back to a random port when oauth.redirectUri is set");
|
||||
await expect(flow.login()).rejects.toThrow(
|
||||
/oauth\.redirectUri \(https:\/\/public\.example\/slack\/oauth_redirect\) requires this exact port/,
|
||||
);
|
||||
});
|
||||
|
||||
it("fails fast when the preferred port is busy and a static clientId pins the registered redirect URI", async () => {
|
||||
const serveSpy = vi.spyOn(Bun, "serve").mockImplementation(options => {
|
||||
expect(options.port).toBe(14572);
|
||||
throw new Error("EADDRINUSE");
|
||||
});
|
||||
|
||||
const progress: string[] = [];
|
||||
const onAuth = vi.fn();
|
||||
const flow = new MCPOAuthFlow(
|
||||
{
|
||||
authorizationUrl: "https://provider.example/authorize",
|
||||
tokenUrl: "https://provider.example/token",
|
||||
clientId: "demo-client",
|
||||
callbackPort: 14572,
|
||||
},
|
||||
{
|
||||
onAuth,
|
||||
onProgress: msg => progress.push(msg),
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
},
|
||||
);
|
||||
|
||||
await expect(flow.login()).rejects.toThrow(
|
||||
/OAuth callback port 14572 is in use\. The OAuth provider validates redirect URIs/,
|
||||
);
|
||||
// Fallback must NOT have been attempted: only the preferred-port serve call.
|
||||
expect(serveSpy).toHaveBeenCalledTimes(1);
|
||||
// Browser must not be opened — the error fires before generateAuthUrl runs.
|
||||
expect(onAuth).not.toHaveBeenCalled();
|
||||
// And the silent "Preferred port X unavailable, using port Y" message must
|
||||
// never reach the user — that's the regression this test guards against.
|
||||
expect(progress.some(msg => msg.includes("Preferred port"))).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to a random port when DCR will re-register with the actual loopback URI", async () => {
|
||||
// The bot reviewer's concern: blocking fallback for *every* MCP flow
|
||||
// would break first-install DCR users whose preferred port is busy.
|
||||
// Here `clientId` is unset, so `MCPOAuthFlow.#tryRegisterClient` will
|
||||
// register the actual fallback URI with the provider and the
|
||||
// authorization request will use that fresh client_id.
|
||||
const blocker = Bun.serve({ port: 0, fetch: () => new Response("blocker") });
|
||||
const blockerPort = blocker.port;
|
||||
if (typeof blockerPort !== "number") {
|
||||
blocker.stop(true);
|
||||
throw new Error("Bun.serve({ port: 0 }) did not assign a numeric port");
|
||||
}
|
||||
|
||||
const registrations: unknown[] = [];
|
||||
const fetchImpl: FetchImpl = async (input, init) => {
|
||||
const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
|
||||
if (url.endsWith("/.well-known/oauth-authorization-server")) {
|
||||
return new Response(JSON.stringify({ registration_endpoint: "https://provider.example/register" }), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
if (url === "https://provider.example/register") {
|
||||
registrations.push(JSON.parse(String(init?.body)));
|
||||
return new Response(JSON.stringify({ client_id: "dcr-issued-client" }), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
return new Response("not implemented", { status: 501 });
|
||||
};
|
||||
|
||||
const progress: string[] = [];
|
||||
let authCalls = 0;
|
||||
let advertisedUrl = "";
|
||||
try {
|
||||
const flow = new MCPOAuthFlow(
|
||||
{
|
||||
authorizationUrl: "https://provider.example/authorize",
|
||||
tokenUrl: "https://provider.example/token",
|
||||
// No clientId, no redirectUri — pure DCR flow.
|
||||
callbackPort: blockerPort,
|
||||
fetch: fetchImpl,
|
||||
},
|
||||
{
|
||||
onAuth: ({ url }) => {
|
||||
authCalls += 1;
|
||||
advertisedUrl = url;
|
||||
},
|
||||
onProgress: msg => progress.push(msg),
|
||||
// Abort once the flow is waiting for the browser callback we never deliver.
|
||||
signal: AbortSignal.timeout(500),
|
||||
},
|
||||
);
|
||||
|
||||
await expect(flow.login()).rejects.toThrow(); // aborted while awaiting callback
|
||||
|
||||
// 1. The user saw the silent-fallback notice — fallback was attempted, not refused.
|
||||
const fallbackNotice = progress.find(msg => msg.startsWith(`Preferred port ${blockerPort} unavailable`));
|
||||
expect(fallbackNotice).toBeDefined();
|
||||
expect(fallbackNotice).not.toContain(`using port ${blockerPort}`);
|
||||
|
||||
// 2. generateAuthUrl ran with a random-port redirect URI.
|
||||
expect(authCalls).toBe(1);
|
||||
const authParams = new URL(advertisedUrl).searchParams;
|
||||
const advertisedRedirect = authParams.get("redirect_uri") ?? "";
|
||||
expect(advertisedRedirect).toMatch(/^http:\/\/localhost:\d+\/callback$/);
|
||||
expect(advertisedRedirect).not.toContain(`:${blockerPort}/`);
|
||||
|
||||
// 3. DCR re-registered with that same fallback URI, so the
|
||||
// provider's authorization server will accept it.
|
||||
expect(registrations).toEqual([expect.objectContaining({ redirect_uris: [advertisedRedirect] })]);
|
||||
// And the issued client_id was used in the authorize request.
|
||||
expect(authParams.get("client_id")).toBe("dcr-issued-client");
|
||||
expect(flow.resolvedClientId).toBe("dcr-issued-client");
|
||||
} finally {
|
||||
blocker.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("exposes the dynamically registered client_id and client_secret after generateAuthUrl", async () => {
|
||||
|
||||
@@ -331,7 +331,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
settings: Settings.isolated({
|
||||
"mcp.discoveryMode": true,
|
||||
defaultThinkingLevel: "high",
|
||||
serviceTier: "priority",
|
||||
"tier.openai": "priority",
|
||||
}),
|
||||
model: createReasoningModel(),
|
||||
disableExtensionDiscovery: true,
|
||||
@@ -349,7 +349,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
});
|
||||
await firstSession.activateDiscoveredMCPTools(["mcp__slack_post_message"]);
|
||||
firstSession.sessionManager.appendThinkingLevelChange(ThinkingLevel.Off);
|
||||
firstSession.sessionManager.appendServiceTierChange("priority");
|
||||
firstSession.sessionManager.appendServiceTierChange({ openai: "priority" });
|
||||
expect(firstSession.sessionManager.buildSessionContext().thinkingLevel).toBe(ThinkingLevel.Off);
|
||||
expect(firstSession.getSelectedMCPToolNames()).toEqual(["mcp__slack_post_message"]);
|
||||
const sessionFile = firstSession.sessionFile;
|
||||
@@ -368,7 +368,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
settings: Settings.isolated({
|
||||
"mcp.discoveryMode": true,
|
||||
defaultThinkingLevel: "high",
|
||||
serviceTier: "none",
|
||||
"tier.openai": "none",
|
||||
}),
|
||||
model: createReasoningModel(),
|
||||
disableExtensionDiscovery: true,
|
||||
@@ -386,7 +386,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
});
|
||||
try {
|
||||
expect(resumedSession.thinkingLevel).toBe(ThinkingLevel.Off);
|
||||
expect(resumedSession.serviceTier).toBe("priority");
|
||||
expect(resumedSession.serviceTierByFamily).toEqual({ openai: "priority" });
|
||||
expect(resumedSession.getSelectedMCPToolNames()).toEqual(["mcp__slack_post_message"]);
|
||||
expect(resumedSession.getActiveToolNames()).toEqual(
|
||||
expect.arrayContaining(["read", "search_tool_bm25", "mcp__slack_post_message"]),
|
||||
@@ -422,7 +422,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
"mcp.discoveryMode": true,
|
||||
"mcp.discoveryDefaultServers": ["github"],
|
||||
defaultThinkingLevel: "high",
|
||||
serviceTier: "priority",
|
||||
"tier.openai": "priority",
|
||||
}),
|
||||
model: createReasoningModel(),
|
||||
disableExtensionDiscovery: true,
|
||||
@@ -440,7 +440,7 @@ describe("createAgentSession MCP discovery prompt gating", () => {
|
||||
});
|
||||
try {
|
||||
expect(session.thinkingLevel).toBe(ThinkingLevel.High);
|
||||
expect(session.serviceTier).toBe("priority");
|
||||
expect(session.serviceTierByFamily).toEqual({ openai: "priority" });
|
||||
expect(session.getSelectedMCPToolNames()).toEqual(["mcp__github_create_issue"]);
|
||||
expect(session.getActiveToolNames()).toEqual(
|
||||
expect.arrayContaining(["read", "search_tool_bm25", "mcp__github_create_issue"]),
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage";
|
||||
import { getProjectAgentDir, TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { YAML } from "bun";
|
||||
import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state";
|
||||
|
||||
// Locks the back-compat migration of the legacy single `serviceTier` enum (with
|
||||
// scoped `openai-only`/`claude-only` sentinels) plus `serviceTierSubagent`/
|
||||
// `serviceTierAdvisor`/`fastModeScope` into the per-family `tier.*` settings.
|
||||
describe("serviceTier → tier.* settings migration", () => {
|
||||
let settingsState: SettingsTestState | undefined;
|
||||
let tempDir: TempDir;
|
||||
let agentDir: string;
|
||||
let projectDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
settingsState = beginSettingsTest();
|
||||
tempDir = TempDir.createSync("@test-service-tier-migration-");
|
||||
agentDir = path.join(tempDir.path(), "agent");
|
||||
projectDir = path.join(tempDir.path(), "project");
|
||||
fs.mkdirSync(agentDir, { recursive: true });
|
||||
fs.mkdirSync(getProjectAgentDir(projectDir), { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
AgentStorage.resetInstance();
|
||||
restoreSettingsTestState(settingsState);
|
||||
settingsState = undefined;
|
||||
try {
|
||||
await tempDir.remove();
|
||||
} catch {}
|
||||
});
|
||||
|
||||
async function loadWith(raw: Record<string, unknown>): Promise<Settings> {
|
||||
await Bun.write(path.join(agentDir, "config.yml"), YAML.stringify(raw, null, 2));
|
||||
resetSettingsForTest();
|
||||
return Settings.init({ cwd: projectDir, agentDir });
|
||||
}
|
||||
|
||||
it("expands unscoped priority to every family", async () => {
|
||||
const settings = await loadWith({ serviceTier: "priority" });
|
||||
expect(settings.get("tier.openai")).toBe("priority");
|
||||
expect(settings.get("tier.anthropic")).toBe("priority");
|
||||
expect(settings.get("tier.google")).toBe("priority");
|
||||
});
|
||||
|
||||
it("scopes openai-only/claude-only to a single family", async () => {
|
||||
const openai = await loadWith({ serviceTier: "openai-only" });
|
||||
expect(openai.get("tier.openai")).toBe("priority");
|
||||
expect(openai.get("tier.anthropic")).toBe("none");
|
||||
expect(openai.get("tier.google")).toBe("none");
|
||||
|
||||
const claude = await loadWith({ serviceTier: "claude-only" });
|
||||
expect(claude.get("tier.anthropic")).toBe("priority");
|
||||
expect(claude.get("tier.openai")).toBe("none");
|
||||
});
|
||||
|
||||
it("maps plain OpenAI tiers onto the OpenAI family", async () => {
|
||||
const settings = await loadWith({ serviceTier: "flex" });
|
||||
expect(settings.get("tier.openai")).toBe("flex");
|
||||
expect(settings.get("tier.anthropic")).toBe("none");
|
||||
});
|
||||
|
||||
it("carries subagent/advisor over and drops scoped sentinels", async () => {
|
||||
const settings = await loadWith({
|
||||
serviceTierSubagent: "claude-only",
|
||||
serviceTierAdvisor: "flex",
|
||||
});
|
||||
expect(settings.get("tier.subagent")).toBe("priority"); // claude-only → priority
|
||||
expect(settings.get("tier.advisor")).toBe("flex");
|
||||
});
|
||||
|
||||
it("leaves a fresh config on the per-family defaults", async () => {
|
||||
const settings = await loadWith({});
|
||||
expect(settings.get("tier.openai")).toBe("none");
|
||||
expect(settings.get("tier.subagent")).toBe("inherit");
|
||||
expect(settings.get("tier.advisor")).toBe("none");
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,7 @@ import type {
|
||||
SessionMessageEntry,
|
||||
ThinkingLevelChangeEntry,
|
||||
} from "@oh-my-pi/pi-coding-agent/session/session-entries";
|
||||
import * as snapcompact from "@oh-my-pi/snapcompact";
|
||||
|
||||
function msg(id: string, parentId: string | null, role: "user" | "assistant", text: string): SessionMessageEntry {
|
||||
const base = { type: "message" as const, id, parentId, timestamp: "2025-01-01T00:00:00Z" };
|
||||
@@ -230,6 +231,55 @@ describe("buildSessionContext", () => {
|
||||
expect((ctx.messages[1] as { content: string }).content).toBe("after compact");
|
||||
});
|
||||
|
||||
it("caps snapcompact frame payload in LLM context but preserves transcript frames", () => {
|
||||
const oldFrame = "o".repeat(Math.ceil(snapcompact.FRAME_DATA_BYTES_BUDGET / 2) + 1);
|
||||
const newFrame = "n".repeat(oldFrame.length);
|
||||
const compacted: CompactionEntry = {
|
||||
...compaction("3", "2", "Snapcompact summary", "1"),
|
||||
preserveData: {
|
||||
[snapcompact.PRESERVE_KEY]: {
|
||||
frames: [
|
||||
{ data: oldFrame, mimeType: "image/png", cols: 10, rows: 10, chars: 10 },
|
||||
{ data: newFrame, mimeType: "image/png", cols: 10, rows: 10, chars: 10 },
|
||||
],
|
||||
totalChars: 20,
|
||||
truncatedChars: 0,
|
||||
textHead: "old edge",
|
||||
textTail: "new edge",
|
||||
},
|
||||
},
|
||||
};
|
||||
const entries: SessionEntry[] = [
|
||||
msg("1", null, "user", "first"),
|
||||
msg("2", "1", "assistant", "response"),
|
||||
compacted,
|
||||
msg("4", "3", "user", "after compact"),
|
||||
];
|
||||
|
||||
const llmContext = buildSessionContext(entries);
|
||||
const summary = llmContext.messages[0];
|
||||
if (summary?.role !== "compactionSummary") throw new Error("Expected LLM compaction summary");
|
||||
const imageBlocks = summary.blocks?.filter(block => block.type === "image");
|
||||
expect(imageBlocks).toHaveLength(1);
|
||||
const keptImage = imageBlocks?.[0];
|
||||
if (keptImage?.type !== "image") throw new Error("Expected kept snapcompact image");
|
||||
expect(keptImage.data).toBe(newFrame);
|
||||
const blocks = summary.blocks ?? [];
|
||||
const noticeIndex = blocks.findIndex(
|
||||
block => block.type === "text" && block.text.includes("image middle omitted"),
|
||||
);
|
||||
const imageIndex = blocks.findIndex(block => block.type === "image");
|
||||
expect(noticeIndex).toBeGreaterThanOrEqual(0);
|
||||
// Omitted frames are the oldest archived images, so the gap notice must
|
||||
// precede the kept (newer) image to keep blocks oldest-to-newest.
|
||||
expect(noticeIndex).toBeLessThan(imageIndex);
|
||||
|
||||
const transcript = buildSessionContext(entries, undefined, undefined, { transcript: true });
|
||||
const transcriptSummary = transcript.messages[2];
|
||||
if (transcriptSummary?.role !== "compactionSummary") throw new Error("Expected transcript compaction summary");
|
||||
expect(transcriptSummary.blocks?.filter(block => block.type === "image")).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("multiple compactions uses latest", () => {
|
||||
const entries: SessionEntry[] = [
|
||||
msg("1", null, "user", "a"),
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
/**
|
||||
* Contract: `createSettingsAwareStreamFn` layers session provider settings
|
||||
* (`providers.openrouterVariant`, `providers.antigravityEndpoint`,
|
||||
* `providers.maxInFlightRequests`, `model.loopGuard.*`, `textVerbosity` for
|
||||
* Responses-family requests) onto every call while letting caller-supplied
|
||||
* `providers.stream*TimeoutSeconds`, `providers.maxInFlightRequests`,
|
||||
* `model.loopGuard.*`, `textVerbosity` for Responses-family requests)
|
||||
* options win — the same wiring the main agent and the advisor agent share so
|
||||
* OpenRouter sticky-routing / response caching behaves the same on advisor turns
|
||||
* (can1357/oh-my-pi#3639).
|
||||
@@ -65,6 +65,26 @@ describe("createSettingsAwareStreamFn", () => {
|
||||
expect(calls[2]?.options?.textVerbosity).toBe("medium");
|
||||
});
|
||||
|
||||
it("forwards configured stream watchdog budgets while preserving caller overrides", () => {
|
||||
const settings = Settings.isolated({
|
||||
"providers.streamFirstEventTimeoutSeconds": 600,
|
||||
"providers.streamIdleTimeoutSeconds": 300,
|
||||
});
|
||||
const { fn: base, calls } = captureBase();
|
||||
const wrapped = createSettingsAwareStreamFn(settings, base);
|
||||
|
||||
wrapped(stubModel, stubContext, undefined);
|
||||
wrapped(stubModel, stubContext, {
|
||||
streamFirstEventTimeoutMs: 15_000,
|
||||
streamIdleTimeoutMs: 10_000,
|
||||
});
|
||||
|
||||
expect(calls[0]?.options?.streamFirstEventTimeoutMs).toBe(600_000);
|
||||
expect(calls[0]?.options?.streamIdleTimeoutMs).toBe(300_000);
|
||||
expect(calls[1]?.options?.streamFirstEventTimeoutMs).toBe(15_000);
|
||||
expect(calls[1]?.options?.streamIdleTimeoutMs).toBe(10_000);
|
||||
});
|
||||
|
||||
it("treats the default openrouterVariant as absent so the base call carries no variant", () => {
|
||||
const settings = Settings.isolated({ "providers.openrouterVariant": "default" });
|
||||
const { fn: base, calls } = captureBase();
|
||||
|
||||
@@ -73,6 +73,10 @@ describe("STTController preflight", () => {
|
||||
setVolatileText: vi.fn(),
|
||||
clearVolatileText: vi.fn(),
|
||||
commitVolatileText: vi.fn(),
|
||||
getText: vi.fn().mockReturnValue(""),
|
||||
setText: vi.fn(),
|
||||
submit: vi.fn(),
|
||||
deleteBeforeCursor: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,254 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import { Settings, settings } from "../src/config/settings";
|
||||
import * as downloader from "../src/stt/downloader";
|
||||
import * as recorder from "../src/stt/recorder";
|
||||
import { STTController } from "../src/stt/stt-controller";
|
||||
import { evaluateSubmitTrigger, type SttSubmitTrigger } from "../src/stt/submit-trigger";
|
||||
import * as transcriber from "../src/stt/transcriber";
|
||||
import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state";
|
||||
|
||||
describe("STT Submit Trigger Evaluation", () => {
|
||||
describe("never trigger", () => {
|
||||
it("should never submit", () => {
|
||||
expect(evaluateSubmitTrigger("hello world", "never")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("submit", "never")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("", "never")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("release trigger", () => {
|
||||
it("should only submit if utterance has 2+ words", () => {
|
||||
expect(evaluateSubmitTrigger("hello", "release")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger(" hello ", "release")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello world", "release")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello world!", "release")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("one two three", "release")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("", "release")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("release-complete trigger", () => {
|
||||
it("should submit only if utterance ends with terminal punctuation", () => {
|
||||
expect(evaluateSubmitTrigger("hello", "release-complete")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello world", "release-complete")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello.", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello?", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello!", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello...", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
// Full-width punctuation
|
||||
expect(evaluateSubmitTrigger("hello。", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello?", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello!", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello…", "release-complete")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("", "release-complete")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("say-submit trigger", () => {
|
||||
it("should submit and trim trailing word when last word contains submit", () => {
|
||||
// Single word
|
||||
expect(evaluateSubmitTrigger("submit", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 6,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("SUBMIT", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 6,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("submit!", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 7,
|
||||
});
|
||||
|
||||
// Multi word
|
||||
expect(evaluateSubmitTrigger("please submit", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 7, // " submit" has length 7
|
||||
});
|
||||
expect(evaluateSubmitTrigger("please submit.", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 8, // " submit." has length 8
|
||||
});
|
||||
expect(evaluateSubmitTrigger("please submit?", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 8,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("please submit ", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 9, // " submit " has length 9
|
||||
});
|
||||
|
||||
// Word containing submit
|
||||
expect(evaluateSubmitTrigger("please autosubmit", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 11, // " autosubmit" has length 11
|
||||
});
|
||||
expect(evaluateSubmitTrigger("please submitting", "say-submit")).toEqual({
|
||||
submit: true,
|
||||
trimTrailing: 11,
|
||||
});
|
||||
|
||||
// Negative cases
|
||||
expect(evaluateSubmitTrigger("submit please", "say-submit")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("hello", "say-submit")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
expect(evaluateSubmitTrigger("", "say-submit")).toEqual({
|
||||
submit: false,
|
||||
trimTrailing: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("STTController submit trigger integration", () => {
|
||||
let state: SettingsTestState | undefined;
|
||||
let controller: STTController | undefined;
|
||||
|
||||
function makeEditor() {
|
||||
return {
|
||||
insertText: vi.fn(),
|
||||
setVolatileText: vi.fn(),
|
||||
clearVolatileText: vi.fn(),
|
||||
commitVolatileText: vi.fn(),
|
||||
submit: vi.fn(),
|
||||
deleteBeforeCursor: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
function makeOptions() {
|
||||
return {
|
||||
showWarning: vi.fn(),
|
||||
showStatus: vi.fn(),
|
||||
onStateChange: vi.fn(),
|
||||
requestRender: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
async function transcribeBatch(transcript: string, trigger: SttSubmitTrigger) {
|
||||
settings.set("stt.submitTrigger", trigger);
|
||||
vi.spyOn(transcriber, "transcribe").mockResolvedValue(transcript);
|
||||
const editor = makeEditor();
|
||||
const options = makeOptions();
|
||||
controller = new STTController();
|
||||
|
||||
await controller.toggle(editor, options);
|
||||
expect(controller.state).toBe("recording");
|
||||
await controller.toggle(editor, options);
|
||||
expect(controller.state).toBe("idle");
|
||||
|
||||
return { editor, options };
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
state = beginSettingsTest();
|
||||
await Settings.init({ inMemory: true });
|
||||
settings.set("stt.modelName", "fast");
|
||||
settings.set("stt.submitTrigger", "never");
|
||||
vi.spyOn(downloader, "isSttModelCached").mockResolvedValue(true);
|
||||
vi.spyOn(downloader, "downloadSttModel").mockResolvedValue(undefined);
|
||||
vi.spyOn(recorder, "ensureRecorder").mockResolvedValue({ tool: "sox", bin: "sox" });
|
||||
vi.spyOn(recorder, "detectRecorder").mockReturnValue({ tool: "powershell", bin: "powershell" });
|
||||
vi.spyOn(recorder, "startRecording").mockResolvedValue({ stop: vi.fn().mockResolvedValue(undefined) });
|
||||
vi.spyOn(recorder, "verifyRecordingFile").mockResolvedValue(1);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
controller?.dispose();
|
||||
controller = undefined;
|
||||
vi.restoreAllMocks();
|
||||
restoreSettingsTestState(state);
|
||||
});
|
||||
|
||||
it("submits batch dictation on release when the transcript has at least two words", async () => {
|
||||
const { editor } = await transcribeBatch("hello world", "release");
|
||||
|
||||
expect(editor.insertText).toHaveBeenCalledWith("hello world");
|
||||
expect(editor.submit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not submit one-word batch dictation on release", async () => {
|
||||
const { editor } = await transcribeBatch("hello", "release");
|
||||
|
||||
expect(editor.insertText).toHaveBeenCalledWith("hello");
|
||||
expect(editor.submit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("strips the spoken submit command before submitting batch dictation", async () => {
|
||||
const { editor } = await transcribeBatch("please review this submit.", "say-submit");
|
||||
|
||||
expect(editor.insertText).toHaveBeenCalledWith("please review this");
|
||||
expect(editor.submit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("submits the existing draft when the batch transcript only says submit", async () => {
|
||||
const { editor } = await transcribeBatch("submit", "say-submit");
|
||||
|
||||
expect(editor.insertText).not.toHaveBeenCalled();
|
||||
expect(editor.submit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,25 @@ describe("mergeIsolatedChanges", () => {
|
||||
expect(outcome.summary).toContain("nested repository patches captured");
|
||||
});
|
||||
|
||||
it("surfaces branch preparation errors instead of reporting no changes", async () => {
|
||||
const mergeSpy = vi.spyOn(worktreeModule, "mergeTaskBranches");
|
||||
const outcome = await mergeIsolatedChanges({
|
||||
repoRoot: "/repo",
|
||||
mergeMode: "branch",
|
||||
result: result({
|
||||
error: "Merge failed: git apply --3way failed for task dirty-context: conflict",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(mergeSpy).not.toHaveBeenCalled();
|
||||
expect(outcome.changesApplied).toBe(false);
|
||||
expect(outcome.hadAnyChanges).toBe(false);
|
||||
expect(outcome.mergedBranchForNestedPatches).toBe(false);
|
||||
expect(outcome.summary).toContain("Branch merge failed before a task branch could be created");
|
||||
expect(outcome.summary).toContain("git apply --3way failed");
|
||||
expect(outcome.summary).not.toContain("No changes to apply");
|
||||
});
|
||||
|
||||
it("does not mark failed branch-mode runs as nested-patch eligible", async () => {
|
||||
const outcome = await mergeIsolatedChanges({
|
||||
repoRoot: "/repo",
|
||||
|
||||
@@ -31,7 +31,7 @@ describe("buildSpecializationAdvisory", () => {
|
||||
});
|
||||
|
||||
it("treats a whitespace-only role as absent and nudges", () => {
|
||||
expect(buildSpecializationAdvisory("quick_task", [item(" ")], true)).toBeDefined();
|
||||
expect(buildSpecializationAdvisory("sonic", [item(" ")], true)).toBeDefined();
|
||||
});
|
||||
|
||||
it("nudges when one call clones the same agent twice without roles", () => {
|
||||
|
||||
@@ -17,9 +17,12 @@ describe("task schema (single-spawn)", () => {
|
||||
expect(parsed instanceof type.errors).toBe(false);
|
||||
});
|
||||
|
||||
it("requires agent", () => {
|
||||
it("defaults agent to `task` when omitted", () => {
|
||||
const parsed = taskSchema({ assignment: "Map the auth module." });
|
||||
expect(parsed instanceof type.errors).toBe(true);
|
||||
expect(parsed instanceof type.errors).toBe(false);
|
||||
if (!(parsed instanceof type.errors)) {
|
||||
expect(parsed.agent).toBe("task");
|
||||
}
|
||||
});
|
||||
|
||||
it("requires assignment", () => {
|
||||
@@ -68,9 +71,11 @@ describe("task spawn validation", () => {
|
||||
return result.content.find(part => part.type === "text")?.text ?? "";
|
||||
}
|
||||
|
||||
it("rejects a missing agent", async () => {
|
||||
it("defaults a missing agent to `task`", async () => {
|
||||
// With no `agent`, execute() normalizes to the `task` default, so the
|
||||
// failure is unknown-agent (none discovered), not missing-agent.
|
||||
const text = await executeText({ assignment: "..." });
|
||||
expect(text).toContain("Missing `agent`");
|
||||
expect(text).toContain('Unknown agent "task"');
|
||||
});
|
||||
|
||||
it("rejects a missing assignment", async () => {
|
||||
|
||||
@@ -6,6 +6,8 @@ import {
|
||||
applyNestedPatches,
|
||||
captureBaseline,
|
||||
captureDeltaPatch,
|
||||
cleanupTaskBranches,
|
||||
commitToBranch,
|
||||
ensureIsolation,
|
||||
getGitNoIndexNullPath,
|
||||
getRepoRoot,
|
||||
@@ -233,6 +235,44 @@ describe("worktree isolation helpers", () => {
|
||||
expect(stashList).toBe("");
|
||||
});
|
||||
|
||||
it("commits isolated edits when parent dirt only changes nearby context", async () => {
|
||||
const fixtureName = "EXP_DIRTY_TEST.txt";
|
||||
const fixturePath = path.join(repo, fixtureName);
|
||||
const cleanLines = Array.from({ length: 10 }, (_, index) => `line${index + 1}`);
|
||||
await fs.writeFile(fixturePath, `${cleanLines.join("\n")}\n`);
|
||||
await runGit(repo, ["add", fixtureName]);
|
||||
await runGit(repo, ["commit", "-q", "-m", "add dirty merge fixture"]);
|
||||
|
||||
const parentDirtyLines = cleanLines.map((line, index) => (index === 1 ? "LINE2-DIRTY-PARENT" : line));
|
||||
await fs.writeFile(fixturePath, `${parentDirtyLines.join("\n")}\n`);
|
||||
const baseline = await captureBaseline(repo);
|
||||
|
||||
const isoRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-worktree-iso-"));
|
||||
tempDirs.push(isoRoot);
|
||||
const iso = path.join(isoRoot, "repo");
|
||||
await runGit(isoRoot, ["clone", "-q", repo, iso]);
|
||||
await runGit(iso, ["config", "user.email", "test@example.com"]);
|
||||
await runGit(iso, ["config", "user.name", "Test User"]);
|
||||
const isolatedLines = parentDirtyLines.map((line, index) => (index === 4 ? "LINE5-AGENT-EDIT" : line));
|
||||
await fs.writeFile(path.join(iso, fixtureName), `${isolatedLines.join("\n")}\n`);
|
||||
|
||||
const taskId = `dirty-context-${path.basename(isoRoot)}`;
|
||||
let branchName = `omp/task/${taskId}`;
|
||||
try {
|
||||
const commitResult = await commitToBranch(iso, baseline, taskId, "dirty context merge");
|
||||
if (!commitResult?.branchName) throw new Error("expected task branch");
|
||||
branchName = commitResult.branchName;
|
||||
|
||||
const mergeResult = await mergeTaskBranches(repo, [{ branchName, taskId }]);
|
||||
const finalContent = await fs.readFile(fixturePath, "utf8");
|
||||
|
||||
expect(mergeResult).toEqual({ failed: [], merged: [branchName] });
|
||||
expect(finalContent).toBe(`${isolatedLines.join("\n")}\n`);
|
||||
} finally {
|
||||
await cleanupTaskBranches(repo, [branchName]);
|
||||
}
|
||||
});
|
||||
|
||||
it("subtracts baseline dirty state even when the task commits it", async () => {
|
||||
await Promise.all([
|
||||
fs.writeFile(path.join(repo, "merged.txt"), "baseline dirty change\n"),
|
||||
@@ -426,3 +466,179 @@ describe("applyNestedPatches", () => {
|
||||
expect(stashList).toContain("omp-isolation-");
|
||||
});
|
||||
});
|
||||
|
||||
describe("commitToBranch preserves agent commits", () => {
|
||||
let parent: string;
|
||||
let isolation: string;
|
||||
|
||||
async function gitr(repo: string, args: string[]): Promise<string> {
|
||||
return runGit(repo, args);
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
parent = await fs.mkdtemp(path.join(os.tmpdir(), "omp-commit-parent-"));
|
||||
isolation = await fs.mkdtemp(path.join(os.tmpdir(), "omp-commit-iso-"));
|
||||
await gitr(parent, ["init", "-q", "-b", "main"]);
|
||||
await gitr(parent, ["config", "user.email", "user@example.com"]);
|
||||
await gitr(parent, ["config", "user.name", "Parent User"]);
|
||||
await fs.writeFile(
|
||||
path.join(parent, "EXP_CLEAN_COMMIT.txt"),
|
||||
"line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n",
|
||||
);
|
||||
await gitr(parent, ["add", "."]);
|
||||
await gitr(parent, ["commit", "-q", "-m", "add clean test fixture"]);
|
||||
|
||||
// Simulate copy-on-write isolation: a real local clone so the agent's
|
||||
// commit objects live in `isolation/.git`, just like the overlay/rcopy
|
||||
// isolation backends would arrange them at runtime.
|
||||
await fs.rm(isolation, { recursive: true, force: true });
|
||||
await gitr(parent, ["clone", "-q", "--no-hardlinks", "--local", parent, isolation]);
|
||||
await gitr(isolation, ["config", "user.email", "agent@example.com"]);
|
||||
await gitr(isolation, ["config", "user.name", "Agent User"]);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all([removeWithRetries(parent), removeWithRetries(isolation)]);
|
||||
});
|
||||
|
||||
// Reproduces issue #3842: agent commits with a specific message inside
|
||||
// isolation; the merged commit on the parent branch must keep that exact
|
||||
// message instead of an AI-generated summary.
|
||||
it("preserves the agent's commit message after merge", async () => {
|
||||
const baseline = await captureBaseline(parent);
|
||||
|
||||
await fs.writeFile(
|
||||
path.join(isolation, "EXP_CLEAN_COMMIT.txt"),
|
||||
"line1\nline2\nline3\nline4\nLINE5-AGENT-WITH-MESSAGE\nline6\nline7\nline8\nline9\nline10\n",
|
||||
);
|
||||
await gitr(isolation, ["add", "EXP_CLEAN_COMMIT.txt"]);
|
||||
const agentMessage = "fix(test): agent committed with specific message for preservation check";
|
||||
await gitr(isolation, ["commit", "-q", "-m", agentMessage]);
|
||||
|
||||
const taskId = "preservation-check";
|
||||
const aiMessage = vi.fn(async () => "fix: update line5 in clean commit example");
|
||||
const result = await commitToBranch(isolation, baseline, taskId, undefined, aiMessage);
|
||||
|
||||
expect(result?.branchName).toBe(`omp/task/${taskId}`);
|
||||
expect(result?.baseSha).toBe(baseline.root.headCommit);
|
||||
// commitMessage callback must NOT have been invoked — the agent's
|
||||
// message is taken verbatim.
|
||||
expect(aiMessage).not.toHaveBeenCalled();
|
||||
|
||||
const branchSubject = await gitr(parent, ["log", "-1", "--pretty=%s", result!.branchName!]);
|
||||
expect(branchSubject).toBe(agentMessage);
|
||||
|
||||
const merge = await mergeTaskBranches(parent, [
|
||||
{ branchName: result!.branchName!, taskId, baseSha: result!.baseSha! },
|
||||
]);
|
||||
expect(merge.failed).toEqual([]);
|
||||
expect(merge.merged).toEqual([result!.branchName!]);
|
||||
|
||||
const headSubject = await gitr(parent, ["log", "-1", "--pretty=%s"]);
|
||||
expect(headSubject).toBe(agentMessage);
|
||||
});
|
||||
|
||||
it("preserves every message when the agent makes multiple commits", async () => {
|
||||
const baseline = await captureBaseline(parent);
|
||||
|
||||
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
|
||||
await gitr(isolation, ["add", "a.txt"]);
|
||||
await gitr(isolation, ["commit", "-q", "-m", "feat: add alpha file"]);
|
||||
await fs.writeFile(path.join(isolation, "b.txt"), "beta\n");
|
||||
await gitr(isolation, ["add", "b.txt"]);
|
||||
await gitr(isolation, ["commit", "-q", "-m", "test: add beta coverage"]);
|
||||
|
||||
const result = await commitToBranch(isolation, baseline, "multi", undefined);
|
||||
expect(result?.branchName).toBe("omp/task/multi");
|
||||
|
||||
const merge = await mergeTaskBranches(parent, [
|
||||
{ branchName: result!.branchName!, taskId: "multi", baseSha: result!.baseSha! },
|
||||
]);
|
||||
expect(merge).toEqual({ failed: [], merged: ["omp/task/multi"] });
|
||||
|
||||
const subjects = (await gitr(parent, ["log", "-2", "--pretty=%s"])).split("\n");
|
||||
expect(subjects).toEqual(["test: add beta coverage", "feat: add alpha file"]);
|
||||
});
|
||||
|
||||
it("appends one trailing commit when the agent leaves uncommitted work after committing", async () => {
|
||||
const baseline = await captureBaseline(parent);
|
||||
|
||||
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
|
||||
await gitr(isolation, ["add", "a.txt"]);
|
||||
await gitr(isolation, ["commit", "-q", "-m", "feat: add alpha file"]);
|
||||
// Uncommitted change on top of the agent's commit — should land as one
|
||||
// extra commit with the AI-generated message, NOT silently dropped.
|
||||
await fs.writeFile(path.join(isolation, "b.txt"), "beta\n");
|
||||
|
||||
const aiMessage = vi.fn(async () => "chore: leftover beta wip");
|
||||
const result = await commitToBranch(isolation, baseline, "leftover", undefined, aiMessage);
|
||||
expect(result?.branchName).toBe("omp/task/leftover");
|
||||
expect(aiMessage).toHaveBeenCalledTimes(1);
|
||||
|
||||
const subjects = (await gitr(parent, ["log", "-2", "--pretty=%s", result!.branchName!])).split("\n");
|
||||
expect(subjects).toEqual(["chore: leftover beta wip", "feat: add alpha file"]);
|
||||
});
|
||||
|
||||
it("filters baseline WIP when the agent commits with git add -A", async () => {
|
||||
await fs.writeFile(path.join(parent, "staged.txt"), "baseline staged wip\n");
|
||||
await gitr(parent, ["add", "staged.txt"]);
|
||||
await fs.writeFile(path.join(parent, "user-wip.txt"), "baseline untracked wip\n");
|
||||
await fs.writeFile(path.join(isolation, "staged.txt"), "baseline staged wip\n");
|
||||
await gitr(isolation, ["add", "staged.txt"]);
|
||||
await fs.writeFile(path.join(isolation, "user-wip.txt"), "baseline untracked wip\n");
|
||||
const baseline = await captureBaseline(parent);
|
||||
|
||||
await fs.writeFile(
|
||||
path.join(isolation, "EXP_CLEAN_COMMIT.txt"),
|
||||
"line1\nline2\nline3\nline4\nLINE5-AGENT-WITH-MESSAGE\nline6\nline7\nline8\nline9\nline10\n",
|
||||
);
|
||||
await gitr(isolation, ["add", "-A"]);
|
||||
const agentMessage = "fix(test): preserve message without baseline wip";
|
||||
await gitr(isolation, ["commit", "-q", "-m", agentMessage]);
|
||||
|
||||
const aiMessage = vi.fn(async () => "fix: generated fallback");
|
||||
const result = await commitToBranch(isolation, baseline, "dirty-baseline", undefined, aiMessage);
|
||||
expect(result?.branchName).toBe("omp/task/dirty-baseline");
|
||||
expect(aiMessage).not.toHaveBeenCalled();
|
||||
|
||||
const branchFiles = (await gitr(parent, ["show", "--name-only", "--pretty=format:", result!.branchName!]))
|
||||
.split("\n")
|
||||
.filter(Boolean);
|
||||
expect(branchFiles).toEqual(["EXP_CLEAN_COMMIT.txt"]);
|
||||
|
||||
const merge = await mergeTaskBranches(parent, [
|
||||
{ branchName: result!.branchName!, taskId: "dirty-baseline", baseSha: result!.baseSha! },
|
||||
]);
|
||||
expect(merge).toEqual({ failed: [], merged: ["omp/task/dirty-baseline"] });
|
||||
|
||||
const [headSubject, status, fixture] = await Promise.all([
|
||||
gitr(parent, ["log", "-1", "--pretty=%s"]),
|
||||
gitr(parent, ["status", "--porcelain=v1"]),
|
||||
fs.readFile(path.join(parent, "EXP_CLEAN_COMMIT.txt"), "utf8"),
|
||||
]);
|
||||
expect(headSubject).toBe(agentMessage);
|
||||
expect(status.split("\n").sort()).toEqual(["?? user-wip.txt", "A staged.txt"]);
|
||||
expect(fixture).toContain("LINE5-AGENT-WITH-MESSAGE");
|
||||
});
|
||||
|
||||
it("falls back to the AI-generated message when the agent never committed", async () => {
|
||||
const baseline = await captureBaseline(parent);
|
||||
|
||||
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
|
||||
|
||||
const aiMessage = vi.fn(async () => "feat: add alpha");
|
||||
const result = await commitToBranch(isolation, baseline, "nocommit", undefined, aiMessage);
|
||||
|
||||
expect(result?.branchName).toBe("omp/task/nocommit");
|
||||
expect(aiMessage).toHaveBeenCalledTimes(1);
|
||||
|
||||
const branchSubject = await gitr(parent, ["log", "-1", "--pretty=%s", result!.branchName!]);
|
||||
expect(branchSubject).toBe("feat: add alpha");
|
||||
});
|
||||
|
||||
it("returns null when nothing changed in isolation", async () => {
|
||||
const baseline = await captureBaseline(parent);
|
||||
const result = await commitToBranch(isolation, baseline, "empty", undefined);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { resolveModels } from "@oh-my-pi/pi-coding-agent/cli/tiny-models-cli";
|
||||
import { afterEach, describe, expect, it, spyOn, vi } from "bun:test";
|
||||
import { resolveModels, runTinyModelsCommand } from "@oh-my-pi/pi-coding-agent/cli/tiny-models-cli";
|
||||
import { TINY_LOCAL_MODELS } from "@oh-my-pi/pi-coding-agent/tiny/models";
|
||||
import { tinyTitleClient } from "@oh-my-pi/pi-coding-agent/tiny/title-client";
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("tiny-models download model resolution", () => {
|
||||
it("excludes load-blocked models from `all` so the bulk prefetch stays green", () => {
|
||||
@@ -25,4 +30,49 @@ describe("tiny-models download model resolution", () => {
|
||||
if (!blocked) return;
|
||||
expect(resolveModels(blocked.key)).toEqual([blocked.key]);
|
||||
});
|
||||
|
||||
it("includes worker error details in JSON failures", async () => {
|
||||
const output: string[] = [];
|
||||
spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array) => {
|
||||
output.push(typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk));
|
||||
return true;
|
||||
});
|
||||
spyOn(tinyTitleClient, "downloadModel").mockResolvedValue({
|
||||
ok: false,
|
||||
error: "Error: runtime install failed\n at worker",
|
||||
});
|
||||
|
||||
await expect(
|
||||
runTinyModelsCommand({ action: "download", model: "lfm2-700m", flags: { json: true } }),
|
||||
).rejects.toThrow("One or more tiny title models failed to download");
|
||||
|
||||
expect(JSON.parse(output.join(""))).toEqual({
|
||||
results: [{ model: "lfm2-700m", ok: false, error: "Error: runtime install failed\n at worker" }],
|
||||
});
|
||||
});
|
||||
|
||||
it("includes worker error details in text failures", async () => {
|
||||
const output: string[] = [];
|
||||
const isTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY");
|
||||
Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: false });
|
||||
spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array) => {
|
||||
output.push(typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk));
|
||||
return true;
|
||||
});
|
||||
spyOn(tinyTitleClient, "downloadModel").mockResolvedValue({
|
||||
ok: false,
|
||||
error: "Error: runtime install failed\n at worker",
|
||||
});
|
||||
|
||||
try {
|
||||
await expect(runTinyModelsCommand({ action: "download", model: "lfm2-700m", flags: {} })).rejects.toThrow(
|
||||
"One or more tiny title models failed to download",
|
||||
);
|
||||
} finally {
|
||||
if (isTtyDescriptor) Object.defineProperty(process.stdout, "isTTY", isTtyDescriptor);
|
||||
else Reflect.deleteProperty(process.stdout, "isTTY");
|
||||
}
|
||||
|
||||
expect(output.join("")).toContain("Failed to download LFM2 700M: runtime install failed.");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,26 +51,52 @@ describe("tool args reveal", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("reveals raw partial JSON monotonically for renderers that consume it", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const content = "line one\\nline two\\nline three of a streamed write payload";
|
||||
const target = `{"path":"a.ts","content":"${content}"}`;
|
||||
it("reveals what already arrived on the first setTarget call", () => {
|
||||
const { controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"abc"}`;
|
||||
|
||||
const initial = controller.setTarget(
|
||||
"call-1",
|
||||
target,
|
||||
jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }),
|
||||
);
|
||||
expect(partialOf(initial)).toBe("");
|
||||
|
||||
// The provider already delivered a complete partialJson chunk; the
|
||||
// controller MUST surface its parsed fields and raw prefix immediately —
|
||||
// pacing applies only to subsequent growth, never to bytes already in hand.
|
||||
expect(initial.path).toBe("a.ts");
|
||||
expect(initial.content).toBe("abc");
|
||||
expect(partialOf(initial)).toBe(target);
|
||||
});
|
||||
|
||||
it("paces growth across successive setTarget calls for raw-prefix renderers", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const content = "line one\\nline two\\nline three of a streamed write payload";
|
||||
const target = `{"path":"a.ts","content":"${content}"}`;
|
||||
const seed = target.slice(0, 12);
|
||||
|
||||
const initial = controller.setTarget(
|
||||
"call-1",
|
||||
seed,
|
||||
jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }),
|
||||
);
|
||||
// What arrived is exposed immediately, no empty initial frame.
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
// More bytes arrive later — the controller now paces the new backlog.
|
||||
controller.setTarget("call-1", target, jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }));
|
||||
drain(100);
|
||||
|
||||
const partials = component.frames.map(partialOf);
|
||||
expect(partials.length).toBeGreaterThan(0);
|
||||
expect(partials.at(-1)).toBe(target);
|
||||
for (let i = 1; i < partials.length; i++) {
|
||||
expect(partials[i].length).toBeGreaterThanOrEqual(partials[i - 1].length);
|
||||
expect(target.startsWith(partials[i])).toBe(true);
|
||||
let previous = seed.length;
|
||||
for (const partial of partials) {
|
||||
expect(partial.length).toBeGreaterThanOrEqual(previous);
|
||||
expect(target.startsWith(partial)).toBe(true);
|
||||
previous = partial.length;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -79,15 +105,27 @@ describe("tool args reveal", () => {
|
||||
const requestRender = vi.fn();
|
||||
const { component, controller } = makeController({ requestRender });
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(1200)}"}`;
|
||||
const seed = target.slice(0, 10);
|
||||
|
||||
const initial = controller.setTarget("call-1", target, jsonTarget());
|
||||
expect(partialOf(initial)).toBe("");
|
||||
// Seed: small initial slice, revealed immediately, sets parsedLen=seed.length.
|
||||
const initial = controller.setTarget("call-1", seed, jsonTarget());
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
// Full payload arrives; the controller paces the new growth.
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
expect(component.frames).toHaveLength(0);
|
||||
|
||||
// First paced tick lands inside the small-prefix window
|
||||
// (< STREAMING_JSON_PARSE_MIN_GROWTH), so a re-parse is forced and a
|
||||
// frame fires.
|
||||
drain(1);
|
||||
expect(component.frames).toHaveLength(1);
|
||||
expect(requestRender).toHaveBeenCalledTimes(1);
|
||||
const firstPartial = partialOf(component.frames[0]);
|
||||
|
||||
// The next tick crosses into the throttled window: growth from the last
|
||||
// parse hasn't yet hit STREAMING_JSON_PARSE_MIN_GROWTH, so no frame fires.
|
||||
drain(1);
|
||||
expect(component.frames).toHaveLength(1);
|
||||
expect(requestRender).toHaveBeenCalledTimes(1);
|
||||
@@ -98,21 +136,6 @@ describe("tool args reveal", () => {
|
||||
expect(secondPartial.length - firstPartial.length).toBeGreaterThanOrEqual(STREAMING_JSON_PARSE_MIN_GROWTH);
|
||||
});
|
||||
|
||||
it("keeps small JSON args visible before completion", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"abc"}`;
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
drain(20);
|
||||
|
||||
const latest = component.frames.at(-1)!;
|
||||
expect(latest.path).toBe("a.ts");
|
||||
expect(latest.content).toBe("abc");
|
||||
expect(partialOf(latest)).toBe(target);
|
||||
});
|
||||
|
||||
it("passes the full target through untouched when smoothing is disabled", () => {
|
||||
vi.useFakeTimers();
|
||||
const requestRender = vi.fn();
|
||||
@@ -132,11 +155,16 @@ describe("tool args reveal", () => {
|
||||
it("finish drops the reveal so no further frames are pushed", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(400)}"}`;
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", `{"path":"a.ts","content":"abcdefghijklmnop"}`, jsonTarget());
|
||||
controller.setTarget("call-1", seed, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
// Backlog of new bytes for the reveal loop to advance through.
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
drain(1);
|
||||
const frames = component.frames.length;
|
||||
expect(frames).toBeGreaterThan(0);
|
||||
controller.finish("call-1");
|
||||
drain(10);
|
||||
|
||||
@@ -147,9 +175,11 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(500)}"}`;
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
controller.setTarget("call-1", seed, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
drain(1);
|
||||
expect(partialOf(component.frames.at(-1)!).length).toBeLessThan(target.length);
|
||||
controller.flushAll();
|
||||
@@ -164,9 +194,11 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"content":"${"😀🎉".repeat(40)}"}`;
|
||||
const seed = target.slice(0, 12); // before any surrogate
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget({ exposeRawPartialJson: true }));
|
||||
controller.setTarget("call-1", seed, jsonTarget({ exposeRawPartialJson: true }));
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, jsonTarget({ exposeRawPartialJson: true }));
|
||||
drain(100);
|
||||
|
||||
expect(partialOf(component.frames.at(-1)!)).toBe(target);
|
||||
@@ -179,11 +211,16 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = "*** Begin Patch\n*** Update File: a.ts\n-old\n+new\n*** End Patch";
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", target, rawTarget({ input: target }));
|
||||
const initial = controller.setTarget("call-1", seed, rawTarget({ input: seed }));
|
||||
expect(initial.input).toBe(seed);
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, rawTarget({ input: target }));
|
||||
drain(100);
|
||||
|
||||
expect(component.frames.length).toBeGreaterThan(0);
|
||||
for (const frame of component.frames) {
|
||||
expect(frame.input).toBe(partialOf(frame));
|
||||
}
|
||||
|
||||
@@ -578,15 +578,27 @@ describe("Coding Agent Tools", () => {
|
||||
expect(output).toContain("Use :1 to read from the start, or :3 to read the last line.");
|
||||
});
|
||||
|
||||
it("should emit a binary notice instead of mojibake for files with NUL bytes", async () => {
|
||||
const testFile = path.join(testDir, "blob.bin");
|
||||
fs.writeFileSync(testFile, Buffer.from([0x61, 0x62, 0x63, 0x00, 0xff, 0xfe, 0x64, 0x65]));
|
||||
it("should refuse binary files (NUL or invalid UTF-8) instead of emitting mojibake", async () => {
|
||||
const nulFile = path.join(testDir, "blob.bin");
|
||||
fs.writeFileSync(nulFile, Buffer.from([0x61, 0x62, 0x63, 0x00, 0xff, 0xfe, 0x64, 0x65]));
|
||||
// A header with no NUL but invalid UTF-8 (lone 0xFF/0xC0) must also refuse.
|
||||
const invalidUtf8File = path.join(testDir, "font.ttfish");
|
||||
fs.writeFileSync(invalidUtf8File, Buffer.from([0x4d, 0x5a, 0xff, 0xfe, 0xc0, 0xc0, 0x90, 0x91]));
|
||||
|
||||
const result = await readTool.execute("test-call-binary-nul", { path: testFile });
|
||||
const output = getTextOutput(result);
|
||||
for (const file of [nulFile, invalidUtf8File]) {
|
||||
const output = getTextOutput(await readTool.execute("test-call-binary", { path: file }));
|
||||
expect(output).toContain("Cannot read binary file");
|
||||
expect(output).not.toContain("\u0000");
|
||||
expect(output).not.toContain("\uFFFD");
|
||||
}
|
||||
});
|
||||
|
||||
expect(output).toContain("Cannot read binary file");
|
||||
expect(output).toContain("NUL bytes");
|
||||
it("reads a binary file verbatim when :raw is requested", async () => {
|
||||
const testFile = path.join(testDir, "raw-blob.bin");
|
||||
fs.writeFileSync(testFile, Buffer.from([0x61, 0x62, 0x63, 0x00, 0x64, 0x65]));
|
||||
|
||||
const output = getTextOutput(await readTool.execute("test-call-binary-raw", { path: `${testFile}:raw` }));
|
||||
expect(output).not.toContain("Cannot read binary file");
|
||||
});
|
||||
|
||||
it("should reject malformed internal-URL selectors instead of dumping the whole resource", async () => {
|
||||
|
||||
@@ -164,6 +164,27 @@ describe("tool path arrays", () => {
|
||||
expect(details?.scopePath).toBe("apps/, packages/, phases/");
|
||||
});
|
||||
|
||||
it("search accepts JSON-array string paths in direct execute", async () => {
|
||||
const tools = await createTools(createTestSession(tempDir));
|
||||
const tool = tools.find(entry => entry.name === "grep");
|
||||
expect(tool).toBeDefined();
|
||||
if (!tool) throw new Error("Missing grep tool");
|
||||
|
||||
const result = await tool.execute("search-json-array-string-paths", {
|
||||
pattern: "shared-needle",
|
||||
paths: JSON.stringify(["apps/", "packages/", "phases/"]),
|
||||
});
|
||||
const text = getText(result);
|
||||
const details = result.details as { fileCount?: number; scopePath?: string } | undefined;
|
||||
|
||||
expect(text).toMatch(/^# apps\/\n## grep\.txt#[0-9A-F]{4}/m);
|
||||
expect(text).toMatch(/^# packages\/\n## grep\.txt#[0-9A-F]{4}/m);
|
||||
expect(text).toMatch(/^# phases\/\n## grep\.txt#[0-9A-F]{4}/m);
|
||||
expect(text).not.toContain("# other");
|
||||
expect(details?.fileCount).toBe(3);
|
||||
expect(details?.scopePath).toBe("apps/, packages/, phases/");
|
||||
});
|
||||
|
||||
it("search expands delimited path entries", async () => {
|
||||
const tools = await createTools(createTestSession(tempDir));
|
||||
const tool = tools.find(entry => entry.name === "grep");
|
||||
|
||||
@@ -580,6 +580,55 @@ describe("IRC", () => {
|
||||
expect(text).toContain("No message");
|
||||
});
|
||||
|
||||
it("op=inbox drains IRC asides that arrived while the caller was running", async () => {
|
||||
const { session } = createRealSession();
|
||||
sessions.push(session);
|
||||
Object.defineProperty(session, "isStreaming", { value: true, configurable: true });
|
||||
registry.register({ id: "0-Running", displayName: "task", kind: "sub", session });
|
||||
|
||||
const delivery = await session.deliverIrcMessage({
|
||||
id: "msg-running",
|
||||
from: "0-Main",
|
||||
to: "0-Running",
|
||||
body: "parallel note",
|
||||
ts: Date.now(),
|
||||
});
|
||||
expect(delivery).toBe("injected");
|
||||
|
||||
const tool = new IrcTool(makeToolSession(registry, "0-Running"));
|
||||
const result = await tool.execute("call-1", { op: "inbox" });
|
||||
|
||||
expect(result.details?.inbox?.map(msg => msg.body)).toEqual(["parallel note"]);
|
||||
const text = result.content[0]?.type === "text" ? result.content[0].text : "";
|
||||
expect(text).toContain("parallel note");
|
||||
});
|
||||
|
||||
it("op=inbox peek surfaces a pending IRC aside and prevents it auto-injecting", async () => {
|
||||
const { session } = createRealSession();
|
||||
sessions.push(session);
|
||||
Object.defineProperty(session, "isStreaming", { value: true, configurable: true });
|
||||
registry.register({ id: "0-Running", displayName: "task", kind: "sub", session });
|
||||
|
||||
await session.deliverIrcMessage({
|
||||
id: "msg-peek",
|
||||
from: "0-Main",
|
||||
to: "0-Running",
|
||||
body: "peeked note",
|
||||
ts: Date.now(),
|
||||
});
|
||||
|
||||
const tool = new IrcTool(makeToolSession(registry, "0-Running"));
|
||||
const peeked = await tool.execute("call-1", { op: "inbox", peek: true });
|
||||
expect(peeked.details?.inbox?.map(msg => msg.body)).toEqual(["peeked note"]);
|
||||
|
||||
// The peek surfaced the body via the tool result, so the aside-channel
|
||||
// copy must NOT also be auto-injected at the next step: a second drain
|
||||
// returns nothing (the pending aside was consumed out of the
|
||||
// auto-inject queue when peek surfaced it).
|
||||
const second = await tool.execute("call-2", { op: "inbox" });
|
||||
expect(second.details?.inbox).toEqual([]);
|
||||
});
|
||||
|
||||
it("op=inbox drains the caller's mailbox", async () => {
|
||||
const main = makeFakeSession();
|
||||
registry.register({ id: "0-Main", displayName: "main", kind: "main", session: main.session });
|
||||
|
||||
@@ -69,6 +69,36 @@ describe("buildOutputValidator", () => {
|
||||
"required",
|
||||
]);
|
||||
});
|
||||
|
||||
it("exposes per-label sub-validators that accept items (not whole arrays) for elements properties", () => {
|
||||
const { validator } = buildOutputValidator({
|
||||
properties: {
|
||||
overall_correctness: { enum: ["correct", "incorrect"] },
|
||||
explanation: { type: "string" },
|
||||
},
|
||||
optionalProperties: {
|
||||
findings: {
|
||||
elements: {
|
||||
properties: { title: { type: "string" }, body: { type: "string" } },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(validator).toBeDefined();
|
||||
const sections = validator?.validateSection;
|
||||
expect(sections).toBeDefined();
|
||||
// Scalar enum: per-section validator enforces the enum directly.
|
||||
expect(sections?.get("overall_correctness")?.("correct").success).toBe(true);
|
||||
expect(sections?.get("overall_correctness")?.("Correct").success).toBe(false);
|
||||
// String property: any string passes, non-strings fail.
|
||||
expect(sections?.get("explanation")?.("ok").success).toBe(true);
|
||||
expect(sections?.get("explanation")?.(123).success).toBe(false);
|
||||
// Array property: each section validates ONE item against the items schema, not the whole array.
|
||||
expect(sections?.get("findings")?.({ title: "t", body: "b" }).success).toBe(true);
|
||||
expect(sections?.get("findings")?.([{ title: "t", body: "b" }]).success).toBe(false);
|
||||
// Unknown labels have no validator so user-defined sections stay loose.
|
||||
expect(sections?.has("scratchpad")).toBe(false);
|
||||
});
|
||||
});
|
||||
describe("summarizeValidationFailure", () => {
|
||||
it("returns an empty summary when the result is a success", () => {
|
||||
|
||||
@@ -14,7 +14,7 @@ describe("task agent capability descriptions", () => {
|
||||
const agents = loadBundledAgents();
|
||||
|
||||
expect(isReadOnlyAgent(agentByName(agents, "explore"))).toBe(true);
|
||||
for (const name of ["task", "quick_task", "plan", "reviewer", "oracle", "designer"]) {
|
||||
for (const name of ["task", "sonic", "plan", "reviewer", "designer"]) {
|
||||
expect(isReadOnlyAgent(agentByName(agents, name))).toBe(false);
|
||||
}
|
||||
});
|
||||
@@ -24,7 +24,7 @@ describe("task agent capability descriptions", () => {
|
||||
|
||||
expect(agentByName(agents, "explore").readSummarize).toBe(false);
|
||||
expect(agentByName(agents, "librarian").readSummarize).toBe(false);
|
||||
for (const name of ["task", "quick_task", "plan", "reviewer", "oracle", "designer"]) {
|
||||
for (const name of ["task", "sonic", "plan", "reviewer", "designer"]) {
|
||||
expect(agentByName(agents, name).readSummarize).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -68,10 +68,15 @@ describe("DuckDuckGo web search provider", () => {
|
||||
const form = new URLSearchParams(capturedInit?.body as string);
|
||||
expect(form.get("q")).toBe("how to fix bug in code");
|
||||
expect(form.get("kl")).toBe("us-en");
|
||||
expect(form.get("b")).toBe("");
|
||||
expect(form.get("df")).toBe("w");
|
||||
const headers = capturedInit?.headers as Record<string, string>;
|
||||
expect(headers["Content-Type"]).toBe("application/x-www-form-urlencoded");
|
||||
expect(headers["User-Agent"]).toContain("Mozilla/5.0");
|
||||
expect(headers.Referer).toBe("https://html.duckduckgo.com/");
|
||||
expect(headers["Accept-Language"]).toContain("en");
|
||||
expect(headers["Sec-Fetch-Mode"]).toBe("navigate");
|
||||
expect(headers["Sec-Ch-Ua"]).toContain("Chromium");
|
||||
});
|
||||
|
||||
it("omits the df form param when no recency is requested", async () => {
|
||||
|
||||
@@ -121,6 +121,131 @@ describe("YieldTool", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("validates incremental sections against per-label sub-schemas with retry feedback", async () => {
|
||||
// Regression for issue #3870: DeepSeek emits `type: ["overall_correctness"]` with
|
||||
// non-enum values like "Correct" or "approved". The yield tool used to skip
|
||||
// validation for incremental yields entirely, so the model got no retry feedback
|
||||
// and the parent saw a fatal `schema_violation` post-mortem.
|
||||
const tool = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
properties: {
|
||||
overall_correctness: { enum: ["correct", "incorrect"] },
|
||||
explanation: { type: "string" },
|
||||
confidence: { type: "number" },
|
||||
},
|
||||
optionalProperties: {
|
||||
findings: {
|
||||
elements: { properties: { title: { type: "string" }, body: { type: "string" } } },
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
// Attempt 1: off-enum value rejected with the section label in the error.
|
||||
await expect(
|
||||
tool.execute("call-bad-1", { type: ["overall_correctness"], result: { data: "Correct" } } as never),
|
||||
).rejects.toThrow(/Section "overall_correctness" does not match schema.*2 retry attempt\(s\) remain/);
|
||||
|
||||
// Attempt 2 and 3 advertise dwindling retries; attempt 3 names this as the last one.
|
||||
await expect(
|
||||
tool.execute("call-bad-2", { type: ["overall_correctness"], result: { data: "correct." } } as never),
|
||||
).rejects.toThrow(/1 retry attempt\(s\) remain/);
|
||||
await expect(
|
||||
tool.execute("call-bad-3", { type: ["overall_correctness"], result: { data: "approved" } } as never),
|
||||
).rejects.toThrow(/this is the final retry/);
|
||||
|
||||
// 4th invalid yield is accepted with schemaOverridden so the parent still gets a result.
|
||||
const overrideResult = await tool.execute("call-bad-4", {
|
||||
type: ["overall_correctness"],
|
||||
result: { data: "still-wrong" },
|
||||
} as never);
|
||||
expect(overrideResult.details?.schemaOverridden).toBe(true);
|
||||
|
||||
// A fresh tool accepts a valid enum value without ticking the counter.
|
||||
const fresh = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
properties: {
|
||||
overall_correctness: { enum: ["correct", "incorrect"] },
|
||||
explanation: { type: "string" },
|
||||
confidence: { type: "number" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
const valid = await fresh.execute("call-good", {
|
||||
type: ["overall_correctness"],
|
||||
result: { data: "correct" },
|
||||
} as never);
|
||||
expect(valid.details).toEqual({
|
||||
data: "correct",
|
||||
status: "success",
|
||||
error: undefined,
|
||||
type: ["overall_correctness"],
|
||||
useLastTurn: undefined,
|
||||
schemaOverridden: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("validates incremental items for array-typed labels against the element schema", async () => {
|
||||
// Each `type: ["findings"]` yield is one finding; the per-call validator runs against the
|
||||
// items schema (not the array schema), so a missing required field surfaces immediately
|
||||
// instead of being swallowed by the post-mortem assembly.
|
||||
const tool = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
optionalProperties: {
|
||||
findings: {
|
||||
elements: {
|
||||
properties: {
|
||||
title: { type: "string" },
|
||||
body: { type: "string" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const accepted = await tool.execute("call-finding-ok", {
|
||||
type: ["findings"],
|
||||
result: { data: { title: "bug", body: "details" } },
|
||||
} as never);
|
||||
expect(accepted.details?.data).toEqual({ title: "bug", body: "details" });
|
||||
|
||||
await expect(
|
||||
tool.execute("call-finding-missing", {
|
||||
type: ["findings"],
|
||||
result: { data: { title: "only-title" } },
|
||||
} as never),
|
||||
).rejects.toThrow(/Section "findings" does not match schema.*body/);
|
||||
});
|
||||
|
||||
it("leaves user-defined section labels unconstrained", async () => {
|
||||
// Labels that are not top-level properties in the output schema have no per-call
|
||||
// validator — they're scratchpad/streaming sections the agent invents at runtime and
|
||||
// must not be rejected.
|
||||
const tool = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
properties: {
|
||||
overall_correctness: { enum: ["correct", "incorrect"] },
|
||||
explanation: { type: "string" },
|
||||
confidence: { type: "number" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
const result = await tool.execute("call-scratchpad", {
|
||||
type: ["scratchpad"],
|
||||
result: { data: { anything: "goes", n: 3 } },
|
||||
} as never);
|
||||
expect(result.details?.data).toEqual({ anything: "goes", n: 3 });
|
||||
});
|
||||
|
||||
it("rejects missing success data unless a yield type requests last-turn mode", async () => {
|
||||
const tool = new YieldTool(createSession());
|
||||
await expect(tool.execute("call-untyped-empty", { result: {} } as never)).rejects.toThrow(
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import { stripVTControlCharacters } from "node:util";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { setExcludedSearchProviders, setPreferredSearchProvider } from "@oh-my-pi/pi-coding-agent/web/search/provider";
|
||||
import {
|
||||
SEARCH_PROVIDER_ORDER,
|
||||
setExcludedSearchProviders,
|
||||
setPreferredSearchProvider,
|
||||
} from "@oh-my-pi/pi-coding-agent/web/search/provider";
|
||||
import { __resetDirsFromEnvForTests, setAgentDir, TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { runSearchCommand } from "../../../src/cli/web-search-cli";
|
||||
|
||||
@@ -128,17 +132,22 @@ describe("runSearchCommand provider settings", () => {
|
||||
});
|
||||
|
||||
it("treats explicit --provider auto as a one-shot override of the configured preferred provider", async () => {
|
||||
// Same Tavily preference is configured by `beforeEach`, but no exclusions
|
||||
// hide Jina here, so the auto chain order (Jina before Tavily) decides.
|
||||
// Tavily is the configured preference, but `--provider auto` overrides it and walks the
|
||||
// chain. Restrict eligibility to Jina + Tavily so an ambient broker/OAuth provider
|
||||
// (gemini, anthropic, codex, perplexity…) can't win on a dev machine; the chain order
|
||||
// (Jina before Tavily) still decides between the two.
|
||||
const currentTempDir = tempAgentDir;
|
||||
if (!currentTempDir) throw new Error("tempAgentDir missing");
|
||||
// Drive the exclusion through settings too — Settings.init re-applies
|
||||
// `providers.webSearchExclude`, overwriting a bare setExcludedSearchProviders() call.
|
||||
const onlyJinaTavily = SEARCH_PROVIDER_ORDER.filter(id => id !== "jina" && id !== "tavily");
|
||||
resetSettingsForTest();
|
||||
setPreferredSearchProvider("auto");
|
||||
setExcludedSearchProviders([]);
|
||||
setExcludedSearchProviders(onlyJinaTavily);
|
||||
await Settings.init({
|
||||
inMemory: true,
|
||||
cwd: currentTempDir.path(),
|
||||
overrides: { "providers.webSearch": "tavily" },
|
||||
overrides: { "providers.webSearch": "tavily", "providers.webSearchExclude": onlyJinaTavily },
|
||||
});
|
||||
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(makeFetchMock());
|
||||
|
||||
Reference in New Issue
Block a user