feat: restructure schemas as directories with templates #411
Conversation
Move built-in schemas from embedded TypeScript objects to a file-based directory structure. This enables co-located templates alongside schemas. Changes: - Remove builtin-schemas.ts (replaced by file-based schemas) - Add schemas/ directory at package root with spec-driven and tdd schemas - Update resolveSchema() to load from directory structure - Resolution checks user dir → package dir
📝 WalkthroughWalkthroughThe PR migrates from compile-time schema definitions to a filesystem-based schema discovery system. Built-in schemas (spec-driven and TDD workflows) are now packaged as YAML specifications with Markdown templates in a structured directory layout. The resolver dynamically discovers schemas from both package-installed and user-override locations at runtime. Changes
Sequence Diagram(s)sequenceDiagram
actor App as Application
participant UserDir as User Schemas Dir<br/>(XDG_DATA_HOME)
participant PkgDir as Package Schemas Dir<br/>(dist/schemas)
participant Resolver as SchemaResolver
participant FS as Filesystem
App->>Resolver: getSchemaDir(name)
rect rgb(230, 240, 255)
Note over Resolver,FS: Check user overrides first
Resolver->>UserDir: Check for schema.yaml
UserDir->>FS: File exists?
FS-->>UserDir: Yes / No
UserDir-->>Resolver: Path or null
end
alt User schema found
Resolver-->>App: Return user schema path
else User schema not found
rect rgb(240, 230, 255)
Note over Resolver,FS: Fall back to package built-ins
Resolver->>PkgDir: Check for schema.yaml
PkgDir->>FS: File exists?
FS-->>PkgDir: Yes / No
PkgDir-->>Resolver: Path or null
end
alt Package schema found
Resolver-->>App: Return package schema path
else No schema found
Resolver-->>App: Return null
end
end
App->>Resolver: resolveSchema(name)
Resolver->>Resolver: Locate via getSchemaDir()
alt Schema directory found
Resolver->>FS: Read & parse schema.yaml
FS-->>Resolver: YAML content
Resolver->>Resolver: Validate schema
Resolver-->>App: Return SchemaYaml
else Not found or error
Resolver-->>App: Throw SchemaLoadError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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: 1
🧹 Nitpick comments (2)
schemas/spec-driven/templates/spec.md (1)
6-8: Consider adding a GIVEN clause for complete BDD scenarios.The template uses WHEN/THEN but omits GIVEN, which is standard for BDD-style scenarios to establish preconditions. If this is intentional for simplicity, the current format is acceptable.
#### Scenario: <!-- scenario name --> +- **GIVEN** <!-- precondition --> - **WHEN** <!-- condition --> - **THEN** <!-- expected outcome -->test/core/artifact-graph/resolver.test.ts (1)
29-35: Consider asserting the path exists or contains expected structure.The test only checks that
getPackageSchemasDir()returns a non-empty string, but doesn't verify the path is valid or contains expected schemas. This could pass even if the path resolution is incorrect.it('should return a valid path', () => { const schemasDir = getPackageSchemasDir(); expect(typeof schemasDir).toBe('string'); expect(schemasDir.length).toBeGreaterThan(0); // Consider adding: expect(schemasDir).toContain('schemas'); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
package.jsonschemas/spec-driven/schema.yamlschemas/spec-driven/templates/design.mdschemas/spec-driven/templates/proposal.mdschemas/spec-driven/templates/spec.mdschemas/spec-driven/templates/tasks.mdschemas/tdd/schema.yamlschemas/tdd/templates/docs.mdschemas/tdd/templates/implementation.mdschemas/tdd/templates/spec.mdschemas/tdd/templates/test.mdsrc/core/artifact-graph/builtin-schemas.tssrc/core/artifact-graph/index.tssrc/core/artifact-graph/resolver.tstest/core/artifact-graph/resolver.test.ts
💤 Files with no reviewable changes (1)
- src/core/artifact-graph/builtin-schemas.ts
🧰 Additional context used
🧠 Learnings (13)
📓 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>/`
📚 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/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
schemas/spec-driven/templates/spec.mdschemas/spec-driven/templates/design.mdschemas/spec-driven/templates/tasks.mdschemas/tdd/templates/spec.mdschemas/spec-driven/templates/proposal.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/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
schemas/spec-driven/templates/spec.mdschemas/spec-driven/templates/design.mdschemas/tdd/templates/spec.mdschemas/tdd/templates/implementation.mdschemas/spec-driven/templates/proposal.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:
schemas/spec-driven/templates/spec.mdschemas/tdd/templates/test.mdschemas/tdd/schema.yamlschemas/spec-driven/schema.yamlschemas/spec-driven/templates/design.mdschemas/spec-driven/templates/tasks.mdschemas/tdd/templates/spec.mdschemas/tdd/templates/implementation.mdschemas/tdd/templates/docs.mdschemas/spec-driven/templates/proposal.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/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
schemas/spec-driven/templates/spec.mdschemas/tdd/templates/spec.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/*/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:
schemas/spec-driven/templates/spec.mdschemas/spec-driven/schema.yamlschemas/spec-driven/templates/design.mdschemas/tdd/templates/spec.mdschemas/tdd/templates/implementation.mdschemas/tdd/templates/docs.mdschemas/spec-driven/templates/proposal.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/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
schemas/spec-driven/templates/spec.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: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
schemas/spec-driven/templates/spec.mdschemas/tdd/templates/test.mdschemas/spec-driven/templates/design.mdschemas/spec-driven/templates/tasks.mdschemas/tdd/templates/spec.mdschemas/tdd/templates/implementation.mdschemas/tdd/templates/docs.mdschemas/spec-driven/templates/proposal.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/**/specs/**/spec.md : Use `#### Scenario: Name` format (4 hashtags) for scenario headers, not bullets or bold text
Applied to files:
schemas/spec-driven/templates/spec.mdschemas/tdd/templates/spec.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/specs/**/spec.md : Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Applied to files:
schemas/spec-driven/templates/spec.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: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
schemas/spec-driven/schema.yaml
📚 Learning: 2025-11-25T01:08:02.839Z
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
Applied to files:
schemas/spec-driven/schema.yamlschemas/spec-driven/templates/proposal.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/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
schemas/spec-driven/schema.yamlschemas/spec-driven/templates/tasks.md
🧬 Code graph analysis (2)
test/core/artifact-graph/resolver.test.ts (1)
src/core/artifact-graph/resolver.ts (6)
getPackageSchemasDir(26-30)getUserSchemasDir(35-37)getSchemaDir(49-65)resolveSchema(78-122)SchemaLoadError(11-20)listSchemas(128-158)
src/core/artifact-graph/resolver.ts (3)
src/core/artifact-graph/index.ts (6)
getPackageSchemasDir(25-25)getUserSchemasDir(26-26)getSchemaDir(24-24)listSchemas(23-23)parseSchema(12-12)SchemaValidationError(12-12)src/core/global-config.ts (1)
getGlobalDataDir(57-78)src/core/artifact-graph/schema.ts (2)
parseSchema(23-45)SchemaValidationError(5-10)
🔇 Additional comments (19)
package.json (1)
35-35: LGTM! Necessary change to expose schemas directory.Adding the schemas directory to the published package files is required for the new file-based schema structure with co-located templates.
schemas/spec-driven/templates/proposal.md (1)
1-11: LGTM! Template aligns with spec-driven workflow conventions.The three-section structure (Why, What Changes, Impact) matches the expected proposal format. The HTML comment placeholders provide clean guidance without prescribing specific content.
Based on learnings, proposals should follow this structure.
schemas/tdd/templates/test.md (1)
1-11: LGTM! Clean test template with standard Given/When/Then format.The template provides a clear structure for test documentation with a test plan section and Given/When/Then format for test cases, which is industry standard for BDD/TDD.
schemas/spec-driven/templates/tasks.md (1)
1-9: LGTM! Tasks template follows the expected format.The numbered sections with checkbox items and hierarchical numbering (1.1, 1.2, etc.) align with the tasks.md conventions for implementation checklists.
Based on learnings, this structure is correct.
schemas/tdd/templates/spec.md (1)
1-11: LGTM! Appropriate structure for TDD workflow spec.The Feature/Requirements/Acceptance Criteria structure provides a clear foundation for TDD specifications. This template serves a different purpose than spec-driven change proposals, so the simpler format is appropriate.
schemas/tdd/templates/implementation.md (1)
1-11: LGTM! Well-structured implementation documentation template.The three-section structure (Implementation Notes, API, Usage) covers the essential aspects of implementation documentation with a logical flow from technical details to public interface to usage examples.
schemas/spec-driven/templates/design.md (1)
1-19: LGTM! Comprehensive design document template.The structure (Context, Goals/Non-Goals, Decisions, Risks/Trade-offs) follows standard design document format and provides clear guidance for when design documentation is needed for complex changes.
Based on learnings, design.md is optional but valuable for cross-cutting changes, new dependencies, or significant complexity.
schemas/tdd/templates/docs.md (1)
1-15: LGTM! Standard documentation template structure.The four-section structure (Overview, Getting Started, Examples, Reference) follows standard user documentation patterns with a logical progression from introduction to quick start to detailed examples and reference material.
schemas/spec-driven/schema.yaml (1)
1-28: LGTM!The schema correctly defines the spec-driven workflow with proper artifact dependencies and template references. The dependency chain (proposal → specs/design → tasks) is well-structured and acyclic.
schemas/tdd/schema.yaml (1)
4-27: LGTM!The artifact definitions and dependency chain are correctly structured for a TDD workflow.
src/core/artifact-graph/index.ts (1)
21-28: LGTM!The consolidated export block cleanly exposes the new resolver API. This is a well-organized public surface for the filesystem-based schema resolution system.
test/core/artifact-graph/resolver.test.ts (3)
37-70: LGTM!Good test coverage for
getUserSchemasDirandgetSchemaDir, including the user override preference behavior.
72-265: LGTM!Comprehensive test coverage for
resolveSchema:
- Built-in schema loading
- Extension stripping (.yaml, .yml)
- User override precedence
- Validation error handling with path context
- Cycle detection
- Invalid requires references
- YAML syntax error handling
- Fallback to built-in when no user override
267-326: LGTM!Good coverage for
listSchemas:
- Built-in schema listing
- User override inclusion
- Deduplication
- Sorted output
- Only directories with schema.yaml are included
src/core/artifact-graph/resolver.ts (5)
35-37: LGTM!Clean delegation to
getGlobalDataDir()with a clear path structure.
49-65: LGTM!The resolution order (user override → package built-in) is correctly implemented with existence checks for
schema.yamlfiles.
78-122: LGTM!The
resolveSchemafunction has robust error handling:
- Normalizes input by stripping
.yaml/.ymlextensions- Provides helpful error messages listing available schemas
- Wraps IO errors and validation errors in
SchemaLoadErrorwith path context- Properly chains the original error as
cause
128-158: Consider handling potential race conditions in concurrent access.The
listSchemasfunction uses synchronous file operations without any locking. While acceptable for typical CLI usage, if called concurrently while schemas are being added/removed, it could produce inconsistent results.For a CLI tool, this is generally acceptable, but worth documenting or considering if the API is exposed for programmatic use.
26-30: The relative path navigation is correct and not fragile.The path
'..', '..', '..', 'schemas'correctly navigates from the compiled output atdist/core/artifact-graph/resolver.jsto the package root'sschemas/directory. This structure is guaranteed bytsconfig.json(rootDir: "./src",outDir: "./dist"), making the path deterministic and tied directly to the build configuration. Theschemas/directory exists at the package root, and the function is already tested. No runtime checks are needed.Likely an incorrect or invalid review comment.
| @@ -0,0 +1,27 @@ | |||
| name: tdd | |||
| version: 1 | |||
| description: Test-driven development workflow - tests → implementation → docs | |||
There was a problem hiding this comment.
Description doesn't mention the initial spec step.
The description says "tests → implementation → docs" but the workflow actually starts with spec before tests. Consider updating for accuracy:
-description: Test-driven development workflow - tests → implementation → docs
+description: Test-driven development workflow - spec → tests → implementation → docs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: Test-driven development workflow - tests → implementation → docs | |
| description: Test-driven development workflow - spec → tests → implementation → docs |
🤖 Prompt for AI Agents
In schemas/tdd/schema.yaml around line 3, the description omits the initial
"spec" step and currently reads "tests → implementation → docs"; update the
description to include the initial spec step (e.g., "spec → tests →
implementation → docs" or equivalent) so the workflow order accurately reflects
spec before tests.
Summary
builtin-schemas.tsand addschemas/directory at package rootresolveSchema()to load from directory structure (user dir → package dir resolution)Implements:
openspec/changes/restructure-schema-directoriesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.