Skip to content

Release: dev to main: trustworthy CI and review hardening#470

Merged
djm81 merged 359 commits intomainfrom
dev
Mar 30, 2026
Merged

Release: dev to main: trustworthy CI and review hardening#470
djm81 merged 359 commits intomainfrom
dev

Conversation

@djm81
Copy link
Copy Markdown
Collaborator

@djm81 djm81 commented Mar 30, 2026

Description

Release dev to main — consolidates trustworthy green-checks CI hardening, review gate improvements, and docs contract fixes since the last release (v0.43.3).

Fixes #469

Contract References: Runtime contracts hardened for optional_deps, acceptance_criteria, and enrichment_parser utilities under CrossHair symbolic execution.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 📚 Documentation update
  • 🧪 Test enhancement (scenario tests, property-based tests)

Contract-First Testing Evidence

Contract Validation

  • Runtime contracts added/updated (@icontract decorators on public APIs)
  • Type checking enforced (@beartype decorators applied)
  • CrossHair exploration completed: hatch run contract-test-exploration
  • Contract violations reviewed and addressed

Test Execution

  • Contract validation: hatch run contract-test-contracts
  • Contract exploration: hatch run contract-test-exploration
  • Scenario tests: hatch run contract-test-scenarios
  • Full test suite: hatch run contract-test-full

Test Quality

  • CLI commands tested with typer test client
  • Edge cases covered with Hypothesis property tests
  • Error handling tested with invalid inputs
  • Rich console output verified manually or with snapshots

How Has This Been Tested?

Contract-First Approach: All changes validated through contract-test and smart-test gates in each contributing PR.

Manual Testing

  • Tested CLI commands manually
  • Verified rich console output
  • Tested with different input scenarios
  • Checked error messages for clarity

Automated Testing

  • Contract validation passes
  • Property-based tests cover edge cases
  • Scenario tests cover user workflows
  • All existing tests still pass

Test Environment

  • Python version: 3.11, 3.12
  • OS: Ubuntu (CI), Linux (local)

Checklist

  • My code follows the style guidelines (PEP 8, ruff format, isort)
  • I have performed a self-review of my code
  • I have added/updated contracts (@icontract, @beartype)
  • I have added/updated docstrings (Google style)
  • I have made corresponding changes to documentation
  • My changes generate no new warnings (basedpyright, ruff, pylint)
  • All tests pass locally
  • I have added tests that prove my fix/feature works
  • Any dependent changes have been merged

Quality Gates Status

  • Type checking ✅ (hatch run type-check)
  • Linting ✅ (hatch run lint)
  • Contract validation ✅ (hatch run contract-test-contracts)
  • Contract exploration ✅ (hatch run contract-test-exploration)
  • Scenario tests ✅ (hatch run contract-test-scenarios)

🤖 Generated with Claude Code

djm81 and others added 30 commits February 12, 2026 23:23
Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
GitHub's licensee gem only recognizes standard filenames (LICENSE,
LICENSE.txt) — LICENSE.md caused the repo to show "Other" instead
of "Apache License 2.0". Updated all references across pyproject.toml,
README, docs, workflows, and FAQ.

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
)

The LICENSE body had two non-standard edits that pushed it below
GitHub licensee's ~95% similarity threshold, causing "Other" instead
of "Apache License 2.0". Restored the canonical text; only the
copyright line in the appendix is customized (as intended by the
Apache template).

Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
…#262)

* feat(ci): attach test and repro log artifacts to PR orchestrator runs

- Tests job: run smart-test-full, upload logs/tests/ as test-logs artifact
- Contract-first-ci: capture repro to logs/repro/, upload repro-logs and repro-reports
- Docs: CI and GitHub Actions section in troubleshooting (artifact names, usage)
- Version 0.31.1, CHANGELOG entry

Implements OpenSpec change ci-01-pr-orchestrator-log-artifacts. Fixes #260.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix workflow and test

* ci(pr-orchestrator): add log artifacts for all pipeline jobs

- type-check: capture output to logs/type-check/, upload type-check-logs
- lint: capture to logs/lint/, upload lint-logs
- compat-py311: capture to logs/compat-py311/, upload compat-py311-logs
- quality-gates: capture to logs/quality-gates/, upload quality-gates-logs
- compat-py311: use hatch -e ENV run run (not hatch test) for pytest
- docs: list all CI artifact names and jobs in troubleshooting

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* feat: enhanced module manifest security and integrity (arch-06)

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: remove duplicate ModulePackageMetadata import (ruff F811)

* Fix failed tests

* Fix type-check errors

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…rch-07) (#265)

* feat: add schema extension system for modular ProjectBundle extensions
  Enables modules to extend Feature and ProjectBundle with namespaced custom
  fields without modifying core models, supporting marketplace-ready
  interoperability.
  - Add extensions dict field to Feature and ProjectBundle models
  - Implement type-safe get/set extension accessors with namespace enforcement
  - Extend module manifest schema with schema_extensions declaration
  - Add ExtensionRegistry for collision detection and introspection
  - Extend module lifecycle registration to load and validate extensions
  OpenSpec Change: arch-07-schema-extension-system
  Resolves #213

* feat: schema extension system (arch-07) and quality gate fixes

- Add extensions field and get_extension/set_extension to Feature and ProjectBundle
- Add SchemaExtension model and schema_extensions to ModulePackageMetadata
- Add ExtensionRegistry with collision detection; integrate in module registration
- Parse schema_extensions in discover_package_metadata
- Docs: extending-projectbundle guide, architecture section, sidebar
- Version 0.32.0, CHANGELOG entry, TDD_EVIDENCE
- Format: E402 (imports at top in project.py), UP042 (StrEnum in backlog-core),
  RUF043/B017 in schema extension tests
- Type-check: pass schema_metadata/project_metadata in BundleManifest test calls

OpenSpec Change: arch-07-schema-extension-system
Resolves #213

Co-authored-by: Cursor <cursoragent@cursor.com>

* Update change progress

* Add docs guides and update changes

* Use v0.32.0 as version and combine arch-06/arch-07

* Update change order plan

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
* feat(workflow): standardize worktree-first development flow

* docs(openspec): mark workflow-01 delivery tasks complete

* Apply review finding

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* feat(policy-engine): implement unified policy framework

* docs(openspec): mark policy-engine-01 implemented in change order

* fix(policy-engine): make module io contract compliant

* feat(policy-engine): add policy init templates and docs coverage

* fix: refine grouped policy limit semantics and outputs

* docs: clarify policy engine value for new users

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…1) (#275)

- Use discover_all_package_metadata() in init so list-modules/enable/disable
  use same roots as registry (built-in + workspace modules + SPECFACT_MODULES_ROOTS)
- Extend backlog-core-01 OpenSpec: init-module-discovery-alignment spec,
  tasks 0.5.x, TDD evidence
- Bump version to 0.34.0; CHANGELOG

Fixes #116

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* feat: add thorough codebase validation (validation-01)
  - Add --crosshair-per-path-timeout to specfact repro and ReproChecker
  - Add docs/reference/thorough-codebase-validation.md (quick check, contract-full, sidecar, dogfooding)
  - Unit test and TDD evidence for CrossHair per-path timeout
  - OpenSpec validation-01-deep-validation tasks and TDD_EVIDENCE updated

* fix: reject non-positive CrossHair per-path timeout (review)

* docs: CHANGELOG v0.34.0 and doc updates for thorough codebase validation

---------

Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
djm81 and others added 6 commits March 30, 2026 13:35
* docs: sharpen first-contact story and onboarding

* docs: address first-contact review feedback

* docs: address onboarding review fixes

* test: accept default-filtered site tokens in docs parity

* docs: record completed onboarding quality gates

* test: improve first-contact assertion failures

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
* fix: harden trustworthy green checks

* fix: restore contract-first ci repro command

* fix: apply CodeRabbit auto-fixes

Fixed 3 file(s) based on 3 unresolved review comments.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>

* fix: resolve CI failures for trustworthy green checks PR

- Use hatch run contract-test instead of specfact code repro in CI
  (CLI bundle not available in CI environment)
- Allow test_bundle_import.py in migration cleanup legacy-import check
  (_bundle_import is an internal helper, not a removed module package)
- Fix formatting in test_trustworthy_green_checks.py (CodeRabbit commit
  was unformatted)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address CodeRabbit review findings

- Add trailing newline to TDD_EVIDENCE.md (MD047)
- Make _load_hooks() search for repo: local instead of assuming index 0
- Replace fragile multi-line string assertion in actionlint test with
  semantic line-by-line checks

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Release: dev to main — Trustworthy CI and Review Hardening (Fixes #469)

User-Visible Behavior & CLI Surface

Pre-commit workflow restructured:

  • Removed separate verify-module-signatures and specfact-code-review-gate hooks
  • Introduced unified specfact-smart-checks wrapper hook (scripts/pre-commit-smart-checks.sh) that conditionally invokes code review and other checks
  • Local merge-blocking gates now explicitly enumerated: module signature verification, formatter safety, Markdown/YAML checks, workflow lint, and contract-test fast feedback
  • Updated documentation (CONTRIBUTING.md, docs/modules/code-review.md) to clarify which gates block merges (FAIL verdict) versus which remain advisory (PASS_WITH_ADVISORY)

CI workflow trigger hardening:

  • Replaced paths-ignore with explicit path filtering in pr-orchestrator.yml to prevent required checks from disappearing based on commit content
  • Added detection of .github/workflows/** changes and new workflow-lint job (runs conditionally when workflows are modified)
  • Release PR skip-logic (skip_tests_dev_to_main) now requires SHA/merge-base parity proof rather than relying solely on branch names
  • Contract-first validation command changed from hatch run specfact repro to hatch run contract-test with direct failure propagation; CLI validation changed from advisory || echo fallback to strict validation

actionlint execution strategy:

  • Centralized in scripts/run_actionlint.sh with new fallback hierarchy: global actionlint → Docker image (with daemon reachability check) → explicit failure with install guidance
  • Removed repository-local actionlint binary download fallback; pinned version updated to v1.7.11
  • REPO_ROOT requirement enforced to ensure workflows are found regardless of caller's working directory

Auto-review coverage:

  • CodeRabbit configuration extended: PRs targeting both dev and main branches now receive automatic reviews

Contract/API Impact

specfact_cli.modules._bundle_import:

  • bootstrap_local_bundle_sources now respects additional SPECFACT_MODULES_REPO environment variable alongside SPECFACT_CLI_MODULES_REPO
  • De-duplication logic added to prevent duplicate candidate paths in module resolution
  • No signature changes; enhancement is backward compatible

No public API/Pydantic model changes. All modifications are internal tooling, documentation, or CI infrastructure.

Testing & Quality Gates

New test suite added: tests/unit/workflows/test_trustworthy_green_checks.py

  • 9 new integration tests validating CI configuration trustworthiness:
    • Required checks must trigger on every PR head commit (no paths-ignore suppression)
    • Required jobs fail-closed (continue-on-error not set to true)
    • Release-skip logic includes parity proof markers (git merge-base / git rev-parse)
    • Advisory jobs explicitly named as advisory
    • Contract-first job uses hatch run contract-test
    • Module signature check name canonical across orchestrator and dedicated workflows
    • Pre-commit smart-check wrapper configuration validated
    • CodeRabbit auto-review covers both dev and main branches
    • Legacy actionlint runner performs Docker daemon reachability check and propagates exit codes

Additional new tests:

  • tests/unit/modules/test_bundle_import.py: Unit test for SPECFACT_MODULES_REPO environment variable support
  • tests/unit/scripts/test_doc_frontmatter/test_validation.py: New test for YAML parse error surface/reporting

Contract validation: All changes pass existing contract-first test suite (hatch run contract-test-*); no runtime contract violations introduced.

OpenSpec / Documentation / Changelog

OpenSpec change implementation:

  • ci-02-trustworthy-green-checks marked as implemented (2026-03-30; archive pending) in openspec/CHANGE_ORDER.md
  • Comprehensive TDD evidence document (openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md) added with pre-implementation failure cases and post-remediation validation

Specification updates:

  • Trustworthy green checks spec refined: required-check-name standardization scoped to logical gates appearing across multiple workflows; new requirement for documentation to differentiate merge-blocking gates from advisory signals
  • Scenario documentation added clarifying which checks are merge-blocking versus which are advisory warnings

Documentation updates:

  • CONTRIBUTING.md: Local review gates and merge-blocking behavior clarified; actionlint availability requirement added with installation guidance
  • docs/modules/code-review.md: Pre-commit integration updated to describe smart-check wrapper architecture; new "Repository gate taxonomy" section defining merge-blocking vs. advisory semantics
  • docs/contributing/docs-sync.md: Docs-only PR workflow clarified; frontmatter validation and docs test suites marked as merge-blocking for docs-owned changes

CHANGELOG.md: Markdownlint configuration constraint added (MD024 rule).

Workflow display updates:

  • .github/workflows/sign-modules.yml job display name: "Verify module signatures" → "Verify Module Signatures" (for canonical check-name matching)

Walkthrough

Hardened CI/workflow triggers and failure semantics, replaced pre-commit hooks with a repo-owned smart-check wrapper, reworked actionlint runner to prefer global/Docker with clear failures, added CodeRabbit ^main$ branch pattern, extended bundle-import env var handling, and added tests/docs to validate these behaviors.

Changes

Cohort / File(s) Summary
Orchestrator Workflow
.github/workflows/pr-orchestrator.yml
Reworked path filtering into a changes job, added workflow_changed output, changed release-skip parity logic to use SHA/merge-base, added conditional workflow-lint job, tightened downstream job preconditions, and removed failure-masking patterns.
Signature Workflow
.github/workflows/sign-modules.yml
Normalized job display name to "Verify Module Signatures" (casing change only).
Actionlint runner & YAML tools
scripts/run_actionlint.sh, scripts/yaml-tools.sh
Replaced repo-local fallback install with detection order: global actionlint -> Docker (only if daemon reachable) -> exit with install guidance; yaml-tools delegates to the new runner.
Pre-commit configuration
.pre-commit-config.yaml, (referenced) scripts/pre-commit-smart-checks.sh
Removed verify-module-signatures and specfact-code-review-gate hooks; added specfact-smart-checks which always runs the repo-owned smart-check wrapper.
Repo automation config
.coderabbit.yaml
Added ^main$ to auto_review.base_branches so auto-review covers main and dev.
Docs & contributor guidance
CONTRIBUTING.md, docs/contributing/docs-sync.md, docs/modules/code-review.md, CHANGELOG.md, openspec/*
Clarified local smart-check wrapper semantics vs advisory checks, updated headings/formatting, updated spec/TDD/task status and changelog formatting.
Runtime change
src/specfact_cli/modules/_bundle_import.py
bootstrap_local_bundle_sources now also honors SPECFACT_MODULES_REPO and avoids duplicate candidate paths.
Tests
tests/unit/workflows/test_trustworthy_green_checks.py, tests/unit/modules/test_bundle_import.py, tests/unit/scripts/test_doc_frontmatter/test_validation.py, tests/unit/modules/test_bundle_import.py (allowlist)
Added/updated unit tests validating workflow triggers, required-check semantics, pre-commit smart-hook, actionlint runner behavior, CodeRabbit config, and bundle-import env var handling.
Changelog / formatting
CHANGELOG.md
Minor Markdownlint directive and removal of a horizontal rule separator.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Contributor
participant GitHubActions as "GitHub Actions\n(pr-orchestrator)"
participant ChangesJob as "changes job\n(paths-filter)"
participant WorkflowLint as "workflow-lint job"
participant Downstream as "Downstream Jobs\n(tests/lint/contract)"
participant run_actionlint as "scripts/run_actionlint.sh"
participant ActionlintExec as "actionlint / Docker"

Contributor->>GitHubActions: Open PR or push
GitHubActions->>ChangesJob: Run paths-filter -> outputs (code_changed, workflow_changed, skip_tests_dev_to_main)
alt workflow_changed == 'true' or workflow_dispatch
    ChangesJob->>WorkflowLint: trigger workflow-lint
    WorkflowLint->>run_actionlint: invoke runner
    run_actionlint->>ActionlintExec: prefer global -> else docker (daemon reachable) -> else exit(2)
end
ChangesJob->>Downstream: conditionally start jobs (needs.changes.outputs.code_changed == 'true' && skip_tests_dev_to_main != 'true')
Downstream->>GitHubActions: report status (fail closed, no masking)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

Suggested labels

code-review, documentation

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive Title is related to the PR's core objective but does not follow Conventional Commits style and lacks a specific prefix. Consider adopting a Conventional Commits prefix (e.g., 'chore: harden trustworthy CI and review') for consistency with repo guidelines.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed Description comprehensively follows the template with all required sections completed, including type of change, contract-first testing evidence, test execution, manual and automated testing, checklist items, and quality gates status.
Linked Issues check ✅ Passed Changes directly address #469 objectives: CI hardening, workflow/hook improvements, documentation updates, and targeted test enhancements validated through quality gates and TDD evidence.
Out of Scope Changes check ✅ Passed All changes align with linked issue #469 scope: CI/workflow hardening, pre-commit improvements, documentation clarification, and targeted test additions with no extraneous modifications.

✏️ 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.

@djm81 djm81 self-assigned this Mar 30, 2026
@djm81 djm81 changed the title Release dev to main: trustworthy CI and review hardening Release: dev to main: trustworthy CI and review hardening Mar 30, 2026
@djm81 djm81 added enhancement New feature or request QA Quality Assurance labels Mar 30, 2026
@djm81 djm81 moved this to In Progress in SpecFact CLI Mar 30, 2026
@djm81 djm81 linked an issue Mar 30, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/unit/migration/test_module_migration_07_cleanup.py (1)

20-27: 🧹 Nitpick | 🔵 Trivial

Scope this migration exemption more narrowly than a full-file allowlist.

Allowlisting all of test_bundle_import.py means any future specfact_cli.modules.* imports in that file bypass this cleanup gate. Prefer a targeted exception (e.g., explicit import string/symbol) so the guard still catches unrelated regressions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/migration/test_module_migration_07_cleanup.py` around lines 20 -
27, The current allowlist uses allowed_files and includes the entire
test_bundle_import.py which permits any specfact_cli.modules.* import in that
file; change the exemption to be targeted by removing root / "tests" / "unit" /
"modules" / "test_bundle_import.py" from allowed_files and instead add a narrow
allowed_imports (or allowed_symbols) set containing the explicit import/module
string(s) required (e.g., "specfact_cli.modules.<specific_module>" or the exact
symbol name) and update the migration cleanup check to consult that
allowed_imports set rather than permitting the whole file; reference the
allowed_files variable and the test_bundle_import.py entry and ensure the
migration guard uses allowed_imports when deciding to skip cleanup.
🤖 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 43-45: workflow_changed currently only watches
'.github/workflows/**' so changes to scripts/run_actionlint.sh and
scripts/yaml-tools.sh can bypass the Workflow Lint job; update the
workflow_changed definition in pr-orchestrator.yml to also include the script
paths (e.g., add 'scripts/run_actionlint.sh' and 'scripts/yaml-tools.sh' or
'scripts/**') so the Workflow Lint job is triggered when those files change,
ensuring the Workflow Lint job (Workflow Lint) runs on script modifications.

In `@CONTRIBUTING.md`:
- Around line 104-111: Update the CONTRIBUTING.md description of the local
review gate (the "Review gate behavior" bullet referring to `specfact code
review run` and `PASS_WITH_ADVISORY`) to state that the helper now exits
non-zero not only on explicit `FAIL` but also when the
`.specfact/code-review.json` report is missing or malformed, so
`PASS_WITH_ADVISORY` can remain green while the local gate may still block;
mention the `.specfact/code-review.json` path and `PASS_WITH_ADVISORY`
explicitly and clarify that missing/invalid JSON is a separate non-zero exit
condition.

In `@docs/modules/code-review.md`:
- Around line 122-125: Update the "Pre-Commit Review Gate" section to explicitly
mention the actionlint prerequisite when workflow files under .github/workflows/
are staged: reference that scripts/pre_commit_code_review.py (the review gate
hook) requires actionlint on PATH or a Docker-enabled environment to lint GitHub
Actions, and add either a short inline note plus a link to CONTRIBUTING.md or a
direct cross-reference to the CONTRIBUTING.md actionlint setup instructions so
developers know to install/configure actionlint before editing workflows.

In `@openspec/CHANGE_ORDER.md`:
- Line 64: Update the status annotation for the entry labeled
"ci-02-trustworthy-green-checks" to match the standardized format used
elsewhere: replace "implemented 2026-03-30 (archive pending)" with "implemented
2026-03-30; archive pending" so it follows the "implemented YYYY-MM-DD; archive
pending" convention.

In `@openspec/changes/ci-02-trustworthy-green-checks/tasks.md`:
- Around line 11-18: Update the worktree setup checklist to include explicit
pre-flight and cleanup steps: after `hatch env create` and `git branch
--show-current` add checklist sub-items to run `hatch run smart-test-status` and
`hatch run contract-test-status` in the new worktree as required pre-flight
verifications, and add post-merge cleanup steps (e.g., remove the worktree and
branch) referencing the `git worktree add
../specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks -b
feature/ci-02-trustworthy-green-checks origin/dev` and the created branch so
completion is auditable.

In `@openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md`:
- Around line 134-157: Update the follow-up verification evidence to actually
run and capture the "hatch run lint-workflows" invocation so it proves the
end-to-end path (scripts/run_actionlint.sh and scripts/yaml-tools.sh) was
exercised: add the exact command (with the same environment vars used for
tests/type-check) that runs hatch run lint-workflows and include its output, and
mention pr-orchestrator.yml's CI install step if relevant; this ensures the
recorded commands show the new actionlint install strategy and workflow linting
path were executed and passed.

In `@scripts/run_actionlint.sh`:
- Around line 46-50: Change the ACTIONLINT_TAG default and the install hint so
local and Docker runs use the same pinned version as CI: update the
ACTIONLINT_TAG assignment (symbol ACTIONLINT_TAG="${ACTIONLINT_TAG:-latest}") to
default to v1.7.11, and update the example install line that currently shows "go
install ...@latest" to use the variable (e.g., use @${ACTIONLINT_TAG}) so the
echoed hint and execution paths remain consistent with the CI-pinned version;
adjust the two locations referencing ACTIONLINT_TAG and the install hint
accordingly.
- Around line 14-20: The current run_installed_binary() both checks for
actionlint and runs it, conflating "not installed" with "lint failures"; change
run_installed_binary() to only check availability (use command -v actionlint and
return 0/1) and create a separate runner (e.g., run_actionlint_local()) that
invokes actionlint -no-color and returns its exit code; update the calling logic
so that if run_installed_binary succeeds you call run_actionlint_local and
immediately exit with its status (so lint failures propagate), otherwise fall
back to the Docker runner; also align local install guidance from
actionlint@latest to the CI-pinned actionlint@v1.7.11 to avoid version drift.

In `@tests/unit/modules/test_bundle_import.py`:
- Around line 25-30: This test mutates global state by removing/adding entries
from sys.path; update the test around
bootstrap_local_bundle_sources(str(anchor)) to save the original sys.path (or
use pytest's monkeypatch.setattr for sys.path) before modifications and restore
it after the assertion (use a try/finally or fixture) so expected_src is
asserted but sys.path is returned to its original value to avoid cross-test
leakage; reference sys.path, expected_src, anchor, and
bootstrap_local_bundle_sources in the change.

---

Outside diff comments:
In `@tests/unit/migration/test_module_migration_07_cleanup.py`:
- Around line 20-27: The current allowlist uses allowed_files and includes the
entire test_bundle_import.py which permits any specfact_cli.modules.* import in
that file; change the exemption to be targeted by removing root / "tests" /
"unit" / "modules" / "test_bundle_import.py" from allowed_files and instead add
a narrow allowed_imports (or allowed_symbols) set containing the explicit
import/module string(s) required (e.g., "specfact_cli.modules.<specific_module>"
or the exact symbol name) and update the migration cleanup check to consult that
allowed_imports set rather than permitting the whole file; reference the
allowed_files variable and the test_bundle_import.py entry and ensure the
migration guard uses allowed_imports when deciding to skip cleanup.
🪄 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: b985f232-becb-4a4f-bdab-ac6748786d28

📥 Commits

Reviewing files that changed from the base of the PR and between 441c629 and c5efee3.

📒 Files selected for processing (19)
  • .coderabbit.yaml
  • .github/workflows/pr-orchestrator.yml
  • .github/workflows/sign-modules.yml
  • .pre-commit-config.yaml
  • CHANGELOG.md
  • CONTRIBUTING.md
  • docs/contributing/docs-sync.md
  • docs/modules/code-review.md
  • openspec/CHANGE_ORDER.md
  • openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md
  • openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md
  • openspec/changes/ci-02-trustworthy-green-checks/tasks.md
  • scripts/run_actionlint.sh
  • scripts/yaml-tools.sh
  • src/specfact_cli/modules/_bundle_import.py
  • tests/unit/migration/test_module_migration_07_cleanup.py
  • tests/unit/modules/test_bundle_import.py
  • tests/unit/scripts/test_doc_frontmatter/test_validation.py
  • tests/unit/workflows/test_trustworthy_green_checks.py

Comment thread .github/workflows/pr-orchestrator.yml
Comment thread CONTRIBUTING.md
Comment thread docs/modules/code-review.md
Comment thread openspec/CHANGE_ORDER.md Outdated
Comment thread openspec/changes/ci-02-trustworthy-green-checks/tasks.md
Comment thread openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md
Comment thread scripts/run_actionlint.sh Outdated
Comment thread scripts/run_actionlint.sh Outdated
Comment thread tests/unit/modules/test_bundle_import.py Outdated
- Widen workflow_changed filter to include scripts/run_actionlint.sh
  and scripts/yaml-tools.sh so Workflow Lint triggers on script changes
- Pin actionlint default to v1.7.11 (matches CI) instead of latest
- Fix run_actionlint.sh conflating "not installed" with "lint failures"
  by separating availability check from execution
- Restore sys.path after test_bundle_import to avoid cross-test leakage
- Normalize CHANGE_ORDER.md status format to semicolon convention

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
scripts/run_actionlint.sh (1)

22-52: ⚠️ Potential issue | 🟠 Major

Propagate Docker lint failures instead of falling through to install guidance.

run_with_docker still uses the same non-zero path for both “Docker unavailable” and “actionlint found violations”. When the container actually reports lint errors, the if run_with_docker; then ... fi caller drops into the generic install message and exits 2, so the supported Docker path still masks the real failure mode.

🛠️ Proposed fix
 run_with_docker() {
   if ! command -v docker >/dev/null 2>&1; then
-    return 1
+    return 127
   fi
 
   if ! docker info >/dev/null 2>&1; then
     echo "Docker daemon unavailable for actionlint; cannot run via Docker." >&2
-    return 1
+    return 127
   fi
 
   docker run --rm \
       -v "$REPO_ROOT":/repo \
       -w /repo \
       "$DOCKER_IMAGE" -no-color
 }
 
-if run_with_docker; then
-  exit 0
+if command -v docker >/dev/null 2>&1; then
+  if run_with_docker; then
+    exit 0
+  else
+    status=$?
+    if [ "$status" -ne 127 ]; then
+      exit "$status"
+    fi
+  fi
 fi
#!/bin/bash
set -euo pipefail

echo "Inspecting the current Docker fallback branch:"
sed -n '22,52p' scripts/run_actionlint.sh

echo
echo "Reproducing the same bash control-flow shape in isolation:"
tmpdir="$(mktemp -d)"
cat >"${tmpdir}/repro.sh" <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
run_with_docker() { return 1; }
if run_with_docker; then
  exit 0
fi
echo "fell through to guidance"
exit 2
EOF
chmod +x "${tmpdir}/repro.sh"

set +e
"${tmpdir}/repro.sh"
status=$?
set -e

printf 'Observed exit status: %s\n' "${status}"
test "${status}" -eq 2

As per coding guidelines: required jobs must fail non-zero when underlying tools fail, and supported local pre-commit paths must enforce the same core gate semantics as CI.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run_actionlint.sh` around lines 22 - 52, The Docker fallback
currently masks real actionlint failures because the caller checks "if
run_with_docker; then exit 0; fi" and only treats a success as success, dropping
any non-zero container exit into the generic install guidance; update the
control-flow so the script propagates run_with_docker's exit code: invoke
run_with_docker and if it succeeds exit 0, otherwise exit with the same non-zero
status returned by run_with_docker (i.e., capture and propagate its return
code). Locate the run_with_docker function and the caller block that references
it (the lines invoking run_actionlint_local, has_actionlint, and
run_with_docker) and change the caller to propagate run_with_docker's exit code
instead of falling through to the install guidance.
🤖 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 44-47: The workflow_changed filter currently lists only
'.github/workflows/**', 'scripts/run_actionlint.sh', and 'scripts/yaml-tools.sh'
but omits the 'lint-workflows' entrypoint; update the 'workflow' array to
include 'lint-workflows' so changes to the pyproject/entrypoint also trigger
Workflow Lint. Locate the 'workflow' key in this job definition and add
'lint-workflows' alongside the existing entries (reference strings:
'.github/workflows/**', 'scripts/run_actionlint.sh', 'scripts/yaml-tools.sh',
and the missing 'lint-workflows') to ensure the required validation cannot be
bypassed by rewiring the hatch script.

In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 191-203: The test
test_legacy_actionlint_runner_does_not_mask_docker_failures currently checks the
script text (LEGACY_ACTIONLINT_RUNNER) for absence of a literal "return 0",
which is brittle; change it to exercise the wrapper behavior by executing the
script with stubbed binaries for "docker" and "actionlint" (e.g., temporary
executables on PATH that simulate Docker-unavailable and failing actionlint
scenarios), then assert the wrapper exits with a non-zero exit code and emits
the expected error message; update the test to invoke the script binary (or
`bash` on the script) and check process exit code and stderr rather than relying
on string grep for "return 0".

---

Duplicate comments:
In `@scripts/run_actionlint.sh`:
- Around line 22-52: The Docker fallback currently masks real actionlint
failures because the caller checks "if run_with_docker; then exit 0; fi" and
only treats a success as success, dropping any non-zero container exit into the
generic install guidance; update the control-flow so the script propagates
run_with_docker's exit code: invoke run_with_docker and if it succeeds exit 0,
otherwise exit with the same non-zero status returned by run_with_docker (i.e.,
capture and propagate its return code). Locate the run_with_docker function and
the caller block that references it (the lines invoking run_actionlint_local,
has_actionlint, and run_with_docker) and change the caller to propagate
run_with_docker's exit code instead of falling through to the install guidance.
🪄 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: 60d32ffe-3b98-4c86-96f0-6bbc9f1a9029

📥 Commits

Reviewing files that changed from the base of the PR and between c5efee3 and 30c70f3.

📒 Files selected for processing (5)
  • .github/workflows/pr-orchestrator.yml
  • openspec/CHANGE_ORDER.md
  • scripts/run_actionlint.sh
  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: All public APIs must have @icontract decorators and @beartype type checking.
Commands should follow typer patterns with rich console output.
Use Pydantic models for all data structures to ensure data validation.
Write only high-value comments if at all. Avoid talking to the user through comments.
Use GPG-signed commits (git commit -S) when implementing changes in a worktree.

**/*.py: Use snake_case for file and module names
Use PascalCase for class names
Use UPPER_SNAKE_CASE for constants
Use Google-style docstrings
Set line length to 120 characters maximum
All data structures must use Pydantic BaseModel with Field(...) and descriptions
All public APIs must use @icontract decorators (@require, @ensure, @invariant) for contract-first development
All public APIs must use @beartype for runtime type checking
Only write high-value comments; avoid verbose or redundant commentary

**/*.py: No bare except or broad except Exception without re-raising or logging full context - use specific exceptions instead
Enforce consistent return types for public API methods - publishing/IO methods must return bool (True/False), not None
Prefer specific exceptions and re-raise after logging rather than swallowing exceptions
Limit function length to maximum 120 lines and cyclomatic complexity to maximum 12
Avoid filesystem operations with overly permissive modes - do not use 0o777 for mkdir/os.makedirs; use 0o755 or environment-controlled mode
Async / signal safety - signal handlers may only set a flag or call thread-safe routines; do not call asyncio.create_task directly from POSIX signal handlers

**/*.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-o...

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test structure must mirror source with tests/unit/, tests/integration/, tests/e2e/ directories
Use @pytest.mark.asyncio decorator for async test functions
Guard environment-sensitive logic with os.environ.get("TEST_MODE") == "true"

tests/**/*.py: Secure secret redaction guaranteed - LoggerSetup.redact_secrets must be covered by unit tests for nested dicts and strings
Messaging coercion and strict validation - any use of MessagingStandardization.process_standardized_message must have tests validating coercion success/failure and metrics increments

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 oracles

tests/**/*.py: Use @pytest.mark.asyncio for async tests
Guard environment-sensitive logic with os.environ.get("TEST_MODE") == "true"
Test structure must mirror source structure: tests/unit/, tests/integration/, tests/e2e/

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.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.asyncio decorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py
{src,tests}/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/session_startup_instructions.mdc)

Apply pylint src tests for linting before committing

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py
@(src|tests)/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Linting must pass with no errors using: pylint src tests

Files:

  • tests/unit/modules/test_bundle_import.py
  • tests/unit/workflows/test_trustworthy_green_checks.py
**/*.md

📄 CodeRabbit inference engine (.cursorrules)

**/*.md: Finish each output by listing which rulesets have been applied (e.g., .cursorrules, AGENTS.md, specific .cursor/rules/*.mdc), confirm Git Worktree Policy compliance if applicable, and state the AI provider and model version being used.
Follow markdown linting rules to avoid markdown linting errors (via markdown-rules).

Files:

  • openspec/CHANGE_ORDER.md
openspec/CHANGE_ORDER.md

📄 CodeRabbit inference engine (CLAUDE.md)

Update openspec/CHANGE_ORDER.md whenever a change lifecycle event occurs (new change created, archived, modified, blocker resolved) in the same commit

Keep openspec/CHANGE_ORDER.md updated as the single source of truth for change sequencing, module grouping, and inter-change dependencies

Files:

  • openspec/CHANGE_ORDER.md
**/*.{md,mdc}

📄 CodeRabbit inference engine (.cursor/rules/markdown-rules.mdc)

**/*.{md,mdc}: Do not use more than one consecutive blank line anywhere in the document (MD012: No Multiple Consecutive Blank Lines)
Fenced code blocks should be surrounded by blank lines (MD031: Fenced Code Blocks)
Lists should be surrounded by blank lines (MD032: Lists)
Files must end with a single empty line (MD047: Files Must End With Single Newline)
Lines should not have trailing spaces (MD009: No Trailing Spaces)
Use asterisks (**) for strong emphasis, not underscores (__) (MD050: Strong Style)
Fenced code blocks must have a language specified (MD040: Fenced Code Language)
Headers should increment by one level at a time (MD001: Header Increment)
Headers should be surrounded by blank lines (MD022: Headers Should Be Surrounded By Blank Lines)
Only one top-level header (H1) is allowed per document (MD025: Single H1 Header)
Use consistent list markers, preferring dashes (-) for unordered lists (MD004: List Style)
Nested unordered list items should be indented consistently, typically by 2 spaces (MD007: Unordered List Indentation)
Use exactly one space after the list marker (e.g., -, *, +, 1.) (MD030: Spaces After List Markers)
Use incrementing numbers for ordered lists (MD029: Ordered List Item Prefix)
Enclose bare URLs in angle brackets or format them as links (MD034: Bare URLs)
Don't use spaces immediately inside code spans (MD038: Spaces Inside Code Spans)
Use consistent indentation (usually 2 or 4 spaces) throughout markdown files
Keep line length under 120 characters in markdown files
Use reference-style links for better readability in markdown files
Use a trailing slash for directory paths in markdown files
Ensure proper escaping of special characters in markdown files

Files:

  • openspec/CHANGE_ORDER.md
openspec/**/*.md

⚙️ CodeRabbit configuration file

openspec/**/*.md: Treat as specification source of truth: proposal/tasks/spec deltas vs. code behavior,
CHANGE_ORDER consistency, and scenario coverage. Surface drift between OpenSpec and
implementation.

Files:

  • openspec/CHANGE_ORDER.md
.github/workflows/*.{yml,yaml}

📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)

Validate GitHub workflow files using hatch run lint-workflows before committing

.github/workflows/*.{yml,yaml}: Use actionlint for semantic validation of GitHub Actions workflows
Format GitHub Actions workflows using hatch run workflows-fmt and lint them with hatch run workflows-lint after editing

Files:

  • .github/workflows/pr-orchestrator.yml
.github/workflows/!(tests).{yml,yaml}

📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)

Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)

Files:

  • .github/workflows/pr-orchestrator.yml
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test
gates, and action versions.

Files:

  • .github/workflows/pr-orchestrator.yml
🧠 Learnings (41)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Enforce pre-commit and CI gating with checks for: black, isort, mypy, flake8 (with B/C plugins), radon complexity, state-machine sync tests, and rejection of PRs lacking test files when src/ is modified
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: When creating implementation plans or OpenSpec tasks.md, explicitly verify and include: worktree creation from `origin/dev`, `hatch env create` in the worktree, pre-flight checks (`hatch run smart-test-status`, `hatch run contract-test-status`), and worktree cleanup steps post-merge.
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Use lazy-loaded module registry in `src/specfact_cli/modules/` with `module-package.yaml` metadata files and deferred imports until command invocation

Applied to files:

  • tests/unit/modules/test_bundle_import.py
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to **/test_*.py : Organize Python imports in tests using unittest.mock for Mock and patch

Applied to files:

  • tests/unit/modules/test_bundle_import.py
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to **/test_*.py : Ensure each test is independent and repeatable with no shared state between tests

Applied to files:

  • tests/unit/modules/test_bundle_import.py
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Applies to openspec/CHANGE_ORDER.md : Keep `openspec/CHANGE_ORDER.md` updated as the single source of truth for change sequencing, module grouping, and inter-change dependencies

Applied to files:

  • openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:31:22.997Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:22.997Z
Learning: Applies to openspec/CHANGE_ORDER.md : Update `openspec/CHANGE_ORDER.md` whenever a change lifecycle event occurs (new change created, archived, modified, blocker resolved) in the same commit

Applied to files:

  • openspec/CHANGE_ORDER.md
  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Applies to openspec/changes/*/TDD_EVIDENCE.md : Create/update `openspec/changes/<change-id>/TDD_EVIDENCE.md` with test command(s), timestamp, and failure/pass summary before and after implementation

Applied to files:

  • openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: Ensure an OpenSpec change (new or delta) exists for any code modification in specfact-cli (`src/`, `tools/`, tests, or significant docs) unless user explicitly opts out with 'skip openspec', 'direct implementation', 'simple fix', or similar

Applied to files:

  • openspec/CHANGE_ORDER.md
  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:22.997Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:22.997Z
Learning: Before modifying application code, always verify that an active OpenSpec change in `openspec/changes/` explicitly covers the requested modification; do not proceed without one unless user explicitly says 'skip openspec' or 'implement without openspec change'

Applied to files:

  • openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Verify that an active OpenSpec change in `openspec/changes/` explicitly covers any requested code modification before applying edits

Applied to files:

  • openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: When creating implementation plans or OpenSpec tasks.md, explicitly verify and include: worktree creation from `origin/dev`, `hatch env create` in the worktree, pre-flight checks (`hatch run smart-test-status`, `hatch run contract-test-status`), and worktree cleanup steps post-merge.

Applied to files:

  • openspec/CHANGE_ORDER.md
  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: After code implementation in specfact-cli, create or update corresponding GitHub issue with title `[Change] <Brief Description>`, labels `enhancement` and `change-proposal` if targeting public repo, and update `proposal.md` Source Tracking section with issue number, URL, repository, and status

Applied to files:

  • openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: After implementing code changes in specfact-cli, run quality gates: `hatch run format`, `hatch run type-check`, `hatch run contract-test`, `hatch test` (or `hatch run smart-test`)

Applied to files:

  • openspec/CHANGE_ORDER.md
  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: For new code in specfact-cli using OpenSpec workflow, follow TDD discipline: write tests first, then implementation code

Applied to files:

  • openspec/CHANGE_ORDER.md
  • .github/workflows/pr-orchestrator.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: Install actionlint locally, download to ./bin, or rely on Docker fallback for GitHub Actions workflow validation

Applied to files:

  • scripts/run_actionlint.sh
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Use actionlint for semantic validation of GitHub Actions workflows

Applied to files:

  • scripts/run_actionlint.sh
  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: Before executing any workflow command (`/opsx:ff`, `/opsx:apply`, `/opsx:continue`, etc.), perform a pre-execution checklist: verify Git Worktree creation, TDD evidence documentation, user-facing documentation updates, module signing verification, and confirm AGENTS.md compliance.

Applied to files:

  • scripts/run_actionlint.sh
  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 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:

  • scripts/run_actionlint.sh
  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing

Applied to files:

  • scripts/run_actionlint.sh
  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .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/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:47.109Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Code changes modifying public API, state machine YAMLs, or generated state machine outputs MUST include unit tests, update docs/ and CHANGELOG.md, and include 'BREAKING' section in PR if public API changed

Applied to files:

  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: Applies to **/*.md : Finish each output by listing which rulesets have been applied (e.g., `.cursorrules`, `AGENTS.md`, specific `.cursor/rules/*.mdc`), confirm Git Worktree Policy compliance if applicable, and state the AI provider and model version being used.

Applied to files:

  • .github/workflows/pr-orchestrator.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: Applies to .github/workflows/*.{yml,yaml} : Format GitHub Actions workflows using `hatch run workflows-fmt` and lint them with `hatch run workflows-lint` after editing

Applied to files:

  • .github/workflows/pr-orchestrator.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: Applies to **/*.{yml,yaml} : Format all YAML and workflow files using `hatch run yaml-fix-all` before committing

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Always work on feature/bugfix/hotfix branches and submit PRs; protect `dev` and `main` branches from direct commits

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: All changes must be made via Pull Requests to `dev` or `main`. Direct commits to protected branches are not allowed.

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/{quality-gates,contract-enforcement,contracts,no-escape-gating,integrated-ci}.{yml,yaml} : Dependent workflows must download the coverage.xml artifact from the Tests workflow and fail early if it is missing

Applied to files:

  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-30T00:25:28.054Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T00:25:28.054Z
Learning: Sign modules using `hatch run python scripts/sign-modules.py --key-file <private-key.pem>` and verify with `hatch run ./scripts/verify-modules-signature.py --require-signature` before PR creation

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: Applies to **/*.{py} : After any code changes, apply linting and formatting using `hatch run format`, perform type checking with `hatch run type-check` (basedpyright), run contract validation with `hatch run contract-test`, and run full test suite with `hatch test --cover -v`. Verify all tests pass and contracts are satisfied before committing.

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Use `hatch test` command for running tests instead of direct pytest usage to ensure proper Python matrix, dependency resolution, environment variables, and coverage reports

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to src/**/*.py : Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.

Applied to files:

  • .github/workflows/pr-orchestrator.yml
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Run contract-first validation locally before committing: `hatch run contract-test-contracts`, `hatch run contract-test-exploration`, and `hatch run contract-test-scenarios`

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Before making a pull request, locally run format, lint, contract tests, and full test suite: `hatch run format`, `hatch run lint`, `hatch run contract-test`, `hatch test --cover -v`

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: Run `openspec validate <change-id> --strict` before implementing any code changes in specfact-cli to validate the OpenSpec change

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:33.886Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/automatic-openspec-workflow.mdc:0-0
Timestamp: 2026-03-25T21:31:33.886Z
Learning: Read and apply `openspec/config.yaml` for project context (tech stack, constraints, architecture, SDD+TDD discipline) before any code modification in specfact-cli

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to **/*.py : Run type checking with basedpyright on all Python files

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to src/**/*.py : Type checking must pass with no errors using: mypy .

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:29.182Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/python-github-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:29.182Z
Learning: Applies to **/*.py : Run linting with ruff and pylint on all Python files

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to @(src|tests)/**/*.py : Linting must pass with no errors using: pylint src tests

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:32:38.156Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/session_startup_instructions.mdc:0-0
Timestamp: 2026-03-25T21:32:38.156Z
Learning: Applies to {src,tests}/**/*.py : Apply `pylint src tests` for linting before committing

Applied to files:

  • .github/workflows/pr-orchestrator.yml
📚 Learning: 2026-03-25T21:31:47.109Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Enforce pre-commit and CI gating with checks for: black, isort, mypy, flake8 (with B/C plugins), radon complexity, state-machine sync tests, and rejection of PRs lacking test files when src/ is modified

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
🔀 Multi-repo context nold-ai/specfact-cli-modules

nold-ai/specfact-cli-modules

  • Workflow: pr-orchestrator defines outputs code_changed and skip_tests_dev_to_main and uses a branch-name dev->main fast-path (skip_tests_dev_to_main set when PR base=head dev->main). It also defines verify-module-signatures job and wires quality needs: needs: [changes, verify-module-signatures]; many quality steps run behind skip_tests_dev_to_main gating. [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]

  • verify-module-signatures consumer/implementation:

    • Workflow job present: verify-module-signatures (runs scripts/verify-modules-signature.py). [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]
    • Pre-commit config installs a local hook id: verify-module-signatures with entry hatch run ./scripts/verify-modules-signature.py ... — tests expect this hook id. [::nold-ai/specfact-cli-modules::.pre-commit-config.yaml] [::nold-ai/specfact-cli-modules::tests/unit/test_pre_commit_quality_parity.py:29]
  • SPECFACT_MODULES_REPO usage:

    • tests/conftest.py sets/reads SPECFACT_MODULES_REPO for module repo tests. [::nold-ai/specfact-cli-modules::tests/conftest.py:90]
    • Multiple scripts and tests reference bundle-import checks; check-bundle-imports and pre-commit-quality-checks invoke hatch run check-bundle-imports and expect import boundary behavior. [::nold-ai/specfact-cli-modules::scripts/pre-commit-quality-checks.sh] [::nold-ai/specfact-cli-modules::scripts/check-bundle-imports.py]
  • Bundle-import/allowed imports contract:

    • check-bundle-imports enforces allowed specfact_cli prefixes including "specfact_cli.modules" and cross-bundle import rules — this is a contract consumers rely on. [::nold-ai/specfact-cli-modules::scripts/check-bundle-imports.py]
  • Scripts that mediate actionlint / workflow linting etc. — not present in this repo (no .coderabbit.yaml, no scripts/run_actionlint.sh found here). .coderabbit.yaml was not found in this repository. [Search results: no .coderabbit.yaml in this repo]

Implication summary:

  • Changes that adjust CodeRabbit base branches, run_actionlint behavior, or pr-orchestrator outputs in the PR under review are relevant because this repo already contains a verify-module-signatures workflow job, a pre-commit hook with the same id, and tests that assert those exact names/behaviors; any renaming/semantic changes to required check names, outputs, or skip logic must remain consistent here to avoid breaking CI/tests. [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml] [::nold-ai/specfact-cli-modules::.pre-commit-config.yaml] [::nold-ai/specfact-cli-modules::tests/conftest.py:90]
🔇 Additional comments (7)
tests/unit/modules/test_bundle_import.py (2)

9-13: Helper fixture layout is clear and targeted.

The repository shape created here matches the bundle source discovery contract and keeps setup focused.


16-33: Test isolation and env-fallback assertion look solid.

Good use of monkeypatch plus sys.path restoration in finally; this keeps the test repeatable while validating SPECFACT_MODULES_REPO fallback behavior. As per coding guidelines: "Ensure each test is independent and repeatable with no shared state between tests."

scripts/run_actionlint.sh (1)

11-20: Pinned version and helper split look good.

Separating discovery from execution and defaulting ACTIONLINT_TAG to the CI-pinned tag makes the local path deterministic.

openspec/CHANGE_ORDER.md (1)

64-64: OpenSpec status formatting is now aligned.

Both ci-02-trustworthy-green-checks rows use the qualified ✅ ... (implemented 2026-03-30; archive pending) form, and the CI/CD table preserves the blocker text unchanged.

As per coding guidelines: use the ci-02-trustworthy-green-checks change row with the qualified ✅ archive-pending format and keep any “Blocked by” dependency text intact.

Also applies to: 174-174

.github/workflows/pr-orchestrator.yml (2)

53-88: The dev→main fast-path now fails closed.

Defaulting SKIP_TESTS to false and only flipping it after SHA/merge-base proof is the right safety boundary here.

As per coding guidelines: dev -> main release PRs may skip required validation only when PR head parity is provable; otherwise the required set must run.


125-167: Good required-check emission and failure propagation.

Keeping workflow-lint as an always-present job with an explicit skip step preserves the check on every PR head commit, and the shared gating plus PIPESTATUS exits in the blocking shells removes the earlier failure-swallowing behavior.

As per coding guidelines: required branch-protection checks must emit status for every PR head commit, and required jobs must fail non-zero when the underlying tools fail.

Also applies to: 288-289, 327-330, 343-344, 383-385, 405-405, 424-425, 483-484, 518-519, 548-549

tests/unit/workflows/test_trustworthy_green_checks.py (1)

21-188: These policy tests add solid guardrails.

Loading the workflow/config files directly and asserting trigger behavior, canonical check naming, advisory labeling, and branch coverage gives the CI hardening work a useful regression net.

Comment thread .github/workflows/pr-orchestrator.yml
Comment thread tests/unit/workflows/test_trustworthy_green_checks.py Outdated
#472)

Simplify run_actionlint.sh control flow so both local and docker
execution paths propagate actionlint's exit code via `exit $?`. Previously
the docker path used `if run_with_docker; then exit 0; fi` which treated
lint errors as "docker unavailable" and fell through to install guidance.

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
tests/unit/workflows/test_trustworthy_green_checks.py (1)

191-200: ⚠️ Potential issue | 🟠 Major

Replace string-grep assertions with behavioral execution of the wrapper.

This still validates implementation text, not runtime behavior. It can pass while failure propagation regresses. Execute the script with stubbed docker/actionlint binaries and assert actual exit codes/stderr for each path.

As per coding guidelines: “Contract-first testing: meaningful scenarios, not redundant assertions already covered by contracts. Flag flakiness… and missing coverage for changed behavior.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/workflows/test_trustworthy_green_checks.py` around lines 191 -
200, Replace the brittle text-based assertions in
test_legacy_actionlint_runner_does_not_mask_docker_failures by actually
executing the LEGACY_ACTIONLINT_RUNNER script twice with stubbed binaries and
asserting runtime behavior: create a temporary directory with executable stubs
named "docker" and "actionlint" (one test where the actionlint stub exits
non-zero to exercise the local-binary path, another where docker stub exits
non-zero to exercise the docker path), prepend that temp dir to PATH (use
tmp_path fixture), invoke the wrapper via subprocess.run(capture_output=True)
and assert the wrapper returns the same non-zero exit code and forwards stderr
for each scenario; keep references to LEGACY_ACTIONLINT_RUNNER and the test
function name so the test replaces the string-grep checks with these behavioral
checks.
scripts/run_actionlint.sh (1)

22-24: ⚠️ Potential issue | 🟠 Major

Run local actionlint from repo root to preserve CI parity.

The local path runs in the caller’s current directory, while the Docker path forces /repo. That can lint different content and produce false-green local checks.

🔧 Proposed fix
 if has_actionlint; then
-  actionlint -no-color
+  (cd "$REPO_ROOT" && actionlint -no-color)
   exit $?
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run_actionlint.sh` around lines 22 - 24, The script currently runs
actionlint in the caller’s current directory (within the has_actionlint block),
which can differ from CI’s /repo and cause mismatches; update the has_actionlint
branch so that before invoking actionlint -no-color it first changes to the
repository root (e.g., via git rev-parse --show-toplevel or using a REPO_ROOT
variable) and then runs actionlint, preserving exit behavior (exit $?) and
keeping references to has_actionlint and the actionlint -no-color invocation to
locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 168-176: Add an explicit assertion that the smart-checks hook
exposes the required hook id to prevent silent renames: inside
test_pre_commit_config_installs_supported_smart_check_wrapper (after locating
hook = matching[0]), assert hook.get("id") == "specfact-smart-checks" so the
test enforces the contract; keep the existing assertions for pass_filenames and
language and use the existing _load_hooks()/matching logic to find the hook to
check.

---

Duplicate comments:
In `@scripts/run_actionlint.sh`:
- Around line 22-24: The script currently runs actionlint in the caller’s
current directory (within the has_actionlint block), which can differ from CI’s
/repo and cause mismatches; update the has_actionlint branch so that before
invoking actionlint -no-color it first changes to the repository root (e.g., via
git rev-parse --show-toplevel or using a REPO_ROOT variable) and then runs
actionlint, preserving exit behavior (exit $?) and keeping references to
has_actionlint and the actionlint -no-color invocation to locate the change.

In `@tests/unit/workflows/test_trustworthy_green_checks.py`:
- Around line 191-200: Replace the brittle text-based assertions in
test_legacy_actionlint_runner_does_not_mask_docker_failures by actually
executing the LEGACY_ACTIONLINT_RUNNER script twice with stubbed binaries and
asserting runtime behavior: create a temporary directory with executable stubs
named "docker" and "actionlint" (one test where the actionlint stub exits
non-zero to exercise the local-binary path, another where docker stub exits
non-zero to exercise the docker path), prepend that temp dir to PATH (use
tmp_path fixture), invoke the wrapper via subprocess.run(capture_output=True)
and assert the wrapper returns the same non-zero exit code and forwards stderr
for each scenario; keep references to LEGACY_ACTIONLINT_RUNNER and the test
function name so the test replaces the string-grep checks with these behavioral
checks.
🪄 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: 6d41f11f-49a0-46ea-8f1c-5a6409a63d33

📥 Commits

Reviewing files that changed from the base of the PR and between 30c70f3 and 7709718.

📒 Files selected for processing (2)
  • scripts/run_actionlint.sh
  • tests/unit/workflows/test_trustworthy_green_checks.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: Tests (Python 3.12)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: All public APIs must have @icontract decorators and @beartype type checking.
Commands should follow typer patterns with rich console output.
Use Pydantic models for all data structures to ensure data validation.
Write only high-value comments if at all. Avoid talking to the user through comments.
Use GPG-signed commits (git commit -S) when implementing changes in a worktree.

**/*.py: Use snake_case for file and module names
Use PascalCase for class names
Use UPPER_SNAKE_CASE for constants
Use Google-style docstrings
Set line length to 120 characters maximum
All data structures must use Pydantic BaseModel with Field(...) and descriptions
All public APIs must use @icontract decorators (@require, @ensure, @invariant) for contract-first development
All public APIs must use @beartype for runtime type checking
Only write high-value comments; avoid verbose or redundant commentary

**/*.py: No bare except or broad except Exception without re-raising or logging full context - use specific exceptions instead
Enforce consistent return types for public API methods - publishing/IO methods must return bool (True/False), not None
Prefer specific exceptions and re-raise after logging rather than swallowing exceptions
Limit function length to maximum 120 lines and cyclomatic complexity to maximum 12
Avoid filesystem operations with overly permissive modes - do not use 0o777 for mkdir/os.makedirs; use 0o755 or environment-controlled mode
Async / signal safety - signal handlers may only set a flag or call thread-safe routines; do not call asyncio.create_task directly from POSIX signal handlers

**/*.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-o...

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test structure must mirror source with tests/unit/, tests/integration/, tests/e2e/ directories
Use @pytest.mark.asyncio decorator for async test functions
Guard environment-sensitive logic with os.environ.get("TEST_MODE") == "true"

tests/**/*.py: Secure secret redaction guaranteed - LoggerSetup.redact_secrets must be covered by unit tests for nested dicts and strings
Messaging coercion and strict validation - any use of MessagingStandardization.process_standardized_message must have tests validating coercion success/failure and metrics increments

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 oracles

tests/**/*.py: Use @pytest.mark.asyncio for async tests
Guard environment-sensitive logic with os.environ.get("TEST_MODE") == "true"
Test structure must mirror source structure: tests/unit/, tests/integration/, tests/e2e/

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
**/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.asyncio decorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
{src,tests}/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/session_startup_instructions.mdc)

Apply pylint src tests for linting before committing

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
@(src|tests)/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Linting must pass with no errors using: pylint src tests

Files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Enforce pre-commit and CI gating with checks for: black, isort, mypy, flake8 (with B/C plugins), radon complexity, state-machine sync tests, and rejection of PRs lacking test files when src/ is modified
📚 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: Install actionlint locally, download to ./bin, or rely on Docker fallback for GitHub Actions workflow validation

Applied to files:

  • scripts/run_actionlint.sh
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:22.650Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/yaml-and-workflows.md:0-0
Timestamp: 2026-03-25T21:33:22.650Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Use actionlint for semantic validation of GitHub Actions workflows

Applied to files:

  • scripts/run_actionlint.sh
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: Before executing any workflow command (`/opsx:ff`, `/opsx:apply`, `/opsx:continue`, etc.), perform a pre-execution checklist: verify Git Worktree creation, TDD evidence documentation, user-facing documentation updates, module signing verification, and confirm AGENTS.md compliance.

Applied to files:

  • scripts/run_actionlint.sh
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 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:

  • scripts/run_actionlint.sh
  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:47.109Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Enforce pre-commit and CI gating with checks for: black, isort, mypy, flake8 (with B/C plugins), radon complexity, state-machine sync tests, and rejection of PRs lacking test files when src/ is modified

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/!(tests).{yml,yaml} : Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21: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/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: When creating implementation plans or OpenSpec tasks.md, explicitly verify and include: worktree creation from `origin/dev`, `hatch env create` in the worktree, pre-flight checks (`hatch run smart-test-status`, `hatch run contract-test-status`), and worktree cleanup steps post-merge.

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Validate GitHub workflow files using `hatch run lint-workflows` before committing

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:01.827Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursorrules:0-0
Timestamp: 2026-03-25T21:31:01.827Z
Learning: Applies to **/*.{py} : After any code changes, apply linting and formatting using `hatch run format`, perform type checking with `hatch run type-check` (basedpyright), run contract validation with `hatch run contract-test`, and run full test suite with `hatch test --cover -v`. Verify all tests pass and contracts are satisfied before committing.

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Before making a pull request, locally run format, lint, contract tests, and full test suite: `hatch run format`, `hatch run lint`, `hatch run contract-test`, `hatch test --cover -v`

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:31:47.109Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/clean-code-principles.mdc:0-0
Timestamp: 2026-03-25T21:31:47.109Z
Learning: Code changes modifying public API, state machine YAMLs, or generated state machine outputs MUST include unit tests, update docs/ and CHANGELOG.md, and include 'BREAKING' section in PR if public API changed

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .github/workflows/{quality-gates,contract-enforcement,contracts,no-escape-gating,integrated-ci}.{yml,yaml} : Dependent workflows must download the coverage.xml artifact from the Tests workflow and fail early if it is missing

Applied to files:

  • tests/unit/workflows/test_trustworthy_green_checks.py
🔀 Multi-repo context nold-ai/specfact-cli-modules

Linked repositories findings

nold-ai/specfact-cli-modules

  • Workflow job verify-module-signatures exists and is referenced by tests/workflow config:

    • .github/workflows/pr-orchestrator.yml — job verify-module-signatures (needs: [changes]; runs python scripts/verify-modules-signature.py ...) [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]
    • .github/workflows/pr-orchestrator.yml — outputs and skip-dev-to-main gating use skip_tests_dev_to_main and code_changed (PR under review changes how these are computed) [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]
  • Pre-commit hook and local hook id for module signature verification present:

    • .pre-commit-config.yaml — includes a local hook id verify-module-signatures with entry hatch run ./scripts/verify-modules-signature.py ... (tests assert presence of this hook id) [::nold-ai/specfact-cli-modules::.pre-commit-config.yaml]
    • tests/unit/test_pre_commit_quality_parity.py — asserts "verify-module-signatures" in hook_ids (consumer/test expectation) [::nold-ai/specfact-cli-modules::tests/unit/test_pre_commit_quality_parity.py:29]
  • SPECFACT_MODULES_REPO environment usage in tests:

    • tests/conftest.py — sets/defaults SPECFACT_MODULES_REPO for module-repo tests (this repo’s tests rely on that env var) [::nold-ai/specfact-cli-modules::tests/conftest.py:90]
  • Docs reference and parity expectations:

    • docs/authoring/module-signing.md — documents that pr-orchestrator.yml contains strict gate: verify-module-signatures (documentation ties to workflow job name) [::nold-ai/specfact-cli-modules::docs/authoring/module-signing.md:127-129]

Relevance to the PR under review:

  • The reviewed PR changes workflow gating logic and run_actionlint / pre-commit hook ids in the other repo. This repo defines and expects a verify-module-signatures job and a pre-commit hook with the same id/entry; removing or renaming that local hook or changing required-check name text could cause test or CI mismatches here. Also PR changes to skip-tests dev->main parity computation (from branch-name to git-merge-base proof) may affect whether this repo’s CI short-circuits quality jobs in dev->main PRs; verify the new logic remains compatible with this repo’s pr-orchestrator usage. [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]

Comment thread tests/unit/workflows/test_trustworthy_green_checks.py
#473)

- Assert hook id == "specfact-smart-checks" to prevent silent renames
- cd to REPO_ROOT before running local actionlint so it finds workflows
  regardless of caller's cwd

Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request QA Quality Assurance

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Change] Trustworthy green checks for CI, pre-commit, and PR review

1 participant