mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-03 22:13:19 +08:00
Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
77cef014cf | ||
|
|
a32bb65d45 | ||
|
|
e6bfd30da3 |
@@ -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.
|
||||
@@ -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.
|
||||
@@ -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).
|
||||
|
||||
@@ -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
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user