From ffe389b5d2f76509be25b712c9d3fc4a43c37bfd Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 23 Oct 2026 13:33:45 -0400 Subject: [PATCH] Preserve human gate review links in Q&A history --- apps/fabro-web/app/lib/petri-stream.test.ts | 75 +++++++++++++++++++ apps/fabro-web/app/lib/petri-stream.ts | 17 ++++- .../app/routes/run-petri.render.test.tsx | 46 ++++++++++++ 3 files changed, 136 insertions(+), 2 deletions(-) diff --git a/apps/fabro-web/app/lib/petri-stream.test.ts b/apps/fabro-web/app/lib/petri-stream.test.ts index 55dc013ef..b98d87176 100644 --- a/apps/fabro-web/app/lib/petri-stream.test.ts +++ b/apps/fabro-web/app/lib/petri-stream.test.ts @@ -106,6 +106,81 @@ describe("questions", () => { test("a run without a gate asks nothing", () => { expect(parsePetriInterviewPairs(command.stream)).toEqual([]); }); + + function question(reference: unknown, id = "review#1", seq = 1): RunStreamItem { + return { + run_id: "run-review", + stream_seq: seq, + kind: "petri", + id: `execution 0/${seq}/0`, + recorded_at: 1_000 * seq, + item: { + record: { body: { event: "step.progress.recorded" } }, + derived: { + parsed: { + kind: "question", + question: { id, text: "Review the document.", reference }, + }, + }, + }, + }; + } + + const reference = { + label: "Design review", + url: "https://example.com/review/%2Fdraft?revision=one&token=a%2Bb#feedback", + kind: "document", + }; + + test("each question keeps its original review target through answer and replay", () => { + const asked = question(reference); + const answered: RunStreamItem = { + ...asked, + stream_seq: 2, + id: "execution 0/2/0", + recorded_at: 2_000, + item: { + record: { body: { event: "control.requested" } }, + derived: { deliverable: true, answer: { question: "review#1", choice: "Y" } }, + }, + }; + const nextReference = { ...reference, label: "Revised design", url: "https://example.com/next" }; + const stream = [asked, answered, question(nextReference, "review#2", 3)]; + const pending = parsePetriInterviewPairs([asked]); + expect(pending[0].question.reviewTarget).toEqual(reference); + expect(pending[0].resolution).toBeNull(); + + const pairs = parsePetriInterviewPairs(stream); + expect(pairs.map((pair) => pair.question.reviewTarget)).toEqual([reference, nextReference]); + expect(pairs[0].resolution).toMatchObject({ kind: "answered", answer: "Y" }); + expect(pairs[1].resolution).toBeNull(); + // A fresh page reads serialized events, with no live question or context cache. + expect(parsePetriInterviewPairs(JSON.parse(JSON.stringify(stream)))).toEqual(pairs); + }); + + test.each([undefined, null])("a reference with kind %p is a document, like the pending projection", (kind) => { + const [pair] = parsePetriInterviewPairs([question({ ...reference, kind })]); + expect(pair.question.reviewTarget).toEqual(reference); + }); + + test.each([ + undefined, + null, + "https://example.com/review", + [], + {}, + { ...reference, kind: "pull_request" }, + { ...reference, kind: "" }, + { ...reference, kind: 42 }, + { ...reference, label: 42 }, + { ...reference, label: " " }, + { ...reference, url: 42 }, + { ...reference, url: " " }, + ].map((value) => [value]))("a missing or malformed reference falls back to question text: %p", (value) => { + const [pair] = parsePetriInterviewPairs([question(value)]); + expect(pair.question.reviewTarget).toBeNull(); + expect(pair.question.question).toBe("Review the document."); + }); }); describe("run phases", () => { diff --git a/apps/fabro-web/app/lib/petri-stream.ts b/apps/fabro-web/app/lib/petri-stream.ts index 63cb48189..5f75169a2 100644 --- a/apps/fabro-web/app/lib/petri-stream.ts +++ b/apps/fabro-web/app/lib/petri-stream.ts @@ -6,8 +6,9 @@ * keyed by `stream_seq`. The mapping from items to views follows * `lib/components/fabro-petri/VIEWS.md`. */ -import { StageOutcome, StageState } from "@qltysh/fabro-api-client"; +import { ReviewTargetKind, StageOutcome, StageState } from "@qltysh/fabro-api-client"; import type { + ReviewTarget, RunProjection, RunStreamItem, StageProjection, @@ -156,6 +157,18 @@ export function itemsForStage( // ── Questions ─────────────────────────────────────────────────────────── +/** Match the pending-question projection's document reference mapping. */ +function parseReviewTarget(value: unknown): ReviewTarget | null { + if (!isRecord(value)) return null; + if (value.kind != null && value.kind !== ReviewTargetKind.DOCUMENT) return null; + const label = getString(value, "label")?.trim(); + const url = getString(value, "url")?.trim(); + if (!label || !url) return null; + // Keep the recorded URL (including query and fragment). The shared + // question renderer checks URL safety before creating a link. + return { label, url, kind: ReviewTargetKind.DOCUMENT }; +} + function parseOptions(value: unknown): InterviewOption[] { if (!Array.isArray(value)) return []; const out: InterviewOption[] = []; @@ -243,7 +256,7 @@ export function parsePetriInterviewPairs(stream: PetriStream): HumanInterviewPai allowFreeform: getBool(question, "freeform") === true, timeoutSeconds: timeoutMs !== undefined ? Math.round(timeoutMs / 1000) : null, contextDisplay: getString(question, "context") ?? null, - reviewTarget: null, + reviewTarget: parseReviewTarget(question?.reference), }, resolution: null, }); diff --git a/apps/fabro-web/app/routes/run-petri.render.test.tsx b/apps/fabro-web/app/routes/run-petri.render.test.tsx index 5c620be94..cea3b8e42 100644 --- a/apps/fabro-web/app/routes/run-petri.render.test.tsx +++ b/apps/fabro-web/app/routes/run-petri.render.test.tsx @@ -25,11 +25,13 @@ import { itemsForStage, parallelOverviewFromProjection, parsePetriInterviewPairs, + petriParsed, platformRecordsOf, reducerTranscriptFromProjection, stagesFromProjection, } from "../lib/petri-stream"; import { setupReactTestEnv } from "../lib/test-utils"; +import { getObject } from "../lib/unknown"; import { StreamEventsView } from "./run-events"; import { StageChatView, buildPetriStageActivity } from "./run-stages"; @@ -282,4 +284,48 @@ describe("a human gate answered through the API", () => { expect(Object.keys(fixture.projection.pending_interviews)).toEqual([]); expect(stages.map((stage) => stage.id)).toEqual(["start@1", "gate@1", "no@1", "exit@1"]); }); + + test.each([ + ["https://example.com/review/%2Fdraft?revision=one&token=a%2Bb#feedback", true], + ["javascript:alert(1)", false], + ["https://user:password@example.com/review", false], + ["/relative/review", false], + ["ftp://example.com/review", false], + ])("answered Q&A preserves safe review links after replay: %s", (url, linked) => { + // Add a reference to the real API-captured gate stream; keep its delivered + // answer and settled projection, which has no pending interview left. + const stream = structuredClone(fixture.stream); + const question = stream.map((item) => getObject(petriParsed(item), "question")).find(Boolean)!; + question.text = "Review the design document, then choose the next action."; + question.reference = { label: "Design review", url, kind: "document" }; + const gate = stages.find((stage) => stage.id === "gate@1")!; + + for (const events of [stream, JSON.parse(JSON.stringify(stream))]) { + let renderer!: TestRenderer.ReactTestRenderer; + act(() => { + renderer = TestRenderer.create( + , + ); + }); + try { + const links = renderer.root.findAllByType("a"); + expect(links).toHaveLength(linked ? 1 : 0); + if (linked) { + expect(links[0].props).toMatchObject({ + href: url, + target: "_blank", + rel: "noopener noreferrer", + referrerPolicy: "no-referrer", + }); + expect(links[0].findByType("span").children).toEqual(["Design review"]); + } else { + expect(JSON.stringify(renderer.toJSON())).toContain(question.text); + } + expect(JSON.stringify(renderer.toJSON())).toContain("Answered"); + expect(JSON.stringify(renderer.toJSON())).toContain("dev"); + } finally { + act(() => renderer.unmount()); + } + } + }); });