Repository navigation
fix(review): preserve user-scoped module installs - #454
Conversation
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. So far, Strix has reviewed 48 pull requests across this workspace. |
|
Warning Review limit reached
On-demand reviews are free for the next 22 days. After that, they cost $0.25 per reviewed file. Or wait 29 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 62 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Paired core delivery: nold-ai/specfact-cli#700 |
## Summary - replace destructive project-over-user shadowing advice in runtime discovery and `specfact module doctor` - preserve project precedence, the installed user copy, and explicit uninstall behavior - add paired OpenSpec artifacts, schema-v2 Requirements mappings, authentic red evidence, and regression tests - advance module-registry to 0.1.35 and core to 0.55.3 - refresh and cryptographically sign the changed built-in module manifest through the trusted CI signer Closes #699. Paired issue: nold-ai/specfact-cli-modules#452. Paired delivery: nold-ai/specfact-cli-modules#454. ## Verification - original authentic red checkpoint: `b5ad2ea0`; two mapped tests fail before implementation ([run 33274750805](https://github.com/nold-ai/specfact-cli/actions/runs/33274750805)) - review authentic red checkpoint: `daf05baa`; three final reviewed selectors fail before the review-fix implementation ([run 33277091672](https://github.com/nold-ai/specfact-cli/actions/runs/33277091672)) - focused review regressions: 4 passed; related discovery/doctor suite: 69 passed - immutable-fixture SpecFact full review: score 115, zero findings - final Requirements Evidence on `cef94ed7`: passed, including Code Review ([run 33278582519](https://github.com/nold-ai/specfact-cli/actions/runs/33278582519)) - strict local module signature verification against `origin/dev`: passed for module-registry 0.1.35 - final PR Orchestrator on `cef94ed7`: Python 3.11 compatibility and Python 3.12 full test/coverage suites passed; lint, type checking, contracts, signatures, security, runtime matrices, docs, and quality gates passed ([run 33278582512](https://github.com/nold-ai/specfact-cli/actions/runs/33278582512)) - review readback: all 20 inline review threads resolved; 35 checks passed with none failed or pending The paired modules PR updates repository/bootstrap guidance. The PRs are independently safe but intended for the same later patch window.
## Summary Fix only the six Markdown/OpenSpec review findings reported on promotion PR #455. No Python, workflow, package manifest, registry, version, signature, or runtime implementation file changes are included. ## Review findings addressed - Move abandoned, never-implemented R08 planning out of the completed OpenSpec archive into non-canonical abandoned history. - Finalize completed `module-scope-02-preserve-user-installs` through `openspec archive`, promoting its accepted specification delta. - Remove stale active R08 ownership/follow-up guidance from R07 artifacts. - Correct the #414 and core #675 closure date to 2026-08-27. - Make in-memory import eviction, or an equivalent before-import guarantee, mandatory while preserving user-scoped files. - Define the required core #251 `verified-install-result-v1` contract and fail-closed adapter consumption before implementation. ## Scope - OpenSpec Markdown and Requirements evidence metadata only. - Source review: #455. - Delivery PRs retained: #453 and #454. - Issue references retained without changing their state: #431, #432, #433, #434, and #452. ## Verification - `openspec archive -y module-scope-02-preserve-user-installs`: passed; canonical `agent-governance-loading` specification updated. - `openspec validate --all --strict`: 81 passed, 0 failed. - Complete staged `./scripts/pre-commit-quality-checks.sh all`: passed. - Requirements evidence gate: passed. - YAML validation, formatting, import-boundary, command overview/contract, documentation-accountability, and docs-site checks: passed. - Signed Conventional Commit and all commit hooks: passed. - `git diff --check`: passed. ## Rollback Revert this single PR commit. That restores the previous planning locations and wording; no runtime or immutable release artifact rollback is required.
## Summary Promote the current protected dev release train to protected main. This promotion contains the reviewed changes merged through: - #454: preserve user-scoped module installs when a repository-local module shadows them - #453: planning-only seal-bound development assurance and OpenSpec dependency updates No implementation commits are introduced solely for this promotion PR. ## Issue linkage Closes #452. Planning references only: #431, #432, #433, and #434. This promotion does not close or mark implementation complete for those issues. Paired module-scope work: nold-ai/specfact-cli#699 and nold-ai/specfact-cli#700. ## Promotion boundary - Source: protected dev at 6350a0b - Target: protected main - Delivery PRs: #453 and #454 - Runtime impact: src/specfact_cli_modules/dev_bootstrap.py - Planning impact: OpenSpec assurance, dependency order, and superseded R08 records - Package manifests and registry artifacts: unchanged ## Verification evidence ### #454 implementation fix - Focused regression suite: 13 passed - Contract suite: 28 passed - Changed-scope SpecFact review: PASS with zero worktree findings - Protected quality and minimum-core matrices passed on Python 3.11, 3.12, and 3.13 - Requirements Evidence, signature verification, documentation review, static analysis, and security checks passed ### #453 planning changes - openspec validate --all --strict: 82 passed, 0 failed - Complete staged pre-commit pipeline passed - git diff --check passed - No canonical specification under openspec/specs changed ## Scope and release integrity - [x] User-scoped module guidance and regression tests - [x] OpenSpec planning and roadmap updates - [x] Superseded R08 plan preserved as non-authoritative history - [x] No package manifest, signed payload, registry, or release-version change - [ ] Exact-head promotion checks and required review must pass before merge ## Rollout and rollback Merge only after protected exact-head checks and review gates pass. Use a merge commit so the reviewed dev history remains intact. If the promotion causes a regression, revert the promotion merge on main. This PR does not publish immutable module artifacts or require a registry rollback.
Summary
Closes #452.
Paired with nold-ai/specfact-cli#699.
Verification
The paired core PR updates runtime discovery and module doctor wording. This PR can merge independently, but both should ship in the same later patch window.