Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions frontend/components/playbooks/PlaybookEditor.reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@ import {
type PlaybookNode,
} from "@/lib/playbook";

// Parser for the playbook drawer. Validates the three required fields
// the editor's UI relies on, dedupes tabs by playbook_id (latest draft
// for each id wins).
// Parser for the playbook drawer. Validates the two ids the editor's
// UI relies on (the card fetches the diff bodies itself), dedupes tabs
// by playbook_id (latest draft for each id wins).
export function parsePlaybookProposal(
raw: unknown,
): { key: string; payload: ProposalDraftPayload } | null {
const r = raw as Partial<ProposalDraftPayload>;
if (
typeof r?.proposal_id !== "string" ||
typeof r?.playbook_id !== "string" ||
typeof r?.new_yaml !== "string"
typeof r?.playbook_id !== "string"
) {
return null;
}
Expand Down
24 changes: 12 additions & 12 deletions frontend/components/playbooks/ProposalBodyTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import dynamic from "next/dynamic";
import { parsePlaybookYAML, type Playbook } from "@/lib/playbook";
import { PlaybookGraph } from "@/components/playbooks/PlaybookGraph";
import type { ProposalDraftPayload } from "@/components/playbooks/ProposalCard";
import type { ProposalBody } from "@/components/playbooks/ProposalCard";

// react-diff-viewer-continued is client-only and pulls in
// styled-components. Dynamic import keeps SSR clean.
Expand All @@ -24,14 +24,14 @@ const DiffViewer = dynamic(() => import("react-diff-viewer-continued"), {
type Tab = "diagram" | "yaml";

type Props = {
payload: ProposalDraftPayload;
body: ProposalBody;
// Initial tab. Auto-overridden to "yaml" if proposed YAML fails to
// parse. Default: "diagram".
defaultTab?: Tab;
};

export function ProposalBodyTabs({ payload, defaultTab = "diagram" }: Props) {
const parsed = useMemo(() => parseSides(payload), [payload]);
export function ProposalBodyTabs({ body, defaultTab = "diagram" }: Props) {
const parsed = useMemo(() => parseSides(body), [body]);
const proposedBroken = parsed.proposed === null;
const baseBroken = parsed.base === "error";

Expand Down Expand Up @@ -73,7 +73,7 @@ export function ProposalBodyTabs({ payload, defaultTab = "diagram" }: Props) {
baseBroken={baseBroken}
/>
) : (
<YamlPane payload={payload} />
<YamlPane body={body} />
)}
</div>
);
Expand Down Expand Up @@ -149,13 +149,13 @@ function DiagramPane({
);
}

function YamlPane({ payload }: { payload: ProposalDraftPayload }) {
const isNew = !payload.base_yaml || payload.base_yaml.trim() === "";
function YamlPane({ body }: { body: ProposalBody }) {
const isNew = !body.base_yaml || body.base_yaml.trim() === "";
return (
<div className="max-h-[60vh] overflow-y-auto rounded border border-zinc-800 bg-zinc-950/60">
<DiffViewer
oldValue={payload.base_yaml ?? ""}
newValue={payload.new_yaml}
oldValue={body.base_yaml ?? ""}
newValue={body.new_yaml}
splitView
useDarkTheme
hideLineNumbers={false}
Expand Down Expand Up @@ -219,16 +219,16 @@ type ParsedSides = {
base: Playbook | undefined | "error";
};

function parseSides(payload: ProposalDraftPayload): ParsedSides {
function parseSides(body: ProposalBody): ParsedSides {
let proposed: Playbook | null = null;
try {
proposed = parsePlaybookYAML(payload.new_yaml);
proposed = parsePlaybookYAML(body.new_yaml);
} catch {
proposed = null;
}

let base: Playbook | undefined | "error";
const raw = payload.base_yaml ?? "";
const raw = body.base_yaml ?? "";
if (raw.trim() === "") {
base = undefined;
} else {
Expand Down
137 changes: 137 additions & 0 deletions frontend/components/playbooks/ProposalCard.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/* @vitest-environment jsdom */
import { render, screen } from "@testing-library/react";
import { describe, it, expect, vi, beforeEach } from "vitest";
import { ProposalCard, type ProposalDraftPayload } from "@/components/playbooks/ProposalCard";
import { parsePlaybookProposal } from "@/components/playbooks/PlaybookEditor.reducer";
import { api } from "@/lib/api";

// react-diff-viewer-continued is loaded via next/dynamic, which doesn't
// resolve under jsdom. Stub it with a marker so the tests can assert
// which sides the diff received without rendering the real viewer.
vi.mock("next/dynamic", () => ({
default: () =>
function DiffStub(props: { oldValue: string; newValue: string }) {
return (
<div data-testid="diff-stub">
<span data-testid="diff-old">{props.oldValue}</span>
<span data-testid="diff-new">{props.newValue}</span>
</div>
);
},
}));

// The tool result carries only the ids; the bodies come from the
// proposal endpoint.
const payload: ProposalDraftPayload = {
proposal_id: "prop-aaaaaaaaaaaa",
playbook_id: "testpb",
why: "because",
};

const newYaml =
"id: testpb\nschema_version: 1\nsymptom: s\nentrypoint: a\nnodes:\n a:\n description: a\n terminal_advice: done\n";
const baseYaml =
"id: testpb\nschema_version: 1\nsymptom: old\nentrypoint: a\nnodes:\n a:\n description: a\n terminal_advice: done\n";

beforeEach(() => {
vi.restoreAllMocks();
});

describe("ProposalCard body hydration", () => {
it("fetches base/new YAML from the proposal endpoint when the payload has none", async () => {
vi.spyOn(api, "getPlaybookProposal").mockResolvedValue({
proposal_id: payload.proposal_id,
status: "pending",
playbook_id: "testpb",
base_yaml: baseYaml,
new_yaml: newYaml,
});
render(<ProposalCard payload={payload} onSendRefinement={vi.fn()} defaultTab="yaml" />);
expect(await screen.findByText("current → proposed")).toBeInTheDocument();
expect((await screen.findByTestId("diff-new")).textContent).toBe(newYaml);
expect(screen.getByTestId("diff-old").textContent).toBe(baseYaml);
expect(screen.getByRole("button", { name: /approve/i })).toBeInTheDocument();
});

it("shows no new-vs-update label while the body is still loading", async () => {
vi.spyOn(api, "getPlaybookProposal").mockReturnValue(new Promise(() => {}));
render(<ProposalCard payload={payload} onSendRefinement={vi.fn()} />);
expect(await screen.findByText(/loading diff/i)).toBeInTheDocument();
expect(screen.queryByText("new playbook")).not.toBeInTheDocument();
expect(screen.queryByText("current → proposed")).not.toBeInTheDocument();
});

it("treats an empty fetched base as a new playbook", async () => {
vi.spyOn(api, "getPlaybookProposal").mockResolvedValue({
proposal_id: payload.proposal_id,
status: "pending",
playbook_id: "testpb",
new_yaml: newYaml,
});
render(<ProposalCard payload={payload} onSendRefinement={vi.fn()} defaultTab="yaml" />);
expect(await screen.findByText("new playbook")).toBeInTheDocument();
});

it("prefers bodies already present on the payload", async () => {
const spy = vi.spyOn(api, "getPlaybookProposal").mockResolvedValue({
proposal_id: payload.proposal_id,
status: "pending",
});
render(
<ProposalCard
payload={{ ...payload, base_yaml: baseYaml, new_yaml: newYaml }}
onSendRefinement={vi.fn()}
defaultTab="yaml"
/>,
);
expect((await screen.findByTestId("diff-new")).textContent).toBe(newYaml);
expect(spy).toHaveBeenCalledOnce();
});

it("fills in a missing base from the server when the payload only has new_yaml", async () => {
vi.spyOn(api, "getPlaybookProposal").mockResolvedValue({
proposal_id: payload.proposal_id,
status: "pending",
playbook_id: "testpb",
base_yaml: baseYaml,
new_yaml: "id: other\n",
});
render(
<ProposalCard
payload={{ ...payload, new_yaml: newYaml }}
onSendRefinement={vi.fn()}
defaultTab="yaml"
/>,
);
expect(await screen.findByText("current → proposed")).toBeInTheDocument();
expect(screen.getByTestId("diff-old").textContent).toBe(baseYaml);
expect(screen.getByTestId("diff-new").textContent).toBe(newYaml);
});

it("does not call a pending proposal resolved when the fetch fails transiently", async () => {
vi.spyOn(api, "getPlaybookProposal").mockRejectedValue(new Error("network down"));
render(<ProposalCard payload={payload} onSendRefinement={vi.fn()} />);
expect(await screen.findByText(/couldn't load the proposal diff/i)).toBeInTheDocument();
expect(screen.queryByText(/no longer available/i)).not.toBeInTheDocument();
expect(screen.getByRole("button", { name: /approve/i })).toBeInTheDocument();
});

it("explains when a resolved proposal's body is no longer available", async () => {
vi.spyOn(api, "getPlaybookProposal").mockResolvedValue({
proposal_id: payload.proposal_id,
status: "declined",
});
render(<ProposalCard payload={payload} onSendRefinement={vi.fn()} />);
expect(await screen.findByText(/declined\./)).toBeInTheDocument();
expect(screen.getByText(/no longer available/i)).toBeInTheDocument();
expect(screen.queryByTestId("diff-stub")).not.toBeInTheDocument();
});
});

describe("parsePlaybookProposal", () => {
it("accepts a tool result that carries only the ids", () => {
const out = parsePlaybookProposal({ proposal_id: "p1", playbook_id: "testpb" });
expect(out?.key).toBe("testpb");
expect(out?.payload.proposal_id).toBe("p1");
});
});
71 changes: 63 additions & 8 deletions frontend/components/playbooks/ProposalCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,26 @@ import { ProposalBodyTabs } from "@/components/playbooks/ProposalBodyTabs";

// Payload returned by the triagent-strategies/playbook_proposal_draft tool.
// Keep this in lockstep with proposePlaybookDraftOut in
// mcp/internal/strategies/tools_proposal.go.
// pkg/mcp/strategies/tools_proposal.go. The tool result identifies the
// draft only; the YAML bodies are optional because the card fetches them
// from GET /api/playbook-proposals/{id} (the tool result lives in the
// model's context and is size-capped there). Callers that already hold
// the bodies (the editor's own fetch) may pass them to skip the wait.
export type ProposalDraftPayload = {
proposal_id: string;
playbook_id: string;
base_yaml?: string;
new_yaml: string;
new_yaml?: string;
why?: string;
message?: string;
};

// ProposalBody is the diff pair the card renders once it has it.
export type ProposalBody = {
base_yaml?: string;
new_yaml: string;
};

type Status =
// Initial state on mount: we ask the server whether the proposal is
// still pending before showing any actions, so an old chat reloaded
Expand Down Expand Up @@ -51,28 +61,52 @@ type Props = {
// proposal tab, lifted-state surfaces) can clean up. Optional —
// the chat-only usage doesn't need it.
onResolved?: (kind: "approved" | "declined") => void;
// Initial body tab. Forwarded to ProposalBodyTabs.
defaultTab?: "diagram" | "yaml";
};

export function ProposalCard({
payload,
onSendRefinement,
dismissed = false,
onResolved,
defaultTab,
}: Props) {
const [status, setStatus] = useState<Status>({ kind: "checking" });
const [refinement, setRefinement] = useState("");
const isNew = !payload.base_yaml || payload.base_yaml.trim() === "";
// The diff pair. Seeded from the payload when the caller already has
// it; otherwise filled in by the mount fetch below while the draft is
// still pending. Resolved proposals have no draft on disk any more,
// so a card re-mounted after approve/decline may never get a body.
const [body, setBody] = useState<ProposalBody | null>(() =>
typeof payload.new_yaml === "string"
? { base_yaml: payload.base_yaml, new_yaml: payload.new_yaml }
: null,
);
// Unknown until the body is known: the header shows no
// new-vs-update label while hydrating rather than guessing "new".
const isNew = body ? !body.base_yaml || body.base_yaml.trim() === "" : null;

// On mount, ask the server whether this proposal is still pending.
// Reloading a chat after the operator approved/declined elsewhere
// would otherwise show stale Approve/Decline buttons; the server
// tracks the outcome via a tiny resolution ledger.
// tracks the outcome via a tiny resolution ledger. The same response
// carries the diff bodies for a pending draft.
useEffect(() => {
let cancelled = false;
(async () => {
try {
const res = await api.getPlaybookProposal(payload.proposal_id);
if (cancelled) return;
if (typeof res.new_yaml === "string") {
const fetched = { base_yaml: res.base_yaml, new_yaml: res.new_yaml };
// A payload that already carried new_yaml keeps it, but a
// missing base is filled in from the server so the card
// doesn't call an update "new playbook".
setBody((prev) =>
prev ? { base_yaml: prev.base_yaml ?? fetched.base_yaml, new_yaml: prev.new_yaml } : fetched,
);
}
if (res.status === "approved") {
setStatus({
kind: "approved",
Expand Down Expand Up @@ -169,9 +203,11 @@ export function ProposalCard({
<span className="font-mono text-xs text-zinc-100">
{payload.playbook_id}
</span>
<span className="font-mono text-xs text-zinc-500">
{isNew ? "new playbook" : "current → proposed"}
</span>
{isNew !== null && (
<span className="font-mono text-xs text-zinc-500">
{isNew ? "new playbook" : "current → proposed"}
</span>
)}
</div>
</div>

Expand All @@ -180,7 +216,26 @@ export function ProposalCard({
)}

<div className="mb-2">
<ProposalBodyTabs payload={payload} />
{body ? (
<ProposalBodyTabs body={body} defaultTab={defaultTab} />
) : status.kind === "checking" ? (
<div className="flex items-center gap-2 px-2 py-3 text-xs text-zinc-500">
<Spinner className="h-3 w-3" /> loading diff…
</div>
) : status.kind === "pending" ? (
// Still pending but the fetch didn't deliver a body (a
// transient error fell back to pending). The draft exists;
// only this load failed.
<div className="rounded border border-amber-900/60 bg-amber-950/30 px-2 py-1 text-xs text-amber-200/90">
Couldn't load the proposal diff. Reload the page to retry; the
draft is still pending.
</div>
) : (
<div className="rounded border border-zinc-800 bg-zinc-900/40 px-2 py-1 text-xs text-zinc-500">
The proposal body is no longer available — the draft was
removed when the proposal was resolved.
</div>
)}
</div>

{/* Footer: status / actions. Dismissed proposals (handled
Expand Down
3 changes: 2 additions & 1 deletion frontend/lib/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ export const SUMMARIZE_TOOL_NAME = "mcp__triagent-strategies__summarize";
// Wire-format name of the playbook-proposal draft tool. SessionView
// renders this tool's result inline as a diff card with Approve /
// Decline controls; the structured-output JSON carries
// {proposal_id, base_yaml, new_yaml, …}. The agent
// {proposal_id, playbook_id, …} and the card fetches the diff bodies
// from GET /api/playbook-proposals/{id}. The agent
// is instructed (via the playbook_proposal meta-playbook + system
// prompt) to call this preemptively, no chat-side ask required.
export const PROPOSE_PLAYBOOK_DRAFT_TOOL_NAME =
Expand Down
Loading
Loading