Repository navigation
test: restore real ColorPicker and CustomSelect exports after wizard step tests (TA-001 round 2) - #2340
Conversation
…step tests (TA-001 round 2)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly describes the test-only change, identifies the restored ColorPicker and CustomSelect exports, and includes the relevant TA-001 scope. |
| Description check | |
The description follows the repository template and documents the change, root cause, impact, testing commands and results, pre-existing failures, and known limitations. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Risk Surface Disclosed | |
PASS: The PR changes only wizardSteps.test.tsx. The diff restores real ColorPicker and Select test exports after mock.module use. It does not change permissions, auth, provider routing, networ… |
| No Hidden Policy Change | |
PASS — The PR changes only wizardSteps.test.tsx (+16/-1). The changes add cached real test-module exports and restore them in afterAll, while retaining the existing test doubles. No product code, … |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
Full details: Risk Surface Disclosed
-
Explanation
PASS: The PR changes only
wizardSteps.test.tsx. The diff restores realColorPickerandSelecttest exports aftermock.moduleuse. It does not change permissions, auth, provider routing, network behavior, background execution, startup/config-home behavior, skills/plugins/MCP, CI permissions, or release scripts. TheMonitorPermissionRequestreference is only downstream test context, not a changed permission implementation, so no risk-surface disclosure or blocker is required.
Full details: No Hidden Policy Change
-
Explanation
PASS — The PR changes only
wizardSteps.test.tsx(+16/-1). The changes add cached real test-module exports and restore them inafterAll, while retaining the existing test doubles. No product code, routing defaults, telemetry/network behavior, trust model, or permission policy changes are present. No maintainer policy alignment is required.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
— jatmn's PR Review agent
This is a surgical TA-001 round 2 test-isolation slice. I do not see any code defects in the reviewed diff on head d092bb96.
Merge readiness
Rebase onto current main before merge. Live target main is one commit ahead of this branch (90b34cbd — merged TA-001 settings/sleep/fast-mode/loadSkillsDir slice #2339). Merge-base decision for this review was live-base-three-dot at a63ad4a9..d092bb96 (single commit, one file). GitHub reports mergeable: true and mergeable_state: blocked while required checks finish.
CI on head d092bb96: typecheck, all eight unit-tests shards, dedicated-tests, launcher compatibility/floor jobs, pr-intent-scan, changes, and all CodeQL jobs (CodeQL, CodeQL (actions), CodeQL (javascript-typescript), Analyze (javascript-typescript)) completed success. web skipped. smoke-and-tests was still in_progress at review capture time — confirm green before merge. Commit combined status had shown CodeRabbit context pending even though the PR already has an empty APPROVED CodeRabbit review.
Scope: Stands alone — one commit, one test file, no dependency on other open PRs. No product or docs surface changes; appropriate for the stated MonitorPermissionRequest / randomize bisect.
Code review: The ?real= export snapshots plus afterAll mock.module re-registration match the established TA-001 pattern and fix the merge-base cross-file stub leak I reproduced locally (wizard steps then MonitorPermissionRequest without --isolate).
Summary
src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx.mock.moduleto replace../../ColorPicker.jsand../../../CustomSelect/select.jswith lightweight stubs.mock.restore()inafterEachdoes not undomock.moduleregistrations, so both stubs stayed registered for the rest of the process. Later files that render the realCustomSelectgot the stub instead. Inbun test --randomize --seed=2027without--isolate, this caused all 5 interactiveMonitorPermissionRequesttests to fail with "Timed out waiting for Monitor permission test condition". It also caused failures in ExportDialog, sessions, AgentRouteSelector and a few other files that ran later.?real=<nonce>), then re-register those copies inafterAll. This is the same pattern earlier TA-001 slices used. Test bodies, assertions and the per-test cleanup are unchanged.Impact
--isolateruns.--isolatestays in CI.Testing
CONTRIBUTING.mdandAGENTS.md.bun install --frozen-lockfile: pass.bun run typecheck: pass.bun run typecheck:type-tests: pass.node bin/openclaude --versionandNODE_DISABLE_COMPILE_CACHE=1 node bin/openclaude --version: pass.bun run security:pr-scan -- --base FETCH_HEAD --head HEAD: pass.bun run check: pass. An earlier run failed one timing-sensitiveloadSkillsDir.test.tswatcher test (".agents linked target edits refresh invoked content and obey source lifetime", notification count 4 vs 3). That file is not touched here and does not use these mocks. It passed 3 times in a row on its own with--isolate, and the fullbun run checkrerun passed.bun test ./src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx ./src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.test.tsx: 9 pass, 0 fail (before the fix: 5 MonitorPermissionRequest failures).bun test ./src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx: 3 pass.--isolateruns:bun test --shard=K/8): 1066/0, 1530/0, 1373/1, 1492/0, 1070/4, 1459/0, 1287/1, 1182/1 (pass/fail). No new failures compared with currentmain. Shard 8 went from 6 failures to 1.--randomize --seed=2027: 10399 pass, 55 fail. Before the fix: 10366 pass, 88 fail. There are no new failures. 33 tests that failed before now pass: MonitorPermissionRequest 5, ExportDialog 13, sessions 7, AgentRouteSelector 3, ConsoleOAuthFlow 2, and search, AgentDetailDialog and exaFree 1 each.--randomize --seed=101and--seed=6001: both runs hang in an unrelatedloadSkillsDir.test.tslock wait and hit the runner timeout, as they do onmain. Before that point, all 6 MonitorPermissionRequest tests passed in both runs.main. They belong to other TA-001 round 2 slices.Notes
Summary by CodeRabbit