Skip to content

[codex] test(ui): wait for Watchdog menu item - #1314

Merged
kkroo merged 1 commit into
masterfrom
codex/stabilize-watchdog-editor-test
Aug 14, 2026
Merged

[codex] test(ui): wait for Watchdog menu item#1314
kkroo merged 1 commit into
masterfrom
codex/stabilize-watchdog-editor-test

Conversation

@kkroo

@kkroo kkroo commented Aug 11, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip gives teams a dependable control plane for coordinating AI-agent work.
  • The New Issue dialog exposes optional automation controls such as a Watchdog.
  • That control is loaded asynchronously through the experimental-settings query.
  • The existing test waited only one scheduler turn before finding the menu item.
  • On a slower React Query notification, the UI was correct but the test observed it too early and failed.
  • This pull request makes the test wait for the rendered menu item without changing production behavior.
  • The benefit is a stable signal for real regressions instead of intermittent CI noise.

Linked Issues or Issue Description

Type: Bug / test reliability

What happened: NewIssueDialog.test.tsx could fail to find the Watchdog menu item after a single flush() even though the async settings query subsequently rendered it.

Expected behavior: The test should wait for the observable UI state that it asserts rather than assume one event-loop turn is sufficient.

Reproduction: Run pnpm exec vitest run ui/src/components/NewIssueDialog.test.tsx under a schedule where React Query's notification follows the test's one-tick flush. The prior failure was observed in the workspaces-a CI shard while validating #1031.

Version / deployment mode: Current master checkout; local Vitest component-test execution. No production deployment behavior is affected.

Related work: #1031 exposed the baseline test race. A search across the open PR heads found no existing PR that changes this test or its component.

What Changed

  • Re-query the Watchdog menu item inside the existing waitForAssertion helper.
  • Keep the assertion and click behavior unchanged after the item becomes visible.

Verification

  • pnpm exec vitest run ui/src/components/NewIssueDialog.test.tsx — 26 passed.
  • Reviewed the diff: one test file only; no product code or UI behavior changed.

Risks

Low risk. This is a test-only synchronization change. It could conceal a permanently absent item only until the helper timeout, after which the same assertion still fails with the relevant UI state.

Model Used

OpenAI Codex (GPT-5), tool-using coding agent with reasoning enabled; used for diagnosis, patch generation, and focused test execution.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • This is test-only; no user-visible UI behavior changed, so screenshots are not applicable
  • I have updated relevant documentation to reflect my changes (not applicable for a test-only synchronization change)
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

  • Missing section: ## Thinking Path
  • Missing section: ## What Changed
  • Missing section: ## Verification
  • Missing section: ## Risks
  • Missing section: ## Model Used
  • No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
  • Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

@kkroo
kkroo marked this pull request as ready for review August 11, 2026 22:20
@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 6498f4c

Looks good. The test now waits for the observable Watchdog menu item instead of assuming one flush is enough. The assertion remains bounded by the existing retry helper, and production behavior is unchanged.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The change is narrowly scoped to the asynchronous UI condition that the test actually depends on.
  • The existing helper preserves a useful failure signal if the menu item never appears.

Recommended Action

  1. Merge when the remaining required checks pass.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 6498f4c

Looks good. The test now waits for the observable Watchdog menu item instead of assuming one flush is enough. The assertion remains bounded by the existing retry helper, and production behavior is unchanged.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The change is narrowly scoped to the asynchronous UI condition that the test actually depends on.
  • The existing helper preserves a useful failure signal if the menu item never appears.

Recommended Action

  1. Merge when the remaining required checks pass.

@kkroo

kkroo commented Aug 14, 2026

Copy link
Copy Markdown
Author

Merge exception evidence for exact reviewed head 6498f4c:

  • Scope is limited to ui/src/components/NewIssueDialog.test.tsx, changing the Watchdog button lookup to use the existing bounded waitForAssertion helper.
  • Ally approved this exact head with 0 Critical and 0 Important findings.
  • General tests (server 3/4) failed only in unrelated server test src/tests/plugin-orchestration-apis.test.ts (1 failed / 1,269); this PR does not touch plugin orchestration code or tests.
  • E2E failed after the runner received a shutdown signal; the Smoke Lab test was then interrupted with Request context disposed. That is runner/infrastructure termination, unrelated to this UI unit-test change.
  • All other substantive checks passed; aggregate verify only reflects those two unrelated failures.

Proceeding with an admin squash merge under the requested infrastructure/baseline exception.

@kkroo
kkroo merged commit c33be6e into master Aug 14, 2026
16 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants