feat: add per-change schema metadata (.openspec.yaml) #443
Conversation
|
Warning Rate limit exceeded@TabishB has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces per-change schema metadata support. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / CLI
participant WF as artifact-workflow.ts
participant CU as change-utils.ts
participant CM as change-metadata.ts
participant IL as instruction-loader.ts
participant FS as Filesystem
rect rgb(200, 220, 240)
Note over User,FS: Create Change Flow
User->>WF: create change (name, schema?)
WF->>CU: createChange(projectRoot, name, {schema?})
CU->>CM: validateSchemaName(schema || DEFAULT_SCHEMA)
CM-->>CU: ✓ schema valid or thrown
CU->>FS: mkdir change directory
CU->>CM: writeChangeMetadata(changeDir, {schema, created})
CM->>FS: write .openspec.yaml
FS-->>CM: ✓ metadata written
CM-->>CU: ✓ success
CU-->>WF: ✓ change created
WF-->>User: Display: "Change created with schema: {schema}"
end
rect rgb(220, 240, 200)
Note over User,FS: Load Change Context Flow
User->>WF: status/instructions (changeName, schema?)
WF->>IL: loadChangeContext(projectRoot, changeName, schema?)
IL->>CM: resolveSchemaForChange(changeDir, schema?)
CM->>FS: read .openspec.yaml
FS-->>CM: metadata or null
CM-->>CM: resolve: explicit > metadata > default
CM-->>IL: resolved schema name
IL->>IL: build artifact graph (resolved schema)
IL-->>WF: ChangeContext {schemaName: resolved}
WF-->>User: Display: artifacts using resolved schema
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @test/utils/change-metadata.test.ts:
- Around line 88-99: The test currently asserts on the raw YAML string for
writeChangeMetadata which fails when the YAML lib emits unquoted dates; instead,
parse the file contents (e.g., using the project's YAML parser) and assert the
semantic value: read the file at metaPath, parse the YAML into an object, and
expect parsed.created to equal '2025-01-05' (and keep the schema check on
parsed.schema or parsed['schema'] rather than string contains). This replaces
string matching for created with a value equality assertion.
🧹 Nitpick comments (3)
src/core/artifact-graph/types.ts (1)
25-41: Consider using consistent error parameter naming.The
ChangeMetadataSchemausesmessagefor error customization (lines 30, 36), while the existing schemas in this file useerror(lines 5, 6, 8, 16, 18). For consistency and Zod 4 compatibility, consider usingerrorthroughout.🔎 Proposed fix for consistency
export const ChangeMetadataSchema = z.object({ // Required: which workflow schema this change uses - schema: z.string().min(1, { message: 'schema is required' }), + schema: z.string().min(1, { error: 'schema is required' }), // Optional: creation timestamp (ISO date string) created: z .string() .regex(/^\d{4}-\d{2}-\d{2}$/, { - message: 'created must be YYYY-MM-DD format', + error: 'created must be YYYY-MM-DD format', }) .optional(), });test/utils/change-utils.test.ts (1)
141-147: Consider parsing YAML for consistency.While this test currently passes, it uses the same brittle string matching approach as the failing test above. For consistency and robustness, parse the YAML to verify semantic values.
🔎 Suggested refactor for consistency
+import yaml from 'yaml'; + it('should create .openspec.yaml with custom schema', async () => { await createChange(testDir, 'add-auth', { schema: 'tdd' }); const metaPath = path.join(testDir, 'openspec', 'changes', 'add-auth', '.openspec.yaml'); const content = await fs.readFile(metaPath, 'utf-8'); - expect(content).toContain('schema: tdd'); + const parsed = yaml.parse(content); + expect(parsed.schema).toBe('tdd'); });src/utils/change-utils.ts (1)
5-5: Consider extractingDEFAULT_SCHEMAto a shared constant.
DEFAULT_SCHEMA = 'spec-driven'is duplicated here and insrc/commands/artifact-workflow.ts(line 62). Extracting to a shared location would prevent drift.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
openspec/changes/add-per-change-schema-metadata/tasks.mdsrc/commands/artifact-workflow.tssrc/core/artifact-graph/instruction-loader.tssrc/core/artifact-graph/types.tssrc/utils/change-metadata.tssrc/utils/change-utils.tssrc/utils/index.tstest/core/artifact-graph/instruction-loader.test.tstest/utils/change-metadata.test.tstest/utils/change-utils.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/add-per-change-schema-metadata/tasks.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
tasks.mdcontains implementation checklist with numbered sections and checkbox items
Files:
openspec/changes/add-per-change-schema-metadata/tasks.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
openspec/changes/add-per-change-schema-metadata/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/changes/add-per-change-schema-metadata/tasks.mdsrc/utils/change-metadata.tstest/core/artifact-graph/instruction-loader.test.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
openspec/changes/add-per-change-schema-metadata/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
src/commands/artifact-workflow.ts
🧬 Code graph analysis (6)
test/utils/change-utils.test.ts (1)
src/utils/change-utils.ts (1)
createChange(99-131)
src/core/artifact-graph/instruction-loader.ts (1)
src/utils/change-metadata.ts (1)
resolveSchemaForChange(150-171)
src/utils/change-metadata.ts (2)
src/utils/index.ts (5)
ChangeMetadataError(11-11)validateSchemaName(10-10)writeChangeMetadata(8-8)readChangeMetadata(7-7)resolveSchemaForChange(9-9)src/core/artifact-graph/types.ts (2)
ChangeMetadata(41-41)ChangeMetadataSchema(28-39)
test/core/artifact-graph/instruction-loader.test.ts (1)
src/core/artifact-graph/instruction-loader.ts (1)
loadChangeContext(156-177)
test/utils/change-metadata.test.ts (3)
src/core/artifact-graph/types.ts (1)
ChangeMetadataSchema(28-39)src/utils/change-metadata.ts (5)
writeChangeMetadata(47-77)readChangeMetadata(86-136)ChangeMetadataError(12-21)resolveSchemaForChange(150-171)validateSchemaName(30-38)src/utils/index.ts (5)
writeChangeMetadata(8-8)readChangeMetadata(7-7)ChangeMetadataError(11-11)resolveSchemaForChange(9-9)validateSchemaName(10-10)
src/commands/artifact-workflow.ts (2)
src/core/artifact-graph/instruction-loader.ts (1)
loadChangeContext(156-177)src/utils/change-utils.ts (1)
createChange(99-131)
🪛 GitHub Actions: CI
test/utils/change-utils.test.ts
[error] 138-138: createChange test failed. Expected the metadata file to match /created: "\d{4}-\d{2}-\d{2}"/ but received an unquoted date value in the created field.
test/utils/change-metadata.test.ts
[error] 98-98: writeChangeMetadata test failed. Expected YAML to contain created: "YYYY-MM-DD" (date quoted as string) but got an unquoted date value.
🪛 GitHub Check: Test
test/utils/change-utils.test.ts
[failure] 138-138: test/utils/change-utils.test.ts > createChange > creates directory > should create .openspec.yaml metadata file with default schema
AssertionError: expected 'schema: spec-driven\ncreated: 2026-01…' to match /created: "\d{4}-\d{2}-\d{2}"/
- Expected:
/created: "\d{4}-\d{2}-\d{2}"/
- Received:
"schema: spec-driven
created: 2026-01-06
"
❯ test/utils/change-utils.test.ts:138:23
test/utils/change-metadata.test.ts
[failure] 98-98: test/utils/change-metadata.test.ts > writeChangeMetadata > should write valid YAML metadata file
AssertionError: expected 'schema: spec-driven\ncreated: 2025-01…' to contain 'created: "2025-01-05"'
- Expected
- Received
- created: "2025-01-05"
- schema: spec-driven
- created: 2025-01-05
❯ test/utils/change-metadata.test.ts:98:21
🔇 Additional comments (19)
openspec/changes/add-per-change-schema-metadata/tasks.md (1)
1-29: LGTM! Tasks checklist follows guidelines.The tasks.md file properly follows the coding guidelines with numbered sections and checkbox items. All tasks are marked complete, consistent with the PR objectives stating all 654 tests pass.
Based on coding guidelines requiring numbered sections and checkbox items in tasks.md files.
test/core/artifact-graph/instruction-loader.test.ts (1)
90-124: LGTM! Schema resolution tests correctly validate precedence logic.The three new test cases properly exercise the schema resolution order (explicit > metadata > default):
- Auto-detection from
.openspec.yamlwhen no explicit schema provided- Explicit schema parameter overrides metadata file
- Default
spec-drivenfallback when neither existsThe test implementation is thorough and correctly validates both the context's
schemaNameand the graph's name.test/utils/change-utils.test.ts (1)
150-156: LGTM! Schema validation test correctly verifies error handling.The test properly validates that an unknown schema name throws an error during change creation.
src/core/artifact-graph/instruction-loader.ts (2)
6-6: LGTM!Import correctly added for the new schema resolution utility.
146-154: Well-documented schema resolution with clean delegation.The updated JSDoc clearly explains the resolution order (explicit > metadata > default), and the implementation correctly delegates to
resolveSchemaForChange. The optionalschemaNameparameter enables auto-detection while maintaining backward compatibility.Also applies to: 156-177
src/utils/change-utils.ts (2)
3-13: LGTM!Clean interface definition and imports. The
CreateChangeOptionsinterface with optionalschemamaintains backward compatibility.
99-131: Solid implementation of schema-aware change creation.The flow is well-structured:
- Validates name format first
- Validates schema existence before any filesystem operations
- Checks for existing directory to prevent overwrites
- Creates directory and writes metadata atomically
The date formatting using
toISOString().split('T')[0]correctly producesYYYY-MM-DDmatching the schema's regex.src/utils/index.ts (1)
1-12: LGTM!Clean barrel export file with proper separation of type and value exports. All new metadata utilities are correctly exposed in the public API.
src/commands/artifact-workflow.ts (5)
191-197: LGTM!Good pattern: validates schema existence only when explicitly provided, allowing
loadChangeContextto handle auto-detection otherwise.
260-283: LGTM!Instructions command follows the same validation pattern, and correctly uses
context.schemaName(the resolved schema) in error messages.
433-439: LGTM!Apply instructions command correctly delegates schema resolution to
loadChangeContextviagenerateApplyInstructions, maintaining consistent behavior.Also applies to: 519-525
656-697: LGTM!The new change command properly validates schema before creation and provides clear success messaging that includes the schema used.
792-792: LGTM!CLI help text clearly communicates that
--schemais an override and that auto-detection from.openspec.yamlis the default behavior.Also applies to: 809-809, 849-849
src/utils/change-metadata.ts (6)
1-7: LGTM!Clean imports and well-named constant for the metadata filename.
9-21: LGTM!Good custom error class that includes the metadata file path for debugging context. The optional
causeproperty enables error chaining.
23-38: LGTM!Clean validation function with descriptive error message listing available schemas.
40-77: LGTM!Solid implementation with layered validation:
- Schema existence check via
validateSchemaName- Structure validation via Zod schema
- I/O error handling with proper error wrapping
This ensures both valid schema reference and valid YAML structure before writing.
79-136: LGTM!Comprehensive read function with proper error handling:
- Returns
nullfor missing files (expected case for legacy changes)- Throws
ChangeMetadataErrorfor corrupted/invalid files- Validates both YAML structure and schema existence
The post-Zod schema existence check (lines 127-133) is important since
ChangeMetadataSchemaonly validates the string format, not that the schema actually exists.
138-171: LGTM!Resolution logic correctly implements the documented priority:
- Explicit override wins
- Metadata file schema if present
- Default fallback
The silent catch block (line 165) is intentional—if metadata is corrupted or has an invalid schema, falling back to default is reasonable for this resolution function.
| it('should write valid YAML metadata file', async () => { | ||
| writeChangeMetadata(changeDir, { | ||
| schema: 'spec-driven', | ||
| created: '2025-01-05', | ||
| }); | ||
|
|
||
| const metaPath = path.join(changeDir, '.openspec.yaml'); | ||
| const content = await fs.readFile(metaPath, 'utf-8'); | ||
|
|
||
| expect(content).toContain('schema: spec-driven'); | ||
| expect(content).toContain('created: "2025-01-05"'); | ||
| }); |
There was a problem hiding this comment.
Fix test to handle unquoted YAML date values.
The test expects the created field to be quoted (created: "2025-01-05"), but the YAML library outputs unquoted dates (created: 2025-01-05), which is valid YAML for strings. This causes the pipeline failure.
Instead of checking the raw YAML string format, parse the YAML and verify the semantic value.
🔎 Proposed fix using YAML parsing
+import yaml from 'yaml';
+
it('should write valid YAML metadata file', async () => {
writeChangeMetadata(changeDir, {
schema: 'spec-driven',
created: '2025-01-05',
});
const metaPath = path.join(changeDir, '.openspec.yaml');
const content = await fs.readFile(metaPath, 'utf-8');
+ const parsed = yaml.parse(content);
- expect(content).toContain('schema: spec-driven');
- expect(content).toContain('created: "2025-01-05"');
+ expect(parsed.schema).toBe('spec-driven');
+ expect(parsed.created).toBe('2025-01-05');
});Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: CI
[error] 98-98: writeChangeMetadata test failed. Expected YAML to contain created: "YYYY-MM-DD" (date quoted as string) but got an unquoted date value.
🪛 GitHub Check: Test
[failure] 98-98: test/utils/change-metadata.test.ts > writeChangeMetadata > should write valid YAML metadata file
AssertionError: expected 'schema: spec-driven\ncreated: 2025-01…' to contain 'created: "2025-01-05"'
- Expected
- Received
- created: "2025-01-05"
- schema: spec-driven
- created: 2025-01-05
❯ test/utils/change-metadata.test.ts:98:21
🤖 Prompt for AI Agents
In @test/utils/change-metadata.test.ts around lines 88-99, The test currently
asserts on the raw YAML string for writeChangeMetadata which fails when the YAML
lib emits unquoted dates; instead, parse the file contents (e.g., using the
project's YAML parser) and assert the semantic value: read the file at metaPath,
parse the YAML into an object, and expect parsed.created to equal '2025-01-05'
(and keep the schema check on parsed.schema or parsed['schema'] rather than
string contains). This replaces string matching for created with a value
equality assertion.
This feature enables workflow schema auto-detection for changes: - Add ChangeMetadataSchema Zod schema to types.ts - Create change-metadata.ts with writeChangeMetadata(), readChangeMetadata() - Update createChange() to accept optional schema param and write metadata - Modify loadChangeContext() to auto-detect schema from .openspec.yaml - Add --schema option to openspec new change command - Update status/instructions commands to auto-detect schema from metadata Schema resolution order: 1. Explicit --schema flag (if provided) 2. Schema from .openspec.yaml in change directory 3. Default 'spec-driven'
TabishB
force-pushed
the
TabishB/west-monroe
branch
from
d6514b4 to
afce92e
Compare
Summary
.openspec.yamlmetadata file to each change directory, storing the workflow schema and creation datestatus,instructions,new change)--schemaflag >.openspec.yamlmetadata > defaultspec-drivenChangeMetadataSchemaZod schema for validationChanges
src/core/artifact-graph/types.tsChangeMetadataSchemaandChangeMetadatatypesrc/utils/change-metadata.tswriteChangeMetadata(),readChangeMetadata(),resolveSchemaForChange()src/utils/index.tssrc/utils/change-utils.tscreateChange()to accept options with schema, write.openspec.yamlsrc/core/artifact-graph/instruction-loader.tsloadChangeContext()to auto-detect schema from metadatasrc/commands/artifact-workflow.ts--schemaoption tonew change, update commands for auto-detectionTest plan
ChangeMetadataSchemavalidates correctly (valid/invalid cases)writeChangeMetadata()creates valid YAML with schema and created datereadChangeMetadata()parses and validates schema, throws for invalidloadChangeContext()auto-detects schema from.openspec.yamlspec-drivenwhen no metadata exists--schemaflag overrides metadata🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.