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..3b9f05895 --- /dev/null +++ b/openspec/changes/allow-specless-changes/design.md @@ -0,0 +1,152 @@ +## 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; +} +``` + +**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..a2b0c010c --- /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 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 `[-]` + +### 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..68c8d10af --- /dev/null +++ b/openspec/changes/allow-specless-changes/specs/cli-validate/spec.md @@ -0,0 +1,56 @@ +## 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: 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"` +- **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..5d58d4b92 --- /dev/null +++ b/openspec/changes/allow-specless-changes/tasks.md @@ -0,0 +1,43 @@ +## 1. Extend ProjectConfig schema + +- [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 + +- [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 + +- [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 + +- [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 + +- [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 + +- [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..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'; @@ -215,26 +216,22 @@ export class ChangeCommand { throw new Error(`Change "${changeName}" not found at ${changeDir}`); } - const validator = new Validator(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) { console.log(JSON.stringify(report, null, 2)); + if (!report.valid) { + process.exitCode = 1; + } } else { - if (report.valid) { - console.log(`Change "${changeName}" is valid`); - } else { - 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 + printReportIssues(`Change "${changeName}"`, report); + if (!report.valid) { this.printNextSteps(); - if (!options?.json) { - process.exitCode = 1; - } + process.exitCode = 1; } } } 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..e444342f9 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -1,9 +1,11 @@ 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'; +import { printReportIssues } from '../utils/report-printer.js'; type ItemType = 'change' | 'spec'; @@ -128,15 +130,15 @@ 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(); 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'); @@ -144,24 +146,22 @@ 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 { + if (!report.valid) { + 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`); - for (const issue of report.issues) { - const label = issue.level === 'ERROR' ? 'ERROR' : issue.level; - const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); - } + + const label = type === 'change' ? 'Change' : 'Specification'; + printReportIssues(`${label} '${id}'`, report); + if (!report.valid) { this.printNextSteps(type); } } @@ -191,7 +191,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/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..a9a54e0d1 --- /dev/null +++ b/src/utils/report-printer.ts @@ -0,0 +1,33 @@ +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 + * 2. valid with warnings → stdout "is valid" + warnings/info to stderr + * 3. clean → stdout "is valid" + */ +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}`); + } +} diff --git a/test/commands/validate-warnings.test.ts b/test/commands/validate-warnings.test.ts new file mode 100644 index 000000000..16f6e53ed --- /dev/null +++ b/test/commands/validate-warnings.test.ts @@ -0,0 +1,215 @@ +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); + }); + + 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', () => { + 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('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'); + + 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); + }); +}); 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); + }); + }); });