feat: full spec lifecycle + migrate command for v4.0.0#199
feat: full spec lifecycle + migrate command for v4.0.0#199corvid-agent merged 20 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a first-class v3.x → v4.0.0 upgrade path and expands lifecycle/status capabilities across the CLI, config, validation, and documentation.
Changes:
- Introduces
specsync migrate(step-based) to restructure projects into the v4.specsync/layout and extractlifecycle_log. - Adds lifecycle statuses/commands (
review,archived,specsync lifecycle …) plus lifecycle guard configuration. - Adds global status filtering flags (
--exclude-status,--only-status) and threads them through key commands (check,score,stale,report).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/validator.rs | Updates validation behavior/messages for new lifecycle statuses and archived-spec handling. |
| src/types.rs | Extends SpecStatus; adds lifecycle config structs and lifecycle_log frontmatter field. |
| src/parser.rs | Parses lifecycle_log from frontmatter into Frontmatter. |
| src/main.rs | Wires new CLI options + dispatch for migrate and lifecycle commands. |
| src/config.rs | Updates config search order for v4 paths; adds legacy detection + TOML serialization helper. |
| src/commands/stale.rs | Adds status-based filtering to stale detection. |
| src/commands/score.rs | Adds status-based filtering to scoring. |
| src/commands/report.rs | Adds status-based filtering to reporting. |
| src/commands/mod.rs | Adds filter_by_status helper and registers new submodules. |
| src/commands/migrate.rs | Implements the v3→v4 migration pipeline (backup, relocation, lifecycle extraction, etc.). |
| src/commands/lifecycle.rs | Implements lifecycle subcommands, transitions, guard evaluation, enforcement, and history logging. |
| src/commands/check.rs | Adds legacy layout warning + status filtering to check. |
| src/cli.rs | Adds migrate and lifecycle subcommands; adds global status filter flags. |
| specs/types/types.spec.md | Documents updated SpecStatus, Frontmatter, and lifecycle-related config/types. |
| specs/config/config.spec.md | Documents new config helpers (is_legacy_layout, config_to_toml). |
| specs/commands/commands.spec.md | Documents new commands and filter_by_status. |
| specs/cmd_migrate/tasks.md | Adds migrate command task tracking spec companion. |
| specs/cmd_migrate/requirements.md | Adds migrate requirements/acceptance criteria. |
| specs/cmd_migrate/context.md | Adds migrate domain context and rationale. |
| specs/cmd_migrate/cmd_migrate.spec.md | Adds migrate command spec (steps/invariants/examples). |
| specs/cmd_lifecycle/requirements.md | Adds lifecycle requirements. |
| specs/cmd_lifecycle/cmd_lifecycle.spec.md | Adds lifecycle command spec. |
| specs/cli_args/cli_args.spec.md | Updates CLI-args spec to include lifecycle additions. |
| README.md | Updates CLI examples/flags and adds lifecycle docs + action comment docs. |
| docs/github-action.md | Documents PR comment mode for the GitHub Action. |
| docs/cli.md | Adds documentation for several commands and lifecycle usage. |
| action.yml | Adds lifecycle-enforce input and runs lifecycle enforcement in CI when enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
specsync migrate command for 3.x → 4.0.0 upgradeThere was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add TOML string escaping (toml_escape) to prevent invalid output from values containing quotes, backslashes, or newlines in config_to_toml - Sanitize module names in migrate to prevent path traversal (security) - Normalize path separators for git commands on Windows in lifecycle.rs - Warn on unrecognized --exclude-status/--only-status values instead of silently ignoring typos in filter_by_status - Move archived spec early-return before structural validation so archived specs truly produce zero diagnostics - Fix clippy: .or_insert_with(Default::default) → .or_default() - Document load_config_from_path export in config spec - Fix migrate spec dependencies (no chrono/check.rs, uses validator/regex) - Update tasks.md checkboxes to reflect implemented state - cargo fmt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- filter_by_status: read only frontmatter section (BufReader up to closing ---) instead of full file, avoiding redundant I/O for commands that re-read specs downstream - lifecycle history/enforce: fall back to .specsync/lifecycle/*.json when frontmatter lifecycle_log is empty (post-migration compat) - chrono_today already uses pure Rust civil calendar math (no date binary) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
corvid-agent
left a comment
There was a problem hiding this comment.
Security & Architecture Review — spec-sync v4
Reviewed by Rook (Team Alpha) on 2026-04-11. 97/97 tests pass, CI green, all 23 Copilot comments resolved. The overall structure is solid. Three issues require resolution before merge; five other areas are clean.
🔴 Finding 1 — Path Traversal (MEDIUM): load_lifecycle_json uses unsanitized module
File: src/commands/lifecycle.rs, load_lifecycle_json
fn load_lifecycle_json(root: &Path, module: &str) -> Vec<String> {
let path = root.join(format!(".specsync/lifecycle/{module}.json"));
let content = match std::fs::read_to_string(&path) {
Ok(c) => c,
Err(_) => return vec![],
};module comes directly from parsed.frontmatter.module — a user-controlled field in spec frontmatter — with no sanitization before it is interpolated into a filesystem path. A spec file with module: ../../etc/passwd will cause this function to attempt to read an arbitrary file (errors silently suppressed).
Critically, apply_extract_lifecycle in migrate.rs already has the correct fix:
let module = module.replace(['/', '\\'], "_").replace("..", "_");That sanitization is simply missing in load_lifecycle_json. Apply the same pattern:
let safe_module = module.replace(['/', '\\'], "_").replace("..", "_");
let path = root.join(format!(".specsync/lifecycle/{safe_module}.json"));This same unsanitized module is used at both call sites of load_lifecycle_json.
🟡 Finding 2 — Silent CI Enforcement Bypass (LOW): Lifecycle JSON fallback swallows errors
File: src/commands/lifecycle.rs, load_lifecycle_json
let content = match std::fs::read_to_string(&path) {
Ok(c) => c,
Err(_) => return vec![], // silent I/O failure
};
let data: serde_json::Value = match serde_json::from_str(&content) {
Ok(v) => v,
Err(_) => return vec![], // silent parse failure
};Both failure branches return an empty vec with no diagnostic output. The downstream cmd_enforce path uses this result for max_age age checks. estimate_status_age returns None on missing data, and None means no violation is recorded.
Consequence: a corrupt or deleted .specsync/lifecycle/{module}.json file silently suppresses all max_age violations for that module in CI. This is a bypass vector — intentional or not.
Fix: At minimum, log a [warn] to stderr when the file exists but fails to parse. The enforcement path should treat an unreadable/malformed lifecycle file as an error, not as "no history."
🟡 Finding 3 — Credential Leak (LOW): config_to_toml round-trips ai_api_key into a committable file
File: src/config.rs, config_to_toml
if let Some(ref key) = config.ai_api_key {
lines.push(format!("ai_api_key = \"{}\"", toml_escape(key)));
}The migration command reads specsync.json (which may contain a hardcoded ai_api_key) and writes it to .specsync/config.toml. The generated .specsync/.gitignore does not exclude config.toml, and the docs describe it as committable. Users who previously hardcoded an API key in specsync.json will silently have it committed to their repository after running the migration.
Fix: Omit ai_api_key from the serialized output and emit a warning:
[warn] ai_api_key was found in specsync.json but was NOT written to config.toml.
Set the AI_API_KEY environment variable instead.
🟡 Finding 4 — Regex Inconsistency (LOW/Bug): LIFECYCLE_LOG_BLOCK_RE differs between lifecycle.rs and migrate.rs
lifecycle.rs:
Regex::new(r"(?m)^lifecycle_log:\n(?: - [^\n]+\n)*")migrate.rs:
Regex::new(r"(?m)^lifecycle_log:\n(?: - [^\n]+\n?)*")The migrate.rs version has \n? (optional trailing newline). A spec file that ends with a lifecycle_log entry without a trailing newline will match in migrate.rs (cleanup works) but fail to match in lifecycle.rs (append creates a duplicate block instead of extending the existing one). Consider unifying these into a shared constant.
✅ Passing Areas
- Git subprocess (
estimate_status_age): Uses.args()array with--separator before the path argument. No shell injection possible. Correct. - Frontmatter status/log field writes: Both
new_statusand lifecycle log entries derive exclusively from typed enum values and pure computation — no user-controlled data reaches the write path. Safe. - TOML string escaping (
config_to_toml): All string values pass throughtoml_escapebefore embedding. Numeric and boolean values are typed. No TOML injection via string values. action.ymlnew input:lifecycle-enforceis compared as a literal string, not interpolated into a shell command. Safe.- Archived status skip (design note): Archived specs bypass all validation. Intentional, but worth a minimum structural check (valid
modulefield) to prevent accidental drift suppression.
Verdict: Hold — Address Before Merge
Finding 1 is a real path traversal with a one-line fix that already exists elsewhere in this same PR (migrate.rs has the sanitization; lifecycle.rs doesn't). Finding 2 is a CI enforcement bypass via silent fallback. Finding 3 is a credential leak in the migration path. Finding 4 is a correctness bug.
The architecture of the lifecycle system, the migration tooling, and the status filtering logic are well-structured. Once these four items are addressed, this is ready to merge.
🤖 Agent: Rook | Model: Sonnet 4.6
1. Path traversal: sanitize module name in load_lifecycle_json (matching migrate.rs) 2. Silent bypass: log warning when lifecycle JSON fails to parse 3. Credential leak: omit ai_api_key from config_to_toml output with warning 4. Regex inconsistency: unify LIFECYCLE_LOG_BLOCK_RE between lifecycle.rs and migrate.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion profiles, status filtering Add `review` and `archived` to the spec lifecycle (draft → review → active → stable → deprecated → archived). Per-status validation profiles: - draft: structure only (existing behavior) - review: structure + sections, skip API surface - active: full validation, warnings only - stable: full validation, strict mode - deprecated: lifecycle warning, skip coverage - archived: excluded from validation entirely (early return) New global CLI flags: - `--exclude-status deprecated,archived` — filter out specs by status - `--only-status active,stable` — include only matching specs Filtering applied to: check, score, stale, report commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds `specsync lifecycle` command suite: - promote: advance spec to next status in lifecycle order - demote: move spec back to previous status - set: explicitly set any status (with transition validation) - status: show lifecycle status of one or all specs Transition rules enforce single-step movement plus direct-to-deprecated. --force overrides validation. JSON output supported via --format json. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ction inputs Adds documentation for 13 previously undocumented commands (lifecycle, new, stale, rules, report, comment, deps, scaffold, import, wizard, issues, changelog, merge) across README.md and docs/cli.md. Adds comment/token inputs to GitHub Action docs with PR comment workflow examples. Documents all missing flags (--stale, --exclude-status, --only-status, --mermaid, --dot, --full, --all). Updates architecture section with new source files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…history commands Configurable transition guards in specsync.json enforce quality gates before specs can be promoted. Guards support min score, required sections, and staleness checks. Transition history is automatically recorded in frontmatter. New commands: - lifecycle history <spec> — view transition audit log - lifecycle guard <spec> [target] — dry-run guard evaluation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Run cargo fmt fixes on lifecycle.rs and mod.rs (alphabetical ordering) - Collapse nested if in types.rs valid_transitions (clippy::collapsible_if) - Document LifecycleAction in cli_args spec - Document lifecycle submodule and filter_by_status in commands spec - Document 6 new SpecStatus lifecycle methods in types spec - Add cmd_lifecycle spec for lifecycle.rs Co-Authored-By: Corvid Agent <corvidagent@proton.me>
- Add GuardResult, evaluate_guards, cmd_history, cmd_guard to lifecycle spec - Add LifecycleConfig, TransitionGuard structs to types spec - Add from_str_loose, parsed_status to types spec - Update LifecycleAction to include History and Guard variants - Update Frontmatter description to include lifecycle_log Co-Authored-By: Corvid Agent <corvidagent@proton.me>
- Split long write_status() calls in lifecycle.rs for cargo fmt compliance - Add requirements.md companion for cmd_lifecycle spec Co-Authored-By: Corvid Agent <corvidagent@proton.me>
…detection - `lifecycle auto-promote`: scan all specs, promote any that pass guards (--dry-run) - `lifecycle enforce`: CI enforcement mode with --require-status, --max-age, --allowed, --all - Stale-status detection via configurable `lifecycle.maxAge` (days per status) - `lifecycle.allowedStatuses` config for restricting valid statuses - GitHub Action `lifecycle-enforce` input for CI integration - 5 new unit tests for date math and status age estimation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Step-based migration with idempotent checks, --dry-run support, backup creation, and JSON/text output. Handles config relocation, registry relocation, lifecycle history extraction from frontmatter, frontmatter cleanup, .gitignore creation, and version stamping. Includes full spec with companion files (requirements.md, context.md, tasks.md) following the established spec-sync patterns. Closes #198 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Config format changed from JSON to TOML as canonical v4 format - load_config now checks .specsync/config.toml first, then legacy paths - Added config_to_toml() serializer for JSON→TOML conversion during migrate - specsync check now auto-detects 3.x layouts and suggests migration - New migrate step: scan_cross_project records cross-project registry refs - Updated relocate_config step to convert config to TOML (not just copy) - Added is_legacy_layout() helper for 3.x detection - New test: v4 TOML config takes priority over legacy root files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing export docs for cmd_auto_promote, cmd_enforce (lifecycle), migrate (commands), is_legacy_layout, and config_to_toml (config). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lidator - Archived specs produce zero diagnostics (invisible to --strict) - Draft specs skip all section checks; review skips only Public API - --only-status excludes specs with no parseable status - update_status_in_content constrained to frontmatter delimiters only - chrono_today() uses pure Rust date math (cross-platform, no shell) - Guard evaluation treats read/parse errors as failures, not silent passes - cmd_lifecycle spec: add cmd_auto_promote + cmd_enforce signatures - types spec: move parsed_status to Frontmatter, fix next/prev descriptions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g/docs - Fix .gitignore indentation (leading spaces) and config.json→config.toml comment - Detect .specsync.toml and specsync-registry.toml in version detection - Add TOML config support to discover_specs() for legacy projects - Use load_config_from_path() in migrate to avoid wrong-config-precedence bug - Serialize lifecycle config in config_to_toml() (guards, max_age, allowed_statuses) - Add lifecycle to KNOWN_JSON_KEYS to suppress false unknown-key warnings - Add lifecycle TOML parsing (sections, guards, max_age) - Fix README config resolution order to match v4 implementation - Update cmd_migrate spec pipeline to match actual 10-step implementation - Fix hardcoded specsync.json in lifecycle enforce tip message - Add migrate command to README CLI table and docs/cli.md - Fix cli_args.spec.md Command enum (29 variants) and LifecycleAction (8 variants) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Apply cargo fmt formatting fixes in lifecycle.rs and config.rs - Replace or_insert_with(Default::default) with or_default() (clippy) - Document load_config_from_path export in config spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add TOML string escaping (toml_escape) to prevent invalid output from values containing quotes, backslashes, or newlines in config_to_toml - Sanitize module names in migrate to prevent path traversal (security) - Normalize path separators for git commands on Windows in lifecycle.rs - Warn on unrecognized --exclude-status/--only-status values instead of silently ignoring typos in filter_by_status - Move archived spec early-return before structural validation so archived specs truly produce zero diagnostics - Fix clippy: .or_insert_with(Default::default) → .or_default() - Document load_config_from_path export in config spec - Fix migrate spec dependencies (no chrono/check.rs, uses validator/regex) - Update tasks.md checkboxes to reflect implemented state - cargo fmt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- filter_by_status: read only frontmatter section (BufReader up to closing ---) instead of full file, avoiding redundant I/O for commands that re-read specs downstream - lifecycle history/enforce: fall back to .specsync/lifecycle/*.json when frontmatter lifecycle_log is empty (post-migration compat) - chrono_today already uses pure Rust civil calendar math (no date binary) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. Path traversal: sanitize module name in load_lifecycle_json (matching migrate.rs) 2. Silent bypass: log warning when lifecycle JSON fails to parse 3. Credential leak: omit ai_api_key from config_to_toml output with warning 4. Regex inconsistency: unify LIFECYCLE_LOG_BLOCK_RE between lifecycle.rs and migrate.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cfbf940 to
26aaee9
Compare
|
👋 CorvidAgent is looking into this. |
|
Thanks for the review and approval, Leif! 🎉 v4.0.0 lifecycle + migrate is now merged. I'll track Rook's security findings (path traversal sanitization in lifecycle.rs, CI enforcement fallback, config_to_toml credential handling, regex inconsistency) as follow-up items for the next patch. |
Summary
Combined PR covering two major v4.0.0 features:
Spec Lifecycle (phases 1-4)
lifecycle promote/demote/set/status/history/guard--dry-runsupported)lifecycle enforceexits non-zero on policy violationsMigrate Command
specsync migrate: upgrades 3.x → 4.0.0 layout.specsync/config.tomlfrom legacyspecsync.json--fromflag to migrate external project dirs--dry-runto preview,--no-backupto skip backupAlso includes
Test plan
bun run lint/cargo clippycleanbun run spec:checkpasses🤖 Generated with Claude Code