Files
backnotprop__plannotator/apps/opencode-plugin/server.test.ts
Michael Ramos 121082430e fix: QA-gate hardening for the v0.26.8 feature set (overlay perf, numbering, OpenCode 2 parity) (#1258)
* 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
2026-08-10 15:27:04 -07:00

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"));
});
});