mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
121082430e
* fix(opencode): consolidate V2 system parts into one composed prompt (#1114) The OpenCode 2 adapter still shipped the pre-#1114 multi-part system injection: replacePlanningSystemParts kept one part per source and the generic reminder pushed a separate part, so Qwen3.x Jinja template corruption persisted for OpenCode 2 users. Mirror the V1 entry exactly: compose the stripped existing text plus additions into a single system part via composeSystemPrompt, and compose the generic reminder into the existing text instead of appending a second part. Also adds the regression tests for the bug class flagged in #1114's review: both helpers must read/compose the existing system text BEFORE truncating the array (a reorder to 'system.length = 0' first drops the host prompt and goes red here). * perf(annotate): harden the raw-HTML overlay reconcile (dead-target backoff, cull, batching) Bridge-script hardening for mutation-heavy pages and large annotation sets, plus the lost click-to-select hover affordance: - A: dead-target re-search now carries a wall-clock backoff (300ms doubling to a 5s cap, reset on success) ON TOP of the generation gate, plus a 2-searches-per-reconcile-pass budget with a scheduled follow-up pass for budget-skipped eligible targets. A page that mutates every frame advances domGeneration every frame, so the generation gate alone re-ran the whole-document TreeWalker sweep (and anchor re-resolution) per frame forever for permanently unresolvable targets. - B1: early viewport cull (64px margin) for element and range targets: wholly offscreen targets skip targetStyleHidden / getComputedStyle / clipBoundsFor / client-rect collection entirely and just omit their markers, which is what the visible pipeline produced anyway. - B2: read/write batching in renderAnnotationOverlay: highlight rects are queued during the read phase and flushed as one write phase, so the pass no longer forces a synchronous layout per record. - B3: restoreAnnotation defers its render through the existing rAF-coalesced reconcile scheduler; restoring N annotations now renders once instead of N full passes (searches stay synchronous for the mark-applied reply). DOM tests flush the frame via the suite's standard macrotask flush. - B4: zero-work observer gate: page mutations with no records, no pending draft, and pinpoint inactive still bump domGeneration but no longer schedule a reconcile frame. - D: hover affordance for click-to-select: the rAF-throttled mousemove hit-tests the pointer against the CACHED rendered committed rects and toggles a brightness class on that annotation's rect divs inside the shadow root. No page-DOM writes, rects stay pointer-transparent, and shadow-root writes are unobserved so there is no reconcile loop. - G: while a text drag is in progress in drag mode, placed markers yield pointer input (data-pn-hittest) so the 25px bubble cannot capture a selection drag; armed only by a >4px primary-button move from a non-overlay mousedown, so marker clicks and click-to-select paths are untouched. withMarkersYielded now restores (not clears) the attribute. New regression tests for A, B1, B3, B4, D; A/B1/B3 mutation-verified (fix reverted, test observed failing, fix restored). * fix(annotate): make on-page marker numbers match exportAnnotations numbering The HtmlViewer sync excluded GLOBAL_COMMENT annotations before numbering while exportAnnotations numbers '## N.' sections across the FULL list including globals — so an on-page 'Comment 2' could be '## 3.' in the feedback the agent reads. The sync now derives each marker's number from its position in the full createdA-sorted list (globals occupy a number but ship no entry, leaving the correct gaps on-page). Export format is unchanged. New buildSyncNumbering helper + tests asserting a mixed list yields identical numbers between the sync payload and exportAnnotations output (mutation-verified against the pre-fix ordering). * chore: sync stale workspace versions in bun.lock (0.26.1 -> 0.26.7) * docs: document raw-HTML overlay model, multi-target types, and known limitations - Data Types: add htmlAdditionalTargets to the Annotation listing plus the HtmlElementAnchor (including the optional normalized point used by placed markers) and HtmlAnnotationTarget shapes. - Annotation System: describe the post-#1257 raw-HTML surface (placed comment markers + overlay-projected highlights, no inline mark mutation; durable anchors persisted, disposable markers projected) and the print-parity limitation. - URL Sharing: note that share links intentionally drop HTML element anchors and additional targets (restore is text-search based, per sharing.multiTarget.test.ts). * test: fix Range.getClientRects stub typing in the B1 cull test * fix(annotate): hover-race teardown and unbounded one-shot dead-search passes Polish round on the overlay hardening: - Hover race (1): switching into pinpoint mode (or opening a draft) now tears hover down fully via clearHoverHighlight() — cancels the pending rAF hit test and clears the tracked position and id — and the rAF callback itself refuses to paint outside drag mode / with an open draft. Previously the pending callback re-applied the class after the mode switch and every flushQueuedHighlights re-painted it from the stale hoverHighlightId, leaving a permanent phantom hover. - One-shot budgets (3): beginDeadSearchPass takes a per-pass budget. Reconcile passes keep 2 (they repeat, skipped targets get follow-up frames); print and scroll-to are user-initiated one-shots with no follow-up and now run unbounded (backoff and generation gates still apply), so printing with 3+ dead-but-recoverable targets no longer silently prints fewer highlights. Both changes carry new regression tests, mutation-verified (fix reverted, test observed failing, fix restored). * fix(annotate): number markers by array position and cap entries after dropping globals The createdA sort made the export-match invariant false with external annotations: exportAnnotations' sort keys tie for every raw-HTML annotation (blockId '', startOffset 0), so its stable sort numbers the combined [...local, ...external] list in ARRAY order — and external annotations arrive appended with server-stamped createdA values that can interleave with local timestamps. buildSyncNumbering now numbers by array position of the input (verified to be the same combined list both consumers receive from packages/editor/App.tsx allAnnotations; the viewerAnnotations diffContext filter is order-preserving and vacuous on the raw-HTML surface). Also reorders the cap: number the full list, drop globals, THEN slice 512 entries — globals no longer waste sync capacity and a non-global the export numbers past position 512 still syncs while slots remain. Numbers may now exceed 512 (array positions); the bridge's own bound (100000) accepts them and its 512-entry cap still agrees with the sender. Tests updated: interleaved-external agreement with exportAnnotations (mutation-verified against the createdA sort) and slice-after-filter capacity. * docs(opencode): note the accepted cache-hint flattening trade-off in V2 consolidation
266 lines
10 KiB
TypeScript
266 lines
10 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 }> = [],
|
|
) {
|
|
let toolDefinition: Record<string, any> | undefined;
|
|
let sessionContextHook: SessionContextHook | undefined;
|
|
const sessionGet = mock(async () => ({ location: { directory: "/project" } }));
|
|
|
|
return {
|
|
context: {
|
|
options,
|
|
agent: {
|
|
list: async () => ({ 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("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"));
|
|
});
|
|
});
|