From 5becbbd216b8763992838002f3cddc9de2187397 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 10 Aug 2026 16:49:12 +0000 Subject: [PATCH] fix(runner): clear the stale pending-request announcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Phase 7 issue-thread live region re-armed "A request is waiting for your answer." on every snapshot while a request was pending. On the live path a submit re-renders with the same request still pending (state `submitting`), so the pending copy overwrote "Your answer was recorded." and then stayed put once the request settled — clean-room QA (PAP-16974) heard the stale guidance during later turns. The pending copy is now announced once per request, on arrival, and the settle path replaces it (and only it) with the request's resolved state label, so a request that goes away server-side corrects itself too. Covers pending -> answered -> later turn, plus a server-side withdrawal, in the issue-thread browser suite: 60/60 pass, `check:phase7:ui` still reproduces all 24 screenshots byte-for-byte. --- .../issue-thread/issue-thread.spec.ts | 200 ++++++++++++++++++ .../devtools/issue-thread/src/App.tsx | 54 ++++- .../docs/phase-07-verification-commands.md | 2 +- 3 files changed, 250 insertions(+), 6 deletions(-) diff --git a/packages/paperclip-runner/devtools/issue-thread/issue-thread.spec.ts b/packages/paperclip-runner/devtools/issue-thread/issue-thread.spec.ts index 2ca967d38f9..eafa527ccb7 100644 --- a/packages/paperclip-runner/devtools/issue-thread/issue-thread.spec.ts +++ b/packages/paperclip-runner/devtools/issue-thread/issue-thread.spec.ts @@ -3,6 +3,10 @@ import { createRequire } from "node:module"; import { expect, test, type Page } from "@playwright/test"; import { PHASE7_UI_SHOT_SLUGS } from "../../src/issue-thread/fixtures"; +import type { + Phase7IssueThreadSnapshot, + Phase7ThreadItem, +} from "../../src/issue-thread/types"; const require = createRequire(import.meta.url); const AXE_PATH = require.resolve("axe-core/axe.min.js"); @@ -666,3 +670,199 @@ test.describe("Phase 7M clean-room chat", () => { expect(await seriousAxeViolations(page)).toEqual([]); }); }); + +/* ------------------------------------ pending-request live region (PAP-16978) */ + +/** + * Clean-room QA (PAP-16974) heard the visually hidden status region still + * announce "A request is waiting for your answer." after the request had been + * answered: the live path re-renders with the same request still pending while + * the submit is in flight, which used to re-arm the pending copy and leave it + * standing once the request settled. These tests pin the live region to the + * request's actual state across pending → answered → a later turn. + */ + +const PENDING_ANNOUNCEMENT = "A request is waiting for your answer."; + +function questionsItem(state: "pending" | "answered" | "withdrawn"): Phase7ThreadItem { + return { + kind: "interaction", + id: "item-ix-1", + at: "2026-08-10T12:00:03.000Z", + interactionId: "ix-questions-01", + interactionKind: "questions", + title: "One scoping question", + prompt: "Answer this before I continue.", + payload: { + kind: "questions", + submitLabel: "Submit answers", + questions: [ + { + id: "q1", + prompt: "Should the runner own process cleanup?", + control: "radio", + options: ["Yes — runner owns it", "No — harness owns it"], + required: true, + }, + ], + }, + state, + target: null, + stateLabel: + state === "pending" ? "Waiting for you" : state === "answered" ? "Answered" : "Withdrawn", + resolvedSummary: + state === "answered" ? ["Should the runner own process cleanup? Yes — runner owns it"] : [], + reason: null, + supersededBy: null, + evidenceRef: { section: "authorization", recordId: "authz-ix-1" }, + }; +} + +/** A clean-room view carrying one request in the given state. */ +function interactionView( + identifier: string, + state: "pending" | "answered", + options: { withdrawn?: boolean; extraTurn?: boolean } = {}, +): Phase7IssueThreadSnapshot { + const view = JSON.parse( + JSON.stringify(cleanRoomView(identifier, true)), + ) as Phase7IssueThreadSnapshot; + view.turns[0].items.push(questionsItem(options.withdrawn === true ? "withdrawn" : state)); + if (options.extraTurn === true) { + view.turns.push({ + id: "turn-2", + ordinal: 2, + mode: "live", + toolCallCount: 0, + at: "2026-08-10T12:01:00.000Z", + stoppedByUser: false, + items: [ + { + kind: "user_message", + id: "transcript-3", + at: "2026-08-10T12:01:00.000Z", + author: "You (board user)", + body: "Carry on with the plan.", + }, + { + kind: "agent_message", + id: "transcript-4", + at: "2026-08-10T12:01:01.000Z", + author: "Real Codex", + body: "Folded the answer into the plan.", + streaming: false, + }, + ], + }); + } + view.composer = + state === "pending" + ? { + state: "waiting", + helper: "Answer the pending request above to continue.", + reason: null, + pendingInteractionId: "ix-questions-01", + } + : { state: "ready", helper: null, reason: null, pendingInteractionId: null }; + return view; +} + +async function openWithPendingRequest(page: Page) { + await page.addInitScript(() => window.localStorage.clear()); + await page.route(CLEAN_ROOM_API, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + ...cleanRoomPayload("MCK-1000"), + view: interactionView("MCK-1000", "pending"), + }), + }); + }); + await page.goto("/#/chat"); + await expect(page.locator('[data-thread-state="settled"]')).toBeVisible(); +} + +test.describe("pending-request live region", () => { + test("the pending announcement is dropped once the request is answered", async ({ page }) => { + await openWithPendingRequest(page); + + const live = page.getByTestId("thread-live-region"); + await expect(live).toHaveText(PENDING_ANNOUNCEMENT); + await expect(page.locator('[data-composer-state="waiting"]')).toBeVisible(); + + await page.route("**/api/phase7/ui/interaction", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + ...cleanRoomPayload("MCK-1000"), + view: interactionView("MCK-1000", "answered"), + }), + }); + }); + + const card = page.locator('[data-interaction-id="ix-questions-01"]'); + await card.getByRole("radio", { name: "Yes — runner owns it" }).check(); + await card.getByRole("button", { name: "Submit answers" }).click(); + + await expect(card).toHaveAttribute("data-interaction-state", "answered"); + await expect(page.locator('[data-composer-state="ready"]')).toHaveCount(1); + await expect(live).not.toContainText(PENDING_ANNOUNCEMENT); + await expect(live).toHaveText("Your answer was recorded."); + + // A later turn must not resurrect the pending guidance either. + await page.route("**/api/phase7/ui/message", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + ...cleanRoomPayload("MCK-1000"), + view: interactionView("MCK-1000", "answered", { extraTurn: true }), + }), + }); + }); + await page.locator("#composer-input").fill("Carry on with the plan."); + await page.getByTestId("composer-send").click(); + + await expect(page.locator("[data-turn-id]")).toHaveCount(2); + await expect(live).not.toContainText(PENDING_ANNOUNCEMENT); + }); + + test("a request withdrawn server-side replaces the pending announcement", async ({ page }) => { + // Nobody answers here: the request goes away while the session is + // reconnecting, so the correction has to come from the settled snapshot. + await page.addInitScript(() => window.localStorage.clear()); + await page.route(CLEAN_ROOM_API, async (route) => { + const view = interactionView("MCK-1000", "pending"); + view.composer.state = "reconnecting"; + view.connection = { state: "reconnecting", attempt: 2 }; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ ...cleanRoomPayload("MCK-1000"), view }), + }); + }); + await page.goto("/#/chat"); + await expect(page.locator('[data-thread-state="settled"]')).toBeVisible(); + + const live = page.getByTestId("thread-live-region"); + await expect(live).toHaveText(PENDING_ANNOUNCEMENT); + + await page.route("**/api/phase7/ui/reconnect", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + ...cleanRoomPayload("MCK-1000"), + view: interactionView("MCK-1000", "answered", { withdrawn: true }), + }), + }); + }); + await page.getByRole("button", { name: "Retry now" }).click(); + + await expect(page.locator('[data-interaction-state="withdrawn"]')).toBeVisible(); + await expect(live).not.toContainText(PENDING_ANNOUNCEMENT); + await expect(live).toHaveText("The pending request is resolved. Withdrawn."); + }); +}); diff --git a/packages/paperclip-runner/devtools/issue-thread/src/App.tsx b/packages/paperclip-runner/devtools/issue-thread/src/App.tsx index 1920d2ec515..41f0a770446 100644 --- a/packages/paperclip-runner/devtools/issue-thread/src/App.tsx +++ b/packages/paperclip-runner/devtools/issue-thread/src/App.tsx @@ -37,6 +37,14 @@ const PANEL_MIN = 320; const PANEL_MAX = 640; /** Play-all cadence for the replay strip (§6); slow enough to read a turn. */ const REPLAY_STEP_MS = 800; +/** + * The pending-request guidance is a constant because the settle path has to + * recognise its own stale announcement: a screen reader must not still be told + * a request is waiting after it was answered (PAP-16978). + */ +const PENDING_ANNOUNCEMENT = "A request is waiting for your answer."; +const ANSWERED_ANNOUNCEMENT = "Your answer was recorded."; +const SETTLED_ANNOUNCEMENT = "The pending request is resolved."; const SCENARIOS = ["hb-baseline", "dp-documents", "ix-interactions", "ar-artifacts"]; function readStoredNumber(key: string, fallback: number): number { @@ -76,6 +84,18 @@ function describe(cause: unknown): string { return String(cause instanceof Error ? cause.message : cause); } +/** Resolution copy for a request that settled without this client answering it. */ +function settledAnnouncement(snapshot: Phase7IssueThreadSnapshot, interactionId: string): string { + for (const turn of snapshot.turns) { + for (const item of turn.items) { + if (item.kind === "interaction" && item.interactionId === interactionId) { + return `${SETTLED_ANNOUNCEMENT} ${item.stateLabel}.`; + } + } + } + return SETTLED_ANNOUNCEMENT; +} + function useRoute(): Phase7Route { const [route, setRoute] = useState(() => parsePhase7Route(window.location)); useEffect(() => { @@ -136,6 +156,8 @@ export function App() { const [playing, setPlaying] = useState(false); const scrollRef = useRef(null); const cancelResetRef = useRef(null); + /** Last announced pending request, so the live region tracks transitions. */ + const announcedPendingRef = useRef(null); /* --------------------------------------------------------------- loading */ @@ -234,10 +256,27 @@ export function App() { useEffect(() => { if (snapshot === null) return; const pending = snapshot.composer.pendingInteractionId; - if (pending !== null) setAnnouncement("A request is waiting for your answer."); - else if (snapshot.connection.state === "reconnecting") { - setAnnouncement(`Connection lost — retrying (attempt ${snapshot.connection.attempt}).`); + const announced = announcedPendingRef.current; + announcedPendingRef.current = pending; + + if (pending !== null) { + // Announce a request once, when it arrives. Re-announcing on every + // snapshot would talk over the outcome the user just triggered: a live + // submit re-renders with the same request still pending. + if (pending !== announced) setAnnouncement(PENDING_ANNOUNCEMENT); + return; } + if (snapshot.connection.state === "reconnecting") { + setAnnouncement(`Connection lost — retrying (attempt ${snapshot.connection.attempt}).`); + return; + } + if (announced === null) return; + // The request settled. Anything the resolution itself announced stands; + // only the now-false "still waiting" guidance is replaced, so a screen + // reader never carries pending-state copy into later turns (PAP-16978). + setAnnouncement((current) => + current === PENDING_ANNOUNCEMENT ? settledAnnouncement(snapshot, announced) : current, + ); }, [snapshot]); /* -------------------------------------------------------------- handlers */ @@ -318,7 +357,7 @@ export function App() { } return applyFakeInteractionResponse(current, response); }); - setAnnouncement("Your answer was recorded."); + setAnnouncement(ANSWERED_ANNOUNCEMENT); }, [route.mode], ); @@ -828,7 +867,12 @@ export function App() { ) : null} -

+

{announcement}

diff --git a/packages/paperclip-runner/docs/phase-07-verification-commands.md b/packages/paperclip-runner/docs/phase-07-verification-commands.md index 46cf02e46b9..aaa4968cfd0 100644 --- a/packages/paperclip-runner/docs/phase-07-verification-commands.md +++ b/packages/paperclip-runner/docs/phase-07-verification-commands.md @@ -15,7 +15,7 @@ acceptance record are in | 106-case conformance | `test:phase7-evals` | 1 file (all 106 cases) passes | | Fake-agent matrix + bounded Codex + parity report | `report:phase7-evals` | `Phase 7 eval conformance passed: 106 cases across 16 groups.` | | Scenario runtime + explorer + clean room + routes | `test:phase7` | 159 tests pass | -| Browser IA, accessibility, determinism, boundary | `test:browser:phase7` | Playwright suites pass (58 in the issue-thread/clean-room suite) | +| Browser IA, accessibility, determinism, boundary | `test:browser:phase7` | Playwright suites pass (60 in the issue-thread/clean-room suite) | | Screenshot acceptance set | `record:phase7` | 24 deterministic images | | Doc links + OKF bundle | `docs:validate` | 73 files, 28 OKF concepts, 5 indexes |