Updating docs and fix code review findings

[djm81]

Summary

Updating docs and fix code review findings

Refs:

• specfact-cli issue: #
• related module migration item/change: #

Scope

• Bundle source changes under packages/
• Registry/manifest changes (registry/index.json, packages/*/module-package.yaml)
• CI/workflow changes (.github/workflows/*)
• Documentation changes (docs/*, README.md, AGENTS.md)
• Security/signing changes (scripts/sign-modules.py, scripts/verify-modules-signature.py)

Bundle Impact

List impacted bundles and version updates:

• nold-ai/specfact-project: old -> new
• nold-ai/specfact-backlog: old -> new
• nold-ai/specfact-codebase: old -> new
• nold-ai/specfact-spec: old -> new
• nold-ai/specfact-govern: old -> new

Validation Evidence

Paste command output snippets or link workflow runs.

Required local gates

• hatch run format
• hatch run type-check
• hatch run lint
• hatch run yaml-lint
• hatch run check-bundle-imports
• hatch run contract-test
• hatch run smart-test (or hatch run test)

Signature + version integrity (required)

• hatch run verify-modules-signature --payload-from-filesystem --enforce-version-bump passes (matches PRs targeting dev)
• If this PR targets main, also confirmed: hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump (and/or approval triggered sign-modules-on-approval for same-repo PRs)
• Changed bundle versions were bumped when module payloads changed
• Manifests signed when required by your target branch (CI may sign on approval for dev/main same-repo PRs)

CI and Branch Protection

• PR orchestrator jobs expected:
  • verify-module-signatures
  • sign-modules-on-approval (on approval, same-repo PRs to dev/main only)
  • quality (3.11)
  • quality (3.12)
  • quality (3.13)
• Branch protection required checks are aligned with the above

Docs / Pages

• Bundle/module docs updated in this repo (docs/)
• Pages workflow impact reviewed (docs-pages.yml, if changed)
• Cross-links from specfact-cli docs updated (if applicable)

Checklist

• Self-review completed
• No unrelated files or generated artifacts included
• Backward-compatibility/rollout notes documented (if needed)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 143c60c5-8c61-429d-9ed0-d0d3663c647d

📥 Commits

Reviewing files that changed from the base of the PR and between 77d1ace and b2a983a.

📒 Files selected for processing (4)

• README.md
• docs/authoring/module-signing.md
• docs/authoring/publishing-modules.md
• tests/unit/workflows/test_sign_modules_on_approval.py

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (3)

docs/**/*.md

⚙️ CodeRabbit configuration file

docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.

Files:

• docs/authoring/publishing-modules.md
• docs/authoring/module-signing.md

**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)

Files:

• tests/unit/workflows/test_sign_modules_on_approval.py

tests/**/*.py

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.

Files:

• tests/unit/workflows/test_sign_modules_on_approval.py

🧠 Learnings (4)

📓 Common learnings

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature

Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected

📚 Learning: 2026-04-13T10:38:43.524Z

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected

Applied to files:

• docs/authoring/publishing-modules.md
• docs/authoring/module-signing.md
• README.md

📚 Learning: 2026-04-13T10:38:22.837Z

Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature

Applied to files:

• docs/authoring/publishing-modules.md
• docs/authoring/module-signing.md
• README.md

📚 Learning: 2026-04-02T21:49:11.371Z

Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.

Applied to files:

• docs/authoring/publishing-modules.md
• docs/authoring/module-signing.md
• tests/unit/workflows/test_sign_modules_on_approval.py
• README.md

🪛 LanguageTool

README.md

[uncategorized] ~53-~53: The official name of this software platform is spelled with a capital “H”.
Context: ...rom this repository (not forks) run .github/workflows/sign-modules-on-approval.yml...

(GITHUB)

🔀 Multi-repo context nold-ai/specfact-cli

nold-ai/specfact-cli — relevant consumers & contracts

• scripts/verify-modules-signature.py is present and referenced by many docs, workflows and tests:

  • scripts/verify-modules-signature.py (file) — verifier implementation. [::nold-ai/specfact-cli::scripts/verify-modules-signature.py]
  • tests/unit/specfact_cli/registry/test_signing_artifacts.py — asserts existence and usage of verifier; checks for invocation string containing "--require-signature". [::nold-ai/specfact-cli::tests/unit/specfact_cli/registry/test_signing_artifacts.py:440,508]
  • scripts/pre-commit-smart-checks.sh — runs verifier with --require-signature --enforce-version-bump. [::nold-ai/specfact-cli::scripts/pre-commit-smart-checks.sh:94]
  • .github/workflows/sign-modules.yml, .github/workflows/pr-orchestrator.yml, .github/workflows/pr-orchestrator.yml (lines invoking verifier with various flags including --payload-from-filesystem, --enforce-version-bump, --version-check-base). [::nold-ai/specfact-cli::.github/workflows/sign-modules.yml:55-57] [::nold-ai/specfact-cli::.github/workflows/pr-orchestrator.yml:120-122]

• scripts/sign-modules.py and sign workflows:

  • scripts/sign-modules.py (file) — signer used by CI and publish scripts. [::nold-ai/specfact-cli::scripts/sign-modules.py]
  • .github/workflows/sign-modules.yml — workflow invoking scripts/sign-modules.py and using BASE_REF to drive verifier. [::nold-ai/specfact-cli::.github/workflows/sign-modules.yml:13,22,96]
  • .github/workflows/publish-modules.yml and pr-orchestrator.yml — call sign-modules with --payload-from-filesystem / --changed-only flags. [::nold-ai/specfact-cli::.github/workflows/publish-modules.yml:77] [::nold-ai/specfact-cli::.github/workflows/pr-orchestrator.yml:736]

• Tests & CI expectations that could be impacted by documentation/behavior changes:

  • tests/unit/workflows/test_trustworthy_green_checks.py — inspects sign-modules workflow structure. [::nold-ai/specfact-cli::tests/unit/workflows/test_trustworthy_green_checks.py:15,158-163]
  • tests/unit/specfact_cli/registry/test_signing_artifacts.py — many tests validating signer/verifier behavior and workflow secrets/flags. (See test_sign_modules_workflow_exists, test_sign_modules_workflow_valid_yaml, test_sign_modules_workflow_uses_private_key_and_passphrase_secrets). [::nold-ai/specfact-cli::tests/unit/specfact_cli/registry/test_signing_artifacts.py:422-427,519-520]

• Documentation references that the PR’s README/docs edits should stay consistent with:

  • docs/guides/publishing-modules.md, docs/guides/module-signing-and-key-rotation.md, docs/reference/module-security.md — all instruct using verify-modules-signature.py with flags like --require-signature, --enforce-version-bump, --version-check-base, and --payload-from-filesystem. Changes in doc guidance (dev vs main CI) should match workflow invocations above. [::nold-ai/specfact-cli::docs/guides/publishing-modules.md:64,107] [::nold-ai/specfact-cli::docs/guides/module-signing-and-key-rotation.md:55,91,114] [::nold-ai/specfact-cli::docs/reference/module-security.md:50]

• Scripts that call the verifier/signer programmatically:

  • scripts/publish-module.py — invokes sign-modules.py and expects behavior around signing and integrity. [::nold-ai/specfact-cli::scripts/publish-module.py:148-177,555]

Assessment (concise):

• The specfact-cli repo contains the canonical signer and verifier scripts plus workflows and tests that assert specific flag combinations and workflow behavior. The PR’s documentation/test changes (dev vs main verification flags, adding --enforce-version-bump and --payload-from-filesystem in some places) map directly to invocations and tests here; ensure the updated docs/tests in specfact-cli-modules align with the actual workflow steps and tests in this repo (workflows: .github/workflows/sign-modules.yml and pr-orchestrator.yml; tests referencing verifier invocation strings and workflow structure). Discrepancies between docs and the concrete workflow/test expectations here may cause CI/test assertions to fail.

🔇 Additional comments (5)

docs/authoring/publishing-modules.md (1)

85-85: Good policy split for dev vs main verification.

This keeps module-signing guidance aligned with adapter boundaries (module-package.yaml + registry verification): checksum/version enforcement stays on in dev, and signature strictness is reintroduced for main-level promotion.

As per coding guidelines: “For dev guidance, clarify that signature strictness may differ (omit --require-signature), but version bump enforcement should remain enabled, and re-enable --require-signature before promotion.”

docs/authoring/module-signing.md (1)

92-102: Nice improvement to verification-runbook parity.

The updated automation snippet now cleanly distinguishes dev-policy checks from main-equivalent strict verification, while keeping payload mode and version-bump enforcement consistent with signing flow expectations.

As per coding guidelines: “ensure the verifier flags match the target CI policy… keep the payload mode (--payload-from-filesystem) consistent and clearly describe dev vs main behavior.”

README.md (1)

53-53: README link update looks correct.

Pointing to the in-repo authoring doc improves source-of-truth discoverability for signing policy details in this repo context.

tests/unit/workflows/test_sign_modules_on_approval.py (2)

17-80: Strong shift to structured workflow assertions.

Parsing YAML and asserting trigger/job/concurrency semantics directly is a solid reliability upgrade over raw substring checks, especially for workflow contract coverage.

---

104-114: Step-level validation is much tighter now.

The helper-based checks around secrets guard, discover outputs, commit/push behavior, and summary wiring materially improve confidence in the sign-modules-on-approval adapter boundary.

Also applies to: 131-167

---

📝 Walkthrough

Documentation and Workflow Verification Updates

Module Signing & Verification Policy Clarification

docs/authoring/module-signing.md enhances the "Changed-modules automation" runbook section with explicit verification commands differentiating dev and main branch policies:

• Dev-branch verification: --payload-from-filesystem --enforce-version-bump --version-check-base origin/dev (checksum + version policy only; --require-signature omitted)
• Main-branch verification: Adds strict commands with --require-signature --payload-from-filesystem --enforce-version-bump --version-check-base origin/main to mirror production signing enforcement
• Updated comments clarify that dev CI accepts checksum-only manifests, while signatures are required by the time changes reach main

docs/authoring/publishing-modules.md refines pre-release verification guidance:

• Consolidates --require-signature, --enforce-version-bump, and --payload-from-filesystem as the expected bundle of flags for main releases
• On dev branch, clarifies that CI may omit signature strictness (--require-signature) but should still enforce version bumps (--enforce-version-bump) when signed assets/manifests are present
• Recommends adding --require-signature locally before main promotion to validate against production integrity policy

Documentation Link Accuracy

README.md corrects a documentation link from site-root-relative (/authoring/module-signing/) to relative file path (./docs/authoring/module-signing.md), improving local development IDE resolution without affecting published GitHub Pages URLs.

Workflow Test Robustness

tests/unit/workflows/test_sign_modules_on_approval.py refactors from raw string assertions to structured YAML parsing:

• Replaces text-based contains checks with yaml.safe_load() and explicit type casts
• Adds helper functions: _parsed_workflow(), _workflow_on_section(), _sign_modules_job(), _step_by_field() to locate and validate YAML structure
• Validates pull_request_review trigger types == ["submitted"], job branch/repo conditions, and concurrency/permissions values from the parsed document
• Strengthens secret-guard validation: asserts step-level run contents for missing-secret error messages and ensures exit 1 appears at least twice
• Separates commit/push assertions: validates manifest discovery output (manifests_count), exact git commit/git push commands with failure handling, and "Write job summary" step using if: always() with expected environment mappings and GITHUB_STEP_SUMMARY reference

Manifest & Integrity Scope

No changes to module-package.yaml signatures, versions, or integrity checksums. Module integrity metadata (checksum and signature fields in packages/*/module-package.yaml) remains immutable; verification logic documentation is clarified for maintainer workflows.

Cross-Repository Contract

No new specfact-cli API changes or import requirements. The documentation updates align local signing/verification workflows with existing CI enforcement in pr-orchestrator.yml (verify-module-signatures job) and sign-modules-on-approval.yml, which already enforce conditional signature requirements based on branch target.

Walkthrough

Four files updated: README link corrected to relative path, module-signing and publishing-modules documentation clarified for dev versus main branch verification policies, and test_sign_modules_on_approval.py refactored to use structured YAML parsing instead of raw string assertions with enhanced step-level validation.

Changes

Cohort / File(s)	Summary
Documentation Link & Docs README.md, docs/authoring/module-signing.md, docs/authoring/publishing-modules.md	Updated module-signing link from site-root to relative path; clarified dev vs. main CI verification policies in "Changed-modules automation" section; added --enforce-version-bump guidance for pre-release verification steps.
Workflow Test Refactoring tests/unit/workflows/test_sign_modules_on_approval.py	Replaced raw string assertions with yaml.safe_load parsing; added helper functions (_parsed_workflow, _workflow_on_section, _sign_modules_job, _step_by_field) for structured validation; strengthened assertions for trigger conditions, job filters, GITHUB_OUTPUT writes, git commands, and step environment mappings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• chore(release): promote dev to main (post marketplace-06 CI signing) #189 — Directly related: modifies module-signing verification docs and updates the same sign-modules-on-approval workflow tests.
• feat(ci): marketplace-06 module signing on approval + branch-aware verify #188 — Related: refactors the same workflow tests and introduces branch-aware verification behavior (dev vs. main).

Suggested labels

documentation, enhancement

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	Title does not follow Conventional Commits-style prefix convention; uses generic phrasing without a clear semantic prefix.	Rewrite title using Conventional Commits format (e.g., 'docs: update module signing and verification docs' or 'test: refactor workflow signature tests and fix review findings').
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	Description follows the required template structure but contains incomplete/placeholder information: no issue references filled in, all validation checklist items unchecked, bundle impact shows placeholder versions, and summary is vague.	Fill in concrete references (issue #s), check completed validation gates with evidence, document actual bundle version changes, and clarify which specific docs/code review findings were addressed.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev

---

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