Skip to content

release: promote dev to main for v0.42.5 (patch)#446

Merged
djm81 merged 317 commits intomainfrom
dev
Mar 25, 2026
Merged

release: promote dev to main for v0.42.5 (patch)#446
djm81 merged 317 commits intomainfrom
dev

Conversation

@djm81
Copy link
Copy Markdown
Collaborator

@djm81 djm81 commented Mar 25, 2026

Summary

Patch release v0.42.5 — promotes current dev to main for PyPI and docs.

Highlights (see CHANGELOG)

  • specfact init ide: prompt-source catalog, --prompts, namespaced IDE exports (feat(init): IDE prompt source catalog, --prompts, namespaced exports #445)
  • VS Code chat.promptFilesRecommendations aligned with selective exports
  • Integration test cleanup: restore module roots after command-audit temp home (stable full suite)
  • Version 0.42.5 across manifests and changelog (no [Unreleased] section)

Type of change

  • Patch release (non-breaking)

Checklist

  • CHANGELOG [0.42.5]
  • Version synced (pyproject.toml, setup.py, __version__)
  • dev pushed (38a9d21 and prior)

CI: required checks must pass before merge.

Made with Cursor

djm81 and others added 30 commits February 6, 2026 14:57
- Fix unreachable code in contract init (Prompt.ask after raise typer.Exit)
- Replace empty except with print_warning for contract file load failures
- Fix repo-root fallback path depth in backlog commands after module migration

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… UX (#204)

* Release v0.28.0: Module package separation for command implementations (#201)

* perf: optimize startup performance with metadata tracking and update command (#142)

* feat: implement backlog field mapping and refinement improvements

- Add FieldMapper abstract base class with canonical field names
- Implement GitHubFieldMapper and AdoFieldMapper
- Add custom field mapping support with YAML templates
- Add field validation in refinement (story_points, business_value, priority)
- Add comprehensive unit and integration tests (42 tests)
- Add custom field mapping documentation
- Fix custom_field_mapping parameter connection
- Add early validation for custom mapping files

Implements OpenSpec change: improve-backlog-field-mapping-and-refinement

* perf: optimize startup performance with metadata tracking and update command

- Add metadata management module for tracking version and check timestamps
- Optimize startup checks to only run when needed:
  - Template checks: Only after version changes detected
  - Version checks: Limited to once per day (24h threshold)
- Add --skip-checks flag for CI/CD environments
- Add new 'specfact update' command for manual update checking and installation
- Add comprehensive unit and integration tests (35 tests, all passing)
- Update startup_checks to use metadata for conditional execution
- Ensure backward compatibility (first-time users still get all checks)

Performance Impact:
- Startup time: Reduced from several seconds to < 1-2 seconds
- Network requests: Reduced from every startup to once per day
- File system operations: Reduced from every startup to only after version changes

Fixes #140
Implements OpenSpec change: optimize-startup-performance

* feat: request offline_access scope for Azure DevOps refresh tokens

- Add offline_access scope to Azure DevOps OAuth requests
- Refresh tokens now last 90 days (vs 1 hour for access tokens)
- Automatic token refresh via persistent cache (no re-authentication needed)
- Update documentation to reflect 90-day refresh token lifetime

This addresses the issue where tokens were expiring too quickly.
Refresh tokens obtained via offline_access scope enable automatic
token renewal for 90 days without user interaction.

Fixes token lifetime limitation issue

* feat: improve CLI UX with banner control and upgrade command

- Change banner to hidden by default, shown on first run or with --banner flag
- Add simple version line (SpecFact CLI - vXYZ) for regular use
- Rename 'update' command to 'upgrade' to avoid confusion
- Update documentation for new banner behavior and upgrade command
- Update startup checks message to reference 'specfact upgrade'

* fix: suppress version line in test mode and fix field mapping issues

- Suppress version line output in test mode and for help/version commands to prevent test failures
- Fix ADO custom field mapping to honor --custom-field-mapping on writeback
- Fix GitHub issue body updates to prevent duplicate sections
- Ensure proper type handling for story points and business value calculations

* Fix failed tests

* chore: bump version to 0.26.7 and update changelog

- Fixed adapter token validation tests (ADO and GitHub)
- Resolved test timeout issues (commit history, AST parsing, Semgrep)
- Improved test file discovery to exclude virtual environments
- Added file size limits for AST parsing to prevent timeouts

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>

* fix: add missing ADO field mappings and assignee display (#145)

* fix: add missing ADO field mappings and assignee display

- Add Microsoft.VSTS.Common.AcceptanceCriteria to default field mappings
- Update AdoFieldMapper to support multiple field name alternatives
- Fix assignee extraction to include displayName, uniqueName, and mail
- Add assignee display in preview output
- Add interactive template mapping command (specfact backlog map-fields)
- Update specfact init to copy backlog field mapping templates
- Extend documentation with step-by-step guides

Fixes #144

* test: add unit tests for ADO field mapping and assignee fixes

- Add tests for Microsoft.VSTS.Common.AcceptanceCriteria field extraction
- Add tests for multiple field name alternatives
- Add tests for assignee extraction with displayName, uniqueName, mail
- Add tests for assignee filtering with multiple identifiers
- Add tests for assignee display in preview output
- Add tests for interactive mapping command
- Add tests for template copying in init command
- Update existing tests to match new assignee extraction behavior

* docs: update init command docstring to mention template copying

* docs: update documentation for ADO field mapping and interactive mapping features

- Update authentication guide with ADO token resolution priority
- Update custom field mapping guide with interactive mapping details
- Update backlog refinement guide with progress indicators and required field display
- Update Azure DevOps adapter guide with field mapping improvements
- Update command reference with map-fields command documentation
- Update troubleshooting guide with ADO-specific issues
- Update README files with new features
- Update getting started guide with template initialization

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: address review findings for ADO field mapping

- Prefer System.* fields over Microsoft.VSTS.Common.* when writing updates
  (fixes issue where PATCH requests could fail for Scrum templates)
- Preserve existing work_item_type_mappings when saving field mappings
  (prevents silent erasure of custom work item type mappings)

Fixes review comments:
- P1: Prefer System.AcceptanceCriteria when writing updates
- P2: Preserve existing work_item_type_mappings on save

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: mitigate code scanning vulnerabilities (#148)

* fix: mitigate code scanning vulnerabilities

- Fix ReDoS vulnerability in github_mapper.py by replacing regex with line-by-line processing
- Fix incomplete URL sanitization in github.py, bridge_sync.py, and ado.py using proper URL parsing
- Add explicit permissions blocks to 7 GitHub Actions jobs following least-privilege model

Resolves all 13 code scanning findings:
- 1 ReDoS error
- 5 URL sanitization warnings
- 7 missing workflow permissions warnings

Fixes #147

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: accept GitHub SSH host aliases in repo detection

Accept ssh.github.com (port 443) in addition to github.com when
detecting GitHub repositories via SSH remotes. This ensures
repositories using git@ssh.github.com:owner/repo.git are properly
detected as GitHub repos.

Addresses review feedback on PR #148

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: prevent async cleanup issues in test mode

Remove manual Live display cleanup that could cause EOFError.
The _safe_progress_display function already handles test mode
by skipping progress display, so direct save path is sufficient.

Fixes test_unlock_section failure with EOFError/ValueError.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: detect GitHub remotes using ssh:// and git:// URLs

Extend URL pattern matching to support ssh://git@github.com/owner/repo.git
and git://github.com/owner/repo.git formats in addition to existing
https?:// and scp-style git@host:path URLs.

This fixes a regression where these valid GitHub URL formats were not
detected, causing detect() to return false for repos using these schemes.

Addresses review feedback on PR #149

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: bump version to 0.26.9 and update changelog

- Update version from 0.26.8 to 0.26.9
- Add changelog entry for GitHub remote detection fix and code scanning fixes

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: compare GitHub SSH hostnames case-insensitively

Lowercase host_part before comparison to handle mixed-case hostnames
like git@GitHub.com:org/repo.git. This restores the case-insensitive
behavior from the previous config_content.lower() check and prevents
regression where valid GitHub repos with mixed-case hostnames would
not be detected.

Addresses review feedback on PR #150

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add openspec and workflow commands for transparency

* Add specs from openspec

* Remove aisp change which wasn't implemented

* Fix openspec gitignore pattern

* Update gitignore

* Update contribution standards to use openspec for SDD

* Migrate to new opsx openspec commands

* Migrate workflow and openspec config

* fix: bump version to 0.26.10 for PyPI publish

- Sync version across pyproject.toml, setup.py, src/__init__.py, src/specfact_cli/__init__.py
- Add CHANGELOG entry for 0.26.10 (fixes incorrect version publish issue)

Co-authored-by: Cursor <cursoragent@cursor.com>

* Update version and changelog

* Add canonical user-friendly workitem url for ado workitems

* Update to support OSPX

* feat(backlog): implement refine --import-from-tmp and fix type-check (#156)

* feat(backlog): implement --import-from-tmp for refine export/import round-trip

- Add _parse_refined_export_markdown() to parse export-format markdown (ID, Body, Acceptance Criteria, optional title/metrics)
- Import branch: read file, match by ID, update items; --write calls adapter.update_backlog_item()
- Remove 'Import functionality pending implementation' message
- Unit tests for parser (single item, AC/metrics, header-only, blocks without ID)
- Bump version to 0.26.11 and sync across pyproject.toml, setup.py, src/__init__.py, src/specfact_cli/__init__.py
- OpenSpec change: implement-backlog-refine-import-from-tmp (proposal, tasks, spec delta)

Fixes #155

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix type check issues

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* feat: debug logs under ~/.specfact/logs and release 0.26.13 (#159)

* feat: add debug logs under ~/.specfact/logs with operation metadata

- User-level log dir: get_specfact_home_logs_dir() (~/.specfact/logs, 0o755)
- debug_print() routes to console and rotating specfact-debug.log when --debug
- debug_log_operation() for structured metadata (ADO, GitHub, backlog, init)
- CLI init_debug_log_file() when --debug; help text updated

Closes #158
OpenSpec change: add-debug-logs-specfact-home

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add debug logging for selected commands at first

* release: 0.26.13 - debug log parity for upgrade, versions and changelog

- Log upgrade success (up to date) to ~/.specfact/logs/specfact-debug.log
- Bump version to 0.26.13; sync pyproject.toml, setup.py, src/__init__.py, specfact_cli/__init__.py
- CHANGELOG: 0.26.13 Fixed entry for upgrade debug parity

Co-authored-by: Cursor <cursoragent@cursor.com>

* Remove pr markdown

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Potential fix for pull request finding 'Empty except'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>

* Fix unused variable review

* Fix unused variable review

* Fix type and test errors

* Finalize change

* Change for debug logs archived

* fix: improve ADO backlog refine error logging and user-facing error UX (#164)

* Improving error logging capabilities

* small fix on changelog

* Archived change

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>

* feat: backlog refine --ignore-refined and --id, startup docs (fixes #166) (#167)

* feat: backlog refine --ignore-refined and --id, startup docs (fixes #166)

OpenSpec change: improve-backlog-refine-and-cli-startup. Adds --ignore-refined/--no-ignore-refined, --id <issue-id>; helper _item_needs_refinement; interactive refinement prompt section; version 0.26.15.

* Add change for this branch and improve change create workflow

* Improve refinement prompt and add specification feedback, update docs and add backlog refinement tutorial

* Fix spec update and tasks

* Improve pr orchestrator pipeline triggers

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>

* Add change proposals for full scrum support

* Add support for systematic, structured issue creation with copilot help

* feat(backlog): daily standup defaults, iteration/sprint, unassigned items view (#174)

* Issue 179 resolution (#180)

* fix(backlog): address CodeQL/Codex PR 181 findings

- Replace empty except with debug_log_operation in _load_standup_config and _load_backlog_config (correct signature: operation, target, status, error)
- Add dim console message in sprint end date parse except block
- Gate summarize prompt description/comments on --comments; add include_comments to _build_summarize_prompt_content and call site
- Add test for metadata-only summarize when include_comments=False; update existing test to pass include_comments=True

Co-authored-by: Cursor <cursoragent@cursor.com>

* Update openspec enforcement rules

* Structure openspec changes

* Fix ruff finding

* Fix linter issues with StrEnum and parameters

* Fix tests and depcreation warnings

* Improve sync script

* Add change for modular command registry

* Fix review finding on dev sync script

* Update modular change proposal

* feat: CLI modular command registry and lazy load (arch-01) (#196)

* feat: CLI modular command registry and lazy load (arch-01)

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add missing exports

* Fix lazy loading review findigns

* Removed example package and fixed tests

* Fix test failures and lazy load logic for modules

* Fix tests

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* docs: document CLI modules design; sync version and cleanup

- Add Modules design section to architecture (registry, module packages, state)
- Update module structure tree with registry/ and modules/
- Cross-reference directory-structure to architecture#modules-design
- Changelog, version, and project file updates; remove obsolete commands/prompts

Co-authored-by: Cursor <cursoragent@cursor.com>

* Archive modular change and specs

* Fix banner display on help screen

* Improve action runner on main

* Setup claude skills and instructions

* feat: module package separation for command implementations (#200)

* feat: separate module package command implementations

* docs: finalize openspec apply checklist for arch-02

* Archived arch-02 change and updated specs

* fix: restore plan sync shared compatibility import

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>

* fix: address CodeQL and Codex review findings from PR #201

- Fix unreachable code in contract init (Prompt.ask after raise typer.Exit)
- Replace empty except with print_warning for contract file load failures
- Fix repo-root fallback path depth in backlog commands after module migration

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add module lifecycle management and split init ide setup

* docs: update arch-03 tasks after pr creation

* docs: update init help text for module lifecycle and ide split

* Format missing

* fix: tighten ado assignee typing for basedpyright warning

* fix: honor init install-deps and tighten ado typing

* test: satisfy bundle converter constructor typing

* test: isolate module registry state in migration compatibility test

* Update change

* disable claude review due to high costs

---------

Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* feat: add ModuleIOContract protocol and core-module isolation

- Create ModuleIOContract protocol with four core operations

- Add static analysis enforcement preventing core->module imports

- Add ProjectBundle schema versioning (schema_version field)

- Update 5 modules to implement ModuleIOContract

- Add protocol compliance tracking in module discovery

- Create docs for ProjectBundle schema and module contracts

- Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* chore: update arch-04 task tracking after implementation and PR

* test: fix flaky help assertions and typing warnings

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Add three OpenSpec changes from Module Marketplace Decoupling Plan:
- arch-07-schema-extension-system: Schema extension mechanism for ProjectBundle
- marketplace-01-central-module-registry: Central registry MVP with module discovery
- marketplace-02-advanced-marketplace-features: Dependency resolution and custom registries

All changes include:
- Proposal, design, specs, tasks, and validation artifacts
- Source tracking linked to GitHub issues #213, #214, #215
- TDD/SDD ordering with contract-first development
- Backward compatibility guarantees

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* docs: add openspec change arch-05 bridge registry

* feat: apply arch-05 bridge registry workflow

* docs: update arch-05 apply task execution state

* fix: resolve arch-05 protocol reporting and duplicate lifecycle logs

* fix: close arch-05 review gaps for protocol reporting

* docs: mark arch-05 PR task complete

* fix: complete arch-05 module io contract migration

* fix: make module protocol startup reporting user-friendly

* fix: make debug logging work for eager cli flags

* fix: print active debug log path on debug startup

* fix: harden repro output and telemetry fallback behavior

* test: fix service bridge metadata typing in unit tests

* fix: add strict crosshair mode and clearer repro diagnostics

* fix: remove contracts import side-effects for crosshair

* fix: make crosshair exploration output specific and deduplicated

* fix: make crosshair exploration skip noisy signature-limited files

* ci: reduce specfact workflow env setup overhead

* ci: avoid hatch env sync in specfact validation workflow

* fix: stabilize crosshair exploration for side-effectful modules

* fix: improve crosshair compatibility for backlog converters

* ci: require crosshair in specfact repro workflows

* Apply fixes on crosshair tests

* ci: speed up workflow setup with cache and lean hatch installs

* ci: pin contract scenario test env to py3.12

* ci: improve contract test progress logging

* ci: increase and expose smart test timeout for scenario runs

* Fix test failure logic

* Fix test failure logic

* Reformat files

* Fix contract test findings

* Update docs integrity

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Corrects three broken spec-delta references flagged in PR #221 review:
- backlog-core-02: add-backlog-add-interactive-issue-creation → backlog-core-02-interactive-issue-creation
- backlog-scrum-02: sprint-planning-capacity-commitment-support → backlog-scrum-02-sprint-planning
- backlog-scrum-03: story-complexity-splitting-hints-support → backlog-scrum-03-story-complexity

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ndency graph

- Replace old flat backlog-01..09 naming with module-scoped groups:
  backlog-core, backlog-scrum, backlog-kanban, backlog-safe,
  policy-engine, patch-mode, bundle-mapper, ceremony-cockpit
- Add arch-06/07 and marketplace-01/02 as pending changes
- Mark arch-01 through arch-05 as implemented (archived 2026-02-04..10)
- Update all GitHub issue numbers to current (incl. new #208, #213, #214,
  #215, #220 from recent changes)
- Clarify hard vs optional dependencies; optional deps are graceful no-ops
  and not set as GitHub blockers
- Update implementation waves to reflect current unblocked state (Wave 0 done)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
djm81 and others added 6 commits March 24, 2026 22:59
…445)

* feat(init): IDE prompt source catalog, --prompts, namespaced exports

Implement init-ide-prompt-source-selection: discover core + module prompts,
default export all sources, interactive multi-select, non-interactive --prompts,
source-namespaced IDE paths. Fix project module roots to use metadata source
project. Extend discovery roots with user/marketplace. Update startup_checks
for nested exports. Bump init module to 0.1.14 with signed manifest.

Made-with: Cursor

* fix(init): scope VS Code prompt recommendations to exported sources

- Pass prompts_by_source into create_vscode_settings from copy_prompts_by_source_to_ide
- Strip prior .github/prompts/* recommendations on selective export to avoid stale paths
- Extract helpers for catalog paths and fallbacks; keep code review clean

Made-with: Cursor

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
- Remove [Unreleased] sections; fold historical arch-08 notes under [0.34.0]
- Document init ide catalog, VS Code recommendations, integration test isolation

Made-with: Cursor
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • specfact init ide now discovers prompts from core plus installed modules across effective roots, supports non‑interactive --prompts and interactive multi‑select, and exports IDE files into per‑source namespaced subfolders (creates or refreshes templates).
  • Bug Fixes

    • IDE recommendation lists and startup drift checks reflect actual exports (no stale entries); selective exports prune stale files and avoid filename collisions; CLI error reporting improved for invalid/empty prompt selections.
  • Documentation

    • Updated CLI/module docs, changelog (0.42.5), design and migration guidance.
  • Tests

    • Added unit/integration/e2e tests for source selection, namespaced exports, pruning, persistence, and stability.

Walkthrough

Build prompt-source catalogs from core plus installed modules across effective module roots for specfact init ide; add --prompts non‑interactive selection and interactive multi‑select; export IDE files into per‑source namespaced subfolders; update docs, tests, startup checks, and CLI wiring to persist export state.

Changes

Cohort / File(s) Summary
Version metadata
pyproject.toml, setup.py, src/__init__.py, src/specfact_cli/__init__.py, src/specfact_cli/modules/init/module-package.yaml
Bumped package and module versions; updated module integrity metadata.
CLI init command
src/specfact_cli/modules/init/src/commands.py
Added --prompts option, prompt-source catalog wiring, interactive selection and parsing, missing-source error paths, export-state persistence, and extended copy_templates_to_ide to accept prompts_by_source.
IDE setup utilities
src/specfact_cli/utils/ide_setup.py
Introduced source-aware model (PROMPT_SOURCE_CORE), discover_prompt_sources_catalog, source_id_to_path_segment, namespaced export/copy (copy_prompts_by_source_to_ide, modified _copy_template_files_to_ide), export-audit helpers, state read/write, and updated create_vscode_settings to emit namespaced recommendations and prune stale SpecFact-managed entries.
Startup drift checks
src/specfact_cli/utils/startup_checks.py
Added _find_ide_exported_prompt_file and updated drift scanning to resolve exports in flat or namespaced layouts.
CLI registry helper
src/specfact_cli/cli.py
Added rebuild_root_app_from_registry() to rebuild the Typer root app from the CommandRegistry.
Docs & changelog
CHANGELOG.md, docs/core-cli/init.md, docs/core-cli/module.md, openspec/CHANGE_ORDER.md
Documented v0.42.5 behavior: multi-root discovery, --prompts, namespaced exports, and updated change-tracking entry.
Design/spec/validation/TDD
openspec/changes/init-ide-prompt-source-selection/...
Added/expanded proposal, design, spec, validation, TDD evidence, and tasks defining root-aware discovery, non-download re-sync semantics, interactive/non-interactive selection, and namespaced exports.
Tests — unit, integration, e2e
tests/unit/..., tests/integration/test_command_package_runtime_validation.py, tests/e2e/test_init_command.py
Added unit tests for source-id sanitization, catalog discovery, namespaced copying, --prompts parsing; restored module-root globals after temp-home in integration test; updated e2e to assert namespaced export directories and tightened missing-catalog messaging.

Sequence Diagram

sequenceDiagram
    participant User as User
    participant CLI as "init_ide Command"
    participant Discovery as "discover_prompt_sources_catalog"
    participant Roots as "Effective Module Roots"
    participant IDE as "copy_prompts_by_source_to_ide"
    participant Settings as "create_vscode_settings"

    User->>CLI: specfact init ide [--prompts tokens]
    CLI->>Discovery: discover_prompt_sources_catalog(repo_path)
    Discovery->>Roots: scan core, project, user, marketplace, configured roots
    Roots-->>Discovery: catalog { core: [...], module:id: [...], ... }
    Discovery-->>CLI: prompt sources catalog

    alt --prompts provided
        CLI->>CLI: _parse_prompts_option_to_catalog(tokens)
    else interactive
        CLI->>CLI: _select_prompt_sources_interactive(catalog)
    else default
        CLI->>CLI: use all discovered sources
    end

    CLI->>IDE: copy_prompts_by_source_to_ide(selected_catalog, force?)
    loop per selected source
        IDE->>IDE: source_id_to_path_segment(source_id) -> safe_segment
        IDE->>IDE: copy files -> <ide_dir>/<safe_segment>/...
    end

    IDE->>Settings: create_vscode_settings(prompts_by_source=selected_catalog)
    Settings->>User: write namespaced .github/prompts/<segment>/ recommendations
    IDE-->>CLI: summary of exported sources
    CLI-->>User: "Prompt sources exported: ..."
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • IDE Prompt Source Selection #382: Implements the init-ide prompt source selection orchestration (catalog discovery, namespaced exports, --prompts parsing, and tests), directly addressing that issue.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes a patch release promotion from dev to main for v0.42.5, accurately reflecting the main purpose of the changeset.
Description check ✅ Passed The PR description covers key highlights from the changelog, identifies it as a patch release, and completes the provided checklist, though it does not follow the repository's detailed description template structure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

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

@djm81 djm81 self-assigned this Mar 25, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38a9d218b0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/specfact_cli/utils/ide_setup.py Outdated
Comment thread src/specfact_cli/modules/init/src/commands.py Outdated
@djm81 djm81 added bug Something isn't working enhancement New feature or request dependencies Dependency resolution and management labels Mar 25, 2026
@djm81 djm81 moved this from Todo to In Progress in SpecFact CLI Mar 25, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

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

⚠️ Outside diff range comments (1)
src/specfact_cli/modules/init/src/commands.py (1)

328-338: ⚠️ Potential issue | 🟠 Major

Don't audit against the full discovered catalog after a selective export.

Line 328 and Line 338 now compute missing and outdated from every currently discoverable prompt source. After a valid specfact init ide --prompts core, a later specfact init will still report the unselected module prompts as missing/outdated, even though that selective export was intentional. Audit needs to key off the last exported source set, not the full catalog.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/specfact_cli/modules/init/src/commands.py` around lines 328 - 338, The
code currently computes expected_paths via
expected_ide_prompt_export_paths(repo_path, detected_ide) and then reports
missing/outdated against the whole discovered catalog; instead, change the
lookup to use the last exported prompt set (e.g. read the last export metadata
for the IDE or introduce a helper like last_exported_ide_prompt_paths(repo_path,
detected_ide)) and base both expected_paths and the outdated count on that set;
update the call site (where expected_ide_prompt_export_paths is used and where
count_outdated_ide_prompt_exports(repo_path, detected_ide) is invoked) so
missing = [p for p in expected_paths if not p.exists()] and outdated =
count_outdated_ide_prompt_exports(repo_path, detected_ide, expected_paths) (or
equivalent) operate only on the last exported source set rather than the full
catalog.
🧹 Nitpick comments (3)
openspec/CHANGE_ORDER.md (1)

152-152: Use the exact change-folder identifier for the module-migration-11 blocker.

Using the canonical id improves traceability and avoids ambiguity in future parsing/search.

🧭 Suggested wording tweak
-| init-ide | 01 | init-ide-prompt-source-selection | [`#382`](https://github.com/nold-ai/specfact-cli/issues/382) | backlog-module-ownership-cleanup; packaging-02-cross-platform-runtime-and-module-resources; modules-repo/packaging-01-bundle-resource-payloads (`#101`); module-migration-11 command-ownership alignment |
+| init-ide | 01 | init-ide-prompt-source-selection | [`#382`](https://github.com/nold-ai/specfact-cli/issues/382) | backlog-module-ownership-cleanup; packaging-02-cross-platform-runtime-and-module-resources; modules-repo/packaging-01-bundle-resource-payloads (`#101`); module-migration-11-project-codebase-ownership-realignment |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openspec/CHANGE_ORDER.md` at line 152, Replace the informal token
"module-migration-11" in the table row for "init-ide | 01 |
init-ide-prompt-source-selection" with the exact canonical change-folder
identifier (the exact folder name used under the changes/changelogs repository
for that blocker); locate the correct folder name in the changes directory or PR
that corresponds to the module-migration-11 blocker and substitute it verbatim
so the table entry matches the change-folder ID used for parsing and
traceability.
openspec/changes/init-ide-prompt-source-selection/specs/init-ide-prompt-source-selection/spec.md (2)

7-31: Add blank lines before scenario headings to fix MD022 warnings.

Static analysis flagged missing blank lines before headings at lines 7, 17, 21, and 26. As per coding guidelines, headers should be surrounded by blank lines.

📝 Proposed fix for MD022 compliance
 - **AND** the catalog is built from the effective built-in, project, user, and configured custom module roots for that repository context.
 
 ### Requirement: Init IDE Must Discover Sources From Installed Module Roots Only
 
 `specfact init ide` SHALL discover prompt and related module-owned resources from installed module roots and packaged resource directories. It SHALL not fetch module archives or treat the modules source repository as a runtime extraction source.
 
+
 #### Scenario: Installed project-scope bundle contributes prompt resources
 - **WHEN** a repository has an installed module under `<repo>/.specfact/modules`
 - **THEN** `specfact init ide` can discover that module's packaged prompt resources for export in that repository.
 
+
 #### Scenario: Installed user-scope bundle contributes prompt resources
 - **WHEN** a user has installed a module under `~/.specfact/modules`
 - **AND** no overriding project-scope copy shadows it
 - **THEN** `specfact init ide` can discover that module's packaged prompt resources for export.
 
+
 #### Scenario: Missing selected source does not trigger install work
 - **WHEN** a selected prompt source is not installed or does not expose the required packaged resources
 - **THEN** `specfact init ide` fails or warns with actionable guidance
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@openspec/changes/init-ide-prompt-source-selection/specs/init-ide-prompt-source-selection/spec.md`
around lines 7 - 31, Add a blank line before each heading that currently
violates MD022 so the markdown has blank lines surrounding headers; specifically
insert an empty line immediately before the headings "#### Scenario: Default
export includes core and installed modules across effective roots", "###
Requirement: Init IDE Must Discover Sources From Installed Module Roots Only",
"#### Scenario: Installed project-scope bundle contributes prompt resources",
and "#### Scenario: Installed user-scope bundle contributes prompt resources" in
the spec file to satisfy MD022 and ensure headers are separated from prior text.

54-61: Add blank line before scenario heading at line 58.

MD022 requires blank lines around headings.

📝 Proposed fix
 Exported prompt files SHALL preserve module/core provenance so collisions are deterministic and later command-surface migrations do not silently overwrite unrelated prompts.
 
+
 #### Scenario: Multiple sources expose similarly named prompts
 - **WHEN** `core` and one or more installed modules expose prompt files with overlapping basenames or command affinity
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@openspec/changes/init-ide-prompt-source-selection/specs/init-ide-prompt-source-selection/spec.md`
around lines 54 - 61, Add a single blank line immediately before the "####
Scenario: Multiple sources expose similarly named prompts" heading so there is
an empty line between the preceding paragraph/heading ("Requirement: Exported
Prompt Files Must Preserve Source Provenance") and the scenario heading; this
satisfies MD022 by ensuring blank lines around the heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Line 415: Move the two bullets currently listed under the 0.34.0 entry in
CHANGELOG.md (the one about `specfact init` module state/validation built from
`discover_all_package_metadata()` and the related bullet at the other location)
into the 0.42.5 release notes under the appropriate section (e.g.,
Changed/Fixed); remove them from 0.34.0 so chronology is correct and ensure the
exact bullet text is preserved when adding it to the 0.42.5 section.

In `@docs/core-cli/module.md`:
- Around line 12-13: The paragraph mentioning "specfact module init --scope
user|project", "specfact module install --scope user|project", and "specfact
init ide" is a single long line; wrap it to multiple lines so no line exceeds
120 characters (split between logical phrases such as after commands or commas)
to satisfy markdown lint and reduce diff churn.

In `@openspec/changes/init-ide-prompt-source-selection/proposal.md`:
- Around line 21-22: Add surrounding blank lines before and after the markdown
headings "## Capabilities" and "### New Capabilities" in proposal.md so they
comply with MD022; specifically ensure there is a blank line above "##
Capabilities" and a blank line between "## Capabilities" and "### New
Capabilities" (and a blank line after "### New Capabilities") to fix the lint
warnings referenced at the other occurrences (lines ~27 and ~36) as well.

In `@src/specfact_cli/modules/init/src/commands.py`:
- Around line 404-412: The checkbox call using questionary.checkbox(...,
default=labels) is wrong because questionary expects a single default value;
replace the multi-select preselection by constructing choices as questionary
Choice objects with checked=True for items present in labels instead of passing
default=labels: locate the checkbox invocation around selected = (cast(Any,
questionary).checkbox(...).ask()) and change the choices list (the labels
variable) to a list of Choice(name, checked=True) for items that should be
preselected while keeping _questionary_style() and the prompt text intact.

In `@src/specfact_cli/utils/ide_setup.py`:
- Around line 483-503: The selective export currently only copies selected
sources and leaves previously exported files behind; update the logic in the
function using prompts_by_source (the loop that calls
_copy_template_files_to_ide and uses source_id_to_path_segment) to first compute
the set of currently managed target paths for the chosen sources under the IDE
export tree (using repo_path, ide and the same source_segment logic) and remove
any existing files/directories in those source segments that are not in that set
before calling _copy_template_files_to_ide; ensure deletion is safe (only within
the IDE export root/segment) and preserve behavior of force/write_settings, then
proceed to copy and return all_copied and settings_path as before.

In `@tests/e2e/test_init_command.py`:
- Around line 225-229: The assertion currently allows any generic "Error" to
pass; remove the broad '"Error" in result.stdout' alternative and instead assert
against specific expected failure messages from the command (e.g., "No prompt
templates found" or "Templates directory not found") or add exact, deterministic
error text/regex that the CLI emits; update the failing test's assertion
expression (the block checking result.stdout) to only accept those specific
messages or a precise pattern so unrelated failures cannot be masked.

---

Outside diff comments:
In `@src/specfact_cli/modules/init/src/commands.py`:
- Around line 328-338: The code currently computes expected_paths via
expected_ide_prompt_export_paths(repo_path, detected_ide) and then reports
missing/outdated against the whole discovered catalog; instead, change the
lookup to use the last exported prompt set (e.g. read the last export metadata
for the IDE or introduce a helper like last_exported_ide_prompt_paths(repo_path,
detected_ide)) and base both expected_paths and the outdated count on that set;
update the call site (where expected_ide_prompt_export_paths is used and where
count_outdated_ide_prompt_exports(repo_path, detected_ide) is invoked) so
missing = [p for p in expected_paths if not p.exists()] and outdated =
count_outdated_ide_prompt_exports(repo_path, detected_ide, expected_paths) (or
equivalent) operate only on the last exported source set rather than the full
catalog.

---

Nitpick comments:
In `@openspec/CHANGE_ORDER.md`:
- Line 152: Replace the informal token "module-migration-11" in the table row
for "init-ide | 01 | init-ide-prompt-source-selection" with the exact canonical
change-folder identifier (the exact folder name used under the
changes/changelogs repository for that blocker); locate the correct folder name
in the changes directory or PR that corresponds to the module-migration-11
blocker and substitute it verbatim so the table entry matches the change-folder
ID used for parsing and traceability.

In
`@openspec/changes/init-ide-prompt-source-selection/specs/init-ide-prompt-source-selection/spec.md`:
- Around line 7-31: Add a blank line before each heading that currently violates
MD022 so the markdown has blank lines surrounding headers; specifically insert
an empty line immediately before the headings "#### Scenario: Default export
includes core and installed modules across effective roots", "### Requirement:
Init IDE Must Discover Sources From Installed Module Roots Only", "####
Scenario: Installed project-scope bundle contributes prompt resources", and
"#### Scenario: Installed user-scope bundle contributes prompt resources" in the
spec file to satisfy MD022 and ensure headers are separated from prior text.
- Around line 54-61: Add a single blank line immediately before the "####
Scenario: Multiple sources expose similarly named prompts" heading so there is
an empty line between the preceding paragraph/heading ("Requirement: Exported
Prompt Files Must Preserve Source Provenance") and the scenario heading; this
satisfies MD022 by ensuring blank lines around the heading.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b50cf179-fee2-422d-a0ab-c0366830d8c7

📥 Commits

Reviewing files that changed from the base of the PR and between ea9ee60 and 38a9d21.

📒 Files selected for processing (23)
  • CHANGELOG.md
  • docs/core-cli/init.md
  • docs/core-cli/module.md
  • openspec/CHANGE_ORDER.md
  • openspec/changes/init-ide-prompt-source-selection/CHANGE_VALIDATION.md
  • openspec/changes/init-ide-prompt-source-selection/TDD_EVIDENCE.md
  • openspec/changes/init-ide-prompt-source-selection/design.md
  • openspec/changes/init-ide-prompt-source-selection/proposal.md
  • openspec/changes/init-ide-prompt-source-selection/specs/init-ide-prompt-source-selection/spec.md
  • openspec/changes/init-ide-prompt-source-selection/tasks.md
  • pyproject.toml
  • setup.py
  • src/__init__.py
  • src/specfact_cli/__init__.py
  • src/specfact_cli/modules/init/module-package.yaml
  • src/specfact_cli/modules/init/src/commands.py
  • src/specfact_cli/utils/ide_setup.py
  • src/specfact_cli/utils/startup_checks.py
  • tests/e2e/test_init_command.py
  • tests/integration/test_command_package_runtime_validation.py
  • tests/unit/modules/init/test_init_ide_prompt_selection.py
  • tests/unit/modules/init/test_resource_resolution.py
  • tests/unit/utils/test_ide_setup.py

Comment thread CHANGELOG.md
Comment thread docs/core-cli/module.md
Comment thread openspec/changes/init-ide-prompt-source-selection/proposal.md
Comment thread src/specfact_cli/modules/init/src/commands.py Outdated
Comment thread src/specfact_cli/utils/ide_setup.py
Comment thread src/specfact_cli/utils/ide_setup.py Outdated
Comment thread tests/e2e/test_init_command.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)
tests/unit/registry/test_category_groups.py (2)

17-21: 🧹 Nitpick | 🔵 Trivial

Consider resetting app.registered_groups in the fixture for extra isolation.

While the current implementation works because all tests call rebuild_root_app_from_registry(), adding app state reset to the fixture would provide defense-in-depth against future tests that might forget to call it.

♻️ Proposed enhancement for fixture
 `@pytest.fixture`(autouse=True)
 def _clear_registry() -> Generator[None, None, None]:
+    from specfact_cli.cli import app
     CommandRegistry._clear_for_testing()
+    app.registered_groups = []
+    if hasattr(app, "registered_commands"):
+        app.registered_commands = []
     yield
     CommandRegistry._clear_for_testing()
+    app.registered_groups = []
+    if hasattr(app, "registered_commands"):
+        app.registered_commands = []
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/registry/test_category_groups.py` around lines 17 - 21, The
fixture _clear_registry currently calls CommandRegistry._clear_for_testing()
before and after each test but doesn't reset FastAPI app state; modify the
fixture to also clear app.registered_groups (e.g., set app.registered_groups =
[] or call the app cleanup method) both before yield and after yield to ensure
tests are isolated even if they don't call rebuild_root_app_from_registry();
keep references to CommandRegistry._clear_for_testing() and
rebuild_root_app_from_registry() intact.

69-90: 🧹 Nitpick | 🔵 Trivial

Consider removing unused tmp_path parameters.

Several test functions accept tmp_path: Path but never use it. While harmless, removing unused fixtures keeps tests cleaner and more explicit about their dependencies.

♻️ Proposed fix to remove unused parameters
-def test_code_analyze_routes_same_as_flat_analyze(
-    tmp_path: Path,
-) -> None:
+def test_code_analyze_routes_same_as_flat_analyze() -> None:

Apply similar changes to:

  • test_govern_help_when_not_installed_suggests_install
  • test_flat_validate_is_not_found_in_copilot_mode
  • test_flat_validate_is_not_found_in_cicd_mode
  • test_spec_api_validate_routes_correctly

Also applies to: 92-110, 112-133, 135-153, 156-174

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/registry/test_category_groups.py` around lines 69 - 90, Tests like
test_code_analyze_routes_same_as_flat_analyze declare an unused tmp_path
fixture; remove the unused tmp_path: Path parameter from that function and from
the other listed test functions
(test_govern_help_when_not_installed_suggests_install,
test_flat_validate_is_not_found_in_copilot_mode,
test_flat_validate_is_not_found_in_cicd_mode,
test_spec_api_validate_routes_correctly) so the test signatures only include
required fixtures; update any import/usage references if present and run tests
to ensure no missing fixtures remain.
♻️ Duplicate comments (2)
src/specfact_cli/modules/init/src/commands.py (1)

411-420: ⚠️ Potential issue | 🔴 Critical

questionary.checkbox(default=labels) still passes a list, causing runtime errors.

This issue was flagged in a previous review. The questionary.checkbox prompt's default parameter accepts only a single Optional[str], not a list. To preselect all items, use Choice(..., checked=True) objects in the choices list.

Suggested fix
-    labels = [f"{k}  ({len(catalog[k])} template(s))" for k in keys]
-    label_to_key = {labels[i]: keys[i] for i in range(len(keys))}
-
-    selected = (
-        cast(Any, questionary)
-        .checkbox(
-            "Select prompt sources:",
-            choices=labels,
-            default=labels,
-            style=_questionary_style(),
-        )
-        .ask()
-    )
+    checkbox_choices = [
+        cast(Any, questionary).Choice(
+            f"{k}  ({len(catalog[k])} template(s))",
+            value=k,
+            checked=True,
+        )
+        for k in keys
+    ]
+
+    selected = (
+        cast(Any, questionary)
+        .checkbox(
+            "Select prompt sources:",
+            choices=checkbox_choices,
+            style=_questionary_style(),
+        )
+        .ask()
+    )

Note: With this change, selected will contain the source keys directly (not the display labels), so adjust the loop at lines 424-428 accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/specfact_cli/modules/init/src/commands.py` around lines 411 - 420, The
checkbox prompt is passing default=labels (a list) which is invalid; instead
construct the choices as questionary Choice objects with checked=True to
preselect all entries (replace the current labels list with Choice(label,
value=key, checked=True) items) and call questionary.checkbox(...,
choices=choices).ask(); also update the post-selection loop that currently
expects display labels to handle the returned values (which will now be source
keys) when building the selected sources.
src/specfact_cli/utils/ide_setup.py (1)

517-544: ⚠️ Potential issue | 🟠 Major

Selective exports do not prune stale files from previous runs.

This concern was raised in a previous review. When a user runs init ide --prompts all, then later init ide --prompts core, the old module prompt files remain on disk. Only the VS Code recommendations are cleaned up, not the actual files.

Consider whether this is intentional (users can manually delete) or if file pruning should be implemented to avoid accumulating stale exports.

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/specfact_cli/modules/init/module-package.yaml`:
- Around line 20-21: The committed module-package.yaml contains a stale
integrity.signature that fails CI; reopen the manifest, re-sign the file with
the project's configured signing key and replace the integrity.signature value
(and recompute/update checksum if the content changed) so the signature
validates; after signing run the project's module signature verification command
(the same verifier used in CI) locally to confirm the new integrity.signature
and checksum pass before committing the updated module-package.yaml.

---

Outside diff comments:
In `@tests/unit/registry/test_category_groups.py`:
- Around line 17-21: The fixture _clear_registry currently calls
CommandRegistry._clear_for_testing() before and after each test but doesn't
reset FastAPI app state; modify the fixture to also clear app.registered_groups
(e.g., set app.registered_groups = [] or call the app cleanup method) both
before yield and after yield to ensure tests are isolated even if they don't
call rebuild_root_app_from_registry(); keep references to
CommandRegistry._clear_for_testing() and rebuild_root_app_from_registry()
intact.
- Around line 69-90: Tests like test_code_analyze_routes_same_as_flat_analyze
declare an unused tmp_path fixture; remove the unused tmp_path: Path parameter
from that function and from the other listed test functions
(test_govern_help_when_not_installed_suggests_install,
test_flat_validate_is_not_found_in_copilot_mode,
test_flat_validate_is_not_found_in_cicd_mode,
test_spec_api_validate_routes_correctly) so the test signatures only include
required fixtures; update any import/usage references if present and run tests
to ensure no missing fixtures remain.

---

Duplicate comments:
In `@src/specfact_cli/modules/init/src/commands.py`:
- Around line 411-420: The checkbox prompt is passing default=labels (a list)
which is invalid; instead construct the choices as questionary Choice objects
with checked=True to preselect all entries (replace the current labels list with
Choice(label, value=key, checked=True) items) and call questionary.checkbox(...,
choices=choices).ask(); also update the post-selection loop that currently
expects display labels to handle the returned values (which will now be source
keys) when building the selected sources.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1c8f888c-5a10-4aa8-b6d6-91b78c12c3b9

📥 Commits

Reviewing files that changed from the base of the PR and between 38a9d21 and f552107.

📒 Files selected for processing (6)
  • src/specfact_cli/cli.py
  • src/specfact_cli/modules/init/module-package.yaml
  • src/specfact_cli/modules/init/src/commands.py
  • src/specfact_cli/utils/ide_setup.py
  • tests/unit/registry/test_category_groups.py
  • tests/unit/utils/test_ide_setup.py

Comment thread src/specfact_cli/modules/init/module-package.yaml Outdated
…ation strip

- Prune stale exports and unselected catalog segments in copy_prompts_by_source_to_ide
- Strip only specfact*.prompt.md under .github/prompts/ when merging VS Code settings
- Tighten e2e missing-templates assertions to match CLI output
- Add unit tests for prompt path helper and selective export behavior

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (3)
src/specfact_cli/modules/init/module-package.yaml (1)

20-21: ⚠️ Potential issue | 🔴 Critical

Re-sign this manifest before merge.

The committed integrity.signature is still failing the module-signature hardening check, so this release cannot pass required CI until the manifest is re-signed with the configured key and re-verified locally. Based on learnings, "Module signature verification must pass before PR creation; run hatch run ./scripts/verify-modules-signature.py --require-signature".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/specfact_cli/modules/init/module-package.yaml` around lines 20 - 21, The
module manifest's signature in module-package.yaml (fields integrity.signature
and checksum) is invalid; re-sign the manifest using the repository's configured
signing key, update integrity.signature accordingly, and then locally verify the
signature by running the verification helper (hatch run
./scripts/verify-modules-signature.py --require-signature) until it passes;
ensure you reference the same module-package.yaml when signing and repeat
signing+verification until the script reports success before pushing the PR.
tests/unit/modules/init/test_init_ide_prompt_selection.py (1)

128-133: ⚠️ Potential issue | 🟠 Major

Don't let a generic "Error" satisfy this test.

The invalid-source path has deterministic output, but or "Error" in result.stdout will also pass on unrelated CLI failures and hide regressions in the prompt-selection flow.

Possible fix
-    assert "not available" in result.stdout.lower() or "Error" in result.stdout
+    out = result.stdout
+    assert "not available" in out.lower(), out
+    assert "available sources:" in out.lower(), out

As per coding guidelines, "Tests must be meaningful and test actual functionality, cover both success and failure cases".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/modules/init/test_init_ide_prompt_selection.py` around lines 128 -
133, The test currently allows any generic "Error" text to satisfy failure,
which can mask unrelated CLI regressions; update the assertion in
test_init_ide_prompt_selection.py to check for the deterministic failure message
produced when the prompt source is invalid (e.g., assert "not available" in
result.stdout.lower() or assert the specific prompt identifier
"nold-ai/not-installed" or the exact deterministic error text your
prompt-selection code emits), keep the exit_code == 1 check, and remove the
broad 'or "Error" in result.stdout' branch so the test only passes when the
known invalid-source message from the init/ide prompt-selection flow (invoked
via runner.invoke(app, ["init","ide", "--repo", str(tmp_path),
"--ide","cursor","--prompts","nold-ai/not-installed","--force"])) appears.
src/specfact_cli/utils/ide_setup.py (1)

421-440: ⚠️ Potential issue | 🟠 Major

Cleanup still misses sources that disappeared from discovery.

_remove_unselected_prompt_export_segments() only walks the current catalog. If a module was exported in a previous run and is later uninstalled or renamed, its old <ide>/<segment>/... directory is no longer in catalog, so it never gets pruned and the IDE keeps surfacing stale prompts. This cleanup needs to consider the previously exported source set (or existing managed segment dirs), not just the sources discoverable right now.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/specfact_cli/utils/ide_setup.py` around lines 421 - 440, The current
_remove_unselected_prompt_export_segments only iterates
discover_prompt_sources_catalog and therefore misses previously exported
segments that no longer appear in the current catalog; update
_remove_unselected_prompt_export_segments to enumerate existing managed segment
directories (e.g. list children under the IDE export root resolved via
_safe_resolved_segment_dir or by constructing repo_path / ide and iterating its
segments), compute the set of segment IDs from those directories (using
source_id_to_path_segment for comparisons), and remove any on-disk segment not
present in the current selected set (prompts_by_source.keys()), while keeping
the same OSError handling and console messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/specfact_cli/utils/ide_setup.py`:
- Around line 815-850: The saved export state includes an "ide" but
load_ide_prompt_export_source_ids currently ignores it; change
load_ide_prompt_export_source_ids to accept an ide: str parameter and after
reading the YAML check that raw_dict.get("ide") matches the provided ide (if
not, return None), keeping the rest of the validation/return logic intact;
update the function signature and its contracts/annotations (and any callers) so
the load uses the IDE-scoped file content written by
write_ide_prompt_export_state (which uses IDE_PROMPT_EXPORT_STATE_FILE and
payload["ide"]).

In `@tests/e2e/test_init_command.py`:
- Around line 223-230: The test's empty-catalog branch must be made
deterministic by stubbing discover_prompt_sources_catalog() (or the module root
discovery it calls) to return an empty dict so prompt discovery cannot pick up
global prompts; update the test around the existing branch that checks
result.exit_code to patch discover_prompt_sources_catalog() to return {} for
this run, then assert result.exit_code == 1 and that both "No prompt templates
found" and "Seed or install modules first" appear in result.stdout. Ensure the
stub is applied only for this test case and removed/reset after the assertion so
other tests are unaffected.

---

Duplicate comments:
In `@src/specfact_cli/modules/init/module-package.yaml`:
- Around line 20-21: The module manifest's signature in module-package.yaml
(fields integrity.signature and checksum) is invalid; re-sign the manifest using
the repository's configured signing key, update integrity.signature accordingly,
and then locally verify the signature by running the verification helper (hatch
run ./scripts/verify-modules-signature.py --require-signature) until it passes;
ensure you reference the same module-package.yaml when signing and repeat
signing+verification until the script reports success before pushing the PR.

In `@src/specfact_cli/utils/ide_setup.py`:
- Around line 421-440: The current _remove_unselected_prompt_export_segments
only iterates discover_prompt_sources_catalog and therefore misses previously
exported segments that no longer appear in the current catalog; update
_remove_unselected_prompt_export_segments to enumerate existing managed segment
directories (e.g. list children under the IDE export root resolved via
_safe_resolved_segment_dir or by constructing repo_path / ide and iterating its
segments), compute the set of segment IDs from those directories (using
source_id_to_path_segment for comparisons), and remove any on-disk segment not
present in the current selected set (prompts_by_source.keys()), while keeping
the same OSError handling and console messages.

In `@tests/unit/modules/init/test_init_ide_prompt_selection.py`:
- Around line 128-133: The test currently allows any generic "Error" text to
satisfy failure, which can mask unrelated CLI regressions; update the assertion
in test_init_ide_prompt_selection.py to check for the deterministic failure
message produced when the prompt source is invalid (e.g., assert "not available"
in result.stdout.lower() or assert the specific prompt identifier
"nold-ai/not-installed" or the exact deterministic error text your
prompt-selection code emits), keep the exit_code == 1 check, and remove the
broad 'or "Error" in result.stdout' branch so the test only passes when the
known invalid-source message from the init/ide prompt-selection flow (invoked
via runner.invoke(app, ["init","ide", "--repo", str(tmp_path),
"--ide","cursor","--prompts","nold-ai/not-installed","--force"])) appears.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 313f0a23-7166-480f-8d10-2be5d688b1a0

📥 Commits

Reviewing files that changed from the base of the PR and between f552107 and dab2ffe.

📒 Files selected for processing (6)
  • src/specfact_cli/modules/init/module-package.yaml
  • src/specfact_cli/modules/init/src/commands.py
  • src/specfact_cli/utils/ide_setup.py
  • tests/e2e/test_init_command.py
  • tests/unit/modules/init/test_init_ide_prompt_selection.py
  • tests/unit/utils/test_ide_setup.py

Comment thread src/specfact_cli/utils/ide_setup.py
Comment thread tests/e2e/test_init_command.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/specfact_cli/modules/init/module-package.yaml (1)

19-21: ⚠️ Potential issue | 🔴 Critical

Module signature verification fails in CI.

The pipeline reports that module signatures are stale for the configured signing key. The integrity.signature value must be recomputed after all module content changes are finalized, and verified locally before committing.

Based on learnings: "Module signature verification must pass before PR creation; run hatch run ./scripts/verify-modules-signature.py --require-signature" and "Verify module-package.yaml files with module signature verification before committing".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/specfact_cli/modules/init/module-package.yaml` around lines 19 - 21, The
integrity.signature in module-package.yaml is stale; recompute and re-sign the
package after finalizing module content and update the integrity.signature field
with the new signature. Run the provided verification script locally (hatch run
./scripts/verify-modules-signature.py --require-signature) to generate and
validate the new signature, and only commit the updated integrity.signature once
the script succeeds; ensure integrity.checksum remains correct for the package
contents before pushing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/specfact_cli/modules/init/module-package.yaml`:
- Around line 19-21: The integrity.signature in module-package.yaml is stale;
recompute and re-sign the package after finalizing module content and update the
integrity.signature field with the new signature. Run the provided verification
script locally (hatch run ./scripts/verify-modules-signature.py
--require-signature) to generate and validate the new signature, and only commit
the updated integrity.signature once the script succeeds; ensure
integrity.checksum remains correct for the package contents before pushing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 395c24c9-c814-491f-b2a5-7d5230bd788b

📥 Commits

Reviewing files that changed from the base of the PR and between dab2ffe and fbb2307.

📒 Files selected for processing (6)
  • src/specfact_cli/modules/init/module-package.yaml
  • src/specfact_cli/modules/init/src/commands.py
  • src/specfact_cli/utils/ide_setup.py
  • tests/e2e/test_init_command.py
  • tests/unit/modules/init/test_init_ide_prompt_selection.py
  • tests/unit/utils/test_ide_setup.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Dependency resolution and management enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant