Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions openspec/changes/improve-cli-e2e-plan/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## Why
Recent cross-shell regressions for `openspec` commands revealed that our existing unit/integration tests do not exercise the packaged CLI or shell-specific behavior. The prior attempt at Vitest spawn tests stalled because it coupled e2e coverage with `pnpm pack` installs, which fail in network-restricted environments. With those findings incorporated, we now need an approved plan to realign the work.

## What Changes
- Adopt a phased strategy that first stabilizes direct spawn testing of the built CLI (`node dist/cli/index.js`) using lightweight fixtures and shared helpers.
- Expand coverage to cross-shell/OS matrices once the spawn harness is stable, ensuring both the direct `node dist/cli/index.js` invocation and the bin shim are exercised with non-TTY defaults and captured diagnostics.
- Treat packaging/install validation as an optional CI safeguard: when a runner has registry access, run a simple pnpm-based pack→install→smoke-test flow; otherwise document it as out of scope while closing remaining hardening items.

## Impact
- Tests: add `test/cli-e2e` spawn suite, helpers, and fixture usage updates; adjust `vitest.setup.ts` as needed.
- Tooling: update GitHub Actions workflows to add shell/OS matrices and (optionally) a packaging install check where network is available.
- Docs: keep `CROSS-SHELL-PLAN.md` aligned with the phased rollout and record any limitations called out during execution.
13 changes: 13 additions & 0 deletions openspec/changes/improve-cli-e2e-plan/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
## 1. Phase 1 – Stabilize Local Spawn Coverage
- [ ] 1.1 Update `vitest.setup.ts` and helpers so the CLI build runs once and `runCLI` executes `node dist/cli.js` with non-TTY defaults.
- [ ] 1.2 Reuse the minimal fixture set (`tmp-init` or copy) to seed initial spawn tests for help/version, a happy-path `validate`, and a representative error flow.
- [ ] 1.3 Document the Phase 1 coverage details in `CROSS-SHELL-PLAN.md`, noting any outstanding gaps.

## 2. Phase 2 – Expand Cross-Shell Validation
- [ ] 2.1 Exercise both entry points (`node dist/cli.js`, `bin/openspec.js`) in the spawn suite and add diagnostics for shell/OS context.
- [ ] 2.2 Extend GitHub Actions to run the spawn suite across a matrix of shells (bash, zsh, fish, pwsh, cmd) on macOS, Linux, and Windows runners.

## 3. Phase 3 – Package Validation (Optional)
- [ ] 3.1 Add a simple CI job on runners with registry access that runs `pnpm pack`, installs the tarball into a temp workspace (e.g., `pnpm add --no-save`), and executes `pnpm exec openspec --version`.
- [ ] 3.2 If network-restricted environments can’t exercise installs, document the limitation in `CROSS-SHELL-PLAN.md` and skip the job there.
- [ ] 3.3 Close out remaining hardening items from the original cross-shell plan (e.g., `.gitattributes`, chmod enforcement, SIGINT follow-ups) and update the plan accordingly.
19 changes: 19 additions & 0 deletions openspec/changes/update-markdown-parser-crlf/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Update Markdown Parser CRLF Handling

## Problem
Windows users report that `openspec validate` raises “Change must have a Why section” even when the section exists (see GitHub issue #77). The CLI currently splits markdown on `\n` and compares headers without stripping `\r`, so files saved with CRLF line endings keep a trailing carriage return in the header token. As a result the parser fails to detect `## Why`/`## What Changes`, triggering false validation errors and breaking the workflow on Windows-default editors.

## Solution
- Normalize markdown content inside the parser so CRLF and lone-CR inputs are treated as `\n` before section detection, trimming any carriage returns from titles and content comparisons.
- Reuse the normalized reader everywhere `MarkdownParser` is constructed to keep behavior consistent for validation, view, spec, and list flows.
- Add regression coverage that reproduces the failure (unit test around `parseChange` and a CLI spawn/e2e test that writes a CRLF change then runs `openspec validate`).
- Update the `cli-validate` spec to codify the expectation that required sections are recognized regardless of line-ending style.

## Benefits
- Restores correct validation behavior for Windows editors without requiring manual line-ending conversion.
- Locks in the fix with targeted tests so future parser refactors keep cross-platform support.
- Clarifies the spec so downstream work (e.g., cross-shell e2e plan) understands the non-negotiable behavior.

## Risks
- Low: parser normalization touches shared code paths that parse specs and changes; need to ensure no regressions in other command consumers (mitigated by existing parser tests plus the new CRLF fixtures).

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
## ADDED Requirements
### Requirement: Parser SHALL handle cross-platform line endings
The markdown parser SHALL correctly identify sections regardless of line ending format (LF, CRLF, CR).

#### Scenario: Required sections parsed with CRLF line endings
- **GIVEN** a change proposal markdown saved with CRLF line endings
- **AND** the document contains `## Why` and `## What Changes`
- **WHEN** running `openspec validate <change-id>`
- **THEN** validation SHALL recognize the sections and NOT raise parsing errors
11 changes: 11 additions & 0 deletions openspec/changes/update-markdown-parser-crlf/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## 1. Guard the regression
- [ ] 1.1 Add a unit test that feeds a CRLF change document into `MarkdownParser.parseChange` and asserts `Why`/`What Changes` are detected.
- [ ] 1.2 Add a CLI spawn/e2e test that writes a CRLF change, runs `openspec validate`, and expects success.

## 2. Normalize parsing
- [ ] 2.1 Normalize line endings when constructing `MarkdownParser` so headers and content comparisons ignore `\r`.
- [ ] 2.2 Ensure all CLI entry points (validate, view, spec conversion) reuse the normalized parser path.

## 3. Document and verify
- [ ] 3.1 Update the `cli-validate` spec with a scenario covering CRLF line endings.
- [ ] 3.2 Run the parser and CLI test suites (`pnpm test`, relevant spawn tests) to confirm the fix.
Loading