diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ef3d41..c65671c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.2.0] - 2026-06-22 +## [0.3.0] - 2026-06-25 Sync with upstream OpenSpec `v1.2.0` → `v1.4.1`, plus a parity audit of the whole port against upstream that fixed several pre-existing interop/correctness @@ -20,11 +20,16 @@ gaps. - **Workspaces (beta)** — `openspec workspace `, with `.openspec-workspace/view.yaml` view state, a managed `workspaces/registry.yaml`, AGENTS.md guidance + `.code-workspace` generation, and per-tool workspace skills - `openspec templates [--schema ] [--json]` — show resolved template paths per artifact - `openspec schema ` — inspect, validate, copy, and scaffold workflow schemas +- `openspec feedback [--body ]` — file feedback as a GitHub issue via the `gh` CLI, with a manual issue-URL fallback when `gh` is missing or unauthenticated - `openspec set change --initiative ` — link a repo-local change to an initiative ### Changed +- `config` now uses upstream-style subcommands (`path`, `list`, `get`, `set`, `unset`, `reset`, `edit`, `profile`) instead of the old flat `--set/--get/--list` flags. This is a breaking CLI surface change. +- `new change` now supports upstream parity flags: `--json`, `--goal`, `--affected-areas`, `--initiative`, `--store`, and `--store-path` +- `show --json` now supports spec-filter flags `--requirements`, `--requirement `, and `--no-scenarios`; those flags are JSON-only and warn when used on a change +- `validate` now supports bounded parallelism via `--concurrency ` and matches upstream strictness — ERROR (not warning) when a spec has no requirements or a requirement lacks SHALL/MUST or a scenario; change validation detects duplicate and cross-section delta conflicts; `--strict` makes warnings fail; plus a hint when SHALL/MUST appears only in a requirement header +- Global `--no-interactive` is accepted as a no-op for upstream CLI compatibility - Spec parser: requirement headers parsed case-insensitively; **Markdown headers inside fenced code blocks are ignored** (not parsed as requirements/scenarios), matching upstream -- `validate`: now matches upstream strictness — ERROR (not warning) when a spec has no requirements or a requirement lacks SHALL/MUST or a scenario; change validation detects duplicate and cross-section delta conflicts; `--strict` makes warnings fail; plus a hint when SHALL/MUST appears only in a requirement header - `show --json`: emits the upstream JSON shape (camelCase `requirementCount`/`deltaCount`/`rawText`, spec `metadata` block, rich change deltas with `spec`/`operation`/`requirement`/`rename`) - Global `config.json` written with upstream camelCase keys (`featureFlags`, `anonymousId`, `noticeSeen`); snake_case still read for back-compat - Shell completion install is opt-in @@ -36,10 +41,10 @@ gaps. - `--json` output no longer leaks spinner text to stderr - `context-store setup --path

` refuses a path inside an existing Git repository (git-nesting guard), with a `--allow-inside-git-repository` escape hatch - `archive` re-validates each merged main spec before writing it (honoring `--no-validate`), so a bad merge can no longer silently corrupt specs +- Built-in package-schema resolution now uses the embedded `spec-driven` definition instead of a dev-only `vendor/OpenSpec/schemas` path, so installed binaries work from arbitrary directories ### Known gaps -- `openspec feedback` and the interactive `openspec view` dashboard are not ported -- Package-schema resolution uses a dev-only `vendor/OpenSpec/schemas` path; installed binaries fall back to the embedded schema +- The interactive `openspec view` dashboard is not ported ## [0.1.4] - 2026-03-15 diff --git a/Cargo.lock b/Cargo.lock index 369fa24..1356a55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -730,7 +730,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" [[package]] name = "openspec" -version = "0.2.0" +version = "0.3.0" dependencies = [ "anyhow", "atty", diff --git a/Cargo.toml b/Cargo.toml index c17cd29..cdd3a1a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openspec" -version = "0.2.0" +version = "0.3.0" edition = "2021" rust-version = "1.75" description = "AI-native system for spec-driven development" diff --git a/README.md b/README.md index cc42db4..b2b56c6 100644 --- a/README.md +++ b/README.md @@ -107,17 +107,18 @@ AI: Archived. Specs updated. | Command | Description | |---------|-------------| | `init [path]` | Initialize OpenSpec in a project | -| `new change ` | Create a new change directory | +| `new change ` | Create a new change directory; supports `--json`, `--goal`, `--affected-areas`, `--initiative`, `--store`, and `--store-path` | | `list` | List changes (or specs with `--specs`) | | `status` | Show artifact completion status | | `instructions [artifact]` | Get instructions for creating artifacts | | `schemas` | List available workflow schemas | -| `show [item]` | Show a change or spec | -| `validate [item]` | Validate changes and specs | +| `show [item]` | Show a change or spec; `--json` supports `--requirements`, `--requirement `, and `--no-scenarios` | +| `validate [item]` | Validate changes and specs with `--all`, `--changes`, `--specs`, `--strict`, and `--concurrency ` | | `archive [change]` | Archive a completed change | +| `feedback ` | File feedback as a GitHub issue (via `gh`, with a manual-URL fallback) | | `update [path]` | Update AI tool instruction files | -| `config` | View and modify configuration | -| `completion` | Generate shell completions | +| `config` | View and modify configuration via `path`, `list`, `get`, `set`, `unset`, `reset`, `edit`, and `profile` | +| `completion` | Generate or install shell completions | | `context-store` | Set up and inspect local context stores (beta) | | `initiative` | Create and inspect coordinated initiatives (beta) | | `workspace` | Set up and inspect coordination workspaces (beta) | @@ -125,6 +126,8 @@ AI: Archived. Specs updated. | `schema` | Inspect, validate, fork, or init workflow schemas | | `set change` | Link a repo-local change to an initiative | +Global compatibility flags: `--no-color` and `--no-interactive` are accepted on all commands; `--no-interactive` is a no-op in the Rust CLI. + ## Coordination: Context Stores, Initiatives & Workspaces (beta) For work that spans multiple repositories, OpenSpec-rs adds three coordination diff --git a/docs/STATE.md b/docs/STATE.md index e5f3c47..5318d45 100644 --- a/docs/STATE.md +++ b/docs/STATE.md @@ -244,6 +244,7 @@ Location: `openspec/changes/archive/2026-03-15-port-to-rust-binary/` | Version | Date | Notes | |---------|------|-------| +| v0.3.0 | 2026-06-25 | CLI parity with upstream v1.4.1: config subcommands (breaking), feedback (GitHub issue), schema fork/init, templates, set change, show spec-filters, validate --concurrency, --no-interactive, embedded package-schema resolution | | v0.2.0 | 2026-06-20 | Sync to upstream v1.4.1: new AI tools, parser/validation/completion/telemetry fixes, macOS dir interop, and context-store / initiatives / workspace subsystems | | v0.1.4 | 2026-03-15 | Documentation complete, port-to-rust-binary archived, main specs created | | v0.1.3 | 2026-03-14 | Rust 1.75 MSRV, pinned dependencies, GitHub Actions updates | @@ -251,15 +252,15 @@ Location: `openspec/changes/archive/2026-03-15-port-to-rust-binary/` | v0.1.1 | 2026-03-14 | Fix CI build before tests | | v0.1.0 | 2026-03-14 | Initial release | -## Current Version: 0.2.0 +## Current Version: 0.3.0 - MSRV: Rust 1.75+ -- Synced to upstream OpenSpec v1.4.1 (see CHANGELOG `[0.2.0]`) -- New AI tools and `detectionPaths` auto-detection; opt-in completion install; firewall-silent telemetry -- Global config/data dirs aligned with upstream on macOS +- Synced to upstream OpenSpec v1.4.1 (see CHANGELOG `[0.2.0]` and `[0.3.0]`) +- Full CLI parity with upstream v1.4.1: config subcommands (breaking change), feedback (files a GitHub issue), schema fork/init, templates, set change, show spec-filters, validate --concurrency + OPENSPEC_CONCURRENCY, global --no-interactive +- Package-schema resolution uses the embedded `spec-driven` definition (no dev-only vendor path) - New subsystems: context-store, initiatives, workspace (beta) — with CLI commands - All 6 skills embedded: explore, propose, apply-change, archive-change, verify-change, sync-specs -- Change `sync-upstream-v1-4-1` lives in `openspec/changes/` (archive after release) +- Known gaps: interactive `view` dashboard, and deprecated `spec`/`change` noun command groups, not ported ## See Also diff --git a/openspec/changes/cli-parity-upstream/.openspec.yaml b/openspec/changes/cli-parity-upstream/.openspec.yaml new file mode 100644 index 0000000..38f7628 --- /dev/null +++ b/openspec/changes/cli-parity-upstream/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-22 diff --git a/openspec/changes/cli-parity-upstream/design.md b/openspec/changes/cli-parity-upstream/design.md new file mode 100644 index 0000000..35b794d --- /dev/null +++ b/openspec/changes/cli-parity-upstream/design.md @@ -0,0 +1,49 @@ +## Context + +The Rust port reached behavioral + on-disk parity with upstream v1.4.1 in v0.2.0. The remaining differences are CLI-surface only: missing commands/flags identified by the parity audit. This change closes them. The port is intentionally non-interactive and flag-driven; upstream interactive flows become flags or are no-ops. + +## Goals / Non-Goals + +**Goals:** +- Full CLI command/flag parity with upstream v1.4.1 for the in-scope items. +- Keep on-disk formats unchanged (config.json already camelCase; `.openspec.yaml` initiative link already defined). + +**Non-Goals:** +- The `view` interactive TUI dashboard (separate effort if ever wanted). +- Re-adding deprecated noun command groups (`spec`/`change`) or the `experimental` alias — upstream deprecates them and the verb-first equivalents exist. + +## Decisions + +### 1. `config` → subcommands (BREAKING) +Replace the flat `Config { set, get, list }` with a `Config(ConfigCommands)` group: `Path | List {json} | Get {key} | Set {key,value} | Unset {key} | Reset {all} | Edit | Profile {preset}`. Rationale: match upstream exactly; the flat form has no upstream analogue and confuses users switching tools. Document the break in CHANGELOG. `edit` shells out to `$EDITOR`/`$VISUAL` (fallback to a sensible default). + +### 2. `feedback` reuses the telemetry transport +`feedback ` posts via the existing telemetry client (PostHog) under the same opt-out gating (`OPENSPEC_TELEMETRY`/`DO_NOT_TRACK`/CI). No new network stack. If disabled, it no-ops with a clear message. *Alternative:* a dedicated endpoint — rejected (reuse existing, respect existing consent). + +### 3. `--no-interactive` as a global no-op clap flag +Add a global `--no-interactive` (like `--no-color`) so any command accepts it without error. Simplest faithful adaptation of upstream's per-command flag. *Alternative:* per-command flags — more surface, no benefit here. + +### 4. `new change` flags reuse existing infra +`--initiative`/`--store`/`--store-path` reuse the initiatives resolution + the `set change` linking logic (write `initiative` into `.openspec.yaml`). `--goal`/`--affected-areas` add the optional ChangeMetadata fields upstream defines. `--json` prints `{id, path}`. + +### 5. `show` spec filters operate on parsed JSON +`--requirements` / `--requirement ` / `--no-scenarios` filter the already-parsed spec model before serializing (mirror upstream `filterSpec`). JSON-only (ignored for change/human output, with a warning like upstream). + +### 6. `validate --concurrency ` +Bound parallel validation with a worker pool (default mirrors upstream). Determinism of aggregated output preserved (sort results). + +### 7. Package-schema resolution fix +`get_package_schemas_dir` currently returns the hardcoded relative `vendor/OpenSpec/schemas`. Change resolution so the built-in schema comes from the embedded definition (already in the binary) rather than a path that only exists in the dev checkout; keep project/user schema dirs. This makes `schema which`/`templates` correct for installed users. + +## Risks / Trade-offs + +- **config BREAKING change** → mitigation: clear CHANGELOG note + matching upstream reduces long-term confusion; pre-1.0 so acceptable. +- **`config edit` shelling to an editor** → not unit-testable end-to-end; factor editor-command resolution into a testable helper, gate the actual spawn. +- **Schema-resolution change** → ensure `schemas`/`schema which`/`templates`/`validate` still work in-repo and installed; cover with tests using an isolated dir. + +## Migration Plan +Land per command group (config, feedback, show flags, new-change flags, validate concurrency, no-interactive, schema fix), each with the full CI gate (fmt + clippy --all-targets -D warnings + test) and `validate --all` 8/8+. Update CHANGELOG (note the config break) and README command table. + +## Open Questions +- `config edit` default editor on each platform when `$EDITOR`/`$VISUAL` unset (vi vs notepad) — match upstream's choice. +- Exact `feedback` payload/event name upstream uses (confirm against `cli/index.ts` feedback action during implementation). diff --git a/openspec/changes/cli-parity-upstream/proposal.md b/openspec/changes/cli-parity-upstream/proposal.md new file mode 100644 index 0000000..cc07d83 --- /dev/null +++ b/openspec/changes/cli-parity-upstream/proposal.md @@ -0,0 +1,32 @@ +## Why + +After the v1.2→v1.4.1 sync and parity audit, the Rust port matches upstream OpenSpec on behavior and on-disk formats, but a set of **CLI surface gaps** remain: commands and flags present in the npm CLI (v1.4.1) that the Rust binary is missing. These break muscle memory and scripts for users switching between the two. This change closes the remaining gaps to reach full CLI parity with upstream v1.4.1. + +(Upstream v1.4.1 is the latest release; this is not a new version sync — it's the unported remainder identified by the audit.) + +## What Changes + +**In scope — parity gaps to close:** +- **`config` subcommands**: replace the flat `--set/--get/--list` with upstream's subcommand shape — `config path | list | get | set | unset | reset [--all] | edit | profile [preset]`. +- **`feedback `**: submit anonymous feedback (respects telemetry opt-out / `--body`). +- **`show` spec-filter flags**: `--requirements` (requirements only), `--requirement ` (one requirement, 1-based), `--no-scenarios`. +- **`new change` flags**: `--json`, `--goal`, `--affected-areas`, and initiative linking `--initiative`/`--store`/`--store-path`. +- **`validate --concurrency `**: bounded parallel validation. +- **`--no-interactive`**: accepted as a no-op wherever upstream defines it (the Rust port is already non-interactive), so scripts passing it don't error. +- **Fix: package-schema resolution** — stop resolving package schemas from the dev-only `vendor/OpenSpec/schemas` path; installed binaries SHALL resolve the embedded schema (and any project/user schemas), so `schema which`/`templates` report correct paths for end users. + +**Out of scope (documented decisions, not gaps to fix here):** +- **`view`** interactive TUI dashboard — a large, different class of work (full-screen terminal UI) with low value for a scriptable CLI port; deferred to its own change if ever wanted. +- Deprecated noun command groups `spec ` and `change `, and the deprecated `experimental` alias — upstream emits deprecation warnings for these; the Rust port exposes the verb-first equivalents (`show`, `validate`, `list`) already. + +## Capabilities + +### Modified Capabilities +- `cli-commands`: add `feedback`; `show` spec-filter flags; `new change` flags (`--json`/`--goal`/`--affected-areas`/`--initiative`/`--store`/`--store-path`); `validate --concurrency`; accept `--no-interactive` as a no-op; correct package-schema resolution for installed binaries. +- `config-management`: `config` gains the upstream subcommand surface (path/list/get/set/unset/reset/edit/profile) replacing the flat flags. + +## Impact + +- **Affected code**: `src/cli/args.rs` (clap surface for config/new/show/validate/feedback + global `--no-interactive`), `src/cli/config.rs` (subcommand rewrite), `src/cli/show.rs` (filter flags), `src/cli/new_change.rs` (flags + initiative link, reusing the initiatives resolution), `src/cli/validate.rs` (concurrency), a new `src/cli/feedback.rs`, and `src/core/schema.rs` (package-schema resolution). +- **Compatibility**: `config` changes are the one BREAKING surface change (flat `--set/--get/--list` → subcommands) — it aligns with upstream; document in CHANGELOG. Everything else is additive. +- **Reference**: upstream `vendor/OpenSpec/` at v1.4.1 (`commands/config.ts`, `commands/spec.ts` filters, `cli/index.ts` feedback/new/templates). diff --git a/openspec/changes/cli-parity-upstream/specs/cli-commands/spec.md b/openspec/changes/cli-parity-upstream/specs/cli-commands/spec.md new file mode 100644 index 0000000..67296fa --- /dev/null +++ b/openspec/changes/cli-parity-upstream/specs/cli-commands/spec.md @@ -0,0 +1,92 @@ +## MODIFIED Requirements + +### Requirement: CLI New Change Command + +The system SHALL provide an `openspec new change ` command that creates a new change directory, supporting JSON output and optional initiative linking. + +#### Scenario: Create new change +- **WHEN** user runs `openspec new change add-dark-mode` +- **THEN** system creates `openspec/changes/add-dark-mode/` directory +- **AND** system creates `.openspec.yaml` with schema configuration + +#### Scenario: Create change with schema option +- **WHEN** user runs `openspec new change my-feature --schema spec-driven` +- **THEN** system creates change directory with specified schema + +#### Scenario: Create change with JSON output +- **WHEN** user runs `openspec new change my-feature --json` +- **THEN** system outputs the created change as JSON (id, path) + +#### Scenario: Create change linked to an initiative +- **WHEN** user runs `openspec new change my-feature --initiative ` (optionally `--store ` or `--store-path `) +- **THEN** system records the initiative link in the change's `.openspec.yaml` metadata +- **AND** system supports `--goal` and `--affected-areas` metadata fields + +### Requirement: CLI Show Command + +The system SHALL provide an `openspec show [item-name]` command that displays change or spec details, with spec-filtering flags for JSON output. + +#### Scenario: Show change as JSON +- **WHEN** user runs `openspec show add-dark-mode --json` +- **THEN** system outputs JSON with change details including deltas + +#### Scenario: Show only requirements of a spec +- **WHEN** user runs `openspec show --type spec --json --requirements` +- **THEN** system outputs the spec's requirements without scenario content + +#### Scenario: Show a single requirement +- **WHEN** user runs `openspec show --type spec --json --requirement ` +- **THEN** system outputs only the nth (1-based) requirement + +#### Scenario: Exclude scenarios +- **WHEN** user runs `openspec show --type spec --json --no-scenarios` +- **THEN** system outputs requirements with scenario content excluded + +### Requirement: CLI Validate Command + +The system SHALL provide an `openspec validate [item-name]` command that validates changes and specs, emits actionable hints for common authoring mistakes, and supports bounded parallelism. + +#### Scenario: Validate all changes +- **WHEN** user runs `openspec validate --all` +- **THEN** system validates all changes and specs +- **AND** system reports validation errors if any + +#### Scenario: Hint when SHALL/MUST only in header +- **WHEN** a requirement has SHALL/MUST only in its `### Requirement:` header line and not in the requirement body +- **THEN** system reports a hint to move the keyword onto the requirement body line +- **AND** the hint is more specific than the generic missing-normative-keyword error + +#### Scenario: Bounded concurrency +- **WHEN** user runs `openspec validate --all --concurrency ` +- **THEN** system validates items with at most `n` concurrent operations + +## ADDED Requirements + +### Requirement: CLI Feedback Command + +The system SHALL provide an `openspec feedback ` command to submit anonymous feedback, respecting the telemetry opt-out settings. + +#### Scenario: Submit feedback +- **WHEN** user runs `openspec feedback "the new workspace flow is great"` +- **THEN** system submits the message anonymously and reports success + +#### Scenario: Feedback respects opt-out +- **WHEN** telemetry is disabled (`OPENSPEC_TELEMETRY=0` or `DO_NOT_TRACK=1`) +- **THEN** `openspec feedback` does not transmit and informs the user it is disabled + +### Requirement: Non-Interactive Flag Acceptance + +The system SHALL accept the `--no-interactive` flag wherever upstream defines it, treating it as a no-op (the Rust CLI is already non-interactive), so scripts passing it do not error. + +#### Scenario: --no-interactive accepted +- **WHEN** user passes `--no-interactive` to a command that upstream documents it on (e.g. `show`, `validate`, `workspace setup`) +- **THEN** system accepts the flag without error and behaves identically + +### Requirement: Installed Package Schema Resolution + +The system SHALL resolve the built-in (package) schema from the embedded schema for installed binaries, not from a development-only `vendor/OpenSpec/schemas` path. + +#### Scenario: Package schema resolves when run outside the dev repo +- **WHEN** an installed binary runs `openspec schema which spec-driven` or `openspec templates` from any directory +- **THEN** system resolves the built-in schema from the embedded definition (source: package) +- **AND** system does not depend on a `vendor/OpenSpec/schemas` directory existing diff --git a/openspec/changes/cli-parity-upstream/specs/config-management/spec.md b/openspec/changes/cli-parity-upstream/specs/config-management/spec.md new file mode 100644 index 0000000..d6c1784 --- /dev/null +++ b/openspec/changes/cli-parity-upstream/specs/config-management/spec.md @@ -0,0 +1,37 @@ +## ADDED Requirements + +### Requirement: Config Subcommand Surface + +The system SHALL provide an `openspec config` command group matching upstream's subcommand shape, replacing the flat `--set/--get/--list` flags. (**BREAKING**: the flat-flag form is removed in favor of subcommands.) + +#### Scenario: Show config file path +- **WHEN** user runs `openspec config path` +- **THEN** system prints the path to the global config file + +#### Scenario: List all settings +- **WHEN** user runs `openspec config list` +- **THEN** system prints all configuration keys and values (with `--json` support) + +#### Scenario: Get a value +- **WHEN** user runs `openspec config get ` +- **THEN** system prints the value for that key (or indicates it is unset) + +#### Scenario: Set a value +- **WHEN** user runs `openspec config set ` +- **THEN** system persists the value to the global config + +#### Scenario: Unset a value +- **WHEN** user runs `openspec config unset ` +- **THEN** system removes that key from the global config + +#### Scenario: Reset config +- **WHEN** user runs `openspec config reset` (or `reset --all`) +- **THEN** system restores configuration to defaults + +#### Scenario: Edit config +- **WHEN** user runs `openspec config edit` +- **THEN** system opens the global config file in the user's editor + +#### Scenario: Manage profile +- **WHEN** user runs `openspec config profile [preset]` +- **THEN** system shows or sets the active profile (e.g. `core`, `custom`) diff --git a/openspec/changes/cli-parity-upstream/tasks.md b/openspec/changes/cli-parity-upstream/tasks.md new file mode 100644 index 0000000..4b77a92 --- /dev/null +++ b/openspec/changes/cli-parity-upstream/tasks.md @@ -0,0 +1,47 @@ +## 1. config subcommands (BREAKING) + +- [x] 1.1 Replace flat `Config { set, get, list }` with `Config(ConfigCommands)` in `src/cli/args.rs` (Path/List/Get/Set/Unset/Reset/Edit/Profile) +- [x] 1.2 Implement subcommands in `src/cli/config.rs` (get/set/unset/reset/list/path against the global config; `--json` for list) +- [x] 1.3 `config profile [preset]` shows/sets the active profile +- [x] 1.4 `config edit` opens `$VISUAL`/`$EDITOR` (testable editor-resolution helper; platform fallback) +- [x] 1.5 Tests for get/set/unset/reset round-trips; CHANGELOG note for the breaking change + +## 2. feedback command + +- [x] 2.1 Add `Feedback { message }` to args + `src/cli/feedback.rs` +- [x] 2.2 Submit via the existing telemetry client; respect opt-out (OPENSPEC_TELEMETRY/DO_NOT_TRACK/CI) → no-op with message when disabled +- [x] 2.3 Confirm payload/event name against upstream `cli/index.ts`; tests for the opt-out path + +## 3. show spec-filter flags + +- [x] 3.1 Add `--requirements`, `--requirement `, `--no-scenarios` to `show` (args + `src/cli/show.rs`) +- [x] 3.2 Filter the parsed spec model before serializing (mirror upstream `filterSpec`); JSON-only, warn if used with a change +- [x] 3.3 Tests for each filter + +## 4. new change flags + +- [x] 4.1 Add `--json`, `--goal`, `--affected-areas`, `--initiative`, `--store`, `--store-path` to `new change` +- [x] 4.2 `--json` outputs `{id, path}`; `--goal`/`--affected-areas` write the optional ChangeMetadata fields +- [x] 4.3 `--initiative` links via the initiatives resolution + `.openspec.yaml` (reuse `set change` logic) +- [x] 4.4 Tests for json output + initiative link on create + +## 5. validate --concurrency + +- [x] 5.1 Add `--concurrency ` to `validate`; bound parallel validation with a worker pool +- [x] 5.2 Preserve deterministic aggregated output (sorted); test concurrency bound + +## 6. --no-interactive acceptance + +- [x] 6.1 Add a global `--no-interactive` clap flag (like `--no-color`), accepted as a no-op everywhere +- [x] 6.2 Test that representative commands accept it without error + +## 7. package-schema resolution fix + +- [x] 7.1 Change `get_package_schemas_dir`/resolution in `src/core/schema.rs` so the built-in schema resolves from the embedded definition, not `vendor/OpenSpec/schemas` +- [x] 7.2 Verify `schemas`, `schema which`, `templates`, `schema validate` work both in-repo and from an isolated dir; tests + +## 8. Finalize + +- [x] 8.1 `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test`, `openspec validate --all` all clean +- [x] 8.2 Update CHANGELOG (config BREAKING + new flags/commands) and README command table +- [x] 8.3 Bump version as appropriate (patch — 0.2.0 -> 0.2.1 for continued upstream compatibility verification) diff --git a/src/cli/args.rs b/src/cli/args.rs index 243c7f6..b073d7e 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -2,6 +2,16 @@ use clap::{Parser, Subcommand}; const VERSION: &str = env!("CARGO_PKG_VERSION"); +fn parse_positive_usize(value: &str) -> std::result::Result { + let parsed = value + .parse::() + .map_err(|_| format!("Invalid positive integer: {}", value))?; + if parsed == 0 { + return Err("Concurrency must be at least 1".to_string()); + } + Ok(parsed) +} + #[derive(Parser, Debug)] #[command(name = "openspec")] #[command(author, version, about, long_about = None)] @@ -10,8 +20,15 @@ pub struct Cli { #[command(subcommand)] pub command: Commands, - #[arg(long, global = true, env = "NO_COLOR")] + #[arg(long, global = true)] pub no_color: bool, + + #[arg( + long = "no-interactive", + global = true, + help = "Disable interactive prompts" + )] + pub no_interactive: bool, } #[derive(Subcommand, Debug)] @@ -113,6 +130,19 @@ pub enum Commands { #[arg(long, help = "Show only deltas (JSON only)")] deltas_only: bool, + + #[arg(long, help = "JSON only: Show only requirements (exclude scenarios)")] + requirements: bool, + + #[arg( + long = "requirement", + short = 'r', + help = "JSON only: Show a specific requirement by ID (1-based)" + )] + requirement: Option, + + #[arg(long, help = "JSON only: Exclude scenario content")] + no_scenarios: bool, }, #[command(about = "Validate changes and specs")] @@ -137,6 +167,9 @@ pub enum Commands { #[arg(long, help = "Output validation results as JSON")] json: bool, + + #[arg(long, help = "Maximum number of validation workers", value_parser = parse_positive_usize)] + concurrency: Option, }, #[command(about = "Archive a completed change and update main specs")] @@ -154,18 +187,18 @@ pub enum Commands { no_validate: bool, }, - #[command(about = "View and modify global OpenSpec configuration")] - Config { - #[arg(long, help = "Set a configuration value (key=value)")] - set: Option, - - #[arg(long, help = "Get a configuration value")] - get: Option, + #[command(about = "Submit feedback about OpenSpec")] + Feedback { + #[arg(help = "Feedback message")] + message: String, - #[arg(long, help = "List all configuration values")] - list: bool, + #[arg(long, help = "Detailed description for the feedback")] + body: Option, }, + #[command(subcommand, about = "View and modify global OpenSpec configuration")] + Config(ConfigCommands), + #[command(subcommand, about = "Create new items")] New(NewCommands), @@ -209,6 +242,54 @@ pub enum SetCommands { }, } +#[derive(Subcommand, Debug)] +pub enum ConfigCommands { + #[command(about = "Show the global config file path")] + Path, + + #[command(about = "List all configuration values")] + List { + #[arg(long, help = "Output as JSON")] + json: bool, + }, + + #[command(about = "Get a configuration value")] + Get { + #[arg(help = "Configuration key")] + key: String, + }, + + #[command(about = "Set a configuration value")] + Set { + #[arg(help = "Configuration key")] + key: String, + + #[arg(help = "Configuration value")] + value: String, + }, + + #[command(about = "Unset a configuration value")] + Unset { + #[arg(help = "Configuration key")] + key: String, + }, + + #[command(about = "Reset configuration values")] + Reset { + #[arg(long, help = "Reset the entire global config")] + all: bool, + }, + + #[command(about = "Edit the global config in your editor")] + Edit, + + #[command(about = "Show or set the active profile")] + Profile { + #[arg(help = "Preset profile name")] + preset: Option, + }, +} + #[derive(Subcommand, Debug)] pub enum SchemaCommands { #[command(about = "Show where a schema resolves from")] @@ -272,6 +353,30 @@ pub enum NewCommands { #[arg(long, help = "Workflow schema to use")] schema: Option, + + #[arg(long, help = "Workspace product goal to store with the change")] + goal: Option, + + #[arg( + long = "affected-areas", + help = "Comma-separated affected workspace link names" + )] + affected_areas: Option, + + #[arg(long, help = "Link the repo-local change to an initiative")] + initiative: Option, + + #[arg(long, help = "Context store id for --initiative")] + store: Option, + + #[arg( + long = "store-path", + help = "Existing local context store root for --initiative" + )] + store_path: Option, + + #[arg(long, help = "Output as JSON")] + json: bool, }, } @@ -497,7 +602,17 @@ fn get_command_path(cli: &Cli) -> String { Commands::Show { .. } => "show".to_string(), Commands::Validate { .. } => "validate".to_string(), Commands::Archive { .. } => "archive".to_string(), - Commands::Config { .. } => "config".to_string(), + Commands::Feedback { .. } => "feedback".to_string(), + Commands::Config(cmd) => match cmd { + ConfigCommands::Path => "config:path".to_string(), + ConfigCommands::List { .. } => "config:list".to_string(), + ConfigCommands::Get { .. } => "config:get".to_string(), + ConfigCommands::Set { .. } => "config:set".to_string(), + ConfigCommands::Unset { .. } => "config:unset".to_string(), + ConfigCommands::Reset { .. } => "config:reset".to_string(), + ConfigCommands::Edit => "config:edit".to_string(), + ConfigCommands::Profile { .. } => "config:profile".to_string(), + }, Commands::New(NewCommands::Change { .. }) => "new:change".to_string(), Commands::Completion(cmd) => match cmd { CompletionCommands::Generate { .. } => "completion:generate".to_string(), @@ -612,13 +727,24 @@ pub fn run() -> Result<(), Box> { json, r#type, deltas_only, + requirements, + requirement, + no_scenarios, } => { let name = name.as_deref().ok_or_else(|| { crate::core::error::OpenSpecError::Custom( "Missing required argument ".to_string(), ) })?; - crate::cli::show::run_show(name, r#type.as_deref(), json, deltas_only)?; + crate::cli::show::run_show( + name, + r#type.as_deref(), + json, + deltas_only, + requirements, + requirement, + no_scenarios, + )?; } Commands::Validate { name, @@ -628,16 +754,18 @@ pub fn run() -> Result<(), Box> { r#type, strict, json, + concurrency, } => { - crate::cli::validate::run_validate( - name.as_deref(), + crate::cli::validate::run_validate(crate::cli::validate::ValidateOptions { + name: name.as_deref(), all, changes, specs, - r#type.as_deref(), + item_type: r#type.as_deref(), strict, json, - )?; + concurrency, + })?; } Commands::Archive { name, @@ -647,19 +775,57 @@ pub fn run() -> Result<(), Box> { } => { crate::cli::archive::run_archive(name.as_deref(), yes, skip_specs, no_validate)?; } - Commands::Config { set, get, list } => { - crate::cli::config::run_config(set.as_deref(), get.as_deref(), list)?; + Commands::Feedback { message, body } => { + crate::cli::feedback::run_feedback(&message, body.as_deref())?; } + Commands::Config(cmd) => match cmd { + ConfigCommands::Path => { + crate::cli::config::run_config_path()?; + } + ConfigCommands::List { json } => { + crate::cli::config::run_config_list(json)?; + } + ConfigCommands::Get { key } => { + crate::cli::config::run_config_get(&key)?; + } + ConfigCommands::Set { key, value } => { + crate::cli::config::run_config_set(&key, &value)?; + } + ConfigCommands::Unset { key } => { + crate::cli::config::run_config_unset(&key)?; + } + ConfigCommands::Reset { all } => { + crate::cli::config::run_config_reset(all)?; + } + ConfigCommands::Edit => { + crate::cli::config::run_config_edit()?; + } + ConfigCommands::Profile { preset } => { + crate::cli::config::run_config_profile(preset.as_deref())?; + } + }, Commands::New(NewCommands::Change { name, description, schema, + goal, + affected_areas, + initiative, + store, + store_path, + json, }) => { - crate::cli::new_change::run_new_change( - &name, - description.as_deref(), - schema.as_deref(), - )?; + crate::cli::new_change::run_new_change(crate::cli::new_change::NewChangeOptions { + name: &name, + description: description.as_deref(), + schema: schema.as_deref(), + goal: goal.as_deref(), + affected_areas: affected_areas.as_deref(), + initiative: initiative.as_deref(), + store: store.as_deref(), + store_path: store_path.as_deref(), + json, + })?; } Commands::Completion(cmd) => match cmd { CompletionCommands::Generate { shell } => { diff --git a/src/cli/config.rs b/src/cli/config.rs index a5c7af0..fabde2a 100644 --- a/src/cli/config.rs +++ b/src/cli/config.rs @@ -1,57 +1,37 @@ use crate::core::config::{ConfigManager, GlobalConfig}; use crate::core::error::{OpenSpecError, Result}; +use crate::templates::skills::CORE_WORKFLOWS; +use serde_json::{Map, Value}; +use std::ffi::OsString; +use std::process::Command; -pub fn run_config(set: Option<&str>, get: Option<&str>, list: bool) -> Result<()> { - if list || (set.is_none() && get.is_none()) { - run_config_list() - } else if let Some(key) = get { - run_config_get(key) - } else if let Some(kv) = set { - run_config_set(kv) - } else { - run_config_list() - } +pub fn run_config_path() -> Result<()> { + println!("{}", ConfigManager::global_config_path().display()); + Ok(()) } -fn run_config_list() -> Result<()> { +pub fn run_config_list(json: bool) -> Result<()> { let config = ConfigManager::load_global_config(); - let config_path = ConfigManager::global_config_path(); - - println!("Config file: {}", config_path.display()); - println!(); - println!("Profile settings:"); - println!(" profile: {}", config.profile); - println!(" delivery: {}", config.delivery); - println!(" workflows: {}", config.workflows.join(", ")); - - if !config.feature_flags.is_empty() { - println!(); - println!("Feature flags:"); - for (key, value) in &config.feature_flags { - println!(" {}: {}", key, value); - } - } - - if let Some(telemetry) = &config.telemetry { - println!(); - println!("Telemetry:"); - if let Some(id) = &telemetry.anonymous_id { - println!(" anonymous_id: {}", id); - } - println!(" notice_seen: {}", telemetry.notice_seen); + if json { + println!("{}", serde_json::to_string_pretty(&config)?); + return Ok(()); } + let yaml = serde_yaml::to_string(&config)?; + print_yaml_without_document_marker(&yaml); Ok(()) } -fn run_config_get(key: &str) -> Result<()> { +pub fn run_config_get(key: &str) -> Result<()> { let config = ConfigManager::load_global_config(); let value = get_nested_value(&config, key); match value { Some(v) => { - if v.is_object() { + if let Some(s) = v.as_str() { + println!("{}", s); + } else if v.is_object() || v.is_array() { println!("{}", serde_json::to_string_pretty(&v)?); } else { println!("{}", v); @@ -65,31 +45,155 @@ fn run_config_get(key: &str) -> Result<()> { } } -fn run_config_set(kv: &str) -> Result<()> { - let parts: Vec<&str> = kv.splitn(2, '=').collect(); - if parts.len() != 2 { +pub fn run_config_set(key: &str, value: &str) -> Result<()> { + let mut config = ConfigManager::load_global_config(); + set_nested_value(&mut config, key, value)?; + ConfigManager::save_global_config(&config)?; + println!("Set {} = {}", key, value); + Ok(()) +} + +pub fn run_config_unset(key: &str) -> Result<()> { + let mut config = ConfigManager::load_global_config(); + let existed = unset_nested_value(&mut config, key)?; + + if existed { + ConfigManager::save_global_config(&config)?; + println!("Unset {} (reverted to default)", key); + } else { + println!("Key \"{}\" was not set", key); + } + + Ok(()) +} + +pub fn run_config_reset(all: bool) -> Result<()> { + // Mirror upstream: refuse to wipe the config unless --all is explicitly passed. + if !all { return Err(OpenSpecError::Custom( - "Invalid format. Use: --set key=value".to_string(), + "--all flag is required for reset. Usage: openspec config reset --all".to_string(), )); } + ConfigManager::save_global_config(&GlobalConfig::default())?; + println!("Configuration reset to defaults"); + Ok(()) +} + +pub fn run_config_profile(preset: Option<&str>) -> Result<()> { + match preset { + None => { + let config = ConfigManager::load_global_config(); + println!("{}", config.profile); + Ok(()) + } + Some("core") => { + let mut config = ConfigManager::load_global_config(); + apply_profile_preset(&mut config, "core"); + ConfigManager::save_global_config(&config)?; + println!("core"); + Ok(()) + } + Some(preset) => Err(OpenSpecError::Custom(format!( + "Unknown profile preset \"{}\". Available presets: core", + preset + ))), + } +} - let key = parts[0]; - let value = parts[1]; +pub fn run_config_edit() -> Result<()> { + let config_path = ConfigManager::global_config_path(); + ensure_global_config_exists(&config_path)?; - let mut config = ConfigManager::load_global_config(); - set_nested_value(&mut config, key, value); + let editor = resolve_config_editor( + std::env::var_os("VISUAL") + .as_deref() + .and_then(|s| s.to_str()), + std::env::var_os("EDITOR") + .as_deref() + .and_then(|s| s.to_str()), + cfg!(windows), + ); - ConfigManager::save_global_config(&config)?; + let status = Command::new(&editor.program) + .args(&editor.args) + .arg(&config_path) + .status() + .map_err(|err| { + OpenSpecError::Custom(format!( + "Failed to launch editor '{}': {}", + editor.program, err + )) + })?; - println!("Set {} = {}", key, value); + if !status.success() { + return Err(OpenSpecError::Custom(format!( + "Editor '{}' exited with status {}", + editor.program, status + ))); + } Ok(()) } -fn get_nested_value(config: &GlobalConfig, key: &str) -> Option { - let json = serde_json::to_value(config).ok()?; +fn apply_profile_preset(config: &mut GlobalConfig, preset: &str) { + if preset == "core" { + config.profile = "core".to_string(); + config.workflows = CORE_WORKFLOWS + .iter() + .map(|workflow| workflow.to_string()) + .collect(); + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedEditor { + pub program: String, + pub args: Vec, +} + +pub fn resolve_config_editor( + visual: Option<&str>, + editor: Option<&str>, + is_windows: bool, +) -> ResolvedEditor { + let program = visual + .filter(|value| !value.trim().is_empty()) + .or_else(|| editor.filter(|value| !value.trim().is_empty())) + .map(|value| value.to_string()) + .unwrap_or_else(|| { + if is_windows { + "notepad".to_string() + } else { + "vi".to_string() + } + }); + + ResolvedEditor { + program, + args: Vec::new(), + } +} - let parts: Vec<&str> = key.split('.').collect(); +fn ensure_global_config_exists(config_path: &std::path::Path) -> Result<()> { + if config_path.exists() { + return Ok(()); + } + + ConfigManager::save_global_config(&GlobalConfig::default()) +} + +fn print_yaml_without_document_marker(yaml: &str) { + if let Some(stripped) = yaml.strip_prefix("---\n") { + print!("{}", stripped); + } else { + print!("{}", yaml); + } +} + +fn get_nested_value(config: &GlobalConfig, key: &str) -> Option { + let json = serde_json::to_value(config).ok()?; + let normalized = normalize_key_path(key); + let parts: Vec<&str> = normalized.split('.').collect(); let mut current = &json; for part in &parts { @@ -103,34 +207,312 @@ fn get_nested_value(config: &GlobalConfig, key: &str) -> Option { - config.profile = value.to_string(); +fn set_nested_value(config: &mut GlobalConfig, key: &str, value: &str) -> Result<()> { + let normalized = normalize_key_path(key); + // Validate enum-constrained values, mirroring upstream config-schema (profile/delivery). + match normalized.as_str() { + "profile" if !["core", "custom"].contains(&value) => { + return Err(OpenSpecError::Custom(format!( + "Invalid value for 'profile': '{}'. Allowed: core, custom", + value + ))); } - "delivery" => { - config.delivery = value.to_string(); + "delivery" if !["both", "skills", "commands"].contains(&value) => { + return Err(OpenSpecError::Custom(format!( + "Invalid value for 'delivery': '{}'. Allowed: both, skills, commands", + value + ))); } - "workflows" => { - config.workflows = value.split(',').map(|s| s.trim().to_string()).collect(); + _ => {} + } + let coerced = if normalized == "workflows" { + Value::Array( + value + .split(',') + .map(|item| Value::String(item.trim().to_string())) + .collect(), + ) + } else { + coerce_value(value) + }; + let mut json = serde_json::to_value(&*config)?; + set_nested_json_value(&mut json, &normalized, coerced)?; + *config = serde_json::from_value(json).map_err(|err| { + OpenSpecError::Custom(format!( + "Invalid configuration value for '{}': {}", + key, err + )) + })?; + Ok(()) +} + +fn unset_nested_value(config: &mut GlobalConfig, key: &str) -> Result { + let mut json = serde_json::to_value(&*config)?; + let existed = delete_nested_json_value(&mut json, &normalize_key_path(key)); + *config = serde_json::from_value(json).map_err(|err| { + OpenSpecError::Custom(format!( + "Invalid configuration after unsetting '{}': {}", + key, err + )) + })?; + Ok(existed) +} + +fn normalize_key_path(key: &str) -> String { + key.split('.') + .map(|part| match part { + "feature_flags" => "featureFlags", + "anonymous_id" => "anonymousId", + "notice_seen" => "noticeSeen", + other => other, + }) + .collect::>() + .join(".") +} + +fn coerce_value(value: &str) -> Value { + if value == "true" { + Value::Bool(true) + } else if value == "false" { + Value::Bool(false) + } else if let Ok(n) = value.parse::() { + Value::Number(n.into()) + } else { + Value::String(value.to_string()) + } +} + +fn set_nested_json_value(root: &mut Value, key: &str, value: Value) -> Result<()> { + let parts: Vec<&str> = key.split('.').filter(|part| !part.is_empty()).collect(); + if parts.is_empty() { + return Err(OpenSpecError::Custom( + "Configuration key cannot be empty".to_string(), + )); + } + + let mut current = root + .as_object_mut() + .ok_or_else(|| OpenSpecError::Custom("Configuration root must be an object".to_string()))?; + + for part in &parts[..parts.len() - 1] { + let entry = current + .entry((*part).to_string()) + .or_insert_with(|| Value::Object(Map::new())); + if !entry.is_object() { + *entry = Value::Object(Map::new()); } - _ => { - let coerced: serde_json::Value = if value == "true" { - serde_json::Value::Bool(true) - } else if value == "false" { - serde_json::Value::Bool(false) - } else if let Ok(n) = value.parse::() { - serde_json::Value::Number(n.into()) - } else { - serde_json::Value::String(value.to_string()) - }; - - if key.starts_with("feature_flags.") { - let flag_name = key.strip_prefix("feature_flags.").unwrap(); - if let Some(b) = coerced.as_bool() { - config.feature_flags.insert(flag_name.to_string(), b); - } + current = entry.as_object_mut().expect("entry forced to object"); + } + + current.insert(parts[parts.len() - 1].to_string(), value); + Ok(()) +} + +fn delete_nested_json_value(root: &mut Value, key: &str) -> bool { + let parts: Vec<&str> = key.split('.').filter(|part| !part.is_empty()).collect(); + if parts.is_empty() { + return false; + } + + let Some(object) = root.as_object_mut() else { + return false; + }; + + delete_nested_from_map(object, &parts) +} + +fn delete_nested_from_map(map: &mut Map, parts: &[&str]) -> bool { + if parts.len() == 1 { + return map.remove(parts[0]).is_some(); + } + + let Some(child) = map.get_mut(parts[0]) else { + return false; + }; + let Some(child_map) = child.as_object_mut() else { + return false; + }; + + let existed = delete_nested_from_map(child_map, &parts[1..]); + if existed && child_map.is_empty() { + map.remove(parts[0]); + } + existed +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Mutex, OnceLock}; + use tempfile::TempDir; + + static ENV_LOCK: OnceLock> = OnceLock::new(); + + struct EnvGuard { + key: &'static str, + previous: Option, + } + + impl EnvGuard { + fn set(key: &'static str, value: &std::path::Path) -> Self { + let previous = std::env::var_os(key); + std::env::set_var(key, value); + Self { key, previous } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), } } } + + #[test] + fn config_list_yaml_uses_camelcase_keys() { + let yaml = serde_yaml::to_string(&GlobalConfig::default()).unwrap(); + assert!(yaml.contains("featureFlags")); + assert!(!yaml.contains("feature_flags")); + } + + #[test] + fn set_and_get_nested_camelcase_and_legacy_aliases() { + let mut config = GlobalConfig::default(); + set_nested_value(&mut config, "feature_flags.beta", "true").unwrap(); + set_nested_value(&mut config, "telemetry.anonymous_id", "abc").unwrap(); + + assert_eq!( + get_nested_value(&config, "featureFlags.beta"), + Some(Value::Bool(true)) + ); + assert_eq!( + get_nested_value(&config, "telemetry.anonymousId"), + Some(Value::String("abc".to_string())) + ); + } + + #[test] + fn unset_nested_value_removes_empty_parent_objects() { + let mut config = GlobalConfig::default(); + set_nested_value(&mut config, "telemetry.anonymousId", "abc").unwrap(); + + let existed = unset_nested_value(&mut config, "telemetry.anonymousId").unwrap(); + + assert!(existed); + assert_eq!(get_nested_value(&config, "telemetry.anonymousId"), None); + } + + #[test] + fn unset_missing_key_reports_false() { + let mut config = GlobalConfig::default(); + let existed = unset_nested_value(&mut config, "featureFlags.beta").unwrap(); + assert!(!existed); + } + + #[test] + fn profile_preset_sets_core_and_workflows() { + let mut config = GlobalConfig { + profile: "custom".to_string(), + workflows: vec!["custom-one".to_string()], + ..GlobalConfig::default() + }; + + apply_profile_preset(&mut config, "core"); + + assert_eq!(config.profile, "core"); + assert_eq!( + config.workflows, + CORE_WORKFLOWS + .iter() + .map(|workflow| workflow.to_string()) + .collect::>() + ); + } + + #[test] + fn resolve_config_editor_prefers_visual_then_editor_then_platform_default() { + let visual = resolve_config_editor(Some("nano"), Some("vim"), false); + assert_eq!(visual.program, "nano"); + + let editor = resolve_config_editor(None, Some("vim"), false); + assert_eq!(editor.program, "vim"); + + let unix_default = resolve_config_editor(None, None, false); + assert_eq!(unix_default.program, "vi"); + + let windows_default = resolve_config_editor(None, None, true); + assert_eq!(windows_default.program, "notepad"); + } + + #[test] + fn config_round_trip_set_get_unset_reset() { + let _guard = ENV_LOCK.get_or_init(|| Mutex::new(())).lock().unwrap(); + let temp_dir = TempDir::new().unwrap(); + let _env_guard = EnvGuard::set("XDG_CONFIG_HOME", temp_dir.path()); + + run_config_set("profile", "custom").unwrap(); + run_config_set("delivery", "skills").unwrap(); + run_config_set("workflows", "alpha,beta").unwrap(); + run_config_set("featureFlags.beta", "true").unwrap(); + run_config_set("telemetry.anonymousId", "anon-1").unwrap(); + + let config = ConfigManager::load_global_config(); + assert_eq!( + get_nested_value(&config, "profile"), + Some(Value::String("custom".to_string())) + ); + assert_eq!( + get_nested_value(&config, "delivery"), + Some(Value::String("skills".to_string())) + ); + assert_eq!( + get_nested_value(&config, "workflows"), + Some(Value::Array(vec![ + Value::String("alpha".to_string()), + Value::String("beta".to_string()), + ])) + ); + assert_eq!( + get_nested_value(&config, "featureFlags.beta"), + Some(Value::Bool(true)) + ); + assert_eq!( + get_nested_value(&config, "telemetry.anonymousId"), + Some(Value::String("anon-1".to_string())) + ); + + run_config_unset("telemetry.anonymousId").unwrap(); + let config = ConfigManager::load_global_config(); + assert_eq!(get_nested_value(&config, "telemetry.anonymousId"), None); + + run_config_reset(true).unwrap(); + let config = ConfigManager::load_global_config(); + let default = GlobalConfig::default(); + assert_eq!(config.profile, default.profile); + assert_eq!(config.delivery, default.delivery); + assert_eq!(config.workflows, default.workflows); + assert_eq!(config.feature_flags, default.feature_flags); + assert!(config.telemetry.is_none()); + } + + #[test] + fn set_rejects_invalid_enum_values() { + let mut config = GlobalConfig::default(); + assert!(set_nested_value(&mut config, "delivery", "bogus").is_err()); + assert!(set_nested_value(&mut config, "profile", "nope").is_err()); + // valid values still accepted + assert!(set_nested_value(&mut config, "delivery", "skills").is_ok()); + assert_eq!(config.delivery, "skills"); + assert!(set_nested_value(&mut config, "profile", "custom").is_ok()); + assert_eq!(config.profile, "custom"); + } + + #[test] + fn reset_requires_all_flag() { + // Without --all, reset must error before touching the config (mirrors upstream). + let err = run_config_reset(false).unwrap_err(); + assert!(err.to_string().contains("--all")); + } } diff --git a/src/cli/feedback.rs b/src/cli/feedback.rs new file mode 100644 index 0000000..ca2bfea --- /dev/null +++ b/src/cli/feedback.rs @@ -0,0 +1,185 @@ +use crate::core::error::{OpenSpecError, Result}; +use std::process::Command; + +/// GitHub repository that feedback issues are filed against. +const FEEDBACK_REPO: &str = "oonid/OpenSpec-rs"; + +/// Submit feedback as a GitHub issue, mirroring upstream `FeedbackCommand`: +/// format a title/body, file it via the `gh` CLI when available + authenticated, +/// otherwise fall back to printing the formatted feedback and a manual issue URL. +/// (This is an explicit user action — not telemetry — so it is NOT gated by the +/// telemetry opt-out.) +pub fn run_feedback(message: &str, body: Option<&str>) -> Result<()> { + let title = format_title(message); + let body = format_body(body); + + if !gh_installed() { + return handle_fallback(&title, &body, Fallback::Missing); + } + if !gh_authenticated() { + return handle_fallback(&title, &body, Fallback::Unauthenticated); + } + submit_via_gh(&title, &body) +} + +fn format_title(message: &str) -> String { + format!("Feedback: {}", message) +} + +fn format_body(body_text: Option<&str>) -> String { + let mut parts: Vec = Vec::new(); + if let Some(text) = body_text.filter(|t| !t.trim().is_empty()) { + parts.push(text.to_string()); + parts.push(String::new()); // blank line before metadata + } + parts.push(generate_metadata()); + parts.join("\n") +} + +fn generate_metadata() -> String { + format!( + "---\nSubmitted via OpenSpec CLI\n- Version: {}\n- Platform: {}\n- Timestamp: {}", + env!("CARGO_PKG_VERSION"), + std::env::consts::OS, + chrono::Utc::now().to_rfc3339() + ) +} + +fn gh_installed() -> bool { + Command::new("gh") + .arg("--version") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +fn gh_authenticated() -> bool { + Command::new("gh") + .args(["auth", "status"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +fn submit_via_gh(title: &str, body: &str) -> Result<()> { + let output = Command::new("gh") + .args([ + "issue", + "create", + "--repo", + FEEDBACK_REPO, + "--title", + title, + "--body", + body, + "--label", + "feedback", + ]) + .output() + .map_err(|e| OpenSpecError::Custom(format!("Failed to run gh: {}", e)))?; + + if output.status.success() { + let issue_url = String::from_utf8_lossy(&output.stdout).trim().to_string(); + println!("\n✓ Feedback submitted successfully!"); + println!("Issue URL: {}\n", issue_url); + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(OpenSpecError::Custom(if stderr.is_empty() { + "gh issue create failed".to_string() + } else { + stderr + })) + } +} + +enum Fallback { + Missing, + Unauthenticated, +} + +fn handle_fallback(title: &str, body: &str, reason: Fallback) -> Result<()> { + match reason { + Fallback::Missing => println!("⚠️ GitHub CLI not found. Manual submission required."), + Fallback::Unauthenticated => { + println!("⚠️ GitHub authentication required. Manual submission required.") + } + } + + println!("\n--- FORMATTED FEEDBACK ---"); + println!("Title: {}", title); + println!("Labels: feedback"); + println!("\nBody:"); + println!("{}", body); + println!("--- END FEEDBACK ---\n"); + + println!("Please submit your feedback manually:"); + println!("{}", manual_submission_url(title, body)); + + if matches!(reason, Fallback::Unauthenticated) { + println!("\nTo auto-submit in the future: gh auth login"); + } + Ok(()) +} + +fn manual_submission_url(title: &str, body: &str) -> String { + format!( + "https://github.com/{}/issues/new?title={}&body={}&labels={}", + FEEDBACK_REPO, + encode_uri_component(title), + encode_uri_component(body), + encode_uri_component("feedback"), + ) +} + +/// Percent-encode like JavaScript's `encodeURIComponent`: leave unreserved +/// characters (A-Z a-z 0-9 - _ . ! ~ * ' ( )) as-is, encode everything else as +/// UTF-8 bytes. +fn encode_uri_component(input: &str) -> String { + const UNRESERVED: &[u8] = b"-_.!~*'()"; + let mut out = String::with_capacity(input.len()); + for &byte in input.as_bytes() { + if byte.is_ascii_alphanumeric() || UNRESERVED.contains(&byte) { + out.push(byte as char); + } else { + out.push_str(&format!("%{:02X}", byte)); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn title_and_body_formatting() { + assert_eq!(format_title("hello"), "Feedback: hello"); + + let with_body = format_body(Some("details here")); + assert!(with_body.starts_with("details here\n")); + assert!(with_body.contains("Submitted via OpenSpec CLI")); + assert!(with_body.contains("- Version:")); + + // No body text → just the metadata footer (no leading blank line). + let no_body = format_body(None); + assert!(no_body.starts_with("---")); + assert!(no_body.contains("Submitted via OpenSpec CLI")); + } + + #[test] + fn manual_url_is_percent_encoded_against_repo() { + let url = manual_submission_url("Feedback: a b/c", "x&y"); + assert!(url.starts_with("https://github.com/oonid/OpenSpec-rs/issues/new?")); + assert!(url.contains("title=Feedback%3A%20a%20b%2Fc")); + assert!(url.contains("body=x%26y")); + assert!(url.contains("labels=feedback")); + } + + #[test] + fn encode_uri_component_matches_js_semantics() { + assert_eq!(encode_uri_component("a b"), "a%20b"); + assert_eq!(encode_uri_component("a/b?c=d&e"), "a%2Fb%3Fc%3Dd%26e"); + assert_eq!(encode_uri_component("keep-_.!~*'()"), "keep-_.!~*'()"); + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index db971e1..614b627 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -3,6 +3,7 @@ mod args; mod completion; mod config; mod context_store; +mod feedback; mod init; mod initiative; mod instructions; diff --git a/src/cli/new_change.rs b/src/cli/new_change.rs index 9eba52d..2da36a6 100644 --- a/src/cli/new_change.rs +++ b/src/cli/new_change.rs @@ -1,9 +1,38 @@ -use regex::Regex; - use crate::core::error::{OpenSpecError, Result}; +use crate::core::schema::ChangeMetadata; +use regex::Regex; +use serde::Serialize; pub const DEFAULT_SCHEMA: &str = "spec-driven"; +#[derive(Debug, Serialize)] +struct ChangeInfo { + id: String, + path: String, + #[serde(rename = "metadataPath")] + metadata_path: String, + schema: String, +} + +#[derive(Debug, Serialize)] +struct NewChangeOutput { + change: ChangeInfo, + #[serde(skip_serializing_if = "Option::is_none")] + initiative: Option, +} + +pub struct NewChangeOptions<'a> { + pub name: &'a str, + pub description: Option<&'a str>, + pub schema: Option<&'a str>, + pub goal: Option<&'a str>, + pub affected_areas: Option<&'a str>, + pub initiative: Option<&'a str>, + pub store: Option<&'a str>, + pub store_path: Option<&'a str>, + pub json: bool, +} + pub struct ValidationResult { pub valid: bool, pub error: Option, @@ -89,7 +118,19 @@ pub fn validate_change_name(name: &str) -> ValidationResult { } } -pub fn run_new_change(name: &str, description: Option<&str>, schema: Option<&str>) -> Result<()> { +pub fn run_new_change(options: NewChangeOptions<'_>) -> Result<()> { + let NewChangeOptions { + name, + description, + schema, + goal, + affected_areas, + initiative, + store, + store_path, + json, + } = options; + let validation = validate_change_name(name); if !validation.valid { return Err(OpenSpecError::Custom( @@ -99,6 +140,25 @@ pub fn run_new_change(name: &str, description: Option<&str>, schema: Option<&str )); } + if initiative.is_none() && (store.is_some() || store_path.is_some()) { + return Err(OpenSpecError::Custom( + "Pass --initiative when using --store or --store-path.".to_string(), + )); + } + + let resolved_initiative = match initiative { + Some(id) if id.trim().is_empty() => { + return Err(OpenSpecError::Custom( + "Pass --initiative to link a change to an initiative.".to_string(), + )); + } + Some(id) => Some( + crate::cli::set::resolve_initiative_link(id.trim(), store, store_path) + .map_err(OpenSpecError::Custom)?, + ), + None => None, + }; + let project_root = std::env::current_dir() .map_err(|e| OpenSpecError::Custom(format!("Failed to get current directory: {}", e)))?; @@ -126,9 +186,16 @@ pub fn run_new_change(name: &str, description: Option<&str>, schema: Option<&str std::fs::create_dir_all(&change_dir) .map_err(|e| OpenSpecError::Custom(format!("Failed to create change directory: {}", e)))?; - let today = chrono_lite_today(); - let metadata_content = format!("schema: {}\ncreated: {}\n", schema_name, today); + let metadata = ChangeMetadata { + schema: schema_name.clone(), + created: Some(chrono_lite_today()), + goal: normalize_optional(goal), + affected_areas: parse_affected_areas(affected_areas), + initiative: resolved_initiative, + }; let metadata_path = change_dir.join(".openspec.yaml"); + let metadata_content = serde_yaml::to_string(&metadata) + .map_err(|e| OpenSpecError::Custom(format!("Failed to serialize metadata file: {}", e)))?; std::fs::write(&metadata_path, metadata_content) .map_err(|e| OpenSpecError::Custom(format!("Failed to write metadata file: {}", e)))?; @@ -139,14 +206,54 @@ pub fn run_new_change(name: &str, description: Option<&str>, schema: Option<&str .map_err(|e| OpenSpecError::Custom(format!("Failed to write README file: {}", e)))?; } - println!( - "Created change '{}' at openspec/changes/{}/ (schema: {})", - name, name, schema_name - ); + if json { + let output = NewChangeOutput { + change: ChangeInfo { + id: name.to_string(), + path: format!("openspec/changes/{}", name), + metadata_path: format!("openspec/changes/{}/.openspec.yaml", name), + schema: schema_name.clone(), + }, + initiative: metadata.initiative.clone(), + }; + println!("{}", serde_json::to_string_pretty(&output)?); + } else { + println!( + "Created change '{}' at openspec/changes/{}/ (schema: {})", + name, name, schema_name + ); + } Ok(()) } +fn normalize_optional(value: Option<&str>) -> Option { + value.and_then(|raw| { + let trimmed = raw.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }) +} + +fn parse_affected_areas(value: Option<&str>) -> Option> { + let areas: Vec = value + .unwrap_or("") + .split(',') + .map(|area| area.trim()) + .filter(|area| !area.is_empty()) + .map(|area| area.to_string()) + .collect(); + + if areas.is_empty() { + None + } else { + Some(areas) + } +} + fn chrono_lite_today() -> String { use std::time::{SystemTime, UNIX_EPOCH}; @@ -199,3 +306,88 @@ fn days_to_ymd(days: i32) -> (i32, u32, u32) { fn is_leap_year(year: i32) -> bool { (year % 4 == 0 && year % 100 != 0) || (year % 400 == 0) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_affected_areas_trims_and_ignores_empty_values() { + assert_eq!(parse_affected_areas(None), None); + assert_eq!(parse_affected_areas(Some("")), None); + assert_eq!( + parse_affected_areas(Some(" cli , docs ,, ")), + Some(vec!["cli".to_string(), "docs".to_string()]) + ); + } + + #[test] + fn test_build_json_output_uses_id_and_relative_path() { + let output = NewChangeOutput { + change: ChangeInfo { + id: "add-telemetry".to_string(), + path: format!("openspec/changes/{}", "add-telemetry"), + metadata_path: "openspec/changes/add-telemetry/.openspec.yaml".to_string(), + schema: "spec-driven".to_string(), + }, + initiative: None, + }; + + let value = serde_json::to_value(&output).unwrap(); + assert_eq!(value["change"]["id"], "add-telemetry"); + assert_eq!(value["change"]["path"], "openspec/changes/add-telemetry"); + assert_eq!( + value["change"]["metadataPath"], + "openspec/changes/add-telemetry/.openspec.yaml" + ); + assert_eq!(value["change"]["schema"], "spec-driven"); + assert!(value.get("initiative").is_none()); + } + + #[test] + fn test_run_new_change_writes_goal_and_affected_areas() { + let temp = tempfile::tempdir().unwrap(); + + let result = crate::test_support::with_current_dir(temp.path(), || { + run_new_change(NewChangeOptions { + name: "add-telemetry", + description: Some("Add telemetry"), + schema: Some("spec-driven"), + goal: Some("Ship telemetry"), + affected_areas: Some("cli, docs"), + initiative: None, + store: None, + store_path: None, + json: false, + }) + }); + + result.unwrap(); + + let metadata_path = temp + .path() + .join("openspec") + .join("changes") + .join("add-telemetry") + .join(".openspec.yaml"); + let metadata: ChangeMetadata = + serde_yaml::from_str(&std::fs::read_to_string(&metadata_path).unwrap()).unwrap(); + assert_eq!(metadata.schema, "spec-driven"); + assert_eq!(metadata.goal.as_deref(), Some("Ship telemetry")); + assert_eq!( + metadata.affected_areas.as_deref(), + Some(&["cli".to_string(), "docs".to_string()][..]) + ); + assert!(metadata.initiative.is_none()); + + let readme = std::fs::read_to_string( + temp.path() + .join("openspec") + .join("changes") + .join("add-telemetry") + .join("README.md"), + ) + .unwrap(); + assert!(readme.contains("Add telemetry")); + } +} diff --git a/src/cli/schema.rs b/src/cli/schema.rs index b555ec5..90d919d 100644 --- a/src/cli/schema.rs +++ b/src/cli/schema.rs @@ -3,9 +3,8 @@ use std::path::Path; use crate::cli::args::SchemaCommands; use crate::core::error::{OpenSpecError, Result}; use crate::core::schema::{ - get_embedded_spec_driven_schema, get_package_schemas_dir, get_project_schemas_dir, - get_user_schemas_dir, list_schemas, load_schema, resolve_schema, Artifact, SchemaSource, - SchemaYaml, + get_embedded_spec_driven_schema, get_project_schemas_dir, get_user_schemas_dir, list_schemas, + load_schema, resolve_schema, Artifact, SchemaSource, SchemaYaml, }; const DEFAULT_SCHEMA: &str = "spec-driven"; @@ -42,7 +41,6 @@ fn resolve_with_shadows(name: &str, project_root: &Path) -> Option = Vec::new(); @@ -608,25 +606,13 @@ mod tests { } // The fork/init commands operate relative to the current working directory. - // A process-wide mutex serializes the tests that chdir so they don't race. - static CWD_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - + // A shared process-wide helper serializes the tests that chdir so they don't race. fn fork_into(dir: &Path, source: &str, name: Option<&str>, force: bool) -> Result<()> { - let _guard = CWD_LOCK.lock().unwrap(); - let prev = std::env::current_dir().unwrap(); - std::env::set_current_dir(dir).unwrap(); - let result = run_fork(source, name, force, true); - std::env::set_current_dir(prev).unwrap(); - result + crate::test_support::with_current_dir(dir, || run_fork(source, name, force, true)) } fn init_into(dir: &Path, name: &str, force: bool) -> Result<()> { - let _guard = CWD_LOCK.lock().unwrap(); - let prev = std::env::current_dir().unwrap(); - std::env::set_current_dir(dir).unwrap(); - let result = run_init(name, force, true); - std::env::set_current_dir(prev).unwrap(); - result + crate::test_support::with_current_dir(dir, || run_init(name, force, true)) } #[test] diff --git a/src/cli/set.rs b/src/cli/set.rs index 127694b..ecec5d1 100644 --- a/src/cli/set.rs +++ b/src/cli/set.rs @@ -173,6 +173,8 @@ fn set_change_inner( let base_metadata = existing_metadata.unwrap_or_else(|| ChangeMetadata { schema: crate::cli::new_change::DEFAULT_SCHEMA.to_string(), created: None, + goal: None, + affected_areas: None, initiative: None, }); @@ -193,6 +195,8 @@ fn set_change_inner( let updated = ChangeMetadata { schema: base_metadata.schema.clone(), created: base_metadata.created.clone(), + goal: base_metadata.goal.clone(), + affected_areas: base_metadata.affected_areas.clone(), initiative: Some(link), }; write_change_metadata(&metadata_path, &updated)?; @@ -202,7 +206,7 @@ fn set_change_inner( } /// Resolve the requested initiative into a concrete `{ store, id }` link. -fn resolve_initiative_link( +pub(crate) fn resolve_initiative_link( initiative_id: &str, store: Option<&str>, store_path: Option<&str>, @@ -315,6 +319,8 @@ mod tests { let meta = ChangeMetadata { schema: "spec-driven".to_string(), created: Some("2026-01-01".to_string()), + goal: None, + affected_areas: None, initiative: Some(link("team", "roadmap")), }; write_change_metadata(&path, &meta).unwrap(); diff --git a/src/cli/show.rs b/src/cli/show.rs index 4f5f0a8..4e975b8 100644 --- a/src/cli/show.rs +++ b/src/cli/show.rs @@ -39,12 +39,70 @@ pub struct ShowChangeOutput { pub deltas: Vec, } -pub fn run_show(name: &str, item_type: Option<&str>, json: bool, _deltas_only: bool) -> Result<()> { +fn validate_spec_filter_usage_for_output( + resolved_type: &str, + json: bool, + uses_spec_filter_flags: bool, +) -> Result<()> { + if resolved_type == "spec" && uses_spec_filter_flags && !json { + return Err(OpenSpecError::Custom( + "Options --requirements, --requirement, and --no-scenarios require --json".to_string(), + )); + } + + Ok(()) +} + +#[derive(Debug, Clone, Copy)] +struct SpecFilterOptions { + requirements_only: bool, + requirement: Option, + no_scenarios: bool, +} + +#[derive(Debug, Clone)] +struct SpecOutputInput { + id: String, + title: String, + overview: String, + requirements: Vec, + metadata: SpecMetadataOutput, +} + +#[derive(Debug, Clone, serde::Serialize)] +struct SpecMetadataOutput { + version: String, + format: String, + #[serde(rename = "sourcePath", skip_serializing_if = "Option::is_none")] + source_path: Option, +} + +#[derive(Debug, Clone, serde::Serialize)] +struct SpecOutput { + id: String, + title: String, + overview: String, + #[serde(rename = "requirementCount")] + requirement_count: usize, + requirements: Vec, + metadata: SpecMetadataOutput, +} + +pub fn run_show( + name: &str, + item_type: Option<&str>, + json: bool, + _deltas_only: bool, + requirements: bool, + requirement: Option, + no_scenarios: bool, +) -> Result<()> { let project_root = std::env::current_dir() .map_err(|e| OpenSpecError::Custom(format!("Failed to get current directory: {}", e)))?; let changes = get_available_changes(&project_root)?; let specs = get_available_specs(&project_root)?; + let uses_spec_filter_flags = requirements || requirement.is_some() || no_scenarios; let is_change = changes.contains(&name.to_string()); let is_spec = specs.contains(&name.to_string()); @@ -73,9 +131,25 @@ pub fn run_show(name: &str, item_type: Option<&str>, json: bool, _deltas_only: b ))); } + if resolved_type == "change" && uses_spec_filter_flags { + eprintln!( + "Warning: Ignoring spec-only flags for change '{}': --requirements, --requirement, --no-scenarios", + name + ); + } + + validate_spec_filter_usage_for_output(resolved_type, json, uses_spec_filter_flags)?; + match resolved_type { "change" => show_change(&project_root, name, json), - "spec" => show_spec(&project_root, name, json), + "spec" => show_spec( + &project_root, + name, + json, + requirements, + requirement, + no_scenarios, + ), _ => Err(OpenSpecError::Custom(format!( "Unknown type '{}'. Use 'change' or 'spec'", resolved_type @@ -127,7 +201,14 @@ fn show_change(project_root: &std::path::Path, change_name: &str, json: bool) -> Ok(()) } -fn show_spec(project_root: &std::path::Path, spec_id: &str, json: bool) -> Result<()> { +fn show_spec( + project_root: &std::path::Path, + spec_id: &str, + json: bool, + requirements_only: bool, + requirement: Option, + no_scenarios: bool, +) -> Result<()> { let spec_path = project_root .join(OPENSPEC_DIR_NAME) .join("specs") @@ -153,25 +234,6 @@ fn show_spec(project_root: &std::path::Path, spec_id: &str, json: bool) -> Resul .parse_spec(spec_id) .map_err(|e| OpenSpecError::Custom(format!("Failed to parse spec: {}", e)))?; - #[derive(Debug, Clone, serde::Serialize)] - struct SpecMetadataOutput { - version: String, - format: String, - #[serde(rename = "sourcePath", skip_serializing_if = "Option::is_none")] - source_path: Option, - } - - #[derive(Debug, Clone, serde::Serialize)] - struct SpecOutput { - id: String, - title: String, - overview: String, - #[serde(rename = "requirementCount")] - requirement_count: usize, - requirements: Vec, - metadata: SpecMetadataOutput, - } - let metadata = match &spec.metadata { Some(m) => SpecMetadataOutput { version: m.version.clone(), @@ -185,27 +247,33 @@ fn show_spec(project_root: &std::path::Path, spec_id: &str, json: bool) -> Resul }, }; - let output = SpecOutput { - id: spec_id.to_string(), - title: spec.name, - overview: spec.overview, - requirement_count: spec.requirements.len(), - requirements: spec - .requirements - .iter() - .map(|r| RequirementJson { - text: r.text.clone(), - scenarios: r - .scenarios - .iter() - .map(|s| ScenarioJson { - raw_text: s.raw_text.clone(), - }) - .collect(), - }) - .collect(), - metadata, - }; + let output = build_filtered_spec_output( + SpecOutputInput { + id: spec_id.to_string(), + title: spec.name, + overview: spec.overview, + requirements: spec + .requirements + .iter() + .map(|r| RequirementJson { + text: r.text.clone(), + scenarios: r + .scenarios + .iter() + .map(|s| ScenarioJson { + raw_text: s.raw_text.clone(), + }) + .collect(), + }) + .collect(), + metadata, + }, + SpecFilterOptions { + requirements_only, + requirement, + no_scenarios, + }, + )?; println!("{}", serde_json::to_string_pretty(&output)?); } else { @@ -215,6 +283,60 @@ fn show_spec(project_root: &std::path::Path, spec_id: &str, json: bool) -> Resul Ok(()) } +fn build_filtered_spec_output( + input: SpecOutputInput, + filters: SpecFilterOptions, +) -> Result { + let SpecOutputInput { + id, + title, + overview, + requirements, + metadata, + } = input; + let SpecFilterOptions { + requirements_only, + requirement, + no_scenarios, + } = filters; + + if requirements_only && requirement.is_some() { + return Err(OpenSpecError::Custom( + "Options --requirements and --requirement cannot be used together".to_string(), + )); + } + + let mut requirements = if let Some(index) = requirement { + let idx = index + .checked_sub(1) + .ok_or_else(|| OpenSpecError::Custom(format!("Requirement {} not found", index)))?; + if idx >= requirements.len() { + return Err(OpenSpecError::Custom(format!( + "Requirement {} not found", + index + ))); + } + vec![requirements[idx].clone()] + } else { + requirements + }; + + if requirements_only || no_scenarios { + for req in &mut requirements { + req.scenarios.clear(); + } + } + + Ok(SpecOutput { + id, + title, + overview, + requirement_count: requirements.len(), + requirements, + metadata, + }) +} + fn get_available_changes(project_root: &std::path::Path) -> Result> { let changes_dir = project_root.join(OPENSPEC_DIR_NAME).join("changes"); @@ -491,4 +613,213 @@ mod tests { assert_eq!(v["deltaCount"], 2); assert!(v.get("delta_count").is_none()); } + + #[test] + fn test_build_filtered_spec_output_selects_requirement_and_strips_scenarios() { + let output = build_filtered_spec_output( + SpecOutputInput { + id: "spec-a".to_string(), + title: "Spec A".to_string(), + overview: "Overview".to_string(), + requirements: vec![ + RequirementJson { + text: "Req 1".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 1".to_string(), + }], + }, + RequirementJson { + text: "Req 2".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 2".to_string(), + }], + }, + ], + metadata: SpecMetadataOutput { + version: "1.0.0".to_string(), + format: "openspec".to_string(), + source_path: None, + }, + }, + SpecFilterOptions { + requirements_only: false, + requirement: Some(2), + no_scenarios: true, + }, + ) + .unwrap(); + + assert_eq!(output.requirement_count, 1); + assert_eq!(output.requirements.len(), 1); + assert_eq!(output.requirements[0].text, "Req 2"); + assert!(output.requirements[0].scenarios.is_empty()); + } + + #[test] + fn test_build_filtered_spec_output_requirements_only_strips_all_scenarios() { + let output = build_filtered_spec_output( + SpecOutputInput { + id: "spec-a".to_string(), + title: "Spec A".to_string(), + overview: "Overview".to_string(), + requirements: vec![ + RequirementJson { + text: "Req 1".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 1".to_string(), + }], + }, + RequirementJson { + text: "Req 2".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 2".to_string(), + }], + }, + ], + metadata: SpecMetadataOutput { + version: "1.0.0".to_string(), + format: "openspec".to_string(), + source_path: None, + }, + }, + SpecFilterOptions { + requirements_only: true, + requirement: None, + no_scenarios: false, + }, + ) + .unwrap(); + + assert_eq!(output.requirement_count, 2); + assert_eq!(output.requirements.len(), 2); + assert!(output + .requirements + .iter() + .all(|req| req.scenarios.is_empty())); + } + + #[test] + fn test_build_filtered_spec_output_no_scenarios_strips_all_scenarios() { + let output = build_filtered_spec_output( + SpecOutputInput { + id: "spec-a".to_string(), + title: "Spec A".to_string(), + overview: "Overview".to_string(), + requirements: vec![ + RequirementJson { + text: "Req 1".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 1".to_string(), + }], + }, + RequirementJson { + text: "Req 2".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 2".to_string(), + }], + }, + ], + metadata: SpecMetadataOutput { + version: "1.0.0".to_string(), + format: "openspec".to_string(), + source_path: None, + }, + }, + SpecFilterOptions { + requirements_only: false, + requirement: None, + no_scenarios: true, + }, + ) + .unwrap(); + + assert_eq!(output.requirement_count, 2); + assert_eq!(output.requirements.len(), 2); + assert!(output + .requirements + .iter() + .all(|req| req.scenarios.is_empty())); + } + + #[test] + fn test_build_filtered_spec_output_requirement_selection_keeps_scenarios() { + let output = build_filtered_spec_output( + SpecOutputInput { + id: "spec-a".to_string(), + title: "Spec A".to_string(), + overview: "Overview".to_string(), + requirements: vec![ + RequirementJson { + text: "Req 1".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 1".to_string(), + }], + }, + RequirementJson { + text: "Req 2".to_string(), + scenarios: vec![ScenarioJson { + raw_text: "Scenario 2".to_string(), + }], + }, + ], + metadata: SpecMetadataOutput { + version: "1.0.0".to_string(), + format: "openspec".to_string(), + source_path: None, + }, + }, + SpecFilterOptions { + requirements_only: false, + requirement: Some(2), + no_scenarios: false, + }, + ) + .unwrap(); + + assert_eq!(output.requirement_count, 1); + assert_eq!(output.requirements.len(), 1); + assert_eq!(output.requirements[0].text, "Req 2"); + assert_eq!(output.requirements[0].scenarios.len(), 1); + assert_eq!(output.requirements[0].scenarios[0].raw_text, "Scenario 2"); + } + + #[test] + fn test_build_filtered_spec_output_rejects_conflicting_flags() { + let err = build_filtered_spec_output( + SpecOutputInput { + id: "spec-a".to_string(), + title: "Spec A".to_string(), + overview: "Overview".to_string(), + requirements: vec![RequirementJson { + text: "Req 1".to_string(), + scenarios: vec![], + }], + metadata: SpecMetadataOutput { + version: "1.0.0".to_string(), + format: "openspec".to_string(), + source_path: None, + }, + }, + SpecFilterOptions { + requirements_only: true, + requirement: Some(1), + no_scenarios: false, + }, + ) + .unwrap_err(); + + assert!(err.to_string().contains("cannot be used together")); + } + + #[test] + fn test_show_spec_filter_flags_require_json() { + let err = validate_spec_filter_usage_for_output("spec", false, true).unwrap_err(); + + assert!(err.to_string().contains("require --json")); + } + + #[test] + fn test_show_change_filter_flags_do_not_require_json() { + validate_spec_filter_usage_for_output("change", false, true).unwrap(); + } } diff --git a/src/cli/templates.rs b/src/cli/templates.rs index 199763a..a6d3032 100644 --- a/src/cli/templates.rs +++ b/src/cli/templates.rs @@ -95,12 +95,12 @@ mod tests { let schema = get_embedded_spec_driven_schema().unwrap(); let resolved = ResolvedSchema { schema, - path: "vendor/OpenSpec/schemas/spec-driven/schema.yaml".to_string(), - source: crate::core::schema::SchemaSource::Package, + path: "/tmp/custom-schema/schema.yaml".to_string(), + source: crate::core::schema::SchemaSource::Project, }; assert_eq!( template_reference(&resolved, "proposal.md"), - "vendor/OpenSpec/schemas/spec-driven/templates/proposal.md" + "/tmp/custom-schema/templates/proposal.md" ); } } diff --git a/src/cli/validate.rs b/src/cli/validate.rs index 70aa429..a80aef4 100644 --- a/src/cli/validate.rs +++ b/src/cli/validate.rs @@ -1,4 +1,8 @@ use crate::core::config::OPENSPEC_DIR_NAME; +use std::collections::VecDeque; +use std::path::PathBuf; +use std::sync::{Arc, Mutex}; + use crate::core::error::{OpenSpecError, Result}; use crate::core::spec_parser::{ build_code_fence_mask, normalize_requirement_name, parse_delta_spec, RequirementBlock, @@ -50,20 +54,63 @@ pub struct BulkOutput { pub version: String, } -pub fn run_validate( - name: Option<&str>, - all: bool, - changes: bool, - specs: bool, - item_type: Option<&str>, - strict: bool, - json: bool, -) -> Result<()> { +const DEFAULT_VALIDATE_CONCURRENCY: usize = 6; + +/// Resolve the validation concurrency, mirroring upstream: the `--concurrency` flag wins when +/// it is a positive integer, else the `OPENSPEC_CONCURRENCY` env var (positive integer), else +/// the default. Non-positive / invalid values are ignored rather than used. +fn resolve_validate_concurrency(flag: Option) -> usize { + flag.filter(|&n| n > 0) + .or_else(|| { + std::env::var("OPENSPEC_CONCURRENCY") + .ok() + .and_then(|v| v.trim().parse::().ok()) + .filter(|&n| n > 0) + }) + .unwrap_or(DEFAULT_VALIDATE_CONCURRENCY) +} + +#[derive(Debug, Clone)] +struct BulkValidationTask { + id: String, + item_type: String, + path: PathBuf, +} + +pub struct ValidateOptions<'a> { + pub name: Option<&'a str>, + pub all: bool, + pub changes: bool, + pub specs: bool, + pub item_type: Option<&'a str>, + pub strict: bool, + pub json: bool, + pub concurrency: Option, +} + +pub fn run_validate(options: ValidateOptions<'_>) -> Result<()> { + let ValidateOptions { + name, + all, + changes, + specs, + item_type, + strict, + json, + concurrency, + } = options; let project_root = std::env::current_dir() .map_err(|e| OpenSpecError::Custom(format!("Failed to get current directory: {}", e)))?; if all || changes || specs { - return run_bulk_validation(&project_root, all || changes, all || specs, strict, json); + return run_bulk_validation( + &project_root, + all || changes, + all || specs, + strict, + json, + resolve_validate_concurrency(concurrency), + ); } let name = name.ok_or_else(|| { @@ -170,12 +217,103 @@ pub fn run_validate( Ok(()) } +fn execute_bounded_tasks(tasks: Vec, concurrency: usize, worker: F) -> Result> +where + T: Send + 'static, + R: Send + 'static, + F: Fn(T) -> R + Send + Sync + 'static, +{ + if tasks.is_empty() { + return Ok(vec![]); + } + + let worker_count = std::cmp::min(concurrency.max(1), tasks.len()); + let queue = Arc::new(Mutex::new(VecDeque::from(tasks))); + let worker = Arc::new(worker); + let (tx, rx) = std::sync::mpsc::channel(); + + let mut handles = Vec::new(); + for _ in 0..worker_count { + let queue = Arc::clone(&queue); + let worker = Arc::clone(&worker); + let tx = tx.clone(); + let handle = std::thread::spawn(move || loop { + let task = { + let mut queue = queue.lock().expect("bulk validation queue lock poisoned"); + queue.pop_front() + }; + + let Some(task) = task else { + break; + }; + + let result = worker(task); + if tx.send(result).is_err() { + break; + } + }); + handles.push(handle); + } + drop(tx); + + let results: Vec = rx.into_iter().collect(); + for handle in handles { + handle + .join() + .map_err(|_| OpenSpecError::Custom("Validation worker panicked".to_string()))?; + } + + Ok(results) +} + +fn execute_bulk_validation_tasks( + tasks: Vec, + strict: bool, + concurrency: usize, +) -> Result> { + execute_bounded_tasks(tasks, concurrency, move |task| { + let start = std::time::Instant::now(); + let outcome = if task.item_type == "change" { + validate_change(&task.path, strict) + } else { + validate_spec(&task.path, strict) + }; + let duration_ms = start.elapsed().as_millis() as u64; + + match outcome { + Ok(report) => BulkItemResult { + id: task.id, + item_type: task.item_type, + valid: report.valid, + issues: report.issues, + duration_ms, + }, + Err(err) => BulkItemResult { + id: task.id, + item_type: task.item_type, + valid: false, + issues: vec![ValidationIssue { + level: "ERROR".to_string(), + path: "file".to_string(), + message: err.to_string(), + }], + duration_ms, + }, + } + }) +} + +fn sort_bulk_results(results: &mut [BulkItemResult]) { + results.sort_by(|a, b| a.id.cmp(&b.id).then_with(|| a.item_type.cmp(&b.item_type))); +} + fn run_bulk_validation( project_root: &std::path::Path, validate_changes: bool, validate_specs: bool, strict: bool, json: bool, + concurrency: usize, ) -> Result<()> { let change_ids = if validate_changes { get_available_changes(project_root)? @@ -232,58 +370,35 @@ fn run_bulk_validation( return Ok(()); } - let mut results: Vec = Vec::new(); - let mut passed = 0; - let mut failed = 0; - + let mut tasks: Vec = Vec::new(); for id in &change_ids { - let start = std::time::Instant::now(); - let change_dir = project_root - .join(OPENSPEC_DIR_NAME) - .join("changes") - .join(id); - let report = validate_change(&change_dir, strict)?; - let duration_ms = start.elapsed().as_millis() as u64; - let valid = report.valid; - if valid { - passed += 1; - } else { - failed += 1; - } - results.push(BulkItemResult { + tasks.push(BulkValidationTask { id: id.clone(), item_type: "change".to_string(), - valid, - issues: report.issues, - duration_ms, + path: project_root + .join(OPENSPEC_DIR_NAME) + .join("changes") + .join(id), }); } - for id in &spec_ids { - let start = std::time::Instant::now(); - let spec_path = project_root - .join(OPENSPEC_DIR_NAME) - .join("specs") - .join(id) - .join("spec.md"); - let report = validate_spec(&spec_path, strict)?; - let duration_ms = start.elapsed().as_millis() as u64; - let valid = report.valid; - if valid { - passed += 1; - } else { - failed += 1; - } - results.push(BulkItemResult { + tasks.push(BulkValidationTask { id: id.clone(), item_type: "spec".to_string(), - valid, - issues: report.issues, - duration_ms, + path: project_root + .join(OPENSPEC_DIR_NAME) + .join("specs") + .join(id) + .join("spec.md"), }); } - results.sort_by(|a, b| a.id.cmp(&b.id)); + let mut results = execute_bulk_validation_tasks(tasks, strict, concurrency)?; + + let passed = results.iter().filter(|result| result.valid).count(); + let failed = results.len() - passed; + + sort_bulk_results(&mut results); if json { let output = BulkOutput { @@ -860,6 +975,37 @@ fn count_scenarios(block_raw: &str) -> usize { #[cfg(test)] mod tests { use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use std::time::Duration; + + #[test] + fn resolve_concurrency_flag_env_default() { + // This test owns the OPENSPEC_CONCURRENCY env var (no other test touches it). + std::env::remove_var("OPENSPEC_CONCURRENCY"); + assert_eq!(resolve_validate_concurrency(Some(3)), 3); + // Non-positive flag is ignored → default (no env set). + assert_eq!( + resolve_validate_concurrency(Some(0)), + DEFAULT_VALIDATE_CONCURRENCY + ); + assert_eq!( + resolve_validate_concurrency(None), + DEFAULT_VALIDATE_CONCURRENCY + ); + // Env fallback when no flag. + std::env::set_var("OPENSPEC_CONCURRENCY", "4"); + assert_eq!(resolve_validate_concurrency(None), 4); + // Flag wins over env. + assert_eq!(resolve_validate_concurrency(Some(2)), 2); + // Invalid env is ignored → default. + std::env::set_var("OPENSPEC_CONCURRENCY", "bogus"); + assert_eq!( + resolve_validate_concurrency(None), + DEFAULT_VALIDATE_CONCURRENCY + ); + std::env::remove_var("OPENSPEC_CONCURRENCY"); + } #[test] fn test_contains_shall_or_must() { @@ -1008,4 +1154,73 @@ mod tests { let strict = validate_spec(&spec_path, true).unwrap(); assert!(!strict.valid); } + #[test] + fn test_sort_bulk_results_orders_by_id_then_type() { + let mut results = vec![ + BulkItemResult { + id: "beta".to_string(), + item_type: "spec".to_string(), + valid: true, + issues: vec![], + duration_ms: 1, + }, + BulkItemResult { + id: "alpha".to_string(), + item_type: "spec".to_string(), + valid: true, + issues: vec![], + duration_ms: 1, + }, + BulkItemResult { + id: "alpha".to_string(), + item_type: "change".to_string(), + valid: true, + issues: vec![], + duration_ms: 1, + }, + ]; + + sort_bulk_results(&mut results); + + assert_eq!(results[0].id, "alpha"); + assert_eq!(results[0].item_type, "change"); + assert_eq!(results[1].id, "alpha"); + assert_eq!(results[1].item_type, "spec"); + assert_eq!(results[2].id, "beta"); + } + + #[test] + fn test_execute_bounded_tasks_respects_concurrency_limit() { + let active = Arc::new(AtomicUsize::new(0)); + let max_active = Arc::new(AtomicUsize::new(0)); + + let mut results = execute_bounded_tasks((0..8).collect::>(), 3, { + let active = Arc::clone(&active); + let max_active = Arc::clone(&max_active); + move |task| { + let current = active.fetch_add(1, Ordering::SeqCst) + 1; + let mut observed = max_active.load(Ordering::SeqCst); + while current > observed { + match max_active.compare_exchange( + observed, + current, + Ordering::SeqCst, + Ordering::SeqCst, + ) { + Ok(_) => break, + Err(value) => observed = value, + } + } + + std::thread::sleep(Duration::from_millis(20)); + active.fetch_sub(1, Ordering::SeqCst); + task + } + }) + .unwrap(); + + results.sort(); + assert_eq!(results, (0..8).collect::>()); + assert!(max_active.load(Ordering::SeqCst) <= 3); + } } diff --git a/src/core/schema.rs b/src/core/schema.rs index b060cb2..f82d2bb 100644 --- a/src/core/schema.rs +++ b/src/core/schema.rs @@ -50,6 +50,10 @@ pub struct ChangeMetadata { #[serde(default)] pub created: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub goal: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub affected_areas: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] pub initiative: Option, } @@ -254,7 +258,7 @@ pub fn get_user_schemas_dir() -> std::path::PathBuf { } pub fn get_package_schemas_dir() -> std::path::PathBuf { - std::path::PathBuf::from("vendor/OpenSpec/schemas") + std::path::PathBuf::from("embedded:package-schemas") } pub fn get_embedded_spec_driven_schema() -> Result { @@ -289,17 +293,6 @@ pub fn resolve_schema(name: &str, project_root: Option<&Path>) -> Result) -> Vec { } } - let package_dir = get_package_schemas_dir(); - if package_dir.exists() { - if let Ok(entries) = std::fs::read_dir(&package_dir) { - for entry in entries.flatten() { - if entry.path().is_dir() { - let name = entry.file_name().to_string_lossy().to_string(); - if !seen_names.contains(&name) { - let schema_path = entry.path().join("schema.yaml"); - if schema_path.exists() { - if let Ok(schema) = load_schema(&schema_path) { - schemas.push(SchemaInfo { - name: name.clone(), - description: schema.description.clone().unwrap_or_default(), - artifacts: schema - .artifacts - .iter() - .map(|a| a.id.clone()) - .collect(), - source: SchemaSource::Package, - }); - seen_names.insert(name); - } - } - } - } - } - } - } - if !seen_names.contains("spec-driven") { if let Ok(schema) = get_embedded_spec_driven_schema() { schemas.push(SchemaInfo { @@ -626,6 +590,13 @@ artifacts: assert_eq!(resolved.source, SchemaSource::Package); } + #[test] + fn test_resolve_schema_spec_driven_uses_embedded_path() { + let resolved = resolve_schema("spec-driven", None).unwrap(); + assert_eq!(resolved.path, "embedded:spec-driven.yaml"); + assert_eq!(resolved.source, SchemaSource::Package); + } + #[test] fn test_resolve_schema_not_found() { let result = resolve_schema("nonexistent-schema", None); @@ -662,6 +633,8 @@ artifacts: let meta: ChangeMetadata = serde_yaml::from_str(yaml).unwrap(); assert_eq!(meta.schema, "spec-driven"); assert_eq!(meta.created.as_deref(), Some("2026-01-01")); + assert!(meta.goal.is_none()); + assert!(meta.affected_areas.is_none()); assert!(meta.initiative.is_none()); // Serializing back should NOT emit an `initiative` key. @@ -676,6 +649,8 @@ artifacts: let meta = ChangeMetadata { schema: "spec-driven".to_string(), created: Some("2026-01-01".to_string()), + goal: Some("Ship telemetry".to_string()), + affected_areas: Some(vec!["cli".to_string(), "docs".to_string()]), initiative: Some(InitiativeLink { store: "team".to_string(), id: "roadmap".to_string(), @@ -684,6 +659,11 @@ artifacts: let serialized = serde_yaml::to_string(&meta).unwrap(); let reparsed: ChangeMetadata = serde_yaml::from_str(&serialized).unwrap(); assert_eq!(reparsed, meta); + assert_eq!(reparsed.goal.as_deref(), Some("Ship telemetry")); + assert_eq!( + reparsed.affected_areas.as_deref(), + Some(&["cli".to_string(), "docs".to_string()][..]) + ); let link = reparsed.initiative.unwrap(); assert_eq!(link.store, "team"); assert_eq!(link.id, "roadmap"); diff --git a/src/lib.rs b/src/lib.rs index e7eb647..f8779c5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,3 +4,6 @@ pub mod core; pub mod telemetry; pub mod templates; pub mod utils; + +#[cfg(test)] +pub mod test_support; diff --git a/src/telemetry/client.rs b/src/telemetry/client.rs index 9ce2052..ee9bccb 100644 --- a/src/telemetry/client.rs +++ b/src/telemetry/client.rs @@ -7,7 +7,7 @@ const POSTHOG_API_KEY: &str = "phc_Hthu8YvaIJ9QaFKyTG4TbVwkbd5ktcAFzVTKeMmoW2g"; const POSTHOG_HOST: &str = "https://edge.openspec.dev"; static ANONYMOUS_ID: OnceLock = OnceLock::new(); -static PENDING_EVENTS: OnceLock>> = OnceLock::new(); +static PENDING_EVENTS: OnceLock>> = OnceLock::new(); #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct TelemetryConfig { @@ -135,9 +135,11 @@ pub fn track_command(command_name: &str, version: &str) { }, }; + let event_value = serde_json::to_value(&event).unwrap_or(serde_json::Value::Null); + if let Some(pending) = PENDING_EVENTS.get() { if let Ok(mut events) = pending.lock() { - events.push(event); + events.push(event_value); return; } } diff --git a/src/test_support.rs b/src/test_support.rs new file mode 100644 index 0000000..95833ed --- /dev/null +++ b/src/test_support.rs @@ -0,0 +1,25 @@ +use std::path::{Path, PathBuf}; +use std::sync::{Mutex, OnceLock}; + +pub fn cwd_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) +} + +struct CwdReset { + previous: PathBuf, +} + +impl Drop for CwdReset { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.previous); + } +} + +pub fn with_current_dir(dir: &Path, f: impl FnOnce() -> T) -> T { + let _guard = cwd_lock().lock().unwrap_or_else(|err| err.into_inner()); + let previous = std::env::current_dir().unwrap(); + std::env::set_current_dir(dir).unwrap(); + let _reset = CwdReset { previous }; + f() +} diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 1c7b18c..340dd4a 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1,3 +1,4 @@ +use serde_json::Value; use std::path::PathBuf; use tempfile::TempDir; @@ -17,6 +18,7 @@ fn run_openspec(args: &[&str], cwd: &PathBuf) -> Result { let output = std::process::Command::new(&binary) .args(args) .current_dir(cwd) + .env_remove("NO_COLOR") .output() .map_err(|e| format!("Failed to execute openspec: {}", e))?; @@ -27,6 +29,163 @@ fn run_openspec(args: &[&str], cwd: &PathBuf) -> Result { } } +fn run_openspec_with_clean_env(args: &[&str], cwd: &PathBuf) -> Result { + let binary = get_binary_path(); + let env_root = TempDir::new().unwrap(); + let xdg_config_home = env_root.path().join("config-home"); + let xdg_data_home = env_root.path().join("data-home"); + std::fs::create_dir_all(&xdg_config_home).unwrap(); + std::fs::create_dir_all(&xdg_data_home).unwrap(); + + let output = std::process::Command::new(&binary) + .args(args) + .current_dir(cwd) + .env_remove("NO_COLOR") + .env("HOME", env_root.path()) + .env("XDG_CONFIG_HOME", &xdg_config_home) + .env("XDG_DATA_HOME", &xdg_data_home) + .output() + .map_err(|e| format!("Failed to execute openspec: {}", e))?; + + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).to_string()) + } else { + Err(String::from_utf8_lossy(&output.stderr).to_string()) + } +} + +#[derive(Debug)] +struct CapturedCommandOutput { + stdout: String, + stderr: String, + success: bool, +} + +fn run_openspec_capture(args: &[&str], cwd: &PathBuf) -> CapturedCommandOutput { + let binary = get_binary_path(); + let output = std::process::Command::new(&binary) + .args(args) + .current_dir(cwd) + .env_remove("NO_COLOR") + .output() + .unwrap(); + + CapturedCommandOutput { + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + success: output.status.success(), + } +} + +fn run_openspec_capture_in_env_root( + args: &[&str], + cwd: &PathBuf, + env_root: &std::path::Path, + extra_env: &[(&str, &str)], +) -> CapturedCommandOutput { + let binary = get_binary_path(); + let xdg_config_home = env_root.join("config-home"); + let xdg_data_home = env_root.join("data-home"); + std::fs::create_dir_all(&xdg_config_home).unwrap(); + std::fs::create_dir_all(&xdg_data_home).unwrap(); + + let mut command = std::process::Command::new(&binary); + command + .args(args) + .current_dir(cwd) + .env_remove("NO_COLOR") + .env("HOME", env_root) + .env("XDG_CONFIG_HOME", &xdg_config_home) + .env("XDG_DATA_HOME", &xdg_data_home); + + for (key, value) in extra_env { + command.env(key, value); + } + + let output = command.output().unwrap(); + + CapturedCommandOutput { + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + success: output.status.success(), + } +} + +fn run_openspec_capture_with_clean_env( + args: &[&str], + cwd: &PathBuf, + extra_env: &[(&str, &str)], +) -> CapturedCommandOutput { + let env_root = TempDir::new().unwrap(); + run_openspec_capture_in_env_root(args, cwd, env_root.path(), extra_env) +} + +fn assert_embedded_schema_commands_work(cwd: &PathBuf) { + let schemas_result = run_openspec_with_clean_env(&["schemas"], cwd); + assert!( + schemas_result.is_ok(), + "schemas failed: {:?}", + schemas_result + ); + let schemas_output = schemas_result.unwrap(); + assert!( + schemas_output.contains("spec-driven"), + "spec-driven schema not listed" + ); + + let schemas_json_result = run_openspec_with_clean_env(&["schemas", "--json"], cwd); + assert!( + schemas_json_result.is_ok(), + "schemas --json failed: {:?}", + schemas_json_result + ); + let schemas_json: Value = serde_json::from_str(&schemas_json_result.unwrap()).unwrap(); + let spec_driven = schemas_json["schemas"] + .as_array() + .unwrap() + .iter() + .find(|schema| schema["name"] == "spec-driven") + .unwrap(); + assert_eq!(spec_driven["source"], "package"); + + let which_result = + run_openspec_with_clean_env(&["schema", "which", "spec-driven", "--json"], cwd); + assert!( + which_result.is_ok(), + "schema which failed: {:?}", + which_result + ); + let which_json: Value = serde_json::from_str(&which_result.unwrap()).unwrap(); + assert_eq!(which_json["name"], "spec-driven"); + assert_eq!(which_json["source"], "package"); + assert_eq!(which_json["path"], "embedded:spec-driven.yaml"); + assert_eq!(which_json["shadows"].as_array().unwrap().len(), 0); + + let templates_result = run_openspec_with_clean_env(&["templates", "--json"], cwd); + assert!( + templates_result.is_ok(), + "templates --json failed: {:?}", + templates_result + ); + let templates_json: Value = serde_json::from_str(&templates_result.unwrap()).unwrap(); + assert_eq!(templates_json["proposal"], "proposal.md"); + assert_eq!(templates_json["specs"], "spec.md"); + assert_eq!(templates_json["design"], "design.md"); + assert_eq!(templates_json["tasks"], "tasks.md"); + + let validate_result = + run_openspec_with_clean_env(&["schema", "validate", "spec-driven", "--json"], cwd); + assert!( + validate_result.is_ok(), + "schema validate failed: {:?}", + validate_result + ); + let validate_json: Value = serde_json::from_str(&validate_result.unwrap()).unwrap(); + assert_eq!(validate_json["name"], "spec-driven"); + assert_eq!(validate_json["valid"], true); + assert_eq!(validate_json["issues"].as_array().unwrap().len(), 0); +} + #[test] fn test_init_creates_directory_structure() { let temp_dir = TempDir::new().unwrap(); @@ -94,6 +253,113 @@ fn test_new_change_creates_directory() { ); } +#[test] +fn test_new_change_json_output_and_initiative_link() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + let store_path = path.join("team-context"); + let store_path_str = store_path.to_string_lossy().to_string(); + + run_openspec(&["init", "--tools", "none", "."], &path).unwrap(); + let create_initiative = run_openspec( + &[ + "initiative", + "create", + "roadmap", + "--title", + "Roadmap", + "--summary", + "Shared planning context", + "--store-path", + &store_path_str, + ], + &path, + ); + assert!( + create_initiative.is_ok(), + "initiative create failed: {:?}", + create_initiative + ); + + let result = run_openspec( + &[ + "new", + "change", + "test-feature", + "--initiative", + "roadmap", + "--store-path", + &store_path_str, + "--json", + ], + &path, + ); + assert!(result.is_ok(), "new change --json failed: {:?}", result); + + let output = result.unwrap(); + let value: Value = serde_json::from_str(&output).unwrap(); + assert_eq!(value["change"]["id"], "test-feature"); + assert_eq!(value["change"]["path"], "openspec/changes/test-feature"); + assert_eq!( + value["change"]["metadataPath"], + "openspec/changes/test-feature/.openspec.yaml" + ); + // Linked change → initiative present in the output. + assert_eq!(value["initiative"]["store"], "team-context"); + assert_eq!(value["initiative"]["id"], "roadmap"); + + let metadata = + std::fs::read_to_string(path.join("openspec/changes/test-feature/.openspec.yaml")).unwrap(); + assert!(metadata.contains("initiative:")); + assert!(metadata.contains("store: team-context")); + assert!(metadata.contains("id: roadmap")); +} + +#[test] +fn test_new_change_json_output_writes_goal_and_affected_areas() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + + run_openspec(&["init", "--tools", "none", "."], &path).unwrap(); + + let result = run_openspec( + &[ + "new", + "change", + "add-telemetry", + "--description", + "Add telemetry", + "--goal", + "Ship telemetry", + "--affected-areas", + "cli, docs , ,", + "--json", + ], + &path, + ); + assert!(result.is_ok(), "new change --json failed: {:?}", result); + + let output = result.unwrap(); + let value: Value = serde_json::from_str(&output).unwrap(); + assert_eq!(value["change"]["id"], "add-telemetry"); + assert_eq!(value["change"]["path"], "openspec/changes/add-telemetry"); + assert!(value.get("initiative").is_none()); + + let metadata_path = path + .join("openspec") + .join("changes") + .join("add-telemetry") + .join(".openspec.yaml"); + let metadata: serde_yaml::Value = + serde_yaml::from_str(&std::fs::read_to_string(metadata_path).unwrap()).unwrap(); + assert_eq!(metadata["schema"], "spec-driven"); + assert_eq!(metadata["goal"], "Ship telemetry"); + assert_eq!( + metadata["affected_areas"], + serde_yaml::to_value(vec!["cli", "docs"]).unwrap() + ); +} + #[test] fn test_status_shows_change_status() { let temp_dir = TempDir::new().unwrap(); @@ -165,10 +431,162 @@ fn test_validate_empty_change() { #[test] fn test_config_list() { - let result = run_openspec(&["config", "--list"], &std::env::current_dir().unwrap()); + let result = run_openspec(&["config", "list"], &std::env::current_dir().unwrap()); assert!(result.is_ok(), "config list failed: {:?}", result); } +#[test] +fn test_config_subcommands_roundtrip_with_clean_env() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + let env_root = TempDir::new().unwrap(); + + let set_profile = run_openspec_capture_in_env_root( + &["config", "set", "profile", "custom"], + &path, + env_root.path(), + &[], + ); + assert!( + set_profile.success, + "config set profile failed: {:?}", + set_profile + ); + + let set_feature_flag = run_openspec_capture_in_env_root( + &["config", "set", "featureFlags.beta", "true"], + &path, + env_root.path(), + &[], + ); + assert!( + set_feature_flag.success, + "config set feature flag failed: {:?}", + set_feature_flag + ); + + let profile_output = + run_openspec_capture_in_env_root(&["config", "profile"], &path, env_root.path(), &[]); + assert!( + profile_output.success, + "config profile failed: {:?}", + profile_output + ); + assert_eq!(profile_output.stdout.trim(), "custom"); + + let profile_core = run_openspec_capture_in_env_root( + &["config", "profile", "core"], + &path, + env_root.path(), + &[], + ); + assert!( + profile_core.success, + "config profile core failed: {:?}", + profile_core + ); + assert_eq!(profile_core.stdout.trim(), "core"); + + let list_output = run_openspec_capture_in_env_root( + &["config", "list", "--json"], + &path, + env_root.path(), + &[], + ); + assert!( + list_output.success, + "config list --json failed: {:?}", + list_output + ); + let list_json: Value = serde_json::from_str(&list_output.stdout).unwrap(); + assert_eq!(list_json["profile"], "core"); + assert_eq!(list_json["featureFlags"]["beta"], true); + + let unset_feature_flag = run_openspec_capture_in_env_root( + &["config", "unset", "featureFlags.beta"], + &path, + env_root.path(), + &[], + ); + assert!( + unset_feature_flag.success, + "config unset feature flag failed: {:?}", + unset_feature_flag + ); + + let get_missing_flag = run_openspec_capture_in_env_root( + &["config", "get", "featureFlags.beta"], + &path, + env_root.path(), + &[], + ); + assert!( + !get_missing_flag.success, + "config get missing key should fail" + ); + assert!(get_missing_flag.stderr.contains("not found")); + + // `reset` requires --all (mirrors upstream); bare reset must be refused. + let reset_no_flag = + run_openspec_capture_in_env_root(&["config", "reset"], &path, env_root.path(), &[]); + assert!( + !reset_no_flag.success, + "config reset without --all should fail" + ); + assert!(reset_no_flag.stderr.contains("--all")); + + let reset_output = run_openspec_capture_in_env_root( + &["config", "reset", "--all"], + &path, + env_root.path(), + &[], + ); + assert!( + reset_output.success, + "config reset --all failed: {:?}", + reset_output + ); + + let after_reset = run_openspec_capture_in_env_root( + &["config", "list", "--json"], + &path, + env_root.path(), + &[], + ); + assert!( + after_reset.success, + "config list after reset failed: {:?}", + after_reset + ); + let reset_json: Value = serde_json::from_str(&after_reset.stdout).unwrap(); + assert_eq!(reset_json["profile"], "core"); + assert!(reset_json["featureFlags"].as_object().unwrap().is_empty()); +} + +#[test] +fn test_feedback_falls_back_to_manual_url_without_gh() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + + // Force the gh-not-found fallback by pointing PATH at an empty dir, so the test never + // files a real GitHub issue. Feedback files an issue (not telemetry), so it is not gated + // by OPENSPEC_TELEMETRY. + let empty_bin = temp_dir.path().join("empty-bin"); + std::fs::create_dir_all(&empty_bin).unwrap(); + + let result = run_openspec_capture_with_clean_env( + &["feedback", "compatibility looks good", "--body", "details"], + &path, + &[("PATH", empty_bin.to_str().unwrap())], + ); + assert!(result.success, "feedback fallback failed: {:?}", result); + assert!(result.stdout.contains("Manual submission required")); + assert!(result + .stdout + .contains("https://github.com/oonid/OpenSpec-rs/issues/new?")); + assert!(result.stdout.contains("Feedback%3A%20compatibility")); +} + #[test] fn test_version_flag() { let temp_dir = TempDir::new().unwrap(); @@ -178,6 +596,56 @@ fn test_version_flag() { assert!(result.is_ok(), "version flag failed: {:?}", result); let output = result.unwrap(); assert!(output.contains("openspec"), "version output missing name"); + assert!( + output.contains(env!("CARGO_PKG_VERSION")), + "version output missing current package version" + ); +} + +#[test] +fn test_no_interactive_flag_is_accepted_by_representative_commands() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + + let init_result = run_openspec(&["--no-interactive", "init", "--tools", "none", "."], &path); + assert!( + init_result.is_ok(), + "init with --no-interactive failed: {:?}", + init_result + ); + + let new_change_result = run_openspec( + &[ + "--no-interactive", + "new", + "change", + "test-feature", + "--description", + "Test", + ], + &path, + ); + assert!( + new_change_result.is_ok(), + "new change with --no-interactive failed: {:?}", + new_change_result + ); + + let validate_result = run_openspec( + &[ + "--no-interactive", + "validate", + "--changes", + "--concurrency", + "1", + ], + &path, + ); + assert!( + validate_result.is_ok(), + "validate with --no-interactive failed: {:?}", + validate_result + ); } #[test] @@ -230,6 +698,153 @@ fn test_show_json_output() { ); } +#[test] +fn test_show_spec_filter_flags_apply_to_json_output() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + let spec_dir = path.join("openspec/specs/test-spec"); + + run_openspec(&["init", "--tools", "none", "."], &path).unwrap(); + std::fs::create_dir_all(&spec_dir).unwrap(); + std::fs::write( + spec_dir.join("spec.md"), + "# Test Spec + +## Purpose +Spec overview. + +## Requirements + +### Requirement: First +The system SHALL do the first thing. + +#### Scenario: First +- **WHEN** the first case happens +- **THEN** the first result is produced + +### Requirement: Second +The system SHALL do the second thing. + +#### Scenario: Second +- **WHEN** the second case happens +- **THEN** the second result is produced +", + ) + .unwrap(); + + let requirements_only = run_openspec( + &[ + "show", + "test-spec", + "--type", + "spec", + "--json", + "--requirements", + ], + &path, + ) + .unwrap(); + let requirements_only_json: Value = serde_json::from_str(&requirements_only).unwrap(); + assert_eq!(requirements_only_json["requirementCount"], 2); + assert_eq!( + requirements_only_json["requirements"] + .as_array() + .unwrap() + .len(), + 2 + ); + assert!(requirements_only_json["requirements"] + .as_array() + .unwrap() + .iter() + .all(|requirement| requirement["scenarios"].as_array().unwrap().is_empty())); + + let single_requirement = run_openspec( + &[ + "show", + "test-spec", + "--type", + "spec", + "--json", + "--requirement", + "2", + ], + &path, + ) + .unwrap(); + let single_requirement_json: Value = serde_json::from_str(&single_requirement).unwrap(); + assert_eq!(single_requirement_json["requirementCount"], 1); + assert_eq!( + single_requirement_json["requirements"] + .as_array() + .unwrap() + .len(), + 1 + ); + assert_eq!( + single_requirement_json["requirements"][0]["text"], + "The system SHALL do the second thing." + ); + assert_eq!( + single_requirement_json["requirements"][0]["scenarios"] + .as_array() + .unwrap() + .len(), + 1 + ); + + let no_scenarios = run_openspec( + &[ + "show", + "test-spec", + "--type", + "spec", + "--json", + "--no-scenarios", + ], + &path, + ) + .unwrap(); + let no_scenarios_json: Value = serde_json::from_str(&no_scenarios).unwrap(); + assert_eq!(no_scenarios_json["requirementCount"], 2); + assert!(no_scenarios_json["requirements"] + .as_array() + .unwrap() + .iter() + .all(|requirement| requirement["scenarios"].as_array().unwrap().is_empty())); + + let plain_text = run_openspec_capture( + &["show", "test-spec", "--type", "spec", "--requirements"], + &path, + ); + assert!(!plain_text.success, "show without --json should fail"); + assert!(plain_text.stderr.contains("require --json")); +} + +#[test] +fn test_show_change_warns_when_spec_filter_flags_are_used() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + + run_openspec(&["init", "--tools", "none", "."], &path).unwrap(); + run_openspec( + &["new", "change", "test-feature", "--description", "Test"], + &path, + ) + .unwrap(); + + let result = run_openspec_capture(&["show", "test-feature", "--json", "--requirements"], &path); + assert!( + result.success, + "show change should still succeed: {:?}", + result + ); + assert!(result + .stderr + .contains("Ignoring spec-only flags for change 'test-feature'")); + assert!(result.stdout.contains("\"id\": \"test-feature\"")); +} + #[test] fn test_status_json_output() { let temp_dir = TempDir::new().unwrap(); @@ -293,6 +908,19 @@ fn test_schemas_json_output() { ); } +#[test] +fn test_embedded_schema_commands_work_from_repo_root() { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + assert_embedded_schema_commands_work(&path); +} + +#[test] +fn test_embedded_schema_commands_work_from_isolated_dir() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().to_path_buf(); + assert_embedded_schema_commands_work(&path); +} + #[test] fn test_completion_generate() { let temp_dir = TempDir::new().unwrap();