Merge pull request #1067 from fabro-sh/codex/retain-review-history-links

Preserve human gate review links in Q&A history

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", () => {

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

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(
+          <HumanQA stage={gate} pairs={parsePetriInterviewPairs(events)} />,
+        );
+      });
+      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());
+      }
+    }
+  });
 });