Conversation
…ing, and filter parity (#222) * feat(backlog): finalize daily/refine comment context, interactive posting, and docs parity * docs(openspec): mark backlog-scrum-01 standup change checklist complete * fix(openspec): mark backlog-refinement delta as ADDED for archive apply * Archived completed change backlog-scrum-01 * fix(backlog): make map-fields exit cleanly under CliRunner * Fix format * fix(backlog): stabilize map-fields tests in non-interactive env * docs(agents): enforce signed-commit handoff flow --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
#224) * fix: parse backlog refine writeback fields and refactor refine command * fix: preserve heading-style narrative sections in refine parser * chore: sync OpenSpec change to GitHub issue tracking --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
…228) * fix: harden backlog refine prompt scaffold and parsing * fix: normalize mixed notes parsing and boundary flushing --------- Co-authored-by: Dominikus Nold <djm81@users.noreply.github.com>
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>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughAdds a Typer-based doc-frontmatter validation CLI, integrates it into pre-commit and CI (docs-review and pr-orchestrator), changes the pre-commit code-review gate to emit/consume a JSON report at Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CI
participant Hook as Pre-commit Hook / CI Step
participant Validator as check_doc_frontmatter CLI
participant Enforced as docs/.doc-frontmatter-enforced
participant Files as Docs Files
participant Pydantic as DocFrontmatter Model
Dev->>Hook: Commit or CI job triggers
Hook->>Validator: Invoke (args or --all-docs)
alt enforced-list mode
Validator->>Enforced: Read docs/.doc-frontmatter-enforced
Enforced-->>Validator: List of markdown paths
else discovery mode
Validator->>Files: Discover tracked docs/**/*.md
Files-->>Validator: File list
end
loop for each file
Validator->>Files: Extract YAML frontmatter
Validator->>Pydantic: Validate fields (doc_owner, tracks, last_reviewed, exempt)
Pydantic-->>Validator: Valid / ValidationError
Validator->>Validator: Resolve owner & validate glob patterns
end
Validator->>Hook: Emit aggregated findings and exit code (0/1)
Hook->>Dev: Summary and structured report path (.specfact/code-review.json)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openspec/CHANGE_ORDER.md (1)
113-117:⚠️ Potential issue | 🟡 MinorKeep blocker linkage format consistent for
ci-docs-sync-check.This table still uses
docs-02 (doc-frontmatter-schema)without the#461reference, while the blocked-by section below now includesdoc-frontmatter-schema ([#461]). Aligning both prevents governance drift.As per coding guidelines: “Use the docs governance order and dependency notes … update ‘Blocked by’ links to include
#461where applicable.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/CHANGE_ORDER.md` around lines 113 - 117, The blocked-by cell for the "ci-docs-sync-check" row uses "docs-02 (doc-frontmatter-schema)" without the issue reference; update the dependency text to match the governance format used elsewhere by adding the issue link “[`#461`]” (e.g., change "docs-02 (doc-frontmatter-schema)" to "docs-02 (doc-frontmatter-schema) [`#461`]" or to "doc-frontmatter-schema ([`#461`])") so the "ci-docs-sync-check" entry and the blocked-by section consistently include the `#461` reference; edit the table row containing "ci-docs-sync-check" to include that change.pyproject.toml (1)
713-721: 🧹 Nitpick | 🔵 TrivialNarrow the
E402waiver to the tests that actually bootstrap imports.Adding
E402to"tests/**/*"turns off import-order checking for the entire test tree, even though the rationale here is specific to the doc-frontmatter harness. Scope it to the helper and test modules that really need dynamic import/bootstrap behavior so the rest of the suite keeps the check.🛠️ One way to keep the waiver local
"tests/**/*" = [ "S101", # assert used "PLR2004", # magic value comparison "T20", # print found "SLF001", # private member accessed - "E402", # imports after sys.path bootstrap in targeted test modules ] +"tests/helpers/doc_frontmatter*.py" = ["E402"] +"tests/unit/scripts/test_doc_frontmatter/**/*.py" = ["E402"] +"tests/integration/scripts/test_doc_frontmatter/**/*.py" = ["E402"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 713 - 721, The E402 waiver is too broad under [tool.ruff.lint.per-file-ignores] for "tests/**/*"; remove "E402" from that global tests rule and instead add a targeted per-file-ignore that only includes E402 for the doc-frontmatter harness and its bootstrap helper modules (the files referenced by the doc-frontmatter test harness), so import-order checking remains enabled for the rest of the test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Around line 78-79: The semgrep block enables Semgrep but lacks a config_file
binding so CodeRabbit won't load the repo's custom rules; update the semgrep
configuration by adding a config_file key pointing to your repository rules
(e.g., set config_file: "tools/semgrep/semgrep.yml" or to the directory
"tools/semgrep/") so the semgrep runner uses the project's rules defined under
tools/semgrep/; modify the semgrep block (the semgrep: enabled: true entry) to
include this config_file reference.
In @.pre-commit-config.yaml:
- Around line 14-16: The frontmatter hook's files matcher (files:
^(docs/.*\.md|USAGE-FAQ\.md)$) excludes the special file
docs/.doc-frontmatter-enforced so commits that only modify that file bypass the
hook; update the matcher in .pre-commit-config.yaml to also match
docs/.doc-frontmatter-enforced (for example by extending the regex to include
that exact filename or a broader pattern that covers dotfiles in docs/) so
changes to docs/.doc-frontmatter-enforced will trigger the hook.
In `@CHANGELOG.md`:
- Around line 29-35: Wrap overlong markdown lines in CHANGELOG.md to under 120
characters so lint passes; break the long bullet entries about OpenSpec
(mentioning openspec/config.yaml, .specfact/code-review.json, and
openspec/changes/<change-id>/TDD_EVIDENCE.md) and the
scripts/pre_commit_code_review.py description into multiple shorter lines
(respecting markdown list indentation and sentence boundaries), ensuring the
TimeoutExpired mention and the 300s timeout note remain intact and readable
across wrapped lines; apply the same wrapping to the other affected ranges
(notably lines referenced around 42-45).
In `@CONTRIBUTING.md`:
- Line 73: Update the wording about required frontmatter for pages listed in
docs/.doc-frontmatter-enforced to state that exempt_reason is conditional:
require doc_owner, tracks, last_reviewed for enforced pages, and require
exempt_reason only when exempt: true (i.e., when the exempt field is set to
true); reference the frontmatter fields doc_owner, tracks, last_reviewed,
exempt, and exempt_reason and align the sentence with the
openspec/specs/doc-frontmatter-schema/spec.md semantics.
In `@docs/contributing/docs-sync.md`:
- Around line 44-45: Clarify that docs/.doc-frontmatter-enforced supports glob
patterns and that lines beginning with # or that are blank are ignored by the
checker; update the docs text near the rollout explanation to say the file
accepts globs (e.g., patterns matched against docs/**/*.md) and that comment
lines starting with `#` and empty lines are skipped so users won’t accidentally
misconfigure tracks/globs.
In `@openspec/changes/archive/2026-03-29-doc-frontmatter-schema/proposal.md`:
- Around line 27-29: Fix markdown spacing around headings that are immediately
followed by list items: for the "PR Orchestrator Workflow" heading (and the
other affected headings referenced in the review), insert a single blank line
after each heading so headings are separated from the subsequent list rows,
ensuring MD022/MD032 lint rules are satisfied; repeat this for the other
problematic heading/list pairs mentioned in the comment.
In
`@openspec/changes/archive/2026-03-29-doc-frontmatter-schema/specs/doc-frontmatter-validation/spec.md`:
- Around line 137-140: The new heading "Requirement: PR Orchestrator Parallel
Job Graph" needs blank-line separation to satisfy MD022; edit the spec.md
content around that heading so there is an empty line both immediately before
and immediately after the "### Requirement: PR Orchestrator Parallel Job Graph"
line (ensure the heading is isolated from surrounding paragraphs or lists) to
produce proper Markdown-compliant spacing.
In `@openspec/changes/archive/2026-03-29-doc-frontmatter-schema/TDD_EVIDENCE.md`:
- Around line 133-136: The document contains duplicate same-level heading text
"Pre-Implementation Test Failure (Expected)" which triggers MD024; locate the
repeated heading(s) and either rename one to a unique, descriptive heading
(e.g., "Pre-Implementation Test Failure (Expected) — PR Orchestrator Delta" or
similar) or merge their content under a single heading so that each H3 heading
text is unique; update any internal references/anchors if present to match the
new heading text.
- Around line 100-104: In
openspec/changes/archive/2026-03-29-doc-frontmatter-schema/TDD_EVIDENCE.md there
are fenced code blocks missing blank lines around them (MD031) — add a single
empty line immediately before the opening ``` and immediately after the closing
``` for the blocks around the ranges noted (100-104 plus the other occurrences
at 140-143 and 159-163) so each fenced block is separated from adjacent
paragraph labels; ensure the same fix is applied to any other adjacent fenced
blocks in the file to satisfy the MD031 rule.
In `@openspec/config.yaml`:
- Around line 166-181: The freshness rule for `.specfact/code-review.json`
currently only invalidates on changes under `src/`, `scripts/`, `tools/`,
`tests/`, or `openspec/changes/<change-id>/`, which lets edits to workflow or
config files slip through; update the check in the OpenSpec config (the
freshness logic around `.specfact/code-review.json`) to either 1) treat any
changed file as a trigger except `TDD_EVIDENCE.md`, i.e. compare the review file
mtime against the full set of changed files, or 2) explicitly include
behavior-affecting paths such as `.github/workflows/`,
`.pre-commit-config.yaml`, and `pyproject.toml` in the invalidation list so
changes to CI/pre-commit/build config also force regen of
`.specfact/code-review.json`. Ensure the change references
`.specfact/code-review.json` and the exception `TDD_EVIDENCE.md`.
In `@openspec/specs/doc-frontmatter-schema/spec.md`:
- Around line 3-5: Replace the placeholder Purpose text with a finished,
meaningful one-sentence purpose for this spec (replace "TBD - created by
archiving change doc-frontmatter-schema" in the Purpose section) and update the
heading structure to satisfy markdownlint MD022 by having a single top-level
heading and correct subordinate heading levels (e.g., use one H1 for the spec
title and make "Purpose" and "Requirements" H2s or convert the current
duplicated H2s appropriately); ensure the updated Purpose and heading layout
make the archived/main spec pair publishable as-is (edit the "Purpose" and
"Requirements" headings and their contents to be final and lint-compliant).
- Around line 81-92: Update the spec to reference the actual implemented API
name: replace mentions of extract_frontmatter(path) with parse_frontmatter(path)
in the Frontmatter Extraction requirement and scenarios so the spec points to
the real function (parse_frontmatter) exposed by
scripts/check_doc_frontmatter.py; ensure both WHEN clauses and any related text
use parse_frontmatter(path) and keep the documented behavior unchanged.
In `@openspec/specs/doc-frontmatter-validation/spec.md`:
- Around line 3-5: Replace the "TBD" placeholder under the "Purpose" heading
with a final, publication-ready purpose statement and fix the heading layout to
satisfy markdownlint MD022 (avoid multiple top-level headings or inconsistent
heading levels); specifically edit the "Purpose" and "Requirements" headings in
this spec file so they follow a single top-level title with subordinate headings
(e.g., use one H1 and H2s) and run markdownlint to confirm no MD022 violations
before archiving.
- Around line 9-21: The spec's "success-path" claims a success message but the
implementation in scripts/check_doc_frontmatter.py's _run_validation() only
returns 0 and emits no message; update either the code or the spec: to fix code,
add a positive confirmation log/print (e.g., process/console message like "All
tracked docs have valid frontmatter") in _run_validation() when it returns 0; or
to fix the spec, remove the "output SHALL show success message" assertion from
the Scenario in openspec/specs/doc-frontmatter-validation/spec.md so the spec
matches the current behavior.
- Around line 6-98: The spec omits behaviors introduced in the validation
lifecycle: add scenarios for the full-site mode flag `--all-docs`, the
contract-first decorator expectations (reference `@icontract` on validation
functions), and the PR-orchestrator parallelization rule so the spec matches
runtime workflow; update openspec/specs/doc-frontmatter-validation/spec.md to
include a Scenario for running `check_doc_frontmatter.py --all-docs`,
Scenario(s) asserting that validation functions adhere to `@icontract` pre/post
conditions (naming the functions or modules that use `@icontract`), and a
Scenario that states validation must run independent/parallel under the
PR-orchestrator (mention PR-orchestrator behavior), ensuring each new scenario
includes WHEN/THEN clauses and any flags or tokens (`--fix-hint`,
`VALID_OWNER_TOKENS`) already used elsewhere.
In `@openspec/specs/docs-contributing-updates/spec.md`:
- Around line 3-70: There are markdownlint MD022 heading blank-line violations
and a hyphenation issue: ensure each section heading (e.g., "## Purpose", "##
Requirements", "### Requirement: Frontmatter Documentation", all subsequent
"#### Scenario: ..." headings) is preceded and followed by a single blank line
to satisfy MD022, and change the phrase "copy-paste ready templates" to the
hyphenated form "copy-paste-ready templates" to fix the style inconsistency.
- Around line 3-5: Replace the placeholder "TBD" under the "## Purpose" heading
with a concrete, normative objective that states the spec's intent and scope
(e.g., what this document governs, who must follow it, and the expected
outcome), ensuring the language is prescriptive (use must/shall) and aligns with
the behavior in the implementation; update the "## Purpose" paragraph directly
by removing "TBD" and adding a one- to two-sentence statement that references
this spec as the authoritative source for contributing updates and ties into the
"## Requirements" section so future deltas are enforceable and traceable.
In `@scripts/check_doc_frontmatter.py`:
- Around line 339-368: In get_all_md_files(), when parse_frontmatter(file_path)
raises OSError or yaml.YAMLError you should still add the file to filtered but
also emit a debug-level log containing the file path and the exception details
so parse failures are visible; update the except block that currently appends
file_path to also call the repo logger (or module logger) with the file_path and
exception (e.g., include str(e) or exc_info) referencing parse_frontmatter and
file_path so maintainers can trace malformed YAML while preserving the current
fallback behavior.
In `@scripts/pre_commit_code_review.py`:
- Around line 187-206: The hook currently discards subprocess stdout/stderr and
treats a missing REVIEW_JSON_OUT as a benign missing-report; ensure the parent
directory for REVIEW_JSON_OUT exists before running the subprocess (use
_repo_root() and REVIEW_JSON_OUT to compute and mkdir parents), preserve the
completed subprocess result from the subprocess.run call (the local variable
result) and, if report_path.is_file() is False after the run, print or write
result.stdout/result.stderr to stderr and return a non-zero exit status instead
of silently continuing; keep the existing TimeoutExpired handling and still call
_print_review_findings_summary only when the JSON report actually exists.
- Around line 76-145: The code manually validates and casts the parsed JSON in
_print_review_findings_summary and then buckets severities in
_count_findings_by_severity; instead, add lightweight local Pydantic models
(e.g., CodeReviewReport(BaseModel) with findings: list[ReviewFinding],
overall_verdict: str | None and ReviewFinding(BaseModel) with severity: str and
a validator that normalizes/lowercases and maps unknown values to "other" or an
Enum) inside this module, parse the file with CodeReviewReport.parse_obj(data)
(catch ValidationError) and then refactor _print_review_findings_summary to use
the validated model (no casts) and simplify _count_findings_by_severity to
accept list[ReviewFinding] and count the normalized severity values emitted by
the model’s validator/Enum; ensure to import pydantic.BaseModel and
pydantic.Field and update error handling to surface validation errors instead of
the current ad-hoc checks.
In `@tests/helpers/doc_frontmatter.py`:
- Around line 38-48: The VALID_DOC_FRONTMATTER fixture uses doc_owner:
src/test/module which requires that path under DOC_FRONTMATTER_ROOT so
resolve_owner() returns true; update the fixture to either use a guaranteed
valid owner token (e.g., "specfact-cli", "nold-ai", or "openspec") or ensure
test setup creates the src/test/module directory under DOC_FRONTMATTER_ROOT
before resolving; modify VALID_DOC_FRONTMATTER or the test setup/fixture that
references it and/or adjust calls to resolve_owner() to use the created path so
tests are self-contained.
In `@tests/unit/scripts/test_doc_frontmatter/test_schema.py`:
- Around line 49-64: Update the test_missing_required_fields case to assert
parse_frontmatter returned a frontmatter object with title == "Incomplete
Document" and that required fields (e.g., any expected keys like "date",
"author", or other schema-required fields) are missing or None; use the
parse_frontmatter result (from CheckDocFrontmatterModule.parse_frontmatter) to
check both that the present title survived and that each required field is not
present in the parsed mapping (or explicitly is None) so the test verifies
missing-field handling rather than only non-None return.
In `@tests/unit/scripts/test_doc_frontmatter/test_validation.py`:
- Around line 221-225: The test's --fix-hint assertion is too permissive; update
the assertion after calling validation_main(["--fix-hint"]) to check that the
stderr hint contains specific required frontmatter keys (for example assert that
captured.err contains "doc_owner", "doc_type", and "summary" or whichever
required fields your validator emits) rather than just "Suggested frontmatter"
or "---"; locate the test in test_validation.py around the validation_main call
and replace the loose OR assertion with explicit substring checks against
captured.err for those required hint fields.
In `@tests/unit/scripts/test_pre_commit_code_review.py`:
- Around line 187-214: The test test_print_summary_rejects_non_object_json
currently asserts exit_code == 0 allowing malformed .specfact/code-review.json
to pass; change the assertion to expect a non-zero failure (e.g., assert
exit_code != 0 or assert exit_code == 1) so module.main(["src/app.py"]) fails on
a non-object JSON, and keep the existing check for the "expected a JSON object"
error text in err to ensure the failure reason is validated.
---
Outside diff comments:
In `@openspec/CHANGE_ORDER.md`:
- Around line 113-117: The blocked-by cell for the "ci-docs-sync-check" row uses
"docs-02 (doc-frontmatter-schema)" without the issue reference; update the
dependency text to match the governance format used elsewhere by adding the
issue link “[`#461`]” (e.g., change "docs-02 (doc-frontmatter-schema)" to "docs-02
(doc-frontmatter-schema) [`#461`]" or to "doc-frontmatter-schema ([`#461`])") so the
"ci-docs-sync-check" entry and the blocked-by section consistently include the
`#461` reference; edit the table row containing "ci-docs-sync-check" to include
that change.
In `@pyproject.toml`:
- Around line 713-721: The E402 waiver is too broad under
[tool.ruff.lint.per-file-ignores] for "tests/**/*"; remove "E402" from that
global tests rule and instead add a targeted per-file-ignore that only includes
E402 for the doc-frontmatter harness and its bootstrap helper modules (the files
referenced by the doc-frontmatter test harness), so import-order checking
remains enabled for the rest of the test suite.
🪄 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: 3254ea67-e219-4a0b-b2b1-7b1c9ba2b5b7
📒 Files selected for processing (50)
.coderabbit.yaml.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.yml.pre-commit-config.yamlAGENTS.mdCHANGELOG.mdCONTRIBUTING.mddocs/.doc-frontmatter-enforceddocs/contributing/docs-sync.mddocs/contributing/frontmatter-schema.mddocs/core-cli/init.mddocs/getting-started/quickstart.mddocs/index.mddocs/modules/code-review.mddocs/reference/documentation-url-contract.mdopenspec/CHANGE_ORDER.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/.openspec.yamlopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/TDD_EVIDENCE.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/design.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/proposal.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/specs/doc-frontmatter-schema/spec.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/specs/doc-frontmatter-validation/spec.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/specs/docs-contributing-updates/spec.mdopenspec/changes/archive/2026-03-29-doc-frontmatter-schema/tasks.mdopenspec/config.yamlopenspec/specs/doc-frontmatter-schema/spec.mdopenspec/specs/doc-frontmatter-validation/spec.mdopenspec/specs/docs-contributing-updates/spec.mdpyproject.tomlscripts/check-cross-site-links.pyscripts/check-docs-commands.pyscripts/check_doc_frontmatter.pyscripts/pre_commit_code_review.pyscripts/publish-module.pyscripts/validate-modules-repo-sync.pyscripts/verify-bundle-published.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pytests/conftest.pytests/helpers/__init__.pytests/helpers/doc_frontmatter.pytests/helpers/doc_frontmatter_fixtures.pytests/helpers/doc_frontmatter_types.pytests/integration/scripts/test_doc_frontmatter/test_integration.pytests/unit/scripts/test_code_review_module_docs.pytests/unit/scripts/test_doc_frontmatter/test_schema.pytests/unit/scripts/test_doc_frontmatter/test_validation.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_cli/registry/test_signing_artifacts.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/ci-02-trustworthy-green-checks/tasks.md`:
- Around line 14-16: Task 1.1.4 in tasks.md is vague about creating the virtual
environment; update the checklist to explicitly call out the hatch command by
adding a step like "Run `hatch env create` in the worktree to create the virtual
environment and then install dev dependencies" (reference the checklist item
1.1.4 and the command `hatch env create`) so the worktree setup follows the
repository's documented workflow.
🪄 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: e072b59a-f534-4449-a960-feb7790e0e66
📒 Files selected for processing (8)
openspec/CHANGE_ORDER.mdopenspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdscripts/check_doc_frontmatter.pytests/helpers/doc_frontmatter_types.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/CHANGE_ORDER.md
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: All public APIs must have@icontractdecorators and@beartypetype 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: Usesnake_casefor file and module names
UsePascalCasefor class names
UseUPPER_SNAKE_CASEfor constants
Use Google-style docstrings
Set line length to 120 characters maximum
All data structures must use PydanticBaseModelwithField(...)and descriptions
All public APIs must use@icontractdecorators (@require,@ensure,@invariant) for contract-first development
All public APIs must use@beartypefor 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/helpers/doc_frontmatter_types.pyscripts/check_doc_frontmatter.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test structure must mirror source withtests/unit/,tests/integration/,tests/e2e/directories
Use@pytest.mark.asynciodecorator for async test functions
Guard environment-sensitive logic withos.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 incrementsTests 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 oraclesAll tests must use
@pytest.mark.asynciofor async tests and guard environment-sensitive logic with os.environ.get("TEST_MODE") == "true"
Files:
tests/helpers/doc_frontmatter_types.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/helpers/doc_frontmatter_types.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/session_startup_instructions.mdc)
Apply
pylint src testsfor linting before committing
Files:
tests/helpers/doc_frontmatter_types.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/helpers/doc_frontmatter_types.py
openspec/CHANGE_ORDER.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update
openspec/CHANGE_ORDER.mdwhenever a change lifecycle event occurs (new change created, archived, modified, blocker resolved) in the same commitUpdate openspec/CHANGE_ORDER.md in the same commit for any change lifecycle event: new change created, change archived, change modified/renamed, blocker resolved
Files:
openspec/CHANGE_ORDER.md
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented
quality gates (format, type-check, module signing).
Files:
scripts/check_doc_frontmatter.py
🧠 Learnings (29)
📓 Common learnings
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.
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Applies to openspec/CHANGE_ORDER.md : Update openspec/CHANGE_ORDER.md in the same commit for any change lifecycle event: new change created, change archived, change modified/renamed, blocker resolved
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Keep docs in docs/ current with every code change affecting user-facing behaviour; preserve all front-matter on every edit
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
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to docs/**/*.md : Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
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
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 docs/**/*.md : Preserve all front-matter in documentation files when editing (title, layout, nav_order, permalink, etc.); check `docs/_layouts/default.html` and `docs/index.md` before modifying front-matter
📚 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:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.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: Use `.cursor/commands/wf-create-change-from-plan.md` (`/wf-change-from-plan`) when creating a change from a Markdown plan; use `.cursor/commands/wf-validate-change.md` (`/wf-validate-change`) after any change creation or modification and capture output in `openspec/changes/<change-id>/CHANGE_VALIDATION.md`
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.md
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Applies to openspec/changes/*/TDD_EVIDENCE.md : Create/update openspec/changes/<change-id>/TDD_EVIDENCE.md with test commands, timestamps, and failure/success summaries for behavior changes
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Applies to openspec/CHANGE_ORDER.md : Update openspec/CHANGE_ORDER.md in the same commit for any change lifecycle event: new change created, change archived, change modified/renamed, blocker resolved
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/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: When creating OpenSpec changes for specfact-cli, use unique verb-led `change-id` and scaffold `proposal.md`, `tasks.md`, optional `design.md`, and spec deltas in `changes/<id>/specs/<capability>/spec.md`
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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: 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:
openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.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/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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 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/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.mdopenspec/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: 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:
openspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md
📚 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:
openspec/changes/ci-02-trustworthy-green-checks/proposal.mdopenspec/changes/ci-02-trustworthy-green-checks/design.mdopenspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to .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:
openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.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: Implement strict TDD order: update/add spec deltas first → add/modify tests mapped to spec scenarios → run tests and capture failing result → only then modify production code → re-run tests and quality gates until passing; record evidence in `openspec/changes/<change-id>/TDD_EVIDENCE.md`
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/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: Applies to TDD_EVIDENCE.md : For tasks involving behavior changes, create and update `TDD_EVIDENCE.md` per AGENTS.md 'Hard Gate: Strict TDD Order', recording failing test evidence before implementation and passing test evidence after implementation.
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.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: For new code in specfact-cli using OpenSpec workflow, follow TDD discipline: write tests first, then implementation code
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:32:57.944Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/spec-fact-cli-rules.mdc:0-0
Timestamp: 2026-03-25T21:32:57.944Z
Learning: Applies to docs/**/*.md : Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.mdopenspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: For any behavior change, the TDD order is mandatory: 1) Update specs, 2) Add/modify tests, 3) Run tests with failing result, 4) Modify production code, 5) Re-run tests
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.md
📚 Learning: 2026-03-25T21:31:55.908Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/estimation-bias-prevention.mdc:0-0
Timestamp: 2026-03-25T21:31:55.908Z
Learning: Use TDD with quality gates (contract-driven, test-driven implementation, quality gate validation, documentation) as the standard development pattern
Applied to files:
openspec/changes/ci-02-trustworthy-green-checks/tasks.md
📚 Learning: 2026-03-25T21:33:15.296Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: .cursor/rules/testing-and-build-guide.mdc:0-0
Timestamp: 2026-03-25T21:33:15.296Z
Learning: Applies to src/**/*.py : Add/update contracts on new or modified public APIs, stateful classes and adapters using `icontract` decorators and `beartype` runtime type checks
Applied to files:
tests/helpers/doc_frontmatter_types.py
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Applies to **/*.py : Use icontract decorators (require, ensure, invariant) and beartype for runtime type checking on all public APIs
Applied to files:
tests/helpers/doc_frontmatter_types.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 @(README.md|AGENTS.md) : Check README.md and AGENTS.md for current project status and development guidelines. Review .cursor/rules/ for detailed development standards and testing procedures.
Applied to files:
openspec/CHANGE_ORDER.md
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Applies to src/specfact_cli/modules/**/module-package.yaml : Keep module-package.yaml metadata current with name, version, commands, and dependencies for each module in modules/{name}/
Applied to files:
openspec/CHANGE_ORDER.md
📚 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: Review `docs/README.md` to understand current development phase and priorities before proceeding with work
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
📚 Learning: 2026-03-25T21:33:45.940Z
Learnt from: CR
Repo: nold-ai/specfact-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-25T21:33:45.940Z
Learning: Keep docs in docs/ current with every code change affecting user-facing behaviour; preserve all front-matter on every edit
Applied to files:
openspec/CHANGE_ORDER.mdscripts/check_doc_frontmatter.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: Applies to **/*.py : No bare except or broad except Exception without re-raising or logging full context - use specific exceptions instead
Applied to files:
scripts/check_doc_frontmatter.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 **/*.py : Include comprehensive exception handling with specific exception types in Python code
Applied to files:
scripts/check_doc_frontmatter.py
🪛 LanguageTool
openspec/changes/ci-02-trustworthy-green-checks/proposal.md
[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: ...control. ## What Changes - EXTEND .github/workflows/pr-orchestrator.yml so requi...
(GITHUB)
[uncategorized] ~14-~14: The official name of this software platform is spelled with a capital “H”.
Context: ...EXTEND* workflow/static validation so .github/workflows/** changes always run mandat...
(GITHUB)
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...overage. ## Impact - Affected CI: .github/workflows/pr-orchestrator.yml, `.githu...
(GITHUB)
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ....github/workflows/pr-orchestrator.yml, .github/workflows/pre-merge-check.yml`, and any...
(GITHUB)
[style] ~33-~33: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd associated developer setup/docs. - Affected review automation: .coderabbit.yaml...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/ci-02-trustworthy-green-checks/design.md
[style] ~34-~34: Consider an alternative for the overused word “exactly”.
Context: ...ll suite only if the release PR head is exactly the already-validated commit set from `...
(EXACTLY_PRECISELY)
[uncategorized] ~40-~40: The official name of this software platform is spelled with a capital “H”.
Context: ...kflow-change enforcement Changes under .github/workflows/** must trigger mandatory CI...
(GITHUB)
openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...workflow lint validation Changes under .github/workflows/** SHALL trigger mandatory C...
(GITHUB)
[grammar] ~51-~51: Use a hyphen to join words.
Context: ...workflow, Markdown, and module-signature relevant changes receive the documented ...
(QB_NEW_EN_HYPHEN)
openspec/changes/ci-02-trustworthy-green-checks/tasks.md
[uncategorized] ~39-~39: The official name of this software platform is spelled with a capital “H”.
Context: ...mandatory workflow validation in CI for .github/workflows/** changes. - [ ] 4.4 Rework...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
All results are from the nold-ai/specfact-cli-modules repository.
-
.specfact/code-review.json / JSON review contract
- docs/modules/code-review.md contains concrete examples and docs for the CLI using --json and --out (e.g. "specfact code review run --json --out /tmp/review-report.json") demonstrating consumers/expected usage of the JSON report. [::nold-ai/specfact-cli-modules::docs/modules/code-review.md:100-102,291-293]
- Multiple openspec TDD_EVIDENCE and spec files reference running
specfact code review run --json --out ...(evidence the JSON output is used in process/docs). [::nold-ai/specfact-cli-modules::openspec/changes/archive/2026-03-17-review-run-dogfood-followup/TDD_EVIDENCE.md:5,32-33,40-47]
-
Consumers / callers / tests that expect
specfact code review runbehavior- tests/cli-contracts/specfact-code-review-run.scenarios.yaml invokes the command. [::nold-ai/specfact-cli-modules::tests/cli-contracts/specfact-code-review-run.scenarios.yaml:1]
- Multiple unit tests reference the command/help and expect the updated CLI behavior (e.g., tests/unit/test_check_docs_commands_script.py, tests/unit/docs/test_missing_command_docs.py). [::nold-ai/specfact-cli-modules::tests/unit/test_check_docs_commands_script.py:30,40,68] [::nold-ai/specfact-cli-modules::tests/unit/docs/test_missing_command_docs.py:37]
- openspec specs and TDD evidence assert end-to-end review-run behavior and JSON output (see openspec/specs/review-run-command/spec.md and related archived TDD_EVIDENCE). [::nold-ai/specfact-cli-modules::openspec/specs/review-run-command/spec.md:6,14,25]
-
Doc-frontmatter check script and tests
- Tests under tests/unit/scripts and tests/integration/scripts reference doc-frontmatter validation tests (new frontmatter test modules). rg output shows many test files under tests/unit/scripts/test_doc_frontmatter and tests/integration/scripts/test_doc_frontmatter. These tests are direct consumers of the doc-frontmatter validation CLI. [::nold-ai/specfact-cli-modules::tests/unit/scripts/test_doc_frontmatter/test_schema.py] [::nold-ai/specfact-cli-modules::tests/unit/scripts/test_doc_frontmatter/test_validation.py] [::nold-ai/specfact-cli-modules::tests/integration/scripts/test_doc_frontmatter/test_integration.py]
- docs/.doc-frontmatter-enforced and docs/contributing/docs-sync.md are present and referenced as the rollout/enforcement list and guidance the script uses. [::nold-ai/specfact-cli-modules::docs/.doc-frontmatter-enforced] [::nold-ai/specfact-cli-modules::docs/contributing/docs-sync.md]
-
Pre-commit / hook integration and pre-commit gate
- .pre-commit-config.yaml contains hooks referencing the code-review gate and the new doc-frontmatter check (local hook check-doc-frontmatter invoking hatch run doc-frontmatter-check). This shows local pre-commit integration points that will call the new script. [::nold-ai/specfact-cli-modules::.pre-commit-config.yaml]
- Tests updated to expect JSON-report based behavior for the pre-commit gate (tests/unit/scripts/test_pre_commit_code_review.py). This test suite indicates the pre-commit script now requires/handles .specfact/code-review.json. [::nold-ai/specfact-cli-modules::tests/unit/scripts/test_pre_commit_code_review.py]
-
CI / workflow references and possible consumers
- .github/workflows/docs-review.yml and .github/workflows/pr-orchestrator.yml are present and referenced by tests. CI workflow docs include running the doc-frontmatter-check (and tests referencing these workflows). [::nold-ai/specfact-cli-modules::.github/workflows/docs-review.yml] [::nold-ai/specfact-cli-modules::.github/workflows/pr-orchestrator.yml]
- Tests assert pr-orchestrator.yml job dependency wiring (tests/unit/specfact_cli/registry/test_signing_artifacts.py). Changes to workflow 'needs' wiring could affect orchestrated jobs that previously depended on tests. [::nold-ai/specfact-cli-modules::tests/unit/specfact_cli/registry/test_signing_artifacts.py]
Summary assessment
- This repository both documents and contains consumers (docs, CI, and many tests) for the JSON review report (.specfact/code-review.json) and for the doc-frontmatter validation/tooling. Reviewers should validate that the produced JSON path and CLI invocation semantics (.specfact/code-review.json, --json/--out) match any external callers (CI jobs, other repos or automation) and that the pre-commit hook entries (
check-doc-frontmatterand the changed specfact code-review gate) align with the new script names and hatch tasks. Tests in this repo already exercise these integrations (pre-commit gate, doc-frontmatter CLI, CI workflow assertions), indicating direct coupling and consumers inside this repo.
🔇 Additional comments (17)
scripts/check_doc_frontmatter.py (9)
339-368: File discovery with intentional fallback for parse errors.The
get_all_md_filesfunction correctly applies contracts and handles the filtering logic for exempt files. The silent catch ofOSError/yaml.YAMLErrorat lines 361-362 is intentional—these files are passed through to_collect_failureswhere the actual validation and error reporting occurs.A previous review suggested adding debug logging here for troubleshooting; that remains a valid optional improvement if desired.
51-119: Well-designed contract-first frontmatter model.The
DocFrontmatterPydantic model exemplifies the repository's contract-first philosophy: each field is typed, described, and validated with clear error messages. The field validators handle YAML date normalization gracefully (supporting bothdatetime.dateobjects from PyYAML auto-parsing and ISO strings), and the model validators enforce cross-field invariants (owner resolution, exempt-reason coupling).The
model_rebuild()call correctly resolves thedatetimeforward reference in the type namespace.
153-172: Contract-decorated parsing functions follow the repository's public API standards.Both
parse_frontmatterandextract_doc_ownerare properly decorated with@beartypeand@icontractpreconditions/postconditions, ensuring runtime enforcement of the documented contracts. Usingyaml.safe_loadis the correct security choice for untrusted input.
244-266: LRU-cached owner resolution with robust traversal protection.The
resolve_ownerpublic API correctly applies contracts, and the internal_resolve_owner_impluses@functools.lru_cachekeyed by(owner, root_key)to avoid redundant filesystem operations across multiple file validations. The_owner_literal_is_safeguard rejects path traversal attempts (..), empty strings, and glob metacharacters before any filesystem access—this is a sound defense-in-depth approach.The cache-clear capability via
_resolve_owner_impl.cache_clear()enables test isolation, which I see is leveraged by the test fixtures.
269-293: Glob validation with balanced bracket checks and regex compilation.The
validate_glob_patternsfunction applies a pragmatic two-layer check: first verifying bracket/brace balance (catching common authoring errors), then attemptingfnmatch.translate+re.compileto validate the pattern is syntactically valid. This approach aligns with the spec requirement that invalid glob syntax must fail validation.
407-422: Informative fallback when ripgrep fails—addresses prior review feedback.The
rg_missing_doc_ownerfunction now properly logs the exception to stderr (line 420-421) before falling back to the slower per-file scan. This addresses the previous review comment about the empty except clause. The specific exception types (FileNotFoundError,subprocess.TimeoutExpired,OSError) are appropriate for subprocess invocation failures.
383-404: Ripgrep integration with proper exit code semantics and timeout.The
_missing_owner_via_rgfunction correctly interprets ripgrep's exit codes: 0 means files were found without the pattern, 1 means all files match (no missing owners), and 2+ indicates an error requiring fallback. The 60-second timeout prevents unbounded blocking on large repositories.
512-556: Typer CLI with contract-decorated entry point and programmatic exit handling.The CLI structure follows the repository's established patterns:
typer.Typer()for the app,rich.console.Consolefor output, and themain()entry point is decorated with@beartypeand@icontractcontracts ensuring the exit code is always 0 or 1.The use of
get_command(app)withstandalone_mode=Falseenables programmatic exit code capture, which is essential for test harnesses (e.g., the Typer test client) and pre-commit integration.
488-509: Validation orchestration with clear exit semantics and UX hints.The
_run_validationfunction cleanly orchestrates the validation pipeline: discover paths → detect missing owners → collect failures → report. The exit codes are deterministic (0 for success/skip, 1 for failures), and the UX includes a helpful tip about--fix-hintwhen validation fails.tests/helpers/doc_frontmatter_types.py (1)
1-35: Protocol definitions address prior CodeQL findings with explicit NotImplementedError.The
ResolveOwnerImplWithCacheandCheckDocFrontmatterModuleprotocols correctly define structural types for the dynamically loadedscripts/check_doc_frontmatter.pymodule. Usingraise NotImplementedErrorin method bodies addresses the prior CodeQL "statement has no effect" findings for bare...expressions.These protocols enable type-safe access to the module's public surface in tests, including the crucial
_resolve_owner_impl.cache_clear()for test isolation—a good example of exposing cache semantics through typed contracts.openspec/CHANGE_ORDER.md (2)
63-63: OpenSpec lifecycle updates correctly reflect archive status and dependency wiring.The
doc-frontmatter-schemaarchive entry (line 63) with date2026-03-29is correctly recorded. The Documentation table entry (line 113) properly includes[#461]with parent reference[#356]as required by the coding guidelines.The
ci-docs-sync-checkblocker (line 116) now explicitly referencesdocs-02 (doc-frontmatter-schema)rather than a generic change name, which improves traceability.The new
ci-02-trustworthy-green-checksrow (line 172) is appropriately marked aspendingwith dependencies onci-01 ✅andcode-review-08 ✅.Also applies to: 113-116, 172-172
383-384: Blocked-by relationships include explicit issue references.The
ci-docs-sync-checkdependency (line 384) correctly referencesdoc-frontmatter-schema ([#461])with the issue number, matching the coding guideline requirement for explicit issue-linked dependency tracking. This enables GitHub's dependency graph and project automation.openspec/changes/ci-02-trustworthy-green-checks/CHANGE_VALIDATION.md (1)
1-30: Validation evidence properly recorded for proposal-stage change.The
CHANGE_VALIDATION.mdcorrectly captures the required strict validation result (PASS), the validation command used, and a dry-run breaking-change analysis. The scope summary accurately identifiesci-01-pr-orchestrator-log-artifactsandcode-review-08-review-run-integrationas hard dependencies.The assertion that this change is "governance-only" with no production CLI behavior change is appropriate given the proposal's scope is CI/review/hook hardening.
openspec/changes/ci-02-trustworthy-green-checks/proposal.md (1)
1-49: Well-structured proposal with clear capability boundaries and dependency tracking.The proposal articulates a genuine governance gap: "green" checks that don't actually enforce required validation create "governance theater." The capability taxonomy is precise:
trustworthy-green-checksis new, whiledocs-review-gateandcode-review-moduleare modified to clarify their required/advisory status.The hard dependencies on
ci-01-pr-orchestrator-log-artifactsandcode-review-08-review-run-integrationare correctly identified—this change builds on the existing orchestrator model rather than replacing it.The Source Tracking section shows
pendingstatus, which should be updated with the GitHub issue number once created per the workflow guidelines.openspec/changes/ci-02-trustworthy-green-checks/design.md (1)
1-67: Design establishes clear enforcement semantics across four layers.The design document provides architectural clarity for a complex cross-cutting concern. The five-part enforcement model addresses:
- Gate taxonomy: Required vs Advisory classification—eliminates ambiguity from mixed semantics like
|| echoorcontinue-on-error- Release PR parity: Deterministic fast-path rules for
dev -> main- Workflow-change enforcement: Mandatory CI validation for
.github/workflows/**changes- Local-vs-CI parity: Bridges the gap between smart-check wrapper and standard
.pre-commitpath- Review automation coverage: Standardizes CodeRabbit posture across branch targets
The out-of-scope section is appropriately defensive, explicitly excluding production CLI behavior changes and OpenSpec schema rework.
openspec/changes/ci-02-trustworthy-green-checks/tasks.md (1)
18-63: Task checklist enforces strict TDD/SDD order with evidence requirements.The task structure correctly implements the repository's TDD discipline:
- Task 2: Spec-first preparation with audit of current enforcement semantics
- Task 3: Test-first evidence with explicit requirement to capture failing tests in
TDD_EVIDENCE.mdbefore implementation- Tasks 4-5: Implementation only after tests exist
- Task 6: Quality gates including
openspec validate ci-02-trustworthy-green-checks --strict- Task 7: Delivery with
CHANGE_ORDER.mdupdate requirementThis ordering prevents the common anti-pattern of implementing first and backfilling tests.
openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md (1)
1-62: Specification with testable scenarios enables contract-first validation.The spec defines six requirements with clear WHEN/THEN scenarios that map directly to test cases:
- Required CI jobs fail on tool failures: Prevents
|| echoandcontinue-on-errorfrom masking failures- Advisory jobs are explicit: Non-deceptive naming for warn-only jobs
- Release PR parity:
dev -> mainfast-path only when provably safe- Workflow-change enforcement: Mandatory CI validation for
.github/workflows/**changes- Local pre-commit parity: Documented hooks match CI gate semantics
- PR review coverage: Both
devandmaintargets receive automatic reviewThe RFC 2119 SHALL/SHALL NOT language provides unambiguous enforcement semantics. These scenarios can be directly translated to integration tests that verify the CI/hook behavior changes.
Description
Promote
devtomainafter merging the doc frontmatter validation work from #463 and pushing the follow-up OpenSpec lifecycle update ondev(edc89a7).This release-forward includes:
doc-frontmatter-schemachangeFixes #461
New Features #461
Contract References: None in this follow-up release PR. Public API/contract changes were delivered in #463.
Type of Change
Please check all that apply:
@icontractdecorators)Contract-First Testing Evidence
Required for all changes affecting CLI commands or public APIs:
Contract Validation
@icontractdecorators on public APIs)@beartypedecorators applied)hatch run contract-test-explorationTest Execution
hatch run contract-test-contracts✅hatch run contract-test-exploration✅hatch run contract-test-scenarios✅hatch run contract-test-full✅Test Quality
How Has This Been Tested?
Contract-First Approach: The feature PR validated the doc-frontmatter CLI/test surface and CI behavior; the follow-up
devlifecycle commit archived the OpenSpec change and synced its main specs throughopenspec archive.Manual Testing
Automated Testing
Test Environment
Checklist
@icontract,@beartype)Quality Gates Status
hatch run type-check)hatch run lint)hatch run contract-test-contracts)hatch run contract-test-exploration)hatch run contract-test-scenarios)Screenshots/Recordings (if applicable)
Not applicable.