From 653f2c9aabd61fb95ec4695209c47483ba8e7aec Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Thu, 25 Dec 2025 22:22:40 +1100 Subject: [PATCH 1/8] proposal: add change manager - extract + new functionality Slice 2 of the artifact tracker POC. Creates ChangeManager module that: **Extracts existing functionality:** - `listChanges()` from ListCommand + ChangeCommand.getActiveChanges() - `changeExists()` from inline fs.access() checks - `getChangePath()` from inline path.join() calls - `isInitialized()` from ListCommand directory check **Adds new functionality:** - `createChange(name, description?)` - create change directory + README - `validateName(name)` - enforce kebab-case naming **Refactors CLI commands to be thin wrappers:** - ListCommand delegates to ChangeManager - ChangeCommand delegates to ChangeManager Also updates docs/artifact_poc.md to reflect XDG decisions from Slice 1. --- docs/artifact_poc.md | 278 ++++++++++-------- openspec/changes/add-change-manager/design.md | 178 +++++++++++ .../changes/add-change-manager/proposal.md | 58 ++++ .../specs/change-manager/spec.md | 137 +++++++++ openspec/changes/add-change-manager/tasks.md | 81 +++++ 5 files changed, 613 insertions(+), 119 deletions(-) create mode 100644 openspec/changes/add-change-manager/design.md create mode 100644 openspec/changes/add-change-manager/proposal.md create mode 100644 openspec/changes/add-change-manager/specs/change-manager/spec.md create mode 100644 openspec/changes/add-change-manager/tasks.md diff --git a/docs/artifact_poc.md b/docs/artifact_poc.md index 55020233..970ada7f 100644 --- a/docs/artifact_poc.md +++ b/docs/artifact_poc.md @@ -22,7 +22,7 @@ This system is **not** a workflow engine. It's an **artifact tracker with depend | Term | Definition | Example | |------|------------|---------| | **Change** | A unit of work being planned (feature, refactor, migration) | `openspec/changes/add-auth/` | -| **Schema** | An artifact graph definition (what artifacts exist, their dependencies) | `schemas/spec-driven.yaml` | +| **Schema** | An artifact graph definition (what artifacts exist, their dependencies) | `spec-driven.yaml` | | **Artifact** | A node in the graph (a document to create) | `proposal`, `design`, `specs` | | **Template** | Instructions/guidance for creating an artifact | `templates/proposal.md` | @@ -47,25 +47,39 @@ Schemas can vary across multiple dimensions: | Language | `en`, `zh`, `es` | | Custom | `team-alpha`, `experimental` | +### Schema Resolution (XDG Standard) + +Schemas follow the XDG Base Directory Specification with a 2-level resolution: + +``` +1. ${XDG_DATA_HOME}/openspec/schemas/.yaml # Global user override +2. /schemas/.yaml # Built-in defaults +``` + +**Platform-specific paths:** +- Unix/macOS: `~/.local/share/openspec/schemas/` +- Windows: `%LOCALAPPDATA%/openspec/schemas/` +- All platforms: `$XDG_DATA_HOME/openspec/schemas/` (when set) + +**Why XDG?** +- Schemas are workflow definitions (data), not user preferences (config) +- Built-ins baked into package, never auto-copied +- Users customize by creating files in global data dir +- Consistent with modern CLI tooling standards + ### Template Inheritance (2 Levels Max) +Templates also use 2-level resolution (to be implemented in Slice 3): + ``` -.openspec/ -├── templates/ # Shared (Level 1) -│ ├── proposal.md -│ ├── design.md -│ └── specs.md -│ -└── schemas/ - └── tdd/ - ├── schema.yaml - └── templates/ # Overrides (Level 2) - └── tests.md # TDD-specific +1. ${XDG_DATA_HOME}/openspec/schemas//templates/.md # Schema-specific +2. ${XDG_DATA_HOME}/openspec/templates/.md # Shared +3. /templates/.md # Built-in fallback ``` **Rules:** -- Shared templates are the default -- Schema-specific templates override OR add new +- Schema-specific templates override shared templates +- Shared templates override package built-ins - A CLI command shows resolved paths (no guessing) - No inheritance between schemas (copy if you need to diverge) - Max 2 levels - no deeper inheritance chains @@ -90,83 +104,87 @@ The system answers: ## Core Components -### 1. ArtifactGraph +### 1. ArtifactGraph (Slice 1 - COMPLETE) -The dependency graph engine. +The dependency graph engine with XDG-compliant schema resolution. | Responsibility | Approach | |----------------|----------| | Model artifacts as a DAG | Artifact with `requires: string[]` | -| Track completion state | Sets for `completed`, `in_progress`, `failed` | +| Track completion state | `Set` for completed artifacts | | Calculate build order | Kahn's algorithm (topological sort) | | Find ready artifacts | Check if all dependencies are in `completed` set | - -**Key Data Structures:** - -``` -Artifact { - id: string - generates: string // e.g., "proposal.md" or "specs/*.md" - description: string - instruction: string // path to template - requires: string[] // artifact IDs this depends on -} - -ArtifactState { - completed: Set - inProgress: Set - failed: Set -} - -ArtifactGraph { - artifacts: Map -} +| Resolve schemas | XDG global → package built-ins | + +**Key Data Structures (Zod-validated):** + +```typescript +// Zod schemas define types + validation +const ArtifactSchema = z.object({ + id: z.string().min(1), + generates: z.string().min(1), // e.g., "proposal.md" or "specs/*.md" + description: z.string(), + template: z.string(), // path to template file + requires: z.array(z.string()).default([]), +}); + +const SchemaYamlSchema = z.object({ + name: z.string().min(1), + version: z.number().int().positive(), + description: z.string().optional(), + artifacts: z.array(ArtifactSchema).min(1), +}); + +// Derived types +type Artifact = z.infer; +type SchemaYaml = z.infer; ``` **Key Methods:** -- `fromYaml(path)` - Load artifact definitions from YAML -- `getNextArtifacts(state)` - Find artifacts ready to create -- `getBuildOrder()` - Topological sort of all artifacts -- `isComplete(state)` - Check if all artifacts done +- `resolveSchema(name)` - Load schema with XDG fallback +- `ArtifactGraph.fromSchema(schema)` - Build graph from schema +- `detectState(graph, changeDir)` - Scan filesystem for completion +- `getNextArtifacts(graph, completed)` - Find artifacts ready to create +- `getBuildOrder(graph)` - Topological sort of all artifacts +- `getBlocked(graph, completed)` - Artifacts with unmet dependencies --- -### 2. ChangeManager +### 2. ChangeManager (Slice 2) Multi-change orchestration layer. **CLI is fully deterministic** - no "active change" tracking. | Responsibility | Approach | |----------------|----------| | CRUD changes | Create dirs under `openspec/changes//` | -| Template fallback | Schema-specific → Shared (2 levels max) | +| List changes | Scan `openspec/changes/` (excluding `archive/`) | +| Path resolution | Resolve change directory paths | +| Name validation | Enforce kebab-case naming | **Key Paths:** ``` -.openspec/schemas/ → Schema definitions (artifact graphs) -.openspec/templates/ → Shared instruction templates -openspec/changes// → Change instances with artifacts +openspec/changes// → Change instances with artifacts (project-level) ``` **Key Methods:** -- `isInitialized()` - Check for `.openspec/` existence -- `listChanges()` - List all changes in `openspec/changes/` -- `createChange(name, description)` - Create new change directory +- `isInitialized()` - Check for `openspec/changes/` existence +- `listChanges()` - List all changes (excluding archive) +- `createChange(name, description)` - Create new change directory + README - `getChangePath(name)` - Get path to a change directory -- `getSchemaPath(schemaName?)` - Find schema with fallback -- `getTemplatePath(artifactId, schemaName?)` - Find template (schema → shared) +- `changeExists(name)` - Check if change exists -**Note:** No `getActiveChange()`, `setActiveChange()`, or `resolveChange()` - the agent infers which change from conversation context and passes it explicitly to CLI commands. +**Note:** Schema resolution is handled by `artifact-graph` module (Slice 1). Template resolution is handled by `InstructionLoader` (Slice 3). ChangeManager focuses solely on change directory management. --- -### 3. InstructionLoader +### 3. InstructionLoader (Slice 3) -State detection and instruction enrichment. +Template resolution and instruction enrichment. | Responsibility | Approach | |----------------|----------| -| Detect artifact completion | Scan filesystem, support glob patterns | +| Resolve templates | XDG 2-level fallback (schema-specific → shared → built-in) | | Build dynamic context | Gather dependency status, change info | | Enrich templates | Inject context into base templates | | Generate status reports | Formatted markdown with progress | @@ -178,7 +196,7 @@ ChangeState { changeName: string changeDir: string graph: ArtifactGraph - state: ArtifactState + completed: Set // Methods getNextSteps(): string[] @@ -188,13 +206,13 @@ ChangeState { ``` **Key Functions:** +- `getTemplatePath(artifactId, schemaName?)` - Resolve with 2-level fallback - `getEnrichedInstructions(artifactId, projectRoot, changeName?)` - Main entry point - `getChangeStatus(projectRoot, changeName?)` - Formatted status report -- `resolveTemplatePath(artifactId, schemaName?)` - 2-level fallback --- -### 4. CLI +### 4. CLI (Slice 4) User interface layer. **All commands are deterministic** - require explicit `--change` parameter. @@ -252,7 +270,7 @@ This works for ANY artifact in ANY schema - no new slash commands needed when sc │ ORCHESTRATION LAYER │ │ ┌────────────────────┐ ┌──────────────────────────┐ │ │ │ InstructionLoader │───────▶│ ChangeManager │ │ -│ │ │ uses │ │ │ +│ │ (Slice 3) │ uses │ (Slice 2) │ │ │ └─────────┬──────────┘ └──────────────────────────┘ │ └────────────┼────────────────────────────────────────────────┘ │ uses @@ -260,10 +278,9 @@ This works for ANY artifact in ANY schema - no new slash commands needed when sc ┌─────────────────────────────────────────────────────────────┐ │ CORE LAYER │ │ ┌──────────────────────────────────────────────────────┐ │ -│ │ ArtifactGraph │ │ +│ │ ArtifactGraph (Slice 1) │ │ │ │ │ │ -│ │ Artifact ←────── ArtifactState │ │ -│ │ (data) (runtime state) │ │ +│ │ Schema Resolution (XDG) ──→ Graph ──→ State Detection│ │ │ └──────────────────────────────────────────────────────┘ │ └─────────────────────────────────────────────────────────────┘ ▲ @@ -272,9 +289,10 @@ This works for ANY artifact in ANY schema - no new slash commands needed when sc ┌─────────────────────────────────────────────────────────────┐ │ PERSISTENCE LAYER │ │ ┌──────────────────┐ ┌────────────────────────────────┐ │ -│ │ YAML Config │ │ Filesystem Artifacts │ │ -│ │ - config.yaml │ │ - proposal.md, design.md │ │ -│ │ - schema.yaml │ │ - specs/*.md, tasks.md │ │ +│ │ XDG Schemas │ │ Project Artifacts │ │ +│ │ ~/.local/share/ │ │ openspec/changes// │ │ +│ │ openspec/ │ │ - proposal.md, design.md │ │ +│ │ schemas/ │ │ - specs/*.md, tasks.md │ │ │ └──────────────────┘ └────────────────────────────────┘ │ └─────────────────────────────────────────────────────────────┘ ``` @@ -310,17 +328,29 @@ This separation means: - Agent handles all "smartness" - No config.yaml tracking of "active change" -### 3. Two-Level Template Fallback +### 3. XDG-Compliant Schema Resolution + +``` +${XDG_DATA_HOME}/openspec/schemas/.yaml # User override + ↓ (not found) +/schemas/.yaml # Built-in + ↓ (not found) +Error (schema not found) +``` + +### 4. Two-Level Template Fallback (Slice 3) ``` -schema-specific/templates/proposal.md +${XDG_DATA_HOME}/openspec/schemas//templates/.md # Schema-specific ↓ (not found) -.openspec/templates/proposal.md (shared) +${XDG_DATA_HOME}/openspec/templates/.md # Shared + ↓ (not found) +/templates/.md # Built-in ↓ (not found) Error (no silent fallback to avoid confusion) ``` -### 4. Glob Pattern Support +### 5. Glob Pattern Support `specs/*.md` allows multiple files to satisfy a single artifact: @@ -333,7 +363,7 @@ if (artifact.generates.includes("*")) { } ``` -### 5. Stateless State Detection +### 6. Stateless State Detection Every command re-scans the filesystem. No cached state to corrupt. @@ -376,39 +406,41 @@ Structured as **vertical slices** - each slice is independently testable. --- -### Slice 1: "What's Ready?" (Core Query) +### Slice 1: "What's Ready?" (Core Query) ✅ COMPLETE -**Combines:** Types + Graph + State Detection +**Delivers:** Types + Graph + State Detection + Schema Resolution -``` -Input: schema YAML path + change directory -Output: { - completed: ['proposal'], - ready: ['specs'], - blocked: ['design', 'tasks'], - buildOrder: ['proposal', 'specs', 'design', 'tasks'] -} -``` +**Implementation:** `src/core/artifact-graph/` +- `types.ts` - Zod schemas and derived TypeScript types +- `schema.ts` - YAML parsing with Zod validation +- `graph.ts` - ArtifactGraph class with topological sort +- `state.ts` - Filesystem-based state detection +- `resolver.ts` - XDG-compliant schema resolution +- `builtin-schemas.ts` - Package-bundled default schemas -**Testable behaviors:** -- Parse schema YAML → returns correct artifact graph -- Compute build order (topological sort) → correct ordering -- Empty directory → only root artifacts (no dependencies) are ready -- Directory with `proposal.md` → `specs` becomes ready -- Directory with `specs/foo.md` → glob pattern detected as complete -- All artifacts present → `isComplete()` returns true +**Key decisions made:** +- Zod for schema validation (consistent with project) +- XDG for global schema overrides +- `Set` for completion state (immutable, functional) +- `inProgress` and `failed` states deferred (require external tracking) --- ### Slice 2: "Multi-Change Management" -**Delivers:** CRUD for changes, path resolution +**Delivers:** CRUD for changes, path resolution, name validation -**Testable behaviors:** -- `createChange('add-auth')` → creates directory + README -- `listChanges()` → returns directory names -- `getChangePath('add-auth')` → returns correct path -- Missing change → clear error message +**Scope:** +- `createChange(name, description?)` → creates directory + README +- `listChanges()` → returns directory names (excluding `archive/`) +- `getChangePath(name)` → returns absolute path +- `changeExists(name)` → checks directory existence +- `isInitialized()` → checks `openspec/changes/` existence +- Name validation → kebab-case pattern enforcement + +**Not in scope (uses existing or deferred):** +- Schema resolution → reuse `artifact-graph.resolveSchema()` +- Template resolution → Slice 3 --- @@ -417,7 +449,7 @@ Output: { **Delivers:** Template resolution + context injection **Testable behaviors:** -- Template fallback: schema-specific → shared → error +- Template fallback: schema-specific → shared → built-in → error - Context injection: completed deps show ✓, missing show ✗ - Output path shown correctly based on change directory @@ -437,32 +469,38 @@ Output: { ## Directory Structure ``` -.openspec/ -├── schemas/ # Schema definitions +# Global (XDG paths - user overrides) +~/.local/share/openspec/ # Unix/macOS ($XDG_DATA_HOME/openspec/) +%LOCALAPPDATA%/openspec/ # Windows +├── schemas/ # Schema overrides +│ └── custom-workflow.yaml # User-defined schema +└── templates/ # Template overrides (Slice 3) + └── proposal.md # Custom proposal template + +# Package (built-in defaults) +/ +├── schemas/ # Built-in schema definitions │ ├── spec-driven.yaml # Default: proposal → specs → design → tasks -│ ├── spec-driven-v2.yaml # Version 2 -│ ├── tdd.yaml # TDD: tests → implementation → docs -│ └── tdd/ -│ └── templates/ # TDD-specific template overrides -│ └── tests.md -│ -└── templates/ # Shared instruction templates +│ └── tdd.yaml # TDD: tests → implementation → docs +└── templates/ # Built-in templates (Slice 3) ├── proposal.md ├── design.md ├── specs.md └── tasks.md +# Project (change instances) openspec/ └── changes/ # Change instances ├── add-auth/ - │ ├── README.md + │ ├── README.md # Auto-generated on creation │ ├── proposal.md # Created artifacts │ ├── design.md │ └── specs/ │ └── *.md - │ - └── refactor-db/ - └── ... + ├── refactor-db/ + │ └── ... + └── archive/ # Completed changes + └── 2025-01-01-add-auth/ .claude/ ├── settings.local.json # Permissions @@ -475,7 +513,8 @@ openspec/ ## Schema YAML Format ```yaml -# .openspec/schemas/spec-driven.yaml +# Built-in: /schemas/spec-driven.yaml +# Or user override: ~/.local/share/openspec/schemas/spec-driven.yaml name: spec-driven version: 1 description: Specification-driven development @@ -484,7 +523,7 @@ artifacts: - id: proposal generates: "proposal.md" description: "Create project proposal document" - template: "proposal.md" # resolves via 2-level fallback + template: "proposal.md" # resolves via 2-level fallback (Slice 3) requires: [] - id: specs @@ -514,17 +553,18 @@ artifacts: ## Summary -| Layer | Component | Responsibility | -|-------|-----------|----------------| -| Core | ArtifactGraph | Pure dependency logic (no I/O) | -| Core | ChangeManager | Multi-change orchestration | -| Core | InstructionLoader | State detection + enrichment | -| Presentation | CLI | Thin command wrapper | -| Integration | Claude Commands | AI assistant glue | +| Layer | Component | Responsibility | Status | +|-------|-----------|----------------|--------| +| Core | ArtifactGraph | Pure dependency logic + XDG schema resolution | ✅ Slice 1 | +| Core | ChangeManager | Multi-change CRUD + path resolution | Slice 2 | +| Core | InstructionLoader | Template resolution + enrichment | Slice 3 | +| Presentation | CLI | Thin command wrapper | Slice 4 | +| Integration | Claude Commands | AI assistant glue | Slice 4 | **Key Principles:** - **Filesystem IS the database** - stateless, version-control friendly - **Dependencies are enablers** - show what's possible, don't force order - **Deterministic CLI, inferring agent** - CLI requires explicit `--change`, agent infers from context -- **2-level template inheritance** - shared + override, no deeper +- **XDG-compliant paths** - schemas and templates use standard user data directories +- **2-level inheritance** - user override → package built-in (no deeper) - **Schemas are versioned** - support variations by philosophy, version, language diff --git a/openspec/changes/add-change-manager/design.md b/openspec/changes/add-change-manager/design.md new file mode 100644 index 00000000..4f6c8be2 --- /dev/null +++ b/openspec/changes/add-change-manager/design.md @@ -0,0 +1,178 @@ +## Context + +This is Slice 2 of the artifact tracker POC. The goal is to provide a reusable `ChangeManager` module for change directory operations. + +**Current state:** Change logic is duplicated across CLI commands: +- `ListCommand` (`src/core/list.ts:17-61`) - scans directories, excludes archive +- `ChangeCommand` (`src/commands/change.ts:242-260`) - `getActiveChanges()` duplicates this +- Path resolution via inline `path.join()` scattered throughout +- Existence checks via inline `fs.access()` scattered throughout + +**Proposed state:** Single `ChangeManager` module that CLI commands delegate to. + +## Goals / Non-Goals + +### Goals +- **Extract** duplicated logic into reusable module +- **Add** new functionality: `createChange()`, name validation +- **Refactor** CLI commands to be thin wrappers +- **Enable** artifact-graph Slices 3/4 to reuse this module +- **Maintain** existing CLI behavior (no breaking changes) + +### Non-Goals +- Change CLI command interfaces +- Add new CLI commands (that's Slice 4) +- Modify validation logic (stays in `Validator` class) +- Modify parsing logic (stays in `ChangeParser` class) + +## Decisions + +### Decision 1: Extract to Separate Module + +**Choice**: Create `src/core/change-manager/` as a new module directory. + +``` +src/core/change-manager/ +├── index.ts # Public exports +├── change-manager.ts # ChangeManager class +└── validation.ts # Name validation utilities +``` + +**Why**: +- Follows pattern established by `artifact-graph/` +- Clear separation from CLI commands +- Testable in isolation +- Can be imported by other modules + +**Alternatives considered**: +- Add methods to existing `ChangeCommand`: Rejected - mixes core logic with CLI +- Create single file `src/core/change-manager.ts`: Acceptable, but directory allows growth + +### Decision 2: Class-Based API with Explicit Project Root + +**Choice**: `ChangeManager` class instantiated with `projectRoot` path. + +```typescript +class ChangeManager { + constructor(options: { projectRoot: string }) + + // Extracted methods + listChanges(): Promise + changeExists(name: string): boolean + getChangePath(name: string): string + isInitialized(): boolean + + // New methods + createChange(name: string, description?: string): Promise + validateName(name: string): { valid: boolean; error?: string } +} +``` + +**Why**: +- Explicit `projectRoot` makes testing trivial +- Matches pattern used in `artifact-graph` module +- Avoids `process.cwd()` coupling + +### Decision 3: Preserve Existing Behavior During Extraction + +**Choice**: Extracted methods MUST behave identically to current implementations. + +| Method | Source | Behavior to Preserve | +|--------|--------|---------------------| +| `listChanges()` | `ListCommand` + `getActiveChanges()` | Exclude `archive/`, filter to dirs, sort alphabetically | +| `changeExists()` | Inline `fs.access()` | Synchronous-style check (can be sync or async) | +| `getChangePath()` | Inline `path.join()` | Return absolute path, throw if not exists | + +**Why**: +- No behavioral changes = no risk of breaking existing CLI +- Tests can verify parity with current behavior +- Refactor is purely structural + +### Decision 4: CLI Commands Become Thin Wrappers + +**Choice**: After refactor, CLI commands only handle: +- User interaction (prompts, selection) +- Output formatting (console.log, JSON) +- Error presentation + +```typescript +// Before (in ChangeCommand) +async list() { + const entries = await fs.readdir(changesPath, { withFileTypes: true }); + const result = entries.filter(e => e.isDirectory() && e.name !== 'archive'); + // ... more logic +} + +// After +async list() { + const changes = await this.changeManager.listChanges(); + // Just format and output + console.log(JSON.stringify(changes)); +} +``` + +**Why**: +- Clear separation of concerns +- Core logic testable without CLI +- Consistent pattern across commands + +### Decision 5: Kebab-Case Validation Pattern + +**Choice**: Validate names with `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` + +Valid: `add-auth`, `refactor-db`, `add-feature-2`, `refactor` +Invalid: `Add-Auth`, `add auth`, `add_auth`, `-add-auth`, `add-auth-`, `add--auth` + +**Why**: +- Filesystem-safe (no special characters) +- URL-safe (for future web UI) +- Consistent with existing change naming conventions in repo +- Prevents common mistakes (spaces, underscores) + +### Decision 6: README.md for Change Metadata + +**Choice**: `createChange()` generates a `README.md` with name and optional description. + +```markdown +# add-auth + +Add user authentication system +``` + +**Why**: +- Human-readable in GitHub/GitLab UI +- Minimal overhead +- Matches existing manual change creation patterns + +## File Changes + +### New Files +- `src/core/change-manager/index.ts` - Public exports +- `src/core/change-manager/change-manager.ts` - Class implementation +- `src/core/change-manager/validation.ts` - Name validation +- `src/core/change-manager/change-manager.test.ts` - Unit tests + +### Modified Files +- `src/core/list.ts` - Import and use ChangeManager +- `src/commands/change.ts` - Import and use ChangeManager +- `src/core/index.ts` - Export ChangeManager + +## Risks / Trade-offs + +| Risk | Mitigation | +|------|------------| +| Behavioral changes during extraction | Write tests against current behavior FIRST, then refactor | +| Breaking CLI commands | No interface changes, only internal refactor | +| Circular dependencies | ChangeManager has no dependencies on CLI layer | + +## Migration Plan + +1. **Phase 1**: Create `ChangeManager` with extracted methods + tests +2. **Phase 2**: Add new methods (`createChange`, `validateName`) + tests +3. **Phase 3**: Refactor `ListCommand` to use ChangeManager +4. **Phase 4**: Refactor `ChangeCommand` to use ChangeManager +5. **Phase 5**: Verify all existing tests pass + +## Open Questions + +None - all questions resolved. diff --git a/openspec/changes/add-change-manager/proposal.md b/openspec/changes/add-change-manager/proposal.md new file mode 100644 index 00000000..84a73b57 --- /dev/null +++ b/openspec/changes/add-change-manager/proposal.md @@ -0,0 +1,58 @@ +## Why + +Change management logic is currently duplicated across multiple CLI commands: +- `ListCommand` (`src/core/list.ts`) scans `openspec/changes/`, excludes `archive/` +- `ChangeCommand` (`src/commands/change.ts`) has `getActiveChanges()` doing the same +- Path resolution and existence checks are inline `fs.access()` / `path.join()` calls +- No centralized place for change CRUD operations + +This makes the code harder to maintain and prevents reuse by the artifact-graph system (Slices 3/4). + +**This proposal:** +1. **Extracts** existing change logic into a reusable `ChangeManager` module +2. **Adds** new functionality: `createChange()` and kebab-case name validation +3. **Refactors** CLI commands to use the new module (thin wrappers) + +## What Changes + +### New Module: `ChangeManager` (`src/core/change-manager/`) + +A core module consolidating all change directory operations: + +| Method | Source | Description | +|--------|--------|-------------| +| `listChanges()` | **Extract** | From `ListCommand` + `ChangeCommand.getActiveChanges()` | +| `changeExists(name)` | **Extract** | From inline `fs.access()` checks | +| `getChangePath(name)` | **Extract** | From inline `path.join()` logic | +| `isInitialized()` | **Extract** | From `ListCommand` directory check | +| `createChange(name, desc?)` | **NEW** | Create change directory + README | +| `validateName(name)` | **NEW** | Enforce kebab-case naming | + +### Refactored Commands (thin wrappers) + +| Command | Before | After | +|---------|--------|-------| +| `ListCommand.execute()` | Inline fs scanning | Calls `changeManager.listChanges()` | +| `ChangeCommand.list()` | Inline `getActiveChanges()` | Calls `changeManager.listChanges()` | +| `ChangeCommand.show()` | Inline path building | Calls `changeManager.getChangePath()` | +| `ChangeCommand.validate()` | Inline path building | Calls `changeManager.getChangePath()` | + +### Key Design Decisions + +- **Single source of truth** - All change operations go through ChangeManager +- **CLI becomes thin** - Commands handle formatting/interaction only +- **Testable core** - ChangeManager is pure, no CLI coupling +- **Reusable** - artifact-graph Slices 3/4 can import and use it + +## Impact + +- **Affected specs**: None (internal refactor + new capability) +- **New spec**: `change-manager` - Consolidated change operations +- **Affected code**: + - New: `src/core/change-manager/index.ts` - Main module + - New: `src/core/change-manager/change-manager.ts` - Class implementation + - New: `src/core/change-manager/validation.ts` - Name validation + - Refactor: `src/core/list.ts` - Use ChangeManager + - Refactor: `src/commands/change.ts` - Use ChangeManager +- **Dependencies**: Used by `artifact-graph` for Slices 3/4 +- **Breaking changes**: None (CLI interface unchanged) diff --git a/openspec/changes/add-change-manager/specs/change-manager/spec.md b/openspec/changes/add-change-manager/specs/change-manager/spec.md new file mode 100644 index 00000000..fbf2e854 --- /dev/null +++ b/openspec/changes/add-change-manager/specs/change-manager/spec.md @@ -0,0 +1,137 @@ +## ADDED Requirements + +### Requirement: Change Listing +The system SHALL provide a centralized method to list all active changes in a project. + +**Note:** Extracted from `ListCommand.execute()` and `ChangeCommand.getActiveChanges()`. + +#### Scenario: List changes with multiple changes +- **WHEN** `openspec/changes/` contains directories `add-auth/` and `refactor-db/` +- **THEN** `listChanges()` returns `['add-auth', 'refactor-db']` + +#### Scenario: List changes with no changes +- **WHEN** `openspec/changes/` is empty or does not exist +- **THEN** `listChanges()` returns an empty array + +#### Scenario: List changes ignores files +- **WHEN** `openspec/changes/` contains a file `notes.txt` alongside directory `add-auth/` +- **THEN** `listChanges()` returns only `['add-auth']` + +#### Scenario: List changes ignores archive directory +- **WHEN** `openspec/changes/` contains `add-auth/` and `archive/` +- **THEN** `listChanges()` returns only `['add-auth']` (excludes archive) + +#### Scenario: List changes returns sorted results +- **WHEN** `openspec/changes/` contains `zebra/`, `alpha/`, `beta/` +- **THEN** `listChanges()` returns `['alpha', 'beta', 'zebra']` (alphabetically sorted) + +### Requirement: Change Path Resolution +The system SHALL resolve absolute paths to change directories. + +**Note:** Extracted from inline `path.join()` calls in CLI commands. + +#### Scenario: Existing change path +- **WHEN** `getChangePath('add-auth')` is called and the change exists +- **THEN** returns the absolute path to `openspec/changes/add-auth/` + +#### Scenario: Non-existent change path +- **WHEN** `getChangePath('nonexistent')` is called and the change does not exist +- **THEN** throws an error indicating the change was not found with available changes listed + +### Requirement: Change Existence Check +The system SHALL check whether a change directory exists. + +**Note:** Extracted from inline `fs.access()` checks in CLI commands. + +#### Scenario: Change exists +- **WHEN** `changeExists('add-auth')` is called and the directory exists +- **THEN** returns true + +#### Scenario: Change does not exist +- **WHEN** `changeExists('nonexistent')` is called and the directory does not exist +- **THEN** returns false + +### Requirement: Initialization Check +The system SHALL detect whether a project has an OpenSpec changes directory. + +**Note:** Extracted from `ListCommand` directory access check. + +#### Scenario: Initialized project +- **WHEN** the `openspec/changes/` directory exists at the project root +- **THEN** `isInitialized()` returns true + +#### Scenario: Uninitialized project +- **WHEN** the `openspec/changes/` directory does not exist +- **THEN** `isInitialized()` returns false + +### Requirement: Change Creation +The system SHALL create new change directories with proper structure. + +**Note:** New functionality - does not exist in current codebase. + +#### Scenario: Create change with name only +- **WHEN** `createChange('add-auth')` is called +- **THEN** the system creates `openspec/changes/add-auth/` directory +- **AND** creates a `README.md` file with the change name as title + +#### Scenario: Create change with description +- **WHEN** `createChange('add-auth', 'Add user authentication')` is called +- **THEN** the system creates `openspec/changes/add-auth/` directory +- **AND** creates a `README.md` file with the change name and description + +#### Scenario: Duplicate change rejected +- **WHEN** `createChange('add-auth')` is called and `openspec/changes/add-auth/` already exists +- **THEN** the system throws an error indicating the change already exists + +#### Scenario: Creates parent directories if needed +- **WHEN** `createChange('add-auth')` is called and `openspec/changes/` does not exist +- **THEN** the system creates the full path including parent directories + +#### Scenario: Invalid change name rejected +- **WHEN** `createChange('')` is called with an empty name +- **THEN** the system throws an error indicating invalid name + +### Requirement: Change Name Validation +The system SHALL validate change names follow kebab-case conventions. + +**Note:** New functionality - names are not currently validated. + +#### Scenario: Valid kebab-case name accepted +- **WHEN** a change name like `add-user-auth` is validated +- **THEN** validation passes + +#### Scenario: Numeric suffixes accepted +- **WHEN** a change name like `add-feature-2` is validated +- **THEN** validation passes + +#### Scenario: Single word accepted +- **WHEN** a change name like `refactor` is validated +- **THEN** validation passes + +#### Scenario: Uppercase characters rejected +- **WHEN** a change name like `Add-Auth` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Spaces rejected +- **WHEN** a change name like `add auth` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Underscores rejected +- **WHEN** a change name like `add_auth` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Special characters rejected +- **WHEN** a change name like `add-auth!` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Leading hyphen rejected +- **WHEN** a change name like `-add-auth` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Trailing hyphen rejected +- **WHEN** a change name like `add-auth-` is validated +- **THEN** validation fails with descriptive error + +#### Scenario: Consecutive hyphens rejected +- **WHEN** a change name like `add--auth` is validated +- **THEN** validation fails with descriptive error diff --git a/openspec/changes/add-change-manager/tasks.md b/openspec/changes/add-change-manager/tasks.md new file mode 100644 index 00000000..1afba8fa --- /dev/null +++ b/openspec/changes/add-change-manager/tasks.md @@ -0,0 +1,81 @@ +## Phase 1: Create ChangeManager Module Structure + +- [ ] 1.1 Create `src/core/change-manager/` directory +- [ ] 1.2 Create `src/core/change-manager/index.ts` with public exports +- [ ] 1.3 Create `src/core/change-manager/change-manager.ts` with class skeleton +- [ ] 1.4 Create `src/core/change-manager/validation.ts` for name validation +- [ ] 1.5 Export `ChangeManager` from `src/core/index.ts` + +## Phase 2: Extract Existing Functionality + +### 2.1 Extract `listChanges()` +- [ ] 2.1.1 Implement `listChanges()` matching `ListCommand` + `getActiveChanges()` behavior +- [ ] 2.1.2 Exclude `archive/` directory +- [ ] 2.1.3 Filter to directories only (ignore files) +- [ ] 2.1.4 Return sorted array +- [ ] 2.1.5 Add test: multiple changes returns sorted array +- [ ] 2.1.6 Add test: empty directory returns empty array +- [ ] 2.1.7 Add test: ignores files and archive + +### 2.2 Extract `changeExists()` +- [ ] 2.2.1 Implement `changeExists(name)` checking directory existence +- [ ] 2.2.2 Add test: existing change returns true +- [ ] 2.2.3 Add test: non-existent change returns false + +### 2.3 Extract `getChangePath()` +- [ ] 2.3.1 Implement `getChangePath(name)` returning absolute path +- [ ] 2.3.2 Throw error with available changes if not found +- [ ] 2.3.3 Add test: existing change returns absolute path +- [ ] 2.3.4 Add test: non-existent change throws descriptive error + +### 2.4 Extract `isInitialized()` +- [ ] 2.4.1 Implement `isInitialized()` checking `openspec/changes/` exists +- [ ] 2.4.2 Add test: initialized project returns true +- [ ] 2.4.3 Add test: uninitialized project returns false + +## Phase 3: Add New Functionality + +### 3.1 Add Name Validation +- [ ] 3.1.1 Implement `validateName(name)` with kebab-case pattern +- [ ] 3.1.2 Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` +- [ ] 3.1.3 Return `{ valid: boolean; error?: string }` +- [ ] 3.1.4 Add test: valid names accepted (`add-auth`, `refactor`, `add-feature-2`) +- [ ] 3.1.5 Add test: uppercase rejected +- [ ] 3.1.6 Add test: spaces rejected +- [ ] 3.1.7 Add test: underscores rejected +- [ ] 3.1.8 Add test: special characters rejected +- [ ] 3.1.9 Add test: leading/trailing hyphens rejected +- [ ] 3.1.10 Add test: consecutive hyphens rejected + +### 3.2 Add `createChange()` +- [ ] 3.2.1 Implement `createChange(name, description?)` method +- [ ] 3.2.2 Validate name before creating +- [ ] 3.2.3 Create parent directories if needed (`openspec/changes/`) +- [ ] 3.2.4 Generate README.md with name and optional description +- [ ] 3.2.5 Throw if change already exists +- [ ] 3.2.6 Add test: creates directory and README with name only +- [ ] 3.2.7 Add test: creates directory and README with description +- [ ] 3.2.8 Add test: duplicate change throws error +- [ ] 3.2.9 Add test: invalid name throws validation error +- [ ] 3.2.10 Add test: creates parent directories if needed + +## Phase 4: Refactor CLI Commands + +### 4.1 Refactor `ListCommand` +- [ ] 4.1.1 Import `ChangeManager` in `src/core/list.ts` +- [ ] 4.1.2 Replace inline directory scanning with `changeManager.listChanges()` +- [ ] 4.1.3 Verify existing behavior unchanged + +### 4.2 Refactor `ChangeCommand` +- [ ] 4.2.1 Import `ChangeManager` in `src/commands/change.ts` +- [ ] 4.2.2 Replace `getActiveChanges()` with `changeManager.listChanges()` +- [ ] 4.2.3 Replace inline path building with `changeManager.getChangePath()` +- [ ] 4.2.4 Remove `getActiveChanges()` private method +- [ ] 4.2.5 Verify existing behavior unchanged + +## Phase 5: Integration and Cleanup + +- [ ] 5.1 Add JSDoc comments to all public methods +- [ ] 5.2 Run existing tests to verify no regressions +- [ ] 5.3 Add integration test: full workflow (create, list, get path, exists) +- [ ] 5.4 Update any imports in other files if needed From 1c924730dc964b5e6f2d7b7234608b46c56db23e Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Thu, 25 Dec 2025 23:31:29 +1100 Subject: [PATCH 2/8] proposal: simplify to utility functions only Remove extraction/refactor scope. Just add: - createChange(projectRoot, name, description?) - validateChangeName(name) Simple utility functions in src/utils/change-utils.ts. No class, no abstraction layer. --- openspec/changes/add-change-manager/design.md | 156 ++++-------------- .../changes/add-change-manager/proposal.md | 71 ++++---- .../specs/change-manager/spec.md | 102 ++---------- openspec/changes/add-change-manager/tasks.md | 113 ++++--------- 4 files changed, 112 insertions(+), 330 deletions(-) diff --git a/openspec/changes/add-change-manager/design.md b/openspec/changes/add-change-manager/design.md index 4f6c8be2..7f360495 100644 --- a/openspec/changes/add-change-manager/design.md +++ b/openspec/changes/add-change-manager/design.md @@ -1,122 +1,52 @@ ## Context -This is Slice 2 of the artifact tracker POC. The goal is to provide a reusable `ChangeManager` module for change directory operations. +This is Slice 2 of the artifact tracker POC. The goal is to provide utilities for creating change directories programmatically. -**Current state:** Change logic is duplicated across CLI commands: -- `ListCommand` (`src/core/list.ts:17-61`) - scans directories, excludes archive -- `ChangeCommand` (`src/commands/change.ts:242-260`) - `getActiveChanges()` duplicates this -- Path resolution via inline `path.join()` scattered throughout -- Existence checks via inline `fs.access()` scattered throughout +**Current state:** No programmatic way to create changes. Users must manually create directories. -**Proposed state:** Single `ChangeManager` module that CLI commands delegate to. +**Proposed state:** Utility functions for change creation with name validation. ## Goals / Non-Goals ### Goals -- **Extract** duplicated logic into reusable module -- **Add** new functionality: `createChange()`, name validation -- **Refactor** CLI commands to be thin wrappers -- **Enable** artifact-graph Slices 3/4 to reuse this module -- **Maintain** existing CLI behavior (no breaking changes) +- **Add** `createChange()` function to create change directories +- **Add** `validateChangeName()` function for kebab-case validation +- **Enable** automation (Claude commands, scripts) to create changes ### Non-Goals -- Change CLI command interfaces -- Add new CLI commands (that's Slice 4) -- Modify validation logic (stays in `Validator` class) -- Modify parsing logic (stays in `ChangeParser` class) +- Refactor existing CLI commands (they work fine) +- Create abstraction layers or manager classes +- Change how `ListCommand` or `ChangeCommand` work ## Decisions -### Decision 1: Extract to Separate Module +### Decision 1: Simple Utility Functions -**Choice**: Create `src/core/change-manager/` as a new module directory. - -``` -src/core/change-manager/ -├── index.ts # Public exports -├── change-manager.ts # ChangeManager class -└── validation.ts # Name validation utilities -``` - -**Why**: -- Follows pattern established by `artifact-graph/` -- Clear separation from CLI commands -- Testable in isolation -- Can be imported by other modules - -**Alternatives considered**: -- Add methods to existing `ChangeCommand`: Rejected - mixes core logic with CLI -- Create single file `src/core/change-manager.ts`: Acceptable, but directory allows growth - -### Decision 2: Class-Based API with Explicit Project Root - -**Choice**: `ChangeManager` class instantiated with `projectRoot` path. +**Choice**: Add functions to `src/utils/change-utils.ts` - no class. ```typescript -class ChangeManager { - constructor(options: { projectRoot: string }) - - // Extracted methods - listChanges(): Promise - changeExists(name: string): boolean - getChangePath(name: string): string - isInitialized(): boolean - - // New methods - createChange(name: string, description?: string): Promise - validateName(name: string): { valid: boolean; error?: string } -} -``` - -**Why**: -- Explicit `projectRoot` makes testing trivial -- Matches pattern used in `artifact-graph` module -- Avoids `process.cwd()` coupling - -### Decision 3: Preserve Existing Behavior During Extraction +// src/utils/change-utils.ts -**Choice**: Extracted methods MUST behave identically to current implementations. +export function validateChangeName(name: string): { valid: boolean; error?: string } -| Method | Source | Behavior to Preserve | -|--------|--------|---------------------| -| `listChanges()` | `ListCommand` + `getActiveChanges()` | Exclude `archive/`, filter to dirs, sort alphabetically | -| `changeExists()` | Inline `fs.access()` | Synchronous-style check (can be sync or async) | -| `getChangePath()` | Inline `path.join()` | Return absolute path, throw if not exists | - -**Why**: -- No behavioral changes = no risk of breaking existing CLI -- Tests can verify parity with current behavior -- Refactor is purely structural - -### Decision 4: CLI Commands Become Thin Wrappers - -**Choice**: After refactor, CLI commands only handle: -- User interaction (prompts, selection) -- Output formatting (console.log, JSON) -- Error presentation - -```typescript -// Before (in ChangeCommand) -async list() { - const entries = await fs.readdir(changesPath, { withFileTypes: true }); - const result = entries.filter(e => e.isDirectory() && e.name !== 'archive'); - // ... more logic -} - -// After -async list() { - const changes = await this.changeManager.listChanges(); - // Just format and output - console.log(JSON.stringify(changes)); -} +export async function createChange( + projectRoot: string, + name: string, + description?: string +): Promise ``` **Why**: -- Clear separation of concerns -- Core logic testable without CLI -- Consistent pattern across commands +- Simple, no abstraction overhead +- Easy to test +- Easy to import where needed +- Matches existing utility patterns in `src/utils/` -### Decision 5: Kebab-Case Validation Pattern +**Alternatives considered**: +- ChangeManager class: Rejected - over-engineered for 2 functions +- Add to existing command: Rejected - mixes CLI with reusable logic + +### Decision 2: Kebab-Case Validation Pattern **Choice**: Validate names with `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` @@ -126,10 +56,9 @@ Invalid: `Add-Auth`, `add auth`, `add_auth`, `-add-auth`, `add-auth-`, `add--aut **Why**: - Filesystem-safe (no special characters) - URL-safe (for future web UI) -- Consistent with existing change naming conventions in repo -- Prevents common mistakes (spaces, underscores) +- Consistent with existing change naming in repo -### Decision 6: README.md for Change Metadata +### Decision 3: README.md for Change Metadata **Choice**: `createChange()` generates a `README.md` with name and optional description. @@ -147,32 +76,15 @@ Add user authentication system ## File Changes ### New Files -- `src/core/change-manager/index.ts` - Public exports -- `src/core/change-manager/change-manager.ts` - Class implementation -- `src/core/change-manager/validation.ts` - Name validation -- `src/core/change-manager/change-manager.test.ts` - Unit tests +- `src/utils/change-utils.ts` - Utility functions +- `src/utils/change-utils.test.ts` - Unit tests ### Modified Files -- `src/core/list.ts` - Import and use ChangeManager -- `src/commands/change.ts` - Import and use ChangeManager -- `src/core/index.ts` - Export ChangeManager +- None ## Risks / Trade-offs | Risk | Mitigation | |------|------------| -| Behavioral changes during extraction | Write tests against current behavior FIRST, then refactor | -| Breaking CLI commands | No interface changes, only internal refactor | -| Circular dependencies | ChangeManager has no dependencies on CLI layer | - -## Migration Plan - -1. **Phase 1**: Create `ChangeManager` with extracted methods + tests -2. **Phase 2**: Add new methods (`createChange`, `validateName`) + tests -3. **Phase 3**: Refactor `ListCommand` to use ChangeManager -4. **Phase 4**: Refactor `ChangeCommand` to use ChangeManager -5. **Phase 5**: Verify all existing tests pass - -## Open Questions - -None - all questions resolved. +| Function might not cover all use cases | Start simple, extend if needed | +| Naming conflicts with future work | Using clear, specific function names | diff --git a/openspec/changes/add-change-manager/proposal.md b/openspec/changes/add-change-manager/proposal.md index 84a73b57..119fecce 100644 --- a/openspec/changes/add-change-manager/proposal.md +++ b/openspec/changes/add-change-manager/proposal.md @@ -1,58 +1,45 @@ ## Why -Change management logic is currently duplicated across multiple CLI commands: -- `ListCommand` (`src/core/list.ts`) scans `openspec/changes/`, excludes `archive/` -- `ChangeCommand` (`src/commands/change.ts`) has `getActiveChanges()` doing the same -- Path resolution and existence checks are inline `fs.access()` / `path.join()` calls -- No centralized place for change CRUD operations +There's no programmatic way to create a new change directory. Users must manually: +1. Create `openspec/changes//` directory +2. Create a `proposal.md` file +3. Hope they got the naming right -This makes the code harder to maintain and prevents reuse by the artifact-graph system (Slices 3/4). +This is error-prone and blocks automation (e.g., Claude commands, scripts). -**This proposal:** -1. **Extracts** existing change logic into a reusable `ChangeManager` module -2. **Adds** new functionality: `createChange()` and kebab-case name validation -3. **Refactors** CLI commands to use the new module (thin wrappers) +**This proposal adds:** +1. `createChange(name, description?)` - Create change directories programmatically +2. `validateChangeName(name)` - Enforce kebab-case naming conventions ## What Changes -### New Module: `ChangeManager` (`src/core/change-manager/`) +### New Utilities -A core module consolidating all change directory operations: +| Function | Description | +|----------|-------------| +| `createChange(name, description?)` | Creates `openspec/changes//` with README.md | +| `validateChangeName(name)` | Returns `{ valid: boolean; error?: string }` | -| Method | Source | Description | -|--------|--------|-------------| -| `listChanges()` | **Extract** | From `ListCommand` + `ChangeCommand.getActiveChanges()` | -| `changeExists(name)` | **Extract** | From inline `fs.access()` checks | -| `getChangePath(name)` | **Extract** | From inline `path.join()` logic | -| `isInitialized()` | **Extract** | From `ListCommand` directory check | -| `createChange(name, desc?)` | **NEW** | Create change directory + README | -| `validateName(name)` | **NEW** | Enforce kebab-case naming | +### Name Validation Rules -### Refactored Commands (thin wrappers) +Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` -| Command | Before | After | -|---------|--------|-------| -| `ListCommand.execute()` | Inline fs scanning | Calls `changeManager.listChanges()` | -| `ChangeCommand.list()` | Inline `getActiveChanges()` | Calls `changeManager.listChanges()` | -| `ChangeCommand.show()` | Inline path building | Calls `changeManager.getChangePath()` | -| `ChangeCommand.validate()` | Inline path building | Calls `changeManager.getChangePath()` | +| Valid | Invalid | +|-------|---------| +| `add-auth` | `Add-Auth` (uppercase) | +| `refactor-db` | `add auth` (spaces) | +| `add-feature-2` | `add_auth` (underscores) | +| `refactor` | `-add-auth` (leading hyphen) | -### Key Design Decisions +### Location -- **Single source of truth** - All change operations go through ChangeManager -- **CLI becomes thin** - Commands handle formatting/interaction only -- **Testable core** - ChangeManager is pure, no CLI coupling -- **Reusable** - artifact-graph Slices 3/4 can import and use it +New file: `src/utils/change-utils.ts` + +Simple utility functions - no class, no abstraction layer. ## Impact -- **Affected specs**: None (internal refactor + new capability) -- **New spec**: `change-manager` - Consolidated change operations -- **Affected code**: - - New: `src/core/change-manager/index.ts` - Main module - - New: `src/core/change-manager/change-manager.ts` - Class implementation - - New: `src/core/change-manager/validation.ts` - Name validation - - Refactor: `src/core/list.ts` - Use ChangeManager - - Refactor: `src/commands/change.ts` - Use ChangeManager -- **Dependencies**: Used by `artifact-graph` for Slices 3/4 -- **Breaking changes**: None (CLI interface unchanged) +- **Affected specs**: None +- **Affected code**: None (new utilities only) +- **New files**: `src/utils/change-utils.ts` +- **Breaking changes**: None diff --git a/openspec/changes/add-change-manager/specs/change-manager/spec.md b/openspec/changes/add-change-manager/specs/change-manager/spec.md index fbf2e854..efb81d09 100644 --- a/openspec/changes/add-change-manager/specs/change-manager/spec.md +++ b/openspec/changes/add-change-manager/specs/change-manager/spec.md @@ -1,137 +1,69 @@ ## ADDED Requirements -### Requirement: Change Listing -The system SHALL provide a centralized method to list all active changes in a project. - -**Note:** Extracted from `ListCommand.execute()` and `ChangeCommand.getActiveChanges()`. - -#### Scenario: List changes with multiple changes -- **WHEN** `openspec/changes/` contains directories `add-auth/` and `refactor-db/` -- **THEN** `listChanges()` returns `['add-auth', 'refactor-db']` - -#### Scenario: List changes with no changes -- **WHEN** `openspec/changes/` is empty or does not exist -- **THEN** `listChanges()` returns an empty array - -#### Scenario: List changes ignores files -- **WHEN** `openspec/changes/` contains a file `notes.txt` alongside directory `add-auth/` -- **THEN** `listChanges()` returns only `['add-auth']` - -#### Scenario: List changes ignores archive directory -- **WHEN** `openspec/changes/` contains `add-auth/` and `archive/` -- **THEN** `listChanges()` returns only `['add-auth']` (excludes archive) - -#### Scenario: List changes returns sorted results -- **WHEN** `openspec/changes/` contains `zebra/`, `alpha/`, `beta/` -- **THEN** `listChanges()` returns `['alpha', 'beta', 'zebra']` (alphabetically sorted) - -### Requirement: Change Path Resolution -The system SHALL resolve absolute paths to change directories. - -**Note:** Extracted from inline `path.join()` calls in CLI commands. - -#### Scenario: Existing change path -- **WHEN** `getChangePath('add-auth')` is called and the change exists -- **THEN** returns the absolute path to `openspec/changes/add-auth/` - -#### Scenario: Non-existent change path -- **WHEN** `getChangePath('nonexistent')` is called and the change does not exist -- **THEN** throws an error indicating the change was not found with available changes listed - -### Requirement: Change Existence Check -The system SHALL check whether a change directory exists. - -**Note:** Extracted from inline `fs.access()` checks in CLI commands. - -#### Scenario: Change exists -- **WHEN** `changeExists('add-auth')` is called and the directory exists -- **THEN** returns true - -#### Scenario: Change does not exist -- **WHEN** `changeExists('nonexistent')` is called and the directory does not exist -- **THEN** returns false - -### Requirement: Initialization Check -The system SHALL detect whether a project has an OpenSpec changes directory. - -**Note:** Extracted from `ListCommand` directory access check. - -#### Scenario: Initialized project -- **WHEN** the `openspec/changes/` directory exists at the project root -- **THEN** `isInitialized()` returns true - -#### Scenario: Uninitialized project -- **WHEN** the `openspec/changes/` directory does not exist -- **THEN** `isInitialized()` returns false - ### Requirement: Change Creation -The system SHALL create new change directories with proper structure. - -**Note:** New functionality - does not exist in current codebase. +The system SHALL provide a function to create new change directories programmatically. #### Scenario: Create change with name only -- **WHEN** `createChange('add-auth')` is called +- **WHEN** `createChange(projectRoot, 'add-auth')` is called - **THEN** the system creates `openspec/changes/add-auth/` directory - **AND** creates a `README.md` file with the change name as title #### Scenario: Create change with description -- **WHEN** `createChange('add-auth', 'Add user authentication')` is called +- **WHEN** `createChange(projectRoot, 'add-auth', 'Add user authentication')` is called - **THEN** the system creates `openspec/changes/add-auth/` directory - **AND** creates a `README.md` file with the change name and description #### Scenario: Duplicate change rejected -- **WHEN** `createChange('add-auth')` is called and `openspec/changes/add-auth/` already exists +- **WHEN** `createChange(projectRoot, 'add-auth')` is called and `openspec/changes/add-auth/` already exists - **THEN** the system throws an error indicating the change already exists #### Scenario: Creates parent directories if needed -- **WHEN** `createChange('add-auth')` is called and `openspec/changes/` does not exist +- **WHEN** `createChange(projectRoot, 'add-auth')` is called and `openspec/changes/` does not exist - **THEN** the system creates the full path including parent directories #### Scenario: Invalid change name rejected -- **WHEN** `createChange('')` is called with an empty name -- **THEN** the system throws an error indicating invalid name +- **WHEN** `createChange(projectRoot, 'Add Auth')` is called with an invalid name +- **THEN** the system throws a validation error ### Requirement: Change Name Validation The system SHALL validate change names follow kebab-case conventions. -**Note:** New functionality - names are not currently validated. - #### Scenario: Valid kebab-case name accepted - **WHEN** a change name like `add-user-auth` is validated -- **THEN** validation passes +- **THEN** validation returns `{ valid: true }` #### Scenario: Numeric suffixes accepted - **WHEN** a change name like `add-feature-2` is validated -- **THEN** validation passes +- **THEN** validation returns `{ valid: true }` #### Scenario: Single word accepted - **WHEN** a change name like `refactor` is validated -- **THEN** validation passes +- **THEN** validation returns `{ valid: true }` #### Scenario: Uppercase characters rejected - **WHEN** a change name like `Add-Auth` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Spaces rejected - **WHEN** a change name like `add auth` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Underscores rejected - **WHEN** a change name like `add_auth` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Special characters rejected - **WHEN** a change name like `add-auth!` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Leading hyphen rejected - **WHEN** a change name like `-add-auth` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Trailing hyphen rejected - **WHEN** a change name like `add-auth-` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` #### Scenario: Consecutive hyphens rejected - **WHEN** a change name like `add--auth` is validated -- **THEN** validation fails with descriptive error +- **THEN** validation returns `{ valid: false, error: "..." }` diff --git a/openspec/changes/add-change-manager/tasks.md b/openspec/changes/add-change-manager/tasks.md index 1afba8fa..e6571e5b 100644 --- a/openspec/changes/add-change-manager/tasks.md +++ b/openspec/changes/add-change-manager/tasks.md @@ -1,81 +1,32 @@ -## Phase 1: Create ChangeManager Module Structure - -- [ ] 1.1 Create `src/core/change-manager/` directory -- [ ] 1.2 Create `src/core/change-manager/index.ts` with public exports -- [ ] 1.3 Create `src/core/change-manager/change-manager.ts` with class skeleton -- [ ] 1.4 Create `src/core/change-manager/validation.ts` for name validation -- [ ] 1.5 Export `ChangeManager` from `src/core/index.ts` - -## Phase 2: Extract Existing Functionality - -### 2.1 Extract `listChanges()` -- [ ] 2.1.1 Implement `listChanges()` matching `ListCommand` + `getActiveChanges()` behavior -- [ ] 2.1.2 Exclude `archive/` directory -- [ ] 2.1.3 Filter to directories only (ignore files) -- [ ] 2.1.4 Return sorted array -- [ ] 2.1.5 Add test: multiple changes returns sorted array -- [ ] 2.1.6 Add test: empty directory returns empty array -- [ ] 2.1.7 Add test: ignores files and archive - -### 2.2 Extract `changeExists()` -- [ ] 2.2.1 Implement `changeExists(name)` checking directory existence -- [ ] 2.2.2 Add test: existing change returns true -- [ ] 2.2.3 Add test: non-existent change returns false - -### 2.3 Extract `getChangePath()` -- [ ] 2.3.1 Implement `getChangePath(name)` returning absolute path -- [ ] 2.3.2 Throw error with available changes if not found -- [ ] 2.3.3 Add test: existing change returns absolute path -- [ ] 2.3.4 Add test: non-existent change throws descriptive error - -### 2.4 Extract `isInitialized()` -- [ ] 2.4.1 Implement `isInitialized()` checking `openspec/changes/` exists -- [ ] 2.4.2 Add test: initialized project returns true -- [ ] 2.4.3 Add test: uninitialized project returns false - -## Phase 3: Add New Functionality - -### 3.1 Add Name Validation -- [ ] 3.1.1 Implement `validateName(name)` with kebab-case pattern -- [ ] 3.1.2 Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` -- [ ] 3.1.3 Return `{ valid: boolean; error?: string }` -- [ ] 3.1.4 Add test: valid names accepted (`add-auth`, `refactor`, `add-feature-2`) -- [ ] 3.1.5 Add test: uppercase rejected -- [ ] 3.1.6 Add test: spaces rejected -- [ ] 3.1.7 Add test: underscores rejected -- [ ] 3.1.8 Add test: special characters rejected -- [ ] 3.1.9 Add test: leading/trailing hyphens rejected -- [ ] 3.1.10 Add test: consecutive hyphens rejected - -### 3.2 Add `createChange()` -- [ ] 3.2.1 Implement `createChange(name, description?)` method -- [ ] 3.2.2 Validate name before creating -- [ ] 3.2.3 Create parent directories if needed (`openspec/changes/`) -- [ ] 3.2.4 Generate README.md with name and optional description -- [ ] 3.2.5 Throw if change already exists -- [ ] 3.2.6 Add test: creates directory and README with name only -- [ ] 3.2.7 Add test: creates directory and README with description -- [ ] 3.2.8 Add test: duplicate change throws error -- [ ] 3.2.9 Add test: invalid name throws validation error -- [ ] 3.2.10 Add test: creates parent directories if needed - -## Phase 4: Refactor CLI Commands - -### 4.1 Refactor `ListCommand` -- [ ] 4.1.1 Import `ChangeManager` in `src/core/list.ts` -- [ ] 4.1.2 Replace inline directory scanning with `changeManager.listChanges()` -- [ ] 4.1.3 Verify existing behavior unchanged - -### 4.2 Refactor `ChangeCommand` -- [ ] 4.2.1 Import `ChangeManager` in `src/commands/change.ts` -- [ ] 4.2.2 Replace `getActiveChanges()` with `changeManager.listChanges()` -- [ ] 4.2.3 Replace inline path building with `changeManager.getChangePath()` -- [ ] 4.2.4 Remove `getActiveChanges()` private method -- [ ] 4.2.5 Verify existing behavior unchanged - -## Phase 5: Integration and Cleanup - -- [ ] 5.1 Add JSDoc comments to all public methods -- [ ] 5.2 Run existing tests to verify no regressions -- [ ] 5.3 Add integration test: full workflow (create, list, get path, exists) -- [ ] 5.4 Update any imports in other files if needed +## Phase 1: Implement Name Validation + +- [ ] 1.1 Create `src/utils/change-utils.ts` +- [ ] 1.2 Implement `validateChangeName()` with kebab-case pattern +- [ ] 1.3 Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` +- [ ] 1.4 Return `{ valid: boolean; error?: string }` +- [ ] 1.5 Add test: valid names accepted (`add-auth`, `refactor`, `add-feature-2`) +- [ ] 1.6 Add test: uppercase rejected +- [ ] 1.7 Add test: spaces rejected +- [ ] 1.8 Add test: underscores rejected +- [ ] 1.9 Add test: special characters rejected +- [ ] 1.10 Add test: leading/trailing hyphens rejected +- [ ] 1.11 Add test: consecutive hyphens rejected + +## Phase 2: Implement Change Creation + +- [ ] 2.1 Implement `createChange(projectRoot, name, description?)` +- [ ] 2.2 Validate name before creating +- [ ] 2.3 Create parent directories if needed (`openspec/changes/`) +- [ ] 2.4 Generate README.md with name and optional description +- [ ] 2.5 Throw if change already exists +- [ ] 2.6 Add test: creates directory and README with name only +- [ ] 2.7 Add test: creates directory and README with description +- [ ] 2.8 Add test: duplicate change throws error +- [ ] 2.9 Add test: invalid name throws validation error +- [ ] 2.10 Add test: creates parent directories if needed + +## Phase 3: Integration + +- [ ] 3.1 Export functions from `src/utils/index.ts` +- [ ] 3.2 Add JSDoc comments +- [ ] 3.3 Run all tests to verify no regressions From f59d9a94994ea5d566b3e4fc93ddb58fe5b01707 Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Thu, 25 Dec 2025 23:35:42 +1100 Subject: [PATCH 3/8] docs: update artifact_poc.md for simplified Slice 2 - Rename ChangeManager to change-utils (simple utility functions) - Remove extracted methods (listChanges, getChangePath, etc.) - Keep only new functionality: createChange(), validateChangeName() - Update component diagram and summary table --- docs/artifact_poc.md | 50 ++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/docs/artifact_poc.md b/docs/artifact_poc.md index 970ada7f..d6ada338 100644 --- a/docs/artifact_poc.md +++ b/docs/artifact_poc.md @@ -150,15 +150,13 @@ type SchemaYaml = z.infer; --- -### 2. ChangeManager (Slice 2) +### 2. Change Utilities (Slice 2) -Multi-change orchestration layer. **CLI is fully deterministic** - no "active change" tracking. +Simple utility functions for programmatic change creation. No class, no abstraction layer. | Responsibility | Approach | |----------------|----------| -| CRUD changes | Create dirs under `openspec/changes//` | -| List changes | Scan `openspec/changes/` (excluding `archive/`) | -| Path resolution | Resolve change directory paths | +| Create changes | Create dirs under `openspec/changes//` with README | | Name validation | Enforce kebab-case naming | **Key Paths:** @@ -167,14 +165,11 @@ Multi-change orchestration layer. **CLI is fully deterministic** - no "active ch openspec/changes// → Change instances with artifacts (project-level) ``` -**Key Methods:** -- `isInitialized()` - Check for `openspec/changes/` existence -- `listChanges()` - List all changes (excluding archive) -- `createChange(name, description)` - Create new change directory + README -- `getChangePath(name)` - Get path to a change directory -- `changeExists(name)` - Check if change exists +**Key Functions** (`src/utils/change-utils.ts`): +- `createChange(projectRoot, name, description?)` - Create new change directory + README +- `validateChangeName(name)` - Validate kebab-case naming, returns `{ valid, error? }` -**Note:** Schema resolution is handled by `artifact-graph` module (Slice 1). Template resolution is handled by `InstructionLoader` (Slice 3). ChangeManager focuses solely on change directory management. +**Note:** Existing CLI commands (`ListCommand`, `ChangeCommand`) already handle listing, path resolution, and existence checks. No need to extract that logic - it works fine as-is. --- @@ -269,9 +264,10 @@ This works for ANY artifact in ANY schema - no new slash commands needed when sc ┌─────────────────────────────────────────────────────────────┐ │ ORCHESTRATION LAYER │ │ ┌────────────────────┐ ┌──────────────────────────┐ │ -│ │ InstructionLoader │───────▶│ ChangeManager │ │ -│ │ (Slice 3) │ uses │ (Slice 2) │ │ -│ └─────────┬──────────┘ └──────────────────────────┘ │ +│ │ InstructionLoader │ │ change-utils (Slice 2) │ │ +│ │ (Slice 3) │ │ createChange() │ │ +│ └─────────┬──────────┘ │ validateChangeName() │ │ +│ │ └──────────────────────────┘ │ └────────────┼────────────────────────────────────────────────┘ │ uses ▼ @@ -426,21 +422,21 @@ Structured as **vertical slices** - each slice is independently testable. --- -### Slice 2: "Multi-Change Management" +### Slice 2: "Change Creation Utilities" -**Delivers:** CRUD for changes, path resolution, name validation +**Delivers:** Utility functions for programmatic change creation **Scope:** -- `createChange(name, description?)` → creates directory + README -- `listChanges()` → returns directory names (excluding `archive/`) -- `getChangePath(name)` → returns absolute path -- `changeExists(name)` → checks directory existence -- `isInitialized()` → checks `openspec/changes/` existence -- Name validation → kebab-case pattern enforcement +- `createChange(projectRoot, name, description?)` → creates directory + README +- `validateChangeName(name)` → kebab-case pattern enforcement + +**Not in scope (already exists in CLI commands):** +- `listChanges()` → exists in `ListCommand` and `ChangeCommand.getActiveChanges()` +- `getChangePath()` → simple `path.join()` inline +- `changeExists()` → simple `fs.access()` inline +- `isInitialized()` → simple directory check inline -**Not in scope (uses existing or deferred):** -- Schema resolution → reuse `artifact-graph.resolveSchema()` -- Template resolution → Slice 3 +**Why simplified:** Extracting existing CLI logic into a class would require similar refactoring of `SpecCommand` for consistency. The existing code works fine (~15 lines each). Only truly new functionality is `createChange()` + name validation. --- @@ -556,7 +552,7 @@ artifacts: | Layer | Component | Responsibility | Status | |-------|-----------|----------------|--------| | Core | ArtifactGraph | Pure dependency logic + XDG schema resolution | ✅ Slice 1 | -| Core | ChangeManager | Multi-change CRUD + path resolution | Slice 2 | +| Utils | change-utils | Change creation + name validation | Slice 2 | | Core | InstructionLoader | Template resolution + enrichment | Slice 3 | | Presentation | CLI | Thin command wrapper | Slice 4 | | Integration | Claude Commands | AI assistant glue | Slice 4 | From f04d494ed6238db2afdba2e6e4ff921e33261606 Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Thu, 25 Dec 2025 23:45:43 +1100 Subject: [PATCH 4/8] docs: clarify existing vs new functionality in artifact_poc.md Audit artifact_poc.md against existing codebase to mark what already exists vs what's genuinely new: - Slice 4 CLI table: Added Status column (NEW/EXISTS) - Added "Existing CLI commands" section listing what's not in scope - Updated Implementation Order Slice 4 with explicit new vs existing - Summary table: Updated status + added "What already exists" section Key finding: The document was already well-simplified. Only truly new functionality (createChange, validateChangeName, InstructionLoader, artifact graph CLI commands) is proposed. --- docs/artifact_poc.md | 57 ++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/docs/artifact_poc.md b/docs/artifact_poc.md index d6ada338..3d409e14 100644 --- a/docs/artifact_poc.md +++ b/docs/artifact_poc.md @@ -211,18 +211,24 @@ ChangeState { User interface layer. **All commands are deterministic** - require explicit `--change` parameter. -| Command | Function | -|---------|----------| -| `status --change ` | Show change progress | -| `next --change ` | Show artifacts ready to create | -| `instructions --change ` | Get enriched instructions for artifact | -| `list` | List all changes | -| `new ` | Create change | -| `init` | Initialize structure | -| `templates --change ` | Show resolved template paths | +| Command | Function | Status | +|---------|----------|--------| +| `status --change ` | Show change progress (artifact graph) | **NEW** | +| `next --change ` | Show artifacts ready to create | **NEW** | +| `instructions --change ` | Get enriched instructions for artifact | **NEW** | +| `list` | List all changes | EXISTS (`openspec change list`) | +| `new ` | Create change | **NEW** (uses `createChange()`) | +| `init` | Initialize structure | EXISTS (`openspec init`) | +| `templates --change ` | Show resolved template paths | **NEW** | **Note:** Commands that operate on a change require `--change`. Missing parameter → error with list of available changes. Agent infers the change from conversation and passes it explicitly. +**Existing CLI commands** (not part of this slice): +- `openspec change list` / `openspec change show ` / `openspec change validate ` +- `openspec list --changes` / `openspec list --specs` +- `openspec view` (dashboard) +- `openspec init` / `openspec archive ` + --- ### 5. Claude Commands @@ -453,10 +459,23 @@ Structured as **vertical slices** - each slice is independently testable. ### Slice 4: "CLI + Integration" -**Delivers:** Full command interface +**Delivers:** New artifact graph commands (builds on existing CLI) + +**New commands:** +- `status --change ` - Show artifact completion state +- `next --change ` - Show ready-to-create artifacts +- `instructions --change ` - Get enriched template +- `templates --change ` - Show resolved paths +- `new ` - Create change (wrapper for `createChange()`) + +**Already exists (not in scope):** +- `openspec change list/show/validate` - change management +- `openspec list --changes/--specs` - listing +- `openspec view` - dashboard +- `openspec init` - initialization **Testable behaviors:** -- Each command produces expected output +- Each new command produces expected output - Commands compose correctly (status → next → instructions flow) - Error handling for missing changes, invalid artifacts, etc. @@ -551,12 +570,20 @@ artifacts: | Layer | Component | Responsibility | Status | |-------|-----------|----------------|--------| -| Core | ArtifactGraph | Pure dependency logic + XDG schema resolution | ✅ Slice 1 | -| Utils | change-utils | Change creation + name validation | Slice 2 | -| Core | InstructionLoader | Template resolution + enrichment | Slice 3 | -| Presentation | CLI | Thin command wrapper | Slice 4 | +| Core | ArtifactGraph | Pure dependency logic + XDG schema resolution | ✅ Slice 1 COMPLETE | +| Utils | change-utils | Change creation + name validation only | Slice 2 (new functionality only) | +| Core | InstructionLoader | Template resolution + enrichment | Slice 3 (all new) | +| Presentation | CLI | New artifact graph commands | Slice 4 (new commands only) | | Integration | Claude Commands | AI assistant glue | Slice 4 | +**What already exists (not in this proposal):** +- `getActiveChangeIds()` in `src/utils/item-discovery.ts` - list changes +- `ChangeCommand.list/show/validate()` in `src/commands/change.ts` +- `ListCommand.execute()` in `src/core/list.ts` +- `ViewCommand.execute()` in `src/core/view.ts` - dashboard +- `src/core/init.ts` - initialization +- `src/core/archive.ts` - archiving + **Key Principles:** - **Filesystem IS the database** - stateless, version-control friendly - **Dependencies are enablers** - show what's possible, don't force order From 698c5c37a0c3eca1a25cbfc6024ad556f6639569 Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Thu, 25 Dec 2025 23:58:15 +1100 Subject: [PATCH 5/8] rename: change-manager -> change-creation capability The capability name "change-manager" implied a manager class abstraction, but the simplified proposal uses only utility functions. Renamed to "change-creation" to accurately reflect what the capability provides. --- .../specs/{change-manager => change-creation}/spec.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename openspec/changes/add-change-manager/specs/{change-manager => change-creation}/spec.md (100%) diff --git a/openspec/changes/add-change-manager/specs/change-manager/spec.md b/openspec/changes/add-change-manager/specs/change-creation/spec.md similarity index 100% rename from openspec/changes/add-change-manager/specs/change-manager/spec.md rename to openspec/changes/add-change-manager/specs/change-creation/spec.md From 334b23d022382bb5bd3142bd87803aa847e2e633 Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Fri, 26 Dec 2025 00:11:34 +1100 Subject: [PATCH 6/8] feat: implement change creation utilities Add createChange() and validateChangeName() functions for programmatic change directory creation with kebab-case validation. - createChange(projectRoot, name) creates openspec/changes// - validateChangeName() enforces kebab-case naming conventions - Comprehensive test coverage (21 tests) --- openspec/changes/add-change-manager/design.md | 18 +- .../changes/add-change-manager/proposal.md | 4 +- .../specs/change-creation/spec.md | 8 +- openspec/changes/add-change-manager/tasks.md | 46 +++-- src/utils/change-utils.ts | 102 ++++++++++ src/utils/index.ts | 5 +- test/utils/change-utils.test.ts | 176 ++++++++++++++++++ 7 files changed, 307 insertions(+), 52 deletions(-) create mode 100644 src/utils/change-utils.ts create mode 100644 test/utils/change-utils.test.ts diff --git a/openspec/changes/add-change-manager/design.md b/openspec/changes/add-change-manager/design.md index 7f360495..12e83514 100644 --- a/openspec/changes/add-change-manager/design.md +++ b/openspec/changes/add-change-manager/design.md @@ -31,8 +31,7 @@ export function validateChangeName(name: string): { valid: boolean; error?: stri export async function createChange( projectRoot: string, - name: string, - description?: string + name: string ): Promise ``` @@ -58,21 +57,6 @@ Invalid: `Add-Auth`, `add auth`, `add_auth`, `-add-auth`, `add-auth-`, `add--aut - URL-safe (for future web UI) - Consistent with existing change naming in repo -### Decision 3: README.md for Change Metadata - -**Choice**: `createChange()` generates a `README.md` with name and optional description. - -```markdown -# add-auth - -Add user authentication system -``` - -**Why**: -- Human-readable in GitHub/GitLab UI -- Minimal overhead -- Matches existing manual change creation patterns - ## File Changes ### New Files diff --git a/openspec/changes/add-change-manager/proposal.md b/openspec/changes/add-change-manager/proposal.md index 119fecce..047d46db 100644 --- a/openspec/changes/add-change-manager/proposal.md +++ b/openspec/changes/add-change-manager/proposal.md @@ -8,7 +8,7 @@ There's no programmatic way to create a new change directory. Users must manuall This is error-prone and blocks automation (e.g., Claude commands, scripts). **This proposal adds:** -1. `createChange(name, description?)` - Create change directories programmatically +1. `createChange(projectRoot, name)` - Create change directories programmatically 2. `validateChangeName(name)` - Enforce kebab-case naming conventions ## What Changes @@ -17,7 +17,7 @@ This is error-prone and blocks automation (e.g., Claude commands, scripts). | Function | Description | |----------|-------------| -| `createChange(name, description?)` | Creates `openspec/changes//` with README.md | +| `createChange(projectRoot, name)` | Creates `openspec/changes//` directory | | `validateChangeName(name)` | Returns `{ valid: boolean; error?: string }` | ### Name Validation Rules diff --git a/openspec/changes/add-change-manager/specs/change-creation/spec.md b/openspec/changes/add-change-manager/specs/change-creation/spec.md index efb81d09..447bf78b 100644 --- a/openspec/changes/add-change-manager/specs/change-creation/spec.md +++ b/openspec/changes/add-change-manager/specs/change-creation/spec.md @@ -3,15 +3,9 @@ ### Requirement: Change Creation The system SHALL provide a function to create new change directories programmatically. -#### Scenario: Create change with name only +#### Scenario: Create change - **WHEN** `createChange(projectRoot, 'add-auth')` is called - **THEN** the system creates `openspec/changes/add-auth/` directory -- **AND** creates a `README.md` file with the change name as title - -#### Scenario: Create change with description -- **WHEN** `createChange(projectRoot, 'add-auth', 'Add user authentication')` is called -- **THEN** the system creates `openspec/changes/add-auth/` directory -- **AND** creates a `README.md` file with the change name and description #### Scenario: Duplicate change rejected - **WHEN** `createChange(projectRoot, 'add-auth')` is called and `openspec/changes/add-auth/` already exists diff --git a/openspec/changes/add-change-manager/tasks.md b/openspec/changes/add-change-manager/tasks.md index e6571e5b..d07177cd 100644 --- a/openspec/changes/add-change-manager/tasks.md +++ b/openspec/changes/add-change-manager/tasks.md @@ -1,32 +1,30 @@ ## Phase 1: Implement Name Validation -- [ ] 1.1 Create `src/utils/change-utils.ts` -- [ ] 1.2 Implement `validateChangeName()` with kebab-case pattern -- [ ] 1.3 Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` -- [ ] 1.4 Return `{ valid: boolean; error?: string }` -- [ ] 1.5 Add test: valid names accepted (`add-auth`, `refactor`, `add-feature-2`) -- [ ] 1.6 Add test: uppercase rejected -- [ ] 1.7 Add test: spaces rejected -- [ ] 1.8 Add test: underscores rejected -- [ ] 1.9 Add test: special characters rejected -- [ ] 1.10 Add test: leading/trailing hyphens rejected -- [ ] 1.11 Add test: consecutive hyphens rejected +- [x] 1.1 Create `src/utils/change-utils.ts` +- [x] 1.2 Implement `validateChangeName()` with kebab-case pattern +- [x] 1.3 Pattern: `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` +- [x] 1.4 Return `{ valid: boolean; error?: string }` +- [x] 1.5 Add test: valid names accepted (`add-auth`, `refactor`, `add-feature-2`) +- [x] 1.6 Add test: uppercase rejected +- [x] 1.7 Add test: spaces rejected +- [x] 1.8 Add test: underscores rejected +- [x] 1.9 Add test: special characters rejected +- [x] 1.10 Add test: leading/trailing hyphens rejected +- [x] 1.11 Add test: consecutive hyphens rejected ## Phase 2: Implement Change Creation -- [ ] 2.1 Implement `createChange(projectRoot, name, description?)` -- [ ] 2.2 Validate name before creating -- [ ] 2.3 Create parent directories if needed (`openspec/changes/`) -- [ ] 2.4 Generate README.md with name and optional description -- [ ] 2.5 Throw if change already exists -- [ ] 2.6 Add test: creates directory and README with name only -- [ ] 2.7 Add test: creates directory and README with description -- [ ] 2.8 Add test: duplicate change throws error -- [ ] 2.9 Add test: invalid name throws validation error -- [ ] 2.10 Add test: creates parent directories if needed +- [x] 2.1 Implement `createChange(projectRoot, name)` +- [x] 2.2 Validate name before creating +- [x] 2.3 Create parent directories if needed (`openspec/changes/`) +- [x] 2.4 Throw if change already exists +- [x] 2.5 Add test: creates directory +- [x] 2.6 Add test: duplicate change throws error +- [x] 2.7 Add test: invalid name throws validation error +- [x] 2.8 Add test: creates parent directories if needed ## Phase 3: Integration -- [ ] 3.1 Export functions from `src/utils/index.ts` -- [ ] 3.2 Add JSDoc comments -- [ ] 3.3 Run all tests to verify no regressions +- [x] 3.1 Export functions from `src/utils/index.ts` +- [x] 3.2 Add JSDoc comments +- [x] 3.3 Run all tests to verify no regressions diff --git a/src/utils/change-utils.ts b/src/utils/change-utils.ts new file mode 100644 index 00000000..0fcdc2b7 --- /dev/null +++ b/src/utils/change-utils.ts @@ -0,0 +1,102 @@ +import path from 'path'; +import { FileSystemUtils } from './file-system.js'; + +/** + * Result of validating a change name. + */ +export interface ValidationResult { + valid: boolean; + error?: string; +} + +/** + * Validates that a change name follows kebab-case conventions. + * + * Valid names: + * - Start with a lowercase letter + * - Contain only lowercase letters, numbers, and hyphens + * - Do not start or end with a hyphen + * - Do not contain consecutive hyphens + * + * @param name - The change name to validate + * @returns Validation result with `valid: true` or `valid: false` with an error message + * + * @example + * validateChangeName('add-auth') // { valid: true } + * validateChangeName('Add-Auth') // { valid: false, error: '...' } + */ +export function validateChangeName(name: string): ValidationResult { + // Pattern: starts with lowercase letter, followed by lowercase letters/numbers, + // optionally followed by hyphen + lowercase letters/numbers (repeatable) + const kebabCasePattern = /^[a-z][a-z0-9]*(-[a-z0-9]+)*$/; + + if (!name) { + return { valid: false, error: 'Change name cannot be empty' }; + } + + if (!kebabCasePattern.test(name)) { + // Provide specific error messages for common mistakes + if (/[A-Z]/.test(name)) { + return { valid: false, error: 'Change name must be lowercase (use kebab-case)' }; + } + if (/\s/.test(name)) { + return { valid: false, error: 'Change name cannot contain spaces (use hyphens instead)' }; + } + if (/_/.test(name)) { + return { valid: false, error: 'Change name cannot contain underscores (use hyphens instead)' }; + } + if (name.startsWith('-')) { + return { valid: false, error: 'Change name cannot start with a hyphen' }; + } + if (name.endsWith('-')) { + return { valid: false, error: 'Change name cannot end with a hyphen' }; + } + if (/--/.test(name)) { + return { valid: false, error: 'Change name cannot contain consecutive hyphens' }; + } + if (/[^a-z0-9-]/.test(name)) { + return { valid: false, error: 'Change name can only contain lowercase letters, numbers, and hyphens' }; + } + if (/^[0-9]/.test(name)) { + return { valid: false, error: 'Change name must start with a letter' }; + } + + return { valid: false, error: 'Change name must follow kebab-case convention (e.g., add-auth, refactor-db)' }; + } + + return { valid: true }; +} + +/** + * Creates a new change directory. + * + * @param projectRoot - The root directory of the project (where `openspec/` lives) + * @param name - The change name (must be valid kebab-case) + * @throws Error if the change name is invalid + * @throws Error if the change directory already exists + * + * @example + * // Creates openspec/changes/add-auth/ + * await createChange('/path/to/project', 'add-auth') + */ +export async function createChange( + projectRoot: string, + name: string +): Promise { + // Validate the name first + const validation = validateChangeName(name); + if (!validation.valid) { + throw new Error(validation.error); + } + + // Build the change directory path + const changeDir = path.join(projectRoot, 'openspec', 'changes', name); + + // Check if change already exists + if (await FileSystemUtils.directoryExists(changeDir)) { + throw new Error(`Change '${name}' already exists at ${changeDir}`); + } + + // Create the directory (including parent directories if needed) + await FileSystemUtils.createDirectory(changeDir); +} diff --git a/src/utils/index.ts b/src/utils/index.ts index b1cb3e64..46862b54 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,2 +1,3 @@ -// Shared utilities will be implemented here -export {}; \ No newline at end of file +// Shared utilities +export { validateChangeName, createChange } from './change-utils.js'; +export type { ValidationResult } from './change-utils.js'; \ No newline at end of file diff --git a/test/utils/change-utils.test.ts b/test/utils/change-utils.test.ts new file mode 100644 index 00000000..1487e7af --- /dev/null +++ b/test/utils/change-utils.test.ts @@ -0,0 +1,176 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { randomUUID } from 'crypto'; +import { validateChangeName, createChange } from '../../src/utils/change-utils.js'; + +describe('validateChangeName', () => { + describe('valid names', () => { + it('should accept simple kebab-case name', () => { + const result = validateChangeName('add-auth'); + expect(result).toEqual({ valid: true }); + }); + + it('should accept name with multiple segments', () => { + const result = validateChangeName('add-user-auth'); + expect(result).toEqual({ valid: true }); + }); + + it('should accept name with numeric suffix', () => { + const result = validateChangeName('add-feature-2'); + expect(result).toEqual({ valid: true }); + }); + + it('should accept single word name', () => { + const result = validateChangeName('refactor'); + expect(result).toEqual({ valid: true }); + }); + + it('should accept name with numbers in segments', () => { + const result = validateChangeName('upgrade-to-v2'); + expect(result).toEqual({ valid: true }); + }); + }); + + describe('invalid names - uppercase rejected', () => { + it('should reject name with uppercase letters', () => { + const result = validateChangeName('Add-Auth'); + expect(result.valid).toBe(false); + expect(result.error).toContain('lowercase'); + }); + + it('should reject fully uppercase name', () => { + const result = validateChangeName('ADD-AUTH'); + expect(result.valid).toBe(false); + expect(result.error).toContain('lowercase'); + }); + }); + + describe('invalid names - spaces rejected', () => { + it('should reject name with spaces', () => { + const result = validateChangeName('add auth'); + expect(result.valid).toBe(false); + expect(result.error).toContain('spaces'); + }); + }); + + describe('invalid names - underscores rejected', () => { + it('should reject name with underscores', () => { + const result = validateChangeName('add_auth'); + expect(result.valid).toBe(false); + expect(result.error).toContain('underscores'); + }); + }); + + describe('invalid names - special characters rejected', () => { + it('should reject name with exclamation mark', () => { + const result = validateChangeName('add-auth!'); + expect(result.valid).toBe(false); + expect(result.error).toBeDefined(); + }); + + it('should reject name with @ symbol', () => { + const result = validateChangeName('add@auth'); + expect(result.valid).toBe(false); + expect(result.error).toBeDefined(); + }); + }); + + describe('invalid names - leading/trailing hyphens rejected', () => { + it('should reject name with leading hyphen', () => { + const result = validateChangeName('-add-auth'); + expect(result.valid).toBe(false); + expect(result.error).toContain('start with a hyphen'); + }); + + it('should reject name with trailing hyphen', () => { + const result = validateChangeName('add-auth-'); + expect(result.valid).toBe(false); + expect(result.error).toContain('end with a hyphen'); + }); + }); + + describe('invalid names - consecutive hyphens rejected', () => { + it('should reject name with double hyphens', () => { + const result = validateChangeName('add--auth'); + expect(result.valid).toBe(false); + expect(result.error).toContain('consecutive hyphens'); + }); + }); + + describe('invalid names - empty name rejected', () => { + it('should reject empty string', () => { + const result = validateChangeName(''); + expect(result.valid).toBe(false); + expect(result.error).toContain('empty'); + }); + }); +}); + +describe('createChange', () => { + let testDir: string; + + beforeEach(async () => { + testDir = path.join(os.tmpdir(), `openspec-test-${randomUUID()}`); + await fs.mkdir(testDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + describe('creates directory', () => { + it('should create change directory', async () => { + await createChange(testDir, 'add-auth'); + + const changeDir = path.join(testDir, 'openspec', 'changes', 'add-auth'); + const stats = await fs.stat(changeDir); + expect(stats.isDirectory()).toBe(true); + }); + }); + + describe('duplicate change throws error', () => { + it('should throw error if change already exists', async () => { + await createChange(testDir, 'add-auth'); + + await expect(createChange(testDir, 'add-auth')).rejects.toThrow( + /already exists/ + ); + }); + }); + + describe('invalid name throws validation error', () => { + it('should throw error for uppercase name', async () => { + await expect(createChange(testDir, 'Add-Auth')).rejects.toThrow( + /lowercase/ + ); + }); + + it('should throw error for name with spaces', async () => { + await expect(createChange(testDir, 'add auth')).rejects.toThrow( + /spaces/ + ); + }); + + it('should throw error for empty name', async () => { + await expect(createChange(testDir, '')).rejects.toThrow( + /empty/ + ); + }); + }); + + describe('creates parent directories if needed', () => { + it('should create openspec/changes/ directories if they do not exist', async () => { + const newProjectDir = path.join(testDir, 'new-project'); + await fs.mkdir(newProjectDir); + + // openspec/changes/ does not exist yet + await createChange(newProjectDir, 'add-auth'); + + const changeDir = path.join(newProjectDir, 'openspec', 'changes', 'add-auth'); + const stats = await fs.stat(changeDir); + expect(stats.isDirectory()).toBe(true); + }); + }); +}); From cc6c358e730d16f4efbc2bdc95b75d58030dc11f Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Fri, 26 Dec 2025 00:14:14 +1100 Subject: [PATCH 7/8] chore: archive add-change-manager change Move change to archive and create change-creation spec with requirements for createChange() and validateChangeName(). --- .../2025-12-25-add-change-manager}/design.md | 0 .../proposal.md | 0 .../specs/change-creation/spec.md | 0 .../2025-12-25-add-change-manager}/tasks.md | 0 openspec/specs/change-creation/spec.md | 67 +++++++++++++++++++ 5 files changed, 67 insertions(+) rename openspec/changes/{add-change-manager => archive/2025-12-25-add-change-manager}/design.md (100%) rename openspec/changes/{add-change-manager => archive/2025-12-25-add-change-manager}/proposal.md (100%) rename openspec/changes/{add-change-manager => archive/2025-12-25-add-change-manager}/specs/change-creation/spec.md (100%) rename openspec/changes/{add-change-manager => archive/2025-12-25-add-change-manager}/tasks.md (100%) create mode 100644 openspec/specs/change-creation/spec.md diff --git a/openspec/changes/add-change-manager/design.md b/openspec/changes/archive/2025-12-25-add-change-manager/design.md similarity index 100% rename from openspec/changes/add-change-manager/design.md rename to openspec/changes/archive/2025-12-25-add-change-manager/design.md diff --git a/openspec/changes/add-change-manager/proposal.md b/openspec/changes/archive/2025-12-25-add-change-manager/proposal.md similarity index 100% rename from openspec/changes/add-change-manager/proposal.md rename to openspec/changes/archive/2025-12-25-add-change-manager/proposal.md diff --git a/openspec/changes/add-change-manager/specs/change-creation/spec.md b/openspec/changes/archive/2025-12-25-add-change-manager/specs/change-creation/spec.md similarity index 100% rename from openspec/changes/add-change-manager/specs/change-creation/spec.md rename to openspec/changes/archive/2025-12-25-add-change-manager/specs/change-creation/spec.md diff --git a/openspec/changes/add-change-manager/tasks.md b/openspec/changes/archive/2025-12-25-add-change-manager/tasks.md similarity index 100% rename from openspec/changes/add-change-manager/tasks.md rename to openspec/changes/archive/2025-12-25-add-change-manager/tasks.md diff --git a/openspec/specs/change-creation/spec.md b/openspec/specs/change-creation/spec.md new file mode 100644 index 00000000..69285a0f --- /dev/null +++ b/openspec/specs/change-creation/spec.md @@ -0,0 +1,67 @@ +# change-creation Specification + +## Purpose +TBD - created by archiving change add-change-manager. Update Purpose after archive. +## Requirements +### Requirement: Change Creation +The system SHALL provide a function to create new change directories programmatically. + +#### Scenario: Create change +- **WHEN** `createChange(projectRoot, 'add-auth')` is called +- **THEN** the system creates `openspec/changes/add-auth/` directory + +#### Scenario: Duplicate change rejected +- **WHEN** `createChange(projectRoot, 'add-auth')` is called and `openspec/changes/add-auth/` already exists +- **THEN** the system throws an error indicating the change already exists + +#### Scenario: Creates parent directories if needed +- **WHEN** `createChange(projectRoot, 'add-auth')` is called and `openspec/changes/` does not exist +- **THEN** the system creates the full path including parent directories + +#### Scenario: Invalid change name rejected +- **WHEN** `createChange(projectRoot, 'Add Auth')` is called with an invalid name +- **THEN** the system throws a validation error + +### Requirement: Change Name Validation +The system SHALL validate change names follow kebab-case conventions. + +#### Scenario: Valid kebab-case name accepted +- **WHEN** a change name like `add-user-auth` is validated +- **THEN** validation returns `{ valid: true }` + +#### Scenario: Numeric suffixes accepted +- **WHEN** a change name like `add-feature-2` is validated +- **THEN** validation returns `{ valid: true }` + +#### Scenario: Single word accepted +- **WHEN** a change name like `refactor` is validated +- **THEN** validation returns `{ valid: true }` + +#### Scenario: Uppercase characters rejected +- **WHEN** a change name like `Add-Auth` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Spaces rejected +- **WHEN** a change name like `add auth` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Underscores rejected +- **WHEN** a change name like `add_auth` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Special characters rejected +- **WHEN** a change name like `add-auth!` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Leading hyphen rejected +- **WHEN** a change name like `-add-auth` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Trailing hyphen rejected +- **WHEN** a change name like `add-auth-` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + +#### Scenario: Consecutive hyphens rejected +- **WHEN** a change name like `add--auth` is validated +- **THEN** validation returns `{ valid: false, error: "..." }` + From 694c124c8e5d27b1b01681c237f9dd4b8575bb7b Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Fri, 26 Dec 2025 00:29:12 +1100 Subject: [PATCH 8/8] docs: update change-creation spec purpose --- openspec/specs/change-creation/spec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/specs/change-creation/spec.md b/openspec/specs/change-creation/spec.md index 69285a0f..3e85719f 100644 --- a/openspec/specs/change-creation/spec.md +++ b/openspec/specs/change-creation/spec.md @@ -1,7 +1,7 @@ # change-creation Specification ## Purpose -TBD - created by archiving change add-change-manager. Update Purpose after archive. +Provide programmatic utilities for creating and validating OpenSpec change directories. ## Requirements ### Requirement: Change Creation The system SHALL provide a function to create new change directories programmatically.