From 09e698346cfa57466731e4a3ba558ffb392f82b7 Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Thu, 11 Jun 2026 08:21:32 -0600 Subject: [PATCH 1/2] fix: address hands-on testing findings across init, check, generate, and watch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - init: create the v4 .specsync/ layout (config.toml, version stamp, .gitignore, lifecycle/changes/archive dirs) instead of legacy specsync.json, so a fresh project never sees the migration nag - init-registry: write .specsync/registry.toml in v4 projects (legacy root path only for un-migrated 3.x layouts); add registry::local_registry_path and use it in load_registry / register_module - check: draft specs print explicit "skipped (status: draft)" notices and a summary hint instead of misleading green checkmarks; failing frontmatter renders "✗ Frontmatter invalid"; unmatched spec filters exit 1 without the contradictory "No spec files found" message - check --fix: route function/value exports to the functions table and type exports to the types table, padding rows to the target table's column count - generate: exit 1 when AI generation fails (template fallback), with the failures re-printed last on stderr; JSON/MCP output gains ai_errors; both generation entry points return GenerationOutcome - watch: footer parses the child check's summary line so it never prints "All checks passed!" under a failing report - cli: --root pointing at a nonexistent path errors with exit 2 Updated the affected specs (11) and added 12 integration tests plus unit tests for the new helpers. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 9 + specs/cli/cli.spec.md | 9 +- specs/cmd_check/cmd_check.spec.md | 8 +- specs/cmd_generate/cmd_generate.spec.md | 6 +- specs/cmd_init/cmd_init.spec.md | 20 +- .../cmd_init_registry.spec.md | 14 +- specs/commands/commands.spec.md | 7 +- specs/generator/generator.spec.md | 21 +- specs/registry/registry.spec.md | 9 +- specs/types/types.spec.md | 7 +- specs/validator/validator.spec.md | 4 +- specs/watch/watch.spec.md | 6 +- src/commands/check.rs | 244 ++++++++--- src/commands/generate.rs | 46 ++- src/commands/init.rs | 135 ++++-- src/commands/init_registry.rs | 21 +- src/commands/mod.rs | 31 +- src/generator.rs | 64 ++- src/main.rs | 8 + src/mcp.rs | 7 +- src/registry.rs | 35 +- src/types.rs | 5 + src/validator.rs | 1 + src/watch.rs | 110 ++++- tests/integration.rs | 391 ++++++++++++++++-- 25 files changed, 1009 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc00a8..4830fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Fresh `specsync init` now creates the v4 layout** — init writes `.specsync/config.toml`, a `4.0.0` version stamp, `.specsync/.gitignore`, and the `lifecycle/`/`changes/`/`archive/` directories (what `specsync migrate` produces) instead of a legacy root-level `specsync.json`, so a brand-new project no longer sees the "Legacy 3.x layout detected" migration nag on its first `check`. +- **`init-registry` respects the v4 layout** — the registry is written to `.specsync/registry.toml` in v4 projects instead of recreating a root-level `specsync-registry.toml` (which re-triggered the legacy nag after migration). Un-migrated 3.x projects keep the legacy path. `load_registry`/`register_module` now resolve the same location via the new `registry::local_registry_path`. +- **Draft specs no longer pass validation silently** — when a draft skips section/export checks (by design), `check` now prints explicit "Section validation skipped (status: draft)" / "Export validation skipped (status: draft)" notices instead of misleading "✓ All required sections present" lines, plus a summary hint: "N draft spec(s) skipped section and export validation — set `status: active` to enable full checks". +- **`check --fix` routes exports to the matching table** — functions/values are appended to the "… Functions"/"… Methods" table and type exports to the "… Types" table (previously everything landed in the last export table, e.g. functions `add`/`subtract` in "Exported Types"). New rows are padded to the target table's column count. +- **`generate` exits non-zero when AI generation fails** — a failed provider call (e.g. missing API key) still falls back to the template, but the failures are re-printed prominently on stderr *after* the check report and the command exits 1 instead of burying the error and exiting 0. JSON output gains an `ai_errors` array; the MCP `specsync_generate` tool reports `ai_errors` too. Both generation entry points now return a `GenerationOutcome` (count, paths, AI errors). +- **`watch` footer no longer contradicts the report** — the footer parses the child check's summary line instead of trusting only its exit code (which is 0 under the default `enforcement = warn`), so "All checks passed!" is never printed beneath a "… 1 failed" summary. +- **Failing checks render negated labels** — a failing frontmatter check now prints "✗ Frontmatter invalid" instead of "✗ Frontmatter valid". +- **`check ` with an unmatched spec filter exits 1** — and no longer follows the "No specs matched" warning with a contradictory "No spec files found in specs/" message when specs exist. +- **`--root` pointing at a nonexistent path now errors (exit 2)** — previously the CLI silently exited 0 having checked nothing. - **Spec scoring no longer false-flags documented HTML-comment syntax** — the `placeholder_free` check strips fenced and inline code before counting ``, so a spec that *documents* an HTML-comment directive (e.g. ``a `` directive``) isn't penalized for showing real syntax. ## [4.4.0] - 2026-06-07 diff --git a/specs/cli/cli.spec.md b/specs/cli/cli.spec.md index 56e5cd3..7d65946 100644 --- a/specs/cli/cli.spec.md +++ b/specs/cli/cli.spec.md @@ -1,6 +1,6 @@ --- module: cli -version: 3 +version: 4 status: stable files: - src/main.rs @@ -130,7 +130,7 @@ All functions in main.rs are private (no pub keyword). Key internal functions: ## Invariants 1. When no subcommand is given, `check` runs by default -2. `--root` defaults to the current working directory; the path is canonicalized +2. `--root` defaults to the current working directory; the path is validated (must be an existing directory — otherwise an error is printed and the process exits 2) and canonicalized 3. `--strict` causes warnings to produce a non-zero exit code 4. `--require-coverage N` causes exit 1 if file coverage percent < N 5. `--json` switches all output to machine-readable JSON (no ANSI colors) @@ -175,9 +175,9 @@ All functions in main.rs are private (no pub keyword). Key internal functions: ### Scenario: Init idempotency -- **Given** `specsync.json` already exists in the project root +- **Given** a config (v4 `.specsync/config.toml` or legacy `specsync.json`) already exists - **When** `specsync init` is run -- **Then** prints "specsync.json already exists" and returns without modifying it +- **Then** prints an "already exists" message and returns without modifying it ### Scenario: Coverage threshold @@ -347,6 +347,7 @@ Cold start times (first run after boot) may be 2-3x higher due to disk cache war | Date | Change | |------|--------| +| 2026-06-11 | v4: `--root` now errors (exit 2) for nonexistent paths; init scenario covers the v4 config layout | | 2026-04-10 | Add Performance Requirements section with response time targets, cache requirements, resource limits, and scalability targets | | 2026-03-25 | Initial spec | | 2026-04-06 | Add compact, archive-tasks, view, merge, issues subcommands; add --force, --create-issues, --format flags; add hash_cache/github/archive/compact/view/merge dependencies | diff --git a/specs/cmd_check/cmd_check.spec.md b/specs/cmd_check/cmd_check.spec.md index 8d43400..0d3a10a 100644 --- a/specs/cmd_check/cmd_check.spec.md +++ b/specs/cmd_check/cmd_check.spec.md @@ -1,6 +1,6 @@ --- module: cmd_check -version: 2 +version: 3 status: stable files: - src/commands/check.rs @@ -35,7 +35,7 @@ Implements the `specsync check` command — the primary validation entry point. ## Invariants -1. When `--fix` is passed, auto-fix runs in two phases: (a) add undocumented exports to spec markdown tables with generated review prompts, (b) AI-regenerate specs whose requirements have drifted +1. When `--fix` is passed, auto-fix runs in two phases: (a) add undocumented exports to spec markdown tables with generated review prompts — type exports are routed to the "… Types" table and functions/values to the "… Functions"/"… Methods" table (falling back to the last export subsection), with rows padded to the target table's column count, (b) AI-regenerate specs whose requirements have drifted 2. Near-miss header correction (e.g., "Exported Functions" → "### Exported Functions") runs as part of auto-fix 3. Hash cache is consulted before validation unless `--force` is set — unchanged specs are skipped 4. After auto-fix, validation is re-run to verify fixes resolved the issues @@ -56,7 +56,7 @@ Implements the `specsync check` command — the primary validation entry point. - **Given** spec is missing export `pub fn new_function()` - **When** `cmd_check` runs with `--fix` -- **Then** the export is appended to the spec's Public API table with a generated description prompt and the file is rewritten +- **Then** the export is appended to the matching Public API table (functions to the functions table, types to the types table) with a generated description prompt and the file is rewritten ### Scenario: JSON output format @@ -70,6 +70,7 @@ Implements the `specsync check` command — the primary validation entry point. |-----------|----------| | AI provider not available during `--fix` regen | Prints error per spec, continues with remaining specs | | Auto-fix changes a spec but validation still fails | Reports remaining errors, does not loop | +| Spec name filter matches nothing while specs exist | Prints "No specs matched" error (no contradictory "No spec files found" message) and exits 1 | | Hash cache file is corrupted | Falls back to full validation (cache miss) | | `--create-issues` with no GitHub repo | Prints error, skips issue creation | @@ -99,5 +100,6 @@ Implements the `specsync check` command — the primary validation entry point. | Date | Change | |------|--------| +| 2026-06-11 | v3: `--fix` routes exports to the matching table by kind; unmatched spec filters exit 1 without contradictory output | | 2026-06-07 | Document generated review prompts for `--fix` export rows | | 2026-04-09 | Initial spec | diff --git a/specs/cmd_generate/cmd_generate.spec.md b/specs/cmd_generate/cmd_generate.spec.md index e136ad2..526a04b 100644 --- a/specs/cmd_generate/cmd_generate.spec.md +++ b/specs/cmd_generate/cmd_generate.spec.md @@ -1,6 +1,6 @@ --- module: cmd_generate -version: 1 +version: 2 status: stable files: - src/commands/generate.rs @@ -36,6 +36,7 @@ Implements the `specsync generate` command. Scaffolds spec files for unspecced m 2. With `--provider`, resolves AI provider and generates from source code 3. Re-runs validation after generation to verify new specs 4. Exits 1 if AI provider resolution fails +5. If any AI generation fails (template fallback used), the failures are re-printed prominently on stderr after the check report and the command exits 1; JSON output includes an `ai_errors` array ## Behavioral Examples @@ -50,7 +51,7 @@ Implements the `specsync generate` command. Scaffolds spec files for unspecced m | Condition | Behavior | |-----------|----------| | AI provider not found | Exits 1 | -| AI fails for one module | Error printed, continues | +| AI fails for one module | Error printed, continues, then exits 1 with the failures summarized last on stderr | | All modules already specced | Prints "all covered" | ## Dependencies @@ -78,3 +79,4 @@ Implements the `specsync generate` command. Scaffolds spec files for unspecced m | Date | Change | |------|--------| | 2026-04-09 | Initial spec | +| 2026-06-11 | v2: Exit non-zero when AI generation fails, with the errors re-printed last on stderr and `ai_errors` in JSON output | diff --git a/specs/cmd_init/cmd_init.spec.md b/specs/cmd_init/cmd_init.spec.md index 3f55b29..6881334 100644 --- a/specs/cmd_init/cmd_init.spec.md +++ b/specs/cmd_init/cmd_init.spec.md @@ -1,6 +1,6 @@ --- module: cmd_init -version: 1 +version: 2 status: stable files: - src/commands/init.rs @@ -14,7 +14,7 @@ depends_on: ## Purpose -Implements the `specsync init` command. Creates a `specsync.json` configuration file with auto-detected source directories. +Implements the `specsync init` command. Creates the v4 `.specsync/` layout — `config.toml` with auto-detected source directories, a `version` stamp, `.gitignore`, and the `lifecycle/`, `changes/`, and `archive/` state directories — matching what `specsync migrate` produces. ## Public API @@ -22,26 +22,27 @@ Implements the `specsync init` command. Creates a `specsync.json` configuration | Function | Parameters | Returns | Description | |----------|-----------|---------|-------------| -| `cmd_init` | `root: &Path` | `()` | Create specsync.json with auto-detected source dirs | +| `cmd_init` | `root: &Path` | `()` | Create the v4 `.specsync/` layout with auto-detected source dirs | | `ensure_hashes_gitignored` | `root: &Path` | `Result` | Add `.specsync/hashes.json` to the root `.gitignore` (idempotent); returns `Ok(true)` if the entry was added, `Ok(false)` if already present, `Err` if the write fails | ## Invariants 1. Auto-detects source directories via `config::detect_source_dirs()` -2. Will not overwrite existing `specsync.json` -3. Writes default config with detected dirs and standard required sections +2. Will not overwrite an existing config (v4 `.specsync/config.toml`/`config.json` or legacy `specsync.json`/`.specsync.toml`); legacy configs get a `specsync migrate` hint +3. Writes default config with detected dirs and standard required sections via `config::config_to_toml()` +4. A fresh init never triggers the legacy 3.x layout migration nag — `.specsync/version` is stamped with 4.0.0 ## Behavioral Examples ### Scenario: First init -- **Given** no `specsync.json` exists +- **Given** no config exists - **When** `cmd_init(root)` runs -- **Then** creates config with detected source dirs +- **Then** creates `.specsync/config.toml`, `.specsync/version`, `.specsync/.gitignore`, and the `lifecycle/`, `changes/`, `archive/` directories ### Scenario: Config exists -- **Given** `specsync.json` already exists +- **Given** `.specsync/config.toml` (or a legacy config) already exists - **When** `cmd_init(root)` runs - **Then** prints message and returns without changes @@ -58,7 +59,7 @@ Implements the `specsync init` command. Creates a `specsync.json` configuration | Module | What is used | |--------|-------------| -| config | `detect_source_dirs` | +| config | `detect_source_dirs`, `config_to_toml` | ### Consumed By @@ -71,3 +72,4 @@ Implements the `specsync init` command. Creates a `specsync.json` configuration | Date | Change | |------|--------| | 2026-04-09 | Initial spec | +| 2026-06-11 | v2: Init the v4 `.specsync/` layout instead of the legacy `specsync.json` so a fresh project never sees the migration nag | diff --git a/specs/cmd_init_registry/cmd_init_registry.spec.md b/specs/cmd_init_registry/cmd_init_registry.spec.md index d40ff64..7f6f73a 100644 --- a/specs/cmd_init_registry/cmd_init_registry.spec.md +++ b/specs/cmd_init_registry/cmd_init_registry.spec.md @@ -1,6 +1,6 @@ --- module: cmd_init_registry -version: 1 +version: 2 status: stable files: - src/commands/init_registry.rs @@ -15,7 +15,7 @@ depends_on: ## Purpose -Implements the `specsync init-registry` command. Creates a `specsync-registry.toml` for cross-project spec references with auto-detected entries. +Implements the `specsync init-registry` command. Creates a registry file for cross-project spec references with auto-detected entries. The registry is written to the v4 location (`.specsync/registry.toml`); the legacy root-level `specsync-registry.toml` is only used for un-migrated 3.x layouts. ## Public API @@ -27,9 +27,10 @@ Implements the `specsync init-registry` command. Creates a `specsync-registry.to ## Invariants -1. Delegates to `registry::generate_registry()` -2. Will not overwrite existing `specsync-registry.toml` +1. Delegates to `registry::generate_registry()` and resolves the target path via `registry::local_registry_path()` +2. Will not overwrite an existing registry file (v4 or legacy location) 3. `--name` overrides auto-detected project name +4. Never recreates the legacy root-level registry in a v4 project — doing so would re-trigger the legacy-layout migration nag ## Behavioral Examples @@ -37,7 +38,7 @@ Implements the `specsync init-registry` command. Creates a `specsync-registry.to - **Given** 25 specs, no existing registry - **When** `cmd_init_registry(root, None)` runs -- **Then** creates TOML with 25 entries +- **Then** creates TOML with 25 entries at `.specsync/registry.toml` (or `specsync-registry.toml` on a legacy 3.x layout) ## Error Cases @@ -53,7 +54,7 @@ Implements the `specsync init-registry` command. Creates a `specsync-registry.to | Module | What is used | |--------|-------------| | config | `load_config` | -| registry | `generate_registry` | +| registry | `generate_registry`, `local_registry_path` | ### Consumed By @@ -66,3 +67,4 @@ Implements the `specsync init-registry` command. Creates a `specsync-registry.to | Date | Change | |------|--------| | 2026-04-09 | Initial spec | +| 2026-06-11 | v2: Write to v4 `.specsync/registry.toml` (legacy root path only for un-migrated 3.x projects) | diff --git a/specs/commands/commands.spec.md b/specs/commands/commands.spec.md index 878a8bc..03d16f4 100644 --- a/specs/commands/commands.spec.md +++ b/specs/commands/commands.spec.md @@ -1,6 +1,6 @@ --- module: commands -version: 1 +version: 2 status: stable files: - src/commands/mod.rs @@ -74,6 +74,8 @@ Shared command infrastructure used by all CLI subcommands. Provides config loadi 1. `load_and_discover` excludes spec files starting with `_` (underscore prefix marks internal/template specs) 2. `filter_specs` matches against four forms: exact path, relative path, filename stem, and module name (stem minus `.spec` suffix) 3. `run_validation` applies ignore rules (global, inline, per-spec) to filter warnings before counting +4. In text mode, draft specs show explicit "Section validation skipped (status: draft)" and "Export validation skipped (status: draft)" notices instead of misleading green checkmarks, plus a closing hint to set `status: active` +5. Failing checks render negated labels (e.g. "✗ Frontmatter invalid"), never a ✗ next to a passing label 4. Exit code logic by enforcement mode: Warn → always 0; EnforceNew → 1 if unspecced files; Strict → 1 on errors, also 1 on warnings when `--strict` 5. `--require-coverage N` triggers exit 1 if file coverage percent < N regardless of enforcement mode 6. `create_drift_issues` groups errors by spec path and creates one GitHub issue per spec, not per error @@ -103,7 +105,7 @@ Shared command infrastructure used by all CLI subcommands. Provides config loadi | Condition | Behavior | |-----------|----------| | No spec files found and `allow_empty` is false | Prints suggestion to run `specsync generate` and exits 0 | -| Filter matches no specs | Prints warning listing unmatched filters, returns empty vec | +| Filter matches no specs | Prints warning listing unmatched filters, returns empty vec (cmd_check then exits 1) | | `schema_dir` not configured | `build_schema_columns` returns empty map (no error) | | GitHub repo unresolvable for drift issues | Prints error and returns without creating issues | | `gh` CLI fails to create issue | Prints per-spec error but continues with remaining specs | @@ -141,5 +143,6 @@ Shared command infrastructure used by all CLI subcommands. Provides config loadi | Date | Change | |------|--------| +| 2026-06-11 | v2: Draft specs report skipped section/export validation explicitly; failing frontmatter renders a negated label | | 2026-04-09 | Initial spec | | 2026-04-11 | Add lifecycle submodule and filter_by_status function | diff --git a/specs/generator/generator.spec.md b/specs/generator/generator.spec.md index fdb174c..00adfe6 100644 --- a/specs/generator/generator.spec.md +++ b/specs/generator/generator.spec.md @@ -1,6 +1,6 @@ --- module: generator -version: 2 +version: 3 status: stable files: - src/generator.rs @@ -24,14 +24,20 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, | Function | Parameters | Returns | Description | |----------|-----------|---------|-------------| -| `generate_specs_for_unspecced_modules` | `root, report, config, provider` | `usize` | Generate specs for all unspecced modules, returning count of generated specs | -| `generate_specs_for_unspecced_modules_paths` | `root, report, config, provider` | `Vec` | Generate specs for all unspecced modules, returning paths of generated files | +| `generate_specs_for_unspecced_modules` | `root, report, config, provider` | `GenerationOutcome` | Generate specs for all unspecced modules with per-file progress output, returning the generation outcome | +| `generate_specs_for_unspecced_modules_paths` | `root, report, config, provider` | `GenerationOutcome` | Generate specs for all unspecced modules without per-file progress output (JSON/MCP callers), returning the generation outcome | | `generate_companion_files_for_spec` | `spec_dir, module_name, design_enabled` | `()` | Generate companion files (tasks.md, context.md, requirements.md, testing.md, and design.md if enabled) alongside a spec | | `find_files_for_module` | `root, module_name, config` | `Vec` | Find source files for a module by checking config definitions, subdirectories, then flat files | | `generate_spec` | `module_name, source_files, root, specs_dir` | `String` | Generate a spec from a template (custom or language-aware default) | | `generate_spec_from_custom_template` | `template_dir, module_name, source_files, root` | `String` | Generate a spec using files from a custom template directory | | `generate_companion_files_from_template` | `spec_dir, module_name, template_dir, design_enabled` | `()` | Generate companion files from a custom template directory with fallback to defaults; creates design.md only when `design_enabled` is true | +### Exported Types + +| Type | Description | +|------|-------------| +| `GenerationOutcome` | Outcome of a generation run: `generated` count, `generated_paths` (relative spec paths written), and `ai_errors` (one entry per module whose AI generation failed and fell back to the template) | + ## Invariants 1. Specs are never overwritten — if a `module.spec.md` already exists, it is skipped @@ -40,7 +46,7 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, 4. Module title is derived from the module name with dashes converted to title case (e.g. "api-gateway" -> "Api Gateway") 5. Companion files (tasks.md, context.md, requirements.md, testing.md, and design.md when enabled) are only created if they don't already exist and use guidance text instead of empty template comments 6. The design.md template includes its own YAML frontmatter with `spec:` (back-reference to the parent spec) and `sources:` (list of design asset references — Figma URLs, image paths, etc.). This frontmatter is companion-level metadata, not parsed by the spec validation pipeline -7. AI generation falls back to template on failure (with a warning to stderr) +7. AI generation falls back to template on failure (with a warning to stderr) and records the failure in `GenerationOutcome.ai_errors` so callers can exit non-zero 8. Source file paths in frontmatter are relative to the project root 9. Module source files are discovered by checking subdirectory-based modules first, then flat files @@ -56,7 +62,7 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, - **Given** a module "auth" that already has `specs/auth/auth.spec.md` - **When** `generate_specs_for_unspecced_modules` is called -- **Then** skips the module, returns 0 +- **Then** skips the module, returns an outcome with `generated == 0` ### Scenario: Design companion opt-in @@ -74,7 +80,7 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, - **Given** an AI provider that fails with an error - **When** generating a spec for module "auth" -- **Then** falls back to template-based generation and prints a warning +- **Then** falls back to template-based generation, prints a warning, and reports the failure in `GenerationOutcome.ai_errors` ## Error Cases @@ -82,7 +88,7 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, |-----------|----------| | Cannot create spec directory | Prints error to stderr, skips module | | Cannot write spec file | Prints error to stderr, skips module | -| AI generation fails | Falls back to template, prints warning | +| AI generation fails | Falls back to template, prints warning, records the error in `GenerationOutcome.ai_errors` | | No source files found for module | Skips module entirely | ## Dependencies @@ -111,3 +117,4 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, | 2026-04-12 | Update companion files list to include requirements.md, testing.md, and opt-in design.md; add design_enabled parameter | | 2026-04-13 | Fix generate_companion_files_from_template signature to include design_enabled; update scenario for conditional design.md | | 2026-06-07 | Replace unfinished-marker built-in template content with guided starter content | +| 2026-06-11 | Return `GenerationOutcome` (count, paths, AI errors) from both generation entry points so AI failures surface with a non-zero exit | diff --git a/specs/registry/registry.spec.md b/specs/registry/registry.spec.md index 249ac0a..5d0ffcf 100644 --- a/specs/registry/registry.spec.md +++ b/specs/registry/registry.spec.md @@ -1,6 +1,6 @@ --- module: registry -version: 2 +version: 3 status: stable files: - src/registry.rs @@ -39,9 +39,10 @@ Manages cross-project spec registries for dependency resolution. Generates `spec | `fetch_remote_registry` | `repo: &str` | `Result` | Fetch `specsync-registry.toml` from a GitHub repo's default branch via raw content URL | | `fetch_remote_spec` | `repo: &str, spec_path: &str` | `Result` | Fetch a spec file's raw content from a GitHub repo | | `parse_remote_spec` | `module: &str, content: &str` | `Option` | Parse fetched spec content into metadata for verification | -| `load_registry` | `root: &Path` | `Option` | Load a registry from a local `specsync-registry.toml` file | +| `local_registry_path` | `root: &Path` | `PathBuf` | Resolve the local registry path — prefers v4 `.specsync/registry.toml`, falls back to legacy root-level `specsync-registry.toml` for un-migrated 3.x projects | +| `load_registry` | `root: &Path` | `Option` | Load a registry from the local registry file resolved by `local_registry_path` | | `generate_registry` | `root, project_name, specs_dir` | `String` | Generate registry TOML content by scanning for spec files | -| `register_module` | `root, module_name, spec_rel_path` | `bool` | Append a module entry to the registry file; returns false if already exists or file missing | +| `register_module` | `root, module_name, spec_rel_path` | `bool` | Append a module entry to the registry file resolved by `local_registry_path`; returns false if already exists or file missing | ## Invariants @@ -52,6 +53,7 @@ Manages cross-project spec registries for dependency resolution. Generates `spec 5. Generated registry entries are sorted alphabetically by module name 6. `RemoteRegistry::has_spec` performs exact module name matching 7. TOML parsing is zero-dependency — uses line-by-line string parsing +8. Local registry resolution prefers the v4 `.specsync/registry.toml` location; the legacy root-level `specsync-registry.toml` is only used for un-migrated 3.x layouts ## Behavioral Examples @@ -104,3 +106,4 @@ Manages cross-project spec registries for dependency resolution. Generates `spec | 2026-03-25 | Initial spec | | 2026-04-07 | Document register_module function | | 2026-04-10 | v2: Added `fetch_remote_spec`, `parse_remote_spec`, `RemoteSpec`, `spec_path` for cross-repo content verification | +| 2026-06-11 | v3: Added `local_registry_path`; `load_registry`/`register_module` now resolve the v4 `.specsync/registry.toml` location with legacy fallback | diff --git a/specs/types/types.spec.md b/specs/types/types.spec.md index 40056de..ff2f1c6 100644 --- a/specs/types/types.spec.md +++ b/specs/types/types.spec.md @@ -1,6 +1,6 @@ --- module: types -version: 1 +version: 2 status: stable files: - src/types.rs @@ -36,7 +36,7 @@ Core data structures and enums shared across the entire spec-sync codebase. Defi | Type | Description | |------|-------------| | `Frontmatter` | YAML frontmatter parsed from a spec file (module, version, status, files, db_tables, depends_on, implements, tracks, agent_policy, lifecycle_log) | -| `ValidationResult` | Result of validating a single spec — errors, warnings, fixes, and export summary | +| `ValidationResult` | Result of validating a single spec — errors, warnings, fixes, export summary, and the spec's parsed lifecycle status (so reporters can surface status-based skips) | | `CoverageReport` | File and LOC coverage metrics for the project | | `SpecSyncConfig` | User-provided configuration loaded from specsync.json or .specsync.toml | | `RegistryEntry` | Registry entry mapping module names to spec file paths for cross-project resolution | @@ -99,7 +99,7 @@ Core data structures and enums shared across the entire spec-sync codebase. Defi 2. `AiProvider::detection_order` returns API providers only (by `_API_KEY` presence) — auto-detection never shells out to a CLI 3. `Language::from_extension` returns `None` for unsupported extensions — never panics 4. `SpecSyncConfig::default()` always provides sensible defaults (specs_dir="specs", source_dirs=["src"], 7 required sections) -5. `ValidationResult::new` initializes with empty error/warning/fix vectors +5. `ValidationResult::new` initializes with empty error/warning/fix vectors and `status: None` ## Behavioral Examples @@ -165,6 +165,7 @@ Core data structures and enums shared across the entire spec-sync codebase. Defi | Date | Change | |------|--------| +| 2026-06-11 | v2: `ValidationResult` carries the spec's parsed lifecycle status for draft-skip reporting | | 2026-03-25 | Initial spec | | 2026-03-28 | Document OutputFormat, ExportLevel, ModuleDefinition | | 2026-04-06 | Add Frontmatter implements/tracks/agent_policy fields, ValidationRules, GitHubConfig structs | diff --git a/specs/validator/validator.spec.md b/specs/validator/validator.spec.md index 9f7200b..645d2c2 100644 --- a/specs/validator/validator.spec.md +++ b/specs/validator/validator.spec.md @@ -1,6 +1,6 @@ --- module: validator -version: 2 +version: 3 status: stable files: - src/validator.rs @@ -45,6 +45,7 @@ Core validation engine for spec-sync. Validates individual spec files against so 8. File suggestions use Levenshtein distance (max 3) when a referenced source file is missing 9. Flat source files (e.g. `src/config.rs`) are detected as modules, excluding common entry points (main, lib, mod, index, app, `__init__`) 10. Sections with no substantive content are reported as unfinished draft text rather than as template markers +11. `validate_spec` records the spec's parsed lifecycle status on `ValidationResult.status` (None when frontmatter is unreadable) so reporters can surface status-based skips, e.g. drafts skipping section and export checks ## Behavioral Examples @@ -108,6 +109,7 @@ Core validation engine for spec-sync. Validates individual spec files against so | Date | Change | |------|--------| +| 2026-06-11 | v3: `validate_spec` populates `ValidationResult.status` with the parsed lifecycle status so callers can report draft skips | | 2026-06-07 | Update draft-only section warning wording | | 2026-03-25 | Initial spec | | 2026-04-06 | Document archive, compact, merge as consumers of find_spec_files; note hash_cache integration for incremental validation | diff --git a/specs/watch/watch.spec.md b/specs/watch/watch.spec.md index 8818a26..f8451e9 100644 --- a/specs/watch/watch.spec.md +++ b/specs/watch/watch.spec.md @@ -1,6 +1,6 @@ --- module: watch -version: 1 +version: 2 status: stable files: - src/watch.rs @@ -34,6 +34,7 @@ File watcher that re-runs `specsync check` on file changes. Watches spec and sou 6. Check is run as a child process (fork of current executable) to isolate exit calls 7. Screen is cleared before each re-run for clean output 8. Changed file path is displayed in the separator header +9. The pass/fail footer reflects the actual check results — the child's stdout is streamed through and its summary line is parsed, because `enforcement = warn` (the default) exits 0 even when specs failed ## Behavioral Examples @@ -62,7 +63,7 @@ File watcher that re-runs `specsync check` on file changes. Watches spec and sou | No directories to watch | Prints error, exits with code 1 | | Watcher creation fails | Panics with "Failed to create file watcher" | | Individual dir watch fails | Prints warning, continues watching other dirs | -| Check command fails | Prints "Some checks failed", continues watching | +| Check command fails or any spec failed | Prints "Some checks failed", continues watching | ## Dependencies @@ -84,3 +85,4 @@ File watcher that re-runs `specsync check` on file changes. Watches spec and sou | Date | Change | |------|--------| | 2026-03-25 | Initial spec | +| 2026-06-11 | v2: Footer parses the check summary line so it never prints "All checks passed!" under a failing report | diff --git a/src/commands/check.rs b/src/commands/check.rs index b109b57..1441117 100644 --- a/src/commands/check.rs +++ b/src/commands/check.rs @@ -69,6 +69,32 @@ pub fn cmd_check( config.enforcement }); + // Spec name filters that matched nothing are an error — don't fall through + // to the misleading "No spec files found" message when specs do exist. + if spec_files.is_empty() && !spec_filters.is_empty() && !all_spec_files.is_empty() { + match format { + Json => { + let output = serde_json::json!({ + "passed": false, + "errors": [format!("No specs matched: {}", spec_filters.join(", "))], + "warnings": [], + "stale": [], + "specs_checked": 0, + }); + println!("{}", serde_json::to_string_pretty(&output).unwrap()); + } + _ => { + // filter_specs already printed the "No specs matched: ..." warning + eprintln!( + "{} no specs matched the given filter(s) ({} spec file(s) exist)", + "error:".red().bold(), + all_spec_files.len() + ); + } + } + process::exit(1); + } + if spec_files.is_empty() { match format { Json => { @@ -560,6 +586,61 @@ fn auto_regen_stale_specs( use crate::util::levenshtein; +/// Build "### Exported Functions"/"### Exported Types" subsection blocks for +/// auto-added export rows, omitting empty groups. +fn build_export_subsections(value_rows: &str, type_rows: &str) -> String { + let mut block = String::new(); + if !value_rows.is_empty() { + block.push_str(&format!( + "\n\n### Exported Functions\n\n| Export | Description |\n|--------|-------------|\n{value_rows}" + )); + } + if !type_rows.is_empty() { + block.push_str(&format!( + "\n\n### Exported Types\n\n| Type | Description |\n|------|-------------|\n{type_rows}" + )); + } + block +} + +/// Markdown row for one auto-added export. When the target table's column +/// count is known, the symbol goes in the first cell, the guidance text in the +/// last, and any middle cells get a `TODO` placeholder. +fn build_fix_row( + name: &str, + columns: Option, + primary_lang: Option, +) -> String { + const GUIDANCE: &str = "Document caller-visible behavior and constraints."; + match columns { + Some(n) if n > 2 => { + let middle = "TODO | ".repeat(n - 2); + format!("| `{name}` | {middle}{GUIDANCE} |") + } + Some(_) => format!("| `{name}` | {GUIDANCE} |"), + None => match primary_lang { + Some(types::Language::Swift) + | Some(types::Language::Kotlin) + | Some(types::Language::Java) => { + format!("| `{name}` | Type or member kind | {GUIDANCE} |") + } + _ => format!("| `{name}` | {GUIDANCE} |"), + }, + } +} + +/// Column count of the first markdown table row found in `section`, if any. +fn table_column_count(section: &str) -> Option { + section.lines().find_map(|line| { + let trimmed = line.trim(); + if trimmed.len() > 2 && trimmed.starts_with('|') && trimmed.ends_with('|') { + Some(trimmed[1..trimmed.len() - 1].split('|').count()) + } else { + None + } + }) +} + /// Normalize near-miss export headers within ## Public API. /// E.g., "### Exportd Functions" → "### Exported Functions" /// Uses Levenshtein distance ≤ 2 against a canonical list to catch typos, @@ -742,27 +823,37 @@ fn auto_fix_specs( }) .next(); - // Build new rows with language-appropriate columns - let new_rows: String = undocumented + // Classify undocumented exports as types vs functions/values so new + // rows land in the matching table — functions must not be appended to + // an "Exported Types" table. + let mut type_names: std::collections::HashSet = std::collections::HashSet::new(); + for file in &parsed.frontmatter.files { + let full_path = root.join(file); + type_names.extend(get_exported_symbols_full( + &full_path, + types::ExportLevel::Type, + config.parse_mode, + )); + } + let (type_syms, value_syms): (Vec<&str>, Vec<&str>) = undocumented .iter() - .map(|name| match primary_lang { - Some(types::Language::Swift) - | Some(types::Language::Kotlin) - | Some(types::Language::Java) => { - format!("| `{name}` | Type or member kind | Document caller-visible behavior and constraints. |") - } - Some(types::Language::Rust) => { - format!("| `{name}` | Document caller-visible behavior and constraints. |") - } - _ => format!("| `{name}` | Document caller-visible behavior and constraints. |"), - }) - .collect::>() - .join("\n"); + .copied() + .partition(|s| type_names.contains(*s)); + + let build_rows = |syms: &[&str], columns: Option| -> String { + syms.iter() + .map(|name| build_fix_row(name, columns, primary_lang)) + .collect::>() + .join("\n") + }; - // Find insertion point: end of the last recognized export subsection within - // ## Public API, so new rows land where get_spec_symbols will find them. - // Inserting at the end of the whole section causes duplicates when non-export - // subsections (e.g. ### API Endpoints) come after the export table. + // Find insertion points: type exports go to the last "… Types" export + // subsection within ## Public API, everything else to the last + // "… Functions"/"… Methods" subsection (falling back to the last + // recognized export subsection), so new rows land where + // get_spec_symbols will find them. Inserting at the end of the whole + // section causes duplicates when non-export subsections + // (e.g. ### API Endpoints) come after the export table. let mut new_content = content.clone(); if let Some(api_start) = content.find("## Public API") { let after = &content[api_start..]; @@ -776,63 +867,106 @@ fn auto_fix_specs( let sub_positions: Vec = sub_re.find_iter(api_section).map(|m| m.start()).collect(); - // Find the absolute end of the last recognized export subsection - let export_insert = sub_positions + // Recognized export subsections: (lowercased header, absolute start, absolute end) + let export_subs: Vec<(String, usize, usize)> = sub_positions .iter() .enumerate() - .rev() - .find_map(|(i, &rel_pos)| { + .filter_map(|(i, &rel_pos)| { let header_line = api_section[rel_pos..].lines().next().unwrap_or(""); if crate::parser::is_export_header(header_line) { let rel_end = sub_positions .get(i + 1) .copied() .unwrap_or(api_section.len()); - Some(api_start + rel_end) + Some(( + header_line.to_ascii_lowercase(), + api_start + rel_pos, + api_start + rel_end, + )) } else { None } - }); + }) + .collect(); - match export_insert { - Some(pos) => { - // Append rows at end of last recognized export subsection - new_content = format!( - "{}\n{}\n{}", - content[..pos].trim_end(), - new_rows, - &content[pos..] - ); + if !export_subs.is_empty() { + let type_target = export_subs + .iter() + .rposition(|(header, _, _)| header.contains("type")); + let value_target = export_subs + .iter() + .rposition(|(header, _, _)| { + header.contains("function") || header.contains("method") + }) + .or_else(|| { + // No functions table — use the last export subsection + // that is not the types table + export_subs + .iter() + .enumerate() + .rev() + .find(|(i, _)| Some(*i) != type_target) + .map(|(i, _)| i) + }) + .unwrap_or(export_subs.len() - 1); + let type_target = type_target.unwrap_or(value_target); + + // Group symbols per target subsection (a group may be empty) + let mut syms_by_target: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for sym in &value_syms { + syms_by_target.entry(value_target).or_default().push(sym); } - None if sub_positions.is_empty() => { - // No ### subsections — flat table or empty body; insert at section end - new_content = format!( - "{}\n{}\n{}", - content[..api_end].trim_end(), - new_rows, - &content[api_end..] - ); + for sym in &type_syms { + syms_by_target.entry(type_target).or_default().push(sym); } - None => { - // Has subsections but none are recognized export headers; - // create a new ### Exported Functions subsection at the top of the section - let api_header_end = - api_start + api_section.find('\n').unwrap_or(api_section.len()); - let header_block = format!( - "\n\n### Exported Functions\n\n| Export | Description |\n|--------|-------------|\n{new_rows}" - ); + + // Apply insertions in descending offset order so earlier + // offsets remain valid as the content grows + for (&target, syms) in syms_by_target.iter().rev() { + let (_, start, end) = export_subs[target]; + let columns = table_column_count(&content[start..end]); + let rows = build_rows(syms, columns); new_content = format!( - "{}{}{}", - &content[..api_header_end], - header_block, - &content[api_header_end..] + "{}\n{}\n{}", + new_content[..end].trim_end(), + rows, + &new_content[end..] ); } + } else if sub_positions.is_empty() { + // No ### subsections — flat table or empty body; insert at section end + let rows = build_rows(&undocumented, table_column_count(api_section)); + new_content = format!( + "{}\n{}\n{}", + content[..api_end].trim_end(), + rows, + &content[api_end..] + ); + } else { + // Has subsections but none are recognized export headers; + // create new export subsections at the top of the section + let api_header_end = + api_start + api_section.find('\n').unwrap_or(api_section.len()); + let header_block = build_export_subsections( + &build_rows(&value_syms, Some(2)), + &build_rows(&type_syms, Some(2)), + ); + new_content = format!( + "{}{}{}", + &content[..api_header_end], + header_block, + &content[api_header_end..] + ); } } else { // No Public API section — append one let section = format!( - "\n## Public API\n\n### Exported Functions\n\n| Export | Description |\n|--------|-------------|\n{new_rows}\n" + "\n## Public API{}\n", + build_export_subsections( + &build_rows(&value_syms, Some(2)), + &build_rows(&type_syms, Some(2)), + ) ); new_content.push_str(§ion); } diff --git a/src/commands/generate.rs b/src/commands/generate.rs index d109e30..1e0819c 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -150,17 +150,18 @@ fn cmd_generate_all( let ai = resolved_provider.is_some(); if json { - let generated_paths = generate_specs_for_unspecced_modules_paths( + let outcome = generate_specs_for_unspecced_modules_paths( root, &coverage, &config, resolved_provider.as_ref(), ); let output = serde_json::json!({ - "generated": generated_paths, + "generated": outcome.generated_paths, + "ai_errors": outcome.ai_errors, }); println!("{}", serde_json::to_string_pretty(&output).unwrap()); - process::exit(0); + process::exit(if outcome.ai_errors.is_empty() { 0 } else { 1 }); } print_coverage_report(&coverage); @@ -183,8 +184,9 @@ fn cmd_generate_all( ); } - let generated = + let outcome = generate_specs_for_unspecced_modules(root, &coverage, &config, resolved_provider.as_ref()); + let generated = outcome.generated; if generated == 0 && coverage.unspecced_modules.is_empty() { println!( " {} No specs to generate — full module coverage", @@ -221,6 +223,7 @@ fn cmd_generate_all( print_summary(total, passed, total_warnings, total_errors); print_coverage_line(&coverage); + report_ai_failures_and_exit(&outcome.ai_errors); exit_with_status( total_errors, total_warnings, @@ -231,6 +234,27 @@ fn cmd_generate_all( ); } +/// If any AI generation failures occurred, print them prominently on stderr +/// (last, after the check report) and exit non-zero. No-op when empty. +fn report_ai_failures_and_exit(ai_errors: &[String]) { + if ai_errors.is_empty() { + return; + } + eprintln!(); + for e in ai_errors { + eprintln!(" {} {e}", "✗".red()); + } + eprintln!( + "{} AI generation failed for {} module(s) — template fallback was used.", + "✗".red().bold(), + ai_errors.len() + ); + eprintln!( + " Check your provider configuration (API key, --provider, aiProvider/aiCommand) and re-run." + ); + process::exit(1); +} + /// Generate specs for a specific batch of module names. /// Parses comma-separated values within each entry (e.g. "foo,bar" or ["foo", "bar"]). #[allow(clippy::too_many_arguments)] @@ -299,7 +323,7 @@ fn cmd_generate_batch( unspecced_modules: to_generate.clone(), ..coverage.clone() }; - let generated_paths = generate_specs_for_unspecced_modules_paths( + let outcome = generate_specs_for_unspecced_modules_paths( root, &filtered_coverage, &config, @@ -307,12 +331,13 @@ fn cmd_generate_batch( ); let output = serde_json::json!({ "requested": modules, - "generated": generated_paths, + "generated": outcome.generated_paths, + "ai_errors": outcome.ai_errors, "skipped_already_specced": already_specced, "skipped_not_found": not_found, }); println!("{}", serde_json::to_string_pretty(&output).unwrap()); - process::exit(0); + process::exit(if outcome.ai_errors.is_empty() { 0 } else { 1 }); } println!( @@ -338,6 +363,7 @@ fn cmd_generate_batch( ); } + let mut ai_errors: Vec = Vec::new(); if to_generate.is_empty() { println!(" {} Nothing to generate.", "i".blue()); } else { @@ -353,17 +379,18 @@ fn cmd_generate_batch( ..coverage }; - let generated = generate_specs_for_unspecced_modules( + let outcome = generate_specs_for_unspecced_modules( root, &filtered_coverage, &config, resolved_provider.as_ref(), ); + ai_errors = outcome.ai_errors; println!( "\n {} Batch generate complete: {}/{} spec(s) generated", "✓".green(), - generated, + outcome.generated, to_generate.len() ); } @@ -388,6 +415,7 @@ fn cmd_generate_batch( ); print_summary(total, passed, total_warnings, total_errors); + report_ai_failures_and_exit(&ai_errors); exit_with_status( total_errors, total_warnings, diff --git a/src/commands/init.rs b/src/commands/init.rs index f1cb754..9008b2e 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -3,48 +3,51 @@ use std::fs; use std::path::Path; use std::process; -use crate::config::detect_source_dirs; +use crate::config::{config_to_toml, detect_source_dirs}; +use crate::types::SpecSyncConfig; + +/// Version stamp written to `.specsync/version` for fresh v4 projects. +const V4_VERSION: &str = "4.0.0"; + +/// Subdirectories created inside `.specsync/` for a fresh v4 project. +const V4_DIRS: &[&str] = &[ + ".specsync/lifecycle", + ".specsync/changes", + ".specsync/archive", +]; pub fn cmd_init(root: &Path) { - let config_path = root.join("specsync.json"); - let toml_path = root.join(".specsync.toml"); - if config_path.exists() { - println!("specsync.json already exists"); + // Refuse to clobber any existing config — v4 or legacy. + let v4_toml = root.join(".specsync/config.toml"); + let v4_json = root.join(".specsync/config.json"); + let legacy_json = root.join("specsync.json"); + let legacy_toml = root.join(".specsync.toml"); + if v4_toml.exists() { + println!(".specsync/config.toml already exists"); + return; + } + if v4_json.exists() { + println!(".specsync/config.json already exists"); + return; + } + if legacy_json.exists() { + println!("specsync.json already exists (legacy 3.x layout — run `specsync migrate`)"); return; } - if toml_path.exists() { - println!(".specsync.toml already exists"); + if legacy_toml.exists() { + println!(".specsync.toml already exists (legacy 3.x layout — run `specsync migrate`)"); return; } let detected_dirs = detect_source_dirs(root); let dirs_display = detected_dirs.join(", "); - let default = serde_json::json!({ - "specsDir": "specs", - "sourceDirs": detected_dirs, - "requiredSections": [ - "Purpose", - "Public API", - "Invariants", - "Behavioral Examples", - "Error Cases", - "Dependencies", - "Change Log" - ], - "excludeDirs": ["__tests__"], - "excludePatterns": ["**/__tests__/**", "**/*.test.ts", "**/*.spec.ts"] - }); - - let content = serde_json::to_string_pretty(&default).unwrap() + "\n"; - if let Err(e) = fs::write(&config_path, content) { - eprintln!( - "{} Failed to write specsync.json: {e}", - "error:".red().bold() - ); + if let Err(e) = write_v4_layout(root, &detected_dirs) { + eprintln!("{} {e}", "error:".red().bold()); process::exit(1); } - println!("{} Created specsync.json", "✓".green()); + + println!("{} Created .specsync/config.toml (v4 layout)", "✓".green()); println!(" Detected source directories: {dirs_display}"); // Ensure .specsync/hashes.json is gitignored (hash cache is local-only) @@ -55,6 +58,46 @@ pub fn cmd_init(root: &Path) { } } +/// Create the v4 `.specsync/` directory structure: directories, `config.toml`, +/// `version` stamp, and `.specsync/.gitignore`. Mirrors what `specsync migrate` +/// produces so a fresh `init` never triggers the legacy-layout migration nag. +fn write_v4_layout(root: &Path, source_dirs: &[String]) -> Result<(), String> { + for dir in V4_DIRS { + fs::create_dir_all(root.join(dir)).map_err(|e| format!("Failed to create {dir}: {e}"))?; + } + + let config = SpecSyncConfig { + source_dirs: source_dirs.to_vec(), + ..Default::default() + }; + let config_path = root.join(".specsync/config.toml"); + fs::write(&config_path, config_to_toml(&config)) + .map_err(|e| format!("Failed to write .specsync/config.toml: {e}"))?; + + let version_path = root.join(".specsync/version"); + fs::write(&version_path, format!("{V4_VERSION}\n")) + .map_err(|e| format!("Failed to write .specsync/version: {e}"))?; + + let gitignore_path = root.join(".specsync/.gitignore"); + if !gitignore_path.exists() { + let content = [ + "# spec-sync v4 — generated by `specsync init`", + "# Committed: config.toml, registry.toml, lifecycle/, changes/, archive/", + "# Ignored: backups, local config, hash cache (regenerated on each run)", + "", + "backup-3x/", + "config.local.toml", + "hashes.json", + "", + ] + .join("\n"); + fs::write(&gitignore_path, content) + .map_err(|e| format!("Failed to write .specsync/.gitignore: {e}"))?; + } + + Ok(()) +} + /// Append `.specsync/hashes.json` to the root `.gitignore` if not already present. /// Returns `Ok(true)` if added, `Ok(false)` if already present, `Err` on write failure. pub fn ensure_hashes_gitignored(root: &Path) -> Result { @@ -121,4 +164,36 @@ mod tests { assert!(result.is_err()); assert!(result.unwrap_err().contains("Failed to update .gitignore")); } + + #[test] + fn write_v4_layout_creates_full_structure() { + let tmp = TempDir::new().unwrap(); + write_v4_layout(tmp.path(), &["src".to_string()]).unwrap(); + + assert!(tmp.path().join(".specsync/config.toml").exists()); + assert!(tmp.path().join(".specsync/version").exists()); + assert!(tmp.path().join(".specsync/.gitignore").exists()); + assert!(tmp.path().join(".specsync/lifecycle").is_dir()); + assert!(tmp.path().join(".specsync/changes").is_dir()); + assert!(tmp.path().join(".specsync/archive").is_dir()); + + let version = fs::read_to_string(tmp.path().join(".specsync/version")).unwrap(); + assert_eq!(version.trim(), V4_VERSION); + + let config = fs::read_to_string(tmp.path().join(".specsync/config.toml")).unwrap(); + assert!(config.contains("specs_dir = \"specs\"")); + assert!(config.contains("source_dirs = [\"src\"]")); + assert!(config.contains("required_sections")); + } + + #[test] + fn fresh_init_is_not_legacy_layout() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("src")).unwrap(); + cmd_init(tmp.path()); + assert!( + !crate::config::is_legacy_layout(tmp.path()), + "fresh init must produce a v4 layout that does not trigger the migration nag" + ); + } } diff --git a/src/commands/init_registry.rs b/src/commands/init_registry.rs index 4de749b..7c842b3 100644 --- a/src/commands/init_registry.rs +++ b/src/commands/init_registry.rs @@ -7,9 +7,16 @@ use crate::config::load_config; use crate::registry; pub fn cmd_init_registry(root: &Path, name: Option) { - let registry_path = root.join("specsync-registry.toml"); + // Respect the project layout: v4 projects get .specsync/registry.toml, + // un-migrated 3.x projects keep the legacy root-level file. + let registry_path = registry::local_registry_path(root); + let rel_display = registry_path + .strip_prefix(root) + .unwrap_or(®istry_path) + .display() + .to_string(); if registry_path.exists() { - println!("specsync-registry.toml already exists"); + println!("{rel_display} already exists"); return; } @@ -22,12 +29,18 @@ pub fn cmd_init_registry(root: &Path, name: Option) { }); let content = registry::generate_registry(root, &project_name, &config.specs_dir); + if let Some(parent) = registry_path.parent() + && let Err(e) = fs::create_dir_all(parent) + { + eprintln!("Failed to create {}: {e}", parent.display()); + process::exit(1); + } match fs::write(®istry_path, &content) { Ok(_) => { - println!("{} Created specsync-registry.toml", "✓".green()); + println!("{} Created {rel_display}", "✓".green()); } Err(e) => { - eprintln!("Failed to write specsync-registry.toml: {e}"); + eprintln!("Failed to write {rel_display}: {e}"); process::exit(1); } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index f16304d..08a01c1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -235,11 +235,16 @@ pub fn run_validation( let mut total_errors = 0; let mut total_warnings = 0; let mut passed = 0; + let mut drafts_skipped = 0; let mut all_errors: Vec = Vec::new(); let mut all_warnings: Vec = Vec::new(); for spec_file in spec_files { let result = validate_spec(spec_file, root, schema_tables, schema_columns, config); + let is_draft = result.status == Some(SpecStatus::Draft); + if is_draft { + drafts_skipped += 1; + } // Parse inline ignore directives from the spec file let inline_ignores = std::fs::read_to_string(spec_file) @@ -276,7 +281,7 @@ pub fn run_validation( .iter() .any(|e| e.starts_with("Frontmatter") || e.starts_with("Missing or malformed")); if has_fm_errors { - println!(" {} Frontmatter valid", "✗".red()); + println!(" {} Frontmatter invalid", "✗".red()); } else { println!(" {} Frontmatter valid", "✓".green()); } @@ -333,13 +338,20 @@ pub fn run_validation( } // Section check + // Drafts skip required-section validation by design — say so instead of + // printing a misleading "all sections present" checkmark. let section_errors: Vec<&str> = result .errors .iter() .filter(|e| e.starts_with("Missing required section")) .map(|s| s.as_str()) .collect(); - if section_errors.is_empty() { + if is_draft { + println!( + " {} Section validation skipped (status: draft)", + "⊘".yellow() + ); + } else if section_errors.is_empty() { println!(" {} All required sections present", "✓".green()); } else { for e in §ion_errors { @@ -348,6 +360,7 @@ pub fn run_validation( } // API surface + // Drafts skip export drift detection by design — make that visible. let api_line = warnings.iter().find(|w| { w.contains("exports documented") && w.chars() @@ -355,7 +368,12 @@ pub fn run_validation( .map(|c| c.is_ascii_digit()) .unwrap_or(false) }); - if let Some(line) = api_line { + if is_draft { + println!( + " {} Export validation skipped (status: draft)", + "⊘".yellow() + ); + } else if let Some(line) = api_line { println!(" {} {line}", "✓".green()); } else if let Some(ref summary) = result.export_summary { println!(" {} {summary}", "✓".green()); @@ -494,6 +512,13 @@ pub fn run_validation( } } + if !collect && drafts_skipped > 0 { + println!( + "\n{} {drafts_skipped} draft spec(s) skipped section and export validation — set `status: active` to enable full checks", + "ℹ".yellow() + ); + } + ( total_errors, total_warnings, diff --git a/src/generator.rs b/src/generator.rs index 02709dd..0859f38 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -679,7 +679,9 @@ pub fn generate_spec( } /// Generate spec content for a module, using AI if a provider is configured. -/// Returns `(spec_content, ai_was_used)`. +/// Returns `(spec_content, ai_was_used, ai_error)` — `ai_error` is `Some` when +/// an AI provider was requested but generation failed and the template +/// fallback was used instead. fn generate_module_spec( module_name: &str, module_files: &[String], @@ -687,7 +689,8 @@ fn generate_module_spec( specs_dir: &Path, config: &SpecSyncConfig, provider: Option<&ResolvedProvider>, -) -> (String, bool) { +) -> (String, bool, Option) { + let mut ai_error: Option = None; if let Some(provider) = provider { // Make paths relative to root for the AI prompt let rel_files: Vec = module_files @@ -701,12 +704,13 @@ fn generate_module_spec( .collect(); match ai::generate_spec_with_ai(module_name, &rel_files, root, config, provider) { - Ok(spec) => return (spec, true), + Ok(spec) => return (spec, true, None), Err(e) => { eprintln!( " {} AI generation failed for {module_name}: {e} — falling back to template", "⚠".yellow() ); + ai_error = Some(format!("AI generation failed for {module_name}: {e}")); } } } @@ -714,6 +718,7 @@ fn generate_module_spec( ( generate_spec(module_name, module_files, root, specs_dir), false, + ai_error, ) } @@ -933,16 +938,28 @@ pub fn generate_companion_files_from_template( } } +/// Outcome of a spec-generation run. +#[derive(Debug, Default)] +pub struct GenerationOutcome { + /// Number of spec files written. + pub generated: usize, + /// Paths (relative to root) of the spec files written. + pub generated_paths: Vec, + /// AI generation failures (one entry per module that fell back to the template). + pub ai_errors: Vec, +} + /// Generate spec files for all unspecced modules. -/// Returns the number of specs generated. +/// Returns the generation outcome, including any AI failures that fell back +/// to the template so callers can exit non-zero. pub fn generate_specs_for_unspecced_modules( root: &Path, report: &CoverageReport, config: &SpecSyncConfig, provider: Option<&ResolvedProvider>, -) -> usize { +) -> GenerationOutcome { let specs_dir = root.join(&config.specs_dir); - let mut generated = 0; + let mut outcome = GenerationOutcome::default(); for module_name in &report.unspecced_modules { let spec_dir = specs_dir.join(module_name); @@ -968,7 +985,7 @@ pub fn generate_specs_for_unspecced_modules( eprintln!(" Generating {rel} with AI..."); } - let (spec_content, ai_used) = generate_module_spec( + let (spec_content, ai_used, ai_error) = generate_module_spec( module_name, &module_files, root, @@ -976,23 +993,30 @@ pub fn generate_specs_for_unspecced_modules( config, provider, ); + if let Some(e) = ai_error { + outcome.ai_errors.push(e); + } match fs::write(&spec_file, &spec_content) { Ok(_) => { - let rel = spec_file.strip_prefix(root).unwrap_or(&spec_file).display(); + let rel = spec_file.strip_prefix(root).unwrap_or(&spec_file); let from = if provider.is_some() && !ai_used { " from template" } else { "" }; println!( - " {} Generated {rel}{from} ({} files)", + " {} Generated {}{from} ({} files)", "✓".green(), + rel.display(), module_files.len() ); generate_companion_files(&spec_dir, module_name, config.companions.design); let _ = std::io::stdout().flush(); - generated += 1; + outcome.generated += 1; + outcome + .generated_paths + .push(rel.to_string_lossy().to_string()); } Err(e) => { eprintln!(" Failed to write {}: {e}", spec_file.display()); @@ -1000,18 +1024,20 @@ pub fn generate_specs_for_unspecced_modules( } } - generated + outcome } -/// Generate spec files for all unspecced modules, returning the paths of generated files. +/// Generate spec files for all unspecced modules without per-file progress +/// output (used by JSON and MCP callers). Returns the generation outcome, +/// including the paths of the spec files written. pub fn generate_specs_for_unspecced_modules_paths( root: &Path, report: &CoverageReport, config: &SpecSyncConfig, provider: Option<&ResolvedProvider>, -) -> Vec { +) -> GenerationOutcome { let specs_dir = root.join(&config.specs_dir); - let mut generated_paths = Vec::new(); + let mut outcome = GenerationOutcome::default(); for module_name in &report.unspecced_modules { let spec_dir = specs_dir.join(module_name); @@ -1031,7 +1057,7 @@ pub fn generate_specs_for_unspecced_modules_paths( continue; } - let (spec_content, _ai_used) = generate_module_spec( + let (spec_content, _ai_used, ai_error) = generate_module_spec( module_name, &module_files, root, @@ -1039,6 +1065,9 @@ pub fn generate_specs_for_unspecced_modules_paths( config, provider, ); + if let Some(e) = ai_error { + outcome.ai_errors.push(e); + } if fs::write(&spec_file, &spec_content).is_ok() { let rel = spec_file @@ -1046,11 +1075,12 @@ pub fn generate_specs_for_unspecced_modules_paths( .unwrap_or(&spec_file) .to_string_lossy() .to_string(); - generated_paths.push(rel); + outcome.generated += 1; + outcome.generated_paths.push(rel); } } - generated_paths + outcome } #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index 1f83c2d..bee6812 100644 --- a/src/main.rs +++ b/src/main.rs @@ -61,6 +61,14 @@ fn run() { let root = cli .root .unwrap_or_else(|| std::env::current_dir().expect("Cannot determine cwd")); + if !root.is_dir() { + eprintln!( + "{} --root path does not exist or is not a directory: {}", + "error:".red().bold(), + root.display() + ); + process::exit(2); + } let root = root.canonicalize().unwrap_or(root); // --json flag is shorthand for --format json (backward compat) diff --git a/src/mcp.rs b/src/mcp.rs index 9777fc3..aea95b7 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -705,7 +705,7 @@ fn tool_generate(root: &Path, arguments: &Value) -> Result { None }; - let generated_paths = generate_specs_for_unspecced_modules_paths( + let outcome = generate_specs_for_unspecced_modules_paths( root, &coverage, &config, @@ -713,8 +713,9 @@ fn tool_generate(root: &Path, arguments: &Value) -> Result { ); Ok(json!({ - "generated": generated_paths, - "count": generated_paths.len(), + "generated": outcome.generated_paths, + "count": outcome.generated, + "ai_errors": outcome.ai_errors, })) } diff --git a/src/registry.rs b/src/registry.rs index 6dcc872..c8b37de 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -1,10 +1,33 @@ use crate::types::RegistryEntry; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::time::Duration; use walkdir::WalkDir; const REGISTRY_FILENAME: &str = "specsync-registry.toml"; +const V4_REGISTRY_RELATIVE: &str = ".specsync/registry.toml"; + +/// Resolve the local registry path for a project. +/// Prefers the v4 location (`.specsync/registry.toml`) when it exists or when +/// the project uses the v4 layout; falls back to the legacy root-level +/// `specsync-registry.toml` only for un-migrated 3.x projects. +pub fn local_registry_path(root: &Path) -> PathBuf { + let v4_path = root.join(V4_REGISTRY_RELATIVE); + if v4_path.exists() { + return v4_path; + } + let legacy_path = root.join(REGISTRY_FILENAME); + if legacy_path.exists() { + return legacy_path; + } + // Neither exists yet: default to the v4 location unless the project is + // still on the legacy 3.x layout (root-level config, no version stamp). + if crate::config::is_legacy_layout(root) { + legacy_path + } else { + v4_path + } +} /// A parsed remote registry (fetched over HTTPS). #[derive(Debug, Clone)] @@ -128,10 +151,11 @@ pub fn fetch_remote_registry(repo: &str) -> Result { }) } -/// Load a registry from a `specsync-registry.toml` file. +/// Load a registry from the local registry file +/// (`.specsync/registry.toml`, falling back to legacy `specsync-registry.toml`). #[allow(dead_code)] pub fn load_registry(root: &Path) -> Option { - let path = root.join(REGISTRY_FILENAME); + let path = local_registry_path(root); let content = fs::read_to_string(&path).ok()?; parse_registry(&content) } @@ -226,11 +250,12 @@ pub fn generate_registry(root: &Path, project_name: &str, specs_dir: &str) -> St output } -/// Add a module entry to an existing `specsync-registry.toml`. +/// Add a module entry to an existing local registry file +/// (`.specsync/registry.toml`, falling back to legacy `specsync-registry.toml`). /// If the module already exists, it is not duplicated. /// Returns `true` if the entry was added, `false` if it already existed or the file is missing. pub fn register_module(root: &Path, module_name: &str, spec_rel_path: &str) -> bool { - let path = root.join(REGISTRY_FILENAME); + let path = local_registry_path(root); let content = match fs::read_to_string(&path) { Ok(c) => c, Err(_) => return false, diff --git a/src/types.rs b/src/types.rs index 55cdfaf..aef0224 100644 --- a/src/types.rs +++ b/src/types.rs @@ -344,6 +344,10 @@ pub struct ValidationResult { pub export_summary: Option, /// Actionable fix suggestions mapped to errors. pub fixes: Vec, + /// Parsed lifecycle status of the spec (None if frontmatter was unreadable). + /// Lets reporters surface checks that were skipped because of status + /// (e.g. drafts skip section and export validation). + pub status: Option, } impl ValidationResult { @@ -354,6 +358,7 @@ impl ValidationResult { warnings: Vec::new(), export_summary: None, fixes: Vec::new(), + status: None, } } } diff --git a/src/validator.rs b/src/validator.rs index f642030..c773906 100644 --- a/src/validator.rs +++ b/src/validator.rs @@ -180,6 +180,7 @@ pub fn validate_spec( let fm = &parsed.frontmatter; let body = &parsed.body; + result.status = fm.parsed_status(); // File size guard: warn if the spec exceeds the configurable limit (default 512 KB) { diff --git a/src/watch.rs b/src/watch.rs index 3e84507..a6f946b 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -175,7 +175,8 @@ fn build_check_args( fn run_check(root: &Path, strict: bool, require_coverage: Option, force: bool) { // Fork a child process to isolate exit calls from the check command. - use std::process::Command; + use std::io::{BufRead, BufReader, IsTerminal}; + use std::process::{Command, Stdio}; let start = Instant::now(); let args = build_check_args(root, strict, require_coverage, force); @@ -184,19 +185,51 @@ fn run_check(root: &Path, strict: bool, require_coverage: Option, force: cmd.arg(arg); } - match cmd.status() { + // Pipe the child's stdout so the summary line can be inspected — the exit + // code alone is not enough because `enforcement = warn` (the default) + // exits 0 even when specs failed. Keep colors on when we're on a TTY. + cmd.stdout(Stdio::piped()); + if std::io::stdout().is_terminal() { + cmd.env("CLICOLOR_FORCE", "1"); + } + + let mut child = match cmd.spawn() { + Ok(c) => c, + Err(e) => { + eprintln!("{} Failed to run check: {e}", "Error:".red()); + return; + } + }; + + // Stream the child's output through while scanning for the summary line. + let mut failed_specs: Option = None; + if let Some(stdout) = child.stdout.take() { + for line in BufReader::new(stdout).lines() { + let line = match line { + Ok(l) => l, + Err(_) => break, + }; + println!("{line}"); + if let Some(failed) = parse_failed_count(&line) { + failed_specs = Some(failed); + } + } + } + + match child.wait() { Ok(status) => { let elapsed = start.elapsed(); - if status.success() { + let failed = !status.success() || failed_specs.unwrap_or(0) > 0; + if failed { println!( "\n{} ({}ms)", - "All checks passed!".green().bold(), + "Some checks failed.".red().bold(), elapsed.as_millis() ); } else { println!( "\n{} ({}ms)", - "Some checks failed.".red().bold(), + "All checks passed!".green().bold(), elapsed.as_millis() ); } @@ -207,6 +240,39 @@ fn run_check(root: &Path, strict: bool, require_coverage: Option, force: } } +/// Parse the failed-spec count from a check summary line like +/// `"3 specs checked: 2 passed, 1 warning(s), 1 failed"`. +/// Returns `None` for lines that are not the summary. +fn parse_failed_count(line: &str) -> Option { + let plain = strip_ansi(line); + let (_, rest) = plain.split_once(" specs checked: ")?; + let failed_part = rest.rsplit(',').next()?.trim(); + let count = failed_part.strip_suffix(" failed")?.trim(); + count.parse().ok() +} + +/// Remove ANSI escape sequences (colors) from a string. +fn strip_ansi(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + while let Some(c) = chars.next() { + if c == '\u{1b}' { + // Skip a CSI sequence like `\x1b[31m` up to its terminating letter + if chars.peek() == Some(&'[') { + chars.next(); + for esc in chars.by_ref() { + if esc.is_ascii_alphabetic() { + break; + } + } + } + continue; + } + out.push(c); + } + out +} + #[cfg(test)] mod tests { use super::*; @@ -440,4 +506,38 @@ mod tests { assert_eq!(changed_file, None); } + + // --- summary line parsing (watch footer) --- + + #[test] + fn test_parse_failed_count_with_failures() { + let line = "3 specs checked: 2 passed, 1 warning(s), 1 failed"; + assert_eq!(parse_failed_count(line), Some(1)); + } + + #[test] + fn test_parse_failed_count_all_passed() { + let line = "13 specs checked: 13 passed, 0 warning(s), 0 failed"; + assert_eq!(parse_failed_count(line), Some(0)); + } + + #[test] + fn test_parse_failed_count_ignores_other_lines() { + assert_eq!(parse_failed_count("All checks passed!"), None); + assert_eq!(parse_failed_count(" ✗ Frontmatter invalid"), None); + assert_eq!(parse_failed_count(""), None); + } + + #[test] + fn test_parse_failed_count_strips_colors() { + // print_summary colors the failed count red when non-zero + let line = "1 specs checked: \u{1b}[32m0\u{1b}[0m passed, \u{1b}[33m0\u{1b}[0m warning(s), \u{1b}[31m1\u{1b}[0m failed"; + assert_eq!(parse_failed_count(line), Some(1)); + } + + #[test] + fn test_strip_ansi_removes_escape_sequences() { + assert_eq!(strip_ansi("\u{1b}[31mred\u{1b}[0m"), "red"); + assert_eq!(strip_ansi("plain"), "plain"); + } } diff --git a/tests/integration.rs b/tests/integration.rs index 0c1d91f..2bcd88d 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -472,14 +472,70 @@ fn init_creates_config_file() { .arg(root) .assert() .success() - .stdout(predicate::str::contains("Created specsync.json")); + .stdout(predicate::str::contains("Created .specsync/config.toml")); - let config_path = root.join("specsync.json"); + let config_path = root.join(".specsync/config.toml"); assert!(config_path.exists()); let content = fs::read_to_string(&config_path).unwrap(); - assert!(content.contains("specsDir")); - assert!(content.contains("sourceDirs")); - assert!(content.contains("requiredSections")); + assert!(content.contains("specs_dir")); + assert!(content.contains("source_dirs")); + assert!(content.contains("required_sections")); + + // Full v4 layout — version stamp, .gitignore, and state directories + assert!(root.join(".specsync/version").exists()); + assert!(root.join(".specsync/.gitignore").exists()); + assert!(root.join(".specsync/lifecycle").is_dir()); + assert!(root.join(".specsync/changes").is_dir()); + assert!(root.join(".specsync/archive").is_dir()); +} + +#[test] +fn init_then_check_does_not_nag_about_legacy_layout() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/main.rs"), "fn main() {}").unwrap(); + + specsync() + .arg("init") + .arg("--root") + .arg(root) + .assert() + .success(); + + // A fresh v4 init must not trigger the legacy 3.x migration nag + specsync() + .arg("check") + .arg("--root") + .arg(root) + .assert() + .success() + .stderr(predicate::str::contains("Legacy 3.x layout").not()); +} + +#[test] +fn init_does_not_overwrite_existing_v4_config() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + fs::create_dir_all(root.join(".specsync")).unwrap(); + fs::write( + root.join(".specsync/config.toml"), + "specs_dir = \"custom\"\n", + ) + .unwrap(); + + specsync() + .arg("init") + .arg("--root") + .arg(root) + .assert() + .success() + .stdout(predicate::str::contains("already exists")); + + let content = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(content.contains("custom")); } #[test] @@ -1372,7 +1428,8 @@ fn ai_command_overrides_ai_provider() { fs::create_dir_all(root.join("src/newmod")).unwrap(); fs::write(root.join("src/newmod/lib.rs"), "pub fn hello() {}").unwrap(); - // The command succeeds (falls back to template) but stderr shows AI was attempted & failed + // The spec is still written from the template, but the run exits non-zero + // and reports the AI failure prominently on stderr specsync() .arg("generate") .arg("--provider") @@ -1380,8 +1437,10 @@ fn ai_command_overrides_ai_provider() { .arg("--root") .arg(&root) .assert() - .success() - .stderr(predicate::str::contains("AI generation failed")); + .failure() + .code(1) + .stderr(predicate::str::contains("AI generation failed")) + .stderr(predicate::str::contains("template fallback was used")); } #[test] @@ -1703,7 +1762,8 @@ fn ai_api_key_config_field_used_for_anthropic() { ) .unwrap(); - // Should succeed (API call fails, falls back to template) + // The API call fails (bad key) — the template fallback is written but the + // run exits non-zero so the failure is not silent specsync() .arg("generate") .arg("--provider") @@ -1712,7 +1772,8 @@ fn ai_api_key_config_field_used_for_anthropic() { .arg(&root) .env_remove("ANTHROPIC_API_KEY") .assert() - .success() + .failure() + .code(1) .stderr(predicate::str::contains("AI generation failed")); } @@ -1751,11 +1812,8 @@ fn init_auto_detects_src_dir() { .success() .stdout(predicate::str::contains("Detected source directories: src")); - let config: serde_json::Value = - serde_json::from_str(&fs::read_to_string(root.join("specsync.json")).unwrap()).unwrap(); - let dirs = config["sourceDirs"].as_array().unwrap(); - assert_eq!(dirs.len(), 1); - assert_eq!(dirs[0], "src"); + let config = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(config.contains("source_dirs = [\"src\"]")); } #[test] @@ -1775,11 +1833,8 @@ fn init_auto_detects_lib_dir() { .success() .stdout(predicate::str::contains("Detected source directories: lib")); - let config: serde_json::Value = - serde_json::from_str(&fs::read_to_string(root.join("specsync.json")).unwrap()).unwrap(); - let dirs = config["sourceDirs"].as_array().unwrap(); - assert_eq!(dirs.len(), 1); - assert_eq!(dirs[0], "lib"); + let config = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(config.contains("source_dirs = [\"lib\"]")); } #[test] @@ -1803,12 +1858,8 @@ fn init_auto_detects_multiple_dirs() { "Detected source directories: lib, src", )); - let config: serde_json::Value = - serde_json::from_str(&fs::read_to_string(root.join("specsync.json")).unwrap()).unwrap(); - let dirs = config["sourceDirs"].as_array().unwrap(); - assert_eq!(dirs.len(), 2); - assert_eq!(dirs[0], "lib"); - assert_eq!(dirs[1], "src"); + let config = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(config.contains("source_dirs = [\"lib\", \"src\"]")); } #[test] @@ -1836,11 +1887,8 @@ fn init_ignores_node_modules_and_hidden_dirs() { .success() .stdout(predicate::str::contains("Detected source directories: app")); - let config: serde_json::Value = - serde_json::from_str(&fs::read_to_string(root.join("specsync.json")).unwrap()).unwrap(); - let dirs = config["sourceDirs"].as_array().unwrap(); - assert_eq!(dirs.len(), 1); - assert_eq!(dirs[0], "app"); + let config = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(config.contains("source_dirs = [\"app\"]")); } #[test] @@ -1884,11 +1932,8 @@ fn init_falls_back_to_src_when_no_source_files() { .success() .stdout(predicate::str::contains("Detected source directories: src")); - let config: serde_json::Value = - serde_json::from_str(&fs::read_to_string(root.join("specsync.json")).unwrap()).unwrap(); - let dirs = config["sourceDirs"].as_array().unwrap(); - assert_eq!(dirs.len(), 1); - assert_eq!(dirs[0], "src"); + let config = fs::read_to_string(root.join(".specsync/config.toml")).unwrap(); + assert!(config.contains("source_dirs = [\"src\"]")); } // ─── MCP Server Tests ────────────────────────────────────────────────────── @@ -4966,3 +5011,277 @@ fn stale_in_fresh_repo_reports_all_up_to_date() { .success() .stdout(predicate::str::contains("up to date")); } + +// ─── Hands-on findings: v4 init-registry, draft visibility, fix routing ──── + +#[test] +fn init_registry_uses_v4_path_in_migrated_project() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + // v4 project: config + version stamp under .specsync/ + fs::create_dir_all(root.join(".specsync")).unwrap(); + fs::write( + root.join(".specsync/config.toml"), + "specs_dir = \"specs\"\nsource_dirs = [\"src\"]\n", + ) + .unwrap(); + fs::write(root.join(".specsync/version"), "4.0.0\n").unwrap(); + fs::create_dir_all(root.join("specs")).unwrap(); + + specsync() + .args(["init-registry", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stdout(predicate::str::contains("Created .specsync/registry.toml")); + + assert!(root.join(".specsync/registry.toml").exists()); + assert!( + !root.join("specsync-registry.toml").exists(), + "must not recreate the legacy root-level registry in a v4 project" + ); + + // And the v4 registry must not re-trigger the legacy migration nag + specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stderr(predicate::str::contains("Legacy 3.x layout").not()); +} + +#[test] +fn init_registry_keeps_legacy_path_for_legacy_project() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + // Un-migrated 3.x project: root-level config, no version stamp + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("specs")).unwrap(); + + specsync() + .args(["init-registry", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stdout(predicate::str::contains("Created specsync-registry.toml")); + + assert!(root.join("specsync-registry.toml").exists()); +} + +#[test] +fn init_registry_is_idempotent_for_v4_registry() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + fs::create_dir_all(root.join(".specsync")).unwrap(); + fs::write(root.join(".specsync/version"), "4.0.0\n").unwrap(); + fs::write( + root.join(".specsync/registry.toml"), + "[registry]\nname = \"existing\"\n", + ) + .unwrap(); + + specsync() + .args(["init-registry", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stdout(predicate::str::contains("already exists")); + + let content = fs::read_to_string(root.join(".specsync/registry.toml")).unwrap(); + assert!(content.contains("existing")); +} + +#[test] +fn draft_spec_check_reports_skipped_validation() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + + // Draft spec missing several required sections + let draft_spec = r#"--- +module: draft-mod +version: 1 +status: draft +files: + - src/auth/service.ts +db_tables: [] +depends_on: [] +--- + +# Draft Mod + +## Purpose + +Work in progress. +"#; + fs::create_dir_all(root.join("specs/draft-mod")).unwrap(); + fs::write(root.join("specs/draft-mod/draft-mod.spec.md"), draft_spec).unwrap(); + + let assert = specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + + // The draft must not get misleading green checkmarks for skipped checks + assert!( + stdout.contains("Section validation skipped (status: draft)"), + "expected explicit skip notice for sections, got:\n{stdout}" + ); + assert!( + stdout.contains("Export validation skipped (status: draft)"), + "expected explicit skip notice for exports, got:\n{stdout}" + ); + assert!( + stdout.contains("draft spec(s) skipped section and export validation"), + "expected summary hint about skipped drafts, got:\n{stdout}" + ); + assert!( + stdout.contains("set `status: active` to enable full checks"), + "expected guidance to promote the draft, got:\n{stdout}" + ); +} + +#[test] +fn active_spec_check_has_no_draft_skip_notice() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + + let assert = specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + + assert!(!stdout.contains("skipped (status: draft)")); + assert!(!stdout.contains("draft spec(s) skipped")); +} + +#[test] +fn fix_routes_functions_and_types_to_matching_tables() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + + // Source with two functions and one type + fs::create_dir_all(root.join("src/mathx")).unwrap(); + fs::write( + root.join("src/mathx/lib.rs"), + "pub fn add(a: i32, b: i32) -> i32 { a + b }\n\ + pub fn subtract(a: i32, b: i32) -> i32 { a - b }\n\ + pub struct Calculator { pub precision: u8 }\n", + ) + .unwrap(); + + // Spec with empty Public API tables (template layout: Functions then Types) + fs::create_dir_all(root.join("specs/mathx")).unwrap(); + fs::write( + root.join("specs/mathx/mathx.spec.md"), + valid_spec("mathx", &["src/mathx/lib.rs"]), + ) + .unwrap(); + + specsync() + .args(["check", "--fix", "--root", root.to_str().unwrap()]) + .assert() + .success(); + + let updated = fs::read_to_string(root.join("specs/mathx/mathx.spec.md")).unwrap(); + + // Extract each subsection to verify the rows landed in the right table + let functions_start = updated.find("### Exported Functions").unwrap(); + let types_start = updated.find("### Exported Types").unwrap(); + let invariants_start = updated.find("## Invariants").unwrap(); + let functions_table = &updated[functions_start..types_start]; + let types_table = &updated[types_start..invariants_start]; + + assert!( + functions_table.contains("`add`") && functions_table.contains("`subtract`"), + "functions must go to the Exported Functions table, got:\n{functions_table}" + ); + assert!( + !types_table.contains("`add`") && !types_table.contains("`subtract`"), + "functions must NOT land in the Exported Types table, got:\n{types_table}" + ); + assert!( + types_table.contains("`Calculator`"), + "types must go to the Exported Types table, got:\n{types_table}" + ); + assert!( + !functions_table.contains("`Calculator`"), + "types must NOT land in the Exported Functions table, got:\n{functions_table}" + ); + + // The fixed spec passes a re-check without duplicate rows + specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let recheck = fs::read_to_string(root.join("specs/mathx/mathx.spec.md")).unwrap(); + assert_eq!(recheck.matches("`add`").count(), 1); +} + +#[test] +fn failing_frontmatter_shows_negated_label() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/lib.rs"), "pub fn a() {}").unwrap(); + + // Spec missing required frontmatter fields (no version/status) + fs::create_dir_all(root.join("specs/bad")).unwrap(); + fs::write( + root.join("specs/bad/bad.spec.md"), + "---\nmodule: bad\nfiles:\n - src/lib.rs\n---\n\n# Bad\n\n## Purpose\n\nx\n", + ) + .unwrap(); + + let assert = specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + + assert!( + stdout.contains("✗ Frontmatter invalid"), + "failing frontmatter must show a negated label, got:\n{stdout}" + ); + assert!( + !stdout.contains("✗ Frontmatter valid"), + "must not pair ✗ with a non-negated label, got:\n{stdout}" + ); +} + +#[test] +fn check_unmatched_spec_filter_errors_without_contradiction() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + + let assert = specsync() + .args(["check", "nosuchmodule", "--root", root.to_str().unwrap()]) + .assert() + .failure() + .code(1); + let output = assert.get_output(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + + assert!( + stderr.contains("No specs matched"), + "expected unmatched-filter warning, got stderr:\n{stderr}" + ); + assert!( + !stdout.contains("No spec files found"), + "must not claim no spec files exist when specs are present, got:\n{stdout}" + ); +} + +#[test] +fn nonexistent_root_errors_with_nonzero_exit() { + specsync() + .args(["check", "--root", "/nonexistent/path/for/specsync"]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("does not exist")); +} From 96d2af3e2bf2bfa30b92d2f83c3f4ee81181f36b Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Thu, 11 Jun 2026 08:57:28 -0600 Subject: [PATCH 2/2] fix: normalize registry path display to forward slashes on Windows The init_registry_uses_v4_path_in_migrated_project test failed on windows-latest because the created-file message rendered the relative path with backslashes. Normalize the display string so output is consistent with the tool's forward-slash path style on every platform. Co-Authored-By: Claude Fable 5 --- src/commands/init_registry.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/commands/init_registry.rs b/src/commands/init_registry.rs index 7c842b3..1b65351 100644 --- a/src/commands/init_registry.rs +++ b/src/commands/init_registry.rs @@ -10,11 +10,13 @@ pub fn cmd_init_registry(root: &Path, name: Option) { // Respect the project layout: v4 projects get .specsync/registry.toml, // un-migrated 3.x projects keep the legacy root-level file. let registry_path = registry::local_registry_path(root); + // Normalize to forward slashes so output matches the tool's path style on every platform. let rel_display = registry_path .strip_prefix(root) .unwrap_or(®istry_path) .display() - .to_string(); + .to_string() + .replace('\\', "/"); if registry_path.exists() { println!("{rel_display} already exists"); return;