fix(shell): kill the CI-WebKit forms-detail search race and un-skip ui-tools:264 - #201
Merged
Conversation
…I WebKit The shared search shell (GlobalMockupSearchShellClient) re-synced its mode + query from the URL inside a mount requestAnimationFrame. On a no-query detail route like /forms/transport-crisis-form the deferred frame could land just after a programmatic/user fill and reset the controlled input to empty. On slow single-CPU CI Linux WebKit that reliably lost the value: the composer stayed focused-but-empty, the "Search forms" submit stayed disabled, and the search never routed — so ui-tools.spec.ts:264 was skipped on WebKit (PR #182/#186). Replace the rAF sync with a URL-gated one: seed the composer mode/query from the URL at initial state, and re-sync only when the search string actually changes (a real navigation), tracked via lastSyncedSearchParamsRef. Because typing never changes the URL and the initial mount is a no-op, no deferred frame can clobber an in-progress fill regardless of browser timing. This retires the interim isDetailPage / previousUrlHadQueryRef special-casing from PR #186. With the race fixed at the source, remove the `if (browserName === "webkit")` early-return (and its comment) from ui-tools.spec.ts:264 so all three browsers verify the full submit-and-route wiring; the fill-and-submit toPass harness stays as defensive cross-browser navigation-timing cover. Update docs/process-hardening.md to record the resolution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Temporary focused reproduction so the shell fix can be verified on CI Linux WebKit (the only place the forms-detail search race surfaced) without the full ~70 min release-browser-matrix. Runs just tests/ui-tools.spec.ts on WebKit. Remove once the fix is confirmed green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The forms-detail shared-search fix is confirmed green on CI Linux WebKit (ui-tools.spec.ts:264 passes on chromium/firefox/webkit), so the temporary push-triggered check has served its purpose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d0dbdf37e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo added a commit
that referenced
this pull request
Aug 5, 2026
Add #237–#239 from the fastest-wins planning session, repair mislabeled queue rows (#201/#202 vs #183/#185), remove archived #182 from the queue, and enqueue every previously open-but-unqueued item including P1s #207, #226, and #231. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
4 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 7, 2026
* fix: close high-win Grok batch (tokens, a11y, perf, gates) - Drop unused --med-accent-soft (#157) - Document Chip compact=11px / standard=12px (#220) - Reveal phone chrome on source change at top (#176) - Passive wheel listener for modifier-gated PDF zoom (#214) - Point npm run typecheck at source-only tsconfig (#210) - Fail outstanding-issues when queue cites non-open IDs (#201) Also confirms MatchExplanationChips, Favourites empty-state, DocumentFrame role=alert, and refetching dimming claim are already resolved on main (#223/#225/#219/#246). * issues: archive grok-batch wins #157#176#201#210#214#219#220#223#225#246 Prune them from the recommended queue so the new queue-membership gate stays green. * fix: address PR #1651 review (double-zoom, issues:done, tokens) - Keep viewer wheel listener non-passive while wheelZoom is on and preventDefault on Ctrl/⌘+wheel / trackpad pinch so PDF zoom does not also zoom the browser page (Sentry/Devin). - Prune recommended-queue ID citations in resolveIssue so issues:done stays writable under the #201 gate. - Drop restated Chip pixel values from TOKENS.md; amend #214 archive note. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: recompute issues-queue Order boundary after deletions Devin: pruneResolvedIdFromQueue used a pre-splice openStart for renumbering, so enough solo-queue deletions could rewrite Open items headers/ids. Recompute the section limit after edits and cover it in the writer self-test. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: record Run PR sweep ledger for PR #1651 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #178/#180/#186.
tests/ui-tools.spec.ts:264("form detail pages keep the shared forms search wired to form results") was skipped on WebKit (if (browserName === \"webkit\") return;) because it failed only on CI Linux WebKit and could not be reproduced on local Windows/macOS WebKit.Root cause.
GlobalMockupSearchShellClientre-synced its composer mode + query from the URL inside a mountrequestAnimationFrame. On a no-query detail route like/forms/transport-crisis-form, the deferred frame could land after the test's programmaticfilland reset the controlled input to empty. On slow single-CPU CI Linux WebKit this reliably lost the value: the composer stayed focused-but-empty, the Search forms submit stayeddisabled, and the search never routed. #186 added an interimisDetailPageguard but stayed[WIP]and left the WebKit skip in place.Fix. Replace the rAF sync with a URL-gated one:
resolvedSearchMode) and query from the URL at initial state.lastSyncedSearchParamsRef(seeded to the current string, so the initial mount is a no-op).Because typing never changes the URL and the mount is a no-op, no deferred frame can clobber an in-progress fill — regardless of browser timing. This also retires the interim
isDetailPage/previousUrlHadQueryRefspecial-casing from #186. Net diff vsmainis the shell change, the skip removal, and the doc update (a temporary push-triggered WebKit CI workflow was added for verification and then removed in the same branch).With the race fixed at the source, the
browserName === \"webkit\"early-return and its comment are removed fromui-tools.spec.ts:264; thetoPassfill-and-submit harness stays as defensive cross-browser navigation-timing cover.Verification
tsc --noEmit), ESLint (incl.react-hooks/exhaustive-deps), and Prettier — green on the changed files.run-vitest) — 867 passed / 2 skipped.ui-tools.spec.tsacross chromium/firefox/webkit via a temporary push-triggered workflow — 63 passed, 0 failed, 0 skipped. The formerly-skipped test now runs and passes on WebKit:✓ [webkit] tests/ui-tools.spec.ts:264 › form detail pages keep the shared forms search wired to form results (6.3s).npm run verify:cheap/verify:ui/verify:releasewrappers — not run as single commands from the isolated worktree (turbopack rejects the linkednode_modules); their substantive gates (lint, typecheck, unit tests, and the WebKit UI path) were run individually as above. The fullrelease-browser-matrixruns automatically once this lands onmain.Clinical Governance Preflight
N/A — UI-only change to the mockup search-shell composer state on
/forms,/services,/favourites,/differentials,/medications,/applications,/mockups. No ingestion, answer generation, retrieval/ranking, source rendering, document access, privacy, or production/Supabase behavior is touched.Notes
searchModefrom the URL preserves the previous rAF behavior for hard-loads with?mode=while removing the flash; the one tested focus path (/favourites→ "Start a new chat" →?focus=1) is a client navigation, so its focus still fires through the URL-change sync.docs/process-hardening.mdupdated with the resolution under "Cross-browser test robustness".