Skip to content

feat(store): support separate schema stores - #1473

Open
Patodo wants to merge 6 commits into
Fission-AI:mainfrom
Patodo:p/schema-store-sources
Open

feat(store): support separate schema stores#1473
Patodo wants to merge 6 commits into
Fission-AI:mainfrom
Patodo:p/schema-store-sources

Conversation

@Patodo

@Patodo Patodo commented Jul 28, 2026

Copy link
Copy Markdown

Summary

  • add a schemaStore project configuration that selects a registered Store independently from the Planning Store
  • support scalar declarations, exact schema visibility allowlists, and * visibility by default
  • resolve visible Store schemas ahead of user and package schemas while preserving existing behavior when schemaStore is absent
  • propagate the resolved schema context through schema inspection, templates, completion, validation, and the complete change lifecycle
  • report Store provenance consistently in human and JSON output
  • update the OpenSpec change artifacts, user documentation, and Changesets entry

schemas and templates resolve schemaStore from the nearest consumer project. This PR does not add --store to those commands and does not modify generated OpenSpec skills.

Motivation

Departments may want specs and changes to live in one shared Planning Store while workflow schemas and templates are maintained in a separate Store. This lets different teams reuse department-level schemas without coupling schema ownership to the repository that owns planning artifacts.

Example:

store: department-planning
schema: qeda-sdd
schemaStore:
  id: department-schemas
  schemas:
    - qeda-sdd

Schema Store synchronization remains user-managed through normal Git operations; OpenSpec reads the registered local checkout and performs no fetch, pull, clone, or push during schema resolution.

Validation

  • pnpm exec openspec validate add-schema-store-sources --strict
  • pnpm run build
  • pnpm exec tsc --noEmit
  • pnpm lint
  • git diff --check
  • full Vitest suite: 651 suites and 3,338 tests passed

Summary by CodeRabbit

  • New Features

    • Added schemaStore support to source workflow schemas and templates from a registered Store, independently from planning, with visibility controls (wildcards and allowlists) and Store precedence.
    • Updated CLI behavior so schema resolution, validation, completion, and reporting include Store provenance (Store ID), and added guardrails that prevent creating local invisible schemas when schemaStore is configured.
  • Documentation

    • Documented schemaStore configuration, precedence/diagnostics, wildcard/allowlist semantics, and the fact that Stores are read from their local checkout.
  • Tests

    • Added end-to-end and unit coverage for Store context resolution, visibility filtering, and affected command flows.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a88adf1-989c-401f-993d-41183eedd2b3

📥 Commits

Reviewing files that changed from the base of the PR and between db15d07 and 7da5dba.

📒 Files selected for processing (16)
  • docs/cli.md
  • docs/customization.md
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/design.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • openspec/changes/add-schema-store-sources/tasks.md
  • src/cli/index.ts
  • src/commands/change.ts
  • src/commands/schema.ts
  • src/core/completions/completion-provider.ts
  • src/core/validation/validator.ts
  • src/utils/change-utils.ts
  • test/commands/completion.test.ts
  • test/commands/store-root-selection.test.ts
  • test/core/artifact-graph/resolver.test.ts
  • test/core/project-config.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
  • docs/customization.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • src/core/validation/validator.ts
  • docs/cli.md
  • src/utils/change-utils.ts
  • openspec/changes/add-schema-store-sources/tasks.md
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/design.md
  • src/core/completions/completion-provider.ts
  • test/commands/store-root-selection.test.ts
  • test/core/artifact-graph/resolver.test.ts
  • src/commands/schema.ts

📝 Walkthrough

Walkthrough

Adds configurable schemaStore support for resolving shared workflow schemas and templates independently from planning roots, including visibility filtering, Store provenance, diagnostics, root-context propagation, workflow integration, tests, documentation, and release metadata.

Changes

Schema Store resolution and workflow integration

Layer / File(s) Summary
Configuration and schema Store context resolution
src/core/project-config.ts, src/core/root-selection.ts, openspec/changes/add-schema-store-sources/*
Defines schemaStore declaration contract (scalar and object forms), parsing/normalization with strict validation, Store registry lookup, consumer-root overlays, and error diagnostics for malformed or unavailable stores.
Schema resolution with Store visibility and provenance
src/core/artifact-graph/resolver.ts, test/core/artifact-graph/resolver.test.ts, test/core/project-config.test.ts
Extends resolver to accept SchemaResolutionTarget (context with source, storeId, visibleSchemas), applies Store visibility filtering, adds source: 'store' and optional storeId to SchemaInfo, and includes Store-context tests.
Schema commands and provenance output
src/commands/schema.ts, src/commands/workflow/shared.ts
Routes schema CLI (which, validate, fork, init) through resolved Store context, reports Store provenance with source: 'store' and storeId in human and JSON output, and blocks unsafe writes via schemaStoreWriteError guard.
Completion and template discovery with Store context
src/core/completions/completion-provider.ts, src/commands/workflow/{schemas,templates}.ts, test/commands/completion.test.ts
Updates completion provider to support per-target schema-name caching, extends template discovery to report Store source and storeId, filters schemas list through resolved Store context, and includes dynamic completion tests.
Workflow lifecycle: CLI and command wiring
src/cli/index.ts, src/commands/{change,completion,validate}.ts, src/commands/workflow/{instructions,new-change,status}.ts
Threads schemaContext and resolved projectConfig through CLI dispatch, command instantiation, schema validation, and root resolution across list, view, new-change, instructions, status, validate, and change commands.
Instruction loading and workflow context
src/core/artifact-graph/instruction-loader.ts
Adds schemaTarget to ChangeContext and LoadChangeContextOptions, updates loadTemplate to use schemaTarget for lookups, and routes resolved schema context through generateApplyInstructions and archive generation.
Task progress and validation with schema context
src/core/validation/validator.ts, src/utils/{change-metadata,change-utils,task-progress}.ts
Updates validator, task-progress resolution, and change-metadata functions to accept and use schemaTarget for schema membership checks, skip-specs interpretation, and tracked-tasks artifact resolution.
List, view, and archive with resolved context
src/core/{list,view,archive}.ts
Updates list, view, and archive commands to accept schemaTarget and projectConfig, computes task progress using Store-aware schema resolution, ensures change validation respects the resolved schema context.
Documentation and release contract
.changeset/share-schema-stores.md, docs/{cli,customization}.md, docs/stores-beta/user-guide.md
Documents schemaStore syntax, visibility/precedence, command limitations, local checkout handling (no fetch/pull/pin), diagnostics, and the minor release note.
Integration and end-to-end tests
test/commands/store-root-selection.test.ts, test/core/{view,root-selection}.test.ts
Comprehensive CLI tests covering schema Store registration, visibility filtering, workflow execution (change creation, status, instructions, templates, archive), schema discovery/validation/fork/init guards, task progress, and dual planning/schema Store scenarios.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant resolveOpenSpecRoot
  participant StoreRegistry
  participant WorkflowCommand
  participant resolveSchema
  CLI->>resolveOpenSpecRoot: resolve planning root and schema context
  resolveOpenSpecRoot->>StoreRegistry: resolve registered schema Store
  StoreRegistry-->>resolveOpenSpecRoot: canonical checkout and visibility
  resolveOpenSpecRoot-->>CLI: resolved root with schemaContext
  CLI->>WorkflowCommand: execute with schemaTarget and projectConfig
  WorkflowCommand->>resolveSchema: resolve workflow schema
  resolveSchema-->>WorkflowCommand: schema path and Store provenance
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers: alfred-openspec, tabishb, clay-good

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding support for separate schema stores.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Patodo
Patodo marked this pull request as ready for review July 28, 2026 17:19
@Patodo
Patodo requested a review from a team as a code owner July 28, 2026 17:19
@Patodo
Patodo requested review from clay-good and removed request for a team July 28, 2026 17:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/core/completions/completion-provider.ts (1)

93-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Key the schema cache by resolution target.

A call with a different schemaTarget within the TTL reuses schemaCache, returning schemas from the prior Store/visibility context. Invalidate on target changes or cache by a stable target key.

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

In `@src/core/completions/completion-provider.ts` around lines 93 - 104, Update
getSchemaNames so schemaCache is associated with the effective schemaTarget (or
this.projectRoot fallback), using a stable target key and only reusing cached
data when that key matches; otherwise fetch fresh schemas and store the new key
with the result.
src/commands/schema.ts (1)

604-651: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Guard schema fork/schema init writes into sinkRoot when a Store is active.

When schemaStore is configured, writes stay at root.consumerRoot (sinkRoot in the workflow), but schema resolution lists the active project layer via schemaContext.root. If a Store replaces the project schema layer, copying or creating a schema here leaves it invisible to subsequent lookups/listing. Reuse sourceLocation there too so schema fork --json reports whether the destination appears at the active project layer while still telling the user where the file was written.

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

In `@src/commands/schema.ts` around lines 604 - 651, Update the schema fork/init
destination handling after sourceLocation is determined to use the active schema
layer represented by sourceLocation when resolving or reporting the destination,
while continuing to write files under root.consumerRoot. Ensure schema fork
--json distinguishes the actual write location from whether the destination is
visible in the active project layer, including Store-backed configurations.
🧹 Nitpick comments (1)
src/core/artifact-graph/resolver.ts (1)

34-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate visibility logic between resolver.ts and schema.ts. isVisibleFromPrimarySource in resolver.ts is not exported, so schema.ts's checkAllLocations reimplements the identical source === 'project' || visibleSchemas === '*' || visibleSchemas.includes(name) expression inline. One root cause: the helper should be shared, not duplicated.

  • src/core/artifact-graph/resolver.ts#L34-L44: export isVisibleFromPrimarySource.
  • src/commands/schema.ts#L57-L79: import and reuse the exported helper instead of recomputing visible inline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/artifact-graph/resolver.ts` around lines 34 - 44, Export
isVisibleFromPrimarySource from src/core/artifact-graph/resolver.ts. In
src/commands/schema.ts, import and reuse this helper within checkAllLocations
instead of duplicating the visible expression inline; preserve the existing
visibility behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/change.ts`:
- Around line 42-52: Update the constructor comment to reflect that deprecated
noun-form commands can receive root-aware context from the CLI. In
ChangeCommand.validate(), derive changesPath from this.rootPath rather than
process.cwd(), and construct Validator with this.schemaTarget so
schemaStore-backed schemas resolve skip specs consistently with the top-level
validation flow.

---

Outside diff comments:
In `@src/commands/schema.ts`:
- Around line 604-651: Update the schema fork/init destination handling after
sourceLocation is determined to use the active schema layer represented by
sourceLocation when resolving or reporting the destination, while continuing to
write files under root.consumerRoot. Ensure schema fork --json distinguishes the
actual write location from whether the destination is visible in the active
project layer, including Store-backed configurations.

In `@src/core/completions/completion-provider.ts`:
- Around line 93-104: Update getSchemaNames so schemaCache is associated with
the effective schemaTarget (or this.projectRoot fallback), using a stable target
key and only reusing cached data when that key matches; otherwise fetch fresh
schemas and store the new key with the result.

---

Nitpick comments:
In `@src/core/artifact-graph/resolver.ts`:
- Around line 34-44: Export isVisibleFromPrimarySource from
src/core/artifact-graph/resolver.ts. In src/commands/schema.ts, import and reuse
this helper within checkAllLocations instead of duplicating the visible
expression inline; preserve the existing visibility behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f475f1c3-bac5-42e5-a7a4-6f7ceab4beff

📥 Commits

Reviewing files that changed from the base of the PR and between 9a937cb and db15d07.

📒 Files selected for processing (43)
  • .changeset/share-schema-stores.md
  • docs/agent-contract.md
  • docs/cli.md
  • docs/customization.md
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/.openspec.yaml
  • openspec/changes/add-schema-store-sources/design.md
  • openspec/changes/add-schema-store-sources/proposal.md
  • openspec/changes/add-schema-store-sources/specs/config-loading/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-store-sources/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-which-command/spec.md
  • openspec/changes/add-schema-store-sources/tasks.md
  • src/cli/index.ts
  • src/commands/change.ts
  • src/commands/completion.ts
  • src/commands/schema.ts
  • src/commands/validate.ts
  • src/commands/workflow/instructions.ts
  • src/commands/workflow/new-change.ts
  • src/commands/workflow/schemas.ts
  • src/commands/workflow/shared.ts
  • src/commands/workflow/status.ts
  • src/commands/workflow/templates.ts
  • src/core/archive.ts
  • src/core/artifact-graph/index.ts
  • src/core/artifact-graph/instruction-loader.ts
  • src/core/artifact-graph/resolver.ts
  • src/core/completions/completion-provider.ts
  • src/core/list.ts
  • src/core/project-config.ts
  • src/core/root-selection.ts
  • src/core/validation/validator.ts
  • src/core/view.ts
  • src/utils/change-metadata.ts
  • src/utils/change-utils.ts
  • src/utils/task-progress.ts
  • test/commands/completion.test.ts
  • test/commands/store-root-selection.test.ts
  • test/core/artifact-graph/resolver.test.ts
  • test/core/project-config.test.ts
  • test/core/root-selection.test.ts
  • test/core/view.test.ts

Comment thread src/commands/change.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/schema.ts`:
- Line 647: Update the schema fork output after getSchemaResolution to include
the resolved storeId in both JSON sourceLocation data and human-readable output,
while preserving the existing store source labeling and provenance behavior for
other source types.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e5884762-196d-4052-9001-cbc9dee27cc7

📥 Commits

Reviewing files that changed from the base of the PR and between 9a937cb and db15d07.

📒 Files selected for processing (43)
  • .changeset/share-schema-stores.md
  • docs/agent-contract.md
  • docs/cli.md
  • docs/customization.md
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/.openspec.yaml
  • openspec/changes/add-schema-store-sources/design.md
  • openspec/changes/add-schema-store-sources/proposal.md
  • openspec/changes/add-schema-store-sources/specs/config-loading/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-store-sources/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-which-command/spec.md
  • openspec/changes/add-schema-store-sources/tasks.md
  • src/cli/index.ts
  • src/commands/change.ts
  • src/commands/completion.ts
  • src/commands/schema.ts
  • src/commands/validate.ts
  • src/commands/workflow/instructions.ts
  • src/commands/workflow/new-change.ts
  • src/commands/workflow/schemas.ts
  • src/commands/workflow/shared.ts
  • src/commands/workflow/status.ts
  • src/commands/workflow/templates.ts
  • src/core/archive.ts
  • src/core/artifact-graph/index.ts
  • src/core/artifact-graph/instruction-loader.ts
  • src/core/artifact-graph/resolver.ts
  • src/core/completions/completion-provider.ts
  • src/core/list.ts
  • src/core/project-config.ts
  • src/core/root-selection.ts
  • src/core/validation/validator.ts
  • src/core/view.ts
  • src/utils/change-metadata.ts
  • src/utils/change-utils.ts
  • src/utils/task-progress.ts
  • test/commands/completion.test.ts
  • test/commands/store-root-selection.test.ts
  • test/core/artifact-graph/resolver.test.ts
  • test/core/project-config.test.ts
  • test/core/root-selection.test.ts
  • test/core/view.test.ts
🚧 Files skipped from review as they are similar to previous changes (32)
  • openspec/changes/add-schema-store-sources/specs/config-loading/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • src/commands/validate.ts
  • openspec/changes/add-schema-store-sources/.openspec.yaml
  • src/commands/workflow/status.ts
  • src/core/artifact-graph/index.ts
  • src/core/validation/validator.ts
  • src/utils/change-utils.ts
  • .changeset/share-schema-stores.md
  • src/commands/workflow/schemas.ts
  • docs/customization.md
  • src/commands/completion.ts
  • docs/cli.md
  • openspec/changes/add-schema-store-sources/proposal.md
  • openspec/changes/add-schema-store-sources/specs/schema-which-command/spec.md
  • src/utils/task-progress.ts
  • docs/agent-contract.md
  • src/core/list.ts
  • openspec/changes/add-schema-store-sources/specs/schema-store-sources/spec.md
  • src/commands/workflow/shared.ts
  • src/commands/change.ts
  • src/core/artifact-graph/instruction-loader.ts
  • src/commands/workflow/new-change.ts
  • openspec/changes/add-schema-store-sources/design.md
  • src/core/completions/completion-provider.ts
  • src/core/view.ts
  • docs/stores-beta/user-guide.md
  • src/core/project-config.ts
  • src/core/artifact-graph/resolver.ts
  • src/core/root-selection.ts
  • src/commands/workflow/instructions.ts
  • src/core/archive.ts

Comment thread src/commands/schema.ts

// Determine source location
const sourceResolution = getSchemaResolution(source, projectRoot);
const sourceResolution = getSchemaResolution(source, schemaContext);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include the Store identifier in fork output.

Line 647 resolves storeId, but schema fork emits only sourceLocation: "store" and human (store). Include the Store ID in both outputs so users can identify the configured Store that supplied the forked schema.

The PR objective requires Store provenance in human-readable and JSON output.

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

In `@src/commands/schema.ts` at line 647, Update the schema fork output after
getSchemaResolution to include the resolved storeId in both JSON sourceLocation
data and human-readable output, while preserving the existing store source
labeling and provenance behavior for other source types.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
test/core/project-config.test.ts (1)

572-576: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate writeConfig helper.

An identical helper already exists at Lines 490-494; consider hoisting one copy to the enclosing describe if the scopes allow.

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

In `@test/core/project-config.test.ts` around lines 572 - 576, Remove the
duplicate writeConfig helper in the later test scope and reuse the existing
writeConfig defined earlier in the enclosing describe. Hoist the remaining
helper only if needed so all affected tests retain access without changing their
behavior.
src/commands/completion.ts (1)

289-304: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider degrading to project-root schema listing for any root-selection failure.

Only no_root_with_registered_stores is tolerated. Any other RootSelectionError (e.g. invalid_schema_store_declaration from a malformed schemaStore, or an unresolvable Store checkout) propagates to the outer catch at Line 322, so openspec complete schemas emits nothing at all instead of the locally available schemas. Shell completion is a best-effort surface; a config typo silently killing it is unhelpful.

♻️ Suggested fallback
-          } catch (error) {
-            if (
-              !isRootSelectionError(error) ||
-              error.diagnostic.code !== 'no_root_with_registered_stores'
-            ) {
-              throw error;
-            }
-          }
+          } catch (error) {
+            // Completion is best-effort: fall back to project-root listing
+            // rather than emitting nothing on any root-selection failure.
+            if (!isRootSelectionError(error)) {
+              throw error;
+            }
+          }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/completion.ts` around lines 289 - 304, Update the schema
completion flow around resolveOpenSpecRoot and getSchemaNames so any
root-selection failure degrades to listing schemas from the project root, rather
than rethrowing RootSelectionError variants such as invalid schema-store
declarations or unresolved checkouts. Preserve normal resolved-root behavior,
and ensure completion remains best-effort while only falling back when root
resolution fails.
test/core/artifact-graph/resolver.test.ts (1)

752-764: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This test's key assertions are vacuous — the consumer root is never part of the resolution context.

consumerRoot is only used to write a schema on disk; every lookup here is done with storeContext(storeRoot), so the resolver has no path by which it could ever see consumer-only. not.toContain('consumer-only') and toBeNull() would still pass if Store replacement were completely broken. To actually cover replacement, assert that the same schema name resolves to the Store copy while the consumer-root context resolves to the consumer copy.

💚 Suggested strengthening
     it('replaces consumer-local project schemas when a Store is configured', () => {
       process.env.XDG_DATA_HOME = path.join(tempDir, 'user-data');
       const consumerRoot = path.join(tempDir, 'consumer');
       const storeRoot = path.join(tempDir, 'schema-store');
-      writeSchema(consumerRoot, 'consumer-only', 'consumer-version');
-      writeSchema(storeRoot, 'store-only', 'store-version');
+      const consumerDir = writeSchema(consumerRoot, 'shared-schema', 'consumer-version');
+      const storeDir = writeSchema(storeRoot, 'shared-schema', 'store-version');
+      writeSchema(consumerRoot, 'consumer-only', 'consumer-version');
+      writeSchema(storeRoot, 'store-only', 'store-version');
 
+      // Sanity: the consumer root really does own these schemas.
+      expect(getSchemaDir('shared-schema', consumerRoot)).toBe(consumerDir);
+      expect(listSchemas(consumerRoot)).toContain('consumer-only');
+
       const schemas = listSchemas(storeContext(storeRoot));
 
       expect(schemas).toContain('store-only');
       expect(schemas).not.toContain('consumer-only');
+      expect(getSchemaDir('shared-schema', storeContext(storeRoot))).toBe(storeDir);
       expect(getSchemaDir('consumer-only', storeContext(storeRoot))).toBeNull();
     });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/core/artifact-graph/resolver.test.ts` around lines 752 - 764, Strengthen
the test around listSchemas and getSchemaDir by creating the resolution context
with both consumerRoot and storeRoot, then use the same schema name in both
locations with distinguishable versions. Assert the Store-backed context returns
the Store schema while a consumer-root-only context returns the consumer schema,
ensuring the replacement behavior is genuinely exercised rather than merely
confirming the consumer path is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openspec/changes/add-schema-store-sources/tasks.md`:
- Line 6: Update task 1.4 to invoke Vitest with the documented `pnpm exec vitest
run test/core/project-config.test.ts` command, preserving the existing focused
test target and configuration-contract verification.

---

Nitpick comments:
In `@src/commands/completion.ts`:
- Around line 289-304: Update the schema completion flow around
resolveOpenSpecRoot and getSchemaNames so any root-selection failure degrades to
listing schemas from the project root, rather than rethrowing RootSelectionError
variants such as invalid schema-store declarations or unresolved checkouts.
Preserve normal resolved-root behavior, and ensure completion remains
best-effort while only falling back when root resolution fails.

In `@test/core/artifact-graph/resolver.test.ts`:
- Around line 752-764: Strengthen the test around listSchemas and getSchemaDir
by creating the resolution context with both consumerRoot and storeRoot, then
use the same schema name in both locations with distinguishable versions. Assert
the Store-backed context returns the Store schema while a consumer-root-only
context returns the consumer schema, ensuring the replacement behavior is
genuinely exercised rather than merely confirming the consumer path is absent.

In `@test/core/project-config.test.ts`:
- Around line 572-576: Remove the duplicate writeConfig helper in the later test
scope and reuse the existing writeConfig defined earlier in the enclosing
describe. Hoist the remaining helper only if needed so all affected tests retain
access without changing their behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e5884762-196d-4052-9001-cbc9dee27cc7

📥 Commits

Reviewing files that changed from the base of the PR and between 9a937cb and db15d07.

📒 Files selected for processing (43)
  • .changeset/share-schema-stores.md
  • docs/agent-contract.md
  • docs/cli.md
  • docs/customization.md
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/.openspec.yaml
  • openspec/changes/add-schema-store-sources/design.md
  • openspec/changes/add-schema-store-sources/proposal.md
  • openspec/changes/add-schema-store-sources/specs/config-loading/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-store-sources/spec.md
  • openspec/changes/add-schema-store-sources/specs/schema-which-command/spec.md
  • openspec/changes/add-schema-store-sources/tasks.md
  • src/cli/index.ts
  • src/commands/change.ts
  • src/commands/completion.ts
  • src/commands/schema.ts
  • src/commands/validate.ts
  • src/commands/workflow/instructions.ts
  • src/commands/workflow/new-change.ts
  • src/commands/workflow/schemas.ts
  • src/commands/workflow/shared.ts
  • src/commands/workflow/status.ts
  • src/commands/workflow/templates.ts
  • src/core/archive.ts
  • src/core/artifact-graph/index.ts
  • src/core/artifact-graph/instruction-loader.ts
  • src/core/artifact-graph/resolver.ts
  • src/core/completions/completion-provider.ts
  • src/core/list.ts
  • src/core/project-config.ts
  • src/core/root-selection.ts
  • src/core/validation/validator.ts
  • src/core/view.ts
  • src/utils/change-metadata.ts
  • src/utils/change-utils.ts
  • src/utils/task-progress.ts
  • test/commands/completion.test.ts
  • test/commands/store-root-selection.test.ts
  • test/core/artifact-graph/resolver.test.ts
  • test/core/project-config.test.ts
  • test/core/root-selection.test.ts
  • test/core/view.test.ts
🚧 Files skipped from review as they are similar to previous changes (28)
  • openspec/changes/add-schema-store-sources/.openspec.yaml
  • src/core/artifact-graph/index.ts
  • openspec/changes/add-schema-store-sources/specs/schema-which-command/spec.md
  • .changeset/share-schema-stores.md
  • openspec/changes/add-schema-store-sources/specs/schema-resolution/spec.md
  • openspec/changes/add-schema-store-sources/specs/config-loading/spec.md
  • src/commands/workflow/schemas.ts
  • openspec/changes/add-schema-store-sources/specs/schema-store-sources/spec.md
  • src/core/validation/validator.ts
  • src/commands/workflow/shared.ts
  • src/core/view.ts
  • src/utils/change-utils.ts
  • docs/cli.md
  • src/core/completions/completion-provider.ts
  • docs/stores-beta/user-guide.md
  • openspec/changes/add-schema-store-sources/proposal.md
  • src/utils/task-progress.ts
  • src/commands/validate.ts
  • src/commands/workflow/templates.ts
  • docs/agent-contract.md
  • openspec/changes/add-schema-store-sources/design.md
  • src/commands/change.ts
  • src/utils/change-metadata.ts
  • src/core/root-selection.ts
  • src/cli/index.ts
  • src/core/artifact-graph/resolver.ts
  • src/commands/workflow/instructions.ts
  • src/commands/schema.ts

Comment thread openspec/changes/add-schema-store-sources/tasks.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant