From 52c58eabc1c8bd472ebfd121fa4ce93da73cfa78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Val=20Alexander=20=F0=9F=91=91?= Date: Sun, 1 Nov 2026 09:19:50 -0600 Subject: [PATCH] Verify Code Review activity opening and comment link controls --- apps/desktop/e2e/code-review-overview.spec.ts | 29 +++++++++++++++++++ .../2026-11-01-cr-activity-controls.md | 9 ++++++ docs/status.md | 2 ++ 3 files changed, 40 insertions(+) create mode 100644 docs/reviews/2026-11-01-cr-activity-controls.md diff --git a/apps/desktop/e2e/code-review-overview.spec.ts b/apps/desktop/e2e/code-review-overview.spec.ts index ddf91079..c13589e7 100644 --- a/apps/desktop/e2e/code-review-overview.spec.ts +++ b/apps/desktop/e2e/code-review-overview.spec.ts @@ -46,6 +46,35 @@ async function open(hostile = false, mode = "normal") { await page.locator('[data-testid="code-review-row"][data-pr="example-org/lending#73"]').first().click(); } test.afterEach(async () => { await app?.close(); }); +test("activity opening and comment link preserve their identity and keyboard focus", async () => { + await open(); + await app.evaluate(({ ipcMain }) => { + (globalThis as any).__copiedCommentLinks = []; + ipcMain.removeHandler("clipboard:write"); + ipcMain.handle("clipboard:write", (_event, request) => { + (globalThis as any).__copiedCommentLinks.push(request.text); + }); + }); + const opening = page.locator('[data-activity-id="opened:opened-73"]'); + await expect(opening.locator("strong")).toHaveText("ada-fixture"); + await expect(opening).toContainText("opened this pull request"); + await expect(opening.locator("time")).toHaveAttribute("datetime", "2026-10-18T15:44:00Z"); + const card = page.locator('[data-activity-id="comment:comment-9001"]'); + await expect(card.locator("header strong")).toHaveText("preview-bot[bot]"); + await expect(card).toContainText("Comment content"); + const actions = card.getByRole("button", { name: "Comment actions", exact: true }); + await actions.focus(); + await page.keyboard.press("Enter"); + const copy = page.getByRole("menuitem", { name: "Copy comment link", exact: true }); + await copy.focus(); + await page.keyboard.press("Enter"); + await expect.poll(() => app.evaluate(() => (globalThis as any).__copiedCommentLinks)).toEqual([ + "https://github.com/example-org/lending/pull/73#issuecomment-9001", + ]); + await expect(card.getByRole("status")).toHaveText("Comment link copied"); + await expect(actions).toBeFocused(); + await expect(page.getByRole("menu", { name: "Comment actions", exact: true })).toHaveCount(0); +}); test("R1 metadata and R2 pagination stay truthful, deduplicated and keyboard accessible", async () => { await open(); await expect(page.getByRole("heading", { name: R1.overview.title, exact: true })).toBeVisible(); diff --git a/docs/reviews/2026-11-01-cr-activity-controls.md b/docs/reviews/2026-11-01-cr-activity-controls.md new file mode 100644 index 00000000..e77f1114 --- /dev/null +++ b/docs/reviews/2026-11-01-cr-activity-controls.md @@ -0,0 +1,9 @@ +# Code Review activity controls: C50 and C51 + +Candidate based on main `172dcd282704dcf3fa4d76f26d242f5c3074a294`. This adds real Electron behavioral evidence for two gaps identified in the CR-8 matrix: the opening row and the bot comment menu’s Copy comment link. No application behavior changes. + +The test opens the R1/R2 synthetic GitHub fixture, asserts the opening actor, wording and timestamp, and identifies the bot comment. It uses the keyboard to open Comment actions and activate Copy comment link. It checks the exact renderer-to-main clipboard payload (`https://github.com/example-org/lending/pull/73#issuecomment-9001`), the copied status, menu closure and focus returning to the comment actions button. Clipboard IPC is intercepted inside the fixture, so the test does not write to the person's system clipboard. This is Electron/fixture evidence, not authenticated GitHub or human accessibility acceptance. + +The focused test initially passed. Two separate negative controls modified only the built renderer bundle: incorrect opening text failed the row assertion, and an incorrect comment fragment failed the clipboard payload assertion. Each exited 1 for its intended assertion, and the original bundle was restored after each run. Logs are preserved outside the worktree under `Documents/Codex/artifacts/modex-land-and-prune/cr-activity-*`. + +The restored focused test passed. Independent review found no blockers and confirmed that intercepted clipboard IPC is fixture-local. Programmatic focus verifies keyboard activation and focus return, not Tab reachability. `pnpm build && pnpm test` passed with 2,277 unit passes and 26 skips. The full `pnpm test:e2e` run passed on its first run: 479 desktop passes, 7 skips (22.3 minutes), and 7 site passes (12.1 seconds). The new test also passed within that full run. The five-launch cold transcript median was 93.4 ms against the unchanged 500 ms gate on Apple M5 Max, macOS arm64, Node v24.22.0. Reference comparison and VoiceOver remain open; this change does not complete #327 or the full C51 human gate. diff --git a/docs/status.md b/docs/status.md index 0c544692..edca489b 100644 --- a/docs/status.md +++ b/docs/status.md @@ -233,6 +233,8 @@ The published application source is `6a97d23aaea315b29008890f25dde2d7ea07f1d0` ( ## In flight +Code Review C50/C51 behavioral coverage is ready for review: an Electron fixture asserts the opening row and the comment menu’s exact copied URL, status, closure and keyboard focus return. Independent review found no blockers; build and unit validation passed (2,277 passes, 26 skips), and full E2E passed (479 desktop passes, 7 skips; 7 site passes). Reference comparison and VoiceOver remain open. [Evidence](reviews/2026-11-01-cr-activity-controls.md). + - **Deferred Markdown for long threads (hosted cold-open budget).** Hosted CI's approved 500 ms cold-open gate failed on runner variance alone: 33 hosted runs on 2026-10-30 had a 377.7 ms median and a 781.9 ms failure on application code identical to a 436.1 ms pass. Val chose on 2026-10-30 to defer offscreen answers. Turns before the newest eight show their Markdown as written until it nears the view, then parse once; scroll anchoring keeps the reader's place. Locally the five-launch median fell from about 190 to 88 ms, and 3× CPU throttling cut it from 806–870 to 312–340 ms. The gate is unchanged. The first hosted run had a 129.9 ms median (481 desktop E2E passes). It landed through #463 (`32f1cb4`) on 2026-10-31 UTC and is unreleased. [Evidence](reviews/2026-10-30-deferred-markdown.md). Desktop CI memory diagnostics landed through #458 after #456’s cold transcript budget failure. The workflow reuses the existing resource sampler and retains its log as a separate CI artifact; it does not establish the cause of the earlier failure or change the 500 ms gate. The #458 queue passed desktop and site E2E and retained its memory log. See [the diagnostic review](reviews/2026-10-30-desktop-ci-memory.md).