Preserve human gate review links in Q&A history

This commit is contained in:
Scott Werner 2026-10-23 13:33:45 -04:00
parent 56bed5c900
commit ffe389b5d2
3 changed files with 136 additions and 2 deletions

View file

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

View file

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

View file

@ -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());
}
}
});
});