Conversation
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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a tri-state project config Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI
participant Config as ProjectConfigReader
participant Validator
participant Graph as ArtifactGraph
participant FS as Filesystem
CLI->>Config: readProjectConfig(projectRoot)
Config-->>CLI: projectConfig (requireSpecDeltas)
CLI->>Validator: new Validator({ strictMode, requireSpecDeltas })
CLI->>Graph: detectCompleted(graph, changeDir, { requireSpecDeltas })
Graph->>FS: scan generated outputs (globs)
FS-->>Graph: foundFiles / none
alt no files & requireSpecDeltas != "error"
Graph-->>CLI: include 'specs' in completed (synthetic -> 'skipped')
else files present
Graph-->>CLI: include matching artifacts as completed ('done')
end
CLI->>Validator: validateChangeDeltaSpecs(changeDir)
Validator-->>CLI: report (errors/warnings/ok)
CLI->>CLI: render status (use 'skipped' for synthetic specs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
openspec/changes/allow-specless-changes/specs/cli-validate/spec.md (2)
14-14: Include both non-blocking config options in remediation guidance.Current note only points to
false; consider mentioning"warn"too, so users can opt into visible-but-non-blocking behavior.✏️ Proposed wording fix
- - Note: "If this change intentionally has no spec deltas, set `requireSpecDeltas: false` in openspec/config.yaml" + - Note: "If this change intentionally has no spec deltas, set `requireSpecDeltas: "warn"` (visible, non-blocking) or `requireSpecDeltas: false` (silent) in openspec/config.yaml"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/allow-specless-changes/specs/cli-validate/spec.md` at line 14, Update the guidance note that references the openspec/config.yaml requireSpecDeltas setting to mention both non-blocking options: set requireSpecDeltas: false to silence validation for intentionally spec-less changes, or set requireSpecDeltas: "warn" to surface a visible non-blocking warning; update the sentence in spec.md that currently only shows `requireSpecDeltas: false` to include and briefly explain the `"warn"` option as an alternative.
47-47: Minor wording trim for clarity.At Line 47, “before listing scenarios” reads more directly than “prior to listing scenarios.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/allow-specless-changes/specs/cli-validate/spec.md` at line 47, Replace the phrase "prior to listing scenarios" with the simpler wording "before listing scenarios" in the introductory sentence that precedes the scenario list (look for the sentence containing "prior to listing scenarios"); update that single line to read "before listing scenarios" and run a quick proofread to ensure punctuation and flow remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/allow-specless-changes/design.md`:
- Line 113: Remove the editorial artifact text "~5 lines added.
`loadChangeContext()` in `instruction-loader.ts` reads project config and passes
it through." from the design doc; open
openspec/changes/allow-specless-changes/design.md, locate the sentence that
begins with "~5 lines added." near the paragraph describing loadChangeContext()
and delete that patch-note residue so the finalized design contains only the
intended prose about loadChangeContext() and instruction-loader.ts.
In `@openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md`:
- Around line 30-33: The scenario wording is contradictory: "no artifacts" (line
containing WHEN user runs `openspec status --change <id>` on a change with no
artifacts) conflicts with the subsequent THEN/AND steps that display artifacts
and their statuses; update the WHEN clause to clarify it refers to a change with
no generated/completed artifact files yet (suggest: "on a change with no
generated artifact files yet" or "no completed artifacts"), so the WHEN reads
e.g. `WHEN user runs openspec status --change <id> on a change with no generated
artifact files yet`, leaving the THEN/AND lines about displaying artifact
statuses unchanged.
In `@openspec/changes/allow-specless-changes/tasks.md`:
- Line 43: The open checklist item "6.3 Verify Windows CI passes (no
path-separator issues in config loading or state detection)" must be resolved
before merge: either run the Windows CI, confirm there are no path-separator
regressions in the config/state logic, and mark the checkbox complete, or
explicitly defer by replacing the checklist entry with a note linking to a newly
created follow-up issue (include the issue number/URL) that scopes the Windows
verification and assigns an owner; update the tasks.md entry to reflect the
chosen resolution so the PR no longer contains an unresolved Windows
verification TODO.
In `@src/commands/change.ts`:
- Line 218: The change validate command isn't honoring the project's
requireSpecDeltas because that flag isn't passed into the Validator; update the
Validator instantiation in the change validate flow to include requireSpecDeltas
(e.g., pass requireSpecDeltas: projectConfig.requireSpecDeltas or
options?.requireSpecDeltas) alongside strictMode so Validator receives the
project's setting; ensure you reference the existing Validator constructor call
and the project config/options variable used in the change command so behavior
matches openspec validate.
---
Nitpick comments:
In `@openspec/changes/allow-specless-changes/specs/cli-validate/spec.md`:
- Line 14: Update the guidance note that references the openspec/config.yaml
requireSpecDeltas setting to mention both non-blocking options: set
requireSpecDeltas: false to silence validation for intentionally spec-less
changes, or set requireSpecDeltas: "warn" to surface a visible non-blocking
warning; update the sentence in spec.md that currently only shows
`requireSpecDeltas: false` to include and briefly explain the `"warn"` option as
an alternative.
- Line 47: Replace the phrase "prior to listing scenarios" with the simpler
wording "before listing scenarios" in the introductory sentence that precedes
the scenario list (look for the sentence containing "prior to listing
scenarios"); update that single line to read "before listing scenarios" and run
a quick proofread to ensure punctuation and flow remain correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed91a91c-b98f-474c-8f78-673774b5545f
📒 Files selected for processing (23)
openspec/changes/allow-specless-changes/.openspec.yamlopenspec/changes/allow-specless-changes/design.mdopenspec/changes/allow-specless-changes/proposal.mdopenspec/changes/allow-specless-changes/specs/artifact-graph/spec.mdopenspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.mdopenspec/changes/allow-specless-changes/specs/cli-validate/spec.mdopenspec/changes/allow-specless-changes/specs/config-loading/spec.mdopenspec/changes/allow-specless-changes/tasks.mdsrc/commands/change.tssrc/commands/spec.tssrc/commands/validate.tssrc/commands/workflow/shared.tssrc/commands/workflow/status.tssrc/core/artifact-graph/instruction-loader.tssrc/core/artifact-graph/state.tssrc/core/project-config.tssrc/core/validation/constants.tssrc/core/validation/validator.tstest/commands/validate.test.tstest/core/artifact-graph/instruction-loader.test.tstest/core/artifact-graph/state.test.tstest/core/project-config.test.tstest/core/validation.test.ts
|
|
||
| - [x] 6.1 Add integration test: temp project with `requireSpecDeltas: false`, change with no specs → `openspec validate <change>` passes, `openspec status --change <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) |
There was a problem hiding this comment.
Unresolved Windows verification task should be closed before merge (or explicitly deferred).
Line 43 is still open; please either complete this CI verification or document deferral with a linked follow-up issue to avoid cross-platform regression risk.
If you want, I can draft a concise follow-up issue template for the Windows path-separator validation scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/allow-specless-changes/tasks.md` at line 43, The open
checklist item "6.3 Verify Windows CI passes (no path-separator issues in config
loading or state detection)" must be resolved before merge: either run the
Windows CI, confirm there are no path-separator regressions in the config/state
logic, and mark the checkbox complete, or explicitly defer by replacing the
checklist entry with a note linking to a newly created follow-up issue (include
the issue number/URL) that scopes the Windows verification and assigns an owner;
update the tasks.md entry to reflect the chosen resolution so the PR no longer
contains an unresolved Windows verification TODO.
There was a problem hiding this comment.
This is pending confirmation via github CI on this PR.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh pr checks 977 --repo Fission-AI/OpenSpec
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check workflow runs on the PR branch
gh run list --repo Fission-AI/OpenSpec --branch allow-specless-changes --limit 10
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Get jobs for the CI workflow run
gh run view 24478530371 --repo Fission-AI/OpenSpec --json jobs | jq '.jobs[] | {name: .name, status: .status, conclusion: .conclusion}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Try viewing the run directly
gh run view 24478530371 --repo Fission-AI/OpenSpec
# Also try getting all jobs
gh api repos/Fission-AI/OpenSpec/actions/runs/24478530371/jobs --jq '.jobs[] | {name: .name, status: .status, conclusion: .conclusion}'
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Try log-failed
gh run view 24478530371 --log-failed --repo Fission-AI/OpenSpec 2>&1 | head -50
# Try API with different endpoint
gh api repos/Fission-AI/OpenSpec/actions/runs/24478530371 --jq '{status: .status, conclusion: .conclusion, name: .name, event: .event, head_branch: .head_branch}'
Length of output: 164
@bsmedberg-xometry, understood — I've checked the CI run directly. The workflow run (#24478530371) shows a conclusion of action_required, meaning no jobs have actually executed yet — GitHub Actions is waiting for a maintainer to approve the workflow run before it starts. This means Windows CI (task 6.3) is still genuinely unconfirmed.
Once a maintainer approves the workflow run and CI completes — including the Windows job — task 6.3 can be marked [x]. In the meantime, the concern stands: the item is open and the cross-platform verification hasn't happened yet.
A few options to consider:
- Approve and run CI now — a repo maintainer can approve the pending workflow run so the full suite (including Windows) executes before merge.
- Defer explicitly — replace the open
[ ]with a note and a linked follow-up issue so the PR can merge without the item appearing as an unresolved TODO. Would you like me to open that follow-up issue?
🧠 Learnings used
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
…e validator path. Thanks coderabbit.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/change.ts (1)
218-241:⚠️ Potential issue | 🟠 MajorText-mode
warnstill behaves likefalse.Line 227 treats every
report.valid === trueresult as a clean pass. WithrequireSpecDeltas: 'warn'in non-strict mode, the validator returns a valid report that still contains warnings, soopenspec change validateprints onlyChange "...\" is validand never emits the warning this feature is supposed to surface.Suggested fix
const validator = new Validator({ strictMode: options?.strict || false, requireSpecDeltas }); const report = await validator.validateChangeDeltaSpecs(changeDir); + const warnings = report.issues.filter(issue => issue.level === 'WARNING'); if (options?.json) { console.log(JSON.stringify(report, null, 2)); } else { if (report.valid) { + warnings.forEach(issue => { + console.error(`⚠ [WARNING] ${issue.path}: ${issue.message}`); + }); console.log(`Change "${changeName}" is valid`); } else { console.error(`Change "${changeName}" has issues`); report.issues.forEach(issue => { const label = issue.level === 'ERROR' ? 'ERROR' : 'WARNING';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/change.ts` around lines 218 - 241, The code treats report.valid as "no issues" but Validator can return report.valid === true while still including WARNING-level issues when requireSpecDeltas === 'warn'; update the printing branch that currently uses report.valid to instead consider report.issues: print "Change ... is valid" only when report.issues is empty (e.g. report.issues?.length === 0), otherwise always iterate and print report.issues (using the existing formatting), and set process.exitCode = 1 only if any issue has level 'ERROR' (e.g. report.issues.some(i => i.level === 'ERROR')); keep the JSON output path unchanged and still call this.printNextSteps() after printing issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/commands/change.ts`:
- Around line 218-241: The code treats report.valid as "no issues" but Validator
can return report.valid === true while still including WARNING-level issues when
requireSpecDeltas === 'warn'; update the printing branch that currently uses
report.valid to instead consider report.issues: print "Change ... is valid" only
when report.issues is empty (e.g. report.issues?.length === 0), otherwise always
iterate and print report.issues (using the existing formatting), and set
process.exitCode = 1 only if any issue has level 'ERROR' (e.g.
report.issues.some(i => i.level === 'ERROR')); keep the JSON output path
unchanged and still call this.printNextSteps() after printing issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38942833-b8fd-4144-bd0f-3895f6972a5b
📒 Files selected for processing (3)
openspec/changes/allow-specless-changes/design.mdopenspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.mdsrc/commands/change.ts
✅ Files skipped from review due to trivial changes (1)
- openspec/changes/allow-specless-changes/specs/cli-artifact-workflow/spec.md
…f 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.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/validate.ts`:
- Around line 151-180: The code currently only treats ERROR-level issues as
failures; change the logic to treat report.valid === false as a failed
validation regardless of issue levels: define a boolean like isInvalid =
!report.valid (or set hasErrors = hasErrors || !report.valid), use that to set
process.exitCode = 1, and to drive the non-JSON output path so that when
isInvalid is true the block that logs "has issues" and prints report.issues is
executed (still rendering each issue by their level), while only when
report.valid is true should the code print the "is valid" branch; update
references to hasErrors/hasWarnings (and the branch that calls
this.printNextSteps) to use this new isInvalid condition so --strict/warn
semantics correctly fail single-item validations.
In `@test/commands/validate-warnings.test.ts`:
- Around line 45-170: The tests miss coverage for the single-item strict mode
where requireSpecDeltas: "warn" should cause warnings to fail; add tests
invoking runCLI with the --strict flag for both change validate <id> and
validate <id> flows (use the existing scaffoldSpeclessChange and writeConfig
helpers), asserting that when config contains "requireSpecDeltas: warn" the
result.exitCode is non-zero (fail), stderr contains 'WARNING' and 'has issues'
(or equivalent failure message), and that JSON output for --json + --strict
marks the item invalid; add these two test cases near the corresponding
single-item tests (mirror the existing 'prints "is valid"...' and 'includes
warning issues in JSON...' cases) so they exercise the single-item strict
reporting path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 251edd60-b68f-4973-bf93-80689c3caf0d
📒 Files selected for processing (4)
openspec/changes/allow-specless-changes/specs/cli-validate/spec.mdsrc/commands/change.tssrc/commands/validate.tstest/commands/validate-warnings.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/change.ts
…sence of errors or warnings. Fixes --strict with warnings. Thanks coderabbit!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/change.ts (1)
230-245: Consider sharing the report formatter withsrc/commands/validate.ts.This branch now mirrors the severity-prefix and warning-reporting logic in
src/commands/validate.ts:167-175almost exactly. Extracting a shared helper would reduce drift the next time output or exit-code semantics change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/change.ts` around lines 230 - 245, The change.ts block duplicates the severity-prefix and issue-printing logic already used in validate.ts (lines ~167-175); extract a shared helper (e.g., formatOrPrintReportIssues / printReportIssues) that accepts the report (or report.issues), a changeName/context string, and a mode flag for using console.error vs console.log or for returning whether to set exit code, implement the prefix selection logic (✗/⚠/ℹ) and per-issue output there, export it from a common helper module (utils/reportPrinter or similar), and replace the duplicated loops in both src/commands/change.ts (around printNextSteps/exitCode handling) and src/commands/validate.ts to call the shared helper so output and exit-code semantics stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/change.ts`:
- Around line 230-245: The change.ts block duplicates the severity-prefix and
issue-printing logic already used in validate.ts (lines ~167-175); extract a
shared helper (e.g., formatOrPrintReportIssues / printReportIssues) that accepts
the report (or report.issues), a changeName/context string, and a mode flag for
using console.error vs console.log or for returning whether to set exit code,
implement the prefix selection logic (✗/⚠/ℹ) and per-issue output there, export
it from a common helper module (utils/reportPrinter or similar), and replace the
duplicated loops in both src/commands/change.ts (around printNextSteps/exitCode
handling) and src/commands/validate.ts to call the shared helper so output and
exit-code semantics stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19034912-da1d-4af7-bc0d-ceb53a42cb20
📒 Files selected for processing (3)
src/commands/change.tssrc/commands/validate.tstest/commands/validate-warnings.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/commands/validate-warnings.test.ts
- src/commands/validate.ts
…for the suggestion.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/report-printer.ts`:
- Around line 13-26: The JSDoc for printReportIssues is incorrect: it claims the
function "Returns true when the report is invalid" and mentions an "optional
next-steps callback" that doesn't exist; update the docstring to match the
actual signature and behavior (no return value) and remove any reference to a
return value or a non-existent callback, or alternatively if you intend a
callback/boolean return add them to the function signature—edit the comment
above printReportIssues (and references to label, report, ReportIssue) so the
description accurately states the three output behaviors and that the function
returns void (or implement the callback/boolean return consistently across the
function and its callers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dde98a21-4f96-468b-bbbd-6dee529e8787
📒 Files selected for processing (4)
src/commands/change.tssrc/commands/validate.tssrc/utils/index.tssrc/utils/report-printer.ts
✅ Files skipped from review due to trivial changes (1)
- src/utils/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/change.ts
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 validatehard-fails withCHANGE_NO_DELTASwhen a change has zero delta specs, and the artifact dependency graph blockstasksonspecscompletion (determined by file existence ofspecs/**/*.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
requireSpecDeltasas a top-level tri-state setting inopenspec/config.yaml:"error"(default when omitted) — current behavior, hard-fail on missing deltas"warn"— emit a WARNING but pass validationfalse— completely suppress the check, no output at allWhen set to
"warn"orfalse, two things change:CHANGE_NO_DELTAScheck emits the configured level (or nothing) instead of ERRORdetectCompleted()synthetically marksspecsas complete so thattasksis unblocked and the propose workflow can proceed without writing spec filesThe proposal instruction/template in the schema is static and doesn't adapt to this config. Projects using this feature should add a
rules.proposalentry 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:
Considered and deferred: per-change metadata
A
skipSpecDeltas: truefield in.openspec.yamlwould allow per-change overrides of the project default. This was considered but deferred because:detectCompleted()) would need to read change metadata, adding I/O to a currently simple functionCapabilities
New Capabilities
(none — this extends existing capabilities)
Modified Capabilities
config-loading: Add top-levelrequireSpecDeltas(tri-state:"error"|"warn"|false) to the ProjectConfig schema and resilient parsingcli-validate: RespectrequireSpecDeltaswhen evaluating theCHANGE_NO_DELTAScheckartifact-graph: Synthetically markspecsas complete indetectCompleted()whenrequireSpecDeltasis not"error"cli-artifact-workflow: Add"skipped"status to status display for synthetically completed artifacts (indicator[~], dim color,"skipped"in JSON)Non-goals
specsartifact disappear fromopenspec status— it remains visible as[~] skippedChangeSchemaZod validation fordeltas.min(1)— that validates the proposal's Capabilities section, not spec filesrulesconfig to adjust AI guidance.openspec.yaml— deferred for a future changeImpact
src/core/project-config.ts— extendProjectConfigSchemawith optionalrequireSpecDeltassrc/core/validation/validator.ts—validateChangeDeltaSpecs()respects the tri-state settingsrc/commands/validate.ts— read project config and pass setting to the validatorsrc/core/artifact-graph/state.ts—detectCompleted()synthetically marksspecsdone when config allowssrc/core/validation/constants.ts— add message for the warning caseSummary by CodeRabbit
New Features
Behavior
Documentation
Tests