Conversation
…ics (#311) * fix(backlog): harden refine writeback, prompts, and daily any filters * fix(github): default story type fallback to feature * Fix format * Fix codex review findings * bump and sign changed modules * chore(hooks): enforce module signature verification in pre-commit * chore(hooks): add markdownlint to pre-commit checks --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* fix(backlog): harden refine writeback, prompts, and daily any filters * fix(github): default story type fallback to feature * Fix format * Fix codex review findings * bump and sign changed modules * chore(hooks): enforce module signature verification in pre-commit * chore(hooks): add markdownlint to pre-commit checks * fix: finalize backlog-core-06 ado comment api versioning and ci hatch pins * fix: address review findings for formatter safety and ado metric patch guards * docs(openspec): update CHANGE_ORDER status tracking * fix(ado): apply iteration filter for direct issue_id lookup --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…olution, aliases, custom registries, publishing (#318) * feat: advanced marketplace features (marketplace-02) - dependency resolution, aliases, custom registries, namespace enforcement, publishing - dependency_resolver: resolve_dependencies(), --skip-deps, --force on install - alias_manager: alias create/list/remove (no top-level alias commands) - custom_registries: add-registry, list-registries, remove-registry; fetch_all_indexes; search Registry column - module_installer: namespace/name enforcement, collision detection - scripts/publish-module.py + .github/workflows/publish-modules.yml (optional signing) - docs: publishing-modules, custom-registries, dependency-resolution; updated installing-modules, module-marketplace, commands - version 0.38.0, CHANGELOG Made-with: Cursor * docs(openspec): defer 6.2.4 and 6.2.5 (index update/PR, workflow test) to later Made-with: Cursor * Add follow-up change proposals for marketplace * Fix codex review findings --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
0.38.1 Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…nv) (#507) * feat(openspec): add dep-security-cleanup change artifacts 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> * feat: dep-security-cleanup (license gate, pycg, commentjson, review env) 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 * docs(openspec): source tracking for dep-security-cleanup and CHANGE_ORDER row Link PR #507 in proposal Source Tracking; register dep-security-cleanup under openspec/CHANGE_ORDER.md (deps module). Made-with: Cursor * docs(openspec): link dep-security-cleanup to GitHub issue #508 Register tracking issue in proposal Source Tracking and CHANGE_ORDER. Made-with: Cursor * Fix code review findings and add version check * Fix review findings * Fix module sign logic * feat(deps): remove GPL/wrong packages, add license-gate and security-audit (#508) * fix(versioning): enforce packaged artifact bump policy (#508) * Bump registry version * Fix review findings and test failurs * Fix validation script --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes module-verification policy into reusable flag bundles, adds fail-closed license and pip-audit CI gates, migrates call-graph tooling (pyan3 → pycg) and JSONC handling (json5 → commentjson), adds bundled-module auto-publish detection/PR flow, and bumps package version to 0.46.4. Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator as Orchestrator
participant Checkout as "actions/checkout"
participant Detector as "detect_modules_to_publish"
participant Registry as "bundled registry\n(resources/index.json)"
participant Packager as "sign & package\n(sign-modules.py)"
participant PRAuthor as "PR creator\n(update snapshot + artifacts)"
Orchestrator->>Checkout: checkout head (fetch-depth=0)
Orchestrator->>Detector: run detector (--registry-index resources/...)
Detector->>Registry: read bundled snapshot
Detector-->>Orchestrator: eligible module list
Orchestrator->>Packager: for each module -> package & (re)sign
Packager-->>Orchestrator: artifacts + registry fragments
Orchestrator->>PRAuthor: compute index diff
alt index changed
PRAuthor->>Orchestrator: create branch, commit resources/index.json, upload artifacts, open PR
else no change
Orchestrator-->>Orchestrator: exit (no PR)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![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: 5d0d82f03f
ℹ️ 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: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/specfact_cli/registry/module_installer.py (1)
1005-1031:⚠️ Potential issue | 🟠 MajorPrefer the canonical user root before the legacy fallback.
For
source == "marketplace", this still probesMARKETPLACE_MODULES_ROOTbeforeUSER_MODULES_ROOTand returns after the first delete. If both copies exist during migration, the stale legacy directory is removed first and the live user-scope install is left behind, even though the CLI now passesconfirm_user_scope=Trueand reports success.Suggested fix
if install_root is not None: candidate_roots = [install_root] elif source == "marketplace": - candidate_roots = [MARKETPLACE_MODULES_ROOT, USER_MODULES_ROOT] + candidate_roots = [USER_MODULES_ROOT, MARKETPLACE_MODULES_ROOT] else: candidate_roots = [USER_MODULES_ROOT, MARKETPLACE_MODULES_ROOT]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/specfact_cli/registry/module_installer.py` around lines 1005 - 1031, The loop currently picks MARKETPLACE_MODULES_ROOT before USER_MODULES_ROOT when source == "marketplace", which can delete a legacy copy instead of the canonical user install; change the branch in module_installer.py that sets candidate_roots for source == "marketplace" so it uses [USER_MODULES_ROOT, MARKETPLACE_MODULES_ROOT] (mirroring the other branch) so the code will probe and uninstall the canonical USER_MODULES_ROOT first; keep the rest of the logic (the _path_is_under_user_modules_install_tree check, confirm_user_scope/env flag handling, shutil.rmtree and early return) unchanged..github/workflows/sign-modules.yml (1)
117-138:⚠️ Potential issue | 🟠 Major
resign_all_manifests=truestill ends up enforcing a version bump.In this branch you only omit
--version-check-base, butVERIFY_ARGSstill comes from the PR bundle. If that bundle includes--enforce-version-bump, the verifier falls back to its default base (HEAD~1), which breaks the documented "manifest matches base but lacks signatures" recovery flow.Possible workflow fix
- if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ "${{ github.event.inputs.resign_all_manifests }}" = "true" ]; then - python scripts/verify-modules-signature.py "${VERIFY_ARGS[@]}" + if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ "${{ github.event.inputs.resign_all_manifests }}" = "true" ]; then + python scripts/verify-modules-signature.py --skip-checksum-verification else python scripts/verify-modules-signature.py "${VERIFY_ARGS[@]}" --version-check-base "$BASE_REF" fi🤖 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 - 138, The workflow still passes VERIFY_ARGS (populated from VERIFY_MODULES_PR) which can contain --enforce-version-bump or --version-check-base, causing the verifier to enforce a version check even when resign_all_manifests=true; update the branch that handles "${{ github.event.inputs.resign_all_manifests }}" to sanitize VERIFY_ARGS by removing any occurrences of "--enforce-version-bump" and any "--version-check-base <arg>" entries (or otherwise rebuild VERIFY_ARGS without those flags) before calling python scripts/verify-modules-signature.py so the verifier runs without a version-base enforcement when resign_all_manifests is true.
🤖 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 251-272: The tests job is doing a shallow checkout which prevents
git from resolving BASE and makes python
scripts/check_local_version_ahead_of_pypi.py's --skip-when-version-unchanged-vs
"$BASE" optimization ineffective; modify the checkout step in the tests job to
use fetch-depth: 0 (same pattern as the changes and verify-module-signatures
jobs) so full history is available and git show <base>:pyproject.toml succeeds
when check_local_version_ahead_of_pypi.py is invoked.
In @.github/workflows/publish-modules.yml:
- Around line 268-290: The heredoc at the end of the loop (done <<< "${{
steps.detect.outputs.modules }}") creates a command-injection risk because
GitHub Actions output is expanded by the shell; replace the heredoc consumption
with reading the already-created file from the detect step (e.g., iterate by
piping or while IFS= read -r MODULE_DIR; do ... done <
/tmp/modules_to_publish.txt) so the loop reads lines directly from the
filesystem instead of expanding an action output; update references to the loop
terminator and remove the use of the "${{ steps.detect.outputs.modules }}"
expansion in the publish-module step to ensure safe, literal path handling.
In @.pre-commit-config.yaml:
- Around line 24-29: The hook named check-local-version-ahead-of-pypi has an
overly long entry value that will likely violate the repo yamllint line-length
rule; update the entry for that hook (the entry key under id:
check-local-version-ahead-of-pypi) to use a YAML multiline scalar (folded or
literal) so the long command (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) is wrapped across lines and no longer exceeds the configured line-length.
In `@CHANGELOG.md`:
- Around line 30-38: Create a new top-level "### Removed" subsection in this
release entry and move the bullets that list the substantive removals out of the
existing "### Changed" section: specifically the removal of `pyan3` (GPL-2.0;
replaced by `pycg` MIT), `bearer` (wrong PyPI; replaced by `bandit` MIT), and
`syft` (wrong PyPI; Anchore Syft is out-of-band), plus the note about replacing
runtime `json5` with `commentjson` (read) + stdlib `json` (write) if you
consider that a removal; then update the "Dependency hygiene
(`dep-security-cleanup`)" paragraph under "### Changed" to only contain
non-removed changes (e.g., additions `pycg`, `bandit`, `pip-licenses`,
`pip-audit`) so the changelog uses explicit "Added"/"Changed"/"Removed" sections
per guidelines.
- Around line 65-66: The changelog is missing a section separator before the
next release block; insert a horizontal rule (e.g., a line with '---')
immediately after the "## [0.46.2] - 2026-04-15" block (i.e., following the
content belonging to that release) so the next release heading is visually and
structurally separated; update around the "## [0.46.2] - 2026-04-15" heading to
add the separator line.
In `@CONTRIBUTING.md`:
- Around line 82-85: Update the "Code Contributions" section to replace any
guidance that tells contributors to create branches with "git checkout -b" so it
instructs them to follow the repository worktree flow per the AGENTS.md Git
Worktree Policy; specifically find and replace references to "git checkout -b"
or "create a branch" in the "Code Contributions" heading and add a short,
explicit instruction pointing to AGENTS.md and the required worktree commands
(and mention that all development work MUST use git worktrees), ensuring the
contributor guide is consistent with the branch-aware pre-commit pipeline
described elsewhere.
In `@docs/agent-rules/55-dependency-hygiene.md`:
- Around line 28-30: Update the stop_conditions to remove the implied allowlist
escape hatch so that a GPL/AGPL package added to a module manifest is an
unconditional stop; specifically change the stop_conditions entry referencing
"GPL/AGPL package added to module manifest without allowlist entry" to state
"GPL/AGPL package added to module manifest" and ensure Section 1 wording matches
this unconditional block (no allowlist path). Refer to the stop_conditions key
and the Section 1 text in this doc to make the two statements identical and
authoritative.
- Around line 76-80: Update the docs to describe the actual security gate:
replace the old "pip-audit --desc --strict" description for "hatch run
security-audit" with the real behavior implemented in
scripts/security_audit_gate.py (i.e., it invokes pip-audit without --strict and
enforces the repository's current acceptance criteria); explicitly list the
pip-audit flags used by scripts/security_audit_gate.py and the CVE/CVSS
thresholds or pass/fail rules so the CLI example matches the gate semantics.
In `@docs/agent-rules/70-release-commit-and-docs.md`:
- Line 19: The frontmatter `tracks:` list in the rule metadata is missing
src/__init__.py; update the `tracks:` entry in
docs/agent-rules/70-release-commit-and-docs.md to include "src/__init__.py"
alongside "pyproject.toml" and "setup.py" so the metadata matches the rule body
that lists the four canonical version files and keeps tooling/review flows
consistent.
In `@docs/agent-rules/INDEX.md`:
- Line 89: Update specfact-cli-modules to adopt the new policy-bundle approach
introduced in specfact-cli by replacing legacy verifier patterns: remove usages
of the `--require-signature` flag and calls to `verify-modules-signature.py` and
instead invoke the `scripts/module-verify-policy.sh` bundle (or its harmonized
flag equivalents) across CI/workflows and scripts; update and refactor
`pre-commit-verify-modules-signature.sh` and `sign-modules.py` to delegate to
the policy bundle, and revise docs `module-signing.md` and
`agent-rules/50-quality-gates-and-review.md` to document the new
`marketplace-06-ci-module-signing` OpenSpec dependency, include a deprecation
timeline for `--require-signature`/verify-modules-signature.py, and add a
migration checklist to ensure CI consistency and phased adoption.
In `@docs/installation/enhanced-analysis-dependencies.md`:
- Line 52: Replace the loose version specifier for pycg in the enhanced-analysis
optional dependencies (the line showing "pycg>=0.0.7") with a pinned version
"pycg==0.0.7" to avoid pulling in the problematic 0.0.8 release, or
alternatively add a short note in
docs/installation/enhanced-analysis-dependencies.md beside the pycg entry
stating that the PyCG project is archived and may require maintenance (include
the affected versions and recommend pinning to 0.0.7). Ensure the change refers
to the existing "pycg>=0.0.7" entry so reviewers can find and update it.
In `@openspec/changes/dep-security-cleanup/design.md`:
- Around line 93-98: The gate design in the spec is out of sync with the shipped
wrapper: update the "Gate design" section so `hatch run security-audit` is
described as invoking the shipped wrapper `python
scripts/security_audit_gate.py` (and explicitly note that it does not use the
`--strict` flag), rather than `pip-audit --desc --strict`; reference the wrapper
filename `security_audit_gate.py` and the hatch task name `security-audit` to
keep OpenSpec traceability aligned with implementation.
In `@openspec/changes/dep-security-cleanup/proposal.md`:
- Around line 95-109: Add explicit cross-repo tracking and a pre-merge task for
verifier-policy updates in the sibling repo (nold-ai/specfact-cli-modules):
search for and update any references to verify-modules-signature.py (workflows,
CI scripts, docs, tests) to reflect the new policy flag bundles, create a
dedicated PR or issue in that repo linking back to this change, and record that
PR/issue link in this proposal before closing to ensure the verifier-policy
follow-up is completed.
In `@openspec/changes/dep-security-cleanup/specs/call-graph-analysis/spec.md`:
- Around line 52-56: The scenario "enhanced-analysis extra contains no GPL
packages" conflicts with the documented pygments GPL exception because
installing specfact-cli[enhanced-analysis] pulls the base runtime (which may
include pygments); update the scenario text to either (a) assert the
enhanced-analysis extra itself contains no GPL packages (i.e., check only
packages brought in by the extra, excluding base runtime), or (b) explicitly
allow the known pygments/GPL exception in the installed environment while still
requiring the call-graph capability to be provided by pycg (MIT); modify the
scenario header and the THEN clause accordingly so it references the
enhanced-analysis extra scope and the allowed pygments exception and still
asserts pycg provides call-graph capability.
In `@openspec/changes/dep-security-cleanup/specs/dep-license-gate/spec.md`:
- Around line 39-42: Update the requirement text under "Requirement: License
exception allowlist file" to remove LGPL from the exception class and reference
only the prohibited family per design; specifically replace the phrase "GPL/LGPL
exception packages" with "GPL/AGPL exception packages" (or "GPL/AGPL packages")
and ensure the description states the project SHALL maintain
scripts/license_allowlist.yaml documenting accepted GPL/AGPL exception packages
with package name, accepted license, and human-readable reason, matching the
design in openspec/changes/dep-security-cleanup/design.md.
- Around line 11-22: The spec scenario "Scenario: GPL package detected outside
allowlist" is out of sync with the implementation: the implemented gate emits
aggregate summaries and lines beginning with "VIOLATION:" while the spec
requires a `LICENSE VIOLATION: <package>==<version> uses <license> which is
incompatible with Apache-2.0` line plus full package inventory. Fix by either
updating this scenario to expect the actual implemented output (change the
expected message to `VIOLATION:` and assert the presence of the aggregate scan
summary and offending package details in that format) or modify the
implementation of the hatch-run license-check command to emit the exact `LICENSE
VIOLATION:` message and the full package/license inventory; refer to the
scenario title "Scenario: GPL package detected outside allowlist", the command
`hatch run license-check`, and the message tokens `VIOLATION:` and `LICENSE
VIOLATION:` to locate where to align behavior.
In `@openspec/changes/dep-security-cleanup/tasks.md`:
- Around line 81-85: Task 10.1 was marked done using the wrong command; uncheck
the checkbox for "10.1" in the tasks.md until you run the exact required command
`hatch test --cover -v`, capture and attach the actual output (including
coverage summary and timestamp) as the verification evidence, and then re-check
10.1 updating its note to show the exact command run, the coverage numbers, and
the verification date; reference the checklist item "10.1" and the file
openspec/changes/dep-security-cleanup/tasks.md when making this update.
In
`@openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md`:
- Around line 60-64: Spec currently mandates that workflow_dispatch invokes
verify-modules-signature.py with VERIFY_MODULES_PR and --version-check-base
origin/<base_branch>, but workflow_dispatch must instead use the resign-all mode
that omits base comparison; update the spec to add a separate Scenario for
workflow_dispatch that invokes verify-modules-signature.py with the resign-all
invocation (e.g., a distinct VERIFY_MODULES_RESIGN_ALL token or flag name) and
does NOT include --version-check-base origin/<base_branch>, or alternately
document the conditional behavior of verify-modules-signature.py to accept a
resign-all mode that skips the --version-check-base parameter (referencing
verify-modules-signature.py, VERIFY_MODULES_PR, --version-check-base
origin/<base_branch>, and resign-all).
In `@pyproject.toml`:
- Around line 274-276: The pyproject.toml entries verify-modules-signature and
verify-modules-signature-pr re-hardcode flag sets that are centralized in
scripts/module-verify-policy.sh; replace these hardcoded command strings so they
defer to the centralized policy script instead (e.g., invoke
module-verify-policy.sh or the canonical wrapper it provides) and remove the
duplicated
--require-signature/--enforce-version-bump/--payload-from-filesystem/--skip-checksum-verification
flags from the two hatch script definitions so CI, pre-commit, docs, and other
workflows consume the single source of truth in scripts/module-verify-policy.sh.
In `@scripts/_detect_modules_to_publish.py`:
- Around line 23-38: The _load_registry_versions function currently calls
json.loads(path.read_text(...)) without handling malformed JSON; wrap that
read+parse in a try/except that catches json.JSONDecodeError and re-raises a
clearer ValueError (including the registry path and the original exception
message) so callers see a diagnostic error instead of a raw stack trace; keep
the existing `@require/`@ensure decorators and preserve returning the
modules->versions dict when parsing succeeds.
- Around line 101-103: The postcondition on the CLI entrypoint is too strict:
remove or relax the `@ensure`(lambda result: result == 0) decorator on the main
function so main can return non-zero error codes; locate the decorated function
main(argv: list[str] | None = None) -> int and either delete the `@ensure`(...)
line entirely or replace it with a relaxed check such as `@ensure`(lambda result:
result >= 0) to allow graceful non-zero exits.
In `@scripts/check_license_compliance.py`:
- Around line 107-110: The loop currently collapses multiple allowlist entries
for the same package into one by using only normalized_entry["package"] as the
dict key; change it to preserve distinct entries by using a composite key that
includes package+scope+license (e.g., normalize and combine
normalized_entry["package"].strip().lower(), normalized_entry["scope"],
normalized_entry["license"] into a single tuple or joined string) when storing
into result in the for loop that calls _validate_allowlist_entry, and adjust the
result container type to map that composite key (or to map package -> list of
entries) so entries are not overwritten and scope/license distinctions are
retained. Ensure normalization (strip/lower) is applied consistently to package,
and keep the rest of normalized_entry as the stored value.
- Around line 339-346: In _iter_manifest_dependencies, ensure malformed
manifests fail by catching YAML parse errors and validating the pip_dependencies
type: wrap yaml.safe_load in a try/except and re-raise a RuntimeError with a
clear manifest-context message on any parsing exception; after loading, check
that manifest is a dict and that manifest_map.get("pip_dependencies") exists and
is a list, and if it's present but not a list (or manifest is not a dict) raise
RuntimeError describing the manifest path and invalid structure; only when
pip_dependencies is a proper list return the filtered list of strings as before.
- Around line 274-276: Replace the ad-hoc split logic in
_normalize_dependency_name with proper parsing using
packaging.requirements.Requirement: construct Requirement(dep) and return its
.name (this handles extras, markers, inequality operators, direct references,
etc.); catch packaging.requirements.InvalidRequirement (or Exception) and raise
a clear ValueError (or custom error) indicating the dependency spec is invalid
so downstream license lookups fail fast and visibly; and add unit tests for
_normalize_dependency_name covering inputs like "foo<2", "foo!=1.0",
"foo[extra]>=1", environment markers, and direct references to ensure correct
behavior and explicit failures.
In `@scripts/check_version_sources.py`:
- Around line 96-113: The function _version_changed_vs_head actually checks for
a strict version bump (returns True only when current > HEAD), so rename it to a
clearer identifier like _version_bumped_vs_head (or
_version_incremented_vs_head) and update all usages accordingly; change the
function name, any imports or references (call sites within the module or
tests), and update or add a brief docstring/comment on the function to state it
detects a strict version increase versus HEAD to avoid semantic confusion.
- Around line 78-89: The try/except in _read_file_at_git_ref currently catches
Exception broadly and can hide unexpected errors; change it to catch
subprocess.CalledProcessError (and optionally FileNotFoundError if git may be
missing) so expected "ref/file not found" failures return None while other
exceptions are allowed to propagate; update the except block for subprocess.run
in _read_file_at_git_ref to catch those specific exceptions and return None,
leaving any other errors unhandled (or re-raise) for proper debugging.
- Around line 52-63: In _staged_files, replace the broad except Exception block
with targeted exception handling: catch subprocess.CalledProcessError (when git
returns non-zero) and FileNotFoundError (when git is missing) and handle them
appropriately (e.g., print a clear error to stderr and return an empty list),
and for any other unexpected exception re-raise or log it to stderr so failures
aren’t silently swallowed; update references around subprocess.run calls in
_staged_files accordingly.
In `@scripts/license_allowlist.yaml`:
- Around line 24-29: The allowlist entry for package "pygments" in
scripts/license_allowlist.yaml incorrectly lists license: GPL-2.0-or-later; open
the file and either remove the entire "pygments" entry or update its license
field to "BSD-2-Clause" (keeping the existing reason and scope if you want to
retain the audit comment), ensuring the package: pygments entry no longer
misclassifies the license.
In `@scripts/pre_commit_code_review.py`:
- Around line 129-137: The repo discovery loop uses a fixed 12-iteration cap
(for _ in range(12)) which can miss valid ancestors; replace it with an
unbounded traversal that stops only when the filesystem root is reached: iterate
while True (or while here != here.parent) and check candidate = here /
"specfact-cli-modules" and marker = candidate / "packages" / "specfact-codebase"
as before, returning candidate.resolve() if found, break when here ==
here.parent to avoid infinite loops, then return None if not found; update the
block that currently references candidate, marker, and here accordingly.
In `@scripts/security_audit_gate.py`:
- Around line 108-116: The code assumes the parsed JSON is a dict and calls
data.get("dependencies") which will raise AttributeError for list/scalar roots;
update the post-parse validation in the block after json.loads(raw) to check
that data is an instance of dict (e.g., if not isinstance(data, dict):
_emit("ERROR: pip-audit JSON missing or invalid root object", error=True);
return None, 1) before calling data.get("dependencies") so non-object roots are
handled like other malformed responses; keep using the existing _emit and return
(None, 1) behavior to fail closed.
In `@scripts/sign-modules.py`:
- Around line 414-429: Reject an invalid --base-ref early: before iterating
manifests in _apply_version_only_remediation, validate args.base_ref by calling
the same check/path used by _auto_bump_manifest_version (or directly invoking
the git lookup used there) and if it fails, log an error and return non-zero so
the CLI fails rather than silently no-op; keep the remaining loop calling
_maybe_bump_manifest_version unchanged (references:
_apply_version_only_remediation, _maybe_bump_manifest_version,
_auto_bump_manifest_version, args.base_ref, args.bump_version,
_resolve_manifests).
In `@scripts/verify-modules-signature.py`:
- Around line 286-295: The early return when verify_checksum is False lets
manifest integrity checks be completely skipped; change the logic so that when
verify_checksum is False you do not return immediately but still read and
validate the manifest's integrity/signature fields (e.g., inspect the
"integrity" entry from manifest_path after yaml.safe_load) unless an explicit
re-sign/allow-resign flag is passed; if you introduce an explicit flag (e.g.,
allow_resign or force_resign) only then allow the short-circuit return, and
mirror the same fix for the similar block referenced around lines 343-348 so
both code paths enforce checksum/signature presence for PR verification unless
the explicit re-sign workflow is requested.
In `@src/specfact_cli/analyzers/graph_analyzer.py`:
- Around line 119-128: The code assumes json.loads(...) returns dict[str,
list[str]] but doesn't validate shape; update the parsing path (e.g., implement
or call _parse_pycg_json) to validate that raw is a dict whose keys are strings
and whose values are lists of strings, returning {} on any mismatch or parse
error; keep the existing try/except around json.loads but after loading check
isinstance(raw, dict) and for each (caller, callees) ensure isinstance(caller,
str) and isinstance(callees, list) and every callee is a str before populating
call_graph (otherwise return {}), so malformed pycg output or wrong JSON shape
degrades gracefully without raising.
- Around line 78-95: extract_call_graph() currently assumes subprocess.run(...)
always succeeds; if pycg is removed or becomes non-executable the call will
raise (e.g., FileNotFoundError/PermissionError) and bubble up instead of
returning the documented {} fallback. Surround the subprocess.run invocation
(and the subsequent return-on-success logic) with an except block that catches
runtime failures from launching pycg (FileNotFoundError, PermissionError,
subprocess.SubprocessError or a broad Exception), log or ignore the error,
ensure the finally block still removes json_path, and return {} when a launch
error occurs so callers get the documented fallback; keep existing behavior of
parsing via _parse_pycg_json and updating self.call_graphs when returncode == 0.
In `@src/specfact_cli/modules/module_registry/module-package.yaml`:
- Line 2: The module-package.yaml version was bumped to 0.1.19 but the integrity
block (integrity.checksum and integrity.signature) is stale; re-run the module
signing flow to regenerate the integrity fields and commit the updated manifest.
From the repo root run the signing script (scripts/sign-modules.py
--modules-root src/specfact_cli/modules) to update integrity.checksum and
integrity.signature for module-package.yaml (and any other affected manifests),
then stage and commit the updated file(s).
In `@src/specfact_cli/utils/optional_deps.py`:
- Around line 7-8: Update the two places in
src/specfact_cli/utils/optional_deps.py that mention the pycg license: locate
the module docstring near the top and the inline/comment occurrence around the
code that references pycg (search for the literal "pycg"), and change the
license label from "MIT" to "Apache-2.0" so the docstring and comment match
pyproject.toml's dependency manifest; ensure both occurrences are updated to the
exact string "Apache-2.0".
In `@src/specfact_cli/utils/project_artifact_write.py`:
- Line 107: The variable assigned from commentjson.loads(raw_text) should be
given an explicit type to satisfy strict type checking: change the assignment to
annotate loaded as Any (e.g., loaded: Any = commentjson.loads(raw_text)) so
downstream code (the isinstance(loaded, dict) guard) type-checks correctly; also
add an import for Any from typing if it's not already present.
In `@tests/unit/specfact_cli/registry/test_signing_artifacts.py`:
- Around line 502-507: The test
test_sign_modules_workflow_dispatch_resign_all_skips_version_check_base
currently only asserts the presence of '--version-check-base' anywhere in
SIGN_WORKFLOW raw text; narrow the check to the resign-all branch by extracting
the workflow block that handles github.event.inputs.resign_all_manifests (e.g.,
find the YAML/job/if: block or the step that runs 'python
scripts/verify-modules-signature.py "${VERIFY_ARGS[@]}"' when resign_all is
true) and assert that within that resign-all branch the string
'--version-check-base "$BASE_REF"' is NOT present (and optionally assert that
the non-resign branch still contains it) so the test truly verifies the
skip-base-ref behavior.
In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 242-249: Expand the test to assert the dependency jobs themselves
instead of only package-validation.needs: after loading jobs (jobs =
_load_jobs()) fetch license_job = jobs.get("license-check") and security_job =
jobs.get("security-audit") and assert they are not None, then assert each job's
gating condition (e.g., inspect license_job.get("if") and security_job.get("if")
or equivalent condition field) contains the expected gate expressions such as
"license_inputs_changed" and the release-skip logic so the test fails if those
jobs are accidentally gated off or stop honoring their inputs/skip conditions.
---
Outside diff comments:
In @.github/workflows/sign-modules.yml:
- Around line 117-138: The workflow still passes VERIFY_ARGS (populated from
VERIFY_MODULES_PR) which can contain --enforce-version-bump or
--version-check-base, causing the verifier to enforce a version check even when
resign_all_manifests=true; update the branch that handles "${{
github.event.inputs.resign_all_manifests }}" to sanitize VERIFY_ARGS by removing
any occurrences of "--enforce-version-bump" and any "--version-check-base <arg>"
entries (or otherwise rebuild VERIFY_ARGS without those flags) before calling
python scripts/verify-modules-signature.py so the verifier runs without a
version-base enforcement when resign_all_manifests is true.
In `@src/specfact_cli/registry/module_installer.py`:
- Around line 1005-1031: The loop currently picks MARKETPLACE_MODULES_ROOT
before USER_MODULES_ROOT when source == "marketplace", which can delete a legacy
copy instead of the canonical user install; change the branch in
module_installer.py that sets candidate_roots for source == "marketplace" so it
uses [USER_MODULES_ROOT, MARKETPLACE_MODULES_ROOT] (mirroring the other branch)
so the code will probe and uninstall the canonical USER_MODULES_ROOT first; keep
the rest of the logic (the _path_is_under_user_modules_install_tree check,
confirm_user_scope/env flag handling, shutil.rmtree and early return) unchanged.
🪄 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: ca6f7aa3-b89a-4764-ae69-e32159e9ead8
📒 Files selected for processing (70)
.github/workflows/pr-orchestrator.yml.github/workflows/publish-modules.yml.github/workflows/sign-modules.yml.pre-commit-config.yamlCHANGELOG.mdCONTRIBUTING.mdREADME.mdSECURITY.mddocs/agent-rules/50-quality-gates-and-review.mddocs/agent-rules/55-dependency-hygiene.mddocs/agent-rules/70-release-commit-and-docs.mddocs/agent-rules/INDEX.mddocs/guides/module-signing-and-key-rotation.mddocs/guides/publishing-modules.mddocs/installation/enhanced-analysis-dependencies.mddocs/reference/module-security.mdopenspec/CHANGE_ORDER.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.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/_detect_modules_to_publish.pyscripts/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/sign-modules.pyscripts/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/code_analyzer.pysrc/specfact_cli/analyzers/graph_analyzer.pysrc/specfact_cli/modules/module_registry/module-package.yamlsrc/specfact_cli/modules/module_registry/src/commands.pysrc/specfact_cli/registry/module_installer.pysrc/specfact_cli/utils/optional_deps.pysrc/specfact_cli/utils/project_artifact_write.pytests/integration/scripts/test_check_local_version_ahead_of_pypi_integration.pytests/unit/analyzers/test_graph_analyzer.pytests/unit/registry/test_module_installer.pytests/unit/scripts/test_check_license_compliance.pytests/unit/scripts/test_check_local_version_ahead_of_pypi.pytests/unit/scripts/test_check_version_sources.pytests/unit/scripts/test_module_verify_policy.pytests/unit/scripts/test_pre_commit_code_review.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: 3
♻️ Duplicate comments (3)
.github/workflows/publish-modules.yml (1)
268-290:⚠️ Potential issue | 🔴 CriticalCritical: stop interpolating workflow outputs directly into bash here-strings
Line 290 and Line 324 both embed
${{ ... }}output into shell source viadone <<< "...". That is a command-injection vector because Actions expression content is materialized before bash parses the script.Use file-backed iteration (or another non-evaluated transport) for both loops.
🔒 Suggested fix
- name: Detect modules whose version is ahead of the registry id: detect run: | @@ { echo "modules<<EOF" cat /tmp/modules_to_publish.txt echo "EOF" } >> "$GITHUB_OUTPUT" @@ - name: Package, sign, and stage registry entries id: stage if: steps.detect.outputs.modules != '' run: | set -euo pipefail mkdir -p dist + : > /tmp/published_modules.txt PY_ENTRY_SUMMARY='import sys, yaml; from pathlib import Path; data = yaml.safe_load(Path(sys.argv[1]).read_text(encoding="utf-8")); print(f"{data[\"id\"]}@{data[\"latest_version\"]}")' PUBLISHED=() while IFS= read -r MODULE_DIR; do @@ if [ "${CHANGED}" = "true" ]; then ENTRY="$(python -c "${PY_ENTRY_SUMMARY}" "${FRAGMENT}")" PUBLISHED+=("${ENTRY}") + printf '%s\n' "${ENTRY}" >> /tmp/published_modules.txt fi - done <<< "${{ steps.detect.outputs.modules }}" + done < /tmp/modules_to_publish.txt @@ - name: Create combined registry PR if: steps.stage.outputs.any_changed == 'true' env: GH_TOKEN: ${{ env.SPECFACT_MODULES_REPO_TOKEN }} run: | @@ echo "Modules published:" while IFS= read -r line; do [ -n "${line}" ] || continue echo "- \`${line}\`" - done <<< "${{ steps.stage.outputs.published }}" + done < /tmp/published_modules.txt } > /tmp/pr_body.mdAs per coding guidelines, ".github/workflows/**: CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test gates, and action versions."
Also applies to: 321-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-modules.yml around lines 268 - 290, The loop currently uses a bash here-string with an Actions expression (done <<< "${{ steps.detect.outputs.modules }}") which can allow command injection; change both occurrences to write the workflow output to a safe tempfile and iterate from that file instead: create a temp file (mktemp), echo or redirect the value of steps.detect.outputs.modules into it in the workflow (or use run: printf '%s\n' "${{ steps.detect.outputs.modules }}" > "$TMP"), then replace the loop terminator with done < "$TMP" and ensure the tempfile is removed after use; update the while loop blocks (the one reading MODULE_DIR and the other similar loop at lines around 321-324) so they read from the temp file rather than using the here-string, leaving all other logic (SIGN key check, python scripts, CHANGED handling, PUBLISHED array) unchanged.CHANGELOG.md (2)
67-68:⚠️ Potential issue | 🟡 MinorAdd a section separator before the next release block.
Insert
---between Line 67 and Line 68 to keep release entries visually and structurally separated.Suggested patch
- step. + +--- ## [0.46.2] - 2026-04-15🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 67 - 68, Add a Markdown section separator between release entries: insert a line containing three hyphens (`---`) immediately after the "## [0.46.2] - 2026-04-15" heading so the release block is visually and structurally separated from the following content.
30-38:⚠️ Potential issue | 🟡 MinorPromote removals to a dedicated
### Removedsection.Line 31 currently records substantive removals under
### Changed; this should be a first-class### Removedsubsection in the0.46.3entry.Suggested patch
### Changed - **Dependency hygiene (`dep-security-cleanup`)**: - - **Removed** GPL/wrong-PyPI packages from distributed extras: - `pyan3` (GPL-2.0; replaced by `pycg` MIT), `bearer` (wrong PyPI; - replaced by `bandit` MIT), `syft` (wrong PyPI; Anchore Syft is - out-of-band). - **Replaced** runtime `json5` with `commentjson` (read) + stdlib `json` (write). - **Added** `pycg`, `bandit`, `pip-licenses`, and `pip-audit` to the appropriate extras. + +### Removed + +- GPL/wrong-PyPI packages from distributed extras: + `pyan3` (replaced by `pycg`), `bearer` (replaced by `bandit`), and + `syft` (Anchore Syft remains out-of-band).As per coding guidelines,
CHANGELOG.mdshould document significant changes under explicitAdded,Fixed,Changed, orRemovedsections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 30 - 38, Create a new "### Removed" subsection under the 0.46.3 entry and move the bullet that lists the substantive removals into it: the removal of `pyan3` (GPL-2.0), `bearer` (wrong PyPI), and `syft` (out-of-band). Keep the replacement note about runtime `json5` -> `commentjson` (read) + stdlib `json` and the bullets that add `pycg`, `bandit`, `pip-licenses`, and `pip-audit` in their appropriate `### Changed`/`### Added` sections, so only the removal lines from the `dep-security-cleanup` block are promoted into the new `### Removed` subsection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 46-51: Add an explicit bullet under the 0.46.3 release notes
calling out the new module-signature/version-bump policy: update the section
that currently contains the "Pre-commit / CI: `check-version-sources`..." bullet
to include a short “Migration/Compatibility” note stating that module signatures
and version bumps are now enforced when signed module assets or manifests change
(suggested phrasing: “Enforce module signatures and version bumps when signed
module assets or manifests are affected.”) so consumers know to adjust
pre-commit/workflow flags and CI when upgrading.
In `@scripts/security_audit_gate.py`:
- Around line 82-84: The current helper _cvss_for_vuln silently returns 0.0 for
advisories with no numeric CVSS, which downgrades unknown severity; change
_cvss_for_vuln to fail-fast by raising a clear exception (e.g., ValueError or a
custom UnscoredVulnerabilityError) when _gather_cvss_scores(vuln) returns no
numeric scores, and update the caller logic in the vulnerability
aggregation/decision code (the loop/handler around lines 148–182 that consumes
_cvss_for_vuln) to catch/propagate that exception and cause the gate to fail
(exit non-zero or mark unresolved) instead of treating the finding as
informational. Ensure the error message includes the vulnerability id/name to
aid debugging.
In `@scripts/sign-modules.py`:
- Around line 217-226: The verify-modules-signature.py implementation uses
manifest_path.as_posix() directly which can vary by CWD; update its
_read_manifest_version_from_git (or the function that constructs the git ref for
manifest_path) to match sign-modules.py: resolve repo_root =
Path.cwd().resolve(), git_path = path.resolve(), attempt relative_path =
git_path.relative_to(repo_root).as_posix() with a fallback to path.as_posix(),
and then use that relative_path when calling git show so path handling is
consistent with sign-modules.py.
---
Duplicate comments:
In @.github/workflows/publish-modules.yml:
- Around line 268-290: The loop currently uses a bash here-string with an
Actions expression (done <<< "${{ steps.detect.outputs.modules }}") which can
allow command injection; change both occurrences to write the workflow output to
a safe tempfile and iterate from that file instead: create a temp file (mktemp),
echo or redirect the value of steps.detect.outputs.modules into it in the
workflow (or use run: printf '%s\n' "${{ steps.detect.outputs.modules }}" >
"$TMP"), then replace the loop terminator with done < "$TMP" and ensure the
tempfile is removed after use; update the while loop blocks (the one reading
MODULE_DIR and the other similar loop at lines around 321-324) so they read from
the temp file rather than using the here-string, leaving all other logic (SIGN
key check, python scripts, CHANGED handling, PUBLISHED array) unchanged.
In `@CHANGELOG.md`:
- Around line 67-68: Add a Markdown section separator between release entries:
insert a line containing three hyphens (`---`) immediately after the "##
[0.46.2] - 2026-04-15" heading so the release block is visually and structurally
separated from the following content.
- Around line 30-38: Create a new "### Removed" subsection under the 0.46.3
entry and move the bullet that lists the substantive removals into it: the
removal of `pyan3` (GPL-2.0), `bearer` (wrong PyPI), and `syft` (out-of-band).
Keep the replacement note about runtime `json5` -> `commentjson` (read) + stdlib
`json` and the bullets that add `pycg`, `bandit`, `pip-licenses`, and
`pip-audit` in their appropriate `### Changed`/`### Added` sections, so only the
removal lines from the `dep-security-cleanup` block are promoted into the new
`### Removed` subsection.
🪄 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: a6a15eb0-e1c6-44d0-83ee-23e20a499eee
📒 Files selected for processing (5)
.github/workflows/publish-modules.ymlCHANGELOG.mdscripts/security_audit_gate.pyscripts/sign-modules.pytests/unit/scripts/test_security_audit_gate.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Socket Security: Pull Request Alerts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.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:
scripts/sign-modules.pytests/unit/scripts/test_security_audit_gate.pyscripts/security_audit_gate.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:
scripts/sign-modules.pytests/unit/scripts/test_security_audit_gate.pyscripts/security_audit_gate.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/sign-modules.pyscripts/security_audit_gate.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/publish-modules.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/publish-modules.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/publish-modules.yml
**/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/scripts/test_security_audit_gate.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/scripts/test_security_audit_gate.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/scripts/test_security_audit_gate.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/scripts/test_security_audit_gate.py
**/*.{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:
CHANGELOG.md
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
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
CHANGELOG.md
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
📚 Learning: 2026-04-10T22:42:21.860Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
scripts/sign-modules.py.github/workflows/publish-modules.yml
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/publish-modules.yml
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/publish-modules.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 .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing
Applied to files:
.github/workflows/publish-modules.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 .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:
.github/workflows/publish-modules.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:
tests/unit/scripts/test_security_audit_gate.py
📚 Learning: 2026-03-31T22:38:25.299Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-31T22:38:25.299Z
Learning: Applies to tests/**/*.py : Secret redaction via `LoggerSetup.redact_secrets` must be covered by unit tests
Applied to files:
tests/unit/scripts/test_security_audit_gate.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
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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:
CHANGELOG.md
🪛 GitHub Check: CodeQL
.github/workflows/publish-modules.yml
[failure] 290-290: Code injection
Potential code injection in ${{ steps.detect.outputs.modules }}, which may be controlled by an external user (workflow_dispatch).
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Consumers of verify-modules-signature and related flags:
- scripts/pre-commit-verify-modules-signature.sh — builds/executes verify command with hard-coded flags (
--payload-from-filesystem --enforce-version-bump) and is referenced by pre-commit hooks. [::nold-ai/specfact-cli-modules::scripts/pre-commit-verify-modules-signature.sh:31] - scripts/verify-modules-signature.py — present in scripts and invoked widely. [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py]
- scripts/sign-modules.py — signing flow interacts with verification expectations. [::nold-ai/specfact-cli-modules::scripts/sign-modules.py]
- .github/workflows/pr-orchestrator.yml — workflow content asserted in tests to include
scripts/verify-modules-signature.pyand append--require-signaturefor main-targeted PRs. [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml] - .github/workflows/sign-modules.yml / sign-modules-on-approval.yml — workflows that run verification/signing steps. [::nold-ai/specfact-cli-modules::.github/workflows/sign-modules.yml][::nold-ai/specfact-cli-modules::.github/workflows/sign-modules-on-approval.yml]
- pyproject.toml
verify-modules-signatureentrypoint scripts (package script). [::nold-ai/specfact-cli-modules::pyproject.toml:59] - README.md and multiple docs reference explicit flag combinations (examples and operator guidance). [::nold-ai/specfact-cli-modules::README.md]
- Numerous docs under docs/ and openspec/ reference/verbatim the prior flag semantics (
--require-signature,--payload-from-filesystem,--enforce-version-bump). Examples:- docs/reference/module-security.md [::nold-ai/specfact-cli-modules::docs/reference/module-security.md]
- docs/authoring/module-signing.md [::nold-ai/specfact-cli-modules::docs/authoring/module-signing.md]
- docs/guides/* and openspec/* (many files) — several assertions/tests expect the former CLI flags/branch-behavior. [::nold-ai/specfact-cli-modules::docs][::nold-ai/specfact-cli-modules::openspec]
- Tests assert workflow/script contents and flag usage:
- tests/unit/workflows/test_pr_orchestrator_signing.py (expects VERIFY_CMD to be appended with
--require-signatureon main) [::nold-ai/specfact-cli-modules::tests/unit/workflows/test_pr_orchestrator_signing.py] - tests/unit/workflows/test_sign_modules_hardening.py (expects
--require-signature) [::nold-ai/specfact-cli-modules::tests/unit/workflows/test_sign_modules_hardening.py] - tests/unit/test_pre_commit_verify_modules_signature_script.py (pre-commit script expectations re: flags) [::nold-ai/specfact-cli-modules::tests/unit/test_pre_commit_verify_modules_signature_script.py]
- other tests reference the verifier in test fixtures and contract tests. [::nold-ai/specfact-cli-modules::tests]
- tests/unit/workflows/test_pr_orchestrator_signing.py (expects VERIFY_CMD to be appended with
- scripts/pre-commit-verify-modules-signature.sh — builds/executes verify command with hard-coded flags (
-
No occurrences found for the new license/security gate scripts or config (e.g., scripts/check_license_compliance.py, scripts/security_audit_gate.py, scripts/module-verify-policy.sh) in this repo’s files/search output — the repo remains tightly coupled to the existing verify-modules-signature CLI and explicit flag usage. (Search returned no lines for module-verify-policy.sh or check_license_compliance/security_audit_gate.) [::nold-ai/specfact-cli-modules::]
Impact summary:
- This repository is a high-consumption client of the module verification CLI and of workflows/docs/tests that expect specific flag combinations and branch-driven
--require-signaturebehavior. The PR’s change to drive verification via a sourced policy (module-verify-policy.sh), add new flags like--skip-checksum-verification, and change orchestration wiring will likely require coordinated updates here to:- workflow YAML and tests that assert exact flag strings and conditionals,
- scripts that build VERIFY_CMD (pre-commit script),
- documentation/examples that show explicit flag usage,
- any package script entries (pyproject) or tests that invoke the CLI with exact args.
🔇 Additional comments (8)
tests/unit/scripts/test_security_audit_gate.py (1)
35-112: Good coverage of the gate’s CLI contract and fail-closed behavior.These cases lock in the accepted JSON shapes, the empty-stdout/OSError failure paths, and the
--skip-editablewithout--strictinvocation that CI depends on.scripts/security_audit_gate.py (1)
87-136: Nice hardening of the pip-audit JSON parsing path.Accepting both documented shapes and failing closed on anything else removes the old crash path while preserving a deterministic gate outcome.
scripts/sign-modules.py (6)
421-429: Invalid--base-refvalidation gap when explicit manifests are provided.When explicit manifest paths are passed (without
--changed-only),_resolve_manifestsreturns early without validatingargs.base_ref. If the ref is invalid,_auto_bump_manifest_versionsilently treats the git lookup failure as "version unchanged," returningFalseand exiting0. This could cause release automation to incorrectly interpret a failed bump as a successful no-op.
201-203: Good centralization of encrypted-key detection logic.Extracting this predicate avoids duplicating the string-matching heuristic in multiple branches. The function is well-named and clear in intent.
414-419: Clean extraction of conditional version-bump logic.This helper consolidates the "bump if requested" check, reducing duplication between the version-only and signing code paths. Single responsibility is maintained.
432-436: Solid CLI constraint enforcement.The mutual exclusion checks prevent invalid flag combinations early, with clear error messages. Requiring
--bump-versionfor--version-onlyensures the mode has a defined action, and disallowing--allow-unsignedavoids confusion about signing behavior.
439-450: Fix applied: explicit return satisfies static analysis.The
return Noneon line 449 afterparser.error()addresses the mixed implicit/explicit return concern. While technically unreachable (sinceparser.errorcallssys.exit), this explicit return satisfies type checkers and static analysis tools.
526-537: Well-structured integration of--version-onlymode.The early validation followed by conditional early return avoids unnecessary private-key resolution when only version bumping is needed. The wiring aligns with the documented use case in
pre-commit-verify-modules.sh(auto-bumping before deferred signature verification).Per the retrieved learning about enforcing module signatures when manifests are affected: this mode intentionally defers signing to CI, which is acceptable as long as the workflow guarantees signing occurs before merge. The existing workflow at
sign-modules-on-approval.yml(context snippet 3) continues to use--bump-versionwithout--version-only, so signing flows remain intact.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
CHANGELOG.md (3)
75-76:⚠️ Potential issue | 🟡 MinorAdd a separator before the next release block.
Insert a horizontal rule between the
0.46.3section end and Line 76 (## [0.46.2]) to keep release blocks structurally separated and scannable.As per coding guidelines, changelog entries should remain clearly structured and separated for release-history readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 75 - 76, The changelog is missing a separator between release blocks; add a horizontal rule (e.g., a line with ---) immediately before the existing "## [0.46.2] - 2026-04-15" header so that the previous "## [0.46.3]" section is visually separated—edit CHANGELOG.md to insert the separator line between the end of the 0.46.3 block and the "## [0.46.2]" header to match the project's changelog formatting guidelines.
46-51:⚠️ Potential issue | 🟠 MajorAdd an explicit migration note for module signature/version-bump enforcement.
Line 46 summarizes CI checks, but this release also changes module verification policy in ways that impact downstream workflows/tests (notably cross-repo consumers expecting specific flag behavior). Add a short compatibility bullet explicitly stating signature + version-bump enforcement when signed module assets/manifests change.
Proposed patch
- **Pre-commit / CI**: `check-version-sources` always runs; PyPI-ahead check matches orchestrator tests job when version sources change (`pyproject.toml`, `setup.py`, `src/__init__.py`, `src/specfact_cli/__init__.py`; lenient network), with remediation hints on failure. +- **Migration / compatibility**: Enforce module signatures and version + bumps when signed module assets or manifests are affected; consumers + should align pre-commit/workflow verifier flags accordingly.Based on learnings, “Enforce module signatures and version bumps when signed module assets or manifests are affected.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 46 - 51, Update the CHANGELOG.md entry around the "Pre-commit / CI: check-version-sources" bullet to add a short compatibility/migration note that explicitly states the new enforcement: "Enforce module signatures and require version bumps when signed module assets or manifests are modified." Mention the affected check name (check-version-sources) and that this impacts downstream/cross-repo consumers and tests expecting previous flag behavior, so they must bump versions and re-sign modules when signed assets/manifests change.
30-38:⚠️ Potential issue | 🟡 MinorSplit removals into a dedicated
### Removedsubsection.At Line 31, removals are currently nested under
### Changed; move these into a top-level### Removedsection in the0.46.3entry for release governance consistency.Proposed patch
### Changed - **Dependency hygiene (`dep-security-cleanup`)**: - - **Removed** GPL/wrong-PyPI packages from distributed extras: - `pyan3` (GPL-2.0; replaced by `pycg` MIT), `bearer` (wrong PyPI; - replaced by `bandit` MIT), `syft` (wrong PyPI; Anchore Syft is - out-of-band). - **Replaced** runtime `json5` with `commentjson` (read) + stdlib `json` (write). - **Added** `pycg`, `bandit`, `pip-licenses`, and `pip-audit` to the appropriate extras. + +### Removed + +- GPL/wrong-PyPI packages from distributed extras: + `pyan3` (replaced by `pycg`), `bearer` (replaced by `bandit`), and + `syft` (Anchore Syft remains out-of-band).As per coding guidelines,
CHANGELOG.mdversion changes should classify significant removals under explicitAdded,Fixed,Changed, orRemovedsections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 30 - 38, In the 0.46.3 changelog entry, extract the removal points currently nested under the "### Changed" subsection and create a top-level "### Removed" subsection for them; specifically move the bullet that begins with "Removed GPL/wrong-PyPI packages from distributed extras: `pyan3`..., `bearer`..., `syft`..." (and any other explicit "Removed" lines such as the `json5` removal if labeled as removal) out of the `### Changed` block and place it under the new `### Removed` header, leaving `### Changed` to only contain actual changes and keeping `### Added` entries (e.g., `pycg`, `bandit`, `pip-licenses`, `pip-audit`) as they are.
🤖 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/publish-modules.yml:
- Around line 187-190: The current workflow_run condition allows runs triggered
by workflow_dispatch; restrict auto-publish to only push-originated hardening
runs by adding a check for the originating event
(github.event.workflow_run.event == 'push') to the existing if expression
(alongside github.event_name == 'workflow_run' and the head_branch checks) so
only push-triggered workflow_run successes on dev/main can auto-publish.
- Around line 202-206: The auto-publish job currently checks out
github.event.workflow_run.head_sha which may be behind the branch tip after
sign-modules.yml pushed version bumps, so add a sync step before running
_detect_modules_to_publish.py: after the actions/checkout@v4 step, fetch the
remote branch (using the branch ref from the workflow, e.g. HEAD_BRANCH or
github.event.workflow_run.head_branch) and reset the working tree to
origin/<branch> so the working copy reflects the latest pushed commits (this
ensures _detect_modules_to_publish.py sees bumped manifest versions and
auto-publish will consider those modules).
In `@scripts/publish-module.py`:
- Around line 91-102: The function _official_nold_publisher_manifest currently
treats any manifest whose publisher.url contains
OFFICIAL_MODULES_REPO_URL_MARKER as official, which allows crafted query strings
to bypass namespace enforcement; update the check to parse the URL (e.g., with
urllib.parse.urlparse), validate the scheme is http/https, ensure the netloc
matches the expected repo host (e.g., endswith the official host like
github.com) and that the path begins with the official organization prefix
(e.g., path startswith "/nold-ai/" or "/nold-ai" + "/"), and only then return
True; keep the existing exact-email check using OFFICIAL_PUBLISHER_EMAIL as-is
so both email and a strictly validated repo URL are acceptable.
In `@tests/unit/scripts/test_publish_module_namespace.py`:
- Around line 23-94: Add a regression test ensuring a publisher.url that only
contains the official marker in its query or fragment does not mark the manifest
official: create a new test (e.g.,
test_official_publisher_url_marker_in_query_fragment_is_ignored) that loads the
module via _load_script_module(), builds a manifest with publisher.url set to
something like "https://example.com/?marker=nold-ai" and/or
"https://example.com/#nold-ai", assert
_official_nold_publisher_manifest(manifest) is False, and then assert
mod._validate_namespace_for_marketplace(manifest, Path("/tmp/foo")) raises
ValueError (match "namespace/name"); reference _official_nold_publisher_manifest
and _validate_namespace_for_marketplace to locate the logic to be protected.
---
Duplicate comments:
In `@CHANGELOG.md`:
- Around line 75-76: The changelog is missing a separator between release
blocks; add a horizontal rule (e.g., a line with ---) immediately before the
existing "## [0.46.2] - 2026-04-15" header so that the previous "## [0.46.3]"
section is visually separated—edit CHANGELOG.md to insert the separator line
between the end of the 0.46.3 block and the "## [0.46.2]" header to match the
project's changelog formatting guidelines.
- Around line 46-51: Update the CHANGELOG.md entry around the "Pre-commit / CI:
check-version-sources" bullet to add a short compatibility/migration note that
explicitly states the new enforcement: "Enforce module signatures and require
version bumps when signed module assets or manifests are modified." Mention the
affected check name (check-version-sources) and that this impacts
downstream/cross-repo consumers and tests expecting previous flag behavior, so
they must bump versions and re-sign modules when signed assets/manifests change.
- Around line 30-38: In the 0.46.3 changelog entry, extract the removal points
currently nested under the "### Changed" subsection and create a top-level "###
Removed" subsection for them; specifically move the bullet that begins with
"Removed GPL/wrong-PyPI packages from distributed extras: `pyan3`...,
`bearer`..., `syft`..." (and any other explicit "Removed" lines such as the
`json5` removal if labeled as removal) out of the `### Changed` block and place
it under the new `### Removed` header, leaving `### Changed` to only contain
actual changes and keeping `### Added` entries (e.g., `pycg`, `bandit`,
`pip-licenses`, `pip-audit`) as they are.
🪄 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: 9b7b2f97-d55b-4a87-bd16-eebe61121c5e
📒 Files selected for processing (4)
.github/workflows/publish-modules.ymlCHANGELOG.mdscripts/publish-module.pytests/unit/scripts/test_publish_module_namespace.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Socket Security: Pull Request Alerts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.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:
scripts/publish-module.pytests/unit/scripts/test_publish_module_namespace.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:
scripts/publish-module.pytests/unit/scripts/test_publish_module_namespace.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/publish-module.py
**/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/scripts/test_publish_module_namespace.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/scripts/test_publish_module_namespace.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/scripts/test_publish_module_namespace.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/scripts/test_publish_module_namespace.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/publish-modules.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/publish-modules.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/publish-modules.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:
CHANGELOG.md
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
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
CHANGELOG.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/publish-modules.ymlCHANGELOG.md
📚 Learning: 2026-04-10T22:42:21.860Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
.github/workflows/publish-modules.ymlCHANGELOG.md
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/publish-modules.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 .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing
Applied to files:
.github/workflows/publish-modules.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 .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:
.github/workflows/publish-modules.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 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
📚 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
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Consumers of verify-modules-signature and related flags:
- scripts/pre-commit-verify-modules-signature.sh — builds/executes verify command with hard-coded flags (
--payload-from-filesystem --enforce-version-bump) and is referenced by pre-commit hooks. [::nold-ai/specfact-cli-modules::scripts/pre-commit-verify-modules-signature.sh:31] - scripts/verify-modules-signature.py — present in scripts and invoked widely. [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py]
- scripts/sign-modules.py — signing flow interacts with verification expectations. [::nold-ai/specfact-cli-modules::scripts/sign-modules.py]
- .github/workflows/pr-orchestrator.yml — workflow content asserted in tests to include
scripts/verify-modules-signature.pyand append--require-signaturefor main-targeted PRs. [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml] - .github/workflows/sign-modules.yml / sign-modules-on-approval.yml — workflows that run verification/signing steps. [::nold-ai/specfact-cli-modules::.github/workflows/sign-modules.yml][::nold-ai/specfact-cli-modules::.github/workflows/sign-modules-on-approval.yml]
- pyproject.toml
verify-modules-signatureentrypoint scripts (package script). [::nold-ai/specfact-cli-modules::pyproject.toml:59] - README.md and multiple docs reference explicit flag combinations (examples and operator guidance). [::nold-ai/specfact-cli-modules::README.md]
- Numerous docs under docs/ and openspec/ reference/verbatim the prior flag semantics (
--require-signature,--payload-from-filesystem,--enforce-version-bump). Examples:- docs/reference/module-security.md [::nold-ai/specfact-cli-modules::docs/reference/module-security.md]
- docs/authoring/module-signing.md [::nold-ai/specfact-cli-modules::docs/authoring/module-signing.md]
- docs/guides/* and openspec/* (many files) — several assertions/tests expect the former CLI flags/branch-behavior. [::nold-ai/specfact-cli-modules::docs][::nold-ai/specfact-cli-modules::openspec]
- Tests assert workflow/script contents and flag usage:
- tests/unit/workflows/test_pr_orchestrator_signing.py (expects VERIFY_CMD to be appended with
--require-signatureon main) [::nold-ai/specfact-cli-modules::tests/unit/workflows/test_pr_orchestrator_signing.py] - tests/unit/workflows/test_sign_modules_hardening.py (expects
--require-signature) [::nold-ai/specfact-cli-modules::tests/unit/workflows/test_sign_modules_hardening.py] - tests/unit/test_pre_commit_verify_modules_signature_script.py (pre-commit script expectations re: flags) [::nold-ai/specfact-cli-modules::tests/unit/test_pre_commit_verify_modules_signature_script.py]
- other tests reference the verifier in test fixtures and contract tests. [::nold-ai/specfact-cli-modules::tests]
- tests/unit/workflows/test_pr_orchestrator_signing.py (expects VERIFY_CMD to be appended with
- scripts/pre-commit-verify-modules-signature.sh — builds/executes verify command with hard-coded flags (
-
No occurrences found for the new license/security gate scripts or config (e.g., scripts/check_license_compliance.py, scripts/security_audit_gate.py, scripts/module-verify-policy.sh) in this repo’s files/search output — the repo remains tightly coupled to the existing verify-modules-signature CLI and explicit flag usage. (Search returned no lines for module-verify-policy.sh or check_license_compliance/security_audit_gate.) [::nold-ai/specfact-cli-modules::]
Impact summary:
- This repository is a high-consumption client of the module verification CLI and of workflows/docs/tests that expect specific flag combinations and branch-driven
--require-signaturebehavior. The PR’s change to drive verification via a sourced policy (module-verify-policy.sh), add new flags like--skip-checksum-verification, and change orchestration wiring will likely require coordinated updates here to:- workflow YAML and tests that assert exact flag strings and conditionals,
- scripts that build VERIFY_CMD (pre-commit script),
- documentation/examples that show explicit flag usage,
- any package script entries (pyproject) or tests that invoke the CLI with exact args.
🔇 Additional comments (1)
scripts/publish-module.py (1)
54-54: Nice extraction of official repo marker to a dedicated constant.This improves readability and reduces hard-coded duplication risk.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/unit/specfact_cli/registry/test_signing_artifacts.py (1)
194-219:⚠️ Potential issue | 🟡 MinorAdd
--version-onlyto the help coverage.This PR introduces a new CLI mode, but the consolidated help test still exercises only the pre-existing automation flags. A regression that drops
--version-onlyfrom--helpwill stay green.🧪 Minimal test update
- (("--changed-only", "--base-ref", "--bump-version"), "changed-module automation"), + (("--changed-only", "--base-ref", "--bump-version", "--version-only"), "changed-module automation"),As per coding guidelines,
**/*.pychanges should keep critical Python paths fully covered by tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/specfact_cli/registry/test_signing_artifacts.py` around lines 194 - 219, Update the parametrized expected_flags in the test_sign_modules_py_help_mentions_expected_flags test to include the new CLI flag --version-only so the help coverage fails if it’s dropped; specifically, inside the pytest.mark.parametrize tuples (the one described as "changed-module automation") add "--version-only" to the expected_flags tuple checked by test_sign_modules_py_help_mentions_expected_flags (the code that iterates over expected_flags and asserts each flag appears in result.stdout when invoking SIGN_PYTHON_SCRIPT)..github/workflows/publish-modules.yml (2)
82-105:⚠️ Potential issue | 🟠 MajorFail closed before packaging a released module.
This single-module path goes straight from optional signing to
publish-module.py. If the signing secrets are absent, or the checked-out manifest is unsigned/stale,workflow_dispatchand tag publishes still succeed. Please insert the same policy-driven verify step used by the hardening flow before Line 104 so publish-time releases enforce signatures/version bumps instead of assuming them.Based on learnings: Enforce module signatures and version bumps when signed module assets or manifests are affected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-modules.yml around lines 82 - 105, Insert a new verification step between the optional signing block and the Publish module step that invokes the same policy-driven verification used by the hardening flow to enforce signatures and version bumps; use the resolved MODULE_PATH/MANIFEST (same variables set in the signing block) and run the hardening flow's verify script/command (the policy-driven verifier) so the step exits non-zero if the manifest or module artifacts are unsigned, stale, or missing a version bump—give the step an id like "verify" and ensure it runs before the publish step to fail closed when verification does not pass.
62-64:⚠️ Potential issue | 🟠 MajorFix tag parsing to use the last
-vdelimiter for version extraction.Line 64 currently uses
${TAG#*-v}(shortest prefix match), which fails for module names containing-v. For a tag likedata-viewer-v1.2.3, it strips only to the first-v, yielding the corrupted versioniewer-v1.2.3instead of1.2.3.The NAME extraction on line 63 correctly uses
${TAG%-v*}(longest suffix match), so VERSION should match that semantics. Change${TAG#*-v}to${TAG##*-v}(longest prefix match).🛠️ Proposed fix
TAG="${GITHUB_REF#refs/tags/}" NAME="${TAG%-v*}" - VERSION="${TAG#*-v}" + VERSION="${TAG##*-v}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-modules.yml around lines 62 - 64, VERSION is extracted using the shortest prefix removal `${TAG#*-v}` which breaks when the module NAME contains `-v`; update the VERSION extraction to use the longest prefix match `${TAG##*-v}` so it splits at the final `-v` like NAME's longest-suffix pattern (`${TAG%-v*}`); locate the TAG/NAME/VERSION assignments and replace `${TAG#*-v}` with `${TAG##*-v}`.
♻️ Duplicate comments (10)
scripts/verify-modules-signature.py (1)
292-309:⚠️ Potential issue | 🟠 MajorPR verification still accepts stale integrity metadata.
In the new
verify_checksum=Falsepath, the code only requires thatintegrity.checksumexists; it never compares that digest to the current module payload. BecauseVERIFY_MODULES_PRnow enables--skip-checksum-verification, a manifest with an out-of-date checksum can still be reported asOKas long as the version-bump check passes.Based on learnings: "Enforce module signatures and version bumps when signed module assets or manifests are affected".
Also applies to: 357-387
🤖 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 - 309, The code path when verify_checksum is False currently accepts any integrity.checksum value without validating it against the module payload; compute the actual checksum of the module payload using the same algorithm parsed by _parse_checksum (use the existing _module_payload to get the payload if needed), compare that computed digest to the parsed digest, and raise a ValueError (e.g., "integrity.checksum mismatch") if they differ; also apply the same fix to the analogous block around lines 357-387 so both signed and unsigned/skip-checksum flows validate that integrity.checksum matches the current payload.scripts/_detect_modules_to_publish.py (1)
109-131: 🛠️ Refactor suggestion | 🟠 MajorAdd an
@icontractto the public CLI entrypoint.
main()is a new public script entrypoint, but it only carries@beartype. This repo requires public Python APIs to use both@icontractand@beartype.Suggested fix
`@beartype` +@ensure(lambda result: result >= 0, "exit code must be non-negative") def main(argv: list[str] | None = None) -> int:As per coding guidelines "All public APIs must have
@icontractdecorators and@beartypetype checking" and "**/*.py: Public APIs require@icontractand@beartypedecorators".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/_detect_modules_to_publish.py` around lines 109 - 131, Add an `@icontract` decorator to the public CLI entrypoint main: import icontract and annotate main with a precondition and postcondition, e.g. add `@icontract.require`(lambda argv: argv is None or (isinstance(argv, list) and all(isinstance(x, str) for x in argv))) to validate the optional argv list and `@icontract.ensure`(lambda result: isinstance(result, int)) to assert the integer return; keep the existing `@beartype` decorator and place the icontract decorators immediately above def main.CHANGELOG.md (3)
90-91:⚠️ Potential issue | 🟡 MinorInsert a section separator before the next release block.
Add a horizontal rule between
0.46.3and0.46.2to keep release boundaries consistent and scannable.As per coding guidelines,
CHANGELOG.mdentries should remain clearly structured and separated for release-history readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 90 - 91, Insert a horizontal rule separator (e.g., a line with ---) between the release headings so there is a clear boundary above the "## [0.46.2] - 2026-04-15" block; locate the adjacent "0.46.3" release header and add the separator line immediately before the "## [0.46.2] - 2026-04-15" heading to match the project's changelog formatting conventions.
56-60:⚠️ Potential issue | 🟠 MajorAdd a migration/compatibility note for module signature + version-bump policy.
This release changes module verification behavior with downstream impact (including linked
specfact-cli-modulesdivergence). Add one explicit bullet so adopters know they must align pre-commit/workflow flags.Based on learnings, "Enforce module signatures and version bumps when signed module assets or manifests are affected."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 56 - 60, Add a new explicit bullet under the "Pre-commit / CI" or relevant release notes that warns adopters about the changed module verification behavior and required alignment of pre-commit/workflow flags; reference the existing `check-version-sources` change and the linked `specfact-cli-modules` divergence, and state clearly that teams must "Enforce module signatures and version bumps when signed module assets or manifests are affected" so consumers know to update their workflows and version-bump policy accordingly.
40-48:⚠️ Potential issue | 🟡 MinorPromote dependency removals into an explicit
### Removedsection.Removals are still embedded inside
### Changed; this should stay a first-class### Removedblock for release governance clarity.As per coding guidelines,
CHANGELOG.mdshould document significant removals under explicit Added/Fixed/Changed/Removed sections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 40 - 48, The removal of GPL/wrong-PyPI packages is currently nested under the "Changed" block; create a top-level "### Removed" section in CHANGELOG.md and move the bullet points describing the removed packages (`pyan3`, `bearer`, `syft`) and the replacement notes into that new "Removed" heading, leaving other adjustments (like replacements and additions of `pycg`, `bandit`, `pip-licenses`, `pip-audit` and the json5->commentjson change) in their appropriate sections (e.g., `### Changed` or `### Added`) so removals are explicitly documented under `### Removed` for release governance.openspec/changes/dep-security-cleanup/design.md (1)
95-97:⚠️ Potential issue | 🟠 MajorUpdate the gate design to the wrapper command you actually ship.
This still documents
hatch run security-auditaspip-audit --desc --strict, but the implementation in this PR is the wrapperpython scripts/security_audit_gate.py. Leaving the design on the old command breaks OpenSpec traceability for the gate behavior.As per coding guidelines,
openspec/**/*.mdis the specification source of truth and must stay aligned with implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/dep-security-cleanup/design.md` around lines 95 - 97, The spec still documents the old gate command `hatch run security-audit` instead of the actual wrapper shipped; update the openspec entry to reference the real wrapper `python scripts/security_audit_gate.py` (and similarly ensure `license-check` and `bandit-scan` lines match the wrapper/commands you ship), so the `changes/dep-security-cleanup/design.md` entry for the security-audit gate matches the implemented script `scripts/security_audit_gate.py` and maintains OpenSpec traceability.scripts/check_license_compliance.py (1)
206-213:⚠️ Potential issue | 🟠 MajorMatch allowlist exceptions by reviewed license, not just package name.
The new structure keeps multiple allowlist rows, but the actual checks still ignore
entry["license"]. A package-level exception for one reviewed GPL expression will also exempt any future GPL/AGPL change for that package, which defeats the point of recording license-specific approvals.Also applies to: 308-326
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_license_compliance.py` around lines 206 - 213, The allowlist check currently uses entries = allowlist.get(name_lower, []) and ignores entry["license"], so package exceptions are applied regardless of reviewed license; update the logic in the block that builds reason_parts (used with variables name/name_lower/version/license_expr and function _emit) to first filter entries for those whose entry.get("license") matches the current license_expr (normalize/case/whitespace as needed), then build reason_parts from that filtered list and only emit the EXCEPTION when matching-license entries exist; apply the same change to the other occurrence around lines 308-326 where entries are processed.openspec/changes/dep-security-cleanup/tasks.md (1)
81-82:⚠️ Potential issue | 🟡 MinorKeep 10.1 unchecked until
hatch test --cover -vactually runs.The note still records
hatch test -q, so the checklist says the required validation happened when it did not.As per coding guidelines,
**/*.{py,pyi}changes must run the full test suite withhatch test --cover -v.🤖 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 - 82, Update the checklist entry 10.1 in tasks.md to reflect reality: either leave the checkbox unchecked until you actually run the full test command `hatch test --cover -v` and then mark it checked with that exact command and results, or run `hatch test --cover -v` now and replace the current `hatch test -q` note with the true `--cover -v` output; ensure the file's line containing "10.1" and the command text is corrected so it no longer claims verification by `hatch test -q`..github/workflows/publish-modules.yml (2)
178-181:⚠️ Potential issue | 🟠 MajorKeep auto-publish limited to push-originated hardening runs.
This guard still admits successful
workflow_dispatchruns ofModule Signature Hardeningondev/main. The upstream workflow exposes a manual path with relaxed verification, so auto-publish can bypass the signature/version-bump guarantees this release path depends on. Addgithub.event.workflow_run.event == 'push'here.🔒 Proposed fix
if: >- github.event_name == 'workflow_run' && + github.event.workflow_run.event == 'push' && github.event.workflow_run.conclusion == 'success' && (github.event.workflow_run.head_branch == 'dev' || github.event.workflow_run.head_branch == 'main')Based on learnings: Enforce module signatures and version bumps when signed module assets or manifests are affected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-modules.yml around lines 178 - 181, The workflow condition currently allows workflow_run events from manual dispatches; update the if filter in the publish-modules workflow so it also requires github.event.workflow_run.event == 'push' (in the existing conditional that checks github.event_name, github.event.workflow_run.conclusion, and head_branch) to ensure only push-originated successful runs on dev/main trigger auto-publish.
193-197:⚠️ Potential issue | 🟠 MajorDetect modules from the updated branch tip, not the triggering SHA.
_detect_modules_to_publish.pyonly inspects the checked-out manifests. Checking outworkflow_run.head_shameans this job can miss version bumps or re-sign commits thatModule Signature Hardeningpushed before finishing, so the batch publish skips exactly the modules it should release. Sync toorigin/${HEAD_BRANCH}before Line 209.🛠️ Example fix
- name: Checkout repository at workflow_run head uses: actions/checkout@v4 with: ref: ${{ github.event.workflow_run.head_sha }} fetch-depth: 0 + + - name: Sync to remote branch tip + run: | + git fetch origin "${HEAD_BRANCH}" + git reset --hard "origin/${HEAD_BRANCH}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-modules.yml around lines 193 - 197, The checkout currently uses workflow_run.head_sha which can miss commits pushed by Module Signature Hardening; update the workflow so that after the "Checkout repository at workflow_run head" step you fetch and reset to the updated branch tip (origin/${HEAD_BRANCH}) before running _detect_modules_to_publish.py so the script inspects the real branch tip. Concretely, add a step after the existing checkout that fetches origin/${HEAD_BRANCH} and updates the working tree to origin/${HEAD_BRANCH} (so the repository state matches the updated branch tip) ensuring _detect_modules_to_publish.py sees the latest commits.
🤖 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 650-654: The job "security-audit" is erroneously gated by the
condition "if: needs.changes.outputs.code_changed == 'true' &&
needs.changes.outputs.skip_tests_dev_to_main != 'true'"; remove that "if:"
condition (or set it to always true) so the security-audit job always runs on
every PR as required by the dep-license-gate spec, keeping the job name
"security-audit" and its existing "needs: [changes, verify-module-signatures]"
dependency intact.
In `@CHANGELOG.md`:
- Around line 13-20: The top-of-file release entries are out of chronological
order: the header "## [0.46.4] - 2026-04-17" appears above "## [0.46.3] -
2026-04-16" causing ambiguity; update CHANGELOG.md so the promoted release for
this PR is the top entry by either renaming/re-dating "## [0.46.4]" to the
correct promoted version/date (e.g., change "## [0.46.4] - 2026-04-17" to "##
[0.46.3] - 2026-04-16") or move the "## [0.46.3] - 2026-04-16" block above the
0.46.4 block and ensure the headings "## [0.46.4]" and "## [0.46.3]" reflect the
true release sequence.
In `@docs/guides/publishing-modules.md`:
- Around line 96-105: The docs change removed the exact phrase expected by the
unit test, so restore the original wording that mentions pushing to branches:
update docs/guides/publishing-modules.md in the "Release flow summary (bundled
modules in specfact-cli)" section to reintroduce the line that reads "Push to
`dev` and `main`" (or equivalent exact wording) so it matches
test_module_publishing_docs_describe_modules_repo_flow; ensure the surrounding
steps (bump module-package.yaml version, re-sign manifests, verify signatures,
merge/push to branches and mention publish-modules.yml/PR behavior) remain
intact so the doc text parity is preserved.
In `@openspec/changes/dep-security-cleanup/tasks.md`:
- Around line 1-24: The OpenSpec checklist is missing the mandated git worktree
and environment lifecycle steps; add a new top-level section (e.g., "Worktree /
Bootstrap / Cleanup") before "## 1. Spec-Driven Test Scaffolding" that
explicitly lists: create worktree from origin/dev, run "hatch env create" in the
worktree, perform pre-flight/status checks (branch, tests, lint, dependency
pins), post-merge cleanup (remove worktree, close env), and a self-check
checkbox referencing the AGENTS.md "Git Worktree Policy" to confirm compliance;
ensure the new tasks are included alongside the existing numbered steps
(1.1–1.7, 2.1–2.11) so reviewers can verify the lifecycle was followed.
In `@openspec/specs/module-publishing/spec.md`:
- Around line 36-37: The spec text that says only to update
resources/bundled-module-registry/index.json is ambiguous and must explicitly
require the same entry schema used by scripts/update-registry-index.py; update
the spec (module-publishing/spec.md) to state each bundled registry entry MUST
include id, latest_version, download_url, and checksum_sha256 (and any other
required fields the script expects), so the publish/update flow and
scripts/update-registry-index.py remain consistent; reference the registry file
name and the script (scripts/update-registry-index.py) in the wording to make
the requirement explicit.
In `@scripts/_detect_modules_to_publish.py`:
- Around line 64-70: The _is_strictly_newer function currently treats unparsable
versions as "newer" by falling back to string inequality; instead, change the
InvalidVersion handling so that if Version(candidate) or Version(registered)
cannot be parsed, the function returns False (i.e., do not consider malformed
candidate versions publishable). Update _is_strictly_newer to attempt parsing
both candidate and registered (or at least ensure candidate parses) and return
False on InvalidVersion rather than comparing raw strings; keep the function
name and signature unchanged.
- Around line 34-46: The code currently treats a missing or non-list "modules"
field as an empty list, causing malformed registry snapshots to be treated as
having no published modules; instead, change the logic that sets modules
(currently using raw.get("modules") or []) to explicitly validate that "modules"
exists and is a list: if "modules" is not in raw or not
isinstance(raw["modules"], list) raise a ValueError (include context like the
path), otherwise assign modules = cast(list, raw["modules"]) and continue
populating versions (the existing entry/type checks for module_id and latest can
remain).
In `@scripts/check_license_compliance.py`:
- Around line 215-216: The output currently uses "_emit(f\"VIOLATION:
{name}=={version} ({license_expr}) — GPL/AGPL incompatible with Apache-2.0\")"
which violates the dep-license-gate contract; update the call to _emit so the
message starts with the exact literal "LICENSE VIOLATION:" and follows the spec
pattern (e.g., "LICENSE VIOLATION: <package>==<version> uses <license> ..."),
ensuring the formatted string uses the same name, version and license_expr
variables as before so the gate output matches openspec's required format.
In `@scripts/check_version_sources.py`:
- Around line 154-156: The script currently reads files from disk (e.g., using
changelog_path.read_text and then calling _changelog_has_release_header) which
will see unstaged edits; change these checks to read the staged (index) content
instead. For each place that reads a file to validate (CHANGELOG.md and the
version source files referenced in the 177-188 and 201-203 blocks), attempt to
get the staged content via Git (for example use `git show :<path>` or `git
cat-file -p :<path>` via subprocess) and use that string for the existing helper
functions like _changelog_has_release_header and the version-parsing helpers;
only fall back to reading the working-tree file if the path is not staged.
Ensure you reference the same variable names (changelog_text, changelog_path,
and the functions _changelog_has_release_header and the version file readers) so
callers and tests remain consistent.
- Around line 67-77: The _is_packaged_artifact function is overly broad: it
currently treats any path under "src/" or most "resources/" as release-relevant,
forcing semver/CHANGELOG updates for routine edits; update _is_packaged_artifact
to only return True for the explicit packaging/version files (pyproject.toml,
setup.py) and the module version file (src/__init__.py) rather than any file
under src/, and keep resource paths as non-packaged unless you have a small
explicit whitelist for specific resource files; modify the checks in
_is_packaged_artifact to use a finite set of exact paths (e.g.,
{"pyproject.toml","setup.py","src/__init__.py"}) and otherwise return False so
normal src/ and resources/ edits do not require version bumps.
In `@scripts/module-verify-policy.sh`:
- Around line 2-13: The review notes duplicate verification policy bundles; to
fix, centralize the canonical flags by exporting VERIFY_MODULES_STRICT,
VERIFY_MODULES_PR, and VERIFY_MODULES_PUSH_ORCHESTRATOR from the specfact-cli
repository (e.g., expose them via a small shell script or sourced file) and
update scripts/module-verify-policy.sh to source that exported file instead of
redefining the arrays, or alternatively add a documented coordination SLA and a
referenced tracking mechanism (e.g., GitHub issue ID and README section) and
update git-branch-module-signature-flag.sh to read from the documented source so
both repos reference the same policy symbols.
In `@scripts/pre_commit_code_review.py`:
- Around line 127-133: The discovery currently starts at the repository root
(here = root) which makes candidate a child path; change the starting directory
to the repository parent so we perform a sibling lookup: set here: Path =
root.parent (or otherwise initialize here to root.parent) before the while loop
that computes candidate and marker in the _repo_root() discovery code
(variables: root, here, candidate, marker) so the function looks for
../specfact-cli-modules instead of ./specfact-cli-modules.
In `@scripts/run_verify_modules_policy.sh`:
- Around line 8-15: The script run_verify_modules_policy.sh currently passes the
literal `--` separator through to verify-modules-signature.py, causing argparse
to treat following options as positionals; update the handling around mode and
the forwarded "$@" (in the strict, pr and push-orchestrator branches) to detect
and remove a leading `--` if present before exec'ing python
"${ROOT}/verify-modules-signature.py" with the appropriate VERIFY_MODULES_*
array (e.g., VERIFY_MODULES_STRICT, VERIFY_MODULES_PR, etc.), so only the extra
args after `--` are forwarded to verify-modules-signature.py.
In `@scripts/sign-modules.py`:
- Around line 217-227: The function _read_manifest_version_from_git builds git
paths relative to Path.cwd(), which fails when the script is run from a
subdirectory; change it to query the Git repository top-level and build git-show
paths relative to that. Inside _read_manifest_version_from_git, run git
rev-parse --show-toplevel (via subprocess) to get the repo_top path, replace
repo_root = Path.cwd().resolve() with repo_top = Path(<output>).resolve(), then
compute relative_path = git_path.relative_to(repo_top).as_posix() with the same
ValueError fallback; keep the existing git show call and error handling.
In `@tests/unit/scripts/test_check_license_compliance.py`:
- Around line 15-24: The _load_module helper currently hard-codes repository
depth using Path(__file__).resolve().parents[3], which is brittle; change
_load_module to discover the repo root by walking upward for a repo marker
(e.g., "pyproject.toml" or ".git") or accept the script path via a test fixture,
then build the path to "scripts/check_license_compliance.py" from that
discovered root; update the function name _load_module and its Path usage to
perform a loop over Path.parents (or use a fixture-provided Path) so the test no
longer depends on a fixed parent index.
- Line 3: Remove the file-wide pyright suppression ("# pyright:
reportUnknownMemberType=false") and instead add a localized type annotation or
cast for the module object used in these tests: import typing and define a small
Protocol (or use typing.cast) that describes the members your tests access on
the scripts.check_license_compliance module (referenced as mod in the tests),
then annotate/cast mod to that Protocol where it is created/used so strict
pyright checks stay enabled for the rest of the file.
- Around line 285-315: Remove or refactor the tests that directly call internal
helpers _normalize_dependency_name, _is_gpl, and _load_allowlist so they no
longer couple to implementation details; instead write contract-level tests that
exercise the same behavior via the public APIs scan_installed_environment and/or
scan_module_manifests, and only keep a focused unit test for an internal helper
if its behavior cannot be reached through those public entry points (e.g.,
replace TestNormalizeDependencyName/TestIsGplHeuristics/TestAllowlistLoader
assertions with scenarios that assert expected outputs from
scan_installed_environment/scan_module_manifests, and convert the
missing-allowlist case to a public-facing failure scenario rather than calling
_load_allowlist directly).
- Around line 130-337: Multiple tests repeat creating a temporary package tree
and writing module-package.yaml (see tests like
test_dev_only_allowlist_rejected_in_manifest_scan, test_clean_manifests_exit_0,
test_gpl_in_manifest_exits_1, etc.); extract that setup into a shared helper or
pytest fixture (or a class-level setup_method for class-based groups) such as a
fixture create_module_package(tmp_path, name, deps) or a helper method
_create_pkg_with_manifest used by scan_module_manifests tests, move repeated
YAML write_text logic there, and update tests to call the fixture/helper while
preserving the same allowlist/static_license_map inputs and assertions so
behavior remains unchanged.
---
Outside diff comments:
In @.github/workflows/publish-modules.yml:
- Around line 82-105: Insert a new verification step between the optional
signing block and the Publish module step that invokes the same policy-driven
verification used by the hardening flow to enforce signatures and version bumps;
use the resolved MODULE_PATH/MANIFEST (same variables set in the signing block)
and run the hardening flow's verify script/command (the policy-driven verifier)
so the step exits non-zero if the manifest or module artifacts are unsigned,
stale, or missing a version bump—give the step an id like "verify" and ensure it
runs before the publish step to fail closed when verification does not pass.
- Around line 62-64: VERSION is extracted using the shortest prefix removal
`${TAG#*-v}` which breaks when the module NAME contains `-v`; update the VERSION
extraction to use the longest prefix match `${TAG##*-v}` so it splits at the
final `-v` like NAME's longest-suffix pattern (`${TAG%-v*}`); locate the
TAG/NAME/VERSION assignments and replace `${TAG#*-v}` with `${TAG##*-v}`.
In `@tests/unit/specfact_cli/registry/test_signing_artifacts.py`:
- Around line 194-219: Update the parametrized expected_flags in the
test_sign_modules_py_help_mentions_expected_flags test to include the new CLI
flag --version-only so the help coverage fails if it’s dropped; specifically,
inside the pytest.mark.parametrize tuples (the one described as "changed-module
automation") add "--version-only" to the expected_flags tuple checked by
test_sign_modules_py_help_mentions_expected_flags (the code that iterates over
expected_flags and asserts each flag appears in result.stdout when invoking
SIGN_PYTHON_SCRIPT).
---
Duplicate comments:
In @.github/workflows/publish-modules.yml:
- Around line 178-181: The workflow condition currently allows workflow_run
events from manual dispatches; update the if filter in the publish-modules
workflow so it also requires github.event.workflow_run.event == 'push' (in the
existing conditional that checks github.event_name,
github.event.workflow_run.conclusion, and head_branch) to ensure only
push-originated successful runs on dev/main trigger auto-publish.
- Around line 193-197: The checkout currently uses workflow_run.head_sha which
can miss commits pushed by Module Signature Hardening; update the workflow so
that after the "Checkout repository at workflow_run head" step you fetch and
reset to the updated branch tip (origin/${HEAD_BRANCH}) before running
_detect_modules_to_publish.py so the script inspects the real branch tip.
Concretely, add a step after the existing checkout that fetches
origin/${HEAD_BRANCH} and updates the working tree to origin/${HEAD_BRANCH} (so
the repository state matches the updated branch tip) ensuring
_detect_modules_to_publish.py sees the latest commits.
In `@CHANGELOG.md`:
- Around line 90-91: Insert a horizontal rule separator (e.g., a line with ---)
between the release headings so there is a clear boundary above the "## [0.46.2]
- 2026-04-15" block; locate the adjacent "0.46.3" release header and add the
separator line immediately before the "## [0.46.2] - 2026-04-15" heading to
match the project's changelog formatting conventions.
- Around line 56-60: Add a new explicit bullet under the "Pre-commit / CI" or
relevant release notes that warns adopters about the changed module verification
behavior and required alignment of pre-commit/workflow flags; reference the
existing `check-version-sources` change and the linked `specfact-cli-modules`
divergence, and state clearly that teams must "Enforce module signatures and
version bumps when signed module assets or manifests are affected" so consumers
know to update their workflows and version-bump policy accordingly.
- Around line 40-48: The removal of GPL/wrong-PyPI packages is currently nested
under the "Changed" block; create a top-level "### Removed" section in
CHANGELOG.md and move the bullet points describing the removed packages
(`pyan3`, `bearer`, `syft`) and the replacement notes into that new "Removed"
heading, leaving other adjustments (like replacements and additions of `pycg`,
`bandit`, `pip-licenses`, `pip-audit` and the json5->commentjson change) in
their appropriate sections (e.g., `### Changed` or `### Added`) so removals are
explicitly documented under `### Removed` for release governance.
In `@openspec/changes/dep-security-cleanup/design.md`:
- Around line 95-97: The spec still documents the old gate command `hatch run
security-audit` instead of the actual wrapper shipped; update the openspec entry
to reference the real wrapper `python scripts/security_audit_gate.py` (and
similarly ensure `license-check` and `bandit-scan` lines match the
wrapper/commands you ship), so the `changes/dep-security-cleanup/design.md`
entry for the security-audit gate matches the implemented script
`scripts/security_audit_gate.py` and maintains OpenSpec traceability.
In `@openspec/changes/dep-security-cleanup/tasks.md`:
- Around line 81-82: Update the checklist entry 10.1 in tasks.md to reflect
reality: either leave the checkbox unchecked until you actually run the full
test command `hatch test --cover -v` and then mark it checked with that exact
command and results, or run `hatch test --cover -v` now and replace the current
`hatch test -q` note with the true `--cover -v` output; ensure the file's line
containing "10.1" and the command text is corrected so it no longer claims
verification by `hatch test -q`.
In `@scripts/_detect_modules_to_publish.py`:
- Around line 109-131: Add an `@icontract` decorator to the public CLI entrypoint
main: import icontract and annotate main with a precondition and postcondition,
e.g. add `@icontract.require`(lambda argv: argv is None or (isinstance(argv, list)
and all(isinstance(x, str) for x in argv))) to validate the optional argv list
and `@icontract.ensure`(lambda result: isinstance(result, int)) to assert the
integer return; keep the existing `@beartype` decorator and place the icontract
decorators immediately above def main.
In `@scripts/check_license_compliance.py`:
- Around line 206-213: The allowlist check currently uses entries =
allowlist.get(name_lower, []) and ignores entry["license"], so package
exceptions are applied regardless of reviewed license; update the logic in the
block that builds reason_parts (used with variables
name/name_lower/version/license_expr and function _emit) to first filter entries
for those whose entry.get("license") matches the current license_expr
(normalize/case/whitespace as needed), then build reason_parts from that
filtered list and only emit the EXCEPTION when matching-license entries exist;
apply the same change to the other occurrence around lines 308-326 where entries
are processed.
In `@scripts/verify-modules-signature.py`:
- Around line 292-309: The code path when verify_checksum is False currently
accepts any integrity.checksum value without validating it against the module
payload; compute the actual checksum of the module payload using the same
algorithm parsed by _parse_checksum (use the existing _module_payload to get the
payload if needed), compare that computed digest to the parsed digest, and raise
a ValueError (e.g., "integrity.checksum mismatch") if they differ; also apply
the same fix to the analogous block around lines 357-387 so both signed and
unsigned/skip-checksum flows validate that integrity.checksum matches the
current payload.
🪄 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: 2b76dd9b-4c1b-497c-87f7-1108972c2eaf
📒 Files selected for processing (28)
.github/workflows/pr-orchestrator.yml.github/workflows/publish-modules.yml.github/workflows/sign-modules.yml.pre-commit-config.yamlCHANGELOG.mdREADME.mddocs/guides/publishing-modules.mdopenspec/changes/dep-security-cleanup/design.mdopenspec/changes/dep-security-cleanup/tasks.mdopenspec/specs/module-publishing/spec.mdpyproject.tomlresources/bundled-module-registry/index.jsonscripts/_detect_modules_to_publish.pyscripts/check_license_compliance.pyscripts/check_version_sources.pyscripts/license_allowlist.yamlscripts/module-verify-policy.shscripts/pre-commit-verify-modules.shscripts/pre_commit_code_review.pyscripts/run_verify_modules_policy.shscripts/sign-modules.pyscripts/verify-modules-signature.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pytests/unit/scripts/test_check_license_compliance.pytests/unit/scripts/test_check_version_sources.pytests/unit/specfact_cli/registry/test_signing_artifacts.py
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/sign-modules.yml (1)
85-110:⚠️ Potential issue | 🟠 MajorKey rotations still dead-end before the workflow can repair signatures.
This workflow now triggers on
resources/keys/**, but the push auto-sign path still uses--changed-only --base-ref "$BEFORE". A key-only push will not select any manifests for re-signing, so the next strict verify runs against the new key material and fails beforeopen_auto_sign_prcan create the remediation PR. This needs a full-resign fallback when signing keys change.Suggested fix
- python scripts/sign-modules.py \ - --changed-only \ - --base-ref "$BEFORE" \ - --bump-version patch \ - --payload-from-filesystem + if git diff --name-only "$BEFORE" HEAD -- resources/keys | grep -q .; then + mapfile -t MANIFESTS < <(find src/specfact_cli/modules modules -name 'module-package.yaml' -type f 2>/dev/null | sort) + python scripts/sign-modules.py --payload-from-filesystem "${MANIFESTS[@]}" + else + python scripts/sign-modules.py \ + --changed-only \ + --base-ref "$BEFORE" \ + --bump-version patch \ + --payload-from-filesystem + fiBased on learnings: Enforce module signatures and version bumps when signed module assets or manifests are affected.
Also applies to: 112-122
🤖 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 85 - 110, The current auto-sign job (step "Auto-sign changed bundled modules (push to dev/main)") uses sign-modules.py with --changed-only --base-ref "$BEFORE", which means key-only pushes under resources/keys/** won't select any manifests and will leave signatures mismatched; update the workflow to detect when signing key material changed (e.g., changes under resources/keys/** or when GITHUB_EVENT contains key rotations) and in that case invoke sign-modules.py without --changed-only / without --base-ref (full re-sign) so all manifests/assets are re-signed and bumped (keep --bump-version patch and --payload-from-filesystem), ensuring the remediation path (open_auto_sign_pr) can succeed and avoid strict-verify failures.
🤖 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/sign-modules.yml:
- Around line 181-185: The branch name built into BRANCH is not rerun-safe
because it uses github.run_id; update the BRANCH assignment generation (the
BRANCH variable used with git checkout -b, git push -u origin "${BRANCH}", and
gh pr create) to include either github.run_attempt or the commit SHA (e.g.,
GITHUB_SHA or its short form) so each retry produces a unique branch. Ensure the
chosen variable is URL/branch-safe (sanitize GITHUB_REF_NAME if needed) and
propagate the new BRANCH string unchanged to the existing git checkout, git
commit, git push, and gh pr create steps.
- Around line 60-64: The workflow grants write-scoped token to the entire verify
job (see permissions: contents: write and pull-requests: write and
outputs.signing_pr_created), which allows PR code to run with push/PR write
access; split this into two jobs: keep the verification job (the one that checks
out PR HEAD and runs scripts/module-verify-policy.sh and
scripts/verify-modules-signature.py) as read-only by changing its permissions to
contents: read and removing pull-requests: write, and create a separate
follow-up job that runs only when auto-signing is needed (use the existing
outputs.signing_pr_created or the condition currently set by
steps/open_auto_sign_pr.outputs.created) with write permissions (contents: write
and pull-requests: write) to perform push/PR creation; wire the new job to
depend on the read-only verify job and ensure only the new job has write-scoped
permissions.
---
Outside diff comments:
In @.github/workflows/sign-modules.yml:
- Around line 85-110: The current auto-sign job (step "Auto-sign changed bundled
modules (push to dev/main)") uses sign-modules.py with --changed-only --base-ref
"$BEFORE", which means key-only pushes under resources/keys/** won't select any
manifests and will leave signatures mismatched; update the workflow to detect
when signing key material changed (e.g., changes under resources/keys/** or when
GITHUB_EVENT contains key rotations) and in that case invoke sign-modules.py
without --changed-only / without --base-ref (full re-sign) so all
manifests/assets are re-signed and bumped (keep --bump-version patch and
--payload-from-filesystem), ensuring the remediation path (open_auto_sign_pr)
can succeed and avoid strict-verify failures.
🪄 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: cec303f4-b156-4f27-8999-e1f651fe7116
📒 Files selected for processing (1)
.github/workflows/sign-modules.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
.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/sign-modules.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/sign-modules.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/sign-modules.yml
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/`)
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
📚 Learning: 2026-04-10T22:42:21.860Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-10T22:42:21.860Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
.github/workflows/sign-modules.yml
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/sign-modules.yml
📚 Learning: 2026-04-10T22:41:19.090Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-10T22:41:19.090Z
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/sign-modules.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 .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing
Applied to files:
.github/workflows/sign-modules.yml
📚 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:
.github/workflows/sign-modules.yml
🔀 Multi-repo context nold-ai/specfact-cli-modules
Findings for nold-ai/specfact-cli-modules [::nold-ai/specfact-cli-modules::]
-
Repository still uses the older verifier/branch-flag model (no policy-wrapper or new CI gate scripts):
- scripts/verify-modules-signature.py exists and implements the original verifier. [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py]
- scripts/sign-modules.py exists (signing implementation). [::nold-ai/specfact-cli-modules::sign-modules.py]
- Pre-commit hook uses the legacy branch-aware wrapper and hard-coded flags:
- scripts/pre-commit-verify-modules-signature.sh invokes "hatch run ./scripts/verify-modules-signature.py --payload-from-filesystem --enforce-version-bump" and conditionally appends --require-signature for main. [::nold-ai/specfact-cli-modules::scripts/pre-commit-verify-modules-signature.sh]
-
New policy/CI artifacts introduced in the PR are absent (search found no module-verify-policy.sh / run_verify_modules_policy.sh / check_license_compliance.py / security_audit_gate.py / license_allowlist.yaml / module_pip_dependencies_licenses.yaml). This repo will not pick up policy-array-based invocation or the new CI gates until updated. [::nold-ai/specfact-cli-modules::]
-
Tests, docs, and workflows in this repo expect the old CLI flags and flows:
- Multiple docs and tests reference verify-modules-signature.py invocations with --payload-from-filesystem, --enforce-version-bump, and conditional --require-signature (README.md, docs/, tests/unit/workflows/*). Examples: README.md, docs/authoring/module-signing.md, tests/unit/workflows/test_pr_orchestrator_signing.py. [::nold-ai/specfact-cli-modules::README.md][::nold-ai/specfact-cli-modules::docs/authoring/module-signing.md][::nold-ai/specfact-cli-modules::tests/unit/workflows/test_pr_orchestrator_signing.py]
-
Tooling/dependency differences remain:
- This repo references pyan3 for call-graph analysis and json5 in pyproject (pyan3 checks and invocations present in packages/**/analyzers and docs). No pycg/commentjson occurrences were found. These will conflict with specfact-cli’s migration to pycg/commentjson unless synchronized. [::nold-ai/specfact-cli-modules::pyproject.toml][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/graph_analyzer.py][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py]
Impact summary / actionable notes (observed evidence above):
- Upstream changes in specfact-cli (policy-driven VERIFY_MODULES_* arrays, --skip-checksum-verification, license/security gate scripts, pycg/commentjson) will require coordinated updates here to:
- Replace pre-commit and CI invocations to source/run the new policy wrapper scripts (or adapt tests/docs).
- Add the new license/security gate scripts and allowlist/static license map if you plan to apply the same CI gates.
- Migrate call-graph tooling and JSONC handling (pyan3 → pycg, json5 → commentjson) to keep analyzers and docs consistent.
- Until those changes are applied, tests and workflow-related assertions in this repo will diverge from specfact-cli’s new behavior and may fail when cross-repo expectations are enforced.
djm81
added
enhancement
2 participants
Summary
Promotes
devtomainfor v0.46.3 after merging #507 (dependency hygiene, license/CVE gates, call-graph tooling, CI updates).Highlights (from CHANGELOG)
commentjsoninstead ofjson5,pycg/bandit/pip-licenses/pip-auditin extras.scripts/check_license_compliance.py,scripts/security_audit_gate.py, CIlicense-check/security-audit, agent rules underdocs/agent-rules/55-dependency-hygiene.md._detect_modules_to_publish.pyandpublish-modules.ymlauto-publish flow.Checklist
devfast-forwarded toorigin/dev(5d0d82f0).