fix(schema): validate artifacts before forced init

[showms]

Summary

• validate schema-init artifact IDs before forced replacement
• preserve an existing schema when artifact validation fails
• add command-level regression coverage and a patch changeset

Problem

openspec schema init --force removed the destination schema before validating --artifacts.

When an unknown artifact such as task was supplied, the command returned an error but had already deleted the existing schema.

Root cause

The destination directory was removed before the artifact list was parsed and validated.

Fix

Defer removal of the existing schema until:

• command input has been collected
• artifact IDs have been validated
• artifact definitions have been created
• the new schema has been constructed in memory

The existing error output, exit code, and successful force-overwrite behavior remain unchanged.

Tests

• pnpm exec vitest run test/commands/schema.test.ts
• pnpm run lint
• pnpm run build
• openspec validate fix-schema-init-force-validation-order --strict

Summary by CodeRabbit

• Bug Fixes
  • openspec schema init --force now validates requested artifact IDs before replacing an existing project schema.
  • If an unknown artifact is provided, the existing schema.yaml and other existing files are preserved and the command exits with the expected error.
  • With valid artifacts, forced initialization still replaces the schema and generates the expected outputs.
• Tests
  • Added command-level regression tests covering validation-failure preservation and successful forced replacement.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ea433f0-384a-479e-a942-d16ef37abb3d

📥 Commits

Reviewing files that changed from the base of the PR and between 23ea871 and dcde974.

📒 Files selected for processing (1)

• test/commands/schema.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/commands/schema.test.ts

---

📝 Walkthrough

Walkthrough

schema init --force now validates artifact IDs before deleting or recreating an existing project-local schema. Invalid artifacts preserve existing files and return the existing error contract, while valid artifacts proceed with replacement. Command-level tests cover both outcomes.

Changes

Schema init force validation

Layer / File(s)	Summary
Behavior contract and rollout metadata openspec/changes/fix-schema-init-force-validation-order/*, .changeset/schema-init-validates-before-force.md	Documents validation-before-replacement behavior, preservation on failure, successful replacement, regression coverage, and release metadata.
Deferred schema replacement src/commands/schema.ts	Defers schema deletion and directory creation until after inputs and artifact IDs are collected and validated.
Command-level regression coverage test/commands/schema.test.ts	Executes the registered command and verifies output, exit status, preservation on invalid artifacts, and replacement on valid artifacts.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CommanderProgram
  participant registerSchemaCommand
  participant ArtifactValidation
  participant Filesystem
  CommanderProgram->>registerSchemaCommand: parseAsync schema init --force
  registerSchemaCommand->>ArtifactValidation: collect and validate artifact inputs
  ArtifactValidation-->>registerSchemaCommand: validation result
  registerSchemaCommand->>Filesystem: remove existing schema after success
  registerSchemaCommand->>Filesystem: create schema directory

Loading

Possibly related PRs

• Fission-AI/OpenSpec#525: Introduced the schema command group and its schema init implementation and tests.

Suggested reviewers: tabishb

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: validating artifacts before forced schema init.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/commands/schema.test.ts`:
- Around line 288-317: Update the successful forced-init test around
runSchemaCommand to also assert that process.exitCode is 0, alongside the
existing created:true response and artifact checks. Ensure the assertion
verifies successful completion without changing the test’s existing setup or
validations.

🪄 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 Plus

Run ID: a4c9ee34-b2a4-4253-b04f-07afc4ba6bd1

📥 Commits

Reviewing files that changed from the base of the PR and between 19d4171 and 23ea871.

📒 Files selected for processing (8)

• .changeset/schema-init-validates-before-force.md
• openspec/changes/fix-schema-init-force-validation-order/.openspec.yaml
• openspec/changes/fix-schema-init-force-validation-order/design.md
• openspec/changes/fix-schema-init-force-validation-order/proposal.md
• openspec/changes/fix-schema-init-force-validation-order/specs/schema-init-command/spec.md
• openspec/changes/fix-schema-init-force-validation-order/tasks.md
• src/commands/schema.ts
• test/commands/schema.test.ts

[alfred-openspec]

Reviewed at 4005bb9. The validation-before-replacement ordering is correct; the focused command tests, build, strict change validation, and CI all pass.

[clay-good]

Code review — verdict: good to merge

This is the strongest PR in the current batch. Real data-loss bug, minimal fix, honest scoping, verified end-to-end.

1. Real need — yes, and it's a data-loss bug

Confirmed against main. I created a schema, dropped a sentinel file in it to stand in for user customization, then ran schema init --force with a typo'd artifact (task instead of tasks):

### MAIN : before  -> schema.yaml sentinel.txt templates
- Removing existing schema 'myschema'...
Error: Unknown artifact 'task'
Valid artifacts: proposal, specs, design, tasks
### MAIN : after   ->
### MAIN : sentinel survives? NO — DATA LOSS

The command deletes the directory, then validates, then errors out — leaving nothing behind. A single-character typo destroys a hand-customized schema and all its templates, with no backup and no undo. That's about as clear-cut as a bug report gets, and the severity is higher than the diff size suggests.

Same scenario on this branch:

### PR : before  -> schema.yaml sentinel.txt templates
Error: Unknown artifact 'task'
Valid artifacts: proposal, specs, design, tasks
### PR : after   -> schema.yaml sentinel.txt templates
### PR : sentinel survives? YES

Note the error text and the valid-artifact list are byte-identical — only the destruction is gone.

2. Does it work — yes, and I checked the half that's easy to miss

The interesting risk on a "move the destructive call later" fix is the opposite failure: that a successful --force stops replacing properly. I tested that path explicitly:

$ openspec schema init myschema --force --artifacts proposal,tasks
--- after VALID force:    schema.yaml templates
--- sentinel removed?     YES
--- artifacts in schema.yaml:
  - id: proposal
  - id: tasks

Correct: the stale sentinel is gone, the directory is genuinely replaced rather than merged into, and the new artifact set is what was requested.

I also read the code that now sits between the old mkdirSync position and the new one (src/commands/schema.ts, the selectedArtifacts map and the schema object construction). It is purely in-memory — no filesystem writes, no early returns. So relocating the rmSync/mkdirSync pair below it can't strand a half-built directory or skip a write. That's the property the fix depends on, and it holds.

Full suite: 112 files / 2255 tests, all passing (base main is 2253, so the two new command-level tests are additive; no existing expectation was modified).

3. Breaking changes — none

• Failure path: same message, same valid-artifact list, same non-zero exit code. The only observable difference is that your files still exist afterwards, which no one can be depending on.
• Success path: verified identical behaviour above.
• No change to accepted artifact IDs, list parsing, or any other schema subcommand.
• Nothing outside schema init's action handler is touched.

4. Scope — correctly surgical

13 added / 9 removed lines in one function. No new abstractions, no rollback machinery, no temp-directory staging. The design doc's Non-Goals section is worth calling out approvingly:

Make replacement transactional for filesystem failures that occur after validation succeeds.

That's the right call. Full transactional replacement (stage to temp, atomic rename) would drag in cross-platform rename semantics and turn a two-line reordering into a much riskier change. Fixing the deterministic, reachable, user-triggered failure and explicitly deferring the rare post-validation I/O failure is good engineering judgment, and it's documented rather than silently skipped.

Notes (nothing blocking)

• A write failure after validation can still leave a partially replaced schema. The design doc already says this and treats it as separate hardening — I agree, and I'd only suggest filing it as a follow-up issue so it isn't lost.
• openspec/changes/fix-schema-init-force-validation-order/ follows the repo's existing dogfooding convention (there are 82 entries under openspec/changes/archive/), so that's fine — just remember to archive it once this lands.
• Adding the changeset was the right move; this deserves a patch release note given it's data loss.

Nice, disciplined fix.