fix(ui): sync cron validity + settle branch test for radix-ui 1.6.4 (#9966)
## Thinking Path > - Paperclip is the open-source app people use to manage AI agents for work > - The `ui` package contains React components that drive the agent scheduling and workspace configuration UX > - Dependabot PR #9895 bumped `@radix-ui/react-*` from 1.6.0 → 1.6.4, which changed the internal effect-scheduling order inside Dialog and Select primitives > - Two `workspaces-a` CI tests started failing: `RoutineRunVariablesDialog` and `editable-sections / TriggersSection` > - Root cause for both: radix 1.6.4's changed scheduling pushes a cascaded state update one render-tick later, and each test asserted before that tick landed > - This PR fixes the two affected surfaces at the source (one production fix, one test fix) so the radix bump can land cleanly > - The benefit is unblocking PR #9895 without compromising test fidelity or production correctness ## Linked Issues or Issue Description Refs #9895 — this PR fixes the two `workspaces-a` test failures that blocked the radix-ui 1.6.0 → 1.6.4 Dependabot bump. **Root cause:** radix-ui 1.6.4 changed the internal effect-scheduling order inside its Dialog and Select primitives, pushing certain cascaded state updates one render-tick later than before. Two tests each asserted against the intermediate state, before the deferred tick landed. **Affected tests (both now pass at radix 1.6.0 AND 1.6.4):** 1. `editable-sections / TriggersSection` — `ScheduleEditor` disabled-button assertion 2. `RoutineRunVariablesDialog` — workspace branch propagation assertion Production behavior is unchanged and correct in both cases (verified via tracing). ## What Changed - **`ui/src/components/ScheduleEditor.tsx`** — `onValidityChange` is now called synchronously inside the custom-cron `onChange` handler, not only via the passive `useEffect` below. Previously there was a one-render window where an invalid draft still read as valid to the parent (button enabled); this closes that window. The effect call is preserved as a safety net for other entry paths; only the `onChange` path is new. - **`ui/src/components/RoutineRunVariablesDialog.test.tsx`** — the post-mount settle loop now waits for the branch value to actually appear in an `<input>` (`value === "pap-1634-routine-branch"`) rather than exiting as soon as the workspace card mounts. The card reports its branch name through an effect callback that triggers a follow-up render; the old loop exited one tick too early. Iteration cap raised from 10 → 20 to give the extra tick room. This PR intentionally does **not** bump radix-ui — that stays in #9895. ## Verification ```sh # TypeScript — clean at radix 1.6.0 (master): tsc -p ui/tsconfig.json # Targeted vitest: npx vitest run ui/src/components/RoutineRunVariablesDialog.test.tsx npx vitest run ui/src/components/editable-sections npx vitest run ui/src/components/ScheduleEditor # Full ui suite — green with radix 1.6.4 installed locally (371 files / 3035 tests): npx vitest run --project ui # check-forbidden-tokens — clean ``` All of the above pass at **both** radix 1.6.0 (current master) and 1.6.4. ## Risks Low risk. The production change (`ScheduleEditor.tsx`) adds a synchronous call to an already-injected `onValidityChange` prop — same value, earlier in the same event cycle. No new state, no new effects, no API changes. The test change tightens an assertion (waits longer, checks a more specific condition) rather than relaxing one. ## Model Used Claude Sonnet (Anthropic) — Paperclip agent workflow; model family claude-sonnet-4-x with tool use and extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Harold Kim <harold@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
b565603a86
commit
6439572ecd
|
|
@ -393,7 +393,15 @@ describe("RoutineRunVariablesDialog", () => {
|
|||
);
|
||||
});
|
||||
|
||||
for (let i = 0; i < 10 && !document.querySelector('[data-testid="workspace-card"]'); i += 1) {
|
||||
// The workspace card mounts once experimental settings resolve, then reports its
|
||||
// branch name through an effect callback. That callback triggers a follow-up render,
|
||||
// so wait for the branch value itself to land — not merely for the card to appear —
|
||||
// otherwise we assert against the intermediate render before the branch propagates.
|
||||
const hasBranchInput = () =>
|
||||
Array.from(document.querySelectorAll("input")).some(
|
||||
(input) => input.value === "pap-1634-routine-branch",
|
||||
);
|
||||
for (let i = 0; i < 20 && !hasBranchInput(); i += 1) {
|
||||
await settleEffects();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -266,7 +266,13 @@ export function ScheduleEditor({
|
|||
onChange={(e) => {
|
||||
const nextCron = e.target.value;
|
||||
setCustomCron(nextCron);
|
||||
if (getScheduleCronValidation(nextCron).valid) {
|
||||
// Report validity synchronously with the keystroke so consumers can gate
|
||||
// their submit affordance in the same render. Relying solely on the
|
||||
// effect below leaves a one-tick window where an invalid draft still
|
||||
// reads as valid to the parent.
|
||||
const nextValidation = getScheduleCronValidation(nextCron);
|
||||
onValidityChange?.(nextValidation.valid);
|
||||
if (nextValidation.valid) {
|
||||
emitChange("custom", hour, minute, dayOfWeek, dayOfMonth, nextCron);
|
||||
}
|
||||
}}
|
||||
|
|
|
|||
Loading…
Reference in New Issue