feat(config): add project-level configuration via openspec/config.yaml #499
Conversation
📝 WalkthroughWalkthroughAdds a project-level config system ( Changes
Sequence DiagramssequenceDiagram
participant User
participant CLI
participant ConfigReader as Config Reader
participant SchemaResolver as Schema Resolver
participant SchemaFiles as Schema Files
participant InstructionLoader as Instruction Loader
User->>CLI: Create new change (no --schema)
CLI->>ConfigReader: readProjectConfig(projectRoot)
ConfigReader-->>CLI: config {schema, context, rules}
CLI->>SchemaResolver: resolveSchemaForChange(changeName, undefined)
SchemaResolver->>ConfigReader: consult config.schema
ConfigReader-->>SchemaResolver: schema (if present)
SchemaResolver-->>CLI: resolved schema
CLI->>SchemaFiles: Load schema artifacts & templates
User->>CLI: Generate instructions for artifact
CLI->>InstructionLoader: generateInstructions(context, artifactId, projectRoot)
InstructionLoader->>ConfigReader: readProjectConfig(projectRoot)
ConfigReader-->>InstructionLoader: config
InstructionLoader->>SchemaFiles: Load template for artifact
SchemaFiles-->>InstructionLoader: template
InstructionLoader->>InstructionLoader: Inject <context> (if present)
InstructionLoader->>InstructionLoader: Inject <rules> for artifactId (if present)
InstructionLoader->>InstructionLoader: Append <template>
InstructionLoader-->>CLI: Enriched instructions
CLI-->>User: Display instructions
sequenceDiagram
participant User
participant Workflow as artifact-workflow
participant Prompts as config-prompts
participant SchemaFiles as Available Schemas
participant FS as File System
User->>Workflow: Start artifact workflow
Workflow->>FS: Check openspec/config.yaml exists
alt Config missing
Workflow->>Prompts: promptForConfig()
Prompts->>SchemaFiles: listSchemas()
SchemaFiles-->>Prompts: available schema list
Prompts->>User: select schema & edit context & rules
User-->>Prompts: inputs
Prompts-->>Workflow: ConfigPromptResult
Workflow->>Prompts: serializeConfig()
Prompts-->>Workflow: YAML string
Workflow->>FS: Write openspec/config.yaml
FS-->>Workflow: Success
Workflow->>User: Display config summary
else Config exists
Workflow-->>User: Skip config creation
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@openspec/changes/project-config/specs/config-loading/spec.md`:
- Around line 73-75: The scenario is inaccurate: readProjectConfig
(src/core/project-config.ts) uses z.array(z.string()).safeParse(), so if the
rules array contains any non-string element the validation fails and the entire
artifact's rules are ignored; update the spec scenario to state that WHEN config
contains rules with non-string elements (e.g., ["Valid rule", 123, ""]) THEN the
artifact's rules are discarded and a warning is logged about invalid rules
(rather than suggesting individual elements are filtered), referencing
readProjectConfig behavior.
In `@src/core/artifact-graph/instruction-loader.ts`:
- Around line 259-260: The enrichedTemplate construction in
instruction-loader.ts is wrapping the templateContent in <template> tags
(enrichedTemplate += `<template>\n${templateContent}\n</template>`) which causes
double-wrapping because printInstructionsText in artifact-workflow.ts also wraps
the template when printing; choose one place to wrap: remove the wrapper in
instruction-loader.ts by appending only templateContent to enrichedTemplate
(leave enrichment logic in function that builds enrichedTemplate but do not add
<template> tags), or alternatively modify printInstructionsText to use the
template field as-is (stop adding its own <template> wrapper). Update either the
code that builds enrichedTemplate (symbol: enrichedTemplate / templateContent in
instruction-loader.ts) or the printing function (symbol: printInstructionsText
in artifact-workflow.ts) so templates are wrapped exactly once across the
codebase.
In `@src/core/config-prompts.ts`:
- Around line 1-5: Replace the top-level static imports from '@inquirer/prompts'
and '@inquirer/core' with dynamic imports: create an internal async helper
(e.g., getInquirerPrompts) that uses await import('@inquirer/prompts') to return
{ confirm, select, editor, checkbox, input } and await import('@inquirer/core')
to obtain ExitPromptError, then update promptForConfig to call that helper at
runtime (const { confirm, select, editor, checkbox, input } = await
getInquirerPrompts()) so no restricted modules are imported at module-eval time;
also adjust any re-exports or consumers that expect ExitPromptError to either
import it dynamically or access it via the helper so callers can obtain the
ExitPromptError from the dynamic import.
In `@src/core/project-config.ts`:
- Around line 144-146: The current cast "config as ProjectConfig" can hide a
missing required schema; in readProjectConfig inspect the built partial config
and if config.schema is undefined/absent return null instead of casting; update
the return expression to only return config when config.schema is present (and
keep the return type as ProjectConfig | null) so callers always receive a valid
ProjectConfig with schema defined.
🧹 Nitpick comments (17)
openspec/changes/project-config/design.md (3)
82-95: Add language identifier to code block.The XML example should specify the language for proper syntax highlighting.
♻️ Suggested improvement
Change the opening fence from:
To:
126-130: Add language identifier to code block.The YAML example should specify the language for proper syntax highlighting.
♻️ Suggested improvement
Change the opening fence from:
```To:
```yaml
142-145: Add language identifier to code block.The warning message example should specify "text" as the language for consistency.
♻️ Suggested improvement
Change the opening fence from:
```To:
```textsrc/utils/change-utils.ts (1)
111-124: Schema resolution logic looks correct with proper precedence.The implementation correctly follows the precedence: CLI option → project config → default. However, the
try-catchblock may be unnecessary sincereadProjectConfig(per the relevant snippet atsrc/core/project-config.tslines 55-150) already catches all exceptions internally and returnsnullon failure.♻️ Consider simplifying by removing the try-catch
// Determine schema: explicit option → project config → hardcoded default let schemaName: string; if (options.schema) { schemaName = options.schema; } else { - // Try to read from project config - try { - const config = readProjectConfig(projectRoot); - schemaName = config?.schema ?? DEFAULT_SCHEMA; - } catch { - // If config read fails, use default - schemaName = DEFAULT_SCHEMA; - } + // Read from project config (returns null on missing/invalid config) + const config = readProjectConfig(projectRoot); + schemaName = config?.schema ?? DEFAULT_SCHEMA; }test/core/artifact-graph/instruction-loader.test.ts (2)
454-489: Deduplication test may be fragile due to module-level cache.The test at lines 454-489 attempts to verify warning deduplication, but the
shownWarningsSet is module-level state ininstruction-loader.ts(per the relevant snippet at line 237). This means:
- Warnings from previous tests may pollute the cache
- The assertion
toBeGreaterThanOrEqual(1)is too permissive to verify deduplicationThe fresh temp dir and unique artifact ID help, but the test still depends on execution order. Consider exposing a cache reset function for testing.
♻️ Suggested improvement for more reliable deduplication verification
it('should deduplicate validation warnings within session', () => { // Create a fresh temp directory to avoid cache pollution const freshTempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-test-')); try { // Create project config with a uniquely named invalid artifact ID const configDir = path.join(freshTempDir, 'openspec'); fs.mkdirSync(configDir, { recursive: true }); + const uniqueArtifactId = `unique-invalid-artifact-${Date.now()}`; fs.writeFileSync( path.join(configDir, 'config.yaml'), `schema: spec-driven rules: - unique-invalid-artifact-${Date.now()}: + ${uniqueArtifactId}: - Invalid rule ` ); const context = loadChangeContext(freshTempDir, 'my-change'); + // Clear spy call history before our test + consoleWarnSpy.mockClear(); + // Call multiple times generateInstructions(context, 'proposal', freshTempDir); generateInstructions(context, 'specs', freshTempDir); generateInstructions(context, 'design', freshTempDir); - // Warning should be shown only once (deduplication works) - // Note: We may have gotten warnings from other tests, so check that - // the count didn't increase by more than 1 from the first call const callCount = consoleWarnSpy.mock.calls.filter(call => - call[0]?.includes('Unknown artifact ID in rules') + call[0]?.includes(uniqueArtifactId) ).length; - expect(callCount).toBeGreaterThanOrEqual(1); + // Should be exactly 1 due to deduplication + expect(callCount).toBe(1); } finally { fs.rmSync(freshTempDir, { recursive: true, force: true }); } });
491-510: Test for "no warnings" may be affected by module-level cache state.The test expects
consoleWarnSpynot to have been called, but this only verifies no new warnings were logged during this test. If theshownWarningscache already contains entries from previous tests (e.g., from line 431-452), the absence ofconsole.warncalls doesn't prove the rules are valid—it may just mean warnings were already cached.Consider adding
consoleWarnSpy.mockClear()before the assertion or verifying that warnings for the specific artifact IDs (proposal,specs) were not logged.♻️ Suggested fix
it('should not warn for valid artifact IDs', () => { // Create project config with valid artifact IDs const configDir = path.join(tempDir, 'openspec'); fs.mkdirSync(configDir, { recursive: true }); fs.writeFileSync( path.join(configDir, 'config.yaml'), `schema: spec-driven rules: proposal: - Rule 1 specs: - Rule 2 ` ); const context = loadChangeContext(tempDir, 'my-change'); + consoleWarnSpy.mockClear(); // Clear any warnings from prior tests generateInstructions(context, 'proposal', tempDir); expect(consoleWarnSpy).not.toHaveBeenCalled(); });src/utils/change-metadata.ts (1)
171-181: Hardcoded path traversal assumes fixed directory depth.The derivation
path.resolve(changeDir, '../../..')assumeschangeDiris always exactly atprojectRoot/openspec/changes/<change-name>. This works for the current structure but is fragile if:
- The function is called with a differently-structured path
- The directory layout changes in the future
Consider adding a comment documenting this assumption, or validating the derived path by checking for the existence of
openspec/directory at the computed root.📝 Suggested documentation
// 3. Try reading from project config - // Derive project root from changeDir (changeDir is typically projectRoot/openspec/changes/change-name) + // Derive project root from changeDir. + // ASSUMPTION: changeDir is always at projectRoot/openspec/changes/<change-name> (3 levels deep from project root) const projectRoot = path.resolve(changeDir, '../../..');openspec/changes/project-local-schemas/proposal.md (2)
22-26: Add language specifier to fenced code blocks.Static analysis flagged missing language specifiers. For plain text diagrams, use
textorplaintextto satisfy linters.📝 Suggested fix
-``` +```text 1. ./openspec/schemas/<name>/ # Project-local (NEW) 2. ~/.local/share/openspec/schemas/<name>/ # User global (XDG) 3. <npm-package>/schemas/<name>/ # Package built-in</details> --- `43-53`: **Add language specifier to directory structure block.** Same linter issue as above. Use `text` for the directory tree visualization. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text openspec/ ├── schemas/ # Project-local schemas │ └── my-workflow/ │ ├── schema.yaml # Full schema definition │ └── templates/ │ ├── artifact1.md │ ├── artifact2.md │ └── ... └── changes/</details> </blockquote></details> <details> <summary>src/core/artifact-graph/instruction-loader.ts (1)</summary><blockquote> `10-11`: **Module-level cache may affect test isolation.** The `shownWarnings` Set persists for the process lifetime, which is correct for CLI "per-session" behavior. However, in test suites running multiple test cases in the same process, warnings shown in earlier tests won't appear in later tests, potentially causing flaky or missed assertions. Consider exporting a `clearWarningsCache()` function for test cleanup, or document this behavior for test authors. <details> <summary>📝 Suggested addition for testability</summary> ```diff // Session-level cache for validation warnings (avoid repeating same warnings) const shownWarnings = new Set<string>(); +/** + * Clears the warnings cache. Intended for test cleanup. + * `@internal` + */ +export function _clearWarningsCache(): void { + shownWarnings.clear(); +}openspec/changes/project-config/proposal.md (3)
1-6: Missing required proposal.md sections per coding guidelines.The proposal is well-documented but doesn't follow the required format. Per coding guidelines for
openspec/changes/*/proposal.md, the document should include:
- Why section (currently "Motivation" serves this purpose)
- What Changes section with BREAKING markers where applicable
- Impact section
Consider restructuring to include these sections explicitly. The "Scope" section partially covers "What Changes" but lacks the BREAKING marker convention. Based on learnings, this format ensures consistency across change proposals.
418-426: Use direct file references with line numbers per coding guidelines.The Implementation Notes table references files without line numbers. Per coding guidelines for
openspec/changes/*/{proposal,design,tasks}.md, use direct file references in formatfile.ts:42.For example:
src/core/project-config.ts→project-config.ts:56(forreadProjectConfig)src/core/artifact-graph/instruction-loader.ts→instruction-loader.ts:XX(specific line)
24-40: Add language specifier to fenced code block.This ASCII diagram code block lacks a language specifier. While not critical, adding a specifier (e.g.,
textorplaintext) improves markdown linting compliance.Suggested fix
-``` +```text ┌─────────────────────────────────────────────────────────────────┐src/core/config-prompts.ts (1)
130-137: Remove unnecessary validate function.The
validatecallback always returnstrue, making it redundant. Removing it simplifies the code.Proposed fix
const rule = await input({ message: '│', - validate: (value) => { - // Empty string is valid (signals end of input) - return true; - }, });test/core/project-config.test.ts (1)
564-578: Clarify the assertion logic for suggestion limit test.The comment says "Should suggest at most 3" but the assertion allows up to 8 matches (
3 + 5). This is because the same schema names appear in both the "Did you mean" suggestions and the "Available schemas" list. Consider adding a clarifying comment or restructuring the test.Suggested improvement
it('should limit suggestions to top 3 matches', () => { const manySchemas = [ { name: 'test-a', isBuiltIn: true }, { name: 'test-b', isBuiltIn: true }, { name: 'test-c', isBuiltIn: true }, { name: 'test-d', isBuiltIn: true }, { name: 'test-e', isBuiltIn: true }, ]; const message = suggestSchemas('test', manySchemas); - // Should suggest at most 3 - const suggestionCount = (message.match(/test-/g) || []).length; - expect(suggestionCount).toBeGreaterThanOrEqual(3); - expect(suggestionCount).toBeLessThanOrEqual(3 + 5); // 3 in suggestions + 5 in "Available" list + // "Did you mean" section should have at most 3 suggestions + // Total matches include both suggestions (up to 3) and "Available schemas" list (all 5) + const didYouMeanSection = message.split('Available schemas:')[0]; + const suggestionMatches = (didYouMeanSection.match(/test-[a-e]/g) || []).length; + expect(suggestionMatches).toBeLessThanOrEqual(3); });src/core/project-config.ts (2)
106-108: Remove unused variablerulesField.The
rulesFieldschema is defined but never used. The actual validation uses inlinez.array(z.string()).safeParse(rules)on line 116.Proposed fix
// Parse rules field using Zod if (raw.rules !== undefined) { - const rulesField = z.record(z.string(), z.array(z.string())); - // First check if it's an object structure if (typeof raw.rules === 'object' && !Array.isArray(raw.rules)) {
195-218: Remove duplicate Levenshtein implementation and import from existing utility.A
levenshteinfunction already exists insrc/utils/match.tsand is exported. The duplicate implementation at lines 195-218 should be replaced with an import from the utility to avoid code duplication.Suggested fix
+import { levenshtein } from '../utils/match.js'; + export function suggestSchemas( invalidSchemaName: string, availableSchemas: { name: string; isBuiltIn: boolean }[] ): string { - // Simple fuzzy match: Levenshtein distance - function levenshtein(a: string, b: string): number { - const matrix: number[][] = []; - for (let i = 0; i <= b.length; i++) { - matrix[i] = [i]; - } - for (let j = 0; j <= a.length; j++) { - matrix[0][j] = j; - } - for (let i = 1; i <= b.length; i++) { - for (let j = 1; j <= a.length; j++) { - if (b.charAt(i - 1) === a.charAt(j - 1)) { - matrix[i][j] = matrix[i - 1][j - 1]; - } else { - matrix[i][j] = Math.min( - matrix[i - 1][j - 1] + 1, - matrix[i][j - 1] + 1, - matrix[i - 1][j] + 1 - ); - } - } - } - return matrix[b.length][a.length]; - } - // Find closest matches (distance <= 3) const suggestions = availableSchemas
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
openspec/changes/project-config/.openspec.yamlopenspec/changes/project-config/design.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/context-injection/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/tasks.mdopenspec/changes/project-local-schemas/.openspec.yamlopenspec/changes/project-local-schemas/proposal.mdsrc/commands/artifact-workflow.tssrc/core/artifact-graph/instruction-loader.tssrc/core/config-prompts.tssrc/core/project-config.tssrc/utils/change-metadata.tssrc/utils/change-utils.tstest/core/artifact-graph/instruction-loader.test.tstest/core/project-config.test.tstest/utils/change-metadata.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
openspec/changes/*/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/*/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files with at least one#### Scenario:per requirement
Format scenarios with exactly four hashtags (#### Scenario:) followed by bullet points with**WHEN**and**THEN**syntax
In MODIFIED Requirements, copy the entire existing requirement block and paste under## MODIFIED Requirements, then edit to reflect new behavior
Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
In spec deltas, include a**Reason**field for REMOVED requirements and a**Migration**field explaining how to handle the removal
Files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
openspec/**/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/**/spec.md: Use SHALL/MUST for normative requirements in specs; avoid should/may unless intentionally non-normative
Ensure every requirement in spec files has at least one#### Scenario:with WHEN/THEN structure
Files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Create
proposal.mdwith Why, What Changes (with BREAKING markers), and Impact sections
Files:
openspec/changes/project-config/proposal.mdopenspec/changes/project-local-schemas/proposal.md
openspec/changes/*/{proposal,design,tasks}.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use direct file references in format
file.ts:42and reference specs asspecs/auth/spec.mdfor clarity
Files:
openspec/changes/project-config/proposal.mdopenspec/changes/project-config/tasks.mdopenspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/design.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Create
tasks.mdwith numbered implementation sections and checkboxes in format- [ ] X.Y Task description
Files:
openspec/changes/project-config/tasks.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/.openspec.yamlopenspec/changes/project-local-schemas/.openspec.yamlopenspec/changes/project-config/tasks.mdopenspec/changes/project-config/specs/context-injection/spec.mdopenspec/changes/project-config/design.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files with at least one `#### Scenario:` per requirement
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/.openspec.yamlopenspec/changes/project-local-schemas/.openspec.yamlopenspec/changes/project-config/specs/context-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/.openspec.yamlopenspec/changes/project-local-schemas/.openspec.yamlopenspec/changes/project-config/tasks.mdopenspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/design.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Read relevant specs in `specs/[capability]/spec.md` and check pending changes in `changes/` for conflicts before starting any task
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : In spec deltas, include a `**Reason**` field for REMOVED requirements and a `**Migration**` field explaining how to handle the removal
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/proposal.md : Create `proposal.md` with Why, What Changes (with **BREAKING** markers), and Impact sections
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/{proposal,design,tasks}.md : Use direct file references in format `file.ts:42` and reference specs as `specs/auth/spec.md` for clarity
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/tasks.mdopenspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/design.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : In MODIFIED Requirements, copy the entire existing requirement block and paste under `## MODIFIED Requirements`, then edit to reflect new behavior
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.md
📚 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:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/.openspec.yaml
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/**/**/spec.md : Ensure every requirement in spec files has at least one `#### Scenario:` with WHEN/THEN structure
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Format scenarios with exactly four hashtags (`#### Scenario:`) followed by bullet points with `**WHEN**` and `**THEN**` syntax
Applied to files:
openspec/changes/project-config/specs/rules-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Run `openspec validate [change] --strict --no-interactive` before sharing a proposal to resolve validation issues
Applied to files:
openspec/changes/project-config/specs/schema-resolution/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: After deployment, move `changes/[name]/` to `changes/archive/YYYY-MM-DD-[name]/` using `openspec archive <change-id> [--skip-specs] [--yes]`
Applied to files:
openspec/changes/project-config/.openspec.yamlopenspec/changes/project-local-schemas/.openspec.yaml
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Use `openspec show [spec] --json` and `openspec show [change] --json --deltas-only` for machine-readable inspection of specs and changes
Applied to files:
openspec/changes/project-config/.openspec.yaml
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/tasks.md : Create `tasks.md` with numbered implementation sections and checkboxes in format `- [ ] X.Y Task description`
Applied to files:
openspec/changes/project-config/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Complete all tasks sequentially from `tasks.md` and update every task to `- [x]` before marking proposal as complete
Applied to files:
openspec/changes/project-config/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Create `design.md` only when the change is cross-cutting, involves new dependencies, has security/performance complexity, or resolves ambiguity before coding
Applied to files:
openspec/changes/project-config/tasks.md
🧬 Code graph analysis (9)
test/utils/change-metadata.test.ts (2)
src/utils/change-metadata.ts (1)
resolveSchemaForChange(152-185)src/utils/index.ts (1)
resolveSchemaForChange(9-9)
src/core/artifact-graph/instruction-loader.ts (1)
src/core/project-config.ts (2)
readProjectConfig(56-151)validateConfigRules(163-181)
src/utils/change-metadata.ts (1)
src/core/project-config.ts (1)
readProjectConfig(56-151)
test/core/project-config.test.ts (1)
src/core/project-config.ts (3)
readProjectConfig(56-151)validateConfigRules(163-181)suggestSchemas(191-254)
src/commands/artifact-workflow.ts (2)
src/core/artifact-graph/instruction-loader.ts (1)
generateInstructions(199-274)src/core/config-prompts.ts (3)
promptForConfig(28-115)serializeConfig(158-179)ExitPromptError(181-181)
src/utils/change-utils.ts (1)
src/core/project-config.ts (1)
readProjectConfig(56-151)
src/core/project-config.ts (1)
src/utils/match.ts (1)
levenshtein(7-24)
test/core/artifact-graph/instruction-loader.test.ts (1)
src/core/artifact-graph/instruction-loader.ts (2)
loadChangeContext(162-183)generateInstructions(199-274)
src/core/config-prompts.ts (1)
src/core/project-config.ts (1)
ProjectConfig(43-43)
🪛 GitHub Actions: CI
src/core/config-prompts.ts
[error] 1-1: ESLint: '@inquirer/prompts' import is restricted from being used by a pattern. Use dynamic import() for @inquirer modules to prevent pre-commit hook hangs. See #367
🪛 GitHub Check: Lint & Type Check
src/core/config-prompts.ts
[failure] 2-2:
'@inquirer/core' import is restricted from being used by a pattern. Use dynamic import() for @inquirer modules to prevent pre-commit hook hangs. See #367
[failure] 1-1:
'@inquirer/prompts' import is restricted from being used by a pattern. Use dynamic import() for @inquirer modules to prevent pre-commit hook hangs. See #367
🪛 LanguageTool
openspec/changes/project-config/proposal.md
[style] ~85-~85: To form a complete sentence, be sure to include a subject.
Context: ...equired) Which workflow schema to use. Can be: - Built-in name: spec-driven, `td...
(MISSING_IT_THERE)
openspec/changes/project-config/specs/context-injection/spec.md
[uncategorized] ~50-~50: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ns markdown - WHEN context includes markdown formatting like **bold** or `[links](...
(MARKDOWN_NNP)
openspec/changes/project-config/design.md
[uncategorized] ~66-~66: The official name of this software platform is spelled with a capital “H”.
Context: ...rs structure used by other tools (e.g., .github/) Alternatives considered: - `./o...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
openspec/changes/project-config/proposal.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
339-339: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
375-375: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
436-436: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
457-457: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
731-731: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/project-local-schemas/proposal.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/project-config/specs/context-injection/spec.md
46-46: Bare URL used
(MD034, no-bare-urls)
openspec/changes/project-config/design.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
339-339: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
375-375: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
436-436: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
457-457: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (31)
openspec/changes/project-local-schemas/.openspec.yaml (1)
1-2: LGTM!The change metadata is well-formed with appropriate schema and timestamp fields.
openspec/changes/project-config/specs/schema-resolution/spec.md (1)
1-83: LGTM!The specification is well-structured and follows all coding guidelines:
- Proper use of "## ADDED Requirements" header
- SHALL statements for normative requirements
- Each requirement has multiple scenarios with WHEN/THEN format
- Comprehensive coverage including error handling and backwards compatibility
openspec/changes/project-config/tasks.md (1)
1-72: LGTM!The tasks document follows the required format with numbered sections and checkbox format. The task breakdown is comprehensive and well-organized across the 6 implementation phases.
openspec/changes/project-config/design.md (1)
1-660: Excellent design documentation!The design document is comprehensive and well-structured, covering:
- Clear context and problem statement
- Well-defined goals and non-goals
- Thoughtful decision rationale for key choices
- Detailed implementation plan with code examples
- Performance considerations and benchmarking strategy
- Risk analysis and mitigation strategies
- Backwards compatibility guarantees
The document provides strong guidance for implementation and future maintenance.
openspec/changes/project-config/.openspec.yaml (1)
1-2: LGTM!The change metadata is properly formatted with appropriate schema and timestamp values.
openspec/changes/project-config/specs/config-loading/spec.md (1)
1-119: Well-structured spec following conventions.The spec correctly uses
## ADDED Requirementsheaders,#### Scenario:entries with WHEN/THEN format, and SHALL for normative requirements as per coding guidelines. The requirements comprehensively cover the config loading behavior.openspec/changes/project-config/specs/context-injection/spec.md (2)
45-51: Static analysis hints are false positives for example content.The bare URL at line 46 and lowercase "markdown" at line 50 appear in scenario descriptions illustrating what user-provided context might contain. These are examples of input data, not actual documentation content requiring links or capitalization fixes.
1-51: Spec follows conventions and is well-structured.The context injection spec correctly uses
## ADDED Requirementsheader,#### Scenario:entries with WHEN/THEN format, and SHALL for normative requirements. The scenarios comprehensively cover context injection behavior including edge cases for special characters, URLs, and markdown content.test/utils/change-metadata.test.ts (1)
212-288: Comprehensive tests for schema resolution precedence.The new tests thoroughly verify the resolution order (CLI > metadata > config > default) with good coverage including:
- Individual precedence level tests
- Combined precedence verification by progressively removing config sources
- Proper setup/teardown with temp directories
test/core/artifact-graph/instruction-loader.test.ts (1)
199-418: Comprehensive test coverage for project config integration.The new test suite thoroughly covers:
- Context injection for all artifacts
- Multi-line and special character preservation
- Artifact-specific rules injection
- Correct ordering (context → rules → template)
- Edge cases (empty rules, context without rules, rules without context)
- Behavior without projectRoot parameter
src/utils/change-metadata.ts (1)
6-6: LGTM!Import addition for
readProjectConfigis correctly placed and aligns with the new schema resolution logic.openspec/changes/project-config/specs/rules-injection/spec.md (1)
1-99: Well-structured spec following all coding guidelines.The spec correctly uses:
## ADDED Requirementsheader for new capabilities#### Scenario:format with WHEN/THEN bullet syntax- Comprehensive scenario coverage for edge cases (empty rules, undefined rules, unknown artifact IDs)
The requirements clearly specify the injection order (context → rules → template) and per-session warning deduplication, which aligns with the implementation in
instruction-loader.ts.openspec/changes/project-local-schemas/proposal.md (1)
1-167: Well-structured proposal with clear rationale and scope.The proposal follows guidelines with Why (Motivation), What Changes (Design Decisions, Scope), and Impact (Testing Considerations) sections. The fork model decision is well-justified, and the explicit out-of-scope items prevent scope creep.
src/commands/artifact-workflow.ts (4)
33-34: LGTM!Import additions for the new config prompts and project config modules are correctly placed.
287-287: LGTM!Correctly passes
projectRoottogenerateInstructionsto enable config-driven context and rules injection.
904-915: LGTM!Good defensive check for existing config before prompting. Checking both
.yamland.ymlextensions matches the behavior inreadProjectConfig.
918-1000: Solid error handling with graceful degradation.The try-catch structure correctly handles:
- User cancellation (
ExitPromptError) with a friendly message- Write failures with a fallback to display the YAML content
- User opting out of config creation
One minor observation: Lines 970-979 output YAML to stderr as a fallback, which is appropriate for error recovery but could be verbose. This is acceptable given it only triggers on write failure.
src/core/artifact-graph/instruction-loader.ts (3)
7-7: LGTM!Imports correctly added for the new project config integration.
217-224: LGTM!Safe config reading with appropriate fallback. The try-catch is defensive since
readProjectConfigcould theoretically throw on I/O errors beyond just missing files.
244-257: LGTM!Context and rules injection follows the spec correctly:
- Context wrapped in
<context>tags (all artifacts)- Rules wrapped in
<rules>tags with bullet formatting (only matching artifact)- Proper newline separation between sections
openspec/changes/project-config/proposal.md (1)
703-727: LGTM! Well-documented Zod schema with clear comments.The TypeScript schema in the appendix is well-documented and aligns with the implementation in
src/core/project-config.ts. The inline comments clearly explain each field's purpose and constraints.src/core/config-prompts.ts (3)
28-54: LGTM! Well-structured prompt flow with proper error handling.The
promptForConfigfunction has a clear flow:
- Ask for confirmation
- Get available schemas and handle empty case
- Prompt for schema selection with descriptive choices
The early return on
!shouldCreateand error throw for empty schemas are appropriate.
158-179: LGTM! Clean serialization with proper YAML formatting.The
serializeConfigfunction correctly:
- Only includes defined fields (avoids
undefinedin output)- Uses appropriate YAML options (
lineWidth: 0for no wrapping,PLAINstring types)- Handles optional context and rules properly
181-181: Re-exporting ExitPromptError may not work with dynamic imports.If the import is converted to dynamic, this re-export will need adjustment. Consider either:
- Exporting a function that returns the error class
- Documenting that consumers should dynamically import it themselves
test/core/project-config.test.ts (4)
11-23: LGTM! Proper test setup and teardown.Good practices:
- Creates isolated temp directories per test
- Properly mocks and restores
console.warn- Cleans up temp directories with
recursive: true, force: true
25-56: LGTM! Comprehensive test for complete config parsing.Validates that a fully-specified config with schema, multi-line context, and per-artifact rules is parsed correctly without warnings.
265-341: LGTM! Thorough context size limit testing.Excellent coverage of boundary conditions:
- Under limit (1KB)
- Over limit (51KB)
- Exactly at limit (50KB)
- Multi-byte UTF-8 characters (snowman = 3 bytes each)
The UTF-8 test is particularly valuable for ensuring byte-based size calculation works correctly.
461-512: LGTM! Comprehensive validateConfigRules tests.Tests cover:
- Valid artifact IDs (no warnings)
- Unknown artifact IDs (warnings with valid IDs listed)
- Multiple unknown IDs
- Empty rules object
src/core/project-config.ts (3)
19-43: LGTM! Well-documented Zod schema.The schema is clearly documented with:
- Purpose explanation for using Zod
- Descriptive comments for each field
- Proper use of
.describe()for self-documentation
56-64: LGTM! Good file extension precedence logic.Correctly prefers
.yamlover.ymlwhen both exist, and gracefully returnsnullwhen neither exists.
220-253: LGTM! Clear and helpful schema suggestion message.The
suggestSchemasfunction provides excellent UX:
- Shows fuzzy-matched suggestions (distance ≤ 3)
- Lists all available schemas (built-in vs project-local)
- Provides actionable fix instruction
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| // Return partial config even if some fields failed | ||
| return Object.keys(config).length > 0 ? (config as ProjectConfig) : null; |
There was a problem hiding this comment.
Type cast may allow invalid ProjectConfig without required schema field.
The return config as ProjectConfig casts a Partial<ProjectConfig> that may not have the required schema field. This could cause runtime issues for consumers expecting schema to always be present.
Consider either:
- Making
schemaoptional in the type when returned fromreadProjectConfig - Returning
nullwhenschemais missing (since it's documented as required)
Option 2: Return null when schema is missing
// Return partial config even if some fields failed
- return Object.keys(config).length > 0 ? (config as ProjectConfig) : null;
+ // Schema is required - if missing, treat as invalid config
+ if (!config.schema) {
+ return null;
+ }
+ return config as ProjectConfig;📝 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.
| // Return partial config even if some fields failed | |
| return Object.keys(config).length > 0 ? (config as ProjectConfig) : null; | |
| // Return partial config even if some fields failed | |
| // Schema is required - if missing, treat as invalid config | |
| if (!config.schema) { | |
| return null; | |
| } | |
| return config as ProjectConfig; |
🤖 Prompt for AI Agents
In `@src/core/project-config.ts` around lines 144 - 146, The current cast "config
as ProjectConfig" can hide a missing required schema; in readProjectConfig
inspect the built partial config and if config.schema is undefined/absent return
null instead of casting; update the return expression to only return config when
config.schema is present (and keep the return type as ProjectConfig | null) so
callers always receive a valid ProjectConfig with schema defined.
Adds openspec/config.yaml support for project-level customization without forking schemas. Teams can now: - Set a default schema (used when --schema flag not provided) - Inject project context into all artifact instructions - Add per-artifact rules (e.g., proposal rules, specs rules) Key changes: - New `src/core/project-config.ts` with Zod schema and resilient parsing - New `src/core/config-prompts.ts` for interactive config creation - Updated schema resolution order: CLI → change metadata → config → default - Updated instruction generation to inject <context> and <rules> XML sections - Integrated config creation prompts into `artifact-experimental-setup` Schema resolution precedence: 1. --schema CLI flag (explicit override) 2. .openspec.yaml in change directory (change-specific) 3. openspec/config.yaml schema field (project default) 4. "spec-driven" (hardcoded fallback)
TabishB
force-pushed
the
TabishB/schema-custom-audit
branch
from
3e6a639 to
05efd62
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@openspec/changes/project-config/specs/context-injection/spec.md`:
- Around line 49-51: Update the scenario text "Scenario: Context contains
markdown" and its bullet points to capitalize "Markdown" everywhere (e.g.,
change `markdown` to `Markdown` in the scenario title and in the sentence
"context includes markdown formatting like `**bold**` or `[links](url)`") so the
proper noun is consistently capitalized.
In `@openspec/changes/project-config/specs/rules-injection/spec.md`:
- Around line 89-91: The spec expects the warning to use single quotes around
the artifact id but the actual warning emitted by the project-config code (the
log that builds the "Unknown artifact ID in rules" message using the artifactId
variable) uses double quotes; update the rules-injection spec's expected message
to match the implementation (double-quoted artifact id) so tests and
implementation are consistent.
In `@src/commands/artifact-workflow.ts`:
- Around line 33-34: Remove the unused import readProjectConfig from the import
list at the top of the file; keep the remaining imports (promptForConfig,
serializeConfig, isExitPromptError) intact since config reading is performed
elsewhere (generateInstructions in instruction-loader.ts), so simply delete
readProjectConfig from the import statement to eliminate the unused symbol.
In `@src/core/config-prompts.ts`:
- Around line 75-79: In the editor call that sets up contextInput (the const
contextInput = await editor({...}) invocation), fix the typo in the options
object by renaming waitForUseInput to the correct option name waitForUserInput:
update the property key inside that editor(...) call so the option is honored.
♻️ Duplicate comments (3)
openspec/changes/project-config/specs/config-loading/spec.md (1)
73-75: Scenario does not accurately reflect implementation behavior.This issue was previously identified: when rules array contains non-string elements like
123,z.array(z.string()).safeParse()fails entirely, causing the whole artifact's rules to be ignored rather than filtering individual valid elements.📝 Suggested fix
#### 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 +- **THEN** warning is logged and proposal rules are ignored entirely (array validation fails due to non-string element)src/core/artifact-graph/instruction-loader.ts (1)
259-260: Double-wrapping of template content in output.The enriched template now always wraps content in
<template>tags (Line 260). However,printInstructionsTextinartifact-workflow.ts(Lines 371-373) also wraps the template field with<template>tags when printing.This results in double-wrapped output:
<template> <template> ...actual template... </template> </template>Either remove the wrapper here (letting the caller handle formatting), or update
printInstructionsTextto not add its own wrapper.🔧 Option 1: Remove wrapper from enrichedTemplate
// 3. Add original template - enrichedTemplate += `<template>\n${templateContent}\n</template>`; + enrichedTemplate += templateContent;This preserves backward compatibility with existing callers that expect raw template content.
🔧 Option 2: Update printInstructionsText to use template directly
In
src/commands/artifact-workflow.ts:- // Template - console.log('<template>'); - console.log(template.trim()); - console.log('</template>'); + // Template (already wrapped with context/rules if present) + console.log(template.trim());This would require ensuring all code paths produce consistently formatted templates.
src/core/project-config.ts (1)
145-146: Type cast may allow invalidProjectConfigwithout requiredschemafield.The cast
config as ProjectConfigon line 146 can return a config without the requiredschemafield, violating the type contract. The Zod schema definesschemaas required (non-optional), but the partial parsing approach may skip it if invalid.Recommended fix: Return null when schema is missing
// Return partial config even if some fields failed - return Object.keys(config).length > 0 ? (config as ProjectConfig) : null; + // Schema is required - if missing, treat as invalid config + if (!config.schema) { + return null; + } + return config as ProjectConfig;
🧹 Nitpick comments (8)
openspec/changes/project-config/proposal.md (1)
24-40: Consider adding language specifiers to ASCII art code blocks.Static analysis flags missing language specifiers on fenced code blocks. For ASCII diagrams, you can use
```textor```plaintextto satisfy markdown linting while preserving readability.openspec/changes/project-local-schemas/proposal.md (2)
62-77: Consider adding an explicit Impact section.Per coding guidelines, proposals should include an Impact section. While the benefits are mentioned in Motivation, an explicit Impact section would clarify effects on existing users, migration considerations, and how this change interacts with the project-config change.
22-26: Add language specifier to code block.Use
```textfor the resolution order list to satisfy markdown linting.test/core/artifact-graph/instruction-loader.test.ts (1)
481-485: Deduplication assertion is too weak to verify the intended behavior.The assertion
expect(callCount).toBeGreaterThanOrEqual(1)only confirms that at least one warning was emitted, but doesn't actually verify that warnings were deduplicated (i.e., shown exactly once). Since the test callsgenerateInstructionsthree times, a stronger assertion would be:- expect(callCount).toBeGreaterThanOrEqual(1); + expect(callCount).toBe(1);This would confirm that the same warning isn't repeated across the three calls.
src/core/artifact-graph/instruction-loader.ts (1)
10-11: Module-level cache persists for entire process lifetime.The
shownWarningsSet is module-level state that never gets cleared. This is fine for single CLI invocations but could cause issues:
- In long-running processes or watch modes, warnings would only show once ever
- In test suites, the cache persists across tests (the tests work around this with unique IDs)
Consider either:
- Documenting this as intentional per-process behavior
- Adding an internal reset function for testing
- Making it instance-level if multiple independent contexts are needed
For a CLI tool, this is likely acceptable behavior, but worth documenting.
openspec/changes/project-config/design.md (1)
151-289: Design document implementation snippet may diverge from actual code.The implementation snippet in Phase 1 (lines 151-289) shows
readProjectConfig()callingfindProjectRoot()internally, but the actual implementation insrc/core/project-config.tsacceptsprojectRoot: stringas a parameter. This is a better design (caller provides root), but the design doc should be updated to match.Also, per coding guidelines for
openspec/changes/*/*.mdfiles: use direct file references likeproject-config.ts:56for clarity when referencing implementation.src/core/project-config.ts (2)
195-218: Duplicate Levenshtein implementation - reuse existing utility.A
levenshteinfunction already exists insrc/utils/match.ts:6-23. Consider importing it instead of reimplementing:♻️ Proposed refactor
import { existsSync, readFileSync, statSync } from 'fs'; import path from 'path'; import { parse as parseYaml } from 'yaml'; import { z } from 'zod'; +import { levenshtein } from '../utils/match.js'; // ... later in suggestSchemas: export function suggestSchemas( invalidSchemaName: string, availableSchemas: { name: string; isBuiltIn: boolean }[] ): string { - // Simple fuzzy match: Levenshtein distance - function levenshtein(a: string, b: string): number { - const matrix: number[][] = []; - for (let i = 0; i <= b.length; i++) { - matrix[i] = [i]; - } - for (let j = 0; j <= a.length; j++) { - matrix[0][j] = j; - } - for (let i = 1; i <= b.length; i++) { - for (let j = 1; j <= a.length; j++) { - if (b.charAt(i - 1) === a.charAt(j - 1)) { - matrix[i][j] = matrix[i - 1][j - 1]; - } else { - matrix[i][j] = Math.min( - matrix[i - 1][j - 1] + 1, - matrix[i][j - 1] + 1, - matrix[i - 1][j] + 1 - ); - } - } - } - return matrix[b.length][a.length]; - } - // Find closest matches (distance <= 3) const suggestions = availableSchemas
1-1: Unused import:statSync.
statSyncis imported but never used in this file. It appears to be a leftover from the caching strategy mentioned in the design doc (Phase 4) which wasn't implemented.♻️ Remove unused import
-import { existsSync, readFileSync, statSync } from 'fs'; +import { existsSync, readFileSync } from 'fs';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
openspec/changes/project-config/.openspec.yamlopenspec/changes/project-config/design.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/context-injection/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/schema-resolution/spec.mdopenspec/changes/project-config/tasks.mdopenspec/changes/project-local-schemas/.openspec.yamlopenspec/changes/project-local-schemas/proposal.mdsrc/commands/artifact-workflow.tssrc/core/artifact-graph/instruction-loader.tssrc/core/config-prompts.tssrc/core/project-config.tssrc/utils/change-metadata.tssrc/utils/change-utils.tstest/core/artifact-graph/instruction-loader.test.tstest/core/project-config.test.tstest/utils/change-metadata.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- openspec/changes/project-local-schemas/.openspec.yaml
- openspec/changes/project-config/.openspec.yaml
- src/utils/change-utils.ts
- test/core/project-config.test.ts
- openspec/changes/project-config/tasks.md
- openspec/changes/project-config/specs/schema-resolution/spec.md
🧰 Additional context used
📓 Path-based instructions (4)
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Create
proposal.mdwith Why, What Changes (with BREAKING markers), and Impact sections
Files:
openspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/proposal.md
openspec/changes/*/{proposal,design,tasks}.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use direct file references in format
file.ts:42and reference specs asspecs/auth/spec.mdfor clarity
Files:
openspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/design.mdopenspec/changes/project-config/proposal.md
openspec/changes/*/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/*/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files with at least one#### Scenario:per requirement
Format scenarios with exactly four hashtags (#### Scenario:) followed by bullet points with**WHEN**and**THEN**syntax
In MODIFIED Requirements, copy the entire existing requirement block and paste under## MODIFIED Requirements, then edit to reflect new behavior
Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
In spec deltas, include a**Reason**field for REMOVED requirements and a**Migration**field explaining how to handle the removal
Files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
openspec/**/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/**/spec.md: Use SHALL/MUST for normative requirements in specs; avoid should/may unless intentionally non-normative
Ensure every requirement in spec files has at least one#### Scenario:with WHEN/THEN structure
Files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
Applied to files:
openspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/design.mdopenspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/{proposal,design,tasks}.md : Use direct file references in format `file.ts:42` and reference specs as `specs/auth/spec.md` for clarity
Applied to files:
openspec/changes/project-local-schemas/proposal.mdopenspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Create `design.md` only when the change is cross-cutting, involves new dependencies, has security/performance complexity, or resolves ambiguity before coding
Applied to files:
openspec/changes/project-config/design.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/proposal.md : Create `proposal.md` with Why, What Changes (with **BREAKING** markers), and Impact sections
Applied to files:
openspec/changes/project-config/design.mdopenspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/context-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files with at least one `#### Scenario:` per requirement
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : In spec deltas, include a `**Reason**` field for REMOVED requirements and a `**Migration**` field explaining how to handle the removal
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Read relevant specs in `specs/[capability]/spec.md` and check pending changes in `changes/` for conflicts before starting any task
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : In MODIFIED Requirements, copy the entire existing requirement block and paste under `## MODIFIED Requirements`, then edit to reflect new behavior
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.mdopenspec/changes/project-config/specs/context-injection/spec.md
📚 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:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/proposal.mdopenspec/changes/project-config/specs/context-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Format scenarios with exactly four hashtags (`#### Scenario:`) followed by bullet points with `**WHEN**` and `**THEN**` syntax
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/**/**/spec.md : Ensure every requirement in spec files has at least one `#### Scenario:` with WHEN/THEN structure
Applied to files:
openspec/changes/project-config/specs/config-loading/spec.mdopenspec/changes/project-config/specs/rules-injection/spec.md
🧬 Code graph analysis (6)
src/commands/artifact-workflow.ts (4)
src/core/artifact-graph/instruction-loader.ts (1)
generateInstructions(199-274)src/core/artifact-graph/index.ts (1)
generateInstructions(36-36)src/core/config-prompts.ts (2)
promptForConfig(39-129)serializeConfig(175-196)src/utils/file-system.ts (1)
FileSystemUtils(44-252)
src/core/project-config.ts (1)
src/utils/match.ts (1)
levenshtein(7-24)
src/core/config-prompts.ts (1)
src/core/project-config.ts (1)
ProjectConfig(43-43)
src/utils/change-metadata.ts (1)
src/core/project-config.ts (1)
readProjectConfig(56-151)
test/utils/change-metadata.test.ts (2)
src/utils/change-metadata.ts (1)
resolveSchemaForChange(152-185)src/utils/index.ts (1)
resolveSchemaForChange(9-9)
src/core/artifact-graph/instruction-loader.ts (1)
src/core/project-config.ts (2)
readProjectConfig(56-151)validateConfigRules(163-181)
🪛 LanguageTool
openspec/changes/project-config/design.md
[uncategorized] ~66-~66: The official name of this software platform is spelled with a capital “H”.
Context: ...rs structure used by other tools (e.g., .github/) Alternatives considered: - `./o...
(GITHUB)
openspec/changes/project-config/proposal.md
[style] ~85-~85: To form a complete sentence, be sure to include a subject.
Context: ...equired) Which workflow schema to use. Can be: - Built-in name: spec-driven, `td...
(MISSING_IT_THERE)
openspec/changes/project-config/specs/context-injection/spec.md
[uncategorized] ~50-~50: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ns markdown - WHEN context includes markdown formatting like **bold** or `[links](...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.18.1)
openspec/changes/project-local-schemas/proposal.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/project-config/design.md
46-46: Bare URL used
(MD034, no-bare-urls)
openspec/changes/project-config/proposal.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
339-339: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
375-375: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
436-436: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
457-457: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
731-731: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (26)
openspec/changes/project-config/proposal.md (1)
1-6: Well-structured proposal with comprehensive documentation.The proposal follows the required structure with Why (Motivation), What Changes (Design Decisions, Scope), and discusses Impact throughout. The two-path model (configure vs. fork) is clearly articulated, and the UX flows are detailed.
test/utils/change-metadata.test.ts (2)
213-225: Good test coverage for project config integration.This test correctly validates that when change metadata is absent, the schema resolution falls back to project config.
264-288: Excellent precedence order test with progressive verification.This test methodically verifies each level of the precedence chain by removing files and confirming the next level takes over. This provides strong confidence in the resolution logic.
openspec/changes/project-local-schemas/proposal.md (1)
1-6: Clear and focused proposal for project-local schemas.The proposal clearly articulates the 3-level resolution order and the fork model design decision. The rationale for not supporting inheritance is well-justified.
openspec/changes/project-config/specs/config-loading/spec.md (2)
1-24: Well-structured spec with proper ADDED Requirements header.The spec correctly uses SHALL for normative requirements and follows the
#### Scenario:format with WHEN/THEN bullet points. Scenarios for config file existence, invalid YAML, and schema validation are comprehensive.
81-96: Context size limit scenarios are well-defined.The scenarios correctly specify the 50KB limit and the behavior at boundary conditions (within, at, exceeds). This aligns with the
MAX_CONTEXT_SIZEconstant in the implementation.src/utils/change-metadata.ts (3)
6-6: LGTM on the import addition.The import of
readProjectConfigfrom the new project-config module is correctly placed and used.
145-146: Documentation updated to reflect new resolution order.The JSDoc correctly documents the 4-level precedence: explicit schema → metadata → project config → default.
171-181: Remove reference to non-existent findProjectRoot utility.The path derivation
path.resolve(changeDir, '../../..')correctly assumes the standard structure wherechangeDirisprojectRoot/openspec/changes/change-name. This is consistently used across the codebase and enforced by change creation logic. The comment on line 172 documents this assumption clearly, and the implementation includes proper error handling via try/catch. The referencedfindProjectRoot()utility does not exist in the project, so that suggestion should be removed. The current approach is appropriate for the standardized directory structure.test/core/artifact-graph/instruction-loader.test.ts (2)
199-418: Comprehensive test coverage for project config integration.The test suite thoroughly covers context injection, rules injection, ordering (
context -> rules -> template), edge cases (empty rules, missing config, special characters), and the interaction between context and rules. Good job covering all the scenarios specified in the specs.
420-511: Validation and warnings tests are well-structured.The tests properly use
vi.spyOnto mockconsole.warnand verify warning behavior. The cleanup inafterEachensures mocks are restored between tests.openspec/changes/project-config/specs/context-injection/spec.md (1)
1-51: Spec structure follows guidelines correctly.The spec uses
## ADDED Requirementsheader, includes#### Scenario:with**WHEN**/**THEN**syntax for each requirement, and uses SHALL for normative statements. Good coverage of context injection behavior including multi-line, special characters, URLs, and Markdown content preservation.openspec/changes/project-config/specs/rules-injection/spec.md (1)
1-99: Comprehensive rules injection specification.The spec thoroughly covers rule matching, formatting with XML tags, text preservation, multi-artifact support, additive behavior with schema guidance, and validation with deduplication. Follows the required format with
## ADDED Requirements,#### Scenario:headers, and**WHEN**/**THEN**syntax.src/commands/artifact-workflow.ts (3)
903-906: Config existence check covers both extensions.Good defensive check for both
config.yamlandconfig.ymlto avoid overwriting an existing configuration regardless of extension used.
968-980: Good fallback UX for write failures.When file write fails, the code gracefully falls back to displaying the generated YAML content so users can manually create the file. This prevents data loss and provides a clear recovery path.
988-1000: Proper handling of user cancellation.The
isExitPromptErrorcheck correctly identifies Ctrl+C and provides a friendly message reminding users that skills and commands were already created. Re-throwing unexpected errors ensures they aren't silently swallowed.src/core/artifact-graph/instruction-loader.ts (1)
213-260: Template enrichment logic is well-structured.The enrichment follows the documented order (context → rules → template) and properly handles:
- Optional projectRoot with graceful fallback on read errors
- Per-session warning deduplication for invalid artifact IDs
- Empty rules arrays are correctly excluded
- Rules are only injected for matching artifacts
The implementation aligns well with the spec requirements.
src/core/config-prompts.ts (5)
1-4: LGTM!Imports are clean. The static import of
@inquirermodules from the past review has been addressed - they're now dynamically imported within the functions.
9-16: LGTM!Duck-typing approach for
ExitPromptErrordetection is appropriate given the dynamic import constraints. The check is thorough.
21-30: LGTM!The interface cleanly models the prompt result with
createConfigflag to distinguish user acceptance from actual config values.
138-167: LGTM!The rule collection loop is well-designed with clear termination condition (empty line) and proper trimming of input.
175-196: LGTM!Serialization logic correctly includes only defined fields and uses appropriate YAML formatting options for readability.
openspec/changes/project-config/design.md (1)
1-41: Well-structured design document.The document thoroughly covers context, goals, non-goals, and key decisions. The rationale for each decision (YAML format, config location, XML tags, schema resolution order) is clearly articulated with alternatives considered.
src/core/project-config.ts (3)
19-43: LGTM!The Zod schema is well-documented with clear field descriptions. The inline comments explain the purpose of each field effectively.
163-181: LGTM!The validation logic is clean and produces helpful warning messages with valid alternatives listed.
220-254: LGTM!The suggestion logic and message formatting provide clear, actionable guidance when schema resolution fails.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| #### Scenario: Context contains markdown | ||
| - **WHEN** context includes markdown formatting like `**bold**` or `[links](url)` | ||
| - **THEN** markdown is preserved without rendering or escaping |
There was a problem hiding this comment.
Minor: Capitalize "Markdown" as a proper noun.
As per coding guidelines, use consistent capitalization for proper nouns.
-#### Scenario: Context contains markdown
-- **WHEN** context includes markdown formatting like `**bold**` or `[links](url)`
+#### Scenario: Context contains Markdown
+- **WHEN** context includes Markdown formatting like `**bold**` or `[links](url)`🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ns markdown - WHEN context includes markdown formatting like **bold** or `[links](...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In `@openspec/changes/project-config/specs/context-injection/spec.md` around lines
49 - 51, Update the scenario text "Scenario: Context contains markdown" and its
bullet points to capitalize "Markdown" everywhere (e.g., change `markdown` to
`Markdown` in the scenario title and in the sentence "context includes markdown
formatting like `**bold**` or `[links](url)`") so the proper noun is
consistently capitalized.
| #### Scenario: Unknown artifact ID in rules | ||
| - **WHEN** instructions loaded and config has `rules: { unknownartifact: [...] }` | ||
| - **THEN** warning emitted: "Unknown artifact ID in rules: 'unknownartifact'. Valid IDs for schema 'spec-driven': design, proposal, specs, tasks" |
There was a problem hiding this comment.
Warning message quote style inconsistent with implementation.
The spec shows single quotes: "Unknown artifact ID in rules: 'unknownartifact'", but the implementation in src/core/project-config.ts uses double quotes: "Unknown artifact ID in rules: \"${artifactId}\"".
Consider aligning the spec with the actual implementation to avoid confusion during testing:
-- **THEN** warning emitted: "Unknown artifact ID in rules: 'unknownartifact'. Valid IDs for schema 'spec-driven': design, proposal, specs, tasks"
+- **THEN** warning emitted: "Unknown artifact ID in rules: "unknownartifact". Valid IDs for schema "spec-driven": design, proposal, specs, tasks"🤖 Prompt for AI Agents
In `@openspec/changes/project-config/specs/rules-injection/spec.md` around lines
89 - 91, The spec expects the warning to use single quotes around the artifact
id but the actual warning emitted by the project-config code (the log that
builds the "Unknown artifact ID in rules" message using the artifactId variable)
uses double quotes; update the rules-injection spec's expected message to match
the implementation (double-quoted artifact id) so tests and implementation are
consistent.
| import { promptForConfig, serializeConfig, isExitPromptError } from '../core/config-prompts.js'; | ||
| import { readProjectConfig } from '../core/project-config.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'readProjectConfig' src/commands/artifact-workflow.tsRepository: Fission-AI/OpenSpec
Length of output: 128
Remove unused import: readProjectConfig.
The readProjectConfig function is imported on line 34 but is not used anywhere in this file. The config reading is handled within generateInstructions in instruction-loader.ts.
import { promptForConfig, serializeConfig, isExitPromptError } from '../core/config-prompts.js';
-import { readProjectConfig } from '../core/project-config.js';📝 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.
| import { promptForConfig, serializeConfig, isExitPromptError } from '../core/config-prompts.js'; | |
| import { readProjectConfig } from '../core/project-config.js'; | |
| import { promptForConfig, serializeConfig, isExitPromptError } from '../core/config-prompts.js'; |
🤖 Prompt for AI Agents
In `@src/commands/artifact-workflow.ts` around lines 33 - 34, Remove the unused
import readProjectConfig from the import list at the top of the file; keep the
remaining imports (promptForConfig, serializeConfig, isExitPromptError) intact
since config reading is performed elsewhere (generateInstructions in
instruction-loader.ts), so simply delete readProjectConfig from the import
statement to eliminate the unused symbol.
| const contextInput = await editor({ | ||
| message: 'Press Enter to skip, or edit context:', | ||
| default: '', | ||
| waitForUseInput: false, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@inquirer/prompts editor prompt options documentation
💡 Result:
Options for the @inquirer/prompts "editor" prompt (usage + main config properties):
Usage example:
import { editor } from '@inquirer/prompts';
const answer = await editor({ message: 'Enter a description' }); [1][2].
Main options
- message (string, required): question prompt shown to user. [1][2]
- default (string, optional): default text placed into the temporary editor file. [1][2]
- validate (string => boolean | string | Promise<boolean | string>, optional): validator run on submit; returning a string shows that as the error message. [1][2]
Notes
- The launched editor is taken from $VISUAL or $EDITOR (or OS default if unset). [2][3]
Sources
[1] @inquirer/editor (npm) — options & usage.
[2] @inquirer/prompts (npm) — editor usage & options.
[3] Inquirer.js repository (GitHub) — prompts overview.
🏁 Script executed:
# Check the actual file to see the context
cat -n src/core/config-prompts.ts | sed -n '70,85p'Repository: Fission-AI/OpenSpec
Length of output: 729
🌐 Web query:
@inquirer/prompts editor waitForUseInput invalid option
💡 Result:
You're hitting a known typo: the correct option name is waitForUserInput (not waitForUseInput). This was fixed in Inquirer.js releases. [1][2]
References:
- Release note: "fix
@inquirer/editor: Fix typos/waitForUseInput/waitForUserInput." [1] @inquirer/editordocs (options include waitForUseInput / waitForUserInput historically). [2]
🏁 Script executed:
# Search the repo for any usage of waitForUseInput
rg "waitForUseInput" --type ts --type jsRepository: Fission-AI/OpenSpec
Length of output: 117
Fix the waitForUserInput option name typo.
Change waitForUseInput: false to waitForUserInput: false on line 78. The option name is waitForUserInput, not waitForUseInput.
🤖 Prompt for AI Agents
In `@src/core/config-prompts.ts` around lines 75 - 79, In the editor call that
sets up contextInput (the const contextInput = await editor({...}) invocation),
fix the typo in the options object by renaming waitForUseInput to the correct
option name waitForUserInput: update the property key inside that editor(...)
call so the option is honored.
Summary
openspec/config.yamlsupport for project-level customization without forking schemasartifact-experimental-setupcommandChanges
New files:
src/core/project-config.ts- Zod schema, parsing, and validation helperssrc/core/config-prompts.ts- Interactive prompts for config creationtest/core/project-config.test.ts- Unit tests for config parsingModified files:
src/commands/artifact-workflow.ts- Add config creation to experimental setupsrc/core/artifact-graph/instruction-loader.ts- Inject<context>and<rules>sectionssrc/utils/change-metadata.ts- Add config to schema resolution ordersrc/utils/change-utils.ts- Use config schema for new changesChange proposals:
openspec/changes/project-config/- Full proposal, design, specs, and tasksopenspec/changes/project-local-schemas/- Related future work proposalTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.