Skip to content

test(extension): respect chat readiness in queue scenarios - #473

Merged
steipete merged 1 commit into
mainfrom
test/chat-readiness
Sep 12, 2026
Merged

test(extension): respect chat readiness in queue scenarios#473
steipete merged 1 commit into
mainfrom
test/chat-readiness

Conversation

@steipete

Copy link
Copy Markdown
Owner

The chat queue tests bypassed browser actionability with direct DOM clicks. Settings could be hydrated while daemon readiness was still pending, leaving chat hidden and correctly ignoring the synthetic send. This produced intermittent zero-request failures in CI.

Use one Playwright helper for button and keyboard submission. The first queue scenario now holds daemon readiness explicitly, verifies that hidden chat receives neither input nor requests, releases readiness, and checks visible queued content before releasing the first response. A page-command barrier makes that ordering observable. Existing assertions and timeout limits remain; no product behavior or test skips changed.

The controlled gate reproduced the same zero-request failure with the old helper. Both queue scenarios then passed five repetitions each with the fix. Full check and isolated Codex autoreview through P2 pass. Live proof ran the built extension in the supported Chromium test harness, and a real Chrome fixture using the production send handler confirmed hidden submissions are ignored while visible user actions are accepted.

@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 12, 2026
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 7:43 PM ET / 23:43 UTC.

ClawSweeper review

What this changes

The PR updates two Chrome extension chat queue tests to use browser interactions and explicitly checks submission waiting while daemon readiness is delayed.

Merge readiness

Ready for maintainer review

This remains a useful, focused test repair: current main and the latest release retain the synthetic submission helper that bypasses chat readiness. No actionable regression was found. Repository policy and owner authorship also preclude automatic closure.

Priority: P3
Reviewed head: f822e3375c45d2f716fc955363c7091554d78f42

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused test repair consistent with the production readiness contract, with no concrete correctness or security finding.
Proof confidence 🌊 off-meta tidepool Not applicable: The OWNER-authored test repair is exempt from the external-contributor proof gate; the captured body reports repeated built-extension queue runs and a Chrome send-handler fixture, without independently inspected runtime artifacts.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The OWNER-authored test repair is exempt from the external-contributor proof gate; the captured body reports repeated built-extension queue runs and a Chrome send-handler fixture, without independently inspected runtime artifacts.
Evidence reviewed 7 items Applicable repository guidance: Read the full root AGENTS.md; no nested AGENTS.md or maintainer notes were found under apps or .agents. The guidance identifies Chromium as the supported extension test path.
Verified introduced scope: The pinned base-to-head diff changes only this test file: 51 additions and 45 deletions. It replaces synthetic submissions in two queue scenarios and adds a readiness gate without changing production behavior.
Production readiness contract: Chat availability requires both the chat setting and daemon feature availability; the production submission handler returns immediately when chat is unavailable. The existing page-opening helper waits for settings hydration, which does not establish daemon readiness.
Findings None None.
Security None None.

How this fits together

The extension side panel accepts questions about page content and queues additional questions while a response streams. These browser tests control daemon readiness and responses to check when submissions enter that queue.

flowchart TD
  A[Browser test] --> B[Hold daemon readiness]
  B --> C[Chat remains hidden]
  C --> D[Browser input waits]
  B --> E[Release readiness]
  E --> D
  D --> F[Send or queue question]
  F --> G[Response completes and queue drains]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change footprint Production +0/-0; tests +51/-45; 2 scenarios updated The repair remains confined to browser test interactions and synchronization.

Technical review

Best possible solution:

Keep queue coverage aligned with browser actionability while preserving the production readiness guard and deterministic response gating.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection establishes the race: settings hydration can finish before daemon readiness, and the old synthetic click reaches a handler that ignores unavailable chat. Runtime reproduction was reported by the author but not executed in this review.

Is this the best way to solve the issue?

Yes. Browser interactions respect the existing visibility boundary, and the controlled readiness gate exercises the race without weakening production guards or skipping queue assertions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b2606720c9bf.

Labels

Label changes:

  • add P3: This is a bounded test reliability improvement with no production behavior change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The OWNER-authored test repair is exempt from the external-contributor proof gate; the captured body reports repeated built-extension queue runs and a Chrome send-handler fixture, without independently inspected runtime artifacts.

Label justifications:

  • P3: This is a bounded test reliability improvement with no production behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The OWNER-authored test repair is exempt from the external-contributor proof gate; the captured body reports repeated built-extension queue runs and a Chrome send-handler fixture, without independently inspected runtime artifacts.

Evidence

What I checked:

  • Applicable repository guidance: Read the full root AGENTS.md; no nested AGENTS.md or maintainer notes were found under apps or .agents. The guidance identifies Chromium as the supported extension test path. (AGENTS.md:26, f822e3375c45)
  • Verified introduced scope: The pinned base-to-head diff changes only this test file: 51 additions and 45 deletions. It replaces synthetic submissions in two queue scenarios and adds a readiness gate without changing production behavior. (apps/chrome-extension/tests/sidepanel.chat.spec.ts:22, f822e3375c45)
  • Production readiness contract: Chat availability requires both the chat setting and daemon feature availability; the production submission handler returns immediately when chat is unavailable. The existing page-opening helper waits for settings hydration, which does not establish daemon readiness. (apps/chrome-extension/src/entrypoints/sidepanel/interaction-runtime.ts:35, f822e3375c45)
  • Fixture isolation: Each test launches its own extension harness and closes it in finally. The readiness wrapper delegates to the existing fetch mock after release, so its lifetime is confined to the test browser context. (apps/chrome-extension/tests/helpers/extension-fixtures.ts:12, f822e3375c45)
  • Latest release still contains the old helper: The v0.21.14 test source still assigns the input value and calls the button directly through page.evaluate. The pinned main diff likewise confirms that the proposed repair is not already present there. (apps/chrome-extension/tests/sidepanel.chat.spec.ts:60, 30e5ddda4429)
  • Area history and routing: Available main history lists Peter Steinberger on the test fixture consolidation and several preceding extension test and runtime changes. A deeper follow-history read failed on an unavailable historical object; no feature-introduction attribution is claimed. (apps/chrome-extension/tests/sidepanel.chat.spec.ts, 8308778d4e30)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 5b1f563 into main Sep 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant