Skip to content

Fix Windows workspace open CI timeout - #1134

Merged
TabishB merged 1 commit into
mainfrom
codex/fix-main-windows-ci-workspace-open
May 28, 2026
Merged

Fix Windows workspace open CI timeout#1134
TabishB merged 1 commit into
mainfrom
codex/fix-main-windows-ci-workspace-open

Conversation

@TabishB

@TabishB TabishB commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate workspace opener override conflicts before workspace target selection
  • split the Windows-sensitive workspace open error-path test into smaller focused cases
  • add an empty changeset because this is CI/test stability only

Verification

  • pnpm run build
  • pnpm exec vitest run test/commands/workspace.test.ts -t "reports workspace open selection errors|reports unsupported workspace open options before workspace selection|reports unset and unavailable workspace opener errors"
  • pnpm exec vitest run test/commands/workspace.test.ts
  • pnpm exec tsc --noEmit
  • pnpm lint
  • pnpm exec changeset status --since=origin/main
  • pnpm test

Note

The original failure is only fully proven fixed after the post-merge main push matrix runs on Windows, because PR CI currently runs tests on Ubuntu only.

Summary by CodeRabbit

  • Refactor
    • Optimized workspace open command handling for improved performance and code maintainability.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7b4760fd-3560-4a0e-81c4-aa2e7857688c

📥 Commits

Reviewing files that changed from the base of the PR and between 313ca71 and 928e706.

📒 Files selected for processing (2)
  • src/commands/workspace/open-view.ts
  • test/commands/workspace.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/commands/workspace/open-view.ts
  • test/commands/workspace.test.ts

📝 Walkthrough

Walkthrough

This PR optimizes workspace opening by precomputing the opener override once instead of inline, and refactors error tests from a single combined test into three focused test blocks for workspace selection errors, unsupported options, and opener failures.

Changes

Workspace Opener Override Refactoring

Layer / File(s) Summary
Workspace opener override optimization
src/commands/workspace/open-view.ts
prepareWorkspaceOpen now resolves resolveWorkspaceOpenOpenerOverride(options) once into a local openerOverride variable and passes it to selectOrCreateWorkspaceForInitiativeOpen instead of invoking the resolver inline.
Error coverage test restructuring
test/commands/workspace.test.ts
"Workspace open" error tests are split into three focused it blocks: workspace selection errors (no known workspaces, ambiguity), unsupported options (--prepare-only, --change), and opener-related failures (no preferred opener, unavailable opener command).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Fission-AI/OpenSpec#1054: Directly implements workspace-open opener preference and override behavior that this PR refactors.

Suggested reviewers

  • alfred-openspec

Poem

🐰 A speedy hop through opener's way,
The override computes once per day,
Tests now split with clarity and grace,
Three focused checks in proper place!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Fix Windows workspace open CI timeout' refers to fixing a CI timeout issue, but the changeset is specifically about refactoring error handling and splitting tests for workspace open functionality, not directly fixing a timeout. Consider a more accurate title like 'Refactor workspace open error handling and split tests for CI stability' that reflects the actual implementation changes and intent.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-main-windows-ci-workspace-open

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@TabishB
TabishB enabled auto-merge May 28, 2026 06:46
@TabishB
TabishB requested a review from alfred-openspec May 28, 2026 06:49
@TabishB
TabishB force-pushed the codex/fix-main-windows-ci-workspace-open branch from 313ca71 to 928e706 Compare May 28, 2026 06:51

@alfred-openspec alfred-openspec 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.

Looks good. Small targeted CI-stability fix: validates conflicting opener overrides before workspace selection and splits the slow/error-path workspace open test coverage without changing the command contract.

@TabishB
TabishB added this pull request to the merge queue May 28, 2026
Merged via the queue into main with commit 11b2690 May 28, 2026
12 checks passed
@TabishB
TabishB deleted the codex/fix-main-windows-ci-workspace-open branch May 28, 2026 06:57
linjinze999 pushed a commit to linjinze999/OpenSpec that referenced this pull request Jun 4, 2026
studyzy pushed a commit to studyzy/OpenSpec-cn that referenced this pull request Jul 1, 2026
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