feat: dep-security-cleanup (license gate, pycg, commentjson, review env)#507
feat: dep-security-cleanup (license gate, pycg, commentjson, review env)#507
Conversation
Proposal, design, specs (call-graph-analysis, dependency-resolution, dep-license-gate), and tasks for removing GPL/wrong deps and introducing proactive license + CVE gates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements openspec/dep-security-cleanup: pip-licenses-style compliance script, call graph via pycg, commentjson for VS Code JSONC, optional-deps hygiene, subprocess-only SPECFACT_MODULES_REPO for pre-commit code review, docs and CI. Made-with: Cursor
|
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:
📝 WalkthroughWalkthroughReworks dependency and security hygiene: replaces GPL/wrong analysis tools with MIT alternatives, adds license and CVE gates and allowlists, migrates call‑graph tooling and JSONC handling, centralizes module‑verification policy via a sourced script, updates CI/workflows/pre‑commit hooks, adds tests/docs, and bumps version to 0.46.3. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / PR
participant GH as GitHub Actions Orchestrator
participant Changes as changes job
participant LicenseJob as license-check
participant SecurityJob as security-audit
participant LicenseScript as scripts/check_license_compliance.py
participant AuditScript as scripts/security_audit_gate.py
participant PipLicenses as pip-licenses subprocess
participant PipAudit as pip-audit subprocess
participant MergeGate as Merge Gate
Dev->>GH: open/update PR
GH->>Changes: run change-detection
Changes-->>GH: outputs (code_changed, pyproject_changed, version_sources_changed)
GH->>LicenseJob: start (if code_changed && pyproject_changed && !skip)
GH->>SecurityJob: start (if code_changed && !skip)
LicenseJob->>LicenseScript: run license compliance
LicenseScript->>PipLicenses: python -m pip_licenses --format=json
PipLicenses-->>LicenseScript: installed packages JSON
LicenseScript->>LicenseScript: check allowlist + module manifests
LicenseScript-->>LicenseJob: exit code / summary
SecurityJob->>AuditScript: run security audit
AuditScript->>PipAudit: python -m pip_audit -f json -S
PipAudit-->>AuditScript: vulnerabilities JSON
AuditScript->>AuditScript: compute max CVSS, mark FAIL if >=7.0
AuditScript-->>SecurityJob: exit code / summary
LicenseJob-->>MergeGate: status
SecurityJob-->>MergeGate: status
MergeGate-->>GH: block/allow merge based on statuses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
djm81
added
enhancement
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…RDER row Link PR #507 in proposal Source Tracking; register dep-security-cleanup under openspec/CHANGE_ORDER.md (deps module). Made-with: Cursor
Register tracking issue in proposal Source Tracking and CHANGE_ORDER. Made-with: Cursor
![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: 34e3be76ec
ℹ️ 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: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-orchestrator.yml:
- Around line 573-600: The license-check job currently gates on
needs.changes.outputs.code_changed which triggers on any code change; update the
job condition (the if: on the license-check job) to require the PR actually
modified pyproject.toml by checking a dedicated output such as
needs.changes.outputs.pyproject_changed == 'true' (or add a paths-filter step
that sets that output) so the license-check job only runs when pyproject.toml
changes; modify the changes job or add a paths-filter action to emit
pyproject_changed and then change the license-check if: to combine both checks
(e.g., needs.changes.outputs.code_changed == 'true' &&
needs.changes.outputs.pyproject_changed == 'true').
In `@docs/installation/enhanced-analysis-dependencies.md`:
- Around line 117-120: The pip install commands for the packages pycg and
graphviz use unquoted version specifiers (pip install pycg>=0.0.7 and pip
install graphviz>=0.20.1) which cause shell redirection; update these lines to
quote the specifiers so the shell passes the version constraints to pip (use
"pycg>=0.0.7" and "graphviz>=0.20.1" for the pip install commands).
In `@openspec/changes/dep-security-cleanup/design.md`:
- Around line 68-69: The design note (Decision 4) in
openspec/changes/dep-security-cleanup/design.md is out of date: it references
keeping check_cli_tool_available("syft") in
src/specfact_cli/utils/optional_deps.py while the implementation/tests now use
pycg, bandit, and graphviz; update that section to reflect current behavior by
removing or revising the syft-specific text and explicitly documenting the
actual optional checks (check_cli_tool_available calls for pycg, bandit,
graphviz or whichever checks exist in optional_deps.py), mention that the PyPI
syft package was removed from pyproject.toml if applicable, and ensure Decision
4 matches the implemented symbols (optional_deps.py and
check_cli_tool_available) so the spec and code are consistent.
In `@openspec/changes/dep-security-cleanup/proposal.md`:
- Around line 77-79: The three checklist bullets are repetitive because they all
start with "Update/add tests for"; change them to varied lead verbs to improve
scanability—for example reference the target modules by name (graph_analyzer.py,
project_artifact_write.py, optional_deps.py) and rewrite the bullets to use
different verbs like "Add/update tests for graph_analyzer.py (mock pycg
subprocess, validate JSON parse path)", "Verify project_artifact_write.py
handles JSONC read via commentjson and JSON write via stdlib", and "Adjust
optional_deps.py tests to reflect removed/renamed checks" so each item reads
distinctly while preserving the same test requirements.
In `@openspec/changes/dep-security-cleanup/specs/call-graph-analysis/spec.md`:
- Around line 37-44: Update the spec to match the implemented API: replace
references to check_optional_analysis_deps() with
check_enhanced_analysis_dependencies(), state that the check uses
check_cli_tool_available("pycg"), and change the expected tuple shape to (bool,
error_message | None) (where True means available and the second element is
None, and False means unavailable with an error_message); also document that
"pycg" is the active call-graph tool.
- Around line 13-33: The spec expects call-graph mappings as {callee: [callers]}
but the implementation currently returns canonical {caller: [callees]} due to
the inversion in _parse_pycg_json; update _parse_pycg_json (or post-process its
result in extract_call_graph) so it inverts the mapping back to callee→list of
callers, ensure the returned type is dict[str, list[str]], and store that
mapping into self.call_graphs keyed by the relative file path inside
extract_call_graph (keep function names: _parse_pycg_json and extract_call_graph
and attribute self.call_graphs).
In `@openspec/changes/dep-security-cleanup/specs/dep-license-gate/spec.md`:
- Around line 45-49: The script currently returns an empty allowlist on
missing/malformed files; update scripts/check_license_compliance.py so loading
the allowlist fails closed: in the allowlist loader (e.g., function
load_allowlist / read_allowlist or the top-level file-read logic) detect missing
file or YAML parse errors and either raise an exception or call sys.exit(1)
after emitting a clear processLogger/print error; additionally validate each
allowlist entry contains package, license, and reason and fail (raise/exit
non-zero) if validation fails instead of returning an empty list.
In `@openspec/changes/dep-security-cleanup/tasks.md`:
- Around line 81-88: The checklist in
openspec/changes/dep-security-cleanup/tasks.md shows tasks 10.1–10.8 unchecked;
run the required gates locally (hatch test --cover -v, hatch run license-check,
hatch run security-audit, hatch run bandit-scan, hatch run format and hatch run
type-check, and specfact code review run --json --out
.specfact/code-review.json), fix any failures, record the passing test output
into openspec/changes/dep-security-cleanup/TDD_EVIDENCE.md, mark the
corresponding checkboxes 10.1–10.7 as done, and commit with the required message
format `feat(deps): remove GPL/wrong packages, add license-gate and
security-audit (#<issue>)` for 10.8; optionally add a small shell script that
runs these commands in sequence to act as the pre-merge checklist.
In `@pyproject.toml`:
- Around line 214-216: Replace the direct pip-audit invocation with a
thresholding wrapper: create scripts/security_audit_gate.py that runs "pip-audit
--format json", parses the JSON vulnerabilities array, extracts CVSS scores
(fallback to 0 if missing), prints each finding (use "WARNING" for CVSS < 7.0
and "FAIL" for CVSS ≥ 7.0), and exit with code 1 only if max(CVSS) ≥ 7.0; then
update the pyproject.toml "security-audit" script entry to call this wrapper
instead of "pip-audit --desc --strict".
In `@scripts/check_license_compliance.py`:
- Around line 167-170: The manifest-scan is non-enforcing because
static_license_map defaults to an empty dict and main() never populates it,
causing unknown-license warnings to be ignored; change the initialization in the
code that sets static_license_map and packages_dir so that when
static_license_map is None the script loads the canonical license mapping (e.g.,
from the repo's dep-license-gate spec or a shipped YAML/JSON mapping) and ensure
main() calls that loader before scanning manifests; specifically update the
logic around static_license_map and the entrypoint main() so module-package.yaml
files under packages_dir are checked against a populated static_license_map and
the tool fails the gate on unknown or disallowed licenses per
specs/dep-license-gate/spec.md.
- Around line 50-56: The _load_allowlist function currently returns an empty
dict when license_allowlist.yaml is missing or malformed, which silently
bypasses the gate; change _load_allowlist to raise a clear error (e.g., raise
RuntimeError or SystemExit with a descriptive message) when the allowlist file
does not exist and also catch YAML parsing errors (from yaml.safe_load) and
raise a similar descriptive error referencing "license_allowlist.yaml"; update
the code paths around _load_allowlist (the file existence check and the YAML
load/parse block previously around lines 58-60) so missing or invalid YAML fails
fast with a clear message instead of returning {}.
- Around line 80-85: The _is_gpl function currently uses substring checks that
match LGPL (e.g., "GPLv3" in "LGPLv3"); update _is_gpl(license_expr: str) to
normalize the input (e.g., upper()/strip()) and use a word-boundary regex to
match only GPL/AGPL family tokens (AGPL-3.0, GPL-2.0, GPL-3.0, GPLV2, GPLV3,
AGPLV3) while explicitly excluding LGPL variants (e.g., ensure "LGPL" does not
match) and fall back to checking _GPL_EXPRESSIONS; reuse the project's re usage
pattern and make the match case-insensitive via normalization or regex flags.
In `@scripts/license_allowlist.yaml`:
- Line 13: Update the stale allowlist guidance in scripts/license_allowlist.yaml
by replacing the incorrect reference "agent-rules/50-dependency-hygiene.md" with
the canonical file name "agent-rules/55-dependency-hygiene.md" so the Phase 2
entry correctly points to the new dependency hygiene rule file.
In `@src/specfact_cli/analyzers/graph_analyzer.py`:
- Around line 75-94: The pycg invocation in the method that runs subprocess (the
block creating json_path and calling subprocess.run) is missing the required
--package flag and wrong arg order; change the command args to include
"--package", str(self.repo_path) before the file path and use "--output",
str(json_path) (so the list becomes ["pycg", "--package", str(self.repo_path),
str(file_path), "--output", str(json_path)]), keep the existing
timeout/stderr/text handling and the finally cleanup of json_path, and update
the unit test for extract_call_graph (or the test that mocks subprocess.run) to
assert the subprocess.run was invoked with the new command list including
"--package" and the repo_path value.
In `@tests/unit/analyzers/test_graph_analyzer.py`:
- Around line 206-207: Remove the stale comment above the call to
analyzer._parse_pycg_json in the test (the line reading "This method doesn't
exist yet — will raise AttributeError until implemented") since _parse_pycg_json
is now implemented in graph_analyzer.py; keep the call to result =
analyzer._parse_pycg_json(json_path) unchanged and ensure no other misleading
comments remain near that invocation.
In `@tests/unit/scripts/test_check_license_compliance.py`:
- Around line 1-4: The module docstring in
tests/unit/scripts/test_check_license_compliance.py contains a stale note ("All
tests here are FAILING until scripts/check_license_compliance.py is
created.")—remove or update that sentence so the docstring no longer claims
tests are failing; edit the module-level docstring (the triple-quoted string at
top of the file) to simply describe the tests for check_license_compliance
without the misleading “FAILING until created” note.
In `@tests/unit/utils/test_optional_deps.py`:
- Around line 14-16: Remove the stale migration-state comment in
tests/unit/utils/test_optional_deps.py that reads "These tests will FAIL until
optional_deps.py is updated to use pycg." — either delete that comment line or
replace it with a concise, accurate note reflecting the current state (or no
comment at all); ensure only high-value comments remain and do not alter any
test logic or names in the file.
In `@tests/unit/utils/test_project_artifact_write.py`:
- Line 207: The test uses a brittle hardcoded traversal when setting
_MODULE_SOURCE with Path(__file__).parents[3]; replace this with a resilient
project-root discovery or fixture: implement a helper that resolves
Path(__file__).resolve() and walks parents until it finds a known sentinel
(e.g., pyproject.toml, setup.cfg, or the "src/specfact_cli" package) and derive
the path to utils/project_artifact_write.py from that root, or inject the
correct module path via a pytest fixture (e.g., project_root or
importlib_resources) so the test no longer depends on a fixed number of parent
hops.
- Around line 235-260: The test docstring claims JSONC contains both line (//)
and block (/* */) comments but the fixture only uses line comments; update the
test test_merge_vscode_settings_handles_line_and_block_comments_in_jsonc by
either changing the docstring to mention only line comments or (preferred)
modify the settings fixture written to settings_path to include a /* block
comment */ example alongside the existing // comments so it truly exercises
merge_vscode_settings_prompt_recommendations handling of both comment styles.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e0658af2-239e-480b-aff4-6e91c4d2c382
📒 Files selected for processing (26)
.github/workflows/pr-orchestrator.ymlSECURITY.mddocs/agent-rules/55-dependency-hygiene.mddocs/agent-rules/INDEX.mddocs/installation/enhanced-analysis-dependencies.mdopenspec/changes/dep-security-cleanup/.openspec.yamlopenspec/changes/dep-security-cleanup/TDD_EVIDENCE.mdopenspec/changes/dep-security-cleanup/design.mdopenspec/changes/dep-security-cleanup/proposal.mdopenspec/changes/dep-security-cleanup/specs/call-graph-analysis/spec.mdopenspec/changes/dep-security-cleanup/specs/dep-license-gate/spec.mdopenspec/changes/dep-security-cleanup/specs/dependency-resolution/spec.mdopenspec/changes/dep-security-cleanup/tasks.mdpyproject.tomlscripts/check_license_compliance.pyscripts/license_allowlist.yamlscripts/pre_commit_code_review.pysrc/specfact_cli/analyzers/code_analyzer.pysrc/specfact_cli/analyzers/graph_analyzer.pysrc/specfact_cli/utils/optional_deps.pysrc/specfact_cli/utils/project_artifact_write.pytests/unit/analyzers/test_graph_analyzer.pytests/unit/scripts/test_check_license_compliance.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/utils/test_optional_deps.pytests/unit/utils/test_project_artifact_write.py
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
openspec/changes/dep-security-cleanup/proposal.md (1)
77-79: 🧹 Nitpick | 🔵 TrivialReduce repetitive bullet lead-ins in the test list.
The three test bullets still begin with nearly identical phrasing ("Update/add tests for... Update/add tests for... Update"). Vary the lead verbs to improve scannability: "Verify graph_analyzer.py mocks pycg subprocess and validates JSON parse path," "Confirm project_artifact_write.py handles JSONC read via commentjson and strict JSON write," "Adjust optional_deps.py tests for removed/renamed checks."
As per coding guidelines,
**/*.{md,mdc}should use varied language to avoid markdown/style quality issues (LanguageTool ENGLISH_WORD_REPEAT_BEGINNING_RULE).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/dep-security-cleanup/proposal.md` around lines 77 - 79, The three bullets in openspec/changes/dep-security-cleanup/proposal.md are repetitive; change their lead verbs to improve scannability by using varied phrasing—e.g., replace "Update/add tests for `graph_analyzer.py`..." with "Verify `graph_analyzer.py` mocks pycg subprocess and validates JSON parse path," change "Update/add tests for `project_artifact_write.py`..." to "Confirm `project_artifact_write.py` handles JSONC read via commentjson and strict JSON write," and change "Update `optional_deps.py` tests..." to "Adjust `optional_deps.py` tests for removed/renamed checks"; ensure each bullet starts with a different verb and retains the same technical details.
🤖 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 213-217: The folder name and CHANGE_ORDER.md entry violate the
documented pattern: rename the folder dep-security-cleanup to
deps-01-dep-security-cleanup and update the CHANGE_ORDER.md table row to use |
deps-01-dep-security-cleanup |; then update all internal references and links
inside the renamed folder (TDD_EVIDENCE.md, design.md, tasks.md) to point to the
new folder name (and fix any relative paths or Markdown links) so everything
consistently uses the <module>-<NN>-<suffix> convention.
In `@openspec/changes/dep-security-cleanup/proposal.md`:
- Around line 46-82: Update the Impact section of
openspec/changes/dep-security-cleanup/proposal.md to list all implementation
surfaces omitted: add bullets/subsections for Documentation updates
(SECURITY.md, install docs replacing pyan3/bearer/syft with pycg/bandit), CI
integration (.github/workflows license-check and security-audit gates), Agent
rules (docs/agent-rules updates to enforce license gates), and Scripts
(scripts/check_license_compliance.py and scripts/license_allowlist.yaml).
Mention these exact artifacts (SECURITY.md, .github/workflows, docs/agent-rules,
scripts/check_license_compliance.py, scripts/license_allowlist.yaml) so the spec
fully enumerates required changes and aligns with the PR objectives.
- Around line 1-3: Add an explicit note about required cross-repository
coordination: state that nold-ai/specfact-cli-modules must be updated in
lockstep to remove or replace pyan3 call sites in
packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py and
import_cmd/commands.py, update or remove the json5>=0.9.28 dependency from its
pyproject.toml, and align all packages/*/module-package.yaml manifests so they
pass the new scripts/check_license_compliance.py checks; call out these specific
files and artifacts so reviewers know downstream edits are required before
merging.
---
Duplicate comments:
In `@openspec/changes/dep-security-cleanup/proposal.md`:
- Around line 77-79: The three bullets in
openspec/changes/dep-security-cleanup/proposal.md are repetitive; change their
lead verbs to improve scannability by using varied phrasing—e.g., replace
"Update/add tests for `graph_analyzer.py`..." with "Verify `graph_analyzer.py`
mocks pycg subprocess and validates JSON parse path," change "Update/add tests
for `project_artifact_write.py`..." to "Confirm `project_artifact_write.py`
handles JSONC read via commentjson and strict JSON write," and change "Update
`optional_deps.py` tests..." to "Adjust `optional_deps.py` tests for
removed/renamed checks"; ensure each bullet starts with a different verb and
retains the same technical details.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6643cbba-d47c-4b10-9aa0-47630f3ca355
📒 Files selected for processing (2)
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/dep-security-cleanup/proposal.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.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/dep-security-cleanup/proposal.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/dep-security-cleanup/proposal.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/dep-security-cleanup/proposal.md
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to openspec/changes/**/*.md : For `/opsx:archive` (Archive change): Include module signing and cleanup in final tasks. Agents MUST run `openspec archive <change-id>` from repo root (no manual `mv` under `openspec/changes/archive/`)
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to openspec/changes/**/*.md : For `/opsx:archive` (Archive change): Include module signing and cleanup in final tasks. Agents MUST run `openspec archive <change-id>` from repo root (no manual `mv` under `openspec/changes/archive/`)
Applied to files:
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-04-10T22:41:54.419Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-04-10T22:41:54.419Z
Learning: Before implementing code changes, run `openspec validate <change-id> --strict` to validate the OpenSpec change and optionally run `/wf-validate-change <change-id>` for breaking-change and dependency analysis; do not proceed to implementation until validation passes or user explicitly overrides
Applied to files:
openspec/CHANGE_ORDER.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: For OpenSpec changes when a sibling `specfact-cli-internal/` checkout exists, read internal wiki guidance in `docs/agent-rules/40-openspec-and-tdd.md` and consult `wiki/hot.md`, `wiki/graph.md`, and relevant `wiki/concepts/*.md` files
Applied to files:
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: For `/opsx:ff` (Fast-Forward Change Creation): OPSX provides change scaffolding and artifact templates. AGENTS.md requires explicitly adding: Worktree creation task as Step 1 in tasks.md (not just 'create branch'), TDD_EVIDENCE.md tracking task in section 2 (Tests), Documentation research task per `openspec/config.yaml`, Module signing quality gate if applicable, Worktree cleanup steps in final section
Applied to files:
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to docs/**/*.md : Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Applied to files:
openspec/CHANGE_ORDER.mdopenspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: All development work MUST use git worktrees per AGENTS.md Git Worktree Policy. Never create branches with `git checkout -b` in the primary checkout. Create worktree from origin/dev: `git worktree add ../specfact-cli-worktrees/<type>/<slug> -b <branch-name> origin/dev` where allowed types are: `feature/`, `bugfix/`, `hotfix/`, `chore/`. Forbidden in worktrees: `dev`, `main`. After creating worktree: `cd ../specfact-cli-worktrees/<type>/<slug>`. Bootstrap Hatch in worktree: `hatch env create`. Run pre-flight checks: `hatch run smart-test-status` and `hatch run contract-test-status`. Do all implementation work from the worktree, never from primary checkout. After PR merge: cleanup with `git worktree remove`, `git branch -d`, `git worktree prune`
Applied to files:
openspec/CHANGE_ORDER.md
📚 Learning: 2026-04-10T22:41:54.419Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-04-10T22:41:54.419Z
Learning: Applies to {src,tools}/**/*.{js,ts,jsx,tsx,py},**/*.test.{js,ts,jsx,tsx,py},**/*.spec.{js,ts,jsx,tsx,py} : Do not add, modify, or delete any application code in src/, tools/, tests, or significant docs until an OpenSpec change (new or delta) is created and validated, unless the user explicitly opts out with 'skip openspec', 'direct implementation', 'simple fix', or 'just fix it'
Applied to files:
openspec/CHANGE_ORDER.md
📚 Learning: 2026-04-10T22:41:54.419Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-04-10T22:41:54.419Z
Learning: Applies to openspec/**/{proposal.md,tasks.md,design.md,spec.md} : Apply `openspec/config.yaml` project context and per-artifact rules (for proposal, specs, design, tasks) when creating or updating any OpenSpec change artifact in the specfact-cli codebase
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-04-10T22:41:54.419Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-04-10T22:41:54.419Z
Learning: For changes created from plan documents, use `/specfact-cli/wf-create-change-from-plan <plan-path>` workflow to create OpenSpec proposals; the workflow includes plan selection, alignment review, integrity re-check, proposal creation, improvement, optional GitHub issue creation, and completion steps
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-04-10T22:41:54.419Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-04-10T22:41:54.419Z
Learning: Use `/opsx-explore` to research the codebase for conflicts and dependencies, then use `/opsx-new` or `/opsx:ff` to create the change proposal with unique verb-led change-id, scaffolded proposal.md, tasks.md, optional design.md, and spec deltas in changes/<id>/specs/<capability>/spec.md; validate with `openspec validate <id> --strict`
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-03-25T21:32:08.987Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/markdown-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:08.987Z
Learning: Applies to **/*.{md,mdc} : Use consistent list markers, preferring dashes (-) for unordered lists (MD004: List Style)
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-03-25T21:32:08.987Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/markdown-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:08.987Z
Learning: Applies to **/*.{md,mdc} : Use exactly one space after the list marker (e.g., -, *, +, 1.) (MD030: Spaces After List Markers)
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
📚 Learning: 2026-03-25T21:32:08.987Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/markdown-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:08.987Z
Learning: Applies to **/*.{md,mdc} : Lists should be surrounded by blank lines (MD032: Lists)
Applied to files:
openspec/changes/dep-security-cleanup/proposal.md
🪛 LanguageTool
openspec/changes/dep-security-cleanup/proposal.md
[style] ~79-~79: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... commentjson, JSON write via stdlib). - Update optional_deps.py tests to reflect rem...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Module manifests (targets for manifest-scanning):
- packages/*/module-package.yaml present (e.g., packages/specfact-codebase/module-package.yaml, packages/specfact-code-review/module-package.yaml, packages/specfact-backlog/module-package.yaml). These are the exact files scanned by scripts/check_license_compliance.py for module-manifest license violations. [::nold-ai/specfact-cli-modules::packages/specfact-codebase/module-package.yaml], [::nold-ai/specfact-cli-modules::packages/specfact-code-review/module-package.yaml], [::nold-ai/specfact-cli-modules::packages/specfact-backlog/module-package.yaml]
-
Consumers of call-graph tooling (pyan3 → pycg migration impact):
- packages/specfact-project/src/specfact_project/import_cmd/commands.py references/checks for pyan3 (calls check_cli_tool_available("pyan3") and emits install hints and behavior tied to pyan3). These call sites will need to align with the PR’s change to probe/use "pycg". [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/import_cmd/commands.py]
- packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py checks for pyan3 and invokes it (
["pyan3", ... "--dot", ...]). This is a direct consumer of the old CLI behavior the PR replaces. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py] - packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py contains pyan3 availability checks and messaging for dependency-graph feature gating. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py]
-
JSON/JSONC parsing dependency mentions:
- pyproject.toml (repo-level) still lists json5 in dependencies: "json5>=0.9.28". The PR replaces json5 with commentjson; confirm modules repo consumers/readers expecting json5/JSON5 semantics are compatible. [::nold-ai/specfact-cli-modules::pyproject.toml]
- docs mention pyan3/syft/bearer as optional enhanced-analysis tools (docs/getting-started/installation.md and README). These docs will be out-of-date relative to the PR's migration to pycg/bandit. [::nold-ai/specfact-cli-modules::docs/getting-started/installation.md], [::nold-ai/specfact-cli-modules::docs/getting-started/README.md]
-
Pre-commit / review env behavior:
- scripts/pre_commit_code_review.py exists in this repo and already sets SPECFACT_MODULES_REPO / SPECFACT_CLI_MODULES_REPO and adjusts PYTHONPATH for nested review subprocesses by directly mutating env copy in _run_review_subprocess. The PR in the other repo modifies pre_commit_code_review.py to add discover_specfact_modules_repo() and build_review_subprocess_env() and to change _run_review_subprocess signature to accept an env param; reviewers should verify the two implementations align when the repo acting as the "modules" sibling is discovered by the altered behavior. Potential for duplication/interaction when the altered specfact-cli repo tries to discover/inject SPECFACT_MODULES_REPO pointing at this checkout — confirm no unexpected env precedence or double-injection. [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py]
-
Search for new tooling/scripts introduced by PR:
- No occurrences of pycg, commentjson, pip-licenses, pip-audit, check_license_compliance.py, or license_allowlist.yaml were found in this repo (so this repo does not already include the new gate scripts or replacements). The PR will introduce these artifacts into the other repo and run scans against module manifests here. [::nold-ai/specfact-cli-modules::]
Impact summary / reviewers’ checklist (based on observed cross-repo references):
- Confirm scripts/check_license_compliance.py correctly parses the manifest dependency name formats used in packages/*/module-package.yaml in this repo (name/version specifier extraction must match this repo’s manifest conventions).
- Update or coordinate callers in packages/specfact-project that check for and invoke pyan3 to either probe/use pycg or be left as-is only if maintained separately; otherwise those callers will continue to report pyan3 missing or attempt to run pyan3 (breaking call-graph behavior).
- Ensure documentation in this repo (installation docs) and any user-facing messages are updated to reflect pycg/bandit/commentjson replacements to avoid confusing install hints.
- Validate that the pre-commit/code-review environment injection logic between the two repos does not conflict (both set SPECFACT_MODULES_REPO/PATH); confirm which implementation will be authoritative when the changes are merged across repos.
🔇 Additional comments (1)
openspec/changes/dep-security-cleanup/proposal.md (1)
37-39: Verify functional equivalence claim for call-graph extraction.The proposal states the pycg-based pipeline is "functionally equivalent to the previous DOT-based pipeline," but the output format fundamentally differs (JSON
{callee: [caller]}vs. DOT graph edges). While_parse_pycg_jsoninverts to the same{caller: [callee]}dictionary structure, subtle behavioral differences may exist in error handling, timeout behavior, or graph completeness.Confirm that the graph_analyzer test suite validates identical behavior for both missing dependencies (graceful degradation to
{}) and successful extraction (same call-graph structure). The TDD_EVIDENCE.md should document this equivalence proof.As per coding guidelines,
openspec/**/{proposal.md,tasks.md,design.md,spec.md}must validate withopenspec validate <change-id> --strictand preserve specification source-of-truth alignment.
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/specfact_cli/modules/module_registry/src/commands.py (1)
524-548: 🛠️ Refactor suggestion | 🟠 MajorUnify CLI uninstall through one deletion path.
This fixes the implicit marketplace path, but
--scope user|projectstill bypassesuninstall_module()and deletes with rawshutil.rmtree(). That leaves two policy surfaces for the same destructive operation; the next audit/cleanup change will drift immediately. Prefer a shared uninstall helper for both branches and pass the trusted CLI confirmation there.As per coding guidelines, "DRY — single source of truth; eliminate copy-paste duplication."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/modules/module_registry/src/commands.py` around lines 524 - 548, The uninstall logic should be unified so both the marketplace path and the --scope (user|project) paths call the same helper; update _uninstall_marketplace_default and any command handler that currently uses raw shutil.rmtree to call uninstall_module(normalized, confirm_user_scope=True) (or extend uninstall_module to accept a flag for CLI-confirmation if needed) and remove the duplicate rmtree deletion code; ensure the trusted CLI confirmation is passed through to uninstall_module so all destructive removals go through the single uninstall implementation (referencing _uninstall_marketplace_default and uninstall_module to locate and change the duplicated branches).setup.py (1)
17-35:⚠️ Potential issue | 🟠 MajorUpdate setup.py dependency: replace
json5withcommentjsonto align with pyproject.toml and actual imports.The PR migrates from
json5tocommentjsonfor JSONC parsing, butsetup.pystill declaresjson5>=0.9.28whilepyproject.tomland actual code imports usecommentjson>=0.9.0. This violates dependency synchronization requirements and risks licensing/dependency drift, particularly with the new offline license compliance gate that scans module pip dependencies.Proposed patch
- "json5>=0.9.28", + "commentjson>=0.9.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.py` around lines 17 - 35, The setup.py install_requires still lists "json5>=0.9.28" while the project uses commentjson; update the dependency in the install_requires list by replacing the "json5" entry with the correct "commentjson" package and version (e.g., "commentjson>=0.9.0") so it matches pyproject.toml and the actual imports; ensure the change occurs within the install_requires array symbol in setup.py..github/workflows/sign-modules.yml (1)
117-133:⚠️ Potential issue | 🟠 Major
workflow_dispatchresign-all mode is blocked by the verify gate.
VERIFY_MODULES_PRstill carries version-bump enforcement on non-main flows, so a manual dispatch withresign_all_manifests=truewill fail verification when manifests intentionally match the base branch. That makes the documented recovery path for unsigned-but-unchanged manifests unreachable.Use a dispatch-specific verify bundle without version-bump enforcement, or skip the base-version check in
resign_all_manifestsmode before the signing job runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sign-modules.yml around lines 117 - 133, The workflow_dispatch path currently calls python scripts/verify-modules-signature.py with VERIFY_MODULES_PR which enforces a version-bump via the --version-check-base flag and blocks resign_all_manifests flows; modify the dispatch branch so that when the workflow input resign_all_manifests=true (or similar dispatch flag) you either (A) invoke a dispatch-specific verify bundle/variable that omits version-bump enforcement, or (B) skip passing --version-check-base to the python script for that run; update the conditional around BASE_REF/VERIFY_MODULES_PR in the workflow (and any check in scripts/module-verify-policy.sh or verify-modules-signature.py that reads VERIFY_MODULES_PR) to detect the resign_all_manifests mode and bypass the base-version check before the signing job runs.tests/unit/analyzers/test_graph_analyzer.py (2)
100-108:⚠️ Potential issue | 🟠 MajorThe timeout test can pass without exercising the timeout path.
Right now
extract_call_graph()may return beforesubprocess.run()ifpycgis unavailable, and thecontextlib.suppress(Exception)+if mock_run.calledguard turns that into a silent pass. This makes the test environment-coupled and non-assertive.Proposed fix
- with patch("specfact_cli.analyzers.graph_analyzer.subprocess.run") as mock_run: + with ( + patch("specfact_cli.analyzers.graph_analyzer.check_cli_tool_available", return_value=(True, None)), + patch("specfact_cli.analyzers.graph_analyzer.subprocess.run") as mock_run, + ): mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") - with contextlib.suppress(Exception): # May fail if pycg not available - analyzer.extract_call_graph(file_path) - - # Verify timeout was set to 15 seconds - if mock_run.called: - call_kwargs = mock_run.call_args[1] - assert call_kwargs.get("timeout") == 15 + analyzer.extract_call_graph(file_path) + + call_kwargs = mock_run.call_args[1] + assert call_kwargs.get("timeout") == 15As per coding guidelines, "
tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/analyzers/test_graph_analyzer.py` around lines 100 - 108, The test for extract_call_graph is flaky because it may return early when pycg is absent; update the test to force the code path that calls subprocess.run by mocking the availability check (or the import) so extract_call_graph always proceeds, remove the contextlib.suppress(Exception) and the conditional if mock_run.called guard, and assert unconditionally that the subprocess.run call_args include timeout==15; reference the extract_call_graph function and the patched specfact_cli.analyzers.graph_analyzer.subprocess.run used in the test to locate where to change the setup so the timeout path is always exercised.
68-91:⚠️ Potential issue | 🟡 MinorThis test never proves the call-graph hook ran.
len(graph.nodes()) == 5is unaffected by whetherextract_call_graph()executes, so this stays green even if the hook is removed entirely. Please force the availability check on and assert an observable hook effect (subprocess.run()called,call_graphspopulated, etc.).As per coding guidelines, "
tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/analyzers/test_graph_analyzer.py` around lines 68 - 91, The test test_build_dependency_graph_parallel_call_graphs should assert the call-graph hook actually ran: force the "pycg available" path and verify side-effects by mocking subprocess.run and checking it was called and that GraphAnalyzer.extract_call_graph (or the GraphAnalyzer.call_graphs attribute) was populated; specifically, in the test use GraphAnalyzer(tmp_path) then enable the availability check (e.g., set the flag or patch the availability method on GraphAnalyzer), patch specfact_cli.analyzers.graph_analyzer.subprocess.run to return a successful MagicMock, call analyzer.build_dependency_graph(files), assert subprocess.run.assert_called() (or check call count) and assert that analyzer.call_graphs (or whatever container name used inside GraphAnalyzer) contains entries for the processed files rather than only asserting len(graph.nodes()).scripts/verify-modules-signature.py (1)
292-304:⚠️ Potential issue | 🟠 MajorSkip mode still rejects manifests before the skip path.
VERIFY_MODULES_PRis documented and consumed as the relaxed "version-bump only" mode, but this function still requiresintegrity.checksumand parses it beforeverify_checksum=Falsereturns. New or edited manifests that have not been re-signed yet will still fail in PR/pre-commit flows instead of deferring checksum/signature validation to the strict path.Proposed fix
data = cast(dict[str, Any], raw) - integrity_raw = data.get("integrity") - if not isinstance(integrity_raw, dict): - raise ValueError("missing integrity metadata") - integrity = cast(dict[str, Any], integrity_raw) - - checksum = str(integrity.get("checksum", "")).strip() - if not checksum: - raise ValueError("missing integrity.checksum") - algo, digest = _parse_checksum(checksum) if not verify_checksum: if require_signature: raise ValueError("require_signature is incompatible with verify_checksum=False") return + + integrity_raw = data.get("integrity") + if not isinstance(integrity_raw, dict): + raise ValueError("missing integrity metadata") + integrity = cast(dict[str, Any], integrity_raw) + + checksum = str(integrity.get("checksum", "")).strip() + if not checksum: + raise ValueError("missing integrity.checksum") + algo, digest = _parse_checksum(checksum)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/verify-modules-signature.py` around lines 292 - 304, The function currently reads and validates integrity_raw and checksum and calls _parse_checksum before honoring verify_checksum=False, causing "skip mode" to still reject unsigned manifests; change the control flow so verify_checksum is checked as early as possible (before accessing integrity_raw/checksum and calling _parse_checksum) and if verify_checksum is False return immediately (while still rejecting incompatible require_signature=True), or alternatively guard the checksum parsing and _parse_checksum call behind an if verify_checksum: block so manifests in relaxed/VERIFY_MODULES_PR mode do not require integrity.checksum.
♻️ Duplicate comments (5)
scripts/license_allowlist.yaml (1)
13-13:⚠️ Potential issue | 🟡 MinorPoint this to the canonical rules path.
agent-rules/55-dependency-hygiene.mdis not the repo location used elsewhere in this PR; readers needdocs/agent-rules/55-dependency-hygiene.md.Proposed fix
-# Phase 2 entries: track removal in SECURITY.md and agent-rules/55-dependency-hygiene.md. +# Phase 2 entries: track removal in SECURITY.md and docs/agent-rules/55-dependency-hygiene.md.Based on learnings, "Treat
docs/agent-rules/as the canonical location for repository governance rather than relying on inline reminders or other documentation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/license_allowlist.yaml` at line 13, Update the reference in scripts/license_allowlist.yaml that currently points to "agent-rules/55-dependency-hygiene.md" to the canonical repository path "docs/agent-rules/55-dependency-hygiene.md" so readers are directed to the correct docs location; locate the comment line containing "Phase 2 entries: track removal in SECURITY.md and agent-rules/55-dependency-hygiene.md" and replace the latter path with the docs/agent-rules version.tests/unit/utils/test_project_artifact_write.py (1)
239-253:⚠️ Potential issue | 🟡 MinorCover the
/* */JSONC path too.
merge_vscode_settings_prompt_recommendations()still advertises both line and block comments, but this new regression only exercises//. That leaves the block-comment read path unguarded after the parser migration.Proposed test adjustment
settings_path.write_text( """{ // line comment "python.defaultInterpreterPath": "/usr/bin/python3", - // another comment + /* block comment */ "chat": {"promptFilesRecommendations": []} } """, encoding="utf-8", )As per coding guidelines, "
tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/utils/test_project_artifact_write.py` around lines 239 - 253, Update the test to also exercise the block-comment JSONC path so merge_vscode_settings_prompt_recommendations() is covered for both comment styles: create a settings.json content that uses /* ... */ block comments (instead of or in addition to //) and write it to tmp_path/.vscode/settings.json, then call the same merge_vscode_settings_prompt_recommendations() flow and assert the same outcome as the existing test; ensure the test name reflects block-comment coverage (e.g., test_merge_vscode_settings_handles_block_comments_in_jsonc) and reference merge_vscode_settings_prompt_recommendations to locate the code under test.openspec/changes/dep-security-cleanup/proposal.md (2)
46-90:⚠️ Potential issue | 🟠 MajorAdd explicit cross-repository coordination requirements before merge readiness.
The proposal still doesn’t name the required lockstep updates in
nold-ai/specfact-cli-modules(remainingpyan3consumers,json5references, and manifest/license-gate alignment), which is a release-risk dependency for this change-set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/dep-security-cleanup/proposal.md` around lines 46 - 90, Add explicit cross-repository coordination notes: update the proposal's Impact/Source Tracking to enumerate required lockstep changes in nold-ai/specfact-cli-modules — specifically: replace remaining pyan3 consumers (call-graph invocation changes from "pyan3" → "pycg"), update any json5 references to commentjson/json in packaging and runtime (pyproject.toml extras and hatch-test envs, and project_artifact_write import/usage), and align manifest/license-gate entries (dependency pins/accepted exceptions such as pygments/semgrep) before merging; mention the exact files/entries to change (pyproject.toml extras and runtime deps, src/specfact_cli/utils/optional_deps.py checks for "pycg"/"bandit"/"graphviz", src/specfact_cli/analyzers/graph_analyzer.py behavior switch, and src/specfact_cli/utils/project_artifact_write.py json import/usage) and require an explicit PR/issue link from specfact-cli-modules confirming these updates.
46-79:⚠️ Potential issue | 🟡 MinorImpact section still omits several shipped surfaces.
Please include the concrete artifacts for CI workflow wiring, dependency-hygiene agent rules, and license-gate scripts so the proposal fully matches implemented scope.
As per coding guidelines:
openspec/**/*.mdmust act as source-of-truth and surface implementation drift/completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/dep-security-cleanup/proposal.md` around lines 46 - 79, The Impact section omits CI workflow wiring, dependency-hygiene agent rules, and license-gate scripts that were actually changed; update the proposal.md Impact paragraph to list these concrete artifacts and their scopes (e.g., CI workflow files added/modified for dependency checks and build/test, dependency-hygiene agent rule updates that enforce pycg/bandit/json changes, and license-gate script changes that affect runtime deps), and cross-reference the existing implementation targets (pyproject.toml, src/specfact_cli/utils/optional_deps.py, src/specfact_cli/analyzers/graph_analyzer.py, src/specfact_cli/utils/project_artifact_write.py) so the doc fully reflects CI, policy agent, and licensing surfaces touched by this change.openspec/changes/dep-security-cleanup/tasks.md (1)
81-88:⚠️ Potential issue | 🟠 MajorClose or explicitly defer the remaining blocking gates before merge.
license-check,specfact code review, and final commit-task entries are still unchecked, which leaves this OpenSpec task artifact in a “partially complete” state for a change presented as implemented. Please either complete these in this PR or move them to an explicit follow-up change with owner/date.As per coding guidelines:
openspec/**/*.mdis the specification source of truth and should surface implementation/gate completion accurately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/dep-security-cleanup/tasks.md` around lines 81 - 88, Checklist items 10.2, 10.6 and 10.8 are blocking but left unchecked; either complete the actions (run `hatch run license-check` and record its passing, run `specfact code review run --json --out .specfact/code-review.json` and resolve findings, then make the final commit with message `feat(deps): remove GPL/wrong packages, add license-gate and security-audit (#<issue>)`) and mark those boxes done, or explicitly defer them by creating a follow-up issue/PR with owner and target date and update items 10.2, 10.6 and 10.8 to a deferred state (e.g., “Deferred → #<follow-up> owner: `@user` date: YYYY-MM-DD”) so the checklist accurately reflects the remaining work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-orchestrator.yml:
- Around line 45-46: The workflow's path filter only includes 'pyproject.toml'
so changes that affect the license checker (like module manifests, the
allowlist, or the checker itself) can bypass the gate; update the
pr-orchestrator workflow path/include list used by the license gate to also
match modules/**/module-package.yaml,
src/specfact_cli/modules/**/module-package.yaml, scripts/license_allowlist.yaml
and the checker script (scripts/check_license_compliance.py) so that edits to
those files trigger the license check job; locate the block that defines the
'pyproject' entry in the job's paths filter and add these additional glob
patterns to it.
In @.pre-commit-config.yaml:
- Around line 24-29: The pre-commit hook "check-local-version-ahead-of-pypi"
runs the script scripts/check_local_version_ahead_of_pypi.py in the system
Python and must be changed to invoke that script via Hatch's managed
environment; update the hook entry so it runs the same script through "hatch
run" (preserving the SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK export and exec
behavior) so third-party imports like beartype and icontract load from the Hatch
environment instead of the system interpreter.
In `@CHANGELOG.md`:
- Around line 22-25: The three long bullets ("License / CVE hygiene", "Call
graphs", and "Pre-commit / CI") in the CHANGELOG.md exceed the 120-char markdown
line-length rule; wrap each bullet into multiple lines so no line is over 120
chars, breaking after logical separators (commas or semicolons), keep the
initial "- " bullet prefix only on the first line and indent continuation lines
by two spaces for readability, and reflow each bullet (e.g., split the License /
CVE hygiene sentence into separate lines for the hardened script description, CI
gating, and docs/openspec updates) so the markdown linter passes.
In `@docs/agent-rules/55-dependency-hygiene.md`:
- Around line 37-40: The rule text in docs/agent-rules/55-dependency-hygiene.md
is contradictory: the top-level bullet allows GPL/AGPL via a
scripts/license_allowlist.yaml module-manifest entry but later sections state
GPL/AGPL are never acceptable in module manifests; update the document to be
consistent by choosing one policy and making all references match — either (A)
explicitly allow GPL/AGPL only when a module-manifest-scoped entry exists in
scripts/license_allowlist.yaml and change the later “never acceptable” language
to reflect this exception, or (B) make GPL/AGPL categorically forbidden in
module manifests and remove/replace any allowance language in section 1; ensure
all symbols match (module-package.yaml, pip_dependencies, module-manifest,
scripts/license_allowlist.yaml) and mirror the same change in the related files
57-69.
In `@docs/installation/enhanced-analysis-dependencies.md`:
- Around line 61-63: Replace the redundant phrase "CLI interface" in the
migration note that mentions `pyan3` and `pycg` with either "CLI" or
"command-line interface" (e.g., change "The CLI interface changed from DOT
format to JSON" to "The CLI changed from DOT format to JSON" or "The
command-line interface changed from DOT format to JSON") so the wording is
concise and non-redundant.
In `@openspec/changes/dep-security-cleanup/design.md`:
- Line 114: The design doc references docs/agent-rules/50-dependency-hygiene.md
but the implemented file is docs/agent-rules/55-dependency-hygiene.md; update
the reference in openspec/changes/dep-security-cleanup/design.md to point to the
actual artifact name (docs/agent-rules/55-dependency-hygiene.md) or rename the
implemented file to 50-dependency-hygiene.md so names match, and ensure the
INDEX.md and any other agent-rules references are consistent with the chosen
filename so openspec/**/*.md accurately surfaces the drift.
In `@openspec/changes/dep-security-cleanup/specs/call-graph-analysis/spec.md`:
- Line 64: The phrase "CLI interface changes from `pyan3 <file> --dot` to `pycg
--package <repo> <file> --output out.json`" is redundant; update the sentence to
use either "CLI" or "command-line interface" (e.g., "CLI changes from `pyan3
<file> --dot` to `pycg --package <repo> <file> --output out.json`") so remove
the duplicated word "interface" while keeping the example commands and the rest
of the migration note intact.
In `@openspec/changes/dep-security-cleanup/TDD_EVIDENCE.md`:
- Around line 119-121: Replace the direct invocation "hatch run pytest
tests/unit/..." with the repo-standard test runner by using "hatch test --
tests/unit/analyzers/test_graph_analyzer.py
tests/unit/utils/test_optional_deps.py
tests/unit/utils/test_project_artifact_write.py
tests/unit/scripts/test_check_license_compliance.py
tests/unit/scripts/test_security_audit_gate.py -q" so that the evidence block
uses hatch test (with "--" to forward pytest args) instead of "hatch run
pytest".
In `@pyproject.toml`:
- Around line 149-153: Update the sibling repo specfact-cli-modules to replace
direct pyan3 usage with pycg across the three affected modules: in
packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py
replace the subprocess invocation list that calls ["pyan3", ... , "--dot", ...]
to invoke the pycg CLI with equivalent arguments (map pyan3 flags to pycg
flags), in packages/specfact-project/src/specfact_project/import_cmd/commands.py
change the installation check and user prompt that references "pyan3" to check
for "pycg" instead, and in
packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py update
the availability check and conditional graph-analysis gating from pyan3 to pycg;
ensure argument/flag mapping and error messages preserve the public API/UX
(prompts, feature gating) so call-graph analysis behaves identically after the
migration and submit a coordinated PR to that repo alongside this one.
In `@scripts/check_license_compliance.py`:
- Around line 120-137: The _run_pip_licenses function currently calls
subprocess.run(..., timeout=60) but doesn't catch subprocess.TimeoutExpired;
update _run_pip_licenses to wrap the subprocess.run call in a try/except that
catches subprocess.TimeoutExpired (and optionally subprocess.SubprocessError if
you want broader coverage), print a clear fail-closed error message including
the timeout context (similar style to existing error prints), flush output, and
return an empty string on exception; refer to the subprocess.run invocation and
the _run_pip_licenses function to locate where to add the try/except.
In `@scripts/check_local_version_ahead_of_pypi.py`:
- Around line 141-142: The remediation text shown after a successful PyPI
comparison incorrectly suggests re-running with
SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 (which only helps fetch failures);
remove that suggestion from the success/failure branch that runs after a
successful PyPI fetch in scripts/check_local_version_ahead_of_pypi.py and only
keep the relevant advice (e.g., re-run with SPECFACT_SKIP_PYPI_VERSION_CHECK=1
or instructions about updating local version). Locate the code that emits the
string containing "Re-run: SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 python
scripts/check_local_version_ahead_of_pypi.py" and delete or move that phrase so
it is only present in the fetch-error path (where the script previously handles
network/fetch errors).
In `@scripts/module_pip_dependencies_licenses.yaml`:
- Around line 1-6: Populate the top-level "licenses" mapping in
scripts/module_pip_dependencies_licenses.yaml with SPDX license expressions for
all known pip packages used by bundled modules so the offline license gate can
resolve dependencies; add lowercase package keys matching declared
pip_dependencies (examples: beartype, icontract, pydantic, requests, rich,
typer, ruamel.yaml, etc.) and assign their SPDX strings (e.g., MIT, Apache-2.0,
BSD-3-Clause, etc.) as values, ensuring every package referenced by any module's
pip_dependencies has an entry; this will satisfy the checker
(check_license_compliance.py) that expects a complete licenses map for offline
resolution.
In `@scripts/pre_commit_code_review.py`:
- Around line 114-155: Add the required `@icontract` decorators to the two new
public helpers: import icontract at the top of the module and annotate
discover_specfact_modules_repo and build_review_subprocess_env with appropriate
`@icontract` decorators (e.g., `@icontract.ensure` or `@icontract.require` as per
project convention) so they follow the repo’s contract-first rule; ensure the
decorators sit above `@beartype` and express the expected return characteristics
(discover_specfact_modules_repo returns Path | None, build_review_subprocess_env
returns dict[str, str]).
In `@scripts/security_audit_gate.py`:
- Around line 64-66: Wrap the subprocess.run call inside _run_pip_audit in a
try/except that catches subprocess.TimeoutExpired and handles it instead of
letting it propagate; on timeout either raise a clear, descriptive exception
(e.g., RuntimeError("pip_audit timed out after 900s")) or return a
subprocess.CompletedProcess with a non-zero returncode (e.g., 124) and stderr
containing the timeout message so callers of _run_pip_audit get a controlled
failure; ensure you reference the original cmd and include the caught exception
text in the message for diagnostics.
In `@src/specfact_cli/analyzers/graph_analyzer.py`:
- Around line 6-8: Two docstrings still refer to the deprecated "pyan" tool;
update the text in the _build_call_graph_edges() and build_dependency_graph()
docstrings to reference "pycg" (e.g., "pycg call graphs" or "call graphs
generated by pycg") so the documentation matches the subprocess migration to
pycg; locate and replace any other stray "pyan" mentions in those function
docstrings to ensure consistency with the rest of the module.
In `@src/specfact_cli/registry/module_installer.py`:
- Around line 69-80: The module root paths are duplicated; centralize the
canonical definitions by creating (or using) a single source of truth for
USER_MODULES_ROOT and MARKETPLACE_MODULES_ROOT (e.g., a new registry paths
module or the existing module_discovery module) and import those constants into
module_installer.py; update _path_is_under_user_modules_install_tree to use the
imported USER_MODULES_ROOT (remove any local definitions), and update
module_discovery.py and any other files to import the same constants so
discovery, install, and uninstall all reference the identical roots.
In `@tests/unit/analyzers/test_graph_analyzer.py`:
- Around line 148-155: The tests are patching the function at its definition
instead of where graph_analyzer looks it up: update the mock targets in
tests/unit/analyzers/test_graph_analyzer.py to patch
specfact_cli.analyzers.graph_analyzer.check_cli_tool_available (the symbol
imported in graph_analyzer via from specfact_cli.utils.optional_deps import
check_cli_tool_available) instead of
specfact_cli.utils.optional_deps.check_cli_tool_available; change all
occurrences referenced in the comment (around the patch calls that mock
check_cli_tool_available at lines near 150, 177, and 194) so the patch applies
to graph_analyzer's namespace and the subprocess/pycg availability mocks take
effect.
In `@tests/unit/scripts/test_check_license_compliance.py`:
- Around line 60-69: Remove the unused tmp_path fixture parameter from the test
function signature for test_scan_installed_env_prints_summary so it no longer
declares tmp_path: Path; leave (self, mod, capsys) and keep the existing patch
of mod._run_pip_licenses and call to
mod.scan_installed_environment(allowlist={}) unchanged; update any references if
present (there are none) to avoid unused-parameter warnings.
- Around line 50-58: The test function
test_scan_installed_env_passes_with_no_gpl declares an unused fixture parameter
tmp_path: Path; remove the unused parameter from the function signature so it
becomes def test_scan_installed_env_passes_with_no_gpl(self, mod) -> None:,
leaving the body unchanged (still patches mod._run_pip_licenses and asserts
exit_code == 0) to avoid unused-argument warnings and clarify intent.
In `@tests/unit/scripts/test_security_audit_gate.py`:
- Around line 1-79: Add a test to ensure the gate fails closed when pip-audit is
unavailable: create a new test (e.g.
test_main_fail_closed_when_pip_audit_unavailable) in
tests/unit/scripts/test_security_audit_gate.py that patches
gate_mod.subprocess.run to raise FileNotFoundError("pip-audit not found") and
assert gate_mod.main() == 1; use patch.object(gate_mod.subprocess, "run",
side_effect=FileNotFoundError(...)) so the main function's exception handling
for subprocess.run is exercised.
In `@tests/unit/utils/test_optional_deps.py`:
- Around line 14-46: The tests only assert presence/absence of keys; update them
to assert the availability boolean under patch("shutil.which",
return_value=None) so CLI-backed checks resolve to False. Specifically, in
test_check_optional_analysis_deps_includes_pycg_key,
test_check_optional_analysis_deps_includes_bandit_key (and the exclusion tests
for pyan3, syft, bearer) call check_enhanced_analysis_dependencies() under
patch("shutil.which", return_value=None) and should assert result["pycg"] is
False and result["bandit"] is False (and for excluded keys assert the key is
absent or not True as appropriate) rather than only checking key presence; keep
using check_enhanced_analysis_dependencies and the same patching of shutil.which
to ensure CLI presence is being tested.
---
Outside diff comments:
In @.github/workflows/sign-modules.yml:
- Around line 117-133: The workflow_dispatch path currently calls python
scripts/verify-modules-signature.py with VERIFY_MODULES_PR which enforces a
version-bump via the --version-check-base flag and blocks resign_all_manifests
flows; modify the dispatch branch so that when the workflow input
resign_all_manifests=true (or similar dispatch flag) you either (A) invoke a
dispatch-specific verify bundle/variable that omits version-bump enforcement, or
(B) skip passing --version-check-base to the python script for that run; update
the conditional around BASE_REF/VERIFY_MODULES_PR in the workflow (and any check
in scripts/module-verify-policy.sh or verify-modules-signature.py that reads
VERIFY_MODULES_PR) to detect the resign_all_manifests mode and bypass the
base-version check before the signing job runs.
In `@scripts/verify-modules-signature.py`:
- Around line 292-304: The function currently reads and validates integrity_raw
and checksum and calls _parse_checksum before honoring verify_checksum=False,
causing "skip mode" to still reject unsigned manifests; change the control flow
so verify_checksum is checked as early as possible (before accessing
integrity_raw/checksum and calling _parse_checksum) and if verify_checksum is
False return immediately (while still rejecting incompatible
require_signature=True), or alternatively guard the checksum parsing and
_parse_checksum call behind an if verify_checksum: block so manifests in
relaxed/VERIFY_MODULES_PR mode do not require integrity.checksum.
In `@setup.py`:
- Around line 17-35: The setup.py install_requires still lists "json5>=0.9.28"
while the project uses commentjson; update the dependency in the
install_requires list by replacing the "json5" entry with the correct
"commentjson" package and version (e.g., "commentjson>=0.9.0") so it matches
pyproject.toml and the actual imports; ensure the change occurs within the
install_requires array symbol in setup.py.
In `@src/specfact_cli/modules/module_registry/src/commands.py`:
- Around line 524-548: The uninstall logic should be unified so both the
marketplace path and the --scope (user|project) paths call the same helper;
update _uninstall_marketplace_default and any command handler that currently
uses raw shutil.rmtree to call uninstall_module(normalized,
confirm_user_scope=True) (or extend uninstall_module to accept a flag for
CLI-confirmation if needed) and remove the duplicate rmtree deletion code;
ensure the trusted CLI confirmation is passed through to uninstall_module so all
destructive removals go through the single uninstall implementation (referencing
_uninstall_marketplace_default and uninstall_module to locate and change the
duplicated branches).
In `@tests/unit/analyzers/test_graph_analyzer.py`:
- Around line 100-108: The test for extract_call_graph is flaky because it may
return early when pycg is absent; update the test to force the code path that
calls subprocess.run by mocking the availability check (or the import) so
extract_call_graph always proceeds, remove the contextlib.suppress(Exception)
and the conditional if mock_run.called guard, and assert unconditionally that
the subprocess.run call_args include timeout==15; reference the
extract_call_graph function and the patched
specfact_cli.analyzers.graph_analyzer.subprocess.run used in the test to locate
where to change the setup so the timeout path is always exercised.
- Around line 68-91: The test test_build_dependency_graph_parallel_call_graphs
should assert the call-graph hook actually ran: force the "pycg available" path
and verify side-effects by mocking subprocess.run and checking it was called and
that GraphAnalyzer.extract_call_graph (or the GraphAnalyzer.call_graphs
attribute) was populated; specifically, in the test use GraphAnalyzer(tmp_path)
then enable the availability check (e.g., set the flag or patch the availability
method on GraphAnalyzer), patch
specfact_cli.analyzers.graph_analyzer.subprocess.run to return a successful
MagicMock, call analyzer.build_dependency_graph(files), assert
subprocess.run.assert_called() (or check call count) and assert that
analyzer.call_graphs (or whatever container name used inside GraphAnalyzer)
contains entries for the processed files rather than only asserting
len(graph.nodes()).
---
Duplicate comments:
In `@openspec/changes/dep-security-cleanup/proposal.md`:
- Around line 46-90: Add explicit cross-repository coordination notes: update
the proposal's Impact/Source Tracking to enumerate required lockstep changes in
nold-ai/specfact-cli-modules — specifically: replace remaining pyan3 consumers
(call-graph invocation changes from "pyan3" → "pycg"), update any json5
references to commentjson/json in packaging and runtime (pyproject.toml extras
and hatch-test envs, and project_artifact_write import/usage), and align
manifest/license-gate entries (dependency pins/accepted exceptions such as
pygments/semgrep) before merging; mention the exact files/entries to change
(pyproject.toml extras and runtime deps, src/specfact_cli/utils/optional_deps.py
checks for "pycg"/"bandit"/"graphviz",
src/specfact_cli/analyzers/graph_analyzer.py behavior switch, and
src/specfact_cli/utils/project_artifact_write.py json import/usage) and require
an explicit PR/issue link from specfact-cli-modules confirming these updates.
- Around line 46-79: The Impact section omits CI workflow wiring,
dependency-hygiene agent rules, and license-gate scripts that were actually
changed; update the proposal.md Impact paragraph to list these concrete
artifacts and their scopes (e.g., CI workflow files added/modified for
dependency checks and build/test, dependency-hygiene agent rule updates that
enforce pycg/bandit/json changes, and license-gate script changes that affect
runtime deps), and cross-reference the existing implementation targets
(pyproject.toml, src/specfact_cli/utils/optional_deps.py,
src/specfact_cli/analyzers/graph_analyzer.py,
src/specfact_cli/utils/project_artifact_write.py) so the doc fully reflects CI,
policy agent, and licensing surfaces touched by this change.
In `@openspec/changes/dep-security-cleanup/tasks.md`:
- Around line 81-88: Checklist items 10.2, 10.6 and 10.8 are blocking but left
unchecked; either complete the actions (run `hatch run license-check` and record
its passing, run `specfact code review run --json --out
.specfact/code-review.json` and resolve findings, then make the final commit
with message `feat(deps): remove GPL/wrong packages, add license-gate and
security-audit (#<issue>)`) and mark those boxes done, or explicitly defer them
by creating a follow-up issue/PR with owner and target date and update items
10.2, 10.6 and 10.8 to a deferred state (e.g., “Deferred → #<follow-up> owner:
`@user` date: YYYY-MM-DD”) so the checklist accurately reflects the remaining
work.
In `@scripts/license_allowlist.yaml`:
- Line 13: Update the reference in scripts/license_allowlist.yaml that currently
points to "agent-rules/55-dependency-hygiene.md" to the canonical repository
path "docs/agent-rules/55-dependency-hygiene.md" so readers are directed to the
correct docs location; locate the comment line containing "Phase 2 entries:
track removal in SECURITY.md and agent-rules/55-dependency-hygiene.md" and
replace the latter path with the docs/agent-rules version.
In `@tests/unit/utils/test_project_artifact_write.py`:
- Around line 239-253: Update the test to also exercise the block-comment JSONC
path so merge_vscode_settings_prompt_recommendations() is covered for both
comment styles: create a settings.json content that uses /* ... */ block
comments (instead of or in addition to //) and write it to
tmp_path/.vscode/settings.json, then call the same
merge_vscode_settings_prompt_recommendations() flow and assert the same outcome
as the existing test; ensure the test name reflects block-comment coverage
(e.g., test_merge_vscode_settings_handles_block_comments_in_jsonc) and reference
merge_vscode_settings_prompt_recommendations to locate the code under test.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: ea08147a-1d19-41f7-856d-d307b629043a
📒 Files selected for processing (53)
.github/workflows/pr-orchestrator.yml.github/workflows/sign-modules.yml.pre-commit-config.yamlCHANGELOG.mdCONTRIBUTING.mdREADME.mddocs/agent-rules/50-quality-gates-and-review.mddocs/agent-rules/55-dependency-hygiene.mddocs/guides/module-signing-and-key-rotation.mddocs/guides/publishing-modules.mddocs/installation/enhanced-analysis-dependencies.mddocs/reference/module-security.mdopenspec/changes/dep-security-cleanup/TDD_EVIDENCE.mdopenspec/changes/dep-security-cleanup/design.mdopenspec/changes/dep-security-cleanup/proposal.mdopenspec/changes/dep-security-cleanup/specs/call-graph-analysis/spec.mdopenspec/changes/dep-security-cleanup/specs/dep-license-gate/spec.mdopenspec/changes/dep-security-cleanup/tasks.mdopenspec/changes/marketplace-06-ci-module-signing/design.mdopenspec/changes/marketplace-06-ci-module-signing/proposal.mdopenspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.mdopenspec/changes/marketplace-06-ci-module-signing/specs/ci-module-signing-on-approval/spec.mdpyproject.tomlscripts/check_license_compliance.pyscripts/check_local_version_ahead_of_pypi.pyscripts/check_version_sources.pyscripts/git-branch-module-signature-flag.shscripts/license_allowlist.yamlscripts/module-verify-policy.shscripts/module_pip_dependencies_licenses.yamlscripts/pre-commit-verify-modules.shscripts/pre_commit_code_review.pyscripts/security_audit_gate.pyscripts/setup-git-hooks.shscripts/validate_agent_rule_applies_when.pyscripts/verify-modules-signature.pyscripts/verify_safe_project_writes.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/analyzers/graph_analyzer.pysrc/specfact_cli/modules/module_registry/src/commands.pysrc/specfact_cli/registry/module_installer.pytests/unit/analyzers/test_graph_analyzer.pytests/unit/registry/test_module_installer.pytests/unit/scripts/test_check_license_compliance.pytests/unit/scripts/test_module_verify_policy.pytests/unit/scripts/test_pre_commit_verify_modules.pytests/unit/scripts/test_security_audit_gate.pytests/unit/specfact_cli/registry/test_signing_artifacts.pytests/unit/utils/test_optional_deps.pytests/unit/utils/test_project_artifact_write.pytests/unit/workflows/test_trustworthy_green_checks.py
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
CHANGELOG.md (1)
17-24:⚠️ Potential issue | 🟡 MinorWrap overlong bullets in the 0.46.3 entry.
Lines 17, 22, 23, and 24 exceed the markdown line-length rule; please reflow them with indented continuation lines to keep each line under 120 chars.
As per coding guidelines: "Keep line length under 120 characters in markdown files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 17 - 24, Reflow the overlong bullets in the 0.46.3 changelog entry so no markdown line exceeds 120 characters: split the long lines for the bullets mentioning `scripts/security_audit_gate.py` and `scripts/module_pip_dependencies_licenses.yaml` and the three "Changed" paragraphs ("License / CVE hygiene", "Call graphs", and "Pre-commit / CI") into multiple lines at natural break points, indenting continuation lines (e.g., two spaces) to keep markdown formatting intact; update the `scripts/check_license_compliance.py`, `pycg`/`--package` description, and `check-version-sources` paragraphs accordingly so each resulting line is <120 characters while preserving the original text and bullet structure..github/workflows/pr-orchestrator.yml (1)
46-52:⚠️ Potential issue | 🟠 MajorBroaden the license-gate change detector beyond
pyproject.toml.
license-checkis now gated only bypyproject_changed, butscripts/check_license_compliance.pyalso depends on the allowlist, the checker itself, and the module manifests it scans. Those policy changes can merge without the license gate ever running.Patch direction
- pyproject: - - 'pyproject.toml' + license_inputs: + - 'pyproject.toml' + - 'scripts/check_license_compliance.py' + - 'scripts/license_allowlist.yaml' + - 'packages/*/module-package.yaml' ... - pyproject_changed: ${{ steps.out.outputs.pyproject_changed }} + license_inputs_changed: ${{ steps.out.outputs.license_inputs_changed }} ... - echo "pyproject_changed=${{ steps.filter.outputs.pyproject }}" + echo "license_inputs_changed=${{ steps.filter.outputs.license_inputs }}" ... - if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.pyproject_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.license_inputs_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true'Also applies to: 593-597
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-orchestrator.yml around lines 46 - 52, The workflow currently gates the "license-check" job only on the pyproject_changed detector (pyproject.toml) but scripts/check_license_compliance.py, its allowlist/checker files, and module manifests can change without triggering the gate; update the change detector to include those additional sources so the license-check runs when any related policy or scanner code changes — specifically add patterns for scripts/check_license_compliance.py, the allowlist/checker files (e.g., allowlist paths or filenames), and the module manifest files (the manifests scanned by the checker) to the list used by pyproject_changed (or create a new composite detector) so that license-check is triggered whenever any of those files are modified..pre-commit-config.yaml (1)
24-29:⚠️ Potential issue | 🟠 MajorRun the PyPI-ahead hook through Hatch's managed environment.
This hook still invokes the system interpreter, but
scripts/check_local_version_ahead_of_pypi.pyimports Hatch-managed dependencies such asbeartypeandicontract. On a clean contributor machine, pre-commit will fail before the project environment is used.Patch
- entry: bash -c 'export SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 && exec python scripts/check_local_version_ahead_of_pypi.py' + entry: bash -c 'export SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 && exec hatch run python scripts/check_local_version_ahead_of_pypi.py'#!/bin/bash set -euo pipefail echo "=== Hook entry ===" sed -n '24,29p' .pre-commit-config.yaml echo echo "=== Third-party imports used by the script ===" rg -n 'from beartype|from icontract|from packaging' scripts/check_local_version_ahead_of_pypi.pyExpected result: the hook currently uses plain
python, while the script imports third-party packages that are normally provided by the Hatch-managed environment.
As per coding guidelines, "scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented quality gates (format, type-check, module signing)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 24 - 29, The pre-commit hook with id check-local-version-ahead-of-pypi calls the system python but the target script scripts/check_local_version_ahead_of_pypi.py depends on Hatch-managed packages; change the hook entry so it runs the script inside Hatch's environment (e.g., invoke hatch run to execute python) instead of the plain system interpreter, updating the entry for the hook in .pre-commit-config.yaml so the script imports (beartype, icontract, packaging) resolve correctly.scripts/check_local_version_ahead_of_pypi.py (1)
133-144:⚠️ Potential issue | 🟡 MinorRemove the lenient-network re-run advice from the policy-failure path.
This branch only runs after a successful PyPI fetch, so
SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1cannot change thelocal <= PyPIoutcome. Keeping it here points people at a non-fix; reserve that hint for the fetch-error path.Patch
- " 4. Re-run: SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 python scripts/check_local_version_ahead_of_pypi.py\n" - " (pre-commit runs this with lenient network; offline: SPECFACT_SKIP_PYPI_VERSION_CHECK=1 — not for CI.)" + " 4. Re-run this check after bumping the version.\n" + " If a later run hits a PyPI fetch error, use SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 locally.\n" + " Offline: SPECFACT_SKIP_PYPI_VERSION_CHECK=1 — not for CI."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_local_version_ahead_of_pypi.py` around lines 133 - 144, Remove the misleading lenient-network re-run hint from the policy-failure remediation message: in the remediation block that is printed when local <= pypi_latest (the message containing "REMEDIATION (AI / developer checklist):" and referencing local and pypi_latest), delete the suggestion to re-run with SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 (leave any valid offline skip note like SPECFACT_SKIP_PYPI_VERSION_CHECK only if already present). Instead, add the lenient-network suggestion only in the fetch-error branch that handles PyPI fetch failures (the code path that errors when fetching pypi_latest), so the environment hint appears only when a network fetch actually failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-orchestrator.yml:
- Around line 593-649: Update the job dependency graph so the release path waits
for the new gates: add license-check and security-audit to the needs list for
the package-validation job (and ensure publish-pypi also transitively depends on
package-validation), i.e., modify the package-validation job's needs to include
"license-check" and "security-audit" alongside its existing needs so the package
build and the publish-pypi job cannot run until both gates complete
successfully.
In `@CHANGELOG.md`:
- Around line 15-28: Update the CHANGELOG.md entry to explicitly document
dependency removals/replacements: add concrete bullet(s) under the "Changed" or
a new "Removed" section naming the packages replaced or removed and the
replacement (e.g., note that scripts/check_license_compliance.py and the new
scripts/security_audit_gate.py/scripts/module_pip_dependencies_licenses.yaml
reflect the dependency hygiene changes and list the exact packages removed or
swapped), include rationale/context in one short clause per bullet, and ensure
the bullets reference the updated artifacts
(scripts/check_license_compliance.py, scripts/security_audit_gate.py,
scripts/module_pip_dependencies_licenses.yaml) so the release notes clearly
capture the dependency changes.
In `@docs/agent-rules/70-release-commit-and-docs.md`:
- Line 19: The metadata field last_reviewed in
docs/agent-rules/70-release-commit-and-docs.md is set to a future date
(last_reviewed: 2026-04-16); update the last_reviewed value to a non-future date
(e.g., 2026-04-15 or earlier) so the file metadata reads accurately after merge.
- Line 42: Update the "Versioning" lead bullet so it lists all four canonical
version sources (pyproject.toml, setup.py, src/__init__.py, and
src/specfact_cli/__init__.py) to match the PyPI ahead-of check paragraph; find
the lead bullet in the same section and add the missing file
(src/specfact_cli/__init__.py) so the two bullets are consistent.
In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 188-190: Update the unit test to explicitly assert the new
pre-commit hook behavior: read scripts/pre-commit-verify-modules.sh (or the
generated pre-commit config content used by the helper) and assert that the hook
name check-local-version-ahead-of-pypi is present and that its files: scope
exactly matches the canonical version-source set used by the helper; locate the
test in tests/unit/workflows/test_trustworthy_green_checks.py and adjust the
assertions around verify_body/verify_script so they validate the hook name and
compare the files list to the expected canonical set (use the same
constant/source used by the helper to avoid duplication).
- Around line 66-71: Update the
test_pr_orchestrator_pypi_version_check_gated_on_version_sources test to parse
the PR_ORCHESTRATOR YAML and assert structurally that within the job named
"tests" there is a step whose name is "Verify local version is ahead of PyPI"
and that that step's if/condition equals "version_sources_changed == 'true'";
replace the three substring assertions with code that loads the YAML (e.g.,
yaml.safe_load), finds the "jobs" -> "tests" entry, iterates its "steps" to find
the step by name, and assert on that step's "if" field value, raising a clear
failure if the job/step/condition is missing.
---
Duplicate comments:
In @.github/workflows/pr-orchestrator.yml:
- Around line 46-52: The workflow currently gates the "license-check" job only
on the pyproject_changed detector (pyproject.toml) but
scripts/check_license_compliance.py, its allowlist/checker files, and module
manifests can change without triggering the gate; update the change detector to
include those additional sources so the license-check runs when any related
policy or scanner code changes — specifically add patterns for
scripts/check_license_compliance.py, the allowlist/checker files (e.g.,
allowlist paths or filenames), and the module manifest files (the manifests
scanned by the checker) to the list used by pyproject_changed (or create a new
composite detector) so that license-check is triggered whenever any of those
files are modified.
In @.pre-commit-config.yaml:
- Around line 24-29: The pre-commit hook with id
check-local-version-ahead-of-pypi calls the system python but the target script
scripts/check_local_version_ahead_of_pypi.py depends on Hatch-managed packages;
change the hook entry so it runs the script inside Hatch's environment (e.g.,
invoke hatch run to execute python) instead of the plain system interpreter,
updating the entry for the hook in .pre-commit-config.yaml so the script imports
(beartype, icontract, packaging) resolve correctly.
In `@CHANGELOG.md`:
- Around line 17-24: Reflow the overlong bullets in the 0.46.3 changelog entry
so no markdown line exceeds 120 characters: split the long lines for the bullets
mentioning `scripts/security_audit_gate.py` and
`scripts/module_pip_dependencies_licenses.yaml` and the three "Changed"
paragraphs ("License / CVE hygiene", "Call graphs", and "Pre-commit / CI") into
multiple lines at natural break points, indenting continuation lines (e.g., two
spaces) to keep markdown formatting intact; update the
`scripts/check_license_compliance.py`, `pycg`/`--package` description, and
`check-version-sources` paragraphs accordingly so each resulting line is <120
characters while preserving the original text and bullet structure.
In `@scripts/check_local_version_ahead_of_pypi.py`:
- Around line 133-144: Remove the misleading lenient-network re-run hint from
the policy-failure remediation message: in the remediation block that is printed
when local <= pypi_latest (the message containing "REMEDIATION (AI / developer
checklist):" and referencing local and pypi_latest), delete the suggestion to
re-run with SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 (leave any valid
offline skip note like SPECFACT_SKIP_PYPI_VERSION_CHECK only if already
present). Instead, add the lenient-network suggestion only in the fetch-error
branch that handles PyPI fetch failures (the code path that errors when fetching
pypi_latest), so the environment hint appears only when a network fetch actually
failed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e025af0b-e730-4a67-8195-648fe92bc6d3
📒 Files selected for processing (6)
.github/workflows/pr-orchestrator.yml.pre-commit-config.yamlCHANGELOG.mddocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{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:
docs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Files:
docs/agent-rules/70-release-commit-and-docs.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing accuracy: CLI examples match current behavior; preserve Jekyll front matter;
call out when README/docs index need sync.
Files:
docs/agent-rules/70-release-commit-and-docs.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
docs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.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/unit/workflows/test_trustworthy_green_checks.py
**/*.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:
tests/unit/workflows/test_trustworthy_green_checks.pyscripts/check_local_version_ahead_of_pypi.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/unit/workflows/test_trustworthy_green_checks.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/unit/workflows/test_trustworthy_green_checks.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:
tests/unit/workflows/test_trustworthy_green_checks.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:
tests/unit/workflows/test_trustworthy_green_checks.pyscripts/check_local_version_ahead_of_pypi.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/check_local_version_ahead_of_pypi.py
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate GitHub workflow files using
hatch run lint-workflowsbefore committing
.github/workflows/*.{yml,yaml}: Use actionlint for semantic validation of GitHub Actions workflows
Format GitHub Actions workflows usinghatch run workflows-fmtand lint them withhatch run workflows-lintafter editing
Files:
.github/workflows/pr-orchestrator.yml
.github/workflows/!(tests).{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)
Files:
.github/workflows/pr-orchestrator.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test
gates, and action versions.
Files:
.github/workflows/pr-orchestrator.yml
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
🧠 Learnings (33)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: For `/opsx:ff` (Fast-Forward Change Creation): OPSX provides change scaffolding and artifact templates. AGENTS.md requires explicitly adding: Worktree creation task as Step 1 in tasks.md (not just 'create branch'), TDD_EVIDENCE.md tracking task in section 2 (Tests), Documentation research task per `openspec/config.yaml`, Module signing quality gate if applicable, Worktree cleanup steps in final section
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to pyproject.toml : 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
Applied to files:
docs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: For large frontmatter file changes in `docs/agent-rules/`, run `cd ../specfact-cli-internal && python3 scripts/wiki_rebuild_graph.py` from the sibling repo after merging
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:34.504Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T22:41:34.504Z
Learning: Treat the canonical rule docs in docs/agent-rules/INDEX.md as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to docs/**/*.md : Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: Treat `docs/agent-rules/` as the canonical location for repository governance rather than relying on inline reminders or other documentation
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to @(pyproject.toml|setup.py|src/__init__.py|src/specfact_cli/__init__.py) : Maintain synchronized versions across pyproject.toml, setup.py, src/__init__.py, and src/specfact_cli/__init__.py.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yamlscripts/check_local_version_ahead_of_pypi.py.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to {pyproject.toml,setup.py,src/__init__.py} : Manually update version numbers in pyproject.toml, setup.py, and src/__init__.py when making a formal version change
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: For OpenSpec changes when a sibling `specfact-cli-internal/` checkout exists, read internal wiki guidance in `docs/agent-rules/40-openspec-and-tdd.md` and consult `wiki/hot.md`, `wiki/graph.md`, and relevant `wiki/concepts/*.md` files
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: After merges shipping OpenSpec- or GitHub-related work with a sibling `specfact-cli-internal` checkout present, run wiki scripts from that sibling repo's working directory (e.g., `cd ../specfact-cli-internal && python3 scripts/wiki_openspec_gh_status.py`), not from `specfact-cli` or other directories
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:26.519Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-10T22:41:26.519Z
Learning: When a sibling internal repository exists, read its wiki/ files by absolute path before designing an OpenSpec change; refer to Strategic context in AGENTS.md and docs/agent-rules/40-openspec-and-tdd.md
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to @(README.md|AGENTS.md) : Check README.md and AGENTS.md for current project status and development guidelines. Review .cursor/rules/ for detailed development standards and testing procedures.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Before designing or scoping a new OpenSpec change, read the wiki paths listed in docs/agent-rules/40-openspec-and-tdd.md#internal-wiki-and-strategic-context using absolute paths to sibling internal repository files
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to openspec/changes/**/*.md : For `/opsx:archive` (Archive change): Include module signing and cleanup in final tasks. Agents MUST run `openspec archive <change-id>` from repo root (no manual `mv` under `openspec/changes/archive/`)
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to **/*.{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: Run `hatch run contract-test` for 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
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yaml.github/workflows/pr-orchestrator.ymlCHANGELOG.md
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to {src/__init__.py,pyproject.toml,setup.py} : Maintain version synchronization across src/__init__.py, pyproject.toml, and setup.py
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to {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
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: All development work MUST use git worktrees per AGENTS.md Git Worktree Policy. Never create branches with `git checkout -b` in the primary checkout. Create worktree from origin/dev: `git worktree add ../specfact-cli-worktrees/<type>/<slug> -b <branch-name> origin/dev` where allowed types are: `feature/`, `bugfix/`, `hotfix/`, `chore/`. Forbidden in worktrees: `dev`, `main`. After creating worktree: `cd ../specfact-cli-worktrees/<type>/<slug>`. Bootstrap Hatch in worktree: `hatch env create`. Run pre-flight checks: `hatch run smart-test-status` and `hatch run contract-test-status`. Do all implementation work from the worktree, never from primary checkout. After PR merge: cleanup with `git worktree remove`, `git branch -d`, `git worktree prune`
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Before making a pull request, locally run format, lint, contract tests, and full test suite: `hatch run format`, `hatch run lint`, `hatch run contract-test`, `hatch test --cover -v`
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Run `hatch run yaml-check-all` in CI and before pull requests to validate all YAML and workflow files
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yaml.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: When creating implementation plans, task lists, or OpenSpec tasks.md, ALWAYS explicitly verify and include: (1) Worktree creation from `origin/dev` (not `git checkout -b`), (2) `hatch env create` in the worktree, (3) Pre-flight checks (`hatch run smart-test-status`, `hatch run contract-test-status`), (4) Worktree cleanup steps post-merge, (5) Self-check: 'Have I followed AGENTS.md Git Worktree Policy section?'
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to **/*.{yml,yaml} : Validate YAML configuration files locally using `hatch run yaml-lint` before committing
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Applies to **/*.{yml,yaml} : Format all YAML and workflow files using `hatch run yaml-fix-all` before committing
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Activate the Hatch environment before making changes using `hatch shell`
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Use `hatch test` command for running tests instead of direct pytest usage to ensure proper Python matrix, dependency resolution, environment variables, and coverage reports
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to src/**/*.py : 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.
Applied to files:
.pre-commit-config.yamltests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-04-10T22:41:26.519Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-10T22:41:26.519Z
Learning: Enforce the clean-code review gate through `hatch run specfact code review run --json --out .specfact/code-review.json`
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing
Applied to files:
.pre-commit-config.yaml.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
scripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Before executing ANY workflow command (`/opsx:ff`, `/opsx:apply`, `/opsx:continue`, etc.), perform the Pre-Execution Checklist: (1) Git Worktree - create if task creates branches/modifies code, (2) TDD Evidence - create `TDD_EVIDENCE.md` if behavior changes, (3) Documentation - include documentation research if changes affect user-facing behavior, (4) Module Signing - include signature verification if changes modify bundled modules, (5) Confirmation - state clearly that pre-execution checklist is complete and AGENTS.md compliance is confirmed
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to CHANGELOG.md : Update CHANGELOG.md with all code changes as part of version control requirements.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Always finish each output by listing: (1) Which rulesets have been applied (e.g., `.cursorrules`, `AGENTS.md`, specific `.cursor/rules/*.mdc`), (2) Confirmation of Git Worktree Policy compliance (if applicable), (3) Which AI (LLM) provider and model version you are using
Applied to files:
CHANGELOG.md
🪛 LanguageTool
docs/agent-rules/70-release-commit-and-docs.md
[uncategorized] ~42-~42: The official name of this software platform is spelled with a capital “H”.
Context: ...greater** (matching the publish gate in `.github/workflows/scripts/check-and-publish-pyp...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Module manifests (targets for new license gate):
- packages/*/module-package.yaml present (6 files). These manifests will be scanned by scripts/check_license_compliance.py and must map pip_dependencies to scripts/module_pip_dependencies_licenses.yaml. [::nold-ai/specfact-cli-modules::packages/specfact-code-review/module-package.yaml], [::nold-ai/specfact-cli-modules::packages/specfact-project/module-package.yaml], [::nold-ai/specfact-cli-modules::packages/specfact-spec/module-package.yaml]
-
Consumers of pyan3 call-graph workflow (will break or need updating to pycg):
- packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py — invokes ["pyan3", ... "--dot", ...] and parses DOT output. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py]
- packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py — checks pyan3 availability to enable dependency-graph analysis and composes install messaging referencing pyan3. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py]
- packages/specfact-project/src/specfact_project/import_cmd/commands.py — probes check_cli_tool_available("pyan3"), prints install hints, and conditionally runs pyan3-based graph regeneration. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/import_cmd/commands.py]
-
JSON5/commentjson dependency mentions:
- pyproject.toml includes json5 in this repo’s extras; docs reference pyan3/syft/bearer in installation guidance and will be out of sync with specfact-cli’s json5→commentjson and pyan3→pycg changes. [::nold-ai/specfact-cli-modules::pyproject.toml], [::nold-ai/specfact-cli-modules::docs/getting-started/installation.md]
-
Pre-commit / review env interaction:
- scripts/pre_commit_code_review.py in this repo sets/injects SPECFACT_MODULES_REPO into the env (env["SPECFACT_MODULES_REPO"] = ...). The specfact-cli PR also adds sibling-repo discovery and env injection — reviewers should verify precedence/interaction and that the two implementations won’t conflict. [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py]
-
Tests referencing SPECFACT_MODULES_REPO:
- tests/unit/test_dev_bootstrap.py relies on SPECFACT_MODULES_REPO behavior; relevant for validation of the new discovery/injection behavior. [::nold-ai/specfact-cli-modules::tests/unit/test_dev_bootstrap.py]
Summary: this repo contains direct consumers of the removed pyan3 workflow (graph analyzer, enablement messaging, import command), multiple module-package.yaml manifests that will be scanned by the new license gate and must match the static license map format, and an existing pre-commit review helper that already injects SPECFACT_MODULES_REPO — all are directly relevant cross-repo impacts for the PR.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
.pre-commit-config.yaml (1)
24-27:⚠️ Potential issue | 🟠 MajorRun the PyPI-ahead hook through Hatch’s managed environment.
Line 26 executes
scripts/check_local_version_ahead_of_pypi.pyvia system Python, which can fail on clean machines when project-only deps are missing from global interpreter state. Route this throughhatch runlike the other managed hooks.🔧 Proposed fix
- entry: bash -c 'export SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 && exec python scripts/check_local_version_ahead_of_pypi.py --skip-when-version-unchanged-vs HEAD' + entry: bash -c 'export SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 && exec hatch run python scripts/check_local_version_ahead_of_pypi.py --skip-when-version-unchanged-vs HEAD'#!/bin/bash set -euo pipefail echo "=== Hook entry ===" rg -n -C2 'id: check-local-version-ahead-of-pypi|entry:' .pre-commit-config.yaml echo echo "=== Script imports that may require managed env deps ===" rg -n 'from beartype|from icontract|from packaging|import packaging' scripts/check_local_version_ahead_of_pypi.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 24 - 27, The pre-commit hook with id "check-local-version-ahead-of-pypi" currently runs scripts/check_local_version_ahead_of_pypi.py with the system Python (entry uses bash -c ... exec python ...), which can fail when project deps aren't installed globally; update the entry to run the script inside Hatch's managed environment (use hatch run to invoke the same python command and preserve the SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK env var) so the hook executes with project-only dependencies available.docs/agent-rules/70-release-commit-and-docs.md (2)
42-42:⚠️ Potential issue | 🟡 MinorKeep Versioning bullets internally consistent on canonical files.
This section now mentions all four canonical version sources in the PyPI bullet, but the lead Versioning bullet still omits
src/__init__.py. Please align them.🔧 Proposed fix
-- Keep version updates in sync across `pyproject.toml`, `setup.py`, and `src/specfact_cli/__init__.py`. +- Keep version updates in sync across `pyproject.toml`, `setup.py`, `src/__init__.py`, and `src/specfact_cli/__init__.py`.Based on learnings, "Maintain synchronized versions across pyproject.toml, setup.py, src/init.py, and src/specfact_cli/init.py."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agent-rules/70-release-commit-and-docs.md` at line 42, Update the lead "Versioning" bullet to list all four canonical version sources consistently by adding src/__init__.py so it reads: pyproject.toml, setup.py, src/__init__.py, and src/specfact_cli/__init__.py; ensure the phrasing matches the PyPI bullet's canonical-files list and uses the same ordering/format for consistency.
19-19:⚠️ Potential issue | 🟡 MinorUse a non-future
last_revieweddate.Line 19 is set to
2026-04-16, which is in the future relative to this review date (2026-04-15).🔧 Proposed fix
-last_reviewed: 2026-04-16 +last_reviewed: 2026-04-15🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agent-rules/70-release-commit-and-docs.md` at line 19, Update the YAML frontmatter field last_reviewed (currently set to 2026-04-16) to a non-future date relative to the review (e.g., 2026-04-15 or earlier); locate and edit the last_reviewed key in docs/agent-rules/70-release-commit-and-docs.md to set an appropriate past or current date value.tests/unit/workflows/test_trustworthy_green_checks.py (2)
190-193:⚠️ Potential issue | 🟡 MinorAdd explicit assertions for the new pre-commit PyPI-ahead hook.
The helper still checks
check-version-sourcesbut notcheck-local-version-ahead-of-pypior its canonicalfiles:scope.🔧 Proposed fix
def _assert_pre_commit_verify_and_version_hooks(by_id: dict[str, dict[str, Any]]) -> None: @@ verify_body = verify_script.read_text(encoding="utf-8") assert "module-verify-policy.sh" in verify_body assert "VERIFY_MODULES_STRICT" in verify_body assert "check-version-sources" in by_id + assert "check-local-version-ahead-of-pypi" in by_id + pypi_hook = by_id["check-local-version-ahead-of-pypi"] + assert pypi_hook.get("files") == ( + r"^(pyproject\.toml|setup\.py|src/__init__\.py|src/specfact_cli/__init__\.py)$" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/workflows/test_trustworthy_green_checks.py` around lines 190 - 193, The test currently asserts the presence of "check-version-sources" but misses the new pre-commit hook; update the assertions in tests/unit/workflows/test_trustworthy_green_checks.py to also assert that "check-local-version-ahead-of-pypi" is present (e.g., in by_id or verify_body) and that its canonical files scope (the "files:" pattern used by the hook) is included; locate the assertions around verify_body and by_id and add two concise asserts verifying the hook id "check-local-version-ahead-of-pypi" and its "files:" pattern are present.
66-74:⚠️ Potential issue | 🟡 MinorUse structural YAML assertions for the PyPI gate step.
These substring checks can pass even if the condition moves to the wrong job/step. Parse the workflow and assert the
testsjob stepVerify local version is ahead of PyPIhasif: version_sources_changed == 'true'.🔧 Proposed fix
def test_pr_orchestrator_pypi_version_check_gated_on_version_sources() -> None: - """PyPI-ahead must not run on every code PR; gate matches pre-commit staged version files.""" - text = PR_ORCHESTRATOR.read_text(encoding="utf-8") - assert "version_sources:" in text - assert "version_sources_changed" in text - assert "version_sources_changed == 'true'" in text - assert "skip-when-version-unchanged-vs" in text - assert "github.event.pull_request.base.sha" in text + """PyPI-ahead must be gated in the tests job on version source changes.""" + jobs = _load_jobs() + tests_job = jobs.get("tests") + assert isinstance(tests_job, dict), "Expected tests job in pr-orchestrator" + steps = tests_job.get("steps") + assert isinstance(steps, list), "Expected steps list for tests job" + target = next( + (s for s in steps if isinstance(s, dict) and s.get("name") == "Verify local version is ahead of PyPI"), + None, + ) + assert isinstance(target, dict), "Missing 'Verify local version is ahead of PyPI' step in tests job" + assert target.get("if") == "version_sources_changed == 'true'" + run = str(target.get("run", "")) + assert "--skip-when-version-unchanged-vs" in run + assert "github.event.pull_request.base.sha" in run🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/workflows/test_trustworthy_green_checks.py` around lines 66 - 74, The test currently uses substring checks which can be fooled if the condition is moved; instead parse the PR_ORCHESTRATOR YAML, locate the job named "tests" and within its steps find the step with name "Verify local version is ahead of PyPI", then assert that that step's if condition equals "version_sources_changed == 'true'"; update the function test_pr_orchestrator_pypi_version_check_gated_on_version_sources to load PR_ORCHESTRATOR as YAML (e.g., with yaml.safe_load), navigate to ['jobs']['tests']['steps'] to find the step by its "name" and assert step.get('if') == "version_sources_changed == 'true'".scripts/check_local_version_ahead_of_pypi.py (1)
171-183:⚠️ Potential issue | 🟡 MinorRemove the lenient-network remediation from the policy-failure path.
By the time this branch runs, the PyPI request already succeeded, so
SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1cannot change alocal <= PyPIfailure. Keep that hint only in the fetch-error path.Patch direction
- " 4. Re-run: SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 python scripts/check_local_version_ahead_of_pypi.py\n" - " (pre-commit runs this with lenient network; offline: SPECFACT_SKIP_PYPI_VERSION_CHECK=1 — not for CI.)" + " 4. If a later re-run hits a PyPI fetch error, use SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 locally.\n" + " (This does not bypass local<=PyPI policy failures; offline: SPECFACT_SKIP_PYPI_VERSION_CHECK=1 — not for CI.)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_local_version_ahead_of_pypi.py` around lines 171 - 183, The remediation text in the policy-failure message (variable detail) incorrectly suggests using SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 even though the PyPI fetch already succeeded; remove the "SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1" hint from the detail string used when local <= pypi (the policy-failure path) and leave that lenient-network hint only in the fetch-error path where the PyPI request can fail; update the string assembled in the same block that defines detail (search for the variable detail in scripts/check_local_version_ahead_of_pypi.py) so the remediation steps list ends with the re-run instruction using only SPECFACT_SKIP_PYPI_VERSION_CHECK=1 for offline use..github/workflows/pr-orchestrator.yml (2)
667-670:⚠️ Potential issue | 🔴 CriticalMake
package-validationwait for the new compliance gates.The publish path is still gated only by
package-validation, andpackage-validationdoes not depend onlicense-checkorsecurity-audit. Apushtomaincan therefore build and publish while one of the new gates is red or still running.Patch direction
package-validation: name: Package Validation (uvx/pip) runs-on: ubuntu-latest - needs: [tests, compat-py311, contract-first-ci, cli-validation, type-checking, linting] + needs: + - tests + - compat-py311 + - contract-first-ci + - cli-validation + - type-checking + - linting + - license-check + - security-auditAs per coding guidelines,
.github/workflows/**: "CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test gates, and action versions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-orchestrator.yml around lines 667 - 670, The package-validation job currently lists needs: [tests, compat-py311, contract-first-ci, cli-validation, type-checking, linting]; update the package-validation job to also depend on the new compliance gate jobs by adding license-check and security-audit to that needs array so package-validation waits for those gates (refer to the job name package-validation and the gate job names license-check and security-audit when making the change).
46-52:⚠️ Potential issue | 🟠 MajorExpand the license gate trigger to all license-policy inputs.
check_license_compliance.pyenforces more thanpyproject.toml—it also scans manifest/policy inputs. As written, changes to the checker, the allowlist, ormodule-package.yamlfiles can merge without ever running this gate.Patch direction
- pyproject: - - 'pyproject.toml' + license_inputs: + - 'pyproject.toml' + - 'scripts/check_license_compliance.py' + - 'scripts/license_allowlist.yaml' + - 'modules/**/module-package.yaml' + - 'src/specfact_cli/modules/**/module-package.yaml' ... - pyproject_changed: ${{ steps.out.outputs.pyproject_changed }} + license_inputs_changed: ${{ steps.out.outputs.license_inputs_changed }} ... - echo "pyproject_changed=true" + echo "license_inputs_changed=true" ... - echo "pyproject_changed=${{ steps.filter.outputs.pyproject }}" + echo "license_inputs_changed=${{ steps.filter.outputs.license_inputs }}" ... - if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.pyproject_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.license_inputs_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true'Also applies to: 609-613
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-orchestrator.yml around lines 46 - 52, The workflow's license gate only triggers on changes to pyproject.toml; update the path triggers so the job that runs check_license_compliance.py also fires when any license-policy inputs change. Add the check script and all policy/allowlist/manifest files (e.g. check_license_compliance.py, the allowlist file(s), module-package.yaml and any license policy/manifest directories) to the same trigger list so edits to the checker, allowlist, or module-package.yaml will run the license gate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_local_version_ahead_of_pypi.py`:
- Around line 87-101: pyproject_version_at_git_revision currently calls
subprocess.run directly which can raise OSError/FileNotFoundError if git is
absent; wrap the subprocess.run call in a try/except that catches OSError (or
FileNotFoundError) and returns None so the function honors its docstring and
`@icontract` ensure (result is None or str). Keep the existing behavior of
checking completed.returncode != 0 and returning None, and only convert the
subprocess-level exceptions into a graceful None before proceeding to call
read_project_version_from_pyproject_bytes.
In `@tests/unit/scripts/test_check_local_version_ahead_of_pypi.py`:
- Around line 112-118: The test
test_main_skip_when_base_unknown_still_queries_pypi currently only asserts the
exit code; to validate the “base unknown” path you must also assert that
fetch_latest_pypi_version was invoked. Update the test to capture the patched
fetch_latest_pypi_version mock (the patch.object call for
fetch_latest_pypi_version) and add an assertion like
mock_fetch.assert_called_once() after calling mod.main([...]); keep the existing
patches for read_local_version and pyproject_version_at_git_revision and retain
the assert on return code.
---
Duplicate comments:
In @.github/workflows/pr-orchestrator.yml:
- Around line 667-670: The package-validation job currently lists needs: [tests,
compat-py311, contract-first-ci, cli-validation, type-checking, linting]; update
the package-validation job to also depend on the new compliance gate jobs by
adding license-check and security-audit to that needs array so
package-validation waits for those gates (refer to the job name
package-validation and the gate job names license-check and security-audit when
making the change).
- Around line 46-52: The workflow's license gate only triggers on changes to
pyproject.toml; update the path triggers so the job that runs
check_license_compliance.py also fires when any license-policy inputs change.
Add the check script and all policy/allowlist/manifest files (e.g.
check_license_compliance.py, the allowlist file(s), module-package.yaml and any
license policy/manifest directories) to the same trigger list so edits to the
checker, allowlist, or module-package.yaml will run the license gate.
In @.pre-commit-config.yaml:
- Around line 24-27: The pre-commit hook with id
"check-local-version-ahead-of-pypi" currently runs
scripts/check_local_version_ahead_of_pypi.py with the system Python (entry uses
bash -c ... exec python ...), which can fail when project deps aren't installed
globally; update the entry to run the script inside Hatch's managed environment
(use hatch run to invoke the same python command and preserve the
SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK env var) so the hook executes with
project-only dependencies available.
In `@docs/agent-rules/70-release-commit-and-docs.md`:
- Line 42: Update the lead "Versioning" bullet to list all four canonical
version sources consistently by adding src/__init__.py so it reads:
pyproject.toml, setup.py, src/__init__.py, and src/specfact_cli/__init__.py;
ensure the phrasing matches the PyPI bullet's canonical-files list and uses the
same ordering/format for consistency.
- Line 19: Update the YAML frontmatter field last_reviewed (currently set to
2026-04-16) to a non-future date relative to the review (e.g., 2026-04-15 or
earlier); locate and edit the last_reviewed key in
docs/agent-rules/70-release-commit-and-docs.md to set an appropriate past or
current date value.
In `@scripts/check_local_version_ahead_of_pypi.py`:
- Around line 171-183: The remediation text in the policy-failure message
(variable detail) incorrectly suggests using
SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1 even though the PyPI fetch already
succeeded; remove the "SPECFACT_PYPI_VERSION_CHECK_LENIENT_NETWORK=1" hint from
the detail string used when local <= pypi (the policy-failure path) and leave
that lenient-network hint only in the fetch-error path where the PyPI request
can fail; update the string assembled in the same block that defines detail
(search for the variable detail in scripts/check_local_version_ahead_of_pypi.py)
so the remediation steps list ends with the re-run instruction using only
SPECFACT_SKIP_PYPI_VERSION_CHECK=1 for offline use.
In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 190-193: The test currently asserts the presence of
"check-version-sources" but misses the new pre-commit hook; update the
assertions in tests/unit/workflows/test_trustworthy_green_checks.py to also
assert that "check-local-version-ahead-of-pypi" is present (e.g., in by_id or
verify_body) and that its canonical files scope (the "files:" pattern used by
the hook) is included; locate the assertions around verify_body and by_id and
add two concise asserts verifying the hook id
"check-local-version-ahead-of-pypi" and its "files:" pattern are present.
- Around line 66-74: The test currently uses substring checks which can be
fooled if the condition is moved; instead parse the PR_ORCHESTRATOR YAML, locate
the job named "tests" and within its steps find the step with name "Verify local
version is ahead of PyPI", then assert that that step's if condition equals
"version_sources_changed == 'true'"; update the function
test_pr_orchestrator_pypi_version_check_gated_on_version_sources to load
PR_ORCHESTRATOR as YAML (e.g., with yaml.safe_load), navigate to
['jobs']['tests']['steps'] to find the step by its "name" and assert
step.get('if') == "version_sources_changed == 'true'".
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 034c77b1-08bf-48ea-bdcb-ce634e9496ee
📒 Files selected for processing (8)
.github/workflows/pr-orchestrator.yml.pre-commit-config.yamlCHANGELOG.mddocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.pytests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
**/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/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.py
**/*.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:
tests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.pyscripts/check_local_version_ahead_of_pypi.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/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.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/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.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:
tests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.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:
tests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.pyscripts/check_local_version_ahead_of_pypi.py
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate GitHub workflow files using
hatch run lint-workflowsbefore committing
.github/workflows/*.{yml,yaml}: Use actionlint for semantic validation of GitHub Actions workflows
Format GitHub Actions workflows usinghatch run workflows-fmtand lint them withhatch run workflows-lintafter editing
Files:
.github/workflows/pr-orchestrator.yml
.github/workflows/!(tests).{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)
Files:
.github/workflows/pr-orchestrator.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test
gates, and action versions.
Files:
.github/workflows/pr-orchestrator.yml
**/*.{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:
docs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Files:
docs/agent-rules/70-release-commit-and-docs.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing accuracy: CLI examples match current behavior; preserve Jekyll front matter;
call out when README/docs index need sync.
Files:
docs/agent-rules/70-release-commit-and-docs.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
docs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
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/check_local_version_ahead_of_pypi.py
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
🧠 Learnings (39)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: For `/opsx:ff` (Fast-Forward Change Creation): OPSX provides change scaffolding and artifact templates. AGENTS.md requires explicitly adding: Worktree creation task as Step 1 in tasks.md (not just 'create branch'), TDD_EVIDENCE.md tracking task in section 2 (Tests), Documentation research task per `openspec/config.yaml`, Module signing quality gate if applicable, Worktree cleanup steps in final section
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Run `hatch run yaml-check-all` in CI and before pull requests to validate all YAML and workflow files
Applied to files:
.pre-commit-config.yamltests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to **/*.{yml,yaml} : Validate YAML configuration files locally using `hatch run yaml-lint` before committing
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Applies to **/*.{yml,yaml} : Format all YAML and workflow files using `hatch run yaml-fix-all` before committing
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Before making a pull request, locally run format, lint, contract tests, and full test suite: `hatch run format`, `hatch run lint`, `hatch run contract-test`, `hatch test --cover -v`
Applied to files:
.pre-commit-config.yamltests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to **/*.{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: Run `hatch run contract-test` for 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
Applied to files:
.pre-commit-config.yamltests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Activate the Hatch environment before making changes using `hatch shell`
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: When creating implementation plans, task lists, or OpenSpec tasks.md, ALWAYS explicitly verify and include: (1) Worktree creation from `origin/dev` (not `git checkout -b`), (2) `hatch env create` in the worktree, (3) Pre-flight checks (`hatch run smart-test-status`, `hatch run contract-test-status`), (4) Worktree cleanup steps post-merge, (5) Self-check: 'Have I followed AGENTS.md Git Worktree Policy section?'
Applied to files:
.pre-commit-config.yamltests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Use `hatch test` command for running tests instead of direct pytest usage to ensure proper Python matrix, dependency resolution, environment variables, and coverage reports
Applied to files:
.pre-commit-config.yaml.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to src/**/*.py : 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.
Applied to files:
.pre-commit-config.yamltests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:26.519Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-10T22:41:26.519Z
Learning: Enforce the clean-code review gate through `hatch run specfact code review run --json --out .specfact/code-review.json`
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing
Applied to files:
.pre-commit-config.yaml.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to @(pyproject.toml|setup.py|src/__init__.py|src/specfact_cli/__init__.py) : Maintain synchronized versions across pyproject.toml, setup.py, src/__init__.py, and src/specfact_cli/__init__.py.
Applied to files:
.pre-commit-config.yamltests/unit/scripts/test_check_local_version_ahead_of_pypi.py.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: All development work MUST use git worktrees per AGENTS.md Git Worktree Policy. Never create branches with `git checkout -b` in the primary checkout. Create worktree from origin/dev: `git worktree add ../specfact-cli-worktrees/<type>/<slug> -b <branch-name> origin/dev` where allowed types are: `feature/`, `bugfix/`, `hotfix/`, `chore/`. Forbidden in worktrees: `dev`, `main`. After creating worktree: `cd ../specfact-cli-worktrees/<type>/<slug>`. Bootstrap Hatch in worktree: `hatch env create`. Run pre-flight checks: `hatch run smart-test-status` and `hatch run contract-test-status`. Do all implementation work from the worktree, never from primary checkout. After PR merge: cleanup with `git worktree remove`, `git branch -d`, `git worktree prune`
Applied to files:
.pre-commit-config.yamldocs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to pyproject.toml : 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
Applied to files:
tests/unit/scripts/test_check_local_version_ahead_of_pypi.pydocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to {pyproject.toml,setup.py,src/__init__.py} : Manually update version numbers in pyproject.toml, setup.py, and src/__init__.py when making a formal version change
Applied to files:
tests/unit/scripts/test_check_local_version_ahead_of_pypi.pydocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to {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
Applied to files:
tests/unit/scripts/test_check_local_version_ahead_of_pypi.pydocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to {src/__init__.py,pyproject.toml,setup.py} : Maintain version synchronization across src/__init__.py, pyproject.toml, and setup.py
Applied to files:
tests/unit/scripts/test_check_local_version_ahead_of_pypi.pydocs/agent-rules/70-release-commit-and-docs.mdscripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/!(tests).{yml,yaml} : Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)
Applied to files:
tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/{quality-gates,contract-enforcement,contracts,no-escape-gating,integrated-ci}.{yml,yaml} : Dependent workflows must download the coverage.xml artifact from the Tests workflow and fail early if it is missing
Applied to files:
tests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: For `/opsx:apply` (Implementation): OPSX provides task iteration and progress tracking. AGENTS.md requires verification before each task: Confirm you are IN a worktree (not primary checkout) before modifying code, Record failing test evidence in `TDD_EVIDENCE.md` BEFORE implementing, Record passing test evidence AFTER implementation, Run quality gates from worktree (format, type-check, contract-test), GPG-signed commits (`git commit -S`), PR to `dev` branch (never direct push)
Applied to files:
tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Before executing ANY workflow command (`/opsx:ff`, `/opsx:apply`, `/opsx:continue`, etc.), perform the Pre-Execution Checklist: (1) Git Worktree - create if task creates branches/modifies code, (2) TDD Evidence - create `TDD_EVIDENCE.md` if behavior changes, (3) Documentation - include documentation research if changes affect user-facing behavior, (4) Module Signing - include signature verification if changes modify bundled modules, (5) Confirmation - state clearly that pre-execution checklist is complete and AGENTS.md compliance is confirmed
Applied to files:
tests/unit/workflows/test_trustworthy_green_checks.py.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Require public GitHub metadata completeness before implementation when linked issue workflow applies: parent, labels, project assignment, blockers, and blocked-by relationships
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Run the required verification and quality gates for the touched scope before finalization
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Applies to openspec/changes/**/*.md : For `/opsx:archive` (Archive change): Include module signing and cleanup in final tasks. Agents MUST run `openspec archive <change-id>` from repo root (no manual `mv` under `openspec/changes/archive/`)
Applied to files:
.github/workflows/pr-orchestrator.ymldocs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:34.504Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T22:41:34.504Z
Learning: Treat the canonical rule docs in docs/agent-rules/INDEX.md as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: For large frontmatter file changes in `docs/agent-rules/`, run `cd ../specfact-cli-internal && python3 scripts/wiki_rebuild_graph.py` from the sibling repo after merging
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: Treat `docs/agent-rules/` as the canonical location for repository governance rather than relying on inline reminders or other documentation
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to docs/**/*.md : Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:19.077Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.077Z
Learning: Always finish each output by listing: (1) Which rulesets have been applied (e.g., `.cursorrules`, `AGENTS.md`, specific `.cursor/rules/*.mdc`), (2) Confirmation of Git Worktree Policy compliance (if applicable), (3) Which AI (LLM) provider and model version you are using
Applied to files:
docs/agent-rules/70-release-commit-and-docs.mdCHANGELOG.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: For OpenSpec changes when a sibling `specfact-cli-internal/` checkout exists, read internal wiki guidance in `docs/agent-rules/40-openspec-and-tdd.md` and consult `wiki/hot.md`, `wiki/graph.md`, and relevant `wiki/concepts/*.md` files
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:04.362Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-04-10T22:42:04.362Z
Learning: After merges shipping OpenSpec- or GitHub-related work with a sibling `specfact-cli-internal` checkout present, run wiki scripts from that sibling repo's working directory (e.g., `cd ../specfact-cli-internal && python3 scripts/wiki_openspec_gh_status.py`), not from `specfact-cli` or other directories
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:41:26.519Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-10T22:41:26.519Z
Learning: When a sibling internal repository exists, read its wiki/ files by absolute path before designing an OpenSpec change; refer to Strategic context in AGENTS.md and docs/agent-rules/40-openspec-and-tdd.md
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to @(README.md|AGENTS.md) : Check README.md and AGENTS.md for current project status and development guidelines. Review .cursor/rules/ for detailed development standards and testing procedures.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Before designing or scoping a new OpenSpec change, read the wiki paths listed in docs/agent-rules/40-openspec-and-tdd.md#internal-wiki-and-strategic-context using absolute paths to sibling internal repository files
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-10T22:42:21.849Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.849Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
scripts/check_local_version_ahead_of_pypi.py
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to CHANGELOG.md : Update CHANGELOG.md with all code changes as part of version control requirements.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to CHANGELOG.md : Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change
Applied to files:
CHANGELOG.md
🪛 LanguageTool
docs/agent-rules/70-release-commit-and-docs.md
[uncategorized] ~42-~42: The official name of this software platform is spelled with a capital “H”.
Context: ...strictly greater* than PyPI (matching `.github/workflows/scripts/check-and-publish-pyp...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
pyan3 consumers / call-graph: several files in the modules repo still reference and invoke pyan3 (will need updating or will continue to skip call-graph when pycg is absent):
- packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py: checks for pyan3 and invokes ["pyan3", ..., "--dot", ...]. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py:60-73]
- packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py: probes check_cli_tool_available("pyan3") and composes install hints/messages. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py:459-468]
- packages/specfact-project/src/specfact_project/import_cmd/commands.py: checks pyan3 availability, prints install hint, and conditionally regenerates graphs. [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/import_cmd/commands.py:625-646, 774-808]
-
Module manifests that the new license gate will scan: module-package.yaml files present and list pip_dependencies that must be mapped in the static license map (scripts/module_pip_dependencies_licenses.yaml):
- packages/specfact-codebase/module-package.yaml — pip_dependencies includes ruamel.yaml, tomlkit, etc. [::nold-ai/specfact-cli-modules::packages/specfact-codebase/module-package.yaml]
- packages/specfact-project/module-package.yaml — pip_dependencies includes gitpython, networkx, etc. [::nold-ai/specfact-cli-modules::packages/specfact-project/module-package.yaml]
- packages/specfact-spec/module-package.yaml [::nold-ai/specfact-cli-modules::packages/specfact-spec/module-package.yaml]
- packages/specfact-backlog/module-package.yaml [::nold-ai/specfact-cli-modules::packages/specfact-backlog/module-package.yaml]
- packages/specfact-code-review/module-package.yaml [::nold-ai/specfact-cli-modules::packages/specfact-code-review/module-package.yaml]
- packages/specfact-govern/module-package.yaml [::nold-ai/specfact-cli-modules::packages/specfact-govern/module-package.yaml]
Note: the modules repo’s manifests currently include dependencies (e.g., gitpython) the PR’s new license checker will inspect; ensure scripts/module_pip_dependencies_licenses.yaml is populated for these names to avoid manifest-scan violations. (Observed pip_dependencies lists above.) [::nold-ai/specfact-cli-modules::packages/*/module-package.yaml]
-
json5/commentjson mismatch: this repo’s pyproject.toml still lists json5 in the default env dependencies (specfact-cli-modules uses json5), while the specfact-cli PR replaces json5 with commentjson — consider documentation and tooling parity across repos:
- pyproject.toml: default.env dependencies include "json5>=0.9.28". [::nold-ai/specfact-cli-modules::pyproject.toml:tool.hatch.envs.default.dependencies]
-
SPECFACT_MODULES_REPO / env injection overlap: this repo already sets/changes SPECFACT_MODULES_REPO in dev bootstrap and scripts; the specfact-cli PR also injects SPECFACT_MODULES_REPO into subprocess envs — verify precedence and that the two mechanisms don't conflict:
- src/specfact_cli_modules/dev_bootstrap.py: sets os.environ["SPECFACT_MODULES_REPO"]. [::nold-ai/specfact-cli-modules::src/specfact_cli_modules/dev_bootstrap.py:53-59]
- scripts/pre_commit_code_review.py (in this repo) and specfact-cli's change both manipulate or expect SPECFACT_MODULES_REPO — tests here exercise that behavior (tests/unit/test_dev_bootstrap.py). [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py:150] and [::nold-ai/specfact-cli-modules::tests/unit/test_dev_bootstrap.py]
Summary / reviewer notes (concise)
- The modules repo contains direct callers of pyan3 (graph analyzer, code analyzer, import command). After the PR, those callers will either need to accept pycg absence (skip) or be updated to probe/use pycg; confirm expected behavior for cross-repo consumers.
- All package names in module-package.yaml pip_dependencies must be present (lowercased) in scripts/module_pip_dependencies_licenses.yaml used by the new license gate, or manifest scans will fail; populate that map or allowlist as needed (noted gitpython present).
- Align json5 → commentjson change across repos (pyproject in modules repo still depends on json5).
- Confirm SPECFACT_MODULES_REPO discovery/injection semantics between repos to avoid conflicting environment population for nested review/subprocess flows.
[::nold-ai/specfact-cli-modules::]
🔇 Additional comments (7)
.pre-commit-config.yaml (1)
16-20: Good parity update for version-source verification.Running
check-version-sourcesunconditionally improves local/CI consistency and prevents silent drift.tests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.py (1)
31-50: Nice integration coverage for the new skip flag path.This validates the subprocess-level behavior (
exit 0+ skip message) for--skip-when-version-unchanged-vs HEAD.tests/unit/scripts/test_check_local_version_ahead_of_pypi.py (1)
97-109: Good unit coverage for parse and skip semantics.The new tests around
read_project_version_from_pyproject_bytesand skip-with-equal-version behavior are clear and valuable.CHANGELOG.md (3)
22-24: Wrap long bullets to satisfy markdown line-length rules.These lines are still over the markdown line-length limit and should be wrapped with indented continuation lines.
As per coding guidelines: "Keep line length under 120 characters in markdown files".
Also applies to: 29-29
20-30: Explicitly document dependency removals/replacements in this version entry.This release entry still reads as mostly implicit on package-level removals/replacements; add explicit bullets (preferably with a
### Removedsection).As per coding guidelines: "Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change".
13-13:⚠️ Potential issue | 🟡 MinorUse a non-future release date for this published entry.
0.46.3is dated 2026-04-16, which is in the future relative to this PR context. Please set the release date when the release actually occurs (or mark it explicitly as unreleased).⛔ Skipped due to learnings
Learnt from: CR Repo: nold-ai/specfact-cli PR: 0 File: .cursor/rules/testing-and-build-guide.mdc:0-0 Timestamp: 2026-03-25T21:33:15.296Z Learning: Applies to CHANGELOG.md : Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change.github/workflows/pr-orchestrator.yml (1)
617-636: Verify this job can see the bundled-module manifests it is supposed to audit.The linked
nold-ai/specfact-cli-modulesrepo still owns multiplemodule-package.yamlfiles, but this job only checks out the CLI repo. Ifscripts/check_license_compliance.pyresolves manifests fromSPECFACT_MODULES_REPOor the sibling checkout, the manifest half of this gate is incomplete here. Linked repo finding:nold-ai/specfact-cli-modules::packages/*/module-package.yaml.#!/bin/bash set -euo pipefail echo "== Manifest discovery in scripts/check_license_compliance.py ==" rg -n -C3 'SPECFACT_MODULES_REPO|module-package\.yaml|scan_module_manifests|specfact-cli-modules' scripts/check_license_compliance.py echo echo "== Workflow preparation for license-check ==" rg -n -C3 'license-check:|SPECFACT_MODULES_REPO|specfact-cli-modules|check_license_compliance.py' .github/workflows/pr-orchestrator.yml
1 participant
Fixes #508.
Description
This change implements OpenSpec
dep-security-cleanup: dependency and license hygiene for analysis tooling, module manifests, and local dev ergonomics.Summary
scripts/check_license_compliance.pywithscripts/license_allowlist.yamlfor installed-env and module-manifest GPL checks; wire CI/docs (SECURITY, agent rules, installation docs).specfact-cli-modulesfor the nestedcode reviewsubprocess viabuild_review_subprocess_env()only (noos.environmutation; no impact on user-scoped module installs).pyproject.tomlupdates as needed for gates and dependencies.Fixes #(link issue if filed)
New Features #(n/a — hygiene and tooling)
Contract References: Existing
@icontract/@beartypepatterns preserved on touched public helpers; no new public CLI surface in this PR.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) — unchanged on existing APIs; new script/tests follow project normshatch run contract-test-explorationTest Execution
hatch run contract-test(cached / no input delta in last run)hatch run contract-test-explorationhatch run contract-test-scenarioshatch run smart-test-full— 2530 passed, ~63% coverageTest Quality
How Has This Been Tested?
Contract-First Approach: Contract tests run via Hatch; targeted and full pytest runs cover new license script, optional deps, graph analyzer, project artifact write, and
pre_commit_code_reviewhelpers.Manual Testing
Automated Testing
contract-testrun)hatch run smart-test-full)Test Environment
Checklist
@icontract,@beartype) — not required for internal script-only additions beyond existing patternshatch run lintclean; type-check 0 errorsQuality Gates Status
hatch run type-check— 0 errors)hatch run lint)hatch run pre-commit-checks)hatch run contract-test-exploration)hatch run contract-test-scenarios)Screenshots/Recordings (if applicable)
N/A (tooling and backend hygiene).
OpenSpec:
openspec/changes/dep-security-cleanup/—openspec validate dep-security-cleanup --strictpasses.