Updating docs and fix code review findings

README.md

@@ -50,7 +50,7 @@ hatch run specfact code review run --json --out .specfact/code-review.json
 
 **Module signatures:** `pr-orchestrator` enforces `--require-signature` only for events targeting **`main`**; for **`dev`** (and feature branches) CI checks checksums and version bumps without requiring a cryptographic signature yet. Add `--require-signature` to the `verify-modules-signature` command when you want the same bar as **`main`** (for example before merging to `main`). Pre-commit runs `scripts/pre-commit-verify-modules-signature.sh`, which mirrors that policy (signatures required on branch `main`, or when `GITHUB_BASE_REF=main` in Actions).
 
-**CI signing:** Approved PRs to `dev` or `main` from **this repository** (not forks) run `.github/workflows/sign-modules-on-approval.yml`, which can commit signed manifests using repository secrets. See [Module signing](/authoring/module-signing/).
+**CI signing:** Approved PRs to `dev` or `main` from **this repository** (not forks) run `.github/workflows/sign-modules-on-approval.yml`, which can commit signed manifests using repository secrets. See [Module signing](./docs/authoring/module-signing.md).
 
 To mirror CI locally with git hooks, enable pre-commit:
 

docs/authoring/module-signing.md

@@ -89,9 +89,17 @@ hatch run python scripts/sign-modules.py \
   --base-ref origin/dev \
   --bump-version patch
 
-# Verify after signing (must match sign payload mode). For a dev-targeting branch, CI omits
-# --require-signature; add it when checking as for main:
+# Verify after signing (must match sign payload mode). This matches dev-targeting CI: checksum +
+# version policy only—dev CI omits --require-signature:
+hatch run python scripts/verify-modules-signature.py --payload-from-filesystem --enforce-version-bump --version-check-base origin/dev
+
+# Main-equivalent (strict) verification: dev CI does not run this, but use it locally when you want
+# cryptographic signatures enforced like on main. Same verifier flags as above, plus
+# --require-signature. Example with --version-check-base origin/dev (typical feature → dev PR);
+# before merging to main, point --version-check-base at origin/main so version policy matches the
+# integration target:
 hatch run python scripts/verify-modules-signature.py --require-signature --payload-from-filesystem --enforce-version-bump --version-check-base origin/dev
+hatch run python scripts/verify-modules-signature.py --require-signature --payload-from-filesystem --enforce-version-bump --version-check-base origin/main
 ```
 
 Wrapper for single manifest:

docs/authoring/publishing-modules.md

@@ -82,7 +82,7 @@ Repository workflow `.github/workflows/publish-modules.yml`:
 
 - Bump module `version` in `module-package.yaml` whenever payload or manifest content changes; keep versions immutable for published artifacts.
 - Use `namespace/name` for any module you publish to a registry.
-- Before releasing from **`main`**, run `scripts/verify-modules-signature.py --require-signature --payload-from-filesystem` (or your registry’s policy). On **`dev`**, CI may accept checksum-only manifests until promotion; align with your registry’s requirements.
+- Before releasing from **`main`**, run `scripts/verify-modules-signature.py --require-signature --enforce-version-bump --payload-from-filesystem` (or your registry’s policy). On **`dev`**, only signature strictness differs: CI may accept checksum-only manifests (omit `--require-signature`), but when signed module assets or manifests are in play you should still run `scripts/verify-modules-signature.py --enforce-version-bump --payload-from-filesystem` so version bumps stay enforced; add `--require-signature` when you want the same bar as **`main`** before promotion.
 - Prefer `--download-base-url` and `--index-fragment` when integrating with a custom registry index.
 
 ## See also

tests/unit/workflows/test_sign_modules_on_approval.py

@@ -1,6 +1,9 @@
 from __future__ import annotations
 
 from pathlib import Path
+from typing import Any, cast
+
+import yaml
 
 
 REPO_ROOT = Path(__file__).resolve().parents[3]
@@ -11,19 +14,70 @@ def _workflow_text() -> str:
     return path.read_text(encoding="utf-8")
 
 
+def _parsed_workflow() -> dict[Any, Any]:
+    loaded = yaml.safe_load(_workflow_text())
+    assert isinstance(loaded, dict)
+    return cast(dict[Any, Any], loaded)
+
+
+def _workflow_on_section(doc: dict[Any, Any]) -> dict[str, Any]:
+    """Top-level workflow triggers. PyYAML 1.1 parses the key ``on`` as bool ``True``."""
+    section = doc.get(True)
+    if isinstance(section, dict):
+        return cast(dict[str, Any], section)
+    raw = doc.get("on")
+    assert isinstance(raw, dict)
+    return cast(dict[str, Any], raw)
+
+
+def _sign_modules_job(doc: dict[Any, Any]) -> dict[str, Any]:
+    jobs = doc["jobs"]
+    assert isinstance(jobs, dict)
+    job = jobs["sign-modules"]
+    assert isinstance(job, dict)
+    return cast(dict[str, Any], job)
+
+
+def _step_by_field(steps: list[Any], field: str, value: str) -> dict[str, Any]:
+    for raw in steps:
+        if isinstance(raw, dict) and raw.get(field) == value:
+            return cast(dict[str, Any], raw)
+    raise AssertionError(f"No workflow step with {field}={value!r}")
+
+
+def _assert_pull_request_review_submitted(doc: dict[Any, Any]) -> None:
+    on = _workflow_on_section(doc)
+    pr_review = on["pull_request_review"]
+    assert isinstance(pr_review, dict)
+    assert pr_review["types"] == ["submitted"]
+
+
+def _assert_sign_job_branch_filters(doc: dict[Any, Any]) -> None:
+    job = _sign_modules_job(doc)
+    job_if = job["if"]
+    assert isinstance(job_if, str)
+    assert "github.event.review.state == 'approved'" in job_if
+    assert "github.event.pull_request.base.ref == 'dev'" in job_if
+    assert "github.event.pull_request.base.ref == 'main'" in job_if
+    assert "github.event.pull_request.head.repo.full_name == github.repository" in job_if
+
+
+def _assert_concurrency_and_permissions(doc: dict[Any, Any]) -> None:
+    conc = doc["concurrency"]
+    assert isinstance(conc, dict)
+    assert conc["cancel-in-progress"] is True
+    assert "${{ github.event.pull_request.number }}" in conc["group"]
+
+    perms = doc["permissions"]
+    assert isinstance(perms, dict)
+    assert perms["contents"] == "write"
+
+
 def test_sign_modules_on_approval_trigger_and_job_filter() -> None:
-    workflow = _workflow_text()
-    assert "pull_request_review:" in workflow
-    assert "types:" in workflow
-    assert "submitted" in workflow
-    assert "github.event.review.state == 'approved'" in workflow
-    assert "github.event.pull_request.base.ref == 'dev'" in workflow
-    assert "github.event.pull_request.base.ref == 'main'" in workflow
-    assert "github.event.pull_request.head.repo.full_name == github.repository" in workflow
-    assert "concurrency:" in workflow
-    assert "cancel-in-progress: true" in workflow
-    assert "permissions:" in workflow
-    assert "contents: write" in workflow
+    doc = _parsed_workflow()
+    _assert_pull_request_review_submitted(doc)
+    _assert_sign_job_branch_filters(doc)
+    _assert_concurrency_and_permissions(doc)
 
 
 def test_sign_modules_on_approval_checkout_and_python() -> None:
@@ -47,15 +101,17 @@ def test_sign_modules_on_approval_dependencies_and_discover() -> None:
 
 
 def test_sign_modules_on_approval_secrets_guard() -> None:
-    workflow = _workflow_text()
-    assert "Guard signing secrets" in workflow
-    assert '[ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY:-}" ]' in workflow
-    assert '[ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE:-}" ]' in workflow
-    assert "Missing secret: SPECFACT_MODULE_PRIVATE_SIGN_KEY" in workflow
-    assert "Missing secret: SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE" in workflow
-    assert "exit 1" in workflow
-    assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY" in workflow
-    assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE" in workflow
+    job = _sign_modules_job(_parsed_workflow())
+    steps = job["steps"]
+    assert isinstance(steps, list)
+    guard = _step_by_field(steps, "name", "Guard signing secrets")
+    run = guard["run"]
+    assert isinstance(run, str)
+    assert '[ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY:-}" ]' in run
+    assert '[ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE:-}" ]' in run
+    assert 'echo "::error::Missing secret: SPECFACT_MODULE_PRIVATE_SIGN_KEY"' in run
+    assert 'echo "::error::Missing secret: SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE"' in run
+    assert run.count("exit 1") >= 2
 
 
 def test_sign_modules_on_approval_sign_step_merge_base() -> None:
@@ -72,14 +128,40 @@ def test_sign_modules_on_approval_sign_step_merge_base() -> None:
     assert "--payload-from-filesystem" in workflow
 
 
+def _assert_discover_step_writes_outputs(steps: list[Any]) -> None:
+    discover = _step_by_field(steps, "id", "discover")
+    discover_run = discover["run"]
+    assert isinstance(discover_run, str)
+    assert "manifests_count=" in discover_run
+    assert "GITHUB_OUTPUT" in discover_run
+
+
+def _assert_commit_and_push_step(steps: list[Any]) -> None:
+    commit_step = _step_by_field(steps, "name", "Commit and push signed manifests")
+    assert commit_step.get("id") == "commit"
+    commit_run = commit_step["run"]
+    assert isinstance(commit_run, str)
+    assert 'git commit -m "chore(modules): ci sign changed modules [skip ci]"' in commit_run
+    assert 'git push origin "HEAD:${PR_HEAD_REF}"' in commit_run
+    assert "Push to ${PR_HEAD_REF} failed" in commit_run
+
+
+def _assert_job_summary_step(steps: list[Any]) -> None:
+    summary = _step_by_field(steps, "name", "Write job summary")
+    assert summary.get("if") == "always()"
+    env = summary["env"]
+    assert isinstance(env, dict)
+    assert env["COMMIT_CHANGED"] == "${{ steps.commit.outputs.changed }}"
+    assert env["MANIFESTS_COUNT"] == "${{ steps.discover.outputs.manifests_count }}"
+    summary_run = summary["run"]
+    assert isinstance(summary_run, str)
+    assert "GITHUB_STEP_SUMMARY" in summary_run
+
+
 def test_sign_modules_on_approval_commit_push_and_summary() -> None:
-    workflow = _workflow_text()
-    assert "github-actions[bot]" in workflow
-    assert "chore(modules): ci sign changed modules [skip ci]" in workflow
-    assert "git push origin" in workflow
-    assert "Push to ${PR_HEAD_REF} failed" in workflow
-    assert "HEAD:${PR_HEAD_REF}" in workflow
-    assert "GITHUB_STEP_SUMMARY" in workflow
-    assert "COMMIT_CHANGED:" in workflow
-    assert "MANIFESTS_COUNT:" in workflow
-    assert "steps.discover.outputs.manifests_count" in workflow
+    job = _sign_modules_job(_parsed_workflow())
+    steps = job["steps"]
+    assert isinstance(steps, list)
+    _assert_discover_step_writes_outputs(steps)
+    _assert_commit_and_push_step(steps)
+    _assert_job_summary_step(steps)