Skip to content

Conversation

@TabishB
Copy link
Contributor

@TabishB TabishB commented Jan 20, 2026

Summary

  • Fix failing tests on Windows by using path.join() in test expectations instead of hardcoded forward slashes
  • Add openspec/config.yaml with cross-platform requirements to prevent similar issues in future proposals

Problem

The getProjectSchemasDir tests were failing on Windows CI:

Expected: "/path/to/project/openspec/schemas"
Received: "\path\to\project\openspec\schemas"

The function correctly uses path.join() which produces OS-specific separators, but the tests hardcoded forward slashes.

Solution

  1. Test fix: Use path.join() for expected values so tests pass on all platforms
  2. Prevention: Add openspec/config.yaml that injects cross-platform requirements into all OpenSpec proposals, including:
    • Global context reminding that this tool runs on Windows, macOS, and Linux
    • Requirement to use path.join() for file paths
    • Per-artifact rules for specs/tasks/design to consider Windows compatibility

Test plan

  • Verify tests pass locally
  • Windows CI should now pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Enhanced cross-platform compatibility with improved file path handling across Windows, macOS, and Linux.
  • Tests

    • Updated tests to verify OS-agnostic file path behavior and cross-platform consistency.
  • Chores

    • Added development configuration defining platform-specific requirements and best practices.

✏️ Tip: You can customize this high-level summary in your review settings.

- Use path.join() in test expectations instead of hardcoded forward slashes
- Add openspec/config.yaml with cross-platform requirements to prevent
  similar issues in future proposals

The tests were failing on Windows because path.join() uses backslashes
on Windows, but the test expectations hardcoded forward slashes.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 20, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

A new configuration file openspec/config.yaml was added to define cross-platform requirements and context metadata for the project, emphasizing OS-agnostic path handling. Test assertions were updated to use path.join() instead of hard-coded POSIX paths for compatibility.

Changes

Cohort / File(s) Change Summary
Configuration Schema
openspec/config.yaml
New spec-driven configuration file defining context (TypeScript, Node.js ≥20.19.0, ESM, pnpm, Commander.js), cross-platform path requirements (use path.join(), avoid hardcoded slashes, handle case sensitivity), and Windows CI verification task.
Test Path Fixes
test/core/artifact-graph/resolver.test.ts
Replaced hard-coded POSIX path strings with path.join() calls in two test assertions for platform-agnostic directory path construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 Paths across platforms, no more hardcoded woes,
Join the journey, wherever the OS goes,
Windows and Unix, now hand in paw,
Config wisdom, a cross-platform law! 🌍✨

✨ Finishing touches
  • 📝 Generate docstrings

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 merged commit 32fc19a into main Jan 20, 2026
9 of 10 checks passed
@TabishB TabishB deleted the fix/windows-path-compatibility branch January 20, 2026 02:42
@vibe-kanban-cloud
Copy link

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

@greptile-apps
Copy link

greptile-apps bot commented Jan 20, 2026

Greptile Summary

Fixed Windows CI test failures by aligning test expectations with cross-platform path behavior.

Key Changes:

  • Updated getProjectSchemasDir tests to use path.join() for expected values, matching the function's implementation
  • Added openspec/config.yaml with cross-platform development guidelines to prevent similar issues

Technical Details:
The getProjectSchemasDir function correctly uses path.join() to construct paths with OS-appropriate separators (\ on Windows, / on Unix). The tests were failing on Windows because they expected hardcoded forward slashes. The fix ensures tests work on all platforms by using path.join() to generate platform-specific expectations.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are minimal, well-focused, and correctly address the Windows path compatibility issue. The test fix properly mirrors the implementation's use of path.join(), and the config.yaml addition provides valuable preventive guidance without affecting runtime behavior.
  • No files require special attention

Important Files Changed

Filename Overview
test/core/artifact-graph/resolver.test.ts Fixed Windows path compatibility by using path.join() in test expectations instead of hardcoded forward slashes
openspec/config.yaml Added project configuration with cross-platform path handling requirements and best practices to prevent future issues

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Func as getProjectSchemasDir()
    participant Path as path.join()
    participant OS as Operating System
    
    Test->>Func: Call with '/path/to/project'
    Func->>Path: join('/path/to/project', 'openspec', 'schemas')
    Path->>OS: Detect platform
    alt Windows (win32)
        OS-->>Path: Use backslash separator
        Path-->>Func: Return '\path\to\project\openspec\schemas'
    else Unix (darwin/linux)
        OS-->>Path: Use forward slash separator
        Path-->>Func: Return '/path/to/project/openspec/schemas'
    end
    Func-->>Test: Return platform-specific path
    Test->>Path: Generate expected value with path.join()
    Path-->>Test: Platform-specific expected path
    Test->>Test: Compare actual vs expected ✓
Loading

StrayDragon pushed a commit to StrayDragon/OpenSpec that referenced this pull request Jan 20, 2026
- Use path.join() in test expectations instead of hardcoded forward slashes
- Add openspec/config.yaml with cross-platform requirements to prevent
  similar issues in future proposals

The tests were failing on Windows because path.join() uses backslashes
on Windows, but the test expectations hardcoded forward slashes.
harikrishnan83 added a commit to intent-driven-dev/OpenSpec that referenced this pull request Jan 21, 2026
# By Tabish Bidiwale (57) and others
# Via GitHub
* main: (67 commits)
  fix(ci): use workflow_dispatch for polish release notes (Fission-AI#533)
  fix(changelog): convert markdown headers to bold text for proper formatting (Fission-AI#532)
  Version Packages (Fission-AI#531)
  Add changeset for project config and schema commands (Fission-AI#530)
  fix(config): handle null rules field in project config (Fission-AI#529)
  docs: update workflow docs and mark schema commands as experimental (Fission-AI#526)
  feat(cli): add schema management commands (Fission-AI#525)
  fix: Windows path compatibility in resolver tests (Fission-AI#524)
  change(schema-management-cli): proposal for schema management commands (Fission-AI#523)
  feat(resolver): add project-local schema support (Fission-AI#522)
  docs: add project-config demo guide (Fission-AI#521)
  feat(config): add project-level configuration via openspec/config.yaml (Fission-AI#499)
  fix: auto-trigger polish release notes on release publish (Fission-AI#519)
  perf: add path filtering to Nix validation CI job (Fission-AI#518)
  Version Packages (Fission-AI#517)
  Add changeset for v0.21 release (Fission-AI#516)
  fix: prevent implementation during explore mode (Fission-AI#515)
  OPSX apply: infer target change (Fission-AI#513)
  Refine opsx archive sync assessment (Fission-AI#514)
  feat: add nix flake support (sorry for this duplicate) (Fission-AI#459)
  ...

# Conflicts:
#	src/core/templates/slash-command-templates.ts
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