🤖 Optimize integration test suite performance (59% faster) - #24
Merged
Merged
Conversation
Fix timer leak in StreamManager cleanup: - Clear partialWriteTimer in processStreamWithCleanup finally block to prevent keeping process alive after stream completion. - The timer was being set in schedulePartialWrite but wasn't being cleared during stream cleanup, which could keep the Node.js event loop active for up to 500ms (PARTIAL_WRITE_THROTTLE_MS) after tests completed. Fix TypeScript errors in renameWorkspace.test.ts: - Add proper type guards and casts for event data filtering - Use CmuxMessage type for messages with role and id properties - Add null checks before 'role' in e.data check This ensures all timers are properly cleared when a stream ends, whether it completes successfully, errors, or is cancelled. Note: Integration tests may still have a ~15s delay after completion due to HTTP keep-alive connections in the Anthropic SDK, which is expected behavior and not a resource leak.
Reduced CI integration test runtime from ~86s to ~35s (59% improvement). Key optimizations: 1. Fixed git repo creation race condition using fs.mkdtemp 2. Batched git commands to reduce shell exec overhead 3. Enabled test.concurrent for all integration tests 4. Configured jest with forceExit to avoid hanging workers 5. Used 50% of available CPU cores for parallelization All tests now run concurrently within each test file, taking full advantage of jest's parallel test runner. _Generated with `cmux`_
Integration tests now run in ~35 seconds (down from ~2 minutes), so we no longer need to warn about avoiding them. _Generated with `cmux`_
PamelaSprin47685ghall
pushed a commit
to PamelaSprin47685ghall/mux
that referenced
this pull request
Jun 12, 2026
## Summary Adds an explicit visual **Open** affordance to active workflow task rows while preserving the row as the single accessible navigation target. Completed task rows with report markdown continue to show **Report** instead of **Open**. ## Background Running workflow sub-agent rows were already clickable, but that navigation path was implicit compared with completed rows that visibly expose a **Report** action. This change makes the active-task inspection path obvious without adding a duplicate keyboard stop. ## Implementation - Adds a started-task helper and renders a right-aligned visual-only **Open** span for active task rows without report markdown. - Keeps the workflow task row as the only role/button navigation target, including keyboard activation. - Extends tests to assert the visual affordance is aria-hidden, no standalone **Open** button exists, and the active task row remains role/query/keyboard accessible. - Adds a Storybook `TaskActions` fixture for dogfooding active **Open** and completed **Report** behavior. ## Deep review Ran `deep-review-workflow` against `origin/main...HEAD`. It identified three issues, all fixed before PR creation: 1. The test clicked the visual-only **Open** span instead of the accessible row. 2. The new Storybook story ignored Storybook-provided args. 3. An unrelated scratch workflow `.gitignore` hunk was present in the branch diff. ## Validation - `bun test src/browser/features/Tools/WorkflowRunToolCall.test.tsx` - `scratch-gitignore-precedence` local check - `make static-check` - Storybook dogfooding with `agent-browser` screenshots/video captured in the workspace run. ## Risks Low. The visible affordance is gated to started task rows without reports, and the existing row navigation/report dialog paths are preserved. --- <details> <summary>📋 Implementation Plan</summary> # Plan: Add an explicit active-agent Open affordance to workflow task rows ## Recommendation Ship a small, active-row-only **Open** affordance for workflow task rows. - **Recommended approach — in-row visual CTA, single accessible target**: add a button-styled `Open` affordance inside active task rows as a non-interactive `span`, while keeping the existing row as the single semantic `role="button"` / keyboard target. This makes the call to action visible without introducing a duplicate tab stop for the same navigation action. **Estimated product LoC: +25 to +45.** - **Fallback approach — literal sibling button**: add a right-aligned `<button>` sibling that calls the existing navigation handler for active rows. This is slightly smaller, but creates two adjacent keyboard targets with the same action unless extra focus management is added. I would not choose it unless implementation complexity unexpectedly rises. **Estimated product LoC: +10 to +25.** - Do **not** add both `Open` and `Report` to completed rows with reports. For those rows, keep `Report` as the visible primary action and preserve row-click navigation. ## Verified context - `src/browser/features/Tools/WorkflowRunToolCall.tsx` contains `WorkflowTaskRow`, which already makes workflow task rows clickable/focusable and navigates via `workspaceStore.navigateToWorkspace(taskId)`. - Completed task rows with non-empty report markdown already render a right-aligned `Report` button that opens a report dialog. - Active/started task rows are navigable today, but the affordance is implicit: the row cursor/role changes, yet there is no visible CTA comparable to `Report`. - `src/browser/features/Tools/WorkflowRunToolCall.test.tsx` already covers coalesced task rows, row navigation, and report-dialog behavior. - `src/browser/features/Tools/WorkflowRunToolCall.stories.tsx` has useful workflow states for visual validation, including running/discovered runs. - Requested skills reviewed for validation planning: `dogfood`, `agent-browser`, and `dev-server-sandbox`. ## UX intent Make the running sub-agent inspection path obvious while keeping the event list compact. Expected visual model: ```text coder#23 TASK summarize-source-15 / 7b1a07d84d / completed Report coder#24 PHASE claim-extraction coder#25 TASK extract-claims / a36921beca / started Open ``` ## Implementation steps 1. **Define active task status helper** - Add a local helper near the workflow row utilities in `WorkflowRunToolCall.tsx`, e.g. `shouldShowOpenAffordance(event)`. - Start narrowly with `event.type === "task" && event.status === "started"` unless the workflow event type already includes additional active states that should be represented. - Keep the helper local; no shared abstraction unless another caller exists. 2. **Render active-row Open affordance** - In `WorkflowTaskRow`, compute: - `taskReportMarkdown` - `hasReportAction = taskReportMarkdown != null` - `showOpenAffordance = !hasReportAction && shouldShowOpenAffordance(event)` - For `showOpenAffordance`, render an `Open` label styled with `WORKFLOW_ACTION_BUTTON_CLASS` inside the clickable task row as a non-focusable visual affordance, preferably a `span` with `aria-hidden` and `pointer-events-none` so the parent row remains the hit target. - Switch the row to a four-column layout only when the `Open` affordance is shown, so the task label truncates cleanly and `Open` stays right-aligned. - Keep the row itself as the semantic and keyboard-accessible control (`role="button"`, `tabIndex={0}`, Enter/Space behavior, existing `aria-label`). This avoids two tab stops for the same action. - Preserve the existing `Report` dialog button path for rows with report markdown. 3. **Preserve click behavior** - Clicking anywhere in the active task row, including the visible `Open` affordance, should call the existing `activateTaskRow()` path. - Clicking `Report` must continue opening the report dialog only and must not navigate. 4. **Update tests** - Extend the existing `coalesces task attempts, navigates rows, and opens completed reports separately` test in `WorkflowRunToolCall.test.tsx` or add a focused adjacent test. - Assert the active `task_retry` row visibly contains `Open`, but do not query it as a separate role button. - Click the active row / visual `Open` area and assert navigation receives `task_retry`. - Assert completed `task_live` still displays `Report`, opens the report dialog, and does not navigate when the report button is clicked. - Add a no-duplicate-accessibility check if practical: active rows should have one semantic row/button target for navigation, not row + separate focusable duplicate. 5. **Optional story polish only if needed** - If current Storybook stories do not make the visual before/after obvious enough, add or adjust a small story state in `WorkflowRunToolCall.stories.tsx` with one started task and one completed task with a report. - Avoid broad Storybook fixture churn. ## Acceptance criteria - Active workflow task rows visibly show `Open`. - Completed workflow task rows with reports still visibly show `Report` and do not also show `Open`. - Row click and keyboard navigation still open the workflow task workspace. - Clicking `Report` still opens the report dialog without navigating. - The event list remains compact at narrow widths; the task label truncates before overlapping the action affordance. - No new manual memoization, speculative abstractions, or unrelated styling changes. ## Validation plan Run these after implementation: ```bash bun test src/browser/features/Tools/WorkflowRunToolCall.test.tsx make static-check ``` If Storybook fixtures are touched, also run the relevant Storybook validation available in the repo, or at minimum start Storybook and visually inspect the workflow run stories. ## Dogfooding plan Use the requested skills as follows: 1. **Prepare the app/story environment** - Prefer Storybook for fast visual validation of this component state: ```bash make storybook # or, if needed: bun run storybook ``` - For full app validation, use an isolated sandbox rather than the developer's normal profile: ```bash make dev-server-sandbox # or desktop E2E if implementation needs the Electron shell: make dev-desktop-sandbox ``` - Before browser automation, load the installed agent-browser command guide: ```bash agent-browser skills get core ``` 2. **Capture evidence with agent-browser** - Open the Storybook `App/Chat/Tools/WorkflowRun` story that contains an active task row. - Capture an annotated screenshot showing the active task row with `Open`. - Click the `Open` affordance / row and capture evidence that the navigation action is triggered or, in a full app sandbox, that the child workspace opens. - Open a completed-with-report state and capture an annotated screenshot showing `Report` without `Open`. - Click `Report`, capture a screenshot of the report dialog, and record a short video of the interaction so reviewers can verify the behavior. 3. **Quality gates between phases** - After unit tests pass, take the Storybook screenshot before running broader static checks. - After `make static-check` passes, dogfood in the sandbox and collect final screenshot/video evidence. ## Risks and mitigations - **Duplicate accessibility targets**: avoid a second focusable `Open` button for the same action by making the row remain the semantic control and using the `Open` text as an in-row visual affordance. - **Visual clutter**: only show `Open` for active task rows without reports; do not add it to every task row. - **Action ambiguity**: keep `Report` and `Open` mutually exclusive in the visible action slot. - **Narrow layout overflow**: keep the task label truncating and action affordance non-shrinking/right-aligned. </details> --- _Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `440972{MUX_COSTS_USD:-unknown}`_ <!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=14.51 -->
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Reduced CI integration test runtime from ~86s to ~35s (59% improvement).
Performance Results
Key Optimizations
1. Fixed Git Repository Creation
Fixed race condition and reduced shell overhead in
tests/ipcMain/helpers.ts:fs.mkdtemp()instead ofDate.now()to avoid "File exists" errors2. Enabled Parallel Test Execution
Changed all integration tests to use
test.concurrent():sendMessage.test.ts(22 tests)renameWorkspace.test.ts(8 tests)truncate.test.ts(3 tests)createWorkspace.test.ts(2 tests)Jest now runs tests in parallel within each file, not just across files.
3. Optimized Jest Configuration
maxWorkersfrom4to"50%"(dynamic based on CPU cores)forceExit: trueto prevent hanging on lingering async operationsdetectOpenHandles4. Updated Documentation
CLAUDE.mdwith new performance metricsVerification
Ran 3 consecutive test runs to verify consistency:
✅ All lint checks pass
✅ All 165 unit tests pass
✅ All 40 integration tests pass
✅ No new warnings or errors
CI Impact
Files Changed
jest.config.js- Optimized worker configurationtests/ipcMain/helpers.ts- Fixed git repo creationtests/ipcMain/*.test.ts- Enabled concurrent executionCLAUDE.md- Updated documentationGenerated with
cmux