From 39da06a7302e3572c7852ee7eadcecd847afc488 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Wed, 15 Apr 2026 16:52:29 -0400 Subject: [PATCH 1/8] Change proposal: allow-specless-changes --- .../allow-specless-changes/.openspec.yaml | 2 + .../changes/allow-specless-changes/design.md | 154 ++++++++++++++++++ .../allow-specless-changes/proposal.md | 63 +++++++ .../specs/artifact-graph/spec.md | 37 +++++ .../specs/cli-artifact-workflow/spec.md | 47 ++++++ .../specs/cli-validate/spec.md | 48 ++++++ .../specs/config-loading/spec.md | 69 ++++++++ .../changes/allow-specless-changes/tasks.md | 43 +++++ 8 files changed, 463 insertions(+) create mode 100644 openspec/changes/allow-specless-changes/.openspec.yaml create mode 100644 openspec/changes/allow-specless-changes/design.md create mode 100644 openspec/changes/allow-specless-changes/proposal.md create mode 100644 openspec/changes/allow-specless-changes/specs/artifact-graph/spec.md create mode 100644 openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md create mode 100644 openspec/changes/allow-specless-changes/specs/cli-validate/spec.md create mode 100644 openspec/changes/allow-specless-changes/specs/config-loading/spec.md create mode 100644 openspec/changes/allow-specless-changes/tasks.md diff --git a/openspec/changes/allow-specless-changes/.openspec.yaml b/openspec/changes/allow-specless-changes/.openspec.yaml new file mode 100644 index 000000000..859e7bb98 --- /dev/null +++ b/openspec/changes/allow-specless-changes/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-15 diff --git a/openspec/changes/allow-specless-changes/design.md b/openspec/changes/allow-specless-changes/design.md new file mode 100644 index 000000000..374774899 --- /dev/null +++ b/openspec/changes/allow-specless-changes/design.md @@ -0,0 +1,154 @@ +## Context + +OpenSpec's `spec-driven` schema requires every change to include delta specs. Two separate systems enforce this: + +1. **Validation** (`Validator.validateChangeDeltaSpecs()`): hard-fails with `CHANGE_NO_DELTAS` when `totalDeltas === 0` +2. **Artifact graph** (`detectCompleted()` in `state.ts`): `specs` artifact completion is determined by file existence (`specs/**/*.md`). Since `tasks` depends on `specs`, a change with no spec files can never reach `tasks: ready`. + +No config is passed into either system today. The Validator receives only `strictMode`; `detectCompleted()` receives only the graph and change directory. + +### Key code locations + +- `src/core/project-config.ts` — `ProjectConfigSchema` (Zod), `readProjectConfig()` +- `src/core/validation/validator.ts` — `validateChangeDeltaSpecs()`, line 269: `totalDeltas === 0` → ERROR +- `src/core/validation/constants.ts` — `VALIDATION_MESSAGES.CHANGE_NO_DELTAS` +- `src/core/artifact-graph/state.ts` — `detectCompleted()`: iterates artifacts, checks file existence +- `src/commands/validate.ts` — `ValidateCommand.validateByType()`: creates Validator, calls validation + +### Existing patterns + +- Config is flat: `schema` (string), `context` (string), `rules` (record of string arrays). No nested config objects. +- No `z.enum` or tri-state config patterns exist today. +- Validation levels are `'ERROR' | 'WARNING' | 'INFO'`. Report validity: `errors === 0` (non-strict) or `errors === 0 && warnings === 0` (strict). +- The proposal instruction and template are static (from `schema.yaml`). Config-aware guidance uses the existing `rules` injection mechanism. + +## Goals / Non-Goals + +**Goals:** +- Add top-level `requireSpecDeltas` tri-state to project config: `"error"` (default) | `"warn"` | `false` +- Validation respects the setting: ERROR, WARNING, or silent +- Artifact graph respects the setting: synthetically complete `specs` when not required +- Propose workflow works end-to-end for specless changes (with `rules.proposal` guidance in config) + +**Non-Goals:** +- Per-change override via `.openspec.yaml` — deferred (adds I/O to `detectCompleted()`) +- Dynamic instruction adaptation based on config — projects use `rules` config instead +- New `--no-specs` flag for `openspec new change` — potential follow-up + +## Decisions + +### 1. Config shape: top-level `requireSpecDeltas` tri-state + +```typescript +const RequireSpecDeltasSchema = z.union([ + z.enum(["error", "warn"]), + z.literal(false), +]); + +// Added to ProjectConfigSchema alongside schema, context, rules +requireSpecDeltas: RequireSpecDeltasSchema.optional() +``` + +**Why top-level:** The existing config is flat — `schema`, `context`, `rules`. A top-level key follows the same pattern. This setting affects both validation and the artifact graph, so it doesn't belong under a `validation` namespace. + +**Why tri-state over boolean:** The primary use case needs full suppression (no output). A boolean forces a choice between "error" and "silent", losing the middle ground of "visible but non-blocking." + +**Why `false` instead of `"off"`:** `false` is YAML's native representation for "disabled." Bare `off` is a YAML 1.1 boolean alias for `false`, which would cause confusion. + +### 2. Thread config into the Validator + +Pass a config object instead of bare `strictMode`: + +```typescript +type SpecDeltaRequirement = 'error' | 'warn' | false; + +interface ValidatorConfig { + strictMode?: boolean; + requireSpecDeltas?: SpecDeltaRequirement; // default 'error' +} +``` + +`ValidateCommand` reads project config via `readProjectConfig(process.cwd())` and passes `requireSpecDeltas` through. The Validator remains a pure logic class with no I/O. + +### 3. Tri-state behavior in `validateChangeDeltaSpecs()` + +```typescript +if (totalDeltas === 0) { + if (this.requireSpecDeltas === 'error') { + issues.push({ level: 'ERROR', ... }); + } else if (this.requireSpecDeltas === 'warn') { + issues.push({ level: 'WARNING', ... }); + } + // false → no issue emitted +} +``` + +Existing `createReport()` handles the rest: `valid = strictMode ? (errors === 0 && warnings === 0) : (errors === 0)`. + +### 4. Synthetic completion in `detectCompleted()` + +In `state.ts`, after the file-existence scan, if the `specs` artifact is not completed and the config allows specless changes: + +```typescript +export function detectCompleted( + graph: ArtifactGraph, + changeDir: string, + options?: { requireSpecDeltas?: 'error' | 'warn' | false } +): CompletedSet { + const completed = new Set(); + // ... existing file-existence loop ... + + // Synthetically mark specs as complete when not required + const specsArtifact = graph.getAllArtifacts().find(a => a.id === 'specs'); + if (specsArtifact && !completed.has('specs') && + options?.requireSpecDeltas !== undefined && + options?.requireSpecDeltas !== 'error') { + completed.add('specs'); + } + + return completed; +} +``` + +~5 lines added. `loadChangeContext()` in `instruction-loader.ts` reads project config and passes it through. + +**Why hardcode `'specs'`:** The `specs` artifact is the only one with a glob pattern that might legitimately have zero files. A more general "optional artifacts" system would be over-engineering for now. + +### 5. "Skipped" status in display + +`formatChangeStatus()` already has access to the graph, completed set, and change directory. To distinguish "done" from "skipped": + +```typescript +// In formatChangeStatus(), when mapping artifact statuses: +if (context.completed.has(artifact.id)) { + const hasFiles = artifactOutputExists(context.changeDir, artifact.generates); + return { + id: artifact.id, + outputPath: artifact.generates, + status: hasFiles ? 'done' : 'skipped', + }; +} +``` + +If the artifact is in the completed set but has no files on disk, it was synthetically completed → `'skipped'`. + +Add to `ArtifactStatus.status`: `'done' | 'ready' | 'blocked' | 'skipped'` +Add to `getStatusIndicator()`: `'skipped'` → `[~]` with `chalk.dim` +Add to `getStatusColor()`: `'skipped'` → `chalk.dim` + +Skipped artifacts count toward the completed total in `printStatusText()` (they unblock downstream work). + +### 6. No schema changes needed + +`apply.requires: [tasks]` doesn't include `specs` directly. The blocker was `tasks.requires: [specs, design]` in the artifact dependency graph, which the synthetic completion resolves. + +## Risks / Trade-offs + +- **[Risk] Teams accidentally skip specs for feature work** → `"warn"` keeps it visible; `false` requires explicit opt-in in config. Teams can use `--strict` in CI. +- **[Risk] Tri-state is the first `z.union` in config** → Well-established pattern (ESLint). Resilient parsing handles per-field failures. +- **[Risk] Hardcoding `'specs'` in detectCompleted** → Pragmatic for now. If more artifacts need optional behavior, generalize to an `optional` field in schema.yaml. +- **[Risk] `loadChangeContext` needs config access** → It already imports `readProjectConfig` (line 8 of instruction-loader.ts). Minimal wiring. + +## Open Questions + +_(none)_ diff --git a/openspec/changes/allow-specless-changes/proposal.md b/openspec/changes/allow-specless-changes/proposal.md new file mode 100644 index 000000000..cebbb9050 --- /dev/null +++ b/openspec/changes/allow-specless-changes/proposal.md @@ -0,0 +1,63 @@ +## Why + +Teams using OpenSpec for product specs and high-level technical standards often need to track changes that don't modify spec-level requirements — bug fixes, documentation refactoring, dependency upgrades, infrastructure work, or implementation-only refactors. The `openspec/changes/` directory is valuable as a repository for technical change descriptions and team review artifacts even when no spec requirements change. + +Today, `openspec validate` hard-fails with `CHANGE_NO_DELTAS` when a change has zero delta specs, and the artifact dependency graph blocks `tasks` on `specs` completion (determined by file existence of `specs/**/*.md`). This forces teams to either (a) skip the openspec workflow for non-spec changes, fragmenting their process, or (b) create artificial spec changes just to pass validation. Neither is acceptable. + +## What Changes + +Add `requireSpecDeltas` as a top-level tri-state setting in `openspec/config.yaml`: +- `"error"` (default when omitted) — current behavior, hard-fail on missing deltas +- `"warn"` — emit a WARNING but pass validation +- `false` — completely suppress the check, no output at all + +When set to `"warn"` or `false`, two things change: +1. **Validation**: the `CHANGE_NO_DELTAS` check emits the configured level (or nothing) instead of ERROR +2. **Artifact graph**: `detectCompleted()` synthetically marks `specs` as complete so that `tasks` is unblocked and the propose workflow can proceed without writing spec files + +The proposal instruction/template in the schema is static and doesn't adapt to this config. Projects using this feature should add a `rules.proposal` entry in config.yaml to tell the AI that Capabilities sections are optional. This uses the existing rules injection mechanism — no code changes needed. + +Example config for specless workflow: +```yaml +schema: spec-driven +requireSpecDeltas: false + +rules: + proposal: + - "The Capabilities section is optional. If the change has no spec-level requirement changes, leave New Capabilities and Modified Capabilities as 'None'." +``` + +### Considered and deferred: per-change metadata + +A `skipSpecDeltas: true` field in `.openspec.yaml` would allow per-change overrides of the project default. This was considered but deferred because: +- The artifact graph fix (synthetic completion in `detectCompleted()`) would need to read change metadata, adding I/O to a currently simple function +- The project-level config covers the primary use case (teams that routinely make non-spec changes) +- Can be added later without breaking changes + +## Capabilities + +### New Capabilities + +_(none — this extends existing capabilities)_ + +### Modified Capabilities + +- `config-loading`: Add top-level `requireSpecDeltas` (tri-state: `"error"` | `"warn"` | `false`) to the ProjectConfig schema and resilient parsing +- `cli-validate`: Respect `requireSpecDeltas` when evaluating the `CHANGE_NO_DELTAS` check +- `artifact-graph`: Synthetically mark `specs` as complete in `detectCompleted()` when `requireSpecDeltas` is not `"error"` +- `cli-artifact-workflow`: Add `"skipped"` status to status display for synthetically completed artifacts (indicator `[~]`, dim color, `"skipped"` in JSON) + +## Non-goals + +- Making the `specs` artifact disappear from `openspec status` — it remains visible as `[~] skipped` +- Changing the `ChangeSchema` Zod validation for `deltas.min(1)` — that validates the proposal's Capabilities section, not spec files +- Dynamic instruction adaptation — the schema's proposal instruction is static; projects use `rules` config to adjust AI guidance +- Per-change override via `.openspec.yaml` — deferred for a future change + +## Impact + +- `src/core/project-config.ts` — extend `ProjectConfigSchema` with optional `requireSpecDeltas` +- `src/core/validation/validator.ts` — `validateChangeDeltaSpecs()` respects the tri-state setting +- `src/commands/validate.ts` — read project config and pass setting to the validator +- `src/core/artifact-graph/state.ts` — `detectCompleted()` synthetically marks `specs` done when config allows +- `src/core/validation/constants.ts` — add message for the warning case diff --git a/openspec/changes/allow-specless-changes/specs/artifact-graph/spec.md b/openspec/changes/allow-specless-changes/specs/artifact-graph/spec.md new file mode 100644 index 000000000..0e07d94df --- /dev/null +++ b/openspec/changes/allow-specless-changes/specs/artifact-graph/spec.md @@ -0,0 +1,37 @@ +## MODIFIED Requirements + +### Requirement: State Detection +The system SHALL detect artifact completion state by scanning the filesystem, and SHALL synthetically mark artifacts as complete when project config indicates they are not required. + +#### Scenario: Simple file exists +- **WHEN** an artifact generates "proposal.md" and the file exists +- **THEN** the artifact is marked as completed + +#### Scenario: Glob pattern matches files +- **WHEN** an artifact generates "specs/**/*.md" and matching files exist +- **THEN** the artifact is marked as completed + +#### Scenario: No matching files +- **WHEN** an artifact generates "specs/**/*.md" and no matching files exist +- **AND** the project config does not set `requireSpecDeltas` (defaults to `"error"`) +- **THEN** the artifact is marked as not completed + +#### Scenario: Missing change directory +- **WHEN** the change directory does not exist +- **THEN** all artifacts are marked as not completed (empty state) + +#### Scenario: Specs artifact synthetically completed when requireSpecDeltas is not "error" +- **WHEN** the `specs` artifact generates "specs/**/*.md" and no matching files exist +- **AND** the project config has `requireSpecDeltas` set to `"warn"` or `false` +- **THEN** the `specs` artifact SHALL be synthetically marked as completed +- **AND** downstream artifacts that depend on `specs` (e.g. `tasks`) SHALL become ready + +#### Scenario: Specs artifact not synthetically completed when requireSpecDeltas is "error" +- **WHEN** the `specs` artifact generates "specs/**/*.md" and no matching files exist +- **AND** the project config has `requireSpecDeltas` set to `"error"` or is omitted +- **THEN** the `specs` artifact SHALL NOT be synthetically marked as completed +- **AND** downstream artifacts that depend on `specs` SHALL remain blocked + +#### Scenario: Specs artifact with files present ignores config +- **WHEN** the `specs` artifact generates "specs/**/*.md" and matching files exist +- **THEN** the artifact is marked as completed regardless of `requireSpecDeltas` setting diff --git a/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md b/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md new file mode 100644 index 000000000..a8d0dc65d --- /dev/null +++ b/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md @@ -0,0 +1,47 @@ +## MODIFIED Requirements + +### Requirement: Status Command + +The system SHALL display artifact completion status for a change, including scaffolded (empty) changes and skipped artifacts. + +#### Scenario: Show status with all states + +- **WHEN** user runs `openspec status --change ` +- **THEN** the system displays each artifact with status indicator: + - `[x]` for completed artifacts + - `[ ]` for ready artifacts + - `[-]` for blocked artifacts (with missing dependencies listed) + - `[~]` for skipped artifacts (synthetically completed due to config) + +#### Scenario: Status shows completion summary + +- **WHEN** user runs `openspec status --change ` +- **THEN** output includes completion percentage and count (e.g., "2/4 artifacts complete") +- **AND** skipped artifacts count toward the completed total + +#### Scenario: Status JSON output with skipped status + +- **WHEN** user runs `openspec status --change --json` +- **AND** an artifact is synthetically completed (e.g. `specs` when `requireSpecDeltas` is not `"error"` and no spec files exist) +- **THEN** the artifact's status SHALL be `"skipped"` (not `"done"`) + +#### Scenario: Status on scaffolded change + +- **WHEN** user runs `openspec status --change ` on a change with no artifacts +- **THEN** system displays all artifacts with their status +- **AND** root artifacts (no dependencies) show as ready `[ ]` +- **AND** dependent artifacts show as blocked `[-]` + +### Requirement: Output Formatting + +The system SHALL provide consistent output formatting. + +#### Scenario: Color output + +- **WHEN** terminal supports colors +- **THEN** status indicators use colors: green (done), yellow (ready), red (blocked), dim (skipped) + +#### Scenario: No color output + +- **WHEN** `--no-color` flag is used or NO_COLOR environment variable is set +- **THEN** output uses text-only indicators without ANSI colors diff --git a/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md b/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md new file mode 100644 index 000000000..b38e43153 --- /dev/null +++ b/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md @@ -0,0 +1,48 @@ +## MODIFIED Requirements + +### Requirement: Validation SHALL provide actionable remediation steps +Validation output SHALL include specific guidance to fix each error, including expected structure, example headers, and suggested commands to verify fixes. + +#### Scenario: No deltas found in change (default behavior) +- **WHEN** validating a change with zero parsed deltas +- **AND** the project config does not set `requireSpecDeltas` (defaults to `"error"`) +- **THEN** show error "No deltas found" with guidance: + - Explain that change specs must include `## ADDED Requirements`, `## MODIFIED Requirements`, `## REMOVED Requirements`, or `## RENAMED Requirements` + - Remind authors that files must live under `openspec/changes/{id}/specs//spec.md` + - Include an explicit note: "Spec delta files cannot start with titles before the operation headers" + - Suggest running `openspec show {id} --json --deltas-only` for debugging + - Note: "If this change intentionally has no spec deltas, set `requireSpecDeltas: false` in openspec/config.yaml" + +#### Scenario: No deltas found, requireSpecDeltas is "warn" +- **WHEN** validating a change with zero parsed deltas +- **AND** the project config has `requireSpecDeltas` set to `"warn"` +- **THEN** emit a WARNING stating "Change has no spec deltas (allowed by config)" +- **AND** validation SHALL pass (report.valid is `true` in non-strict mode) + +#### Scenario: No deltas found, requireSpecDeltas is "warn", strict mode +- **WHEN** validating a change with zero parsed deltas +- **AND** the project config has `requireSpecDeltas` set to `"warn"` +- **AND** `--strict` mode is enabled +- **THEN** emit a WARNING stating "Change has no spec deltas (allowed by config)" +- **AND** validation SHALL fail (report.valid is `false` because strict mode treats warnings as errors) + +#### Scenario: No deltas found, requireSpecDeltas is false +- **WHEN** validating a change with zero parsed deltas +- **AND** the project config has `requireSpecDeltas` set to `false` +- **THEN** emit no issue (no error, no warning, no info) for the missing deltas +- **AND** validation SHALL pass + +#### Scenario: Missing required sections +- **WHEN** a required section is missing +- **THEN** include expected header names and a minimal skeleton: + - For Spec: `## Purpose`, `## Requirements` + - For Change: `## Why`, `## What Changes` + - Provide an example snippet of the missing section with placeholder prose ready to copy + - Mention the quick-reference section in `openspec/AGENTS.md` as the authoritative template + +#### Scenario: Missing requirement descriptive text +- **WHEN** a requirement header lacks descriptive text before scenarios +- **THEN** emit an error explaining that `### Requirement:` lines must be followed by narrative text before any `#### Scenario:` headers + - Show compliant example: "### Requirement: Foo" followed by "The system SHALL ..." + - Suggest adding 1-2 sentences describing the normative behavior prior to listing scenarios + - Reference the pre-validation checklist in `openspec/AGENTS.md` diff --git a/openspec/changes/allow-specless-changes/specs/config-loading/spec.md b/openspec/changes/allow-specless-changes/specs/config-loading/spec.md new file mode 100644 index 000000000..8e3568967 --- /dev/null +++ b/openspec/changes/allow-specless-changes/specs/config-loading/spec.md @@ -0,0 +1,69 @@ +## MODIFIED Requirements + +### Requirement: Use resilient field-by-field parsing + +The system SHALL parse each config field independently, collecting valid fields and warning about invalid ones without rejecting the entire config. + +#### Scenario: Schema field is valid +- **WHEN** config contains `schema: "spec-driven"` +- **THEN** schema field is included in returned config + +#### Scenario: Schema field is missing +- **WHEN** config lacks the `schema` field +- **THEN** no warning is logged (field is optional at parse level) + +#### Scenario: Schema field is empty string +- **WHEN** config contains `schema: ""` +- **THEN** warning is logged and schema field is not included in returned config + +#### Scenario: Schema field is invalid type +- **WHEN** config contains `schema: 123` (number instead of string) +- **THEN** warning is logged and schema field is not included in returned config + +#### Scenario: Context field is valid +- **WHEN** config contains `context: "Tech stack: TypeScript"` +- **THEN** context field is included in returned config + +#### Scenario: Context field is invalid type +- **WHEN** config contains `context: 123` (number instead of string) +- **THEN** warning is logged and context field is not included in returned config + +#### Scenario: Rules field has valid structure +- **WHEN** config contains `rules: { proposal: ["Rule 1"], specs: ["Rule 2"] }` +- **THEN** rules field is included in returned config with valid rules + +#### Scenario: Rules field has non-array value for artifact +- **WHEN** config contains `rules: { proposal: "not an array", specs: ["Valid"] }` +- **THEN** warning is logged for proposal, but specs rules are still included in returned config + +#### Scenario: Rules array contains non-string elements +- **WHEN** config contains `rules: { proposal: ["Valid rule", 123, ""] }` +- **THEN** only "Valid rule" is included, warning logged about invalid elements + +#### Scenario: Mix of valid and invalid fields +- **WHEN** config contains valid schema, invalid context type, valid rules +- **THEN** config is returned with schema and rules fields, warning logged about context + +#### Scenario: requireSpecDeltas set to "error" +- **WHEN** config contains `requireSpecDeltas: "error"` +- **THEN** requireSpecDeltas SHALL be `"error"` in returned config + +#### Scenario: requireSpecDeltas set to "warn" +- **WHEN** config contains `requireSpecDeltas: "warn"` +- **THEN** requireSpecDeltas SHALL be `"warn"` in returned config + +#### Scenario: requireSpecDeltas set to false +- **WHEN** config contains `requireSpecDeltas: false` +- **THEN** requireSpecDeltas SHALL be `false` in returned config + +#### Scenario: requireSpecDeltas omitted +- **WHEN** config does not contain a `requireSpecDeltas` key +- **THEN** requireSpecDeltas SHALL default to `undefined` in returned config (callers treat missing as `"error"`) + +#### Scenario: requireSpecDeltas with invalid value +- **WHEN** config contains `requireSpecDeltas: "invalid"` +- **THEN** warning SHALL be logged and requireSpecDeltas SHALL not be included in returned config + +#### Scenario: requireSpecDeltas with invalid type +- **WHEN** config contains `requireSpecDeltas: 123` +- **THEN** warning SHALL be logged and requireSpecDeltas SHALL not be included in returned config diff --git a/openspec/changes/allow-specless-changes/tasks.md b/openspec/changes/allow-specless-changes/tasks.md new file mode 100644 index 000000000..4cefbcf18 --- /dev/null +++ b/openspec/changes/allow-specless-changes/tasks.md @@ -0,0 +1,43 @@ +## 1. Extend ProjectConfig schema + +- [ ] 1.1 Add `RequireSpecDeltasSchema` as `z.union([z.enum(["error", "warn"]), z.literal(false)])` and add optional top-level `requireSpecDeltas` to `ProjectConfigSchema` in `src/core/project-config.ts` +- [ ] 1.2 Add resilient field-by-field parsing for `requireSpecDeltas` in `readProjectConfig()`, following the same safeParse pattern as other fields (warn on invalid, omit from result) +- [ ] 1.3 Add unit tests for config parsing: `"error"`, `"warn"`, `false`, missing, invalid string, invalid type + +## 2. Thread config into Validator + +- [ ] 2.1 Change `Validator` constructor to accept a config object `{ strictMode?: boolean; requireSpecDeltas?: 'error' | 'warn' | false }` instead of bare `strictMode`, defaulting `requireSpecDeltas` to `'error'` +- [ ] 2.2 Update `ValidateCommand.validateByType()` and `runBulkValidation()` to read project config via `readProjectConfig(process.cwd())` and pass `requireSpecDeltas` to the Validator +- [ ] 2.3 Update all existing `new Validator(...)` call sites to use the new config object shape + +## 3. Implement tri-state CHANGE_NO_DELTAS behavior + +- [ ] 3.1 Add `CHANGE_NO_DELTAS_ALLOWED` message constant to `src/core/validation/constants.ts` +- [ ] 3.2 In `validateChangeDeltaSpecs()`, change the `totalDeltas === 0` block: emit ERROR when `'error'`, emit WARNING when `'warn'`, emit nothing when `false` +- [ ] 3.3 Add unit test: `requireSpecDeltas: 'error'` (default) with zero deltas → existing ERROR behavior unchanged +- [ ] 3.4 Add unit test: `requireSpecDeltas: 'warn'` with zero deltas → report.valid `true`, one WARNING +- [ ] 3.5 Add unit test: `requireSpecDeltas: 'warn'` + `strictMode: true` → report.valid `false` +- [ ] 3.6 Add unit test: `requireSpecDeltas: false` with zero deltas → report.valid `true`, zero issues from this check + +## 4. Synthetic completion in artifact graph + +- [ ] 4.1 Add optional `options` parameter to `detectCompleted()` in `src/core/artifact-graph/state.ts` with `requireSpecDeltas` field +- [ ] 4.2 After the file-existence loop, synthetically add `'specs'` to the completed set when `requireSpecDeltas` is `'warn'` or `false` and no spec files were found +- [ ] 4.3 Update `loadChangeContext()` in `src/core/artifact-graph/instruction-loader.ts` to read project config and pass `requireSpecDeltas` to `detectCompleted()` +- [ ] 4.4 Add unit test: `requireSpecDeltas: false` → `specs` in completed set even with no files +- [ ] 4.5 Add unit test: `requireSpecDeltas: 'error'` → `specs` not in completed set without files (existing behavior) +- [ ] 4.6 Add unit test: spec files exist → `specs` in completed set regardless of config + +## 5. "Skipped" status display + +- [ ] 5.1 Add `'skipped'` to the `ArtifactStatus.status` type in `src/core/artifact-graph/instruction-loader.ts` +- [ ] 5.2 In `formatChangeStatus()`, check whether a completed artifact actually has files via `artifactOutputExists()` — if not, set status to `'skipped'` instead of `'done'` +- [ ] 5.3 Add `'skipped'` case to `getStatusIndicator()` (`[~]`) and `getStatusColor()` (`chalk.dim`) in `src/commands/workflow/shared.ts` +- [ ] 5.4 Add unit test: synthetically completed artifact shows `"skipped"` in JSON status output +- [ ] 5.5 Add unit test: skipped artifacts count toward completed total in progress display + +## 6. Integration and verification + +- [ ] 6.1 Add integration test: temp project with `requireSpecDeltas: false`, change with no specs → `openspec validate ` passes, `openspec status --change --json` shows `specs: "skipped"` and `tasks: "ready"` +- [ ] 6.2 Verify all existing tests pass (`pnpm test`) +- [ ] 6.3 Verify Windows CI passes (no path-separator issues in config loading or state detection) From 2b838c7e4195358a14d10ea41fdd47fbbd7e66bd Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Wed, 15 Apr 2026 17:07:56 -0400 Subject: [PATCH 2/8] feat: allow changes without spec deltas via requireSpecDeltas config Add top-level `requireSpecDeltas` tri-state setting to config.yaml ("error" | "warn" | false) that controls whether changes must include spec deltas. When set to "warn" or false, validation passes without spec deltas and the artifact graph synthetically completes the specs artifact so downstream artifacts (tasks) are unblocked. Status display shows synthetically completed artifacts as [~] skipped. --- .../changes/allow-specless-changes/tasks.md | 50 +++++----- src/commands/change.ts | 2 +- src/commands/spec.ts | 2 +- src/commands/validate.ts | 9 +- src/commands/workflow/shared.ts | 8 +- src/commands/workflow/status.ts | 2 +- src/core/artifact-graph/instruction-loader.ts | 15 ++- src/core/artifact-graph/state.ts | 22 ++++- src/core/project-config.ts | 24 +++++ src/core/validation/constants.ts | 1 + src/core/validation/validator.ts | 19 +++- test/commands/validate.test.ts | 27 ++++++ .../artifact-graph/instruction-loader.test.ts | 38 ++++++++ test/core/artifact-graph/state.test.ts | 63 +++++++++++++ test/core/project-config.test.ts | 92 +++++++++++++++++++ test/core/validation.test.ts | 78 ++++++++++++++-- 16 files changed, 404 insertions(+), 48 deletions(-) diff --git a/openspec/changes/allow-specless-changes/tasks.md b/openspec/changes/allow-specless-changes/tasks.md index 4cefbcf18..5d58d4b92 100644 --- a/openspec/changes/allow-specless-changes/tasks.md +++ b/openspec/changes/allow-specless-changes/tasks.md @@ -1,43 +1,43 @@ ## 1. Extend ProjectConfig schema -- [ ] 1.1 Add `RequireSpecDeltasSchema` as `z.union([z.enum(["error", "warn"]), z.literal(false)])` and add optional top-level `requireSpecDeltas` to `ProjectConfigSchema` in `src/core/project-config.ts` -- [ ] 1.2 Add resilient field-by-field parsing for `requireSpecDeltas` in `readProjectConfig()`, following the same safeParse pattern as other fields (warn on invalid, omit from result) -- [ ] 1.3 Add unit tests for config parsing: `"error"`, `"warn"`, `false`, missing, invalid string, invalid type +- [x] 1.1 Add `RequireSpecDeltasSchema` as `z.union([z.enum(["error", "warn"]), z.literal(false)])` and add optional top-level `requireSpecDeltas` to `ProjectConfigSchema` in `src/core/project-config.ts` +- [x] 1.2 Add resilient field-by-field parsing for `requireSpecDeltas` in `readProjectConfig()`, following the same safeParse pattern as other fields (warn on invalid, omit from result) +- [x] 1.3 Add unit tests for config parsing: `"error"`, `"warn"`, `false`, missing, invalid string, invalid type ## 2. Thread config into Validator -- [ ] 2.1 Change `Validator` constructor to accept a config object `{ strictMode?: boolean; requireSpecDeltas?: 'error' | 'warn' | false }` instead of bare `strictMode`, defaulting `requireSpecDeltas` to `'error'` -- [ ] 2.2 Update `ValidateCommand.validateByType()` and `runBulkValidation()` to read project config via `readProjectConfig(process.cwd())` and pass `requireSpecDeltas` to the Validator -- [ ] 2.3 Update all existing `new Validator(...)` call sites to use the new config object shape +- [x] 2.1 Change `Validator` constructor to accept a config object `{ strictMode?: boolean; requireSpecDeltas?: 'error' | 'warn' | false }` instead of bare `strictMode`, defaulting `requireSpecDeltas` to `'error'` +- [x] 2.2 Update `ValidateCommand.validateByType()` and `runBulkValidation()` to read project config via `readProjectConfig(process.cwd())` and pass `requireSpecDeltas` to the Validator +- [x] 2.3 Update all existing `new Validator(...)` call sites to use the new config object shape ## 3. Implement tri-state CHANGE_NO_DELTAS behavior -- [ ] 3.1 Add `CHANGE_NO_DELTAS_ALLOWED` message constant to `src/core/validation/constants.ts` -- [ ] 3.2 In `validateChangeDeltaSpecs()`, change the `totalDeltas === 0` block: emit ERROR when `'error'`, emit WARNING when `'warn'`, emit nothing when `false` -- [ ] 3.3 Add unit test: `requireSpecDeltas: 'error'` (default) with zero deltas → existing ERROR behavior unchanged -- [ ] 3.4 Add unit test: `requireSpecDeltas: 'warn'` with zero deltas → report.valid `true`, one WARNING -- [ ] 3.5 Add unit test: `requireSpecDeltas: 'warn'` + `strictMode: true` → report.valid `false` -- [ ] 3.6 Add unit test: `requireSpecDeltas: false` with zero deltas → report.valid `true`, zero issues from this check +- [x] 3.1 Add `CHANGE_NO_DELTAS_ALLOWED` message constant to `src/core/validation/constants.ts` +- [x] 3.2 In `validateChangeDeltaSpecs()`, change the `totalDeltas === 0` block: emit ERROR when `'error'`, emit WARNING when `'warn'`, emit nothing when `false` +- [x] 3.3 Add unit test: `requireSpecDeltas: 'error'` (default) with zero deltas → existing ERROR behavior unchanged +- [x] 3.4 Add unit test: `requireSpecDeltas: 'warn'` with zero deltas → report.valid `true`, one WARNING +- [x] 3.5 Add unit test: `requireSpecDeltas: 'warn'` + `strictMode: true` → report.valid `false` +- [x] 3.6 Add unit test: `requireSpecDeltas: false` with zero deltas → report.valid `true`, zero issues from this check ## 4. Synthetic completion in artifact graph -- [ ] 4.1 Add optional `options` parameter to `detectCompleted()` in `src/core/artifact-graph/state.ts` with `requireSpecDeltas` field -- [ ] 4.2 After the file-existence loop, synthetically add `'specs'` to the completed set when `requireSpecDeltas` is `'warn'` or `false` and no spec files were found -- [ ] 4.3 Update `loadChangeContext()` in `src/core/artifact-graph/instruction-loader.ts` to read project config and pass `requireSpecDeltas` to `detectCompleted()` -- [ ] 4.4 Add unit test: `requireSpecDeltas: false` → `specs` in completed set even with no files -- [ ] 4.5 Add unit test: `requireSpecDeltas: 'error'` → `specs` not in completed set without files (existing behavior) -- [ ] 4.6 Add unit test: spec files exist → `specs` in completed set regardless of config +- [x] 4.1 Add optional `options` parameter to `detectCompleted()` in `src/core/artifact-graph/state.ts` with `requireSpecDeltas` field +- [x] 4.2 After the file-existence loop, synthetically add `'specs'` to the completed set when `requireSpecDeltas` is `'warn'` or `false` and no spec files were found +- [x] 4.3 Update `loadChangeContext()` in `src/core/artifact-graph/instruction-loader.ts` to read project config and pass `requireSpecDeltas` to `detectCompleted()` +- [x] 4.4 Add unit test: `requireSpecDeltas: false` → `specs` in completed set even with no files +- [x] 4.5 Add unit test: `requireSpecDeltas: 'error'` → `specs` not in completed set without files (existing behavior) +- [x] 4.6 Add unit test: spec files exist → `specs` in completed set regardless of config ## 5. "Skipped" status display -- [ ] 5.1 Add `'skipped'` to the `ArtifactStatus.status` type in `src/core/artifact-graph/instruction-loader.ts` -- [ ] 5.2 In `formatChangeStatus()`, check whether a completed artifact actually has files via `artifactOutputExists()` — if not, set status to `'skipped'` instead of `'done'` -- [ ] 5.3 Add `'skipped'` case to `getStatusIndicator()` (`[~]`) and `getStatusColor()` (`chalk.dim`) in `src/commands/workflow/shared.ts` -- [ ] 5.4 Add unit test: synthetically completed artifact shows `"skipped"` in JSON status output -- [ ] 5.5 Add unit test: skipped artifacts count toward completed total in progress display +- [x] 5.1 Add `'skipped'` to the `ArtifactStatus.status` type in `src/core/artifact-graph/instruction-loader.ts` +- [x] 5.2 In `formatChangeStatus()`, check whether a completed artifact actually has files via `artifactOutputExists()` — if not, set status to `'skipped'` instead of `'done'` +- [x] 5.3 Add `'skipped'` case to `getStatusIndicator()` (`[~]`) and `getStatusColor()` (`chalk.dim`) in `src/commands/workflow/shared.ts` +- [x] 5.4 Add unit test: synthetically completed artifact shows `"skipped"` in JSON status output +- [x] 5.5 Add unit test: skipped artifacts count toward completed total in progress display ## 6. Integration and verification -- [ ] 6.1 Add integration test: temp project with `requireSpecDeltas: false`, change with no specs → `openspec validate ` passes, `openspec status --change --json` shows `specs: "skipped"` and `tasks: "ready"` -- [ ] 6.2 Verify all existing tests pass (`pnpm test`) +- [x] 6.1 Add integration test: temp project with `requireSpecDeltas: false`, change with no specs → `openspec validate ` passes, `openspec status --change --json` shows `specs: "skipped"` and `tasks: "ready"` +- [x] 6.2 Verify all existing tests pass (`pnpm test`) - [ ] 6.3 Verify Windows CI passes (no path-separator issues in config loading or state detection) diff --git a/src/commands/change.ts b/src/commands/change.ts index 051b4697c..936f52695 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -215,7 +215,7 @@ export class ChangeCommand { throw new Error(`Change "${changeName}" not found at ${changeDir}`); } - const validator = new Validator(options?.strict || false); + const validator = new Validator({ strictMode: options?.strict || false }); const report = await validator.validateChangeDeltaSpecs(changeDir); if (options?.json) { diff --git a/src/commands/spec.ts b/src/commands/spec.ts index d28052f14..0abac1093 100644 --- a/src/commands/spec.ts +++ b/src/commands/spec.ts @@ -223,7 +223,7 @@ export function registerSpecCommand(rootProgram: typeof program) { throw new Error(`Spec '${specId}' not found at openspec/specs/${specId}/spec.md`); } - const validator = new Validator(options.strict); + const validator = new Validator({ strictMode: options.strict }); const report = await validator.validateSpec(specPath); if (options.json) { diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 9e59a4d48..c7b48920a 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -1,6 +1,7 @@ import ora from 'ora'; import path from 'path'; import { Validator } from '../core/validation/validator.js'; +import { readProjectConfig } from '../core/project-config.js'; import { isInteractive, resolveNoInteractive } from '../utils/interactive.js'; import { getActiveChangeIds, getSpecIds } from '../utils/item-discovery.js'; import { nearestMatches } from '../utils/match.js'; @@ -128,7 +129,9 @@ export class ValidateCommand { } private async validateByType(type: ItemType, id: string, opts: { strict: boolean; json: boolean }): Promise { - const validator = new Validator(opts.strict); + const projectConfig = readProjectConfig(process.cwd()); + const requireSpecDeltas = projectConfig?.requireSpecDeltas ?? 'error'; + const validator = new Validator({ strictMode: opts.strict, requireSpecDeltas }); if (type === 'change') { const changeDir = path.join(process.cwd(), 'openspec', 'changes', id); const start = Date.now(); @@ -191,7 +194,9 @@ export class ValidateCommand { const DEFAULT_CONCURRENCY = 6; const maxSuggestions = 5; // used by nearestMatches const concurrency = normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY; - const validator = new Validator(opts.strict); + const projectConfig = readProjectConfig(process.cwd()); + const requireSpecDeltas = projectConfig?.requireSpecDeltas ?? 'error'; + const validator = new Validator({ strictMode: opts.strict, requireSpecDeltas }); const queue: Array<() => Promise> = []; for (const id of changeIds) { diff --git a/src/commands/workflow/shared.ts b/src/commands/workflow/shared.ts index 43c9aa46c..6d8e6b42f 100644 --- a/src/commands/workflow/shared.ts +++ b/src/commands/workflow/shared.ts @@ -57,7 +57,7 @@ export function isColorDisabled(): boolean { /** * Gets the color function based on status. */ -export function getStatusColor(status: 'done' | 'ready' | 'blocked'): (text: string) => string { +export function getStatusColor(status: 'done' | 'ready' | 'blocked' | 'skipped'): (text: string) => string { if (isColorDisabled()) { return (text: string) => text; } @@ -68,13 +68,15 @@ export function getStatusColor(status: 'done' | 'ready' | 'blocked'): (text: str return chalk.yellow; case 'blocked': return chalk.red; + case 'skipped': + return chalk.dim; } } /** * Gets the status indicator for an artifact. */ -export function getStatusIndicator(status: 'done' | 'ready' | 'blocked'): string { +export function getStatusIndicator(status: 'done' | 'ready' | 'blocked' | 'skipped'): string { const color = getStatusColor(status); switch (status) { case 'done': @@ -83,6 +85,8 @@ export function getStatusIndicator(status: 'done' | 'ready' | 'blocked'): string return color('[ ]'); case 'blocked': return color('[-]'); + case 'skipped': + return color('[~]'); } } diff --git a/src/commands/workflow/status.ts b/src/commands/workflow/status.ts index 1109ab188..1bdbf46fc 100644 --- a/src/commands/workflow/status.ts +++ b/src/commands/workflow/status.ts @@ -85,7 +85,7 @@ export async function statusCommand(options: StatusOptions): Promise { } export function printStatusText(status: ChangeStatus): void { - const doneCount = status.artifacts.filter((a) => a.status === 'done').length; + const doneCount = status.artifacts.filter((a) => a.status === 'done' || a.status === 'skipped').length; const total = status.artifacts.length; console.log(`Change: ${status.changeName}`); diff --git a/src/core/artifact-graph/instruction-loader.ts b/src/core/artifact-graph/instruction-loader.ts index b8b2675bb..e5faaa312 100644 --- a/src/core/artifact-graph/instruction-loader.ts +++ b/src/core/artifact-graph/instruction-loader.ts @@ -3,6 +3,7 @@ import * as path from 'node:path'; import { getSchemaDir, resolveSchema } from './resolver.js'; import { ArtifactGraph } from './graph.js'; import { detectCompleted } from './state.js'; +import { artifactOutputExists } from './outputs.js'; import { resolveSchemaForChange } from '../../utils/change-metadata.js'; import { FileSystemUtils } from '../../utils/file-system.js'; import { readProjectConfig, validateConfigRules } from '../project-config.js'; @@ -94,8 +95,8 @@ export interface ArtifactStatus { id: string; /** Output path pattern */ outputPath: string; - /** Status: done, ready, or blocked */ - status: 'done' | 'ready' | 'blocked'; + /** Status: done, ready, blocked, or skipped (synthetically completed via config) */ + status: 'done' | 'ready' | 'blocked' | 'skipped'; /** Missing dependencies (only for blocked) */ missingDeps?: string[]; } @@ -187,7 +188,12 @@ export function loadChangeContext( const schema = resolveSchema(resolvedSchemaName, projectRoot); const graph = ArtifactGraph.fromSchema(schema); - const completed = detectCompleted(graph, changeDir); + + // Read project config for requireSpecDeltas setting + const projectConfig = readProjectConfig(projectRoot); + const requireSpecDeltas = projectConfig?.requireSpecDeltas; + + const completed = detectCompleted(graph, changeDir, { requireSpecDeltas }); return { graph, @@ -330,10 +336,11 @@ export function formatChangeStatus(context: ChangeContext): ChangeStatus { const artifactStatuses: ArtifactStatus[] = artifacts.map(artifact => { if (context.completed.has(artifact.id)) { + const hasFiles = artifactOutputExists(context.changeDir, artifact.generates); return { id: artifact.id, outputPath: artifact.generates, - status: 'done' as const, + status: hasFiles ? 'done' as const : 'skipped' as const, }; } diff --git a/src/core/artifact-graph/state.ts b/src/core/artifact-graph/state.ts index 47c256ac9..ebc662d37 100644 --- a/src/core/artifact-graph/state.ts +++ b/src/core/artifact-graph/state.ts @@ -2,16 +2,23 @@ import * as fs from 'node:fs'; import type { CompletedSet } from './types.js'; import type { ArtifactGraph } from './graph.js'; import { artifactOutputExists } from './outputs.js'; +import type { RequireSpecDeltas } from '../project-config.js'; + +export interface DetectCompletedOptions { + requireSpecDeltas?: RequireSpecDeltas; +} /** * Detects which artifacts are completed by checking file existence in the change directory. - * Returns a Set of completed artifact IDs. + * When requireSpecDeltas is 'warn' or false, the 'specs' artifact is synthetically + * marked complete even if no spec files exist, unblocking downstream artifacts. * * @param graph - The artifact graph to check * @param changeDir - The change directory to scan for files + * @param options - Optional config for synthetic completion * @returns Set of artifact IDs whose generated files exist */ -export function detectCompleted(graph: ArtifactGraph, changeDir: string): CompletedSet { +export function detectCompleted(graph: ArtifactGraph, changeDir: string, options?: DetectCompletedOptions): CompletedSet { const completed = new Set(); // Handle missing change directory gracefully @@ -25,6 +32,17 @@ export function detectCompleted(graph: ArtifactGraph, changeDir: string): Comple } } + // Synthetically mark 'specs' as complete when spec deltas are not required + const requireSpecDeltas = options?.requireSpecDeltas; + if (!completed.has('specs') && + requireSpecDeltas !== undefined && + requireSpecDeltas !== 'error') { + const specsArtifact = graph.getAllArtifacts().find(a => a.id === 'specs'); + if (specsArtifact) { + completed.add('specs'); + } + } + return completed; } diff --git a/src/core/project-config.ts b/src/core/project-config.ts index 6c1ea04a5..38f1399fe 100644 --- a/src/core/project-config.ts +++ b/src/core/project-config.ts @@ -16,6 +16,14 @@ import { z } from 'zod'; * - Single source of truth for type and validation * - Consistent with other OpenSpec schemas */ + +export const RequireSpecDeltasSchema = z.union([ + z.enum(['error', 'warn']), + z.literal(false), +]); + +export type RequireSpecDeltas = z.infer; + export const ProjectConfigSchema = z.object({ // Required: which schema to use (e.g., "spec-driven", or project-local schema name) schema: z @@ -38,6 +46,12 @@ export const ProjectConfigSchema = z.object({ ) .optional() .describe('Per-artifact rules, keyed by artifact ID'), + + // Optional: whether spec deltas are required for changes + // "error" (default) = hard-fail, "warn" = warning only, false = silent + requireSpecDeltas: RequireSpecDeltasSchema + .optional() + .describe('Whether changes must include spec deltas: "error" | "warn" | false'), }); export type ProjectConfig = z.infer; @@ -152,6 +166,16 @@ export function readProjectConfig(projectRoot: string): ProjectConfig | null { } } + // Parse requireSpecDeltas field + if (raw.requireSpecDeltas !== undefined) { + const result = RequireSpecDeltasSchema.safeParse(raw.requireSpecDeltas); + if (result.success) { + config.requireSpecDeltas = result.data; + } else { + console.warn(`Invalid 'requireSpecDeltas' field in config (must be "error", "warn", or false)`); + } + } + // Return partial config even if some fields failed return Object.keys(config).length > 0 ? (config as ProjectConfig) : null; } catch (error) { diff --git a/src/core/validation/constants.ts b/src/core/validation/constants.ts index a6cf0de60..a12168702 100644 --- a/src/core/validation/constants.ts +++ b/src/core/validation/constants.ts @@ -35,6 +35,7 @@ export const VALIDATION_MESSAGES = { REQUIREMENT_TOO_LONG: `Requirement text is very long (>${MAX_REQUIREMENT_TEXT_LENGTH} characters). Consider breaking it down.`, DELTA_DESCRIPTION_TOO_BRIEF: 'Delta description is too brief', DELTA_MISSING_REQUIREMENTS: 'Delta should include requirements', + CHANGE_NO_DELTAS_ALLOWED: 'Change has no spec deltas (allowed by config)', // Guidance snippets (appended to primary messages for remediation) GUIDE_NO_DELTAS: diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 37b43a37d..7c9b8d9f8 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -14,11 +14,20 @@ import { parseDeltaSpec, normalizeRequirementName } from '../parsers/requirement import { findMainSpecStructureIssues } from '../parsers/spec-structure.js'; import { FileSystemUtils } from '../../utils/file-system.js'; +import type { RequireSpecDeltas } from '../project-config.js'; + +export interface ValidatorConfig { + strictMode?: boolean; + requireSpecDeltas?: RequireSpecDeltas; +} + export class Validator { private strictMode: boolean; + private requireSpecDeltas: RequireSpecDeltas; - constructor(strictMode: boolean = false) { - this.strictMode = strictMode; + constructor(config: ValidatorConfig = {}) { + this.strictMode = config.strictMode ?? false; + this.requireSpecDeltas = config.requireSpecDeltas ?? 'error'; } async validateSpec(filePath: string): Promise { @@ -267,7 +276,11 @@ export class Validator { } if (totalDeltas === 0) { - issues.push({ level: 'ERROR', path: 'file', message: this.enrichTopLevelError('change', VALIDATION_MESSAGES.CHANGE_NO_DELTAS) }); + if (this.requireSpecDeltas === 'error') { + issues.push({ level: 'ERROR', path: 'file', message: this.enrichTopLevelError('change', VALIDATION_MESSAGES.CHANGE_NO_DELTAS) }); + } else if (this.requireSpecDeltas === 'warn') { + issues.push({ level: 'WARNING', path: 'file', message: VALIDATION_MESSAGES.CHANGE_NO_DELTAS_ALLOWED }); + } } return this.createReport(issues); diff --git a/test/commands/validate.test.ts b/test/commands/validate.test.ts index b94f72d35..c9218c9fd 100644 --- a/test/commands/validate.test.ts +++ b/test/commands/validate.test.ts @@ -144,4 +144,31 @@ describe('top-level validate command', () => { // Should complete without hanging and without prompts expect(result.stderr).not.toContain('What would you like to validate?'); }); + + it('passes validation for specless change when requireSpecDeltas is false', async () => { + // Create config with requireSpecDeltas: false + await fs.writeFile( + path.join(testDir, 'openspec', 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: false\n' + ); + + // Create a change with no specs directory at all + const noSpecsDir = path.join(changesDir, 'no-specs-change'); + await fs.mkdir(noSpecsDir, { recursive: true }); + await fs.writeFile( + path.join(noSpecsDir, 'proposal.md'), + '## Why\nBug fix that needs no spec changes and is long enough for validation.\n\n## What Changes\n- Fix a bug' + ); + + const validateResult = await runCLI(['validate', 'no-specs-change', '--json'], { cwd: testDir }); + expect(validateResult.exitCode).toBe(0); + const json = JSON.parse(validateResult.stdout.trim()); + expect(json.items[0].valid).toBe(true); + + const statusResult = await runCLI(['status', '--change', 'no-specs-change', '--json'], { cwd: testDir }); + expect(statusResult.exitCode).toBe(0); + const statusJson = JSON.parse(statusResult.stdout.trim()); + const specs = statusJson.artifacts.find((a: any) => a.id === 'specs'); + expect(specs?.status).toBe('skipped'); + }); }); diff --git a/test/core/artifact-graph/instruction-loader.test.ts b/test/core/artifact-graph/instruction-loader.test.ts index 9d8f612cd..fcc52aefe 100644 --- a/test/core/artifact-graph/instruction-loader.test.ts +++ b/test/core/artifact-graph/instruction-loader.test.ts @@ -605,5 +605,43 @@ rules: expect(proposalIdx).toBeLessThan(specsIdx); expect(specsIdx).toBeLessThan(tasksIdx); }); + + it('should show specs as "skipped" when synthetically completed via config', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, 'config.yaml'), 'schema: spec-driven\nrequireSpecDeltas: false\n'); + + const changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change'); + fs.mkdirSync(changeDir, { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'proposal.md'), '# Proposal'); + + const context = loadChangeContext(tempDir, 'my-change'); + const status = formatChangeStatus(context); + + const specs = status.artifacts.find(a => a.id === 'specs'); + expect(specs?.status).toBe('skipped'); + + const proposal = status.artifacts.find(a => a.id === 'proposal'); + expect(proposal?.status).toBe('done'); + }); + + it('should count skipped artifacts toward completed total', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, 'config.yaml'), 'schema: spec-driven\nrequireSpecDeltas: false\n'); + + const changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change'); + fs.mkdirSync(changeDir, { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'proposal.md'), '# Proposal'); + fs.writeFileSync(path.join(changeDir, 'design.md'), '# Design'); + fs.writeFileSync(path.join(changeDir, 'tasks.md'), '# Tasks'); + + const context = loadChangeContext(tempDir, 'my-change'); + const status = formatChangeStatus(context); + + const doneOrSkipped = status.artifacts.filter(a => a.status === 'done' || a.status === 'skipped'); + expect(doneOrSkipped.length).toBe(4); + expect(status.isComplete).toBe(true); + }); }); }); diff --git a/test/core/artifact-graph/state.test.ts b/test/core/artifact-graph/state.test.ts index 758a7675b..88e2461eb 100644 --- a/test/core/artifact-graph/state.test.ts +++ b/test/core/artifact-graph/state.test.ts @@ -170,5 +170,68 @@ describe('artifact-graph/state', () => { expect(completed.has('design')).toBe(false); expect(completed.has('tasks')).toBe(false); }); + + it('should synthetically complete specs when requireSpecDeltas is false', () => { + const schema = createSchema([ + { id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] }, + { id: 'specs', generates: 'specs/**/*.md', description: 'Specs', template: 't.md', requires: ['proposal'] }, + { id: 'tasks', generates: 'tasks.md', description: 'Tasks', template: 't.md', requires: ['specs'] }, + ]); + const graph = ArtifactGraph.fromSchema(schema); + + fs.writeFileSync(path.join(tempDir, 'proposal.md'), 'content'); + + const completed = detectCompleted(graph, tempDir, { requireSpecDeltas: false }); + + expect(completed.has('proposal')).toBe(true); + expect(completed.has('specs')).toBe(true); + }); + + it('should synthetically complete specs when requireSpecDeltas is "warn"', () => { + const schema = createSchema([ + { id: 'specs', generates: 'specs/**/*.md', description: 'Specs', template: 't.md', requires: [] }, + ]); + const graph = ArtifactGraph.fromSchema(schema); + + const completed = detectCompleted(graph, tempDir, { requireSpecDeltas: 'warn' }); + + expect(completed.has('specs')).toBe(true); + }); + + it('should not synthetically complete specs when requireSpecDeltas is "error"', () => { + const schema = createSchema([ + { id: 'specs', generates: 'specs/**/*.md', description: 'Specs', template: 't.md', requires: [] }, + ]); + const graph = ArtifactGraph.fromSchema(schema); + + const completed = detectCompleted(graph, tempDir, { requireSpecDeltas: 'error' }); + + expect(completed.has('specs')).toBe(false); + }); + + it('should not synthetically complete specs when requireSpecDeltas is undefined (default)', () => { + const schema = createSchema([ + { id: 'specs', generates: 'specs/**/*.md', description: 'Specs', template: 't.md', requires: [] }, + ]); + const graph = ArtifactGraph.fromSchema(schema); + + const completed = detectCompleted(graph, tempDir); + + expect(completed.has('specs')).toBe(false); + }); + + it('should mark specs as complete from files regardless of requireSpecDeltas', () => { + const schema = createSchema([ + { id: 'specs', generates: 'specs/**/*.md', description: 'Specs', template: 't.md', requires: [] }, + ]); + const graph = ArtifactGraph.fromSchema(schema); + + fs.mkdirSync(path.join(tempDir, 'specs', 'test'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'specs', 'test', 'spec.md'), 'content'); + + const completed = detectCompleted(graph, tempDir, { requireSpecDeltas: 'error' }); + + expect(completed.has('specs')).toBe(true); + }); }); }); diff --git a/test/core/project-config.test.ts b/test/core/project-config.test.ts index 88944659d..3c31b1ac8 100644 --- a/test/core/project-config.test.ts +++ b/test/core/project-config.test.ts @@ -286,6 +286,98 @@ rules: }); }); + describe('requireSpecDeltas parsing', () => { + it('should parse requireSpecDeltas: "error"', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: "error"\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBe('error'); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should parse requireSpecDeltas: "warn"', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: "warn"\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBe('warn'); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should parse requireSpecDeltas: false', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: false\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBe(false); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should default to undefined when requireSpecDeltas is omitted', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBeUndefined(); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should warn and omit requireSpecDeltas with invalid string value', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: "invalid"\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBeUndefined(); + expect(config?.schema).toBe('spec-driven'); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'requireSpecDeltas' field") + ); + }); + + it('should warn and omit requireSpecDeltas with invalid type', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nrequireSpecDeltas: 123\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.requireSpecDeltas).toBeUndefined(); + expect(config?.schema).toBe('spec-driven'); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'requireSpecDeltas' field") + ); + }); + }); + describe('context size limit enforcement', () => { it('should accept context under 50KB limit', () => { const configDir = path.join(tempDir, 'openspec'); diff --git a/test/core/validation.test.ts b/test/core/validation.test.ts index 972815e51..d97862da3 100644 --- a/test/core/validation.test.ts +++ b/test/core/validation.test.ts @@ -412,7 +412,7 @@ Then result`; const specPath = path.join(testDir, 'spec.md'); await fs.writeFile(specPath, specContent); - const validator = new Validator(true); // strict mode + const validator = new Validator({ strictMode: true }); // strict mode const report = await validator.validateSpec(specPath); expect(report.valid).toBe(false); // Should fail due to brief overview warning @@ -436,7 +436,7 @@ Then result`; const specPath = path.join(testDir, 'spec.md'); await fs.writeFile(specPath, specContent); - const validator = new Validator(false); // non-strict mode + const validator = new Validator({ strictMode: false }); // non-strict mode const report = await validator.validateSpec(specPath); expect(report.valid).toBe(true); // Should pass despite warnings @@ -468,7 +468,7 @@ The system MUST implement a circuit breaker with three states. const specPath = path.join(specsDir, 'spec.md'); await fs.writeFile(specPath, deltaSpec); - const validator = new Validator(true); + const validator = new Validator({ strictMode: true }); const report = await validator.validateChangeDeltaSpecs(changeDir); expect(report.valid).toBe(true); @@ -498,7 +498,7 @@ The system SHALL handle all errors gracefully. const specPath = path.join(specsDir, 'spec.md'); await fs.writeFile(specPath, deltaSpec); - const validator = new Validator(true); + const validator = new Validator({ strictMode: true }); const report = await validator.validateChangeDeltaSpecs(changeDir); expect(report.valid).toBe(true); @@ -527,7 +527,7 @@ The system will log all events. const specPath = path.join(specsDir, 'spec.md'); await fs.writeFile(specPath, deltaSpec); - const validator = new Validator(true); + const validator = new Validator({ strictMode: true }); const report = await validator.validateChangeDeltaSpecs(changeDir); expect(report.valid).toBe(false); @@ -555,7 +555,7 @@ The system SHALL implement this feature. const specPath = path.join(specsDir, 'spec.md'); await fs.writeFile(specPath, deltaSpec); - const validator = new Validator(true); + const validator = new Validator({ strictMode: true }); const report = await validator.validateChangeDeltaSpecs(changeDir); expect(report.valid).toBe(true); @@ -582,7 +582,7 @@ The system MUST support mixed case delta headers. const specPath = path.join(specsDir, 'spec.md'); await fs.writeFile(specPath, deltaSpec); - const validator = new Validator(true); + const validator = new Validator({ strictMode: true }); const report = await validator.validateChangeDeltaSpecs(changeDir); expect(report.valid).toBe(true); @@ -591,4 +591,68 @@ The system MUST support mixed case delta headers. expect(report.summary.info).toBe(0); }); }); + + describe('requireSpecDeltas config', () => { + it('should emit ERROR when requireSpecDeltas is "error" (default) and no deltas', async () => { + const changeDir = path.join(testDir, 'empty-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator({ requireSpecDeltas: 'error' }); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + expect(report.summary.errors).toBeGreaterThan(0); + expect(report.issues.some(i => i.level === 'ERROR' && i.message.includes('at least one delta'))).toBe(true); + }); + + it('should emit ERROR by default when requireSpecDeltas not specified', async () => { + const changeDir = path.join(testDir, 'empty-change-default'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator(); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + expect(report.summary.errors).toBeGreaterThan(0); + }); + + it('should emit WARNING when requireSpecDeltas is "warn" and no deltas', async () => { + const changeDir = path.join(testDir, 'warn-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator({ requireSpecDeltas: 'warn' }); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(true); + expect(report.summary.errors).toBe(0); + expect(report.summary.warnings).toBe(1); + expect(report.issues[0].level).toBe('WARNING'); + expect(report.issues[0].message).toContain('allowed by config'); + }); + + it('should fail in strict mode when requireSpecDeltas is "warn" and no deltas', async () => { + const changeDir = path.join(testDir, 'warn-strict-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator({ strictMode: true, requireSpecDeltas: 'warn' }); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + expect(report.summary.errors).toBe(0); + expect(report.summary.warnings).toBe(1); + }); + + it('should emit no issues when requireSpecDeltas is false and no deltas', async () => { + const changeDir = path.join(testDir, 'silent-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator({ requireSpecDeltas: false }); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(true); + expect(report.summary.errors).toBe(0); + expect(report.summary.warnings).toBe(0); + expect(report.issues).toHaveLength(0); + }); + }); }); From f633e325e2897fcf817c1f512bd787f074ccf184 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 08:48:47 -0400 Subject: [PATCH 3/8] Followup, clarifications/corrections to the spec. Thanks coderabbit. --- openspec/changes/allow-specless-changes/design.md | 2 -- .../allow-specless-changes/specs/cli-artifact-workflow/spec.md | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/openspec/changes/allow-specless-changes/design.md b/openspec/changes/allow-specless-changes/design.md index 374774899..3b9f05895 100644 --- a/openspec/changes/allow-specless-changes/design.md +++ b/openspec/changes/allow-specless-changes/design.md @@ -110,8 +110,6 @@ export function detectCompleted( } ``` -~5 lines added. `loadChangeContext()` in `instruction-loader.ts` reads project config and passes it through. - **Why hardcode `'specs'`:** The `specs` artifact is the only one with a glob pattern that might legitimately have zero files. A more general "optional artifacts" system would be over-engineering for now. ### 5. "Skipped" status in display diff --git a/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md b/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md index a8d0dc65d..a2b0c010c 100644 --- a/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md +++ b/openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md @@ -27,7 +27,7 @@ The system SHALL display artifact completion status for a change, including scaf #### Scenario: Status on scaffolded change -- **WHEN** user runs `openspec status --change ` on a change with no artifacts +- **WHEN** user runs `openspec status --change ` on a change with no generated artifact files yet - **THEN** system displays all artifacts with their status - **AND** root artifacts (no dependencies) show as ready `[ ]` - **AND** dependent artifacts show as blocked `[-]` From a19539af972747bc6fd71f8930e0e9cb3609812e Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 08:49:20 -0400 Subject: [PATCH 4/8] For the 'allow-specless-changes' feature: load project config into the validator path. Thanks coderabbit. --- src/commands/change.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/commands/change.ts b/src/commands/change.ts index 936f52695..1ad78f6c6 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -215,7 +215,10 @@ export class ChangeCommand { throw new Error(`Change "${changeName}" not found at ${changeDir}`); } - const validator = new Validator({ strictMode: options?.strict || false }); + const { readProjectConfig } = await import('../core/project-config.js'); + const projectConfig = readProjectConfig(process.cwd()); + const requireSpecDeltas = projectConfig?.requireSpecDeltas ?? 'error'; + const validator = new Validator({ strictMode: options?.strict || false, requireSpecDeltas }); const report = await validator.validateChangeDeltaSpecs(changeDir); if (options?.json) { From f065b8cff1f74ac302f02a27c7789c61970ab5ea Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 09:21:22 -0400 Subject: [PATCH 5/8] openspec validate wasn't clear about what it should do in the event of warnings. I'm proposing a spec for this case where the change is listed as valid but the warnings are still printed. Thanks coderabbit for identifying the issue. --- .../specs/cli-validate/spec.md | 8 + src/commands/change.ts | 20 +- src/commands/validate.ts | 30 ++- test/commands/validate-warnings.test.ts | 171 ++++++++++++++++++ 4 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 test/commands/validate-warnings.test.ts diff --git a/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md b/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md index b38e43153..68c8d10af 100644 --- a/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md +++ b/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md @@ -19,6 +19,14 @@ Validation output SHALL include specific guidance to fix each error, including e - **THEN** emit a WARNING stating "Change has no spec deltas (allowed by config)" - **AND** validation SHALL pass (report.valid is `true` in non-strict mode) +#### Scenario: CLI output when validation passes with warnings +- **WHEN** validating a change that passes (report.valid is `true`) +- **AND** the report contains one or more WARNING-level issues +- **THEN** the CLI SHALL print " '' is valid" to stdout +- **AND** the CLI SHALL print each WARNING issue to stderr (e.g. `⚠ [WARNING] file: ...`) +- **AND** the CLI SHALL exit with code 0 +- **AND** the Next steps footer SHALL NOT be printed (it is reserved for errors) + #### Scenario: No deltas found, requireSpecDeltas is "warn", strict mode - **WHEN** validating a change with zero parsed deltas - **AND** the project config has `requireSpecDeltas` set to `"warn"` diff --git a/src/commands/change.ts b/src/commands/change.ts index 1ad78f6c6..0a1976ac2 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -224,20 +224,26 @@ export class ChangeCommand { if (options?.json) { console.log(JSON.stringify(report, null, 2)); } else { - if (report.valid) { - console.log(`Change "${changeName}" is valid`); - } else { + const hasErrors = report.issues.some(i => i.level === 'ERROR'); + const hasWarnings = report.issues.some(i => i.level === 'WARNING'); + + if (hasErrors) { console.error(`Change "${changeName}" has issues`); report.issues.forEach(issue => { const label = issue.level === 'ERROR' ? 'ERROR' : 'WARNING'; const prefix = issue.level === 'ERROR' ? '✗' : '⚠'; console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); }); - // Next steps footer to guide fixing issues this.printNextSteps(); - if (!options?.json) { - process.exitCode = 1; - } + process.exitCode = 1; + } else if (hasWarnings) { + console.log(`Change "${changeName}" is valid`); + report.issues.forEach(issue => { + const prefix = issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); + }); + } else { + console.log(`Change "${changeName}" is valid`); } } } diff --git a/src/commands/validate.ts b/src/commands/validate.ts index c7b48920a..63f7fd8a0 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -138,8 +138,6 @@ export class ValidateCommand { const report = await validator.validateChangeDeltaSpecs(changeDir); const durationMs = Date.now() - start; this.printReport('change', id, report, durationMs, opts.json); - // Non-zero exit if invalid (keeps enriched output test semantics) - process.exitCode = report.valid ? 0 : 1; return; } const file = path.join(process.cwd(), 'openspec', 'specs', id, 'spec.md'); @@ -147,25 +145,39 @@ export class ValidateCommand { const report = await validator.validateSpec(file); const durationMs = Date.now() - start; this.printReport('spec', id, report, durationMs, opts.json); - process.exitCode = report.valid ? 0 : 1; } private printReport(type: ItemType, id: string, report: { valid: boolean; issues: any[] }, durationMs: number, json: boolean): void { + const hasErrors = report.issues.some((i: any) => i.level === 'ERROR'); + const hasWarnings = report.issues.some((i: any) => i.level === 'WARNING'); + const label = type === 'change' ? 'Change' : 'Specification'; + + if (hasErrors) { + process.exitCode = 1; + } + if (json) { const out = { items: [{ id, type, valid: report.valid, issues: report.issues, durationMs }], summary: { totals: { items: 1, passed: report.valid ? 1 : 0, failed: report.valid ? 0 : 1 }, byType: { [type]: { items: 1, passed: report.valid ? 1 : 0, failed: report.valid ? 0 : 1 } } }, version: '1.0' }; console.log(JSON.stringify(out, null, 2)); return; } - if (report.valid) { - console.log(`${type === 'change' ? 'Change' : 'Specification'} '${id}' is valid`); - } else { - console.error(`${type === 'change' ? 'Change' : 'Specification'} '${id}' has issues`); + + if (hasErrors) { + console.error(`${label} '${id}' has issues`); for (const issue of report.issues) { - const label = issue.level === 'ERROR' ? 'ERROR' : issue.level; + const issueLabel = issue.level === 'ERROR' ? 'ERROR' : issue.level; const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); + console.error(`${prefix} [${issueLabel}] ${issue.path}: ${issue.message}`); } this.printNextSteps(type); + } else if (hasWarnings) { + console.log(`${label} '${id}' is valid`); + for (const issue of report.issues) { + const prefix = issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); + } + } else { + console.log(`${label} '${id}' is valid`); } } diff --git a/test/commands/validate-warnings.test.ts b/test/commands/validate-warnings.test.ts new file mode 100644 index 000000000..aa4d137d4 --- /dev/null +++ b/test/commands/validate-warnings.test.ts @@ -0,0 +1,171 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { runCLI } from '../helpers/run-cli.js'; + +/** + * Tests that both `openspec change validate` and `openspec validate` display + * WARNING-level issues alongside "is valid" and set exit codes correctly: + * exit 0 for warnings-only, exit 1 for errors. + */ + +const SPECLESS_PROPOSAL = [ + '# Change: Warn Test', + '', + '## Why', + 'Bug fix that needs no spec changes and is long enough for validation.', + '', + '## What Changes', + '- Fix a bug', +].join('\n'); + +describe('change validate — requireSpecDeltas warning display', () => { + const projectRoot = process.cwd(); + const testDir = path.join(projectRoot, 'test-change-validate-warn-tmp'); + const changesDir = path.join(testDir, 'openspec', 'changes'); + + async function scaffoldSpeclessChange(changeId: string) { + const changeDir = path.join(changesDir, changeId); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'proposal.md'), SPECLESS_PROPOSAL); + } + + async function writeConfig(content: string) { + await fs.writeFile(path.join(testDir, 'openspec', 'config.yaml'), content); + } + + beforeEach(async () => { + await fs.mkdir(changesDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it('prints "is valid" to stdout and WARNING to stderr when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('w1'); + + const result = await runCLI(['change', 'validate', 'w1'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('is valid'); + expect(result.stderr).toContain('WARNING'); + expect(result.stderr).not.toContain('Next steps'); + }); + + it('exits 1 and prints "has issues" when requireSpecDeltas is default (error) and no deltas', async () => { + await scaffoldSpeclessChange('e1'); + + const result = await runCLI(['change', 'validate', 'e1'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('ERROR'); + expect(result.stderr).toContain('has issues'); + expect(result.stderr).toContain('Next steps'); + }); + + it('prints "is valid" with no warnings when requireSpecDeltas is false', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: false\n'); + await scaffoldSpeclessChange('f1'); + + const result = await runCLI(['change', 'validate', 'f1'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('is valid'); + expect(result.stderr).not.toContain('WARNING'); + expect(result.stderr).not.toContain('ERROR'); + }); + + it('includes warning issues in JSON output when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('j1'); + + const result = await runCLI(['change', 'validate', 'j1', '--json'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + const json = JSON.parse(result.stdout.trim()); + expect(json.valid).toBe(true); + const warningIssues = json.issues.filter((i: any) => i.level === 'WARNING'); + expect(warningIssues.length).toBeGreaterThan(0); + }); +}); + +describe('openspec validate — requireSpecDeltas warning display', () => { + const projectRoot = process.cwd(); + const testDir = path.join(projectRoot, 'test-validate-warn-tmp'); + const changesDir = path.join(testDir, 'openspec', 'changes'); + + async function scaffoldSpeclessChange(changeId: string) { + const changeDir = path.join(changesDir, changeId); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'proposal.md'), SPECLESS_PROPOSAL); + } + + async function writeConfig(content: string) { + await fs.writeFile(path.join(testDir, 'openspec', 'config.yaml'), content); + } + + beforeEach(async () => { + await fs.mkdir(changesDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it('prints "is valid" to stdout and WARNING to stderr when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('w2'); + + const result = await runCLI(['validate', 'w2'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('is valid'); + expect(result.stderr).toContain('WARNING'); + expect(result.stderr).not.toContain('Next steps'); + }); + + it('exits 1 and prints "has issues" when requireSpecDeltas is default (error) and no deltas', async () => { + await scaffoldSpeclessChange('e2'); + + const result = await runCLI(['validate', 'e2'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('ERROR'); + expect(result.stderr).toContain('has issues'); + expect(result.stderr).toContain('Next steps'); + }); + + it('prints "is valid" with no warnings when requireSpecDeltas is false', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: false\n'); + await scaffoldSpeclessChange('f2'); + + const result = await runCLI(['validate', 'f2'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('is valid'); + expect(result.stderr).not.toContain('WARNING'); + expect(result.stderr).not.toContain('ERROR'); + }); + + it('includes warning issues in JSON output when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('j2'); + + const result = await runCLI(['validate', 'j2', '--json'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + const json = JSON.parse(result.stdout.trim()); + expect(json.items).toBeDefined(); + expect(json.items[0].valid).toBe(true); + const warningIssues = json.items[0].issues.filter((i: any) => i.level === 'WARNING'); + expect(warningIssues.length).toBeGreaterThan(0); + }); + + it('shows warnings in --changes bulk JSON output and exits 0', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('b1'); + + const result = await runCLI(['validate', '--changes', '--json'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + const json = JSON.parse(result.stdout.trim()); + const item = json.items.find((i: any) => i.id === 'b1'); + expect(item).toBeDefined(); + expect(item.valid).toBe(true); + const warningIssues = item.issues.filter((i: any) => i.level === 'WARNING'); + expect(warningIssues.length).toBeGreaterThan(0); + }); +}); From 828c79e48d99603a2595e830bbcc08508155fbf6 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 14:22:45 -0400 Subject: [PATCH 6/8] Base validate success/failure on the .valid flag, not the presence/absence of errors or warnings. Fixes --strict with warnings. Thanks coderabbit! --- src/commands/change.ts | 11 ++++--- src/commands/validate.ts | 8 ++--- test/commands/validate-warnings.test.ts | 44 +++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/commands/change.ts b/src/commands/change.ts index 0a1976ac2..77ecdc83d 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -223,16 +223,17 @@ export class ChangeCommand { if (options?.json) { console.log(JSON.stringify(report, null, 2)); + if (!report.valid) { + process.exitCode = 1; + } } else { - const hasErrors = report.issues.some(i => i.level === 'ERROR'); const hasWarnings = report.issues.some(i => i.level === 'WARNING'); - if (hasErrors) { + if (!report.valid) { console.error(`Change "${changeName}" has issues`); report.issues.forEach(issue => { - const label = issue.level === 'ERROR' ? 'ERROR' : 'WARNING'; - const prefix = issue.level === 'ERROR' ? '✗' : '⚠'; - console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); + const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); }); this.printNextSteps(); process.exitCode = 1; diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 63f7fd8a0..e16b60348 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -148,11 +148,10 @@ export class ValidateCommand { } private printReport(type: ItemType, id: string, report: { valid: boolean; issues: any[] }, durationMs: number, json: boolean): void { - const hasErrors = report.issues.some((i: any) => i.level === 'ERROR'); const hasWarnings = report.issues.some((i: any) => i.level === 'WARNING'); const label = type === 'change' ? 'Change' : 'Specification'; - if (hasErrors) { + if (!report.valid) { process.exitCode = 1; } @@ -162,12 +161,11 @@ export class ValidateCommand { return; } - if (hasErrors) { + if (!report.valid) { console.error(`${label} '${id}' has issues`); for (const issue of report.issues) { - const issueLabel = issue.level === 'ERROR' ? 'ERROR' : issue.level; const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${issueLabel}] ${issue.path}: ${issue.message}`); + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); } this.printNextSteps(type); } else if (hasWarnings) { diff --git a/test/commands/validate-warnings.test.ts b/test/commands/validate-warnings.test.ts index aa4d137d4..16f6e53ed 100644 --- a/test/commands/validate-warnings.test.ts +++ b/test/commands/validate-warnings.test.ts @@ -85,6 +85,28 @@ describe('change validate — requireSpecDeltas warning display', () => { const warningIssues = json.issues.filter((i: any) => i.level === 'WARNING'); expect(warningIssues.length).toBeGreaterThan(0); }); + + it('exits 1 and prints "has issues" with --strict when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('s1'); + + const result = await runCLI(['change', 'validate', 's1', '--strict'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('WARNING'); + expect(result.stderr).toContain('has issues'); + }); + + it('marks item invalid in JSON output with --strict when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('sj1'); + + const result = await runCLI(['change', 'validate', 'sj1', '--json', '--strict'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + const json = JSON.parse(result.stdout.trim()); + expect(json.valid).toBe(false); + const warningIssues = json.issues.filter((i: any) => i.level === 'WARNING'); + expect(warningIssues.length).toBeGreaterThan(0); + }); }); describe('openspec validate — requireSpecDeltas warning display', () => { @@ -155,6 +177,28 @@ describe('openspec validate — requireSpecDeltas warning display', () => { expect(warningIssues.length).toBeGreaterThan(0); }); + it('exits 1 and prints "has issues" with --strict when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('s2'); + + const result = await runCLI(['validate', 's2', '--strict'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('WARNING'); + expect(result.stderr).toContain('has issues'); + }); + + it('marks item invalid in JSON output with --strict when requireSpecDeltas is "warn"', async () => { + await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); + await scaffoldSpeclessChange('sj2'); + + const result = await runCLI(['validate', 'sj2', '--json', '--strict'], { cwd: testDir }); + expect(result.exitCode).toBe(1); + const json = JSON.parse(result.stdout.trim()); + expect(json.items[0].valid).toBe(false); + const warningIssues = json.items[0].issues.filter((i: any) => i.level === 'WARNING'); + expect(warningIssues.length).toBeGreaterThan(0); + }); + it('shows warnings in --changes bulk JSON output and exits 0', async () => { await writeConfig('schema: spec-driven\nrequireSpecDeltas: warn\n'); await scaffoldSpeclessChange('b1'); From 0971428e4e9e489acb6d4b06432bb3a5510ecab1 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 14:55:14 -0400 Subject: [PATCH 7/8] Implementation refactoring to unify common output; thanks coderabbit for the suggestion. --- src/commands/change.ts | 17 ++--------------- src/commands/validate.ts | 19 +++---------------- src/utils/index.ts | 6 +++++- src/utils/report-printer.ts | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 32 deletions(-) create mode 100644 src/utils/report-printer.ts diff --git a/src/commands/change.ts b/src/commands/change.ts index 77ecdc83d..52d362adf 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -6,6 +6,7 @@ import { ChangeParser } from '../core/parsers/change-parser.js'; import { Change } from '../core/schemas/index.js'; import { isInteractive } from '../utils/interactive.js'; import { getActiveChangeIds } from '../utils/item-discovery.js'; +import { printReportIssues } from '../utils/report-printer.js'; // Constants for better maintainability const ARCHIVE_DIR = 'archive'; @@ -227,24 +228,10 @@ export class ChangeCommand { process.exitCode = 1; } } else { - const hasWarnings = report.issues.some(i => i.level === 'WARNING'); - + printReportIssues(`Change "${changeName}"`, report); if (!report.valid) { - console.error(`Change "${changeName}" has issues`); - report.issues.forEach(issue => { - const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); - }); this.printNextSteps(); process.exitCode = 1; - } else if (hasWarnings) { - console.log(`Change "${changeName}" is valid`); - report.issues.forEach(issue => { - const prefix = issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); - }); - } else { - console.log(`Change "${changeName}" is valid`); } } } diff --git a/src/commands/validate.ts b/src/commands/validate.ts index e16b60348..e444342f9 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -5,6 +5,7 @@ import { readProjectConfig } from '../core/project-config.js'; import { isInteractive, resolveNoInteractive } from '../utils/interactive.js'; import { getActiveChangeIds, getSpecIds } from '../utils/item-discovery.js'; import { nearestMatches } from '../utils/match.js'; +import { printReportIssues } from '../utils/report-printer.js'; type ItemType = 'change' | 'spec'; @@ -148,9 +149,6 @@ export class ValidateCommand { } private printReport(type: ItemType, id: string, report: { valid: boolean; issues: any[] }, durationMs: number, json: boolean): void { - const hasWarnings = report.issues.some((i: any) => i.level === 'WARNING'); - const label = type === 'change' ? 'Change' : 'Specification'; - if (!report.valid) { process.exitCode = 1; } @@ -161,21 +159,10 @@ export class ValidateCommand { return; } + const label = type === 'change' ? 'Change' : 'Specification'; + printReportIssues(`${label} '${id}'`, report); if (!report.valid) { - console.error(`${label} '${id}' has issues`); - for (const issue of report.issues) { - const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); - } this.printNextSteps(type); - } else if (hasWarnings) { - console.log(`${label} '${id}' is valid`); - for (const issue of report.issues) { - const prefix = issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); - } - } else { - console.log(`${label} '${id}' is valid`); } } diff --git a/src/utils/index.ts b/src/utils/index.ts index e77ddf476..2df7c710c 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -15,4 +15,8 @@ export { export { FileSystemUtils, removeMarkerBlock } from './file-system.js'; // Command reference utilities -export { transformToHyphenCommands } from './command-references.js'; \ No newline at end of file +export { transformToHyphenCommands } from './command-references.js'; + +// Report printing utilities +export { printReportIssues, issueSeverityPrefix } from './report-printer.js'; +export type { ReportIssue } from './report-printer.js'; \ No newline at end of file diff --git a/src/utils/report-printer.ts b/src/utils/report-printer.ts new file mode 100644 index 000000000..c2df1ef42 --- /dev/null +++ b/src/utils/report-printer.ts @@ -0,0 +1,35 @@ +export interface ReportIssue { + level: 'ERROR' | 'WARNING' | 'INFO'; + path: string; + message: string; +} + +export function issueSeverityPrefix(level: ReportIssue['level']): string { + if (level === 'ERROR') return '✗'; + if (level === 'WARNING') return '⚠'; + return 'ℹ'; +} + +/** + * Print a validation report's issues in the standard text format. + * + * Handles the three-way branch: + * 1. invalid → stderr "has issues" + all issues + optional next-steps callback + * 2. valid with warnings → stdout "is valid" + warnings/info to stderr + * 3. clean → stdout "is valid" + * + * Returns true when the report is invalid (caller should set exitCode = 1). + */ +export function printReportIssues( + label: string, + report: { valid: boolean; issues: ReportIssue[] }, +): void { + if (!report.valid) { + console.error(`${label} has issues`); + } else { + console.log(`${label} is valid`); + } + for (const issue of report.issues) { + console.error(`${issueSeverityPrefix(issue.level)} [${issue.level}] ${issue.path}: ${issue.message}`); + } +} From c8602192348708700f53d591c98cae83a87777fd Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 16 Apr 2026 15:01:25 -0400 Subject: [PATCH 8/8] Cleanup incorrect docstring from refactoring --- src/utils/report-printer.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/utils/report-printer.ts b/src/utils/report-printer.ts index c2df1ef42..a9a54e0d1 100644 --- a/src/utils/report-printer.ts +++ b/src/utils/report-printer.ts @@ -14,11 +14,9 @@ export function issueSeverityPrefix(level: ReportIssue['level']): string { * Print a validation report's issues in the standard text format. * * Handles the three-way branch: - * 1. invalid → stderr "has issues" + all issues + optional next-steps callback + * 1. invalid → stderr "has issues" + all issues * 2. valid with warnings → stdout "is valid" + warnings/info to stderr * 3. clean → stdout "is valid" - * - * Returns true when the report is invalid (caller should set exitCode = 1). */ export function printReportIssues( label: string,