mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
fe84b3d37e
Review found the capability probe was wrong in the direction that matters.
ctx.command.transform exists on pre-#44765 hosts too: our own pinned
@opencode-ai/plugin@0.0.0-next-16775 declares CommandDraft as
{ list, get, update, remove } with no add. The probe therefore returned true on
next and latest, draft.add was undefined, and because transforms are stored and
replayed the TypeError landed in the batched reload flush and aborted it before
commit, plausibly taking every command registration on the host down with it.
Capability is now read from the draft handed to the callback, which is the only
witness, and the registration call is wrapped so no transform rejection can fail
plugin setup.
The stubs also shadowed the native definitions on new hosts. Command definitions
land in a name-keyed map where add is Map.set, transforms replay in registration
order, and OpenCode's own ConfigCommandPlugin activates in the post group after
package plugins while scanning the exact directory the installer writes the
three stubs to. A setup-time registration is therefore always overwritten on a
normal install. The plugin now re-registers the same transform once activation
settles, so its definitions are last in the replay order, and calls
ctx.command.reload() explicitly because a late registration only adds its reload
to the already-flushed boot batch. Ownership is read back from
ctx.command.list() by description, which is why the native descriptions and the
stub frontmatter are deliberately distinct. If the reclaim cannot run the stubs
keep the names and the commands still work through their fallback bodies.
Also: a failing switchAgent no longer costs the reviewer their feedback on the
command path, feedback is delivered as "queue" rather than replaying the
invocation's admission mode minutes later when a steer would land mid-turn, and
the agent-list comment no longer asserts a bare-array response that could not be
reproduced upstream (accepting both shapes is still right, since reading .data
blindly throws into a catch that degrades silently).
Tests: the real old-host draft shape registers nothing and throws nothing, the
shadowing contest is modelled against upstream's replay semantics, the OpenCode 1
parts-clearing invariant is pinned for all three commands in both plan-agent and
manual mode now that the stubs carry real instructions, and the V2 smoke asserts
the plugin did not activate as failed and that all three commands resolve. The
smoke now also installs the stubs into its sandbox config dir so the contest
actually happens there. scripts/opencode2-native-commands-smoke.sh runs the same
smoke against a dev-channel build with native commands required; CI cannot,
because it pins a next build.
AI-assisted (Claude) under maintainer direction.
376 lines
14 KiB
TypeScript
376 lines
14 KiB
TypeScript
import { afterEach, describe, expect, mock, test } from "bun:test";
|
|
import serverPlugin, {
|
|
pushComposedSystemReminder,
|
|
replacePlanningSystemParts,
|
|
} from "./server";
|
|
|
|
const originalAllowSubagents = process.env.PLANNOTATOR_ALLOW_SUBAGENTS;
|
|
|
|
afterEach(() => {
|
|
if (originalAllowSubagents === undefined) delete process.env.PLANNOTATOR_ALLOW_SUBAGENTS;
|
|
else process.env.PLANNOTATOR_ALLOW_SUBAGENTS = originalAllowSubagents;
|
|
});
|
|
|
|
type SessionContextHook = (event: {
|
|
agent: string;
|
|
system: Array<{ type: "text"; text: string }>;
|
|
messages: unknown[];
|
|
tools: Record<string, { description: string; input: Record<string, unknown> }>;
|
|
}) => Promise<void> | void;
|
|
|
|
function createContext(
|
|
options: Record<string, unknown> = {},
|
|
agents: Array<{ id: string; description?: string; mode: string; hidden: boolean }> = [],
|
|
hostOverrides: {
|
|
// Pre-#44765 hosts DO expose command.transform, but hand the callback a
|
|
// draft with no `add`. The adapter must then register nothing, throw
|
|
// nothing, and behave exactly as it did before.
|
|
command?: { transform: (apply: (draft: any) => void) => Promise<unknown> };
|
|
agentListShape?: "envelope" | "array";
|
|
} = {},
|
|
) {
|
|
let toolDefinition: Record<string, any> | undefined;
|
|
let sessionContextHook: SessionContextHook | undefined;
|
|
const sessionGet = mock(async () => ({ location: { directory: "/project" } }));
|
|
|
|
return {
|
|
context: {
|
|
options,
|
|
...(hostOverrides.command ? { command: hostOverrides.command } : {}),
|
|
agent: {
|
|
list: async () => (hostOverrides.agentListShape === "array"
|
|
? agents
|
|
: { location: { directory: "/project" }, data: agents }),
|
|
transform: async () => ({ dispose: async () => {} }),
|
|
},
|
|
session: {
|
|
get: sessionGet,
|
|
hook: async (name: string, callback: SessionContextHook) => {
|
|
if (name === "context") sessionContextHook = callback;
|
|
return { dispose: async () => {} };
|
|
},
|
|
},
|
|
tool: {
|
|
transform: async (callback: (draft: { add: (tool: Record<string, any>) => void }) => void) => {
|
|
callback({
|
|
add(tool) {
|
|
toolDefinition = tool;
|
|
},
|
|
});
|
|
return { dispose: async () => {} };
|
|
},
|
|
},
|
|
},
|
|
getToolDefinition: () => toolDefinition,
|
|
getSessionContextHook: () => sessionContextHook,
|
|
sessionGet,
|
|
};
|
|
}
|
|
|
|
describe("OpenCode V2 server plugin", () => {
|
|
test("exports a stable V2 plugin object", () => {
|
|
expect(serverPlugin.id).toBe("plannotator");
|
|
expect(serverPlugin.setup).toBeInstanceOf(Function);
|
|
});
|
|
|
|
test("registers submit_plan with the V2 JSON Schema tool contract", async () => {
|
|
const testContext = createContext();
|
|
await serverPlugin.setup(testContext.context as never);
|
|
|
|
const tool = testContext.getToolDefinition();
|
|
expect(tool?.name).toBe("submit_plan");
|
|
expect(tool?.input).toEqual({
|
|
type: "object",
|
|
properties: {
|
|
edits: {
|
|
type: "array",
|
|
items: {
|
|
type: "object",
|
|
properties: {
|
|
start: { type: "number", description: "1-indexed start line (inclusive)" },
|
|
end: {
|
|
type: "number",
|
|
description: "1-indexed end line (inclusive). Omit to replace from start through end of file.",
|
|
},
|
|
content: { type: "string", description: "Replacement content. Empty string deletes the line range." },
|
|
},
|
|
required: ["start", "content"],
|
|
additionalProperties: false,
|
|
},
|
|
description: "Array of line-range edits to apply to the plan.",
|
|
},
|
|
},
|
|
required: ["edits"],
|
|
additionalProperties: false,
|
|
});
|
|
expect(tool?.options).toEqual({ codemode: false });
|
|
expect(tool?.execute).toBeInstanceOf(Function);
|
|
});
|
|
|
|
test("resolves cwd from the V2 session and returns V2 tool content", async () => {
|
|
const testContext = createContext();
|
|
await serverPlugin.setup(testContext.context as never);
|
|
|
|
const result = await testContext.getToolDefinition()?.execute(
|
|
{ edits: [] },
|
|
{
|
|
sessionID: "session-1",
|
|
agent: "plan",
|
|
messageID: "message-1",
|
|
callID: "call-1",
|
|
progress: async () => {},
|
|
},
|
|
);
|
|
|
|
expect(testContext.sessionGet).toHaveBeenCalledWith({ sessionID: "session-1" });
|
|
expect(result).toEqual({
|
|
content: "Error: No edits provided. Pass at least one edit with start and content.",
|
|
});
|
|
});
|
|
|
|
test("uses the context hook for planning prompts and tool visibility", async () => {
|
|
const testContext = createContext();
|
|
await serverPlugin.setup(testContext.context as never);
|
|
const hook = testContext.getSessionContextHook();
|
|
expect(hook).toBeInstanceOf(Function);
|
|
|
|
const planningEvent = {
|
|
agent: "plan",
|
|
system: [
|
|
{ type: "text" as const, text: "Base system prompt", metadata: { source: "base" } },
|
|
{ type: "text" as const, text: "Earlier plugin prompt", cache: { type: "ephemeral" } },
|
|
],
|
|
messages: [],
|
|
tools: {
|
|
submit_plan: { description: "Submit", input: {} },
|
|
plan_exit: { description: "Exit", input: {} },
|
|
todowrite: { description: "Write todos", input: {} },
|
|
},
|
|
};
|
|
await hook?.(planningEvent);
|
|
|
|
// #1114: the planning path emits ONE composed system part (multi-part
|
|
// system arrays corrupt Qwen3.x Jinja chat templates). Existing text
|
|
// survives, in order, ahead of the planning prompt.
|
|
expect(planningEvent.system.length).toBe(1);
|
|
const composedText = planningEvent.system[0]!.text;
|
|
expect(composedText).toContain("Base system prompt");
|
|
expect(composedText).toContain("Earlier plugin prompt");
|
|
expect(composedText).toContain("## Plannotator");
|
|
expect(composedText.indexOf("Base system prompt"))
|
|
.toBeLessThan(composedText.indexOf("Earlier plugin prompt"));
|
|
expect(composedText.indexOf("Earlier plugin prompt"))
|
|
.toBeLessThan(composedText.indexOf("## Plannotator"));
|
|
expect(planningEvent.tools.plan_exit.description).toContain("Use submit_plan instead");
|
|
expect(planningEvent.tools.todowrite.description).toContain("use submit_plan instead");
|
|
|
|
const buildEvent = {
|
|
agent: "build",
|
|
system: [{ type: "text" as const, text: "Base system prompt" }],
|
|
messages: [],
|
|
tools: {
|
|
submit_plan: { description: "Submit", input: {} },
|
|
},
|
|
};
|
|
await hook?.(buildEvent);
|
|
expect(buildEvent.tools.submit_plan).toBeUndefined();
|
|
expect(buildEvent.system).toEqual([{ type: "text", text: "Base system prompt" }]);
|
|
|
|
const strippedEvent = {
|
|
agent: "plan",
|
|
system: [{ type: "text" as const, text: "Call plan_exit when ready." }],
|
|
messages: [],
|
|
tools: {
|
|
submit_plan: { description: "Submit", input: {} },
|
|
},
|
|
};
|
|
await hook?.(strippedEvent);
|
|
const strippedSystemText = strippedEvent.system.map((part) => part.text);
|
|
expect(strippedSystemText.some((text) => text.startsWith("## Plannotator"))).toBe(true);
|
|
expect(strippedSystemText.join("\n")).not.toContain("undefined");
|
|
});
|
|
|
|
test("keeps all-agents mode scoped to primary agents by default", async () => {
|
|
delete process.env.PLANNOTATOR_ALLOW_SUBAGENTS;
|
|
const testContext = createContext(
|
|
{ workflow: "all-agents" },
|
|
[{ id: "researcher", mode: "subagent", hidden: false }],
|
|
);
|
|
await serverPlugin.setup(testContext.context as never);
|
|
const event = {
|
|
agent: "researcher",
|
|
system: [{ type: "text" as const, text: "Base system prompt" }],
|
|
messages: [],
|
|
tools: {
|
|
submit_plan: { description: "Submit", input: {} },
|
|
},
|
|
};
|
|
|
|
await testContext.getSessionContextHook()?.(event);
|
|
expect(event.tools.submit_plan).toBeUndefined();
|
|
});
|
|
|
|
test("registers the slash commands only on a host that exposes the command API", async () => {
|
|
const registered: string[] = [];
|
|
const withCommands = createContext({}, [], {
|
|
command: {
|
|
transform: async (apply) => {
|
|
apply({ add: (definition: { name: string }) => registered.push(definition.name) });
|
|
return { dispose: async () => {} };
|
|
},
|
|
},
|
|
});
|
|
await serverPlugin.setup(withCommands.context as never);
|
|
expect(registered).toEqual([
|
|
"plannotator-review",
|
|
"plannotator-annotate",
|
|
"plannotator-last",
|
|
]);
|
|
|
|
// No command domain: nothing registered, and the pre-existing submit_plan
|
|
// contract is untouched.
|
|
const withoutCommands = createContext();
|
|
await serverPlugin.setup(withoutCommands.context as never);
|
|
expect(withoutCommands.getToolDefinition()?.name).toBe("submit_plan");
|
|
});
|
|
|
|
test("a pre-#44765 command draft registers nothing and does not fail setup", async () => {
|
|
// The real `next` / `latest` shape: transform exists, the draft is
|
|
// { list, get, update, remove }. Touching `add` here would throw inside the
|
|
// host's batched reload flush and abort it before commit.
|
|
let applied = false;
|
|
const testContext = createContext({}, [], {
|
|
command: {
|
|
transform: async (apply) => {
|
|
applied = true;
|
|
apply({ list: () => [], get: () => undefined, update: () => {}, remove: () => {} });
|
|
return { dispose: async () => {} };
|
|
},
|
|
},
|
|
});
|
|
|
|
await serverPlugin.setup(testContext.context as never);
|
|
expect(applied).toBe(true);
|
|
expect(testContext.getToolDefinition()?.name).toBe("submit_plan");
|
|
});
|
|
|
|
test("a rejecting command transform never fails plugin setup", async () => {
|
|
// A slash command has a working markdown fallback; the whole Plannotator
|
|
// integration going down for it would not.
|
|
const testContext = createContext({}, [], {
|
|
command: { transform: async () => { throw new Error("command domain unavailable"); } },
|
|
});
|
|
const originalError = console.error;
|
|
console.error = () => {};
|
|
try {
|
|
await serverPlugin.setup(testContext.context as never);
|
|
} finally {
|
|
console.error = originalError;
|
|
}
|
|
expect(testContext.getToolDefinition()?.name).toBe("submit_plan");
|
|
});
|
|
|
|
test("registers slash commands even when submit_plan is disabled", async () => {
|
|
// `workflow: "manual"` returns early before the tool registration, which is
|
|
// exactly the mode that depends on the slash commands existing.
|
|
const registered: string[] = [];
|
|
const testContext = createContext({ workflow: "manual" }, [], {
|
|
command: {
|
|
transform: async (apply) => {
|
|
apply({ add: (definition: { name: string }) => registered.push(definition.name) });
|
|
return { dispose: async () => {} };
|
|
},
|
|
},
|
|
});
|
|
await serverPlugin.setup(testContext.context as never);
|
|
|
|
expect(registered).toHaveLength(3);
|
|
expect(testContext.getToolDefinition()).toBeUndefined();
|
|
});
|
|
|
|
test("reads a bare-array agent list, so subagent gating still applies", async () => {
|
|
// Newer plugin hosts answer agent.list() with an array rather than the
|
|
// `{ data }` envelope; reading `.data` blindly emptied the list and let
|
|
// subagents keep submit_plan.
|
|
delete process.env.PLANNOTATOR_ALLOW_SUBAGENTS;
|
|
const testContext = createContext(
|
|
{ workflow: "all-agents" },
|
|
[{ id: "researcher", mode: "subagent", hidden: false }],
|
|
{ agentListShape: "array" },
|
|
);
|
|
await serverPlugin.setup(testContext.context as never);
|
|
const event = {
|
|
agent: "researcher",
|
|
system: [{ type: "text" as const, text: "Base system prompt" }],
|
|
messages: [],
|
|
tools: { submit_plan: { description: "Submit", input: {} } },
|
|
};
|
|
|
|
await testContext.getSessionContextHook()?.(event);
|
|
expect(event.tools.submit_plan).toBeUndefined();
|
|
});
|
|
|
|
test("generic reminder composes into the existing part instead of pushing a second one", async () => {
|
|
process.env.PLANNOTATOR_ALLOW_SUBAGENTS = "1";
|
|
const testContext = createContext(
|
|
{ workflow: "all-agents" },
|
|
[{ id: "helper", mode: "primary", hidden: false }],
|
|
);
|
|
await serverPlugin.setup(testContext.context as never);
|
|
const event = {
|
|
agent: "helper",
|
|
system: [{ type: "text" as const, text: "Base system prompt" }],
|
|
messages: [],
|
|
tools: {
|
|
submit_plan: { description: "Submit", input: {} },
|
|
},
|
|
};
|
|
|
|
await testContext.getSessionContextHook()?.(event);
|
|
// #1114: a second system part corrupts Qwen3.x Jinja templates.
|
|
expect(event.system.length).toBe(1);
|
|
expect(event.system[0]!.text).toContain("Base system prompt");
|
|
expect(event.system[0]!.text).toContain("## Plan Submission");
|
|
expect(event.system[0]!.text.indexOf("Base system prompt"))
|
|
.toBeLessThan(event.system[0]!.text.indexOf("## Plan Submission"));
|
|
});
|
|
});
|
|
|
|
describe("system part consolidation (#1114 regression class)", () => {
|
|
// The bug class flagged in #1114's review: truncating the system array
|
|
// BEFORE composing silently drops the host's entire system prompt. These
|
|
// fail if either helper is reordered to `system.length = 0` first.
|
|
|
|
test("replacePlanningSystemParts composes existing text before truncating", () => {
|
|
const system = [
|
|
{ type: "text" as const, text: "Host base rules" },
|
|
{ type: "text" as const, text: "STRICTLY FORBIDDEN: ANY file edits.\nKeep plans concise." },
|
|
];
|
|
replacePlanningSystemParts(system, ["## Plannotator planning prompt"]);
|
|
expect(system.length).toBe(1);
|
|
const text = system[0]!.text;
|
|
// Pre-existing prompt text survives the consolidation (compose ran first).
|
|
expect(text).toContain("Host base rules");
|
|
expect(text).toContain("Keep plans concise.");
|
|
expect(text).toContain("## Plannotator planning prompt");
|
|
expect(text.indexOf("Host base rules")).toBeLessThan(text.indexOf("Keep plans concise."));
|
|
expect(text.indexOf("Keep plans concise.")).toBeLessThan(text.indexOf("## Plannotator planning prompt"));
|
|
// Conflicting plan-mode rules are still stripped.
|
|
expect(text).not.toContain("STRICTLY FORBIDDEN");
|
|
});
|
|
|
|
test("pushComposedSystemReminder keeps prior parts' text before the reminder", () => {
|
|
const system = [
|
|
{ type: "text" as const, text: "Host base rules" },
|
|
{ type: "text" as const, text: "Second host part" },
|
|
];
|
|
pushComposedSystemReminder(system, "## Plan Submission reminder");
|
|
expect(system.length).toBe(1);
|
|
const text = system[0]!.text;
|
|
expect(text).toContain("Host base rules");
|
|
expect(text).toContain("Second host part");
|
|
expect(text.endsWith("## Plan Submission reminder")).toBe(true);
|
|
expect(text.indexOf("Host base rules")).toBeLessThan(text.indexOf("Second host part"));
|
|
});
|
|
});
|