Conversation
…ics (#311) * fix(backlog): harden refine writeback, prompts, and daily any filters * fix(github): default story type fallback to feature * Fix format * Fix codex review findings * bump and sign changed modules * chore(hooks): enforce module signature verification in pre-commit * chore(hooks): add markdownlint to pre-commit checks --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* fix(backlog): harden refine writeback, prompts, and daily any filters * fix(github): default story type fallback to feature * Fix format * Fix codex review findings * bump and sign changed modules * chore(hooks): enforce module signature verification in pre-commit * chore(hooks): add markdownlint to pre-commit checks * fix: finalize backlog-core-06 ado comment api versioning and ci hatch pins * fix: address review findings for formatter safety and ado metric patch guards * docs(openspec): update CHANGE_ORDER status tracking * fix(ado): apply iteration filter for direct issue_id lookup --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…olution, aliases, custom registries, publishing (#318) * feat: advanced marketplace features (marketplace-02) - dependency resolution, aliases, custom registries, namespace enforcement, publishing - dependency_resolver: resolve_dependencies(), --skip-deps, --force on install - alias_manager: alias create/list/remove (no top-level alias commands) - custom_registries: add-registry, list-registries, remove-registry; fetch_all_indexes; search Registry column - module_installer: namespace/name enforcement, collision detection - scripts/publish-module.py + .github/workflows/publish-modules.yml (optional signing) - docs: publishing-modules, custom-registries, dependency-resolution; updated installing-modules, module-marketplace, commands - version 0.38.0, CHANGELOG Made-with: Cursor * docs(openspec): defer 6.2.4 and 6.2.5 (index update/PR, workflow test) to later Made-with: Cursor * Add follow-up change proposals for marketplace * Fix codex review findings --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
0.38.1 Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* feat(backlog): summarize Markdown normalization and TTY/CI rendering * chore(openspec): drop implementation snapshot from change * Update title --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* feat(cli): category groups and flat shims using real module Typer - Add category groups (code, backlog, project, spec, govern) with flatten same-name member - Sort commands under backlog/project groups A–Z - Fix flat shims to expose real module Typer so 'specfact sync bridge' and 'specfact plan update-idea' work - Add first-run init, module grouping, OpenSpec change for 0.40.x remove-flat-shims - Bump version to 0.39.0, CHANGELOG and OpenSpec updates Made-with: Cursor * Fix signature * fix: resolve module grouping regressions and stabilize CI tests * fix: keep uncategorized modules flat during grouped registration --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* docs: add module-migration-02-bundle-extraction to CHANGE_ORDER.md * feat: implement module-migration-02 bundle extraction * fix(ci): checkout module bundles repo for test jobs * Fix test failures * fix(modules): load local bundle sources in compatibility aliases * fix: run worktree policy code in tests/CI and silence reexport deprecation - Prefer src/<name>/main.py over app.py when SPECFACT_REPO_ROOT is set so policy init uses worktree templates.py (SPECFACT_POLICY_TEMPLATES_DIR). - Policy engine module-package.yaml: version 0.1.5 and re-signed checksum. - conftest: set SPECFACT_REPO_ROOT, SPECFACT_POLICY_TEMPLATES_DIR; add bundle package roots when specfact-cli-modules present. - Policy engine integration tests: rely on conftest env, clear registry and re-register before invoke so loader uses worktree. - test_reexport_shims: filter deprecation warning for legacy analyze import. Made-with: Cursor * fix: defer specfact_backlog import in shims so CI can register bridges - backlog and policy_engine __init__.py: import specfact_backlog only in __getattr__ (cached), not at module load. Allows loading .src.adapters.* for bridge registration without requiring specfact_backlog installed. - Re-sign backlog and policy_engine module-package.yaml after init changes. - openspec: update module-migration-02 tasks.md. Made-with: Cursor * fix: defer bundle import in all module shims to fix CI collection errors - Apply deferred import (only in __getattr__, cached) to analyze, contract, drift, enforce, generate, import_cmd, migrate, patch_mode, plan, project, repro, sdd, spec, sync, validate. Matches backlog and policy_engine. - Prevents ImportError when tests import specfact_cli.modules.<name>.src.* without specfact_backlog/specfact_govern/specfact_project/specfact_spec installed (e.g. CI). Fixes 78 collection errors. - Re-sign all affected module-package.yaml manifests. Made-with: Cursor * fix(ci): include module shims in hatch cache key so CI uses current code * feat(modules): registry descriptions, --bump-version for publish, tasks and format fixes - Add description to registry index entries in publish-module.py (module search) - Add --bump-version patch|minor|major for bundle re-publish in publish-module.py - Format fixes in validate-modules-repo-sync.py (SIM108, B007) - Mark completed tasks in module-migration-02-bundle-extraction tasks.md - Update test for publish_bundle(bump_version=) signature Made-with: Cursor * Add missing migration tasks to the open change to completely isolate modules into specfact-cli-modules repo. * Add gap analysis and update changes * Update follow-up changes to avoid ambiguities and overlaps * docs: complete migration-02 section-18 parity and 17.8 gate evidence * docs: mark migration-02 import-categorization commit checkpoint done * Update change constraints and blockers for module migration * docs: add migration-05 issue #334 and complete task 17.10.4 * Update change constraints and blockers for module migration --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…317) (#341) * Prepare module-migration-03 removal of old built-in modules * feat(core): delete specfact-project module source from core (migration-03) Made-with: Cursor * feat(core): delete specfact-backlog module source from core (migration-03) Made-with: Cursor * feat(core): delete specfact-codebase module source from core (migration-03) Made-with: Cursor * feat(core): delete specfact-spec module source from core (migration-03) Made-with: Cursor * feat(core): delete specfact-govern module source from core (migration-03) Made-with: Cursor * chore(tests): skip tests for removed modules when source absent (migration-03) Add pytest.importorskip() for backlog, plan, sync, enforce, generate, patch_mode, import_cmd so tests are skipped when module source was removed from core. Preserves tests for later move to specfact-cli-modules. Update tasks.md and TDD_EVIDENCE.md for Task 10 completion. Made-with: Cursor * feat(bootstrap): remove flat shims and non-core module registrations (migration-03) - Remove _register_category_groups_and_shims (unconditional category/shim registration). - Trim CORE_MODULE_ORDER to 4 core: init, auth, module-registry, upgrade. - Add @beartype to _mount_installed_category_groups. - Category groups and flat shims only for installed bundles via _mount_installed_category_groups. Made-with: Cursor * docs(openspec): mark Task 11.4 done in tasks.md Made-with: Cursor * feat(cli): conditional category group mount from installed bundles (migration-03) - Add _RootCLIGroup (extends ProgressiveDisclosureGroup) with resolve_command override: unknown commands in KNOWN_BUNDLE_GROUP_OR_SHIM_NAMES show actionable error (not installed + specfact init / specfact module install). - Root app uses cls=_RootCLIGroup. Main help docstring adds init/module install hint for workflow bundles. Made-with: Cursor * docs(openspec): mark Task 12.4 done in tasks.md Made-with: Cursor * feat(init): enforce mandatory bundle selection and profile presets (migration-03) * Add module removal core tests * docs(openspec): record Task 14 module signing gate (migration-03) * feat: complete module-migration-03 core slimming and follow-up alignment (#317) * Fix format error * fix: handle detached HEAD registry branch selection and stabilize migration-03 CI tests --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…p detection) (#539) * refactor(upgrade): resolve clean-code warnings in upgrade detection flow * chore(modules): bump upgrade manifest checksum and version * fix(upgrade): address review findings for openspec evidence and path-safe detection * chore(release): bump version artifacts to 0.46.10 * fix(upgrade): pin uv pip upgrades to detected interpreter * fix(upgrade): quote pip executable and suppress uvx check-only command hint * fix(cli): gracefully handle missing lazy command groups in help/delegation * fix(cli,upgrade): handle runtime lazy-command errors and uv tool detection * Fix: failing tests * fix(cli): handle stale flat lazy shims --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (5)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used🔀 Multi-repo context nold-ai/specfact-cli-modulesLinked repositories findingsnold-ai/specfact-cli-modules
Summary: this repo contains multiple consumers affected by the PR areas (module discovery via SPECFACT_MODULES_ROOTS, CLI lazy delegation/help fallback, and upgrade command messaging/behavior). Reviewers should verify docs, check-docs-commands validation, the plan app mounting, and tests that assert upgrade/help text remain consistent with the new install-method-aware outputs and discovery ordering. 📝 WalkthroughUser-Visible CLI Behavior
Contract / API Impact
Testing and Quality Gates
OpenSpec, CHANGELOG, Module Signing & Version Bumps
Notes for Maintainers / Review Focus
Summary of Release Metadata
WalkthroughBumps project to v0.46.16 and implements install-method-aware ChangesLazy delegation / stale flat shim
Install-method-aware upgrade flow
Module discovery explicit roots & pre-commit env builder
Release & metadata
Sequence Diagram(s)sequenceDiagram
participant User as User / CLI
participant LazyGroup as _LazyDelegateGroup
participant Registry as Command Registry
participant Typer as Typer / Delegate
participant ClickCmd as Click Command
User->>LazyGroup: invoke "specfact plan" (no subcommand args)
LazyGroup->>Registry: resolve_command("plan")
alt Registry returns delegate
Registry-->>LazyGroup: (cmd_name, cmd_obj, [])
LazyGroup->>Typer: _load_lazy_delegate_typer()
Typer-->>LazyGroup: Typer app / command
LazyGroup->>ClickCmd: build & invoke delegated Click command
ClickCmd-->>User: result/exit
else Registry stale or missing
Registry-->>LazyGroup: None
LazyGroup->>Typer: attempt to materialize Typer command
alt Typer materializes
LazyGroup->>ClickCmd: invoke fallback Click command
ClickCmd-->>User: result
else materialization fails
LazyGroup-->>User: print minimal fallback help (no internal traceback)
end
end
sequenceDiagram
participant User as User / CLI
participant UpgradeCmd as upgrade command
participant Detect as detect_installation_method
participant Subproc as subprocess.run
participant Build as _build_upgrade_command
participant Execute as _execute_upgrade_command
User->>UpgradeCmd: specfact upgrade [--yes / --check-only]
UpgradeCmd->>Detect: detect_installation_method()
Detect->>Subproc: probe uvx / UV_PROJECT_ENVIRONMENT / "uv tool list" / "pipx list" / "python -m pip show"
Subproc-->>Detect: outputs (method, location)
Detect-->>UpgradeCmd: InstallationMethod{method,command,location}
alt method == "uvx"
UpgradeCmd-->>User: print uvx informational refresh message (no subprocess)
else
UpgradeCmd->>Build: _build_upgrade_command(InstallationMethod)
Build-->>UpgradeCmd: argv list
UpgradeCmd->>Execute: _execute_upgrade_command(argv)
Execute->>Subproc: subprocess.run(argv, check=False, timeout=300)
Subproc-->>Execute: exit_code
alt exit_code == 0
Execute->>UpgradeCmd: update_metadata()
UpgradeCmd-->>User: success message
else
Execute-->>User: failure message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
djm81
added
bug
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b7d968ffa
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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/upgrade/src/commands.py (1)
196-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMetadata update failure masks successful upgrade.
If
subprocess.runsucceeds (line 199-203) butupdate_metadata()(line 206) raisesOSErrororTypeError, the genericexcept Exceptionat line 211 catches it and returnsFalsewith "Update failed". The user sees failure despite the upgrade completing successfully.Per the relevant context snippet (
src/specfact_cli/utils/metadata.py:80-112),update_metadata()can raiseOSErroron file write failures.🛡️ Proposed fix: separate metadata update error handling
console.print("[green]✓ Update successful![/green]") from datetime import datetime - update_metadata(last_checked_version=__version__, last_version_check_timestamp=datetime.now(UTC).isoformat()) + try: + update_metadata(last_checked_version=__version__, last_version_check_timestamp=datetime.now(UTC).isoformat()) + except Exception as meta_err: + console.print(f"[yellow]Warning: Could not update metadata: {meta_err}[/yellow]") return True except subprocess.TimeoutExpired: console.print("[red]✗ Update timed out (exceeded 5 minutes)[/red]") return False except Exception as e: console.print(f"[red]✗ Update failed: {e}[/red]") return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/modules/upgrade/src/commands.py` around lines 196 - 213, The current _execute_upgrade_command masks successful upgrades when update_metadata() fails; change the flow so that subprocess.run success still returns True even if update_metadata raises (e.g., OSError/TypeError). Specifically, after detecting result.returncode == 0 in _execute_upgrade_command, call update_metadata() inside its own try/except that catches OSError and TypeError (and optionally logs the error via console.print) but does not change the overall success return value; keep existing handling for subprocess.TimeoutExpired and non-zero return codes intact.
🧹 Nitpick comments (2)
src/specfact_cli/cli.py (1)
561-563: 💤 Low valueError is echoed twice when raising ClickException.
click.echo(str(exc), err=True)prints the error, thenclick.ClickException(str(exc))also prints the same message when raised. This results in duplicate error output.♻️ Remove redundant echo
def _raise_lazy_delegate_click_exception(exc: Exception) -> NoReturn: - click.echo(str(exc), err=True) raise click.ClickException(str(exc)) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/cli.py` around lines 561 - 563, The helper _raise_lazy_delegate_click_exception currently calls click.echo(str(exc), err=True) and then raises click.ClickException(str(exc)), causing the same error message to be printed twice; remove the explicit echo and simply raise click.ClickException(str(exc)) (or vice-versa keep echo and raise without message) so only one output occurs, ensuring the function only raises click.ClickException in place of the duplicate click.echo call.src/specfact_cli/modules/upgrade/src/commands.py (1)
80-83: 💤 Low valueuvx detection via substring match is fragile.
The check
"uvx" in sys.argv[0] or "uvx" in executable_pathcan produce false positives for paths containing "uvx" as a substring (e.g.,/home/uvx_user/...or/opt/myuvx/python).♻️ Consider checking path segments instead
def _detect_uvx_installation(executable_path: str) -> InstallationMethod | None: - if "uvx" in sys.argv[0] or "uvx" in executable_path: + argv0_base = Path(sys.argv[0]).name if sys.argv else "" + if argv0_base == "uvx" or "uvx" in executable_path.split(os.sep): return InstallationMethod(method="uvx", command="uvx --from specfact-cli specfact --version", location=None) return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/modules/upgrade/src/commands.py` around lines 80 - 83, The uvx detection in _detect_uvx_installation is using substring checks which cause false positives; change it to examine path segments/basenames instead: use sys.argv[0] and executable_path parsed as path objects (or os.path.basename / Path(...).parts) and only match when the final executable name or a complete path segment equals "uvx" (and consider common exe extensions on Windows), then return InstallationMethod(method="uvx", command="uvx --from specfact-cli specfact --version", location=None) as before; reference _detect_uvx_installation, sys.argv[0], and executable_path when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/CHANGE_ORDER.md`:
- Around line 643-647: Open and complete the verification checklist entries
4.1–4.3 for the change "upgrade-01-install-method-aware": run the required
quality gates and specfact code review and record results under tasks.md (item
4.1), execute the explicit merge-readiness command openspec validate
upgrade-01-install-method-aware --strict and attach its passing output to
tasks.md (item 4.2), then run pre-commit validation, fix any findings, and
document the fixes and final passing pre-commit status (item 4.3); once all
gates pass, mark those checklist items as complete so the change can be merged.
In `@openspec/changes/upgrade-01-install-method-aware/tasks.md`:
- Around line 17-19: Tasks 4.1-4.3 in tasks.md are still unchecked but the PR
claims validation passed; run the required strict validation using the exact
command openspec validate upgrade-01-install-method-aware --strict, resolve any
specfact/contract/pre-commit failures, then update tasks.md by marking 4.1–4.3
as [x] and add the validation output and any fixes to TDD_EVIDENCE.md so the
change shows the required strict OpenSpec validation evidence before merge.
In `@openspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.md`:
- Line 16: Add the actual output of running the command `openspec validate
upgrade-01-install-method-aware --strict` into TDD_EVIDENCE.md at the
passing-after section: paste the terminal output showing either a passing result
or the failing errors plus the follow-up fixes and the re-run that resulted in
pass; ensure the entry references the same command string and clearly shows the
final validation status so the change meets the OpenSpec strict validation
requirement.
In `@src/specfact_cli/modules/upgrade/module-package.yaml`:
- Line 20: The checksum metadata (the checksum:
sha256:670467523832fa3fea1fcd8f167cb1ca8f9b45e7ed553ef367a5b7eae53955f0 entry)
is out of sync with the packaged module artifact; regenerate the SHA-256 digest
from the exact module payload that will be published, update the checksum field
with the newly computed sha256 value in module-package.yaml, and commit the
updated metadata together with the package content changes in a single atomic
commit so CI verification will pass.
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 86-100: _detect_uv_installation currently sets
InstallationMethod.location to str(Path(executable_path).parent.parent) which
yields a venv directory; change it to the interpreter path so
_build_upgrade_command can pass a valid --python value. Specifically, in
_detect_uv_installation return an InstallationMethod with location set to the
resolved executable path (use the resolved `executable` or `executable_path`)
instead of the parent directory; keep method="uv" and the existing command
string so _build_upgrade_command (which reads method.location or sys.executable)
supplies a proper interpreter path.
---
Outside diff comments:
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 196-213: The current _execute_upgrade_command masks successful
upgrades when update_metadata() fails; change the flow so that subprocess.run
success still returns True even if update_metadata raises (e.g.,
OSError/TypeError). Specifically, after detecting result.returncode == 0 in
_execute_upgrade_command, call update_metadata() inside its own try/except that
catches OSError and TypeError (and optionally logs the error via console.print)
but does not change the overall success return value; keep existing handling for
subprocess.TimeoutExpired and non-zero return codes intact.
---
Nitpick comments:
In `@src/specfact_cli/cli.py`:
- Around line 561-563: The helper _raise_lazy_delegate_click_exception currently
calls click.echo(str(exc), err=True) and then raises
click.ClickException(str(exc)), causing the same error message to be printed
twice; remove the explicit echo and simply raise click.ClickException(str(exc))
(or vice-versa keep echo and raise without message) so only one output occurs,
ensuring the function only raises click.ClickException in place of the duplicate
click.echo call.
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 80-83: The uvx detection in _detect_uvx_installation is using
substring checks which cause false positives; change it to examine path
segments/basenames instead: use sys.argv[0] and executable_path parsed as path
objects (or os.path.basename / Path(...).parts) and only match when the final
executable name or a complete path segment equals "uvx" (and consider common exe
extensions on Windows), then return InstallationMethod(method="uvx",
command="uvx --from specfact-cli specfact --version", location=None) as before;
reference _detect_uvx_installation, sys.argv[0], and executable_path when making
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 636631f4-8382-415a-b203-7f22f60e1a71
📒 Files selected for processing (20)
CHANGELOG.mdopenspec/CHANGE_ORDER.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdpyproject.tomlscripts/pre_commit_code_review.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/module-package.yamlsrc/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/registry/module_discovery.pytests/integration/test_core_slimming.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.pytests/unit/scripts/test_pre_commit_code_review.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Type Checking (basedpyright)
- GitHub Check: Compatibility (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (20)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use@lru_cacheand Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...
Files:
src/specfact_cli/__init__.pysetup.pysrc/__init__.pytests/integration/test_core_slimming.pysrc/specfact_cli/registry/module_discovery.pytests/unit/scripts/test_pre_commit_code_review.pysrc/specfact_cli/cli.pytests/unit/cli/test_lean_help_output.pyscripts/pre_commit_code_review.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.pysrc/specfact_cli/modules/upgrade/src/commands.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.Add/update contracts on new or modified public APIs, stateful classes and adapters using
icontractdecorators andbeartyperuntime type checks
src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers insrc/must usesnake_case(modules/functions),PascalCase(classes),UPPER_SNAKE_CASE(constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...
Files:
src/specfact_cli/__init__.pysrc/__init__.pysrc/specfact_cli/registry/module_discovery.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
@(src|tests)/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Linting must pass with no errors using: pylint src tests
Files:
src/specfact_cli/__init__.pysrc/__init__.pytests/integration/test_core_slimming.pysrc/specfact_cli/registry/module_discovery.pytests/unit/scripts/test_pre_commit_code_review.pysrc/specfact_cli/cli.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.pysrc/specfact_cli/modules/upgrade/src/commands.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality:hatch run format, (2) Type checking:hatch run type-check(basedpyright), (3) Contract-first approach: Runhatch run contract-testfor contract validation, (4) Run full test suite:hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have@icontractdecorators and@beartypetype checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments
Files:
src/specfact_cli/__init__.pysetup.pysrc/__init__.pytests/integration/test_core_slimming.pysrc/specfact_cli/registry/module_discovery.pytests/unit/scripts/test_pre_commit_code_review.pysrc/specfact_cli/cli.pytests/unit/cli/test_lean_help_output.pyscripts/pre_commit_code_review.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.pysrc/specfact_cli/modules/upgrade/src/commands.py
src/specfact_cli/**/*.py
⚙️ CodeRabbit configuration file
src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify@icontract+@beartypeon public surfaces; prefer centralized logging
(get_bridge_logger) over print().
Files:
src/specfact_cli/__init__.pysrc/specfact_cli/registry/module_discovery.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
{src/__init__.py,pyproject.toml,setup.py}
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
{src/__init__.py,pyproject.toml,setup.py}: Update src/init.py first as primary source of truth for package version, then pyproject.toml and setup.py
Maintain version synchronization across src/init.py, pyproject.toml, and setup.py
Files:
setup.pypyproject.tomlsrc/__init__.py
{pyproject.toml,setup.py,src/__init__.py}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Manually update version numbers in pyproject.toml, setup.py, and src/init.py when making a formal version change
Files:
setup.pypyproject.tomlsrc/__init__.py
pyproject.toml
📄 CodeRabbit inference engine (.cursorrules)
When updating the version in
pyproject.toml, ensure it's newer than the latest PyPI version. The CI/CD pipeline will automatically publish to PyPI only if the new version is greater than the published version
Files:
pyproject.toml
**/*.{md,mdc}
📄 CodeRabbit inference engine (.cursor/rules/markdown-rules.mdc)
**/*.{md,mdc}: Do not use more than one consecutive blank line anywhere in the document (MD012: No Multiple Consecutive Blank Lines)
Fenced code blocks should be surrounded by blank lines (MD031: Fenced Code Blocks)
Lists should be surrounded by blank lines (MD032: Lists)
Files must end with a single empty line (MD047: Files Must End With Single Newline)
Lines should not have trailing spaces (MD009: No Trailing Spaces)
Use asterisks (**) for strong emphasis, not underscores (__) (MD050: Strong Style)
Fenced code blocks must have a language specified (MD040: Fenced Code Language)
Headers should increment by one level at a time (MD001: Header Increment)
Headers should be surrounded by blank lines (MD022: Headers Should Be Surrounded By Blank Lines)
Only one top-level header (H1) is allowed per document (MD025: Single H1 Header)
Use consistent list markers, preferring dashes (-) for unordered lists (MD004: List Style)
Nested unordered list items should be indented consistently, typically by 2 spaces (MD007: Unordered List Indentation)
Use exactly one space after the list marker (e.g., -, *, +, 1.) (MD030: Spaces After List Markers)
Use incrementing numbers for ordered lists (MD029: Ordered List Item Prefix)
Enclose bare URLs in angle brackets or format them as links (MD034: Bare URLs)
Don't use spaces immediately inside code spans (MD038: Spaces Inside Code Spans)
Use consistent indentation (usually 2 or 4 spaces) throughout markdown files
Keep line length under 120 characters in markdown files
Use reference-style links for better readability in markdown files
Use a trailing slash for directory paths in markdown files
Ensure proper escaping of special characters in markdown files
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.mdCHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.mdCHANGELOG.md
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Treat as specification source of truth: proposal/tasks/spec deltas vs. code behavior,
CHANGE_ORDER consistency, and scenario coverage. Surface drift between OpenSpec and
implementation.
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.md
openspec/changes/**/*.md
📄 CodeRabbit inference engine (.cursorrules)
For
/opsx:archive(Archive change): Include module signing and cleanup in final tasks. Agents MUST runopenspec archive <change-id>from repo root (no manualmvunderopenspec/changes/archive/)
Files:
openspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.md
openspec/**/{proposal.md,tasks.md,design.md,spec.md}
📄 CodeRabbit inference engine (.cursor/rules/automatic-openspec-workflow.mdc)
openspec/**/{proposal.md,tasks.md,design.md,spec.md}: Applyopenspec/config.yamlproject context and per-artifact rules (for proposal, specs, design, tasks) when creating or updating any OpenSpec change artifact in the specfact-cli codebase
After implementation, validate the change withopenspec validate <change-id> --strict; fix validation errors in proposal, specs, design, or tasks and re-validate until passing before considering the change complete
Files:
openspec/changes/upgrade-01-install-method-aware/proposal.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdopenspec/changes/upgrade-01-install-method-aware/specs/upgrade-command/spec.md
**/*.yaml
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
YAML files must pass linting using: hatch run yaml-lint with relaxed policy.
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
**/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate YAML configuration files locally using
hatch run yaml-lintbefore committing
**/*.{yml,yaml}: Format all YAML and workflow files usinghatch run yaml-fix-allbefore committing
Use Prettier to fix whitespace, indentation, and final newline across YAML files
Use yamllint with the repo .yamllint configuration (line-length 140, trailing spaces and final newline enforced) to lint non-workflow YAML files
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
Include new version entries at the top of CHANGELOG.md when updating versions
Update CHANGELOG.md with all code changes as part of version control requirements.
Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change
Files:
CHANGELOG.md
**/test_*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/test_*.py: Write tests first in test-driven development (TDD) using the Red-Green-Refactor cycle
Ensure each test is independent and repeatable with no shared state between tests
Organize Python imports in tests using unittest.mock for Mock and patch
Use setup_method() for test initialization and Arrange-Act-Assert pattern in test files
Use@pytest.mark.asynciodecorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module
Files:
tests/integration/test_core_slimming.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Tests must be meaningful and test actual functionality, cover both success and failure cases, be independent and repeatable, and have clear, descriptive names. NO EXCEPTIONS - no placeholder or empty tests.
tests/**/*.py: Trim low-value unit tests when a contract covers the same assertion (type/shape/raises on negative checks)
Delete tests that only assert input validation, datatype/shape enforcement, or raises on negative conditions now guarded by contracts and runtime typing
Convert repeated edge-case permutations into one Hypothesis property with contracts acting as oraclesSecret redaction via
LoggerSetup.redact_secretsmust be covered by unit tests
Files:
tests/integration/test_core_slimming.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.
Files:
tests/integration/test_core_slimming.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/registry/test_module_discovery.py
**/*cli*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
Commands should follow typer patterns with rich console output
Files:
src/specfact_cli/cli.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented
quality gates (format, type-check, module signing).
Files:
scripts/pre_commit_code_review.py
🪛 GitHub Actions: PR Orchestrator - SpecFact CLI
src/specfact_cli/modules/upgrade/module-package.yaml
[error] 1-1: Module verification failed: checksum mismatch. FAIL src/specfact_cli/modules/upgrade/module-package.yaml
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Tests assert presence/behavior of the top-level "plan" command and import the plan command module:
- tests/unit/test_specfact_project_manifest_commands.py — asserts "plan" in commands. [::nold-ai/specfact-cli-modules::]
- tests/integration/specfact_project/test_command_apps.py — references "specfact_project.plan.commands". [::nold-ai/specfact-cli-modules::]
- tests/e2e/specfact_project/test_help_smoke.py — imports "specfact_project.plan.commands". [::nold-ai/specfact-cli-modules::]
- tests/unit/specfact_project/test_module_io_contracts.py — lists "specfact_project.plan.commands". [::nold-ai/specfact-cli-modules::]
Relevance: changes to lazy delegate/help behavior that affect stale flat shims routing (e.g.,
specfact plan) may impact these tests/consumers. -
Environment variable for module discovery referenced in docs:
- docs/guides/module-marketplace.md — documents legacy/custom roots and SPECFACT_MODULES_ROOTS. [::nold-ai/specfact-cli-modules::]
Relevance: PR introduced honoring SPECFACT_MODULES_ROOTS in module discovery and pre-commit child env logic; consumers/tests that set or expect module roots could be affected.
-
No occurrences found in this repo for the new upgrade helpers or function names (e.g., detect_installation_method, install_update, _build_upgrade_command, _execute_upgrade_command, InstallationMethod) — the upgrade implementation lives in the core repo under src/specfact_cli/modules/upgrade; this repository contains modules and consumers but not those function definitions. [::nold-ai/specfact-cli-modules::]
Overall: tests and modules in this repo depend on the presence/behavior of the top-level "plan" command and on module-root discovery via SPECFACT_MODULES_ROOTS; both areas are directly relevant to the PR changes (lazy delegate routing for stale shims and explicit module-roots env handling).
🔇 Additional comments (14)
pyproject.toml (1)
7-7: Version synchronization looks consistent.
pyproject.tomlis aligned with the corresponding version bumps in package init and setup metadata.src/__init__.py (1)
6-6: Package version constant update is correct.The
__version__value is in sync with the release bump across packaging files.src/specfact_cli/__init__.py (1)
48-48: Internal package version bump is aligned.No mismatch detected with the other version sources updated in this PR.
setup.py (1)
10-10: Setup metadata version is correctly synchronized.The release version in
setup.pymatches the other updated version sources.src/specfact_cli/registry/module_discovery.py (1)
78-125: Discovery-root precedence change is well-scoped and consistent with the feature goal.Explicit
SPECFACT_MODULES_ROOTShandling before user/marketplace/custom roots is a solid, deterministic extension.scripts/pre_commit_code_review.py (1)
166-189: Child process environment handling is a good hardening step.The new env builder keeps parent env untouched while reliably wiring module-root and runtime paths for the nested review command.
tests/unit/scripts/test_pre_commit_code_review.py (1)
304-359: Test coverage for the new child-env behavior is strong.The added cases validate repo discovery, explicit env precedence, XDG overrides, and no mutation of ambient
os.environ.src/specfact_cli/cli.py (1)
651-666: LGTM: Lazy delegate invocation handles bare command execution.The updated
invokemethod correctly delegates execution even when no subcommand is provided (ctx.invoked_subcommand is None and not ctx.args). This fixes the stale flat shim issue wherespecfact planwould fail with empty Click usage instead of showing install guidance. Theresolve_commandchange at line 665 returning empty argv instead ofNoneensures consistent delegation.tests/integration/test_core_slimming.py (1)
182-205: LGTM: Stale flat shim regression test is well-structured.The test correctly simulates a stale lazy delegate by registering a
plancommand, rebuilding the root app, then clearing the registry to force the stale shim path. Thefinallyblock ensures registry cleanup for test isolation. This provides essential regression coverage for the CLI delegation fix documented in the PR objectives.tests/unit/cli/test_lean_help_output.py (1)
109-142: LGTM: Unit tests cover lazy delegate edge cases.
test_stale_lazy_flat_shim_prints_install_guidanceverifies that a cleared registry produces actionable output rather than empty failures.test_lazy_delegate_help_falls_back_when_typer_command_build_failsconfirms the help fallback path whentyper.main.get_commandraises RuntimeError. Both tests maintain proper registry cleanup for isolation.tests/unit/commands/test_update.py (2)
196-206: LGTM: uv tool upgrade test coverage.The test correctly verifies that
install_updatefor auv toolinstallation method executes["uv", "tool", "upgrade", "specfact-cli"]with proper timeout handling. This aligns with the OpenSpec requirement for uv-native upgrade invocation.
230-249: This test correctly asserts the current implementation behavior. However, whether passing a venv directory path touv pip install --pythonis correct requires verification with uv itself, as the code currently usesPath(executable_path).parent.parent(a directory) rather than the full interpreter path. If this is indeed incompatible with uv's expectations, both_detect_uv_installationin commands.py and this assertion would need updating.tests/unit/registry/test_module_discovery.py (1)
124-143: LGTM: SPECFACT_MODULES_ROOTS priority test.This test verifies that explicit module roots from
SPECFACT_MODULES_ROOTStake precedence over user-installed modules, correctly returningsource == "custom". This behavior is documented in the linked repository'sdocs/guides/module-marketplace.mdand is essential for developer-selected module sources to shadow stale user installs.CHANGELOG.md (1)
13-21: Release note is clear and correctly scoped for the 0.46.12 fix.This entry is top-ordered, user-impact focused, and clearly explains the stale-shim lazy-delegate regression and expected behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
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/upgrade/src/commands.py (1)
203-219:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't turn a post-install metadata write failure into an install failure.
subprocess.run()has already performed the upgrade beforeupdate_metadata()runs. Sincesrc/specfact_cli/utils/metadata.py:82-111can raise on file writes, the current broadexceptpath reports the whole upgrade as failed after printing success, even though the package may already be updated.As per coding guidelines, "No broad `except Exception` without re-raising or logging in `src/`".Suggested fix
def _execute_upgrade_command(command: list[str]) -> bool: try: console.print("[cyan]Updating SpecFact CLI...[/cyan]") result = subprocess.run(command, check=False, timeout=300) if result.returncode != 0: console.print(f"[red]✗ Update failed with exit code {result.returncode}[/red]") return False - console.print("[green]✓ Update successful![/green]") - from datetime import datetime - - update_metadata(last_checked_version=__version__, last_version_check_timestamp=datetime.now(UTC).isoformat()) - return True except subprocess.TimeoutExpired: console.print("[red]✗ Update timed out (exceeded 5 minutes)[/red]") return False - except Exception as e: - console.print(f"[red]✗ Update failed: {e}[/red]") + except OSError as e: + console.print(f"[red]✗ Update failed: {e}[/red]") return False + + console.print("[green]✓ Update successful![/green]") + from datetime import datetime + + try: + update_metadata(last_checked_version=__version__, last_version_check_timestamp=datetime.now(UTC).isoformat()) + except OSError as e: + console.print(f"[yellow]Upgrade succeeded, but metadata could not be updated: {e}[/yellow]") + return True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/modules/upgrade/src/commands.py` around lines 203 - 219, The function _execute_upgrade_command currently catches all exceptions after running subprocess.run, which turns a post-install metadata write error (from update_metadata) into an install failure; change the flow so subprocess.run errors and TimeoutExpired are still handled as failures, but call update_metadata inside its own try/except that catches file/write errors (from update_metadata) and logs or prints a non-fatal warning without returning False or re-raising; ensure after a successful subprocess.run you print success and still return True even if update_metadata fails; reference the _execute_upgrade_command function and the update_metadata call so you only narrow the exception handling around update_metadata rather than using a broad except Exception for the whole function.
♻️ Duplicate comments (1)
src/specfact_cli/modules/upgrade/src/commands.py (1)
90-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the interpreter path for uv project upgrades.
Line 102 stores the venv root directory in
location, but_build_upgrade_command()later passes that value touv pip install --python .... That makes theUV_PROJECT_ENVIRONMENTbranch generate an invalid target for uv-managed installs.Does `uv pip install --python <target>` require `<target>` to be a Python interpreter path, or can `<target>` be the virtual environment directory?Suggested fix
if executable == uv_root or uv_root in executable.parents: return InstallationMethod( method="uv", command="uv pip install --upgrade specfact-cli", - location=str(Path(executable_path).parent.parent), + location=str(executable), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/modules/upgrade/src/commands.py` around lines 90 - 103, _detect_uv_project_installation currently sets InstallationMethod.location to the virtualenv root (Path(executable_path).parent.parent), but _build_upgrade_command later uses that value with "uv pip install --python <target>" which requires a Python interpreter path; update the UV_PROJECT_ENVIRONMENT branch in _detect_uv_project_installation to set location to the resolved interpreter path (use executable_path or Path(executable_path).resolve() converted to str) so the InstallationMethod for method="uv" supplies the actual python executable to _build_upgrade_command.
🤖 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/cli.py`:
- Around line 561-564: The helper _raise_lazy_delegate_click_exception currently
prints the error with click.echo and then raises click.ClickException, which can
lead to duplicate error output; remove the explicit click.echo call and only
raise click.ClickException(str(exc)) from exc so the top-level exception handler
emits the error once (locate and edit the _raise_lazy_delegate_click_exception
function to eliminate the echo and keep the raise).
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 77-81: The fallback should use the running interpreter like
_detect_pip_installation does: replace the bare "pip install --upgrade
specfact-cli" returned in the InstallationMethod with a command constructed from
sys.executable and "-m pip install --upgrade specfact-cli" (use
shlex.quote(sys.executable) if you need to match the quoted_executable style).
Update the return in the pip fallback so InstallationMethod(method="pip",
command=<quoted sys.executable + " -m pip install --upgrade specfact-cli">,
location=None) to keep behavior consistent with _detect_pip_installation and
_build_upgrade_command.
---
Outside diff comments:
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 203-219: The function _execute_upgrade_command currently catches
all exceptions after running subprocess.run, which turns a post-install metadata
write error (from update_metadata) into an install failure; change the flow so
subprocess.run errors and TimeoutExpired are still handled as failures, but call
update_metadata inside its own try/except that catches file/write errors (from
update_metadata) and logs or prints a non-fatal warning without returning False
or re-raising; ensure after a successful subprocess.run you print success and
still return True even if update_metadata fails; reference the
_execute_upgrade_command function and the update_metadata call so you only
narrow the exception handling around update_metadata rather than using a broad
except Exception for the whole function.
---
Duplicate comments:
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 90-103: _detect_uv_project_installation currently sets
InstallationMethod.location to the virtualenv root
(Path(executable_path).parent.parent), but _build_upgrade_command later uses
that value with "uv pip install --python <target>" which requires a Python
interpreter path; update the UV_PROJECT_ENVIRONMENT branch in
_detect_uv_project_installation to set location to the resolved interpreter path
(use executable_path or Path(executable_path).resolve() converted to str) so the
InstallationMethod for method="uv" supplies the actual python executable to
_build_upgrade_command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f479e2b5-d5ba-4d98-82c2-c400d63224b6
📒 Files selected for processing (9)
CHANGELOG.mdpyproject.tomlsetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/module-package.yamlsrc/specfact_cli/modules/upgrade/src/commands.pytests/unit/commands/test_update.py
✅ Files skipped from review due to trivial changes (5)
- src/init.py
- pyproject.toml
- src/specfact_cli/init.py
- CHANGELOG.md
- tests/unit/commands/test_update.py
🚧 Files skipped from review as they are similar to previous changes (1)
- setup.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Compatibility (Python 3.11)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Compatibility (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.yaml
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
YAML files must pass linting using: hatch run yaml-lint with relaxed policy.
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
**/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate YAML configuration files locally using
hatch run yaml-lintbefore committing
**/*.{yml,yaml}: Format all YAML and workflow files usinghatch run yaml-fix-allbefore committing
Use Prettier to fix whitespace, indentation, and final newline across YAML files
Use yamllint with the repo .yamllint configuration (line-length 140, trailing spaces and final newline enforced) to lint non-workflow YAML files
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use@lru_cacheand Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...
Files:
src/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.Add/update contracts on new or modified public APIs, stateful classes and adapters using
icontractdecorators andbeartyperuntime type checks
src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers insrc/must usesnake_case(modules/functions),PascalCase(classes),UPPER_SNAKE_CASE(constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...
Files:
src/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
@(src|tests)/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Linting must pass with no errors using: pylint src tests
Files:
src/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality:hatch run format, (2) Type checking:hatch run type-check(basedpyright), (3) Contract-first approach: Runhatch run contract-testfor contract validation, (4) Run full test suite:hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have@icontractdecorators and@beartypetype checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments
Files:
src/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
**/*cli*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
Commands should follow typer patterns with rich console output
Files:
src/specfact_cli/cli.py
src/specfact_cli/**/*.py
⚙️ CodeRabbit configuration file
src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify@icontract+@beartypeon public surfaces; prefer centralized logging
(get_bridge_logger) over print().
Files:
src/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/src/commands.py
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
docs/guides/module-marketplace.md: mentions SPECFACT_MODULES_ROOTS and explicit/custom roots (line ~40). Relevant because module discovery now honors SPECFACT_MODULES_ROOTS and precedence rules. [::nold-ai/specfact-cli-modules::docs/guides/module-marketplace.md:40]
-
packages/specfact-project/src/specfact_project/plan/init.py and packages/specfact-project/src/specfact_project/plan/app.py: both re-export from specfact_project.plan.commands (provide the top-level "plan" app/command). Changes to lazy delegate/help fallback for stale flat shims (e.g., producing install guidance for
specfact plan) affect how these providers are discovered/used. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/init.py:3] [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/app.py:3] -
tests and scripts that directly reference the plan command:
- tests/unit/specfact_project/test_module_io_contracts.py: lists "specfact_project.plan.commands" as a provider. [::nold-ai/specfact-cli-modules::tests/unit/specfact_project/test_module_io_contracts.py:14]
- tests/integration/specfact_project/test_command_apps.py and tests/e2e/specfact_project/test_help_smoke.py: import specfact_project.plan.commands and invoke its app (help smoke/integration). These tests will exercise the new lazy-delegate/help-fallback and stale-shim install guidance behavior. [::nold-ai/specfact-cli-modules::tests/integration/specfact_project/test_command_apps.py:9] [::nold-ai/specfact-cli-modules::tests/e2e/specfact_project/test_help_smoke.py:12]
- scripts/check-docs-commands.py: asserts ("specfact_project.plan.commands", "app", ("specfact", "plan")) — may detect changes in command discovery/formatting. [::nold-ai/specfact-cli-modules::scripts/check-docs-commands.py:82]
- tests/unit/test_specfact_project_manifest_commands.py: asserts "plan" in commands — sensitive to top-level command availability and to lazy-delegate fallback behavior. [::nold-ai/specfact-cli-modules::tests/unit/test_specfact_project_manifest_commands.py:14]
-
Multiple packages (spec, codebase, govern, etc.) reference the active plan or call out guidance like "specfact plan select" in CLI help/messages; changes that alter help output or failure modes for stale/no-op plan shims could affect UX and tests across these consumers. Examples: packages/specfact-spec/.py and packages/specfact-codebase/.py contain numerous uses of active plan messaging. [::nold-ai/specfact-cli-modules::packages/specfact-spec/src/specfact_spec/generate/commands.py:124] [::nold-ai/specfact-cli-modules::packages/specfact-codebase/src/specfact_codebase/analyze/commands.py:78]
Summary: This repo contains many consumers and tests that import or expect the "plan" command (specfact_project.plan.commands) and documents/specs that reference SPECFACT_MODULES_ROOTS. The PR's changes to lazy delegate help/fallback behavior and to honoring SPECFACT_MODULES_ROOTS (and precedence of explicit module roots) are directly relevant and may affect tests or runtime discovery/UX in this repo.
🔇 Additional comments (1)
src/specfact_cli/cli.py (1)
621-668: No-args delegation path is a solid regression fixThe
invoke_without_command+invoke()+resolve_command()updates correctly force delegation for empty-argv stale shim cases while preserving passthrough behavior for normal delegated args.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/cli.py`:
- Around line 590-598: The delegated context loses the original command path
because a fresh Click Context has no parent; update the delegation flow so the
original prog name is preserved and used as a fallback: before calling
_delegate_cmd.main(..., prog_name=ctx.command_path, ...), store ctx.command_path
into ctx.meta under a unique key (e.g. "original_prog_name"), then change
_lazy_delegate_prog_name to, when ctx.parent is None or reconstruction yields
nothing, look up and return ctx.meta.get("original_prog_name") before falling
back to cmd_name; reference _lazy_delegate_prog_name and the place where
_delegate_cmd.main is invoked to implement the ctx.meta write/read.
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 118-122: The displayed upgrade command for uv installs (created
via InstallationMethod with method="uv" and command set) doesn't include the
--python <detected interpreter> flag that the actual execution adds; update the
code that builds method.command for the uv branch to include the detected
interpreter (use the same executable variable used for location, e.g., append "
--python {executable}") so the printed command matches the executed one, and
ensure any callers that print method.command (the display at the other sites)
will now show the correct command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 185c461e-7dbf-46ea-a2db-c62304ccca65
📒 Files selected for processing (11)
CHANGELOG.mdopenspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.mdopenspec/changes/upgrade-01-install-method-aware/tasks.mdpyproject.tomlsetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/upgrade/module-package.yamlsrc/specfact_cli/modules/upgrade/src/commands.pytests/unit/commands/test_update.py
✅ Files skipped from review due to trivial changes (4)
- src/init.py
- setup.py
- src/specfact_cli/modules/upgrade/module-package.yaml
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- pyproject.toml
- src/specfact_cli/init.py
- openspec/changes/upgrade-01-install-method-aware/TDD_EVIDENCE.md
- openspec/changes/upgrade-01-install-method-aware/tasks.md
- tests/unit/commands/test_update.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Compatibility (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Compatibility (Python 3.11)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use@lru_cacheand Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...
Files:
src/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/cli.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.Add/update contracts on new or modified public APIs, stateful classes and adapters using
icontractdecorators andbeartyperuntime type checks
src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers insrc/must usesnake_case(modules/functions),PascalCase(classes),UPPER_SNAKE_CASE(constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...
Files:
src/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/cli.py
@(src|tests)/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Linting must pass with no errors using: pylint src tests
Files:
src/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/cli.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality:hatch run format, (2) Type checking:hatch run type-check(basedpyright), (3) Contract-first approach: Runhatch run contract-testfor contract validation, (4) Run full test suite:hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have@icontractdecorators and@beartypetype checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments
Files:
src/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/cli.py
src/specfact_cli/**/*.py
⚙️ CodeRabbit configuration file
src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify@icontract+@beartypeon public surfaces; prefer centralized logging
(get_bridge_logger) over print().
Files:
src/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/cli.py
**/*cli*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
Commands should follow typer patterns with rich console output
Files:
src/specfact_cli/cli.py
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
docs/guides/module-marketplace.md: documents SPECFACT_MODULES_ROOTS as a legacy/custom root and describes discovery priority (built-in, project, user, legacy/custom). Relevant because PR changes module discovery to honor SPECFACT_MODULES_ROOTS and to prioritize explicit/custom roots. [::nold-ai/specfact-cli-modules::docs/guides/module-marketplace.md:40]
-
scripts/check-docs-commands.py: defines CORE_COMMAND_PREFIXES including ("specfact", "upgrade") and MODULE_APP_MOUNTS mapping that mounts ("specfact_project.plan.commands", "app", ("specfact", "plan")). Changes to lazy delegate/help fallback and stale flat-shim behavior may affect this script's validation and the mapping it expects. [::nold-ai/specfact-cli-modules::scripts/check-docs-commands.py:31-39][::nold-ai/specfact-cli-modules::scripts/check-docs-commands.py:70-82]
-
packages/specfact-project/src/specfact_project/plan/commands.py: contains a top-level "plan" app and an "upgrade" subcommand (occurrences near lines ~2324–2474). The repo provides the plan command app that the core CLI may lazy-delegate to; changes to lazy delegation/help-fallback and upgrade install-method detection may affect how this provider is discovered and how "specfact plan upgrade" behaves or is tested. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/commands.py:2324][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/commands.py:2334][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/commands.py:2355-2358][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/plan/commands.py:2440-2474]
-
tests referencing plan and upgrade:
- tests/unit/test_specfact_project_manifest_commands.py: asserts "plan" present in module manifest — sensitive to discovery/manifest changes. [::nold-ai/specfact-cli-modules::tests/unit/test_specfact_project_manifest_commands.py:1-14]
- tests/integration/specfact_project/test_command_apps.py and tests/e2e/specfact_project/test_help_smoke.py: import and invoke the plan app (help smoke/integration). These will exercise changed lazy-delegate/help-fallback behavior and stale-shim install guidance. [::nold-ai/specfact-cli-modules::tests/integration/specfact_project/test_command_apps.py:1-12][::nold-ai/specfact-cli-modules::tests/e2e/specfact_project/test_help_smoke.py:1-12]
-
Various docs and code reference the "upgrade" command and upgrade guidance (docs/reference, docs/guides, packages/specfact-spec generate messages). Changes to install-method-aware upgrade (detect_installation_method, install_update) could affect messaging and tests that assert upgrade output or guidance. Example: packages/specfact-spec/src/specfact_spec/generate/commands.py contains explicit upgrade/install guidance strings. [::nold-ai/specfact-cli-modules::packages/specfact-spec/src/specfact_spec/generate/commands.py:745-882]
Conclusion: this repository contains multiple consumers (module discovery/docs, the specfact_project.plan app, scripts that validate docs/commands, and tests) that are directly relevant to the PR changes in module discovery, lazy CLI delegation, and install-method-aware upgrade flows.
1 participant
Description
Release
devtomainafter the latest stabilization and upgrade-command work.Summary:
specfact upgradeinstall-method-aware foruv/uvx,pipx, andpipinstalls.dev.Fixes #539
New Features #539
Contract References: Updates lazy CLI delegate behavior in
src/specfact_cli/cli.pyand upgrade command behavior insrc/specfact_cli/modules/upgrade/src/commands.py, including@icontract-guarded paths.Type of Change
Please check all that apply:
@icontractdecorators)Contract-First Testing Evidence
Required for all changes affecting CLI commands or public APIs:
Contract Validation
@icontractdecorators on public APIs)@beartypedecorators applied)hatch run contract-testTest Execution
hatch run contract-test✅hatch run contract-test✅hatch run contract-test✅hatch run smart-test✅Test Quality
How Has This Been Tested?
Contract-First Approach: CLI delegate and upgrade command behavior is covered by focused unit/integration tests, contract gates, OpenSpec validation, and full smart-test selection on the merged
devbranch work.Manual Testing
Automated Testing
Test Environment
Checklist
@icontract,@beartype)Quality Gates Status
hatch run type-check)hatch run lint)hatch run contract-test)hatch run contract-test)hatch run contract-test)Additional release evidence:
hatch run formathatch run yaml-lintopenspec validate upgrade-01-install-method-aware --stricthatch run smart-testScreenshots/Recordings (if applicable)
Not applicable.