Promote dev import runtime hardening to main#243
Merged
Conversation
Add ten OpenSpec changes for specfact-cli five-pillar runtime companions (FinOps, knowledge, review resiliency, security, architecture, enterprise), update CHANGE_ORDER with GitHub story links under modules epic #216, and add ensure_github_hierarchy_subissues.py to repair sub-issue edges. Made-with: Cursor
…t hardening - Design/tasks: add H1 titles or promote first headings; clarify paired OpenSpec ids. - Specs: add document H1 plus keep ## ADDED Requirements for OpenSpec delta parsing; name security, finops, enterprise, license, resiliency contracts explicitly. - knowledge/review tasks: verify upstream paths on paired specfact-cli branch; knowledge-02 adds hierarchy-cache refresh; writeback spec pins UTC timestamps. - security-03 proposal: blocked-by policy-02-packs-and-modes for policy-pack work. - ensure_github_hierarchy_subissues: subprocess timeout, dry-run counter fix, paginated subIssues fetch, omit null GraphQL variables. Made-with: Cursor
Made-with: Cursor
OpenSpec: five-pillar modules companion proposals (wave 2)
Co-authored-by: Dom <djm81@users.noreply.github.com>
…alization Replace recursive rglob counting for ignored directories with a single os.scandir of the immediate directory so pruning stays cheap. Strip only leading ./ and / path prefixes from specfactignore patterns instead of lstrip character classes, preserving dot-prefixed names. Co-authored-by: Dom <djm81@users.noreply.github.com>
_install_patch now accepts **kwargs and merges them via dataclasses.replace so positional+keyword calls (e.g. incremental_callback) work after patching. Explicit-root bypass in should_ignore_path only skips the entry root path, not descendants, so scoped discovery still prunes node_modules and defaults. Tests cover entry-point pruning and patch keyword forwarding. Co-authored-by: Dom <djm81@users.noreply.github.com>
- OpenSpec bundle-packaged-resources: heading blank lines for MD041/MD022 - protocol.yaml.j2: YAML-safe string emission via tojson - Remove eager apply_import_runtime_patches from package __init__; call from sync_repository and from_code with commands_module to avoid import cycles - template_paths: RuntimeError when no candidate matches required_glob - import_runtime_patches: RLock around Path.rglob swap; importlib for commands - import_path_policy: merge repo .specfactignore with caller patterns; capped ignored-dir counts; file entry_point handling; discover_code_files kwargs typing - Tests: entry-point under .venv, YAML assertions for protocol/workflow, patch fixture Co-authored-by: Dom <djm81@users.noreply.github.com>
…-b408 Import path policy issues
Co-authored-by: Dom <djm81@users.noreply.github.com>
Co-authored-by: Dom <djm81@users.noreply.github.com>
Co-authored-by: Dom <djm81@users.noreply.github.com>
…ardening-98df fix(project): harden code import runtime
chore(modules): auto-sign module manifests
chore(registry): publish changed modules
chore(modules): auto-sign module manifests
chore(registry): publish changed modules
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtended modules ecosystem with 11 new OpenSpec change sets and docs; added import-runtime hardening (pre-traversal pruning, Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PR as Pull Request Event
participant WF as Workflow (sign-modules-on-approval)
participant GH as GitHub API (GraphQL)
participant Signer as Module Signer
rect rgba(100, 150, 200, 0.5)
Note over PR,WF: pull_request (opened/synchronize/reopened/ready_for_review)
end
PR->>WF: trigger on pull_request
WF->>WF: set concurrency.group using github.event.pull_request.number or github.event.number
WF->>WF: resolve PR_BASE_REF / PR_HEAD_REF with fallbacks
WF->>WF: if github.event_name == "pull_request" then
WF->>GH: gh api graphql: query latestReviews(first:100) for PR
GH-->>WF: return reviews (states + authorAssociation)
WF->>WF: compute trusted APPROVED review existence
alt trusted APPROVED exists
WF->>Signer: sign=true
else
WF->>Signer: sign=false
end
rect rgba(150, 200, 100, 0.5)
Note over PR,WF: pull_request_review (submitted)
end
PR->>WF: trigger on pull_request_review
WF->>WF: use event.review.state and author_association
alt review.state == "approved" and trusted author
WF->>Signer: sign=true
else
WF->>Signer: sign=false
end
sequenceDiagram
autonumber
participant CLI as Command (from_code / sync_repository)
participant Patches as import_runtime_patches.apply_import_runtime_patches
participant Discovery as discover_code_files
participant Analyzer as CodeAnalyzer
participant Console as console.print
rect rgba(100, 150, 200, 0.5)
Note over CLI,Patches: Patch installation at command start
end
CLI->>Patches: apply_import_runtime_patches()
Patches->>Patches: install rglob interception & analyzer wrappers
Patches->>Console: wrap console.print to filter timing chatter
rect rgba(150, 200, 100, 0.5)
Note over Discovery,Analyzer: Discovery + Patched Analysis
end
Patches->>Discovery: discover_code_files(repo_root, options)
Discovery->>Discovery: load .specfactignore, prune heavy dirs, compute provisional ETA
Discovery-->>Patches: return ImportDiscoveryResult(files, warnings, eta)
Patches->>Console: emit runtime warnings if needed
Patches->>Analyzer: execute analyze() under patched rglob (filtered file set)
Analyzer-->>CLI: analysis results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
djm81
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 51
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sign-modules-on-approval.yml:
- Line 74: The current Python one-liner that parses the GraphQL JSON (the
python3 -c "import json,sys; data=json.load(sys.stdin); reviews=...;
print(...)") can raise a KeyError on missing keys; update it to validate the
expected keys or wrap the parsing and key access in a try/except that catches
(KeyError, TypeError, ValueError) and prints a clear, descriptive error message
to stderr (including which top-level key is missing and the raw input snippet or
its summary) before exiting with a non-zero status so the GitHub Actions job
fails with a useful message.
- Around line 68-81: The GraphQL query uses latestReviews(first:100) which can
miss reviews if a PR has >100 distinct reviewers; update the sign workflow to
handle pagination by looping requests using pageInfo.hasNextPage and
pageInfo.endCursor (aggregate nodes across pages) before computing
trusted_review_count, referencing the existing trusted_review_count calculation
and the latestReviews(...) field; alternatively, if you decide 100 is an
acceptable ceiling for this repo, add a clear comment in the workflow near
latestReviews(first:100) explaining that the 100-reviewer limit is intentional
and documented.
In `@docs/bundles/project/import-migration.md`:
- Around line 39-40: Replace the vague phrase "very large" in the two sentences
that start with "When import encounters unusually large ignored artifact
trees..." and "Remaining time is based on processed-versus-discovered work..."
with concrete thresholds; for example, change to "ignored artifact trees with
>10,000 entries or repositories larger than ~100,000 files, ~10 GB of repository
data, or >100,000 commits" and note that estimates remain provisional for repos
above those thresholds—adjust the numeric thresholds to align with product
telemetry if different.
In `@openspec/CHANGE_ORDER.md`:
- Around line 94-95: The two entries for security-02-module-license-compliance
and security-03-module-pii-gdpr-eu both use Order | 04; update the Order on one
of them (e.g., change security-03-module-pii-gdpr-eu to Order | 05 or otherwise
assign a unique, sequential number) so every `security` entry has a distinct
Order value and the CHANGE_ORDER semantics remain consistent across the spec.
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Line 50: The document references a paired core contract named
architecture-02-well-architected-review that isn't present in specfact-cli;
update the design to either (a) point to the existing core contract if it exists
under a different name or folder (search for architecture-01-solution-layer and
other architecture-* entries in the specfact-cli repo), (b) confirm and document
that architecture-01-solution-layer supplies the required boundary rules and
adapter contracts for this module, or (c) add a note that a new core change
architecture-02-well-architected-review must be created/synced into specfact-cli
before module integration—ensure the doc references the exact core contract name
you verified and mention architecture-01-solution-layer when clarifying overlap.
In `@openspec/changes/architecture-02-module-well-architected/proposal.md`:
- Around line 10-12: Add an "Adapter Contract" subsection under "What Changes"
that defines the normalized boundary between analyzers and the core review
contract: list the required core findings schema fields this bundle must
populate (e.g., id, type, severity, description, source, timestamp,
evidence_refs), specify how outputs from the packaged analyzers
(dependency-graph checks, ADR traceability, interface-diff) map to normalized
evidence references and fields (reference example mappings for
ALLOWED_IMPORTS.md violations → evidence_refs, ADR entries → source/trace
links), and state the emission mode (synchronous vs deferred batching) and
retry/ordering semantics for findings emission so implementers know expected
behavior.
- Line 13: Add an explicit core_compatibility field and semantic version range
for the reserved manifest "architecture-02-well-architected-review" so consumers
and the core can validate compatibility; update the proposal text to state the
required core version range (e.g., a semver range such as ">=X.Y.Z <MAJOR+1" or
a caret-style constraint) and where it should appear in the reserved
manifest/registry metadata, referencing the symbol
architecture-02-well-architected-review and the metadata key core_compatibility
so implementers know to include and enforce this constraint.
In `@openspec/changes/codebase-import-runtime-hardening/design.md`:
- Line 1: The document begins with a level-2 heading ("## Context") which
violates markdownlint MD041; add a top-level H1 as the very first line (for
example "# Design" or another project-appropriate title) before the existing "##
Context" heading so the file starts with a level-1 heading and MD041 is
satisfied.
- Line 24: Add a blank line after each section heading to satisfy MD022: insert
an empty line immediately after "Decision 1: Introduce a shared import path
policy helper" and likewise after the other headings referenced (the headings at
the other noted locations), ensuring each heading is followed by a single blank
line before the paragraph content.
In `@openspec/changes/codebase-import-runtime-hardening/proposal.md`:
- Line 1: Change the document to include a top-level H1 heading before the
existing content (replace or precede the current "## Why" with an H1 like "#
Proposal" or "# Changes for codebase-import runtime hardening"), and fix heading
spacing violations by adding a blank line above and below each heading that
currently violates MD022 (specifically the headings around the content at lines
referenced in the review — the "Why" section and the capability headings
mentioned in the proposed fix) so all headings (e.g., the "Why" heading and the
capability headings) are surrounded by blank lines and use the appropriate '#'
level.
In
`@openspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.md`:
- Line 1: Add a top-level H1 heading before the "## MODIFIED Requirements"
section to avoid triggering MD041; specifically insert an H1 like "# Requirement
Changes" or "# Changelog" immediately above the existing "## MODIFIED
Requirements" header so the document begins with an H1 before any H2s (update
the heading text as appropriate for the spec).
In
`@openspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md`:
- Around line 4-14: Clarify the precedence: state that when `specfact code
import` runs the implementation MUST first load the deterministic default ignore
policy (which excludes dot-prefixed directories like `.git/`, `.specfact/`,
virtualenvs, `node_modules/`, etc.), then read and merge repository-local
patterns from `.specfact/.specfactignore` into that policy (repository-local
patterns augment and can override conflicting default patterns), and finally
apply any explicit user target paths which may re-include a path inside an
ignored root; ensure this explicit ordering (default -> merge
`.specfact/.specfactignore` -> explicit target override) is added to the spec so
pruning happens before traversal and precedence is unambiguous.
- Around line 1-40: The markdown has lint warnings from missing blank lines
around headings (MD041/MD022); edit the section headings such as "ADDED
Requirements", "Requirement: Import runtime SHALL prune hidden and heavyweight
trees before discovery", "Scenario: Default traversal skips hidden and
heavyweight directories", "Requirement: Import runtime SHALL surface
large-artifact warnings", and "Requirement: Import progress SHALL use
discovered-versus-processed work" to ensure there is a single blank line above
and below each heading and between consecutive headings/paragraphs so each
heading is properly separated and no multiple consecutive headings lack
intervening blank lines; keep the existing heading text and indentation but
insert the required blank lines consistently throughout the document to resolve
MD041/MD022.
In `@openspec/changes/codebase-import-runtime-hardening/tasks.md`:
- Line 1: Add a top-level H1 heading to the document (replace the current
leading "## 1. GitHub readiness and change scaffolding" start) so the file
begins with an H1 like "# Tasks: codebase-import-runtime-hardening" to satisfy
MD041 and match other task files; ensure the existing content remains unchanged
after the new H1.
In
`@openspec/changes/enterprise-01-module-policy-client/specs/enterprise-policy-client/spec.md`:
- Line 1: Add a top-level H1 above the existing H2 "ADDED Requirements" in the
spec.md (e.g., "Enterprise Policy Client" or another concise title) so the
document begins with an H1 to satisfy MD041; update the first line to a valid
Markdown H1 and ensure the existing H2 remains unchanged.
In `@openspec/changes/enterprise-01-module-policy-client/tasks.md`:
- Line 4: Update checklist item for enterprise-01-policy-resolution-extension to
explicitly require reviewing the paired public OpenSpec/contracts in
nold-ai/specfact-cli before implementation and documenting the minimum required
core_compatibility; add a sub-item that instructs the implementer to verify and
record matching change IDs, model names, and workflow semantics between this
change and the paired specfact-cli artifacts (e.g., "Review paired public
OpenSpec/contracts in nold-ai/specfact-cli and confirm matching change IDs/model
names/workflow semantics; document minimum core_compatibility"). Ensure the new
text appears alongside the existing 1.2 checklist so reviewers cannot skip the
paired-core artifact review step.
In `@openspec/changes/enterprise-02-module-audit-client/design.md`:
- Line 1: The document begins with an H2 ("## Context") which triggers MD041;
add a top-level H1 above the existing "## Context" header (e.g., "# Context" or
a suitable project/title H1) so the file starts with an H1, leaving the existing
"## Context" as-is to preserve structure.
In
`@openspec/changes/enterprise-02-module-audit-client/specs/enterprise-audit-client/spec.md`:
- Line 7: The spec declares a hard core dependency on
enterprise-02-rbac-and-audit-trail for the nold-ai/specfact-enterprise-audit
bundle but that core change does not exist; update
enterprise-audit-client/spec.md to remove or replace the nonexistent core change
reference in the core_compatibility and dependency text (replace
enterprise-02-rbac-and-audit-trail with the correct existing change ID or a TODO
placeholder), and add a note that implementation is blocked until the paired
core change is proposed/merged and the spec's core_compatibility entry is
updated.
In `@openspec/changes/enterprise-02-module-audit-client/tasks.md`:
- Line 4: Update the task checklist in
enterprise-02-module-audit-client/tasks.md to add an explicit step that
reviewers must inspect the paired public core OpenSpec/contracts (not just
upstream sequencing) for the `enterprise-02-rbac-and-audit-trail` change and any
linked artifacts from `enterprise-01-module-policy-client`; call out that
reviewers should confirm the authoritative public artifacts in specfact-cli (or
equivalent public repo) and record the minimum required `core_compatibility`
after that review. Ensure the new line references the paired artifact names
(`enterprise-02-rbac-and-audit-trail`, `enterprise-01-module-policy-client`,
specfact-cli) and replaces or augments the existing weaker "Confirm paired core
change" item.
In
`@openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md`:
- Line 7: The spec references coordination with
telemetry-01-opentelemetry-default-on but doesn't state whether that change is a
bundle dependency, a core_compatibility requirement, or only a sequencing
constraint; update the sentence for nold-ai/specfact-finops to explicitly state
how telemetry-01-opentelemetry-default-on is represented (e.g., add it to
bundle_dependencies, list it under core_compatibility, or note it as a
sequencing prerequisite) and ensure the manifest fields mentioned
(`bundle_dependencies`, `pip dependencies`, `core_compatibility`) clearly
indicate where telemetry-01 appears and that CHANGE_ORDER.md is the
authoritative source for sequencing.
In `@openspec/changes/finops-01-module-cost-outcome/tasks.md`:
- Line 19: Edit the task bullet that currently reads "3.4 Update registry
metadata, docs references, signatures, and any import allowlists required by the
new bundle." to clarify that "signatures" are validated by gates and created by
the publish automation rather than manually committed; e.g., change the phrase
to something like "update registry metadata, docs references, and any import
allowlists required by the new bundle (signatures are validated by CI gates and
produced by publish automation—not committed manually)"; update the text near
the 3.4 task in tasks.md accordingly.
In `@openspec/changes/knowledge-01-module-memory-runtime/design.md`:
- Line 48: The design references a paired core MemoryBackend protocol that isn't
published in specfact-cli; confirm and record this contract by coordinating with
specfact-cli to either (A) publish the MemoryBackend protocol definition and
version/stability statement (ensure the exported symbol is named MemoryBackend
and its public API/JSON schema is documented), or (B) add the MemoryBackend
protocol to the paired core change scope and include its authoritative spec in
this repo's design document (update "migration step 1" to reference the exact
protocol artifact, version, and where it is published). Ensure the design.md
explicitly points to the published spec (or pairing ticket) and notes the
required stability guarantee so the markdown-graph MemoryBackend integration can
proceed.
In `@openspec/changes/knowledge-01-module-memory-runtime/proposal.md`:
- Around line 9-13: Rephrase the repeated "**NEW**:" bullets to improve
readability by varying sentence structure: e.g., keep one "**NEW**:" header for
the group and turn the rest into action-starting phrases ("Add a
`specfact-knowledge` bundle with `memory` subcommands...", "Package the
markdown-graph backend as the default runtime...", "Define gitignore,
repo-layout, and file-lifecycle...", "Add search and status surfaces exposing
`MemoryBackend` behavior...") or combine related items into a single sentence;
target the lines mentioning `specfact-knowledge`, `memory` subcommands,
`markdown-graph backend`, `.specfact/memory/`, and `MemoryBackend` when applying
the edits.
- Line 29: Create and merge the paired core change repository folder
openspec/changes/knowledge-01-distillation-engine into the nold-ai/specfact-cli
repo (ensuring the folder name exactly matches the contract referenced in this
module), document the contract stability and cross-repo compatibility
requirements in TDD_EVIDENCE.md, and confirm the core change is merged into
specfact-cli before proceeding with the module implementation so the module’s
dependency on knowledge-01-distillation-engine is satisfied.
In
`@openspec/changes/knowledge-01-module-memory-runtime/specs/knowledge-memory-runtime/spec.md`:
- Line 1: The file currently starts with an H2 which triggers MD041; fix by
adding a top-level H1 as the very first line of
openspec/changes/knowledge-01-module-memory-runtime/specs/knowledge-memory-runtime/spec.md
(e.g., "# Knowledge Memory Runtime") so the existing H2 becomes a secondary
heading — ensure the H1 is descriptive for the module and placed before any
other content or headings.
In `@openspec/changes/knowledge-01-module-memory-runtime/tasks.md`:
- Line 1: Replace the numeric H1 "# 1. Branch and dependency guardrails" with a
descriptive H1 to match other OpenSpec files (for example "Change: Branch and
dependency guardrails" or "Design: Branch and dependency guardrails"); update
the header string in the tasks.md file so the top-level title uses a descriptive
label rather than a leading number to maintain consistency across specs.
In `@openspec/changes/knowledge-02-module-writeback/design.md`:
- Line 1: Add a top-level H1 heading above the existing "## Context" heading in
the design.md to satisfy MD041; for consistency with other OpenSpec docs use a
title like "# Design: knowledge-02-module-writeback" placed immediately before
the "## Context" section so the file begins with an H1 followed by the existing
subsections.
In `@openspec/changes/knowledge-02-module-writeback/proposal.md`:
- Around line 10-12: Add an "Adapter Contract" (or "Writeback Interface")
subsection that specifies the adapter method signature and manifest schema:
define a function-like contract (e.g., adapter receives selected memory rules
and destination metadata and returns { previewContent, writebackManifest }) and
enumerate manifest fields required for deterministic lineage/audit (ruleIds,
ruleVersions, inputDigest, outputDigest, timestamp, adapterId, destination,
previewHash, dryRunFlag); state whether adapters MUST validate destinations or
rely on caller validation and clarify error/validation semantics for preview
generation vs. final writeback; include example field types and required vs
optional fields so all implementations (BUGBOT.md adapter,
.github/copilot-instructions.md adapter, CodeRabbit draft adapter) follow the
same deterministic output and manifest format.
- Around line 29-30: Add explicit reserved-manifest metadata: define
bundle_dependencies and core_compatibility version ranges for this bundle. In
the Impact/Release section specify bundle_dependencies including
"knowledge-01-module-memory-runtime" with a semver range (e.g., >=X.Y.Z <A.B.C)
and any other module bundles required, and set core_compatibility to the minimum
core version that includes "knowledge-02-preflight-context-assembly" (and an
upper-bound if needed). Reference the manifest fields bundle_dependencies and
core_compatibility and list exact bundle IDs and semver ranges so the registry
can validate compatibility.
In `@openspec/changes/review-resiliency-01-module/proposal.md`:
- Around line 10-12: Add an "Adapter Contract" subsection under the "What
Changes" section that explicitly defines the adapter boundary that normalizes
resiliency detections into the core findings model: list the exact core findings
schema fields the bundle must populate (e.g., id, title, description, location,
timestamp, severity, confidence, evidence_refs), specify the stable rule
identifier format and examples for rule IDs used by the resiliency rule packs,
define the evidence reference format/namespace (how to reference probe logs,
traces, or memory-evidence), and describe the mapping rules for severity and
confidence scoring into the core findings fields so implementations produce a
stable, compatible findings payload.
- Around line 29-30: Specify the exact core_compatibility range for
review-resiliency-01-contracts (e.g., semver range or platform version token) in
module-package.yaml and explicitly state the semantics for optional
integrations: declare knowledge-01-module-memory-runtime under
bundle_dependencies with a feature flag key (or attribute) marking it as
optional/feature-gated, and describe registry resolution behavior when the
feature flag is absent versus present; reference the module-package.yaml,
core_compatibility, bundle_dependencies, review-resiliency-01-contracts, and
knowledge-01-module-memory-runtime symbols in the Impact section.
In `@openspec/changes/security-01-module-sast-sca-secret/design.md`:
- Around line 48-50: Add explicit core_compatibility details to the migration
plan: specify the expected version constraint format (e.g., semver range like
">=X.Y.Z <A.B.C"), declare the initial core contract target version(s) (e.g.,
"core v1.0.0 and v1.1.0"), and state whether bundle_dependencies are required or
optional for this release (e.g., "no bundle_dependencies expected" or "depends
on policy-bundle >=1.2.0"). Update the "registry entry, docs, signatures, and
compatibility range" line to include these three concrete items so implementers
know the exact core_compatibility string format, the targeted core contract
versions, and the expectation for bundle_dependencies.
- Line 9: Add a "Command Surface" subsection under the "Decisions" section of
the specfact-security design that explicitly states whether the bundle exposes a
single command or multiple scanner-specific commands and how they map to the
four scanner classes (sast, sca, sbom, secrets); update the document to declare
the chosen approach (single `security` command with flags like
--sast/--sca/--secrets, separate commands `sast`/`sca`/`sbom`/`secrets`, or a
hybrid) and describe the rationale for discoverability and how commands will be
declared in module-package.yaml so implementers know whether to register
`specfact-security:security` or individual commands such as
`specfact-security:sast`.
In
`@openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md`:
- Line 7: Update the `nold-ai/specfact-security` spec to correct the
`core_compatibility` declarations: if `security-01-unified-findings-model` is
not an accepted core contract, remove it from `core_compatibility` or add a
corresponding proposal in `specfact-cli` first; for `policy-02-packs-and-modes`,
change its entry from a core `core_compatibility` to a module-level bundle
dependency (e.g., list it under bundle/pip dependencies) since it lives in
`specfact-cli-modules`; ensure the spec no longer claims core compatibility for
non-core items and clearly documents the dependency locations for both
`security-01-unified-findings-model` and `policy-02-packs-and-modes`.
In `@openspec/changes/security-01-module-sast-sca-secret/tasks.md`:
- Line 4: Update the task text to explicitly verify both referenced changes
before binding core compatibility: check that security-01-unified-findings-model
exists in specfact-cli (if missing, mark it as "planned/not yet present" and
reference that status), confirm policy-02-packs-and-modes is the module in
specfact-cli-modules (cite its CHANGE_ORDER.md and CHANGE_VALIDATION.md
evidence), and record the minimum required core_compatibility versions against
both plus any sequencing notes in TDD_EVIDENCE.md; also verify
module-package.yaml and bundle registry semantics for policy-02-packs-and-modes
are final before locking core_compatibility.
In `@openspec/changes/security-02-module-license-compliance/design.md`:
- Line 1: Add a top-level H1 heading to this design doc to satisfy MD041; insert
a line like "# Design: security-02-module-license-compliance" (or another
appropriate title) immediately before the existing "## Context" heading so the
file begins with an H1 and retains the current "## Context" section unchanged.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Around line 10-11: Add an "Adapter Contract" subsection under the SBOM/SPDX
adapter description that specifies the exact boundary between adapters and the
core security/license findings model: enumerate the core findings schema fields
(e.g., finding_id, package_name, package_version, license_id,
normalized_license_type, severity, policy_state, confidence, timestamp), define
the SPDX-to-normalized finding mapping rules (how SPDX identifiers map to
normalized_license_type and license_id), and prescribe the evidence reference
format for SBOM lineage (e.g., canonical SBOM source URL/ID, package entry path,
component checksum and provenance fields). Ensure this contract is titled
"Adapter Contract" and referenced by "SBOM ingestion and SPDX normalization
adapters" and the "core security/license findings model" so implementations must
produce findings compatible with these fields and mappings.
- Around line 12-13: Specify concrete inter-bundle compatibility and adapter
boundaries: add a core_compatibility version range entry that references
security-01-unified-findings-model (e.g., semver range) in the
license-compliance bundle manifest, define the inter-bundle hook interface
(choose and document a single approach such as a shared registry schema named
sbom_registry with JSON schema and API surface or a file-based export format
named sbom_export_v1 and its file layout), state whether bundle_dependencies
should be declared (add bundle_dependencies: [
"security-01-unified-findings-model" ] to the license-compliance manifest or
reverse it if the security bundle should depend on license-compliance), and
update manifest/registry/docs/signing sections to include required fields for
registry entry, docs pointer, and signing expectations so adapters can be built
against these stable hooks.
- Line 29: The dependency reference in proposal.md currently points to
security-02-eu-gdpr-baseline but should reference the core policy-pack change
policy-02-packs-and-modes (or else clarify distinct semantics) — update the
dependency on line 29 to replace security-02-eu-gdpr-baseline with
policy-02-packs-and-modes if the intention is to indicate the shared policy-pack
semantics, or add one sentence in the rationale clarifying that
security-02-eu-gdpr-baseline is a separate vendor/supply-chain baseline and
explain how its semantics differ from policy-02-packs-and-modes (cross-reference
security-03-module-pii-gdpr-eu and design.md:44 where policy-pack interpretation
is already referenced).
In
`@openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md`:
- Line 7: Confirm that the core change "security-01-unified-findings-model" is
planned or implemented in the specfact-cli roadmap by checking specfact-cli's
CHANGE_ORDER.md and the core repo; if it is planned or already merged, record
the exact core release or commit that provides the shared findings/report
contracts and add that version as the `core_compatibility` value for
`nold-ai/specfact-license-compliance` in the spec (and document the same
identifier in TDD_EVIDENCE.md); if it is not planned, open/track a
CHANGE_ORDER.md entry or coordination ticket in specfact-cli before implementing
this bundle and do not proceed until the required core compatibility version is
agreed and documented.
In `@openspec/changes/security-02-module-license-compliance/tasks.md`:
- Line 1: Replace the top-level header so the file uses a descriptive H1 like
other OpenSpec files: change the existing "# 1. Branch and dependency
guardrails" to "# Tasks: security-02-module-license-compliance" and then demote
the current header to a section header by adding "## 1. Branch and dependency
guardrails" immediately below; update the headers in
openspec/changes/security-02-module-license-compliance/tasks.md to match this
pattern so it aligns with knowledge-01-module-memory-runtime/tasks.md.
In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md`:
- Line 19: The task line "3.4 Update registry metadata, docs references,
signatures, and any import allowlists required by the new bundle." should be
reworded to clarify that signature artifacts are produced by the publish
automation and must not be manually committed; update the step (referencing the
task text "Update registry metadata, docs references, signatures, and any import
allowlists") to say contributors should update registry metadata, docs, and
import allowlists as needed and ensure the publish workflow will generate
signatures, while pre-merge checks must rely on the signature/version
enforcement gate rather than creating or committing signature files manually.
In `@packages/specfact-project/resources/templates/persona/default.md.j2`:
- Line 20: The template currently dumps business and product with
tojson(indent=2) which can produce noisy Markdown; update the persona template
(default.md.j2) to either (A) replace the JSON dumps for the business and
product blocks with explicit Jinja2 field render loops when those objects follow
predictable schemas (use conditional checks on business.model,
business.revenue_streams, product.features, etc., and render as Markdown list
items) or (B) if those objects are freeform and must remain machine-readable,
retain tojson(indent=2) but add a clear template comment above each dump
explaining the intentional machine-readable JSON embedding so future maintainers
understand the choice. Ensure you modify the occurrences of tojson for the
variables business and product.
In `@packages/specfact-project/src/specfact_project/generators/template_paths.py`:
- Around line 39-40: Remove the early return for templates_dir and instead run
the explicit override through the same path shaping and validation applied to
auto-discovered roots: when templates_dir is provided, join/append subdir (if
subdir is set) to templates_dir, convert to a Path, and validate that it
contains files matching required_glob before returning it; ensure you use the
same logic/code paths that handle discovered roots so explicit overrides honor
subdir and required_glob (referencing the templates_dir, subdir and
required_glob symbols).
In `@packages/specfact-project/src/specfact_project/import_runtime_patches.py`:
- Around line 181-186: The patched wrapper uses signature.bind_partial(...)
which omits defaulted parameters; after binding call bound.apply_defaults() to
populate missing defaults before passing bound.arguments into args_builder, so
in patched (the function that does signature.bind_partial(*args, **kwargs) and
then built = args_builder(dict(bound.arguments))) insert bound.apply_defaults()
and then continue with built transformation and runner(original, context,
built); keep existing replace(built, **kwargs) behavior intact.
In `@packages/specfact-project/src/specfact_project/utils/import_path_policy.py`:
- Around line 443-468: discover_code_files normalizes file extensions only when
building ImportDiscoveryOptions from kwargs, causing inconsistency when callers
pass an ImportDiscoveryOptions instance; update the options branch to normalize
extensions too by calling _normalize_extensions on options.extensions (or
replacing options.extensions with the normalized result) before passing options
to _discover_code_files_with_options so both code paths accept extensions with
or without a leading dot; reference functions/classes: discover_code_files,
ImportDiscoveryOptions, _normalize_extensions, and
_discover_code_files_with_options.
In `@scripts/ensure_github_hierarchy_subissues.py`:
- Around line 148-159: The loop over DEFAULT_EDGES repeatedly calls
_subissue_numbers and _issue_node_id causing excess API calls; introduce simple
in-memory caches (e.g., dicts keyed by parent number and by issue number) before
the loop to store fetched sub-issue sets and node IDs, consult the cache instead
of re-calling _subissue_numbers(parent_num) and _issue_node_id(...) for the same
parent_num/child_num, and only call the underlying functions on cache misses;
still honor args.dry_run, increment skipped/would_add as before, and pass the
cached parent_id/child_id into _add_sub_issue when needed.
- Around line 67-70: The JSON response handling is brittle: change the logic
that reads payload = json.loads(proc.stdout) so you first verify payload is a
dict (isinstance(payload, dict)); only then check for "errors" in payload, and
retrieve the result using payload.get("data") instead of payload["data"]; if
payload is not a dict or payload.get("data") is None (or missing), raise a clear
RuntimeError that includes the raw payload or proc.stdout for debugging. Ensure
you reference the existing variables payload and proc when constructing the
error messages so failures are deterministic and informative.
- Around line 43-51: The current loop building args uses -F for all scalar
string variables which triggers gh's `@file` expansion; update the logic that
builds args (the args list and the variables dict handling) to use -f for string
values (including string list items and repo-owner/repo-name) so they are passed
literally, and reserve -F only for non-string typed values (e.g., ints, bools)
that need type coercion; specifically, change the branch that checks
isinstance(value, list) to emit repeated "-f" entries for string items, and in
the scalar branch emit "-f" when isinstance(value, str) else "-F" for
non-strings. Ensure the initial query part (f"query={query}") remains unchanged
and keep skipping None values.
In `@tests/unit/specfact_project/test_import_runtime_policy.py`:
- Around line 16-23: The test module currently calls
_apply_import_runtime_patches() at import time which applies global patches too
early; remove the top-level call to _apply_import_runtime_patches and instead
create a pytest fixture (e.g., import_runtime_patches_fixture) that imports
specfact_project.import_runtime_patches and calls apply_import_runtime_patches()
in its setup and ensures proper teardown/unpatching in finalizer so patches
(Path.rglob, CodeAnalyzer.analyze, import helpers) are scoped to the fixture;
update tests that need the patch to request this fixture.
In `@tests/unit/workflows/test_sign_modules_on_approval.py`:
- Around line 52-55: The assertions that check event trigger lists are brittle
due to ordering; update the tests to compare sets instead of lists: replace the
exact-equality assertions on pr_review["types"] and pr["types"] with
order-insensitive checks (e.g., assert set(pr_review["types"]) == {"submitted"}
and assert set(pr["types"]) ==
{"opened","synchronize","reopened","ready_for_review"}) so the tests no longer
fail when YAML ordering changes; locate and update the assertions referencing
pr_review["types"] and pr["types"] in test_sign_modules_on_approval.py.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc3e5b23-6543-4060-bc25-c7e4163cac5e
⛔ Files ignored due to path filters (3)
registry/modules/specfact-project-0.41.4.tar.gzis excluded by!**/*.gzregistry/modules/specfact-project-0.41.5.tar.gzis excluded by!**/*.gzregistry/modules/specfact-project-0.41.6.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (82)
.github/workflows/sign-modules-on-approval.ymldocs/bundles/project/import-migration.mdopenspec/CHANGE_ORDER.mdopenspec/changes/architecture-02-module-well-architected/.openspec.yamlopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/architecture-02-module-well-architected/specs/architecture-well-architected-module/spec.mdopenspec/changes/architecture-02-module-well-architected/tasks.mdopenspec/changes/codebase-import-runtime-hardening/.openspec.yamlopenspec/changes/codebase-import-runtime-hardening/TDD_EVIDENCE.mdopenspec/changes/codebase-import-runtime-hardening/design.mdopenspec/changes/codebase-import-runtime-hardening/proposal.mdopenspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/enterprise-01-module-policy-client/.openspec.yamlopenspec/changes/enterprise-01-module-policy-client/design.mdopenspec/changes/enterprise-01-module-policy-client/proposal.mdopenspec/changes/enterprise-01-module-policy-client/specs/enterprise-policy-client/spec.mdopenspec/changes/enterprise-01-module-policy-client/tasks.mdopenspec/changes/enterprise-02-module-audit-client/.openspec.yamlopenspec/changes/enterprise-02-module-audit-client/design.mdopenspec/changes/enterprise-02-module-audit-client/proposal.mdopenspec/changes/enterprise-02-module-audit-client/specs/enterprise-audit-client/spec.mdopenspec/changes/enterprise-02-module-audit-client/tasks.mdopenspec/changes/finops-01-module-cost-outcome/.openspec.yamlopenspec/changes/finops-01-module-cost-outcome/design.mdopenspec/changes/finops-01-module-cost-outcome/proposal.mdopenspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/knowledge-01-module-memory-runtime/.openspec.yamlopenspec/changes/knowledge-01-module-memory-runtime/design.mdopenspec/changes/knowledge-01-module-memory-runtime/proposal.mdopenspec/changes/knowledge-01-module-memory-runtime/specs/knowledge-memory-runtime/spec.mdopenspec/changes/knowledge-01-module-memory-runtime/tasks.mdopenspec/changes/knowledge-02-module-writeback/.openspec.yamlopenspec/changes/knowledge-02-module-writeback/design.mdopenspec/changes/knowledge-02-module-writeback/proposal.mdopenspec/changes/knowledge-02-module-writeback/specs/knowledge-writeback/spec.mdopenspec/changes/knowledge-02-module-writeback/tasks.mdopenspec/changes/review-resiliency-01-module/.openspec.yamlopenspec/changes/review-resiliency-01-module/design.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/review-resiliency-01-module/specs/review-resiliency-module/spec.mdopenspec/changes/review-resiliency-01-module/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/.openspec.yamlopenspec/changes/security-01-module-sast-sca-secret/design.mdopenspec/changes/security-01-module-sast-sca-secret/proposal.mdopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/security-02-module-license-compliance/.openspec.yamlopenspec/changes/security-02-module-license-compliance/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.mdopenspec/changes/security-02-module-license-compliance/tasks.mdopenspec/changes/security-03-module-pii-gdpr-eu/.openspec.yamlopenspec/changes/security-03-module-pii-gdpr-eu/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/specs/privacy-gdpr-module/spec.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdpackages/specfact-project/module-package.yamlpackages/specfact-project/resources/templates/github-action.yml.j2packages/specfact-project/resources/templates/persona/default.md.j2packages/specfact-project/resources/templates/protocol.yaml.j2packages/specfact-project/src/specfact_project/generators/plan_generator.pypackages/specfact-project/src/specfact_project/generators/protocol_generator.pypackages/specfact-project/src/specfact_project/generators/template_paths.pypackages/specfact-project/src/specfact_project/generators/workflow_generator.pypackages/specfact-project/src/specfact_project/import_cmd/commands.pypackages/specfact-project/src/specfact_project/import_runtime_patches.pypackages/specfact-project/src/specfact_project/sync/commands.pypackages/specfact-project/src/specfact_project/utils/import_path_policy.pyregistry/index.jsonregistry/modules/specfact-project-0.41.4.tar.gz.sha256registry/modules/specfact-project-0.41.5.tar.gz.sha256registry/modules/specfact-project-0.41.6.tar.gz.sha256registry/signatures/specfact-project-0.41.5.tar.sigregistry/signatures/specfact-project-0.41.6.tar.sigscripts/ensure_github_hierarchy_subissues.pytests/unit/specfact_project/test_import_runtime_policy.pytests/unit/test_bundle_resource_payloads.pytests/unit/workflows/test_sign_modules_on_approval.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 29 file(s) based on 40 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 29 file(s) based on 40 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 28
♻️ Duplicate comments (9)
openspec/changes/security-01-module-sast-sca-secret/design.md (1)
58-60:⚠️ Potential issue | 🟠 MajorFix contradictory dependency semantics for
policy-02-packs-and-modes.Line 58 treats
policy-02-packs-and-modesas a required core target, while Line 59 says it should be a module-level dependency if shipped separately. Pick one authoritative classification and remove the conflicting path.As per coding guidelines, “Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and drift vs. shipped modules or docs.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-01-module-sast-sca-secret/design.md` around lines 58 - 60, The document currently classifies policy-02-packs-and-modes in two conflicting ways; decide and make one authoritative classification: either declare policy-02-packs-and-modes as a required core contract alongside security-01-unified-findings-model (keep it in core_compatibility) or declare it as a module-level bundle dependency (remove it from the core targets list). Update the lines referencing security-01-unified-findings-model, policy-02-packs-and-modes, core_compatibility and bundle dependency so only the chosen path remains and remove the alternate path and any contradictory wording.openspec/changes/security-01-module-sast-sca-secret/tasks.md (1)
4-4:⚠️ Potential issue | 🟠 MajorCorrect the dependency verification task to distinguish core vs module changes.
Line 4 currently requires both dependencies to be merged in
specfact-cli, which is inconsistent with the documented module-level treatment ofpolicy-02-packs-and-modes. Split this task into: (a) core contract verification, and (b) module bundle dependency verification/manifest semantics.Based on learnings: “When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-01-module-sast-sca-secret/tasks.md` at line 4, Update the task in tasks.md to split the single dependency check into two distinct checks: (1) a core contract verification that ensures the core change security-01-unified-findings-model is merged in specfact-cli and documents the minimum core_compatibility versions in TDD_EVIDENCE.md, and (2) a module bundle/manifest verification that treats policy-02-packs-and-modes as a module-level dependency (verify bundle/manifest semantics and whether it must be present in the module bundle rather than requiring it to be merged into specfact-cli); make the wording explicit about which change is a core vs module dependency and include any sequencing notes.openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md (1)
7-7:⚠️ Potential issue | 🟠 MajorMake the requirement deterministic; move dependency-status gating out of normative spec text.
Line 7 currently mixes a hard requirement with an unresolved “confirm status before implementation” note. That makes the spec non-testable and drifts from “spec as source of truth.” Keep the SHALL clause definitive, and move status-tracking/gating language to
proposal.md/tasks.md/TDD_EVIDENCE.md.As per coding guidelines, “Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and drift vs. shipped modules or docs.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md` at line 7, The SHALL clause must be definitive: update the spec to state that the modules repository SHALL provide a signed official bundle named `nold-ai/specfact-license-compliance` that exposes the `license` command and declares truthful bundle dependencies, pip dependencies, and a `core_compatibility` entry for `security-01-unified-findings-model` (no conditional language), and remove the “Confirm the core change…before implementation” note from spec.md; instead add the status-tracking and gating details to proposal.md or tasks.md or TDD_EVIDENCE.md so implementation guidance (confirmation of `security-01-unified-findings-model` status and any version requirement notes) lives outside the normative spec text.openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md (1)
7-7:⚠️ Potential issue | 🟠 MajorResolve dependency classification now; avoid conditional language in the requirement.
Line 7 keeps
core_compatibilitysemantics conditional (“if ... then ...”). This leaves manifest expectations ambiguous and non-verifiable. Decide and encode one source of truth (core contract vs modulebundle_dependencies) directly in the requirement.Based on learnings: “When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md` at line 7, Update the requirement to remove conditional wording and assert a single, verifiable dependency classification: state that the modules bundle `nold-ai/specfact-security` MUST list `security-01-unified-findings-model` in core_compatibility if that contract is accepted by specfact-cli public artifacts, otherwise explicitly require `policy-02-packs-and-modes` (if shipped as a separate module) to be declared under bundle_dependencies; replace the “if … then …” phrasing with a definitive statement referencing `bundle_dependencies` and `core_compatibility` and call out the exposed `security` command and pip dependencies as mandatory manifest entries so the expectation is unambiguous.openspec/changes/knowledge-02-module-writeback/proposal.md (1)
15-21:⚠️ Potential issue | 🟠 MajorWriteback interface is still ambiguous on validation and manifest strictness.
At Line 19–Line 21, the contract names fields but doesn’t define required vs optional fields, destination-validation ownership, or preview-vs-write error semantics. That leaves adapters free to diverge.
Proposed contract tightening
### Writeback Interface @@ -- **Adapter Method Signature**: Each writeback adapter implements a standard method accepting rule identifiers, destination path, and dry-run flag -- **Manifest Schema Fields**: `ruleIds` (list of rule identifiers), `ruleVersions` (semantic versions or commit hashes), `inputDigest` (hash of source rules), `outputDigest` (hash of generated output), `timestamp` (ISO 8601 generation time), `adapterId` (writeback target type), `destination` (file path or endpoint), `previewHash` (preview content hash for verification), `dryRunFlag` (boolean indicating preview-only mode) +- **Adapter Method Signature**: Each adapter MUST accept `{ruleIds, destination, dryRun}` and return `{previewContent, writebackManifest}`. +- **Destination Validation**: Adapter validates destination format/access; caller validates policy/authorization. +- **Manifest Schema Fields**: + - **Required**: `ruleIds`, `ruleVersions`, `inputDigest`, `outputDigest`, `timestamp`, `adapterId`, `destination`, `dryRunFlag` + - **Optional**: `previewHash` (required when `dryRun=true`) +- **Error Semantics**: Preview generation errors MUST fail without side effects; final writeback errors MUST include manifest context for audit.As per coding guidelines: “openspec/**/*.md: Specification truth … drift vs. shipped modules or docs.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/knowledge-02-module-writeback/proposal.md` around lines 15 - 21, Clarify the writeback manifest by marking which manifest fields are required vs optional (e.g., require ruleIds, adapterId, destination, timestamp, inputDigest/outputDigest for integrity; make ruleVersions, previewHash, dryRunFlag optional) and add concrete validation rules for destination ownership and format (e.g., adapter must validate that destination belongs to the caller or an allowed namespace and reject unauthorized paths), define preview vs write semantics for dryRunFlag and previewHash (dryRunFlag=true must never mutate destination and must return a deterministically verifiable previewHash; when dryRunFlag=false adapters must enforce previewHash match before writing), and state explicit error codes/behaviors (validation errors vs preview-mismatch vs write-failure) referencing the manifest field names ruleIds, ruleVersions, inputDigest, outputDigest, timestamp, adapterId, destination, previewHash, dryRunFlag and the "writeback adapter" contract so implementers must follow a single strict validation and error model.openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md (1)
7-7:⚠️ Potential issue | 🟡 MinorMake telemetry prerequisite field canonical, not “or”.
Line 7 still allows two representations (“
core_compatibilityor dependency metadata fields”), which can cause manifest inconsistency and registry/tooling drift. Pick one canonical field (or define strict precedence).As per coding guidelines,
openspec/**/*.mdmust preserve specification truth and avoid contract ambiguity/drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md` at line 7, The spec currently allows two representations for the telemetry prerequisite which risks inconsistency; update the sentence so the bundle manifest MUST list telemetry-01-opentelemetry-default-on in the core_compatibility field (not “or dependency metadata fields”), e.g. require the nold-ai/specfact-finops bundle that exposes the finops command to explicitly declare telemetry-01-opentelemetry-default-on in core_compatibility (or if you prefer alternate behavior, define strict precedence between core_compatibility and dependency metadata fields and document that precedence in openspec/CHANGE_ORDER.md).openspec/changes/architecture-02-module-well-architected/design.md (1)
5-5:⚠️ Potential issue | 🟠 MajorSpecify the exact core contract/schema for architecture findings.
Lines 5, 13, and 38 reference the "paired core architecture review model" and "paired core report contract" without naming the specific contract or schema. The proposal.md adapter contract (lines 19-22) defines fields like
id,type,evidence_refs, but the coreReviewFindingmodel (fromreview-finding-modelspec) definescategory,tool,rule,file,line,message. This ambiguity creates risk of adapter implementation drift.Clarify whether:
- Architecture findings populate the existing
ReviewFindingmodel (withcategory: "architecture"), or- A separate architecture-specific findings schema is defined in the paired core contract
architecture-02-well-architected-reviewUpdate the design to explicitly name the target schema and reference the specific core contract specification.
Based on learnings, when a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics.
Also applies to: 13-13, 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/design.md` at line 5, Update the design to explicitly name and reference the exact core contract/schema used for architecture findings: state whether the module maps into the existing ReviewFinding model (from review-finding-model) with category: "architecture" and show the field mapping (e.g., map proposal.md fields id→id, type→type, evidence_refs→evidence_refs to ReviewFinding fields like id, type, evidence_refs and map category, tool, rule, file, line, message accordingly), or declare and link the separate architecture-02-well-architected-review contract if you want a distinct schema; replace all vague references to "paired core architecture review model" and "paired core report contract" with the exact contract name and a pointer to the spec so adapter implementers can avoid drift (also update occurrences at lines referenced as 13 and 38).openspec/changes/architecture-02-module-well-architected/proposal.md (2)
10-12: 🧹 Nitpick | 🔵 TrivialConsider rewording to reduce repetitive sentence starts.
Lines 10-12 begin three consecutive bullets with "NEW:", which reduces readability per static analysis (LanguageTool ENGLISH_WORD_REPEAT_BEGINNING_RULE).
✍️ Proposed rewording
-- **NEW**: Add a `specfact-architecture` bundle with an `architecture` command for boundary, interface, and well-architected review. -- **NEW**: Package analyzers and rule resources for dependency-graph checks, ADR traceability, and interface-diff evaluation. -- **NEW**: Encode the `ALLOWED_IMPORTS.md` pattern into reusable review rules so the modules repo and user repositories can share the same boundary-checking approach. +- **NEW**: Add a `specfact-architecture` bundle with an `architecture` command for boundary, interface, and well-architected review. +- **Package** analyzers and rule resources for dependency-graph checks, ADR traceability, and interface-diff evaluation. +- **Encode** the `ALLOWED_IMPORTS.md` pattern into reusable review rules so the modules repo and user repositories can share the same boundary-checking approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/proposal.md` around lines 10 - 12, The three bullets starting with the repeated marker "**NEW**:" hurt readability; revise the three items (the "Package analyzers and rule resources for dependency-graph checks, ADR traceability, and interface-diff evaluation" item, the "Encode the `ALLOWED_IMPORTS.md` pattern into reusable review rules..." item, and the "Introduce report surfaces that map architecture findings..." item) to use varied lead-ins (e.g., "Add", "Encode", "Introduce" or similar) or rephrase one or two bullets so they don't all begin with the same token while preserving meaning and references to ALLOWED_IMPORTS.md and report surfaces.
40-40:⚠️ Potential issue | 🟡 MinorUse a placeholder version range for core compatibility.
The
core_compatibility: ">=1.0.0 <2.0.0"specified on line 40 is premature. The paired core contract (architecture-02-well-architected-review) has not yet been created in specfact-cli (currently at 0.46.4), and task 1.2 in tasks.md explicitly calls for confirming the paired core change's availability before documenting the required compatibility range. Replace with a placeholder such as:core_compatibility: "TBD (set when architecture-02-well-architected-review is available in specfact-cli)"This ensures the version range reflects actual core availability rather than speculative targeting of a future release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/proposal.md` at line 40, Replace the speculative core version in the reserved manifest by changing the core_compatibility value for the manifest named `architecture-02-well-architected-review` to a placeholder until the paired core contract exists; specifically, update the `core_compatibility` field (currently set to ">=1.0.0 <2.0.0") to a clear placeholder like `TBD (set when architecture-02-well-architected-review is available in specfact-cli)` so the document defers exact versioning until task 1.2 confirms the core contract availability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/bundles/project/import-migration.md`:
- Line 272: Add a final trailing newline to the markdown file by ensuring the
last line containing "**Happy importing!** 🚀" ends with a newline character;
simply append a newline after that line so the file ends with a blank line to
satisfy MD047.
- Around line 39-40: Update the doc text that currently lists thresholds
(>10,000 ignored entries, ~100,000 files, ~10 GB, >100,000 commits) to match the
implementation: state the heavy ignored-entry threshold is 500 entries and the
large-repo file threshold is 1,000 files, and remove references to GB or
commit-count thresholds (or replace the fixed values with a note that the system
uses configured thresholds) so the sentence about remaining-time estimates
refers to repositories above those actual/configured thresholds; update the
phrases that mention "ignored artifact trees" and "Remaining time is based
on..." to reflect the corrected thresholds or that such limits are configurable.
In `@openspec/CHANGE_ORDER.md`:
- Line 129: The file's last line (the table row containing "| peer-deps | 01 | ✅
module-bundle-deps-auto-install (archived 2026-04-05) |
[`#135`](https://github.com/nold-ai/specfact-cli-modules/issues/135) | — |") is
missing a terminating newline which triggers MD047; add a single trailing
newline character at the end of CHANGE_ORDER.md so the file ends with a newline
and re-run markdownlint to confirm the warning is cleared.
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Line 50: The document references a required core contract
architecture-02-well-architected-review that is not present in specfact-cli; to
fix, block merging of this design until that core change is created and synced
in specfact-cli or explicitly add a dependency/blocker statement in this file
noting that module integration and any references to
architecture-02-well-architected-review (and overlaps with
architecture-01-solution-layer) are disallowed until the paired specfact-cli
change exists and is marked stable; update design.md to include a clear
dependency checklist and a gating sentence that prevents implementation work
until specfact-cli contains the architecture-02-well-architected-review
artifact.
- Line 57: The file ends without a trailing newline which violates markdownlint
MD047; open the file containing the line "Should interface diff support start
with one ecosystem or ship as a provider abstraction from the first
implementation?" (design.md) and add a single newline character at the end of
the file so it ends with exactly one trailing newline.
- Around line 1-57: Ensure the paired core change
"architecture-02-well-architected-review" exists and is synced into specfact-cli
before merging this module design: verify the core change defines the
findings/report contract referenced by packages/specfact-architecture, confirm
the module-to-core binding points (adapter contracts and report schema used by
the analyzer adapters and rule-resource translation) are finalized and tested
against specfact-cli, update CHANGE_ORDER (order-03) and the Migration Plan to
reflect actual core availability and registry timeline, and block or defer
merging of the architecture-02 module design until those confirmations/pass CI
tests are completed.
In `@openspec/changes/architecture-02-module-well-architected/proposal.md`:
- Line 54: The file ends without a trailing newline (see the line containing
"**Sanitized**: false" in proposal.md); add a single newline character at the
end of the file so the file terminates with exactly one trailing newline to
satisfy markdownlint MD047.
- Around line 15-22: Update the "Adapter Contract" section so it maps analyzer
outputs to the core ReviewFinding model instead of a custom schema: replace the
"Required Core Findings Schema Fields" and "Analyzer Output Mapping"
descriptions with mapping to ReviewFinding fields — set category="architecture",
tool=<analyzer name>, rule=<violation rule id>, file=<file path>, line=<line
number or range>, message=<human-readable description>, and
severity=<error|warning|info>; retain evidence_refs only as supplemental
references but ensure primary location data uses file/line; align wording with
the ReviewFinding spec and mention pylint-runner expectation that category must
be "architecture" and tool "pylint".
In `@openspec/changes/codebase-import-runtime-hardening/design.md`:
- Line 93: Add a terminal newline to the end of the document so the final line
"None for implementation start; warning thresholds can be tuned in-code if test
evidence shows the initial defaults are too noisy." ends with a newline (satisfy
markdownlint MD047); simply ensure the file's EOF contains a trailing newline
character.
In
`@openspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.md`:
- Line 19: The file ends without a trailing newline causing MD047; append a
single final newline character to the end of the spec so the last line "**AND**
the failure message SHALL name the missing template path so the packaging defect
can be corrected before publish" is terminated by a newline (i.e., ensure the
file ends with one newline character).
In
`@openspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md`:
- Line 57: The file ends without a trailing newline on the final line containing
"- **AND** any early estimate before full discovery SHALL be labeled as
provisional rather than a fixed promise"; open the file and add a single newline
character (EOF newline) after that line so the document ends with a trailing
newline to satisfy MD047.
In `@openspec/changes/codebase-import-runtime-hardening/tasks.md`:
- Line 32: Add a single trailing newline at the end of
openspec/changes/codebase-import-runtime-hardening/tasks.md so the final line "-
[ ] 5.4 Commit, push, and open or update the PR to `dev` after verification is
green." is followed by one newline (fixes MD047); simply ensure the file ends
with a single blank line.
In `@openspec/changes/enterprise-02-module-audit-client/design.md`:
- Around line 1-57: The file ending after the "## Open Questions" section (look
for the top-level heading "# Design" or the "## Context" / "## Open Questions"
markers) is missing a trailing newline; to fix, open the markdown file for
writing and append a single newline character at the end of the file so the last
line (after "How should retry cadence and dead-letter behavior be surfaced in
the first client contract?") ends with a newline to satisfy MD047.
In
`@openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md`:
- Line 30: Append a single trailing newline to the end of the markdown file so
the final line "**THEN** it records the session outcome as `rule-updated` in the
normalized FinOps evidence" ends with a newline (fixing MD047); ensure no other
whitespace changes are made.
In `@openspec/changes/finops-01-module-cost-outcome/tasks.md`:
- Line 27: The file ends without a trailing newline causing MD047; add a single
trailing newline to the end of the file so the final checklist line "- [ ] 4.5
Open the modules PR to `dev`, cross-link paired telemetry/FinOps changes, and
note any deferred provider integrations as follow-up issues." is terminated by a
newline character (ensure only one final newline).
In `@openspec/changes/knowledge-01-module-memory-runtime/design.md`:
- Line 55: Add a trailing newline at the end of the file by ensuring the final
line "- Should search support only tags and keywords initially, or also
relationship traversal commands?" is followed by a single newline character
(i.e., ensure EOF ends with a newline) to satisfy markdownlint MD047.
In `@openspec/changes/knowledge-02-module-writeback/proposal.md`:
- Line 52: Add a single trailing newline at the end of the markdown file that
contains the line "- **Sanitized**: false" (the proposal.md for the
knowledge-02-module-writeback change) so the file ends with exactly one newline
character; ensure there are no extra blank lines or removal of other content and
save the file to satisfy MD047.
In `@openspec/changes/review-resiliency-01-module/proposal.md`:
- Line 52: Append a single trailing newline to the end of the file so the last
line "- **Issue URL**:
<https://github.com/nold-ai/specfact-cli-modules/issues/226>" is followed by a
newline character (ensuring the file ends with exactly one EOF newline) to
satisfy MD047.
- Around line 19-24: The proposal's "Core Findings Schema Fields" in proposal.md
currently uses id/title/description/location/timestamp/confidence/evidence_refs
and a severity mapping (Critical/High/Medium/Low), which diverges from the
canonical review finding schema; update the adapter contract in proposal.md to
match openspec/specs/review-finding-model/spec.md by using the canonical fields
(category, severity, tool, rule, file, line, message, fixable) and the severity
enum values (error|warning|info), and adapt any references to
evidence_refs/location to map into file/line and message or add a clear mapping
section showing how id/title/description/timestamp/confidence map to the
canonical fields (e.g., id→rule or tool-specific id, description→message,
location→file/line), so the payloads remain compatible with the core spec.
In `@openspec/changes/security-01-module-sast-sca-secret/design.md`:
- Line 64: Add a trailing newline at the end of the markdown file containing the
line "Should SBOM generation be mandatory for every security run or enabled only
when dependency analysis is requested?" to satisfy MD047; simply ensure the file
ends with a single newline character so the EOF is a blank line.
In
`@openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md`:
- Line 30: Add a single final trailing newline at the end of the file so it ends
with exactly one newline; specifically update the file containing the line "-
**THEN** the bundle surfaces the failure with the normalized report contract and
a non-success command outcome" to ensure there is one and only one newline
character after the last line.
In `@openspec/changes/security-01-module-sast-sca-secret/tasks.md`:
- Line 27: Add a trailing newline to the end of the file to satisfy markdownlint
rule MD047: open the file containing the line "4.5 Open the modules PR to `dev`,
cross-link paired security changes, and call out any deferred scanner providers
as follow-up issues." (tasks.md) and ensure the file ends with a single newline
character, save, and commit the change.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Line 46: Remove the unresolved editorial parenthetical and replace it with a
single definitive dependency sentence: state that this proposal depends on
`security-01-unified-findings-model` and the shared policy-pack semantics from
`policy-02-packs-and-modes`, and remove any mention of
`security-02-eu-gdpr-baseline` and the “if the intention was…” note so the
dependency line references only the final, unambiguous dependency set.
- Line 57: The file containing the line "**GitHub Issue**:
[`#228`](https://github.com/nold-ai/specfact-cli-modules/issues/228)" fails
markdownlint MD047 (missing EOF trailing newline); fix it by adding a single
newline character at the end of that markdown file (ensure the file ends with a
blank line) so the EOF has a trailing newline and the MD047 warning is resolved.
In
`@openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md`:
- Line 30: Add a single trailing newline character to the end of the file to
satisfy markdownlint rule MD047 (i.e., ensure the file ends with a newline after
the last line “**THEN** the bundle reports the finding without converting the
overall command outcome into a blocking failure”).
In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md`:
- Around line 17-18: Add a concrete checklist task to verify compatibility with
nold-ai/specfact-cli whenever changing shared privacy/GDPR findings/report
contracts (e.g., alongside items "3.2 Implement normalization..." and "3.3
Integrate shared policy/profile handling..."); specifically, add a step that
requires running and reviewing the paired public artifacts/tests in specfact-cli
and confirming any required adapter changes or test updates before PR handoff so
the change consumer (specfact-cli) is validated and noted in the checklist.
In `@packages/specfact-project/resources/templates/persona/default.md.j2`:
- Around line 20-21: The JSON output from the Jinja expressions business |
tojson(indent=2) and product | tojson(indent=2) is currently emitted raw; wrap
each rendered JSON block in Markdown fenced code blocks annotated with json so
the template lines that render {{ business | tojson(indent=2) }} and {{ product
| tojson(indent=2) }} become fenced (```json ... ```) to improve readability in
rendered Markdown while preserving the existing tojson usage.
In `@scripts/ensure_github_hierarchy_subissues.py`:
- Around line 118-121: The code iterates raw_nodes = sub.get("nodes", [])
without guarding its type; add a type check before the loop (similar to the
existing guards around issue and pageInfo) to ensure raw_nodes is a list and
raise a controlled RuntimeError if not, then proceed to iterate and call
collected.add(int(node["number"])) for dict entries; reference the variables
raw_nodes and sub and mirror the error messaging/behavior used for issue and
pageInfo so failures remain deterministic.
---
Duplicate comments:
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Line 5: Update the design to explicitly name and reference the exact core
contract/schema used for architecture findings: state whether the module maps
into the existing ReviewFinding model (from review-finding-model) with category:
"architecture" and show the field mapping (e.g., map proposal.md fields id→id,
type→type, evidence_refs→evidence_refs to ReviewFinding fields like id, type,
evidence_refs and map category, tool, rule, file, line, message accordingly), or
declare and link the separate architecture-02-well-architected-review contract
if you want a distinct schema; replace all vague references to "paired core
architecture review model" and "paired core report contract" with the exact
contract name and a pointer to the spec so adapter implementers can avoid drift
(also update occurrences at lines referenced as 13 and 38).
In `@openspec/changes/architecture-02-module-well-architected/proposal.md`:
- Around line 10-12: The three bullets starting with the repeated marker
"**NEW**:" hurt readability; revise the three items (the "Package analyzers and
rule resources for dependency-graph checks, ADR traceability, and interface-diff
evaluation" item, the "Encode the `ALLOWED_IMPORTS.md` pattern into reusable
review rules..." item, and the "Introduce report surfaces that map architecture
findings..." item) to use varied lead-ins (e.g., "Add", "Encode", "Introduce" or
similar) or rephrase one or two bullets so they don't all begin with the same
token while preserving meaning and references to ALLOWED_IMPORTS.md and report
surfaces.
- Line 40: Replace the speculative core version in the reserved manifest by
changing the core_compatibility value for the manifest named
`architecture-02-well-architected-review` to a placeholder until the paired core
contract exists; specifically, update the `core_compatibility` field (currently
set to ">=1.0.0 <2.0.0") to a clear placeholder like `TBD (set when
architecture-02-well-architected-review is available in specfact-cli)` so the
document defers exact versioning until task 1.2 confirms the core contract
availability.
In
`@openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md`:
- Line 7: The spec currently allows two representations for the telemetry
prerequisite which risks inconsistency; update the sentence so the bundle
manifest MUST list telemetry-01-opentelemetry-default-on in the
core_compatibility field (not “or dependency metadata fields”), e.g. require the
nold-ai/specfact-finops bundle that exposes the finops command to explicitly
declare telemetry-01-opentelemetry-default-on in core_compatibility (or if you
prefer alternate behavior, define strict precedence between core_compatibility
and dependency metadata fields and document that precedence in
openspec/CHANGE_ORDER.md).
In `@openspec/changes/knowledge-02-module-writeback/proposal.md`:
- Around line 15-21: Clarify the writeback manifest by marking which manifest
fields are required vs optional (e.g., require ruleIds, adapterId, destination,
timestamp, inputDigest/outputDigest for integrity; make ruleVersions,
previewHash, dryRunFlag optional) and add concrete validation rules for
destination ownership and format (e.g., adapter must validate that destination
belongs to the caller or an allowed namespace and reject unauthorized paths),
define preview vs write semantics for dryRunFlag and previewHash
(dryRunFlag=true must never mutate destination and must return a
deterministically verifiable previewHash; when dryRunFlag=false adapters must
enforce previewHash match before writing), and state explicit error
codes/behaviors (validation errors vs preview-mismatch vs write-failure)
referencing the manifest field names ruleIds, ruleVersions, inputDigest,
outputDigest, timestamp, adapterId, destination, previewHash, dryRunFlag and the
"writeback adapter" contract so implementers must follow a single strict
validation and error model.
In `@openspec/changes/security-01-module-sast-sca-secret/design.md`:
- Around line 58-60: The document currently classifies policy-02-packs-and-modes
in two conflicting ways; decide and make one authoritative classification:
either declare policy-02-packs-and-modes as a required core contract alongside
security-01-unified-findings-model (keep it in core_compatibility) or declare it
as a module-level bundle dependency (remove it from the core targets list).
Update the lines referencing security-01-unified-findings-model,
policy-02-packs-and-modes, core_compatibility and bundle dependency so only the
chosen path remains and remove the alternate path and any contradictory wording.
In
`@openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md`:
- Line 7: Update the requirement to remove conditional wording and assert a
single, verifiable dependency classification: state that the modules bundle
`nold-ai/specfact-security` MUST list `security-01-unified-findings-model` in
core_compatibility if that contract is accepted by specfact-cli public
artifacts, otherwise explicitly require `policy-02-packs-and-modes` (if shipped
as a separate module) to be declared under bundle_dependencies; replace the “if
… then …” phrasing with a definitive statement referencing `bundle_dependencies`
and `core_compatibility` and call out the exposed `security` command and pip
dependencies as mandatory manifest entries so the expectation is unambiguous.
In `@openspec/changes/security-01-module-sast-sca-secret/tasks.md`:
- Line 4: Update the task in tasks.md to split the single dependency check into
two distinct checks: (1) a core contract verification that ensures the core
change security-01-unified-findings-model is merged in specfact-cli and
documents the minimum core_compatibility versions in TDD_EVIDENCE.md, and (2) a
module bundle/manifest verification that treats policy-02-packs-and-modes as a
module-level dependency (verify bundle/manifest semantics and whether it must be
present in the module bundle rather than requiring it to be merged into
specfact-cli); make the wording explicit about which change is a core vs module
dependency and include any sequencing notes.
In
`@openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md`:
- Line 7: The SHALL clause must be definitive: update the spec to state that the
modules repository SHALL provide a signed official bundle named
`nold-ai/specfact-license-compliance` that exposes the `license` command and
declares truthful bundle dependencies, pip dependencies, and a
`core_compatibility` entry for `security-01-unified-findings-model` (no
conditional language), and remove the “Confirm the core change…before
implementation” note from spec.md; instead add the status-tracking and gating
details to proposal.md or tasks.md or TDD_EVIDENCE.md so implementation guidance
(confirmation of `security-01-unified-findings-model` status and any version
requirement notes) lives outside the normative spec text.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9129b38b-0edd-4884-af86-01886b808973
📒 Files selected for processing (29)
docs/bundles/project/import-migration.mdopenspec/CHANGE_ORDER.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/codebase-import-runtime-hardening/design.mdopenspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/enterprise-01-module-policy-client/specs/enterprise-policy-client/spec.mdopenspec/changes/enterprise-02-module-audit-client/design.mdopenspec/changes/enterprise-02-module-audit-client/specs/enterprise-audit-client/spec.mdopenspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/knowledge-01-module-memory-runtime/design.mdopenspec/changes/knowledge-01-module-memory-runtime/proposal.mdopenspec/changes/knowledge-01-module-memory-runtime/specs/knowledge-memory-runtime/spec.mdopenspec/changes/knowledge-02-module-writeback/design.mdopenspec/changes/knowledge-02-module-writeback/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-01-module-sast-sca-secret/design.mdopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/security-02-module-license-compliance/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdpackages/specfact-project/resources/templates/persona/default.md.j2scripts/ensure_github_hierarchy_subissues.pytests/unit/workflows/test_sign_modules_on_approval.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/enterprise-01-module-policy-client/specs/enterprise-policy-client/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.mdopenspec/changes/enterprise-02-module-audit-client/specs/enterprise-audit-client/spec.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/knowledge-01-module-memory-runtime/proposal.mdopenspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.mdopenspec/changes/knowledge-01-module-memory-runtime/design.mdopenspec/changes/enterprise-02-module-audit-client/design.mdopenspec/changes/knowledge-02-module-writeback/design.mdopenspec/changes/security-01-module-sast-sca-secret/design.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/codebase-import-runtime-hardening/design.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/changes/security-02-module-license-compliance/design.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/knowledge-01-module-memory-runtime/specs/knowledge-memory-runtime/spec.mdopenspec/changes/knowledge-02-module-writeback/proposal.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
tests/unit/workflows/test_sign_modules_on_approval.pyscripts/ensure_github_hierarchy_subissues.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/workflows/test_sign_modules_on_approval.py
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/bundles/project/import-migration.md
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/ensure_github_hierarchy_subissues.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
📚 Learning: 2026-04-13T10:38:15.855Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.855Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
openspec/changes/enterprise-01-module-policy-client/specs/enterprise-policy-client/spec.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
openspec/changes/enterprise-02-module-audit-client/specs/enterprise-audit-client/spec.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/knowledge-01-module-memory-runtime/proposal.mdopenspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.mdopenspec/changes/knowledge-01-module-memory-runtime/design.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdtests/unit/workflows/test_sign_modules_on_approval.pyopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/knowledge-02-module-writeback/proposal.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Applied to files:
openspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/CHANGE_ORDER.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
openspec/changes/knowledge-01-module-memory-runtime/proposal.mdopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
openspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
openspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
openspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Work belongs on feature/*, bugfix/*, hotfix/*, or chore/* branches, normally in a worktree rooted under ../specfact-cli-modules-worktrees/
Applied to files:
openspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: This repository enforces the clean-code review gate through hatch run specfact code review run --json --out .specfact/code-review.json
Applied to files:
openspec/changes/finops-01-module-cost-outcome/tasks.mdopenspec/changes/security-01-module-sast-sca-secret/tasks.mdopenspec/changes/codebase-import-runtime-hardening/tasks.mdopenspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Read docs/agent-rules/05-non-negotiable-checklist.md before implementation
Applied to files:
openspec/changes/security-01-module-sast-sca-secret/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Refresh .specfact/backlog/github_hierarchy_cache.md with python scripts/sync_github_hierarchy_cache.py when GitHub hierarchy metadata is missing or stale before parent or blocker work
Applied to files:
scripts/ensure_github_hierarchy_subissues.pyopenspec/changes/codebase-import-runtime-hardening/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: If GitHub hierarchy metadata is needed and `.specfact/backlog/github_hierarchy_cache.md` is missing or stale, refresh it with `python scripts/sync_github_hierarchy_cache.py`
Applied to files:
scripts/ensure_github_hierarchy_subissues.pyopenspec/changes/codebase-import-runtime-hardening/tasks.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
openspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md
🪛 LanguageTool
openspec/changes/architecture-02-module-well-architected/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ity, and interface-diff evaluation. - NEW: Encode the ALLOWED_IMPORTS.md patt...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/review-resiliency-01-module/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... the paired core findings contract. - NEW: Add optional probe adapters for ligh...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s stay local-first and safe for CI. - NEW: Emit findings in the shared review r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/security-02-module-license-compliance/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re security/license findings model. - NEW: Add policy-aware handling for allowe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...face explicit remediation guidance. - NEW: Define bundle manifest, registry, do...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/knowledge-02-module-writeback/proposal.md
[uncategorized] ~10-~10: The official name of this software platform is spelled with a capital “H”.
Context: ...EW**: Package adapters for BUGBOT.md, .github/copilot-instructions.md, and CodeRabbi...
(GITHUB)
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...drafts as opt-in writeback targets. - NEW: Require preview/dry-run output and e...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...eviewable and safe for repo owners. - NEW: Normalize selected rule metadata int...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/codebase-import-runtime-hardening/tasks.md
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...nd change scaffolding - [ ] 1.1 Verify .specfact/backlog/github_hierarchy_cache.md is fresh, confirm t...
(GITHUB)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/codebase-import-runtime-hardening/specs/bundle-packaged-resources/spec.md
[warning] 19-19: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/architecture-02-module-well-architected/proposal.md
[warning] 54-54: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/finops-01-module-cost-outcome/specs/finops-cost-outcome-module/spec.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/knowledge-01-module-memory-runtime/design.md
[warning] 55-55: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/enterprise-02-module-audit-client/design.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-01-module-sast-sca-secret/design.md
[warning] 64-64: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/architecture-02-module-well-architected/design.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/review-resiliency-01-module/proposal.md
[warning] 52-52: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/codebase-import-runtime-hardening/design.md
[warning] 93-93: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/finops-01-module-cost-outcome/tasks.md
[warning] 27-27: Files should end with a single newline character
(MD047, single-trailing-newline)
docs/bundles/project/import-migration.md
[warning] 272-272: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-01-module-sast-sca-secret/specs/security-sast-sca-secret-module/spec.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-01-module-sast-sca-secret/tasks.md
[warning] 27-27: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-02-module-license-compliance/proposal.md
[warning] 57-57: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/knowledge-02-module-writeback/proposal.md
[warning] 52-52: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/codebase-import-runtime-hardening/tasks.md
[warning] 32-32: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-02-module-license-compliance/specs/license-compliance-module/spec.md
[warning] 27-27: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/codebase-import-runtime-hardening/specs/codebase-import-runtime/spec.md
[warning] 57-57: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/CHANGE_ORDER.md
[warning] 129-129: Files should end with a single newline character
(MD047, single-trailing-newline)
🔀 Multi-repo context nold-ai/specfact-cli
Linked repositories findings
nold-ai/specfact-cli
-
init template resolver usage:
- src/specfact_cli/modules/init/src/commands.py:271 — defines
_resolve_templates_dir(repo_path: Path) -> Path | None. This is a consumer of bundled template-resolution behavior. [::nold-ai/specfact-cli::]
- src/specfact_cli/modules/init/src/commands.py:271 — defines
-
Tests asserting template resolution behavior:
- tests/unit/specfact_cli/registry/test_init_module_lifecycle_ux.py:125–133 — test
test_resolve_templates_dir_none_when_no_discoverable_promptscallsinit_commands._resolve_templates_dir(tmp_path)and asserts behavior when no discoverable templates exist. This will exercise any change to template resolution defaults/lookup. [::nold-ai/specfact-cli::]
- tests/unit/specfact_cli/registry/test_init_module_lifecycle_ux.py:125–133 — test
Summary judgement:
- The repository contains direct consumers/tests of template resolution logic (resolve_templates_dir behavior) that are relevant to the promoted changes around template packaging and resolver helpers. No other references to the new import/runtime patching APIs (import_path_policy, import_runtime_patches, apply_import_runtime_patches, discover_code_files) were found in this repo. [::nold-ai/specfact-cli::]
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 21 file(s) based on 27 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 21 file(s) based on 27 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
scripts/ensure_github_hierarchy_subissues.py (1)
185-187:⚠️ Potential issue | 🟡 MinorKeep the in-memory sub-issue cache consistent after writes.
After a successful
_add_sub_issue, the localexistingset is not updated. IfDEFAULT_EDGESever contains a duplicate(parent, child)in the same run, this can trigger a second mutation attempt.Suggested fix
_add_sub_issue(parent_id=parent_id, child_id=child_id) + existing.add(child_num) sys.stdout.write(f"addSubIssue #{child_num} -> parent #{parent_num}\n") added += 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ensure_github_hierarchy_subissues.py` around lines 185 - 187, After calling _add_sub_issue(parent_id=parent_id, child_id=child_id) you need to update the in-memory cache so duplicate edges aren't retried; add the tuple (parent_id, child_id) to the existing set immediately after the successful write (before incrementing added or continuing) so DEFAULT_EDGES duplicates are ignored in the same run and the local state stays consistent.openspec/changes/architecture-02-module-well-architected/proposal.md (1)
69-69:⚠️ Potential issue | 🟡 MinorAdd trailing newline.
The file ends without a trailing newline character, violating markdownlint MD047. Files should end with a single newline character.
📝 Proposed fix
- **Sanitized**: false +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/proposal.md` at line 69, The file ends without a trailing newline (last line contains "- **Sanitized**: false"); add a single newline character at the end of openspec/changes/architecture-02-module-well-architected/proposal.md so the file ends with exactly one trailing newline to satisfy markdownlint MD047.openspec/changes/architecture-02-module-well-architected/design.md (1)
65-65:⚠️ Potential issue | 🟡 MinorAdd trailing newline.
The file ends without a trailing newline character, violating markdownlint MD047. Files should end with a single newline character.
📝 Proposed fix
- Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation? +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/design.md` at line 65, Add a single trailing newline character at the end of the file so the last line "Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation?" is followed by exactly one newline (no extra blank lines) to satisfy markdownlint MD047.
🤖 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/architecture-02-module-well-architected/design.md`:
- Around line 30-35: Decision 2's portable-rule proposal risks losing
enforcement fidelity from ALLOWED_IMPORTS.md; update the portable rule schema
and enforcement code so it preserves all three policy dimensions: (1) dual-mode
prefix matching implemented by preserving the _is_allowed_prefix semantics (add
a flag/field for exact+dot-prefix matching and ensure the rule parser and
matcher honor it), (2) a MIGRATE-tier blocking marker or enforcement-level field
that causes unconditional forbids for non-allowed imports in that tier, and (3)
directional cross-bundle allowlists (explicit source→target allow entries) so
the bundle isolation rules are expressible; modify the rule parser/loader,
matcher (where _is_allowed_prefix is referenced), and enforcement engine to read
these fields and enforce them deterministically as described in Decision 2.
- Around line 50-61: The BLOCKER currently named "Module integration and any
references to `architecture-02-well-architected-review` are disallowed..." is
gating the design merge even though the paired core contract in `specfact-cli`
doesn't exist; either remove or relax that gate and add a clear meta-note: state
that this design can merge as a design artifact but that implementation PRs must
verify the dependency checklist (presence of
`architecture-02-well-architected-review` in `specfact-cli`, documented overlaps
with `architecture-01-solution-layer`, and a stable paired core change) before
any module integration or references are added; update the checklist text and
BLOCKER heading to reflect "Implementation BLOCKER" and reference the exact
contract name `architecture-02-well-architected-review` and repo `specfact-cli`
so reviewers know the required precondition without blocking the design doc
itself.
In `@openspec/changes/architecture-02-module-well-architected/proposal.md`:
- Around line 30-33: The proposal mentions an evidence_refs field but the
ReviewFinding contract (class ReviewFinding in
packages/specfact-code-review/src/specfact_code_review/run/findings.py) lacks
it; either remove the evidence_refs sentence from the proposal or add an
optional evidence_refs attribute to ReviewFinding (e.g., evidence_refs:
Optional[List[Dict[str, Any]]] or similar), update its docstring to state it is
optional and the expected shape (stable file paths, line ranges, artifact
identifiers), and update adapters that emit ReviewFinding to populate
evidence_refs accordingly so the model and proposal stay consistent.
In `@openspec/changes/review-resiliency-01-module/proposal.md`:
- Around line 11-12: The three consecutive bullets all start with the lead-in
"**NEW**", which reduces readability; edit the two bullets "Add optional probe
adapters for lightweight load-profile validation behind explicit opt-in flags so
default review runs stay local-first and safe for CI." and "Emit findings in the
shared review report shape and, when the knowledge bundle is installed, forward
evidence metadata to the memory runtime without making that integration
mandatory." to use varied lead-ins (e.g., "OPTIONAL:", "NOTE:", or drop the tag
for one) so only one line begins with "**NEW**" and the others use distinct
phrasing while preserving the original meaning.
- Line 21: The proposal allows ad-hoc values for the ReviewFinding field
"category", which breaks validation against the canonical enum; update the text
for "category" to require only existing allowed enum values (do not say “extend
with resiliency-specific categories if needed”), and add a note directing
contributors to propose new categories via a core-spec change rather than in
this module; reference the "category" field and "ReviewFinding" type in the
phrasing so readers know which symbol is constrained and mention routing new
categories through the core spec process.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Around line 19-24: This references an unmerged contract
(security-01-unified-findings-model) so either (A) confirm that
security-01-unified-findings-model has been accepted/merged and then explicitly
state that dependency in security-02, or (B) inline the full findings schema
within security-02 (document the `id`, `type`, `severity`, `description`,
`source`, `timestamp`, `evidence_refs` fields and their types/constraints) and
move the SPDX-to-normalized mapping rules into security-02 itself (the mappings
for `type` and `severity` such as `license-violation`, `license-compliant`,
`license-exception-applied`, `license-unknown`) so implementations do not rely
on an external, unaccepted contract; update any wording that currently says
“reused for license findings” to reflect the chosen approach.
In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md`:
- Line 4: Add an explicit checklist task to update the module manifest’s
core_compatibility field: modify the tasks document to include a step in section
3 (near the manifest scaffolding) that instructs the implementer to update
packages/specfact-pii-gdpr/module-package.yaml (module-package.yaml) with the
minimum required core_compatibility and to record that value in the task list;
reference the existing checklist item about documenting minimum required
core_compatibility so it is clear this new task is the concrete action to update
the manifest.
- Line 19: Move the checklist item "3.4 Verify compatibility with
`nold-ai/specfact-cli` whenever changing shared privacy/GDPR findings or report
contracts..." out of section 3 "Bundle implementation" and into section 4
"Verification and delivery" (place it immediately before or after the existing
verification tasks around the start of section 4), so compatibility with
nold-ai/specfact-cli is performed as part of final verification/delivery rather
than during implementation; keep the exact checklist wording and numbering
adjusted to the new section's sequence.
- Line 16: Add a new task to the checklist for "3.1 Scaffold
`packages/specfact-pii-gdpr/`" that enforces packaging validation of bundled
rule resources: implement a pre-publish validation step (e.g., "Validate bundled
rule resources are packaged") which checks that all required bundled
templates/resources shipped with `packages/specfact-pii-gdpr/` are present in
the package bundle and fails the release if any are missing, and ensure the
failure message explicitly includes the missing resource path; place this task
near section 3.5 or immediately after the existing scaffold task so it is part
of the release checklist.
---
Duplicate comments:
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Line 65: Add a single trailing newline character at the end of the file so the
last line "Should interface diff support start with one ecosystem or ship as a
provider abstraction from the first implementation?" is followed by exactly one
newline (no extra blank lines) to satisfy markdownlint MD047.
In `@openspec/changes/architecture-02-module-well-architected/proposal.md`:
- Line 69: The file ends without a trailing newline (last line contains "-
**Sanitized**: false"); add a single newline character at the end of
openspec/changes/architecture-02-module-well-architected/proposal.md so the file
ends with exactly one trailing newline to satisfy markdownlint MD047.
In `@scripts/ensure_github_hierarchy_subissues.py`:
- Around line 185-187: After calling _add_sub_issue(parent_id=parent_id,
child_id=child_id) you need to update the in-memory cache so duplicate edges
aren't retried; add the tuple (parent_id, child_id) to the existing set
immediately after the successful write (before incrementing added or continuing)
so DEFAULT_EDGES duplicates are ignored in the same run and the local state
stays consistent.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 383775c3-c52e-4b76-ae47-ed9d20e8bbe5
📒 Files selected for processing (7)
docs/bundles/project/import-migration.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdscripts/ensure_github_hierarchy_subissues.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/bundles/project/import-migration.md
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.md
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
scripts/ensure_github_hierarchy_subissues.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/ensure_github_hierarchy_subissues.py
🧠 Learnings (12)
📓 Common learnings
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
docs/bundles/project/import-migration.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.md
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.mdopenspec/changes/architecture-02-module-well-architected/proposal.mdopenspec/changes/review-resiliency-01-module/proposal.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:15.855Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.855Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Work belongs on feature/*, bugfix/*, hotfix/*, or chore/* branches, normally in a worktree rooted under ../specfact-cli-modules-worktrees/
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Refresh .specfact/backlog/github_hierarchy_cache.md with python scripts/sync_github_hierarchy_cache.py when GitHub hierarchy metadata is missing or stale before parent or blocker work
Applied to files:
scripts/ensure_github_hierarchy_subissues.py
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: If GitHub hierarchy metadata is needed and `.specfact/backlog/github_hierarchy_cache.md` is missing or stale, refresh it with `python scripts/sync_github_hierarchy_cache.py`
Applied to files:
scripts/ensure_github_hierarchy_subissues.py
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Require public GitHub metadata completeness before implementation when linked issue workflow applies: parent, labels, project assignment, blockers, and blocked-by relationships
Applied to files:
scripts/ensure_github_hierarchy_subissues.py
🪛 LanguageTool
openspec/changes/architecture-02-module-well-architected/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ity, and interface-diff evaluation. - NEW: Encode the ALLOWED_IMPORTS.md patt...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/security-02-module-license-compliance/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re security/license findings model. - NEW: Add policy-aware handling for allowe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...face explicit remediation guidance. - NEW: Define bundle manifest, registry, do...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/review-resiliency-01-module/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... the paired core findings contract. - NEW: Add optional probe adapters for ligh...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s stay local-first and safe for CI. - NEW: Emit findings in the shared review r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔀 Multi-repo context nold-ai/specfact-cli
nold-ai/specfact-cli
-
Consumers of runtime template-resolution helpers / bundled templates:
- src/specfact_cli/modules/init/src/commands.py:271–279 — _resolve_templates_dir (resolves dev resources/templates prompts) [::nold-ai/specfact-cli::]
- src/specfact_cli/generators/workflow_generator.py:29–38 — workflow generator default templates_dir fallback to resources/templates [::nold-ai/specfact-cli::]
- src/specfact_cli/generators/protocol_generator.py:23–34 — protocol generator default templates_dir fallback to resources/templates [::nold-ai/specfact-cli::]
- src/specfact_cli/generators/plan_generator.py:24–33 — plan generator default templates_dir fallback to resources/templates [::nold-ai/specfact-cli::]
- src/specfact_cli/generators/persona_exporter.py:50–66, 387 — PersonaExporter resolves templates_dir (expects persona/default.md.j2) [::nold-ai/specfact-cli::]
- src/specfact_cli/utils/ide_setup.py:127–152, 380–397, 776–796 — IDE template discovery/copying uses resources/prompts and module template discovery logic [::nold-ai/specfact-cli::]
- src/specfact_cli/utils/startup_checks.py:5,156–208,334–454 — startup checks that examine template availability/outdatedness [::nold-ai/specfact-cli::]
- src/specfact_cli/templates/registry.py:240–271, 310–312 — template registry loading from directories [::nold-ai/specfact-cli::]
-
Tests that will exercise or assert template-resolution/runtime template packaging behavior:
- tests/unit/specfact_cli/registry/test_init_module_lifecycle_ux.py:125–133 — tests _resolve_templates_dir behavior when no discoverable prompts exist [::nold-ai/specfact-cli::]
- tests/integration/utils/test_startup_checks_integration.py:118–125 — integration test creating resources/prompts and expecting discovery [::nold-ai/specfact-cli::]
- multiple unit/e2e tests referencing resources/prompts and resources/templates (see rg hits under tests/*) — these tests assume the prior template-resolution/packaging semantics and will be impacted if resolver behavior or packaged template locations change [::nold-ai/specfact-cli::]
-
No occurrences found of the new import-discovery APIs (discover_code_files, import_path_policy, apply_import_runtime_patches) in this repo — runtime import-patching/ignore-policy changes appear not to be directly consumed here. Tests and code in this repo are primarily sensitive to template resolution behavior and presence of packaged templates, not to the import-runtime patches introduced in the modules repo. [::nold-ai/specfact-cli::]
Conclusion: Relevant cross-repo impact is primarily on template-resolution/packaged templates: specfact-cli contains multiple callers and tests that assume templates are discoverable under resources/templates or via installed package resources, so the modules' new template resolver/helpers and bundled templates should be compatible with these consumers. I found no direct consumers of the new import discovery/patching APIs in this repository.
🔇 Additional comments (7)
docs/bundles/project/import-migration.md (3)
21-40: Doc/runtime semantics are aligned and precise.This section now correctly reflects pre-traversal pruning, warning-driven thresholds, and provisional ETA wording without fixed-duration promises.
As per coding guidelines, this matches: “default ignore pruning… before traversal,” “large-artifact conditions should produce warnings… (500 entries)… (1,000 files),” and “remaining-time… provisional/live guidance.”
42-52: Repo-local ignore override behavior is documented correctly.
.specfact/.specfactignoreplacement and “applied before traversal” semantics are clear and consistent with runtime behavior.
As per coding guidelines, this matches: “repo-local overrides must be supported via.specfact/.specfactignore, applied before traversal so they do not inflate scanned-file counts or ETA totals.”
216-216: Best-practice and troubleshooting guidance is consistent with hardened import behavior.The live/provisional ETA guidance, 10,000+ file troubleshooting note, warning callout, and
--entry-pointscope guidance are all in parity with the required user-facing semantics.
As per coding guidelines, this matches: “troubleshooting guidance should reference that ‘10,000+ files’ behavior applies even after default ignore pruning… and use--entry-pointfor scope limiting.”Also applies to: 241-244, 272-272
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md (1)
19-20: Proper handling of past review feedback—nicely done!Lines 19-20 correctly address the two prior review comments:
- Line 19 adds explicit paired
specfact-clicompatibility verification with concrete steps to run/review public artifacts and confirm adapter changes- Line 20 clarifies that signature artifacts are produced by publish automation and should not be manually committed, while emphasizing CI gate validation
Both changes strengthen the adapter boundary integrity between this module and the core CLI.
openspec/changes/architecture-02-module-well-architected/proposal.md (3)
19-28: Adapter contract correctly aligns with ReviewFinding model.The mapping specification (lines 19-28) correctly aligns with the actual
ReviewFindingcontract fields (category,severity,tool,rule,file,line,message,fixable). Settingcategory="architecture"is valid per the VALID_CATEGORIES tuple, and the field semantics match the contract requirements.This properly addresses the past review finding about adapter contract schema alignment.
55-55: Core compatibility specification properly addresses past review.Line 55 adds explicit
core_compatibility: ">=1.0.0 <2.0.0"in the manifest metadata, addressing the past review concern about defining compatibility requirements between the architecture bundle and the paired core change. This ensures runtime compatibility validation.
34-38: Emission semantics claim about failure isolation contradicts existing bundle behavior.The claim that "failures in one analyzer do not block subsequent analyzers" is incorrect. In specfact-code-review's
run_revieworchestrator (runner.py:470–473), there is no exception handling around individual analyzer invocations:for description, runner in _tool_steps(bug_hunt=bug_hunt): if progress_callback is not None: progress_callback(description) findings.extend(runner(files))If any analyzer raises an exception, the loop breaks and subsequent analyzers in the sequence (Ruff, Radon, Semgrep, basedpyright, pylint, contract) do not execute. Either add try/except wrapping to achieve true failure isolation, or revise the proposal to correctly state that analyzer failures block subsequent execution.
> Likely an incorrect or invalid review comment.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 7 file(s) based on 9 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 7 file(s) based on 9 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
openspec/changes/security-02-module-license-compliance/proposal.md (1)
68-68:⚠️ Potential issue | 🟡 MinorAdd trailing newline at EOF (MD047).
The file should end with exactly one newline character.
🔧 Proposed fix
- **Sanitized**: false +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-02-module-license-compliance/proposal.md` at line 68, The file triggers MD047 because it lacks a trailing newline at EOF; update the file "openspec/changes/security-02-module-license-compliance/proposal.md" so that it ends with exactly one newline character (ensure there is a single '\n' as the final character and no extra blank lines).openspec/changes/review-resiliency-01-module/proposal.md (1)
75-75:⚠️ Potential issue | 🟡 MinorAdd a single trailing newline at EOF.
Line 75 still triggers MD047; please end the file with exactly one newline character.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/review-resiliency-01-module/proposal.md` at line 75, The file ends without a proper single trailing newline (violates MD047); open the document and ensure it ends with exactly one newline character (no extra blank lines or missing newline). Remove any extra trailing blank lines after the last content line (the current "**Sanitized**: false" line) and save the file so the final byte is a single '\n' at EOF.openspec/changes/architecture-02-module-well-architected/design.md (1)
65-65:⚠️ Potential issue | 🟡 MinorAdd trailing newline.
The file ends without a trailing newline, violating markdownlint MD047.
📝 Proposed fix
- Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation? +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/design.md` at line 65, The file ends without a trailing newline (violates markdownlint MD047); open the file containing the line "Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation?" and add a single newline character at the end of the file so the file terminates with a trailing newline.
🤖 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/architecture-02-module-well-architected/design.md`:
- Around line 24-28: Implementation PRs must include a complete
module-package.yaml for the new specfact-architecture bundle and explicitly
declare the adapter/manifest boundaries deferred by Decision 1: ensure the
package fields (name, version, commands, tier, publisher, bundle_dependencies,
pip_dependencies, core_compatibility, integrity) follow the same pattern as
specfact-govern/specfact-codebase; set a concrete core_compatibility range
(e.g., ">=0.40.0,<1.0.0") that matches the paired specfact-cli change, confirm
the architecture-02-well-architected-review entry exists and is stable in
specfact-cli, and verify registry integration by passing the existing
sign-modules-on-approval and publish-modules workflows so manifests auto-sign
and tarballs auto-publish before merging.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Line 45: Update the example manifest to use comma-delimited version specifiers
so it matches the shipped manifest convention: change the `core_compatibility`
value in the example `module-package.yaml` from the space-delimited form
(`>=1.0.0 <2.0.0`) to the comma-delimited form (`>=1.0.0,<2.0.0`) ensuring the
string remains quoted.
In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md`:
- Line 30: The file's last line ("- [ ] 4.6 Open the modules PR to `dev`,
cross-link paired privacy/security changes, and note any deferred detector
providers as follow-up issues.") is missing a trailing newline and violates
MD047; fix it by appending a single newline character at EOF so the file ends
with one blank line.
- Line 20: The checklist item in
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md is worded awkwardly
("required bundled rule resources are packaged"); reword that single line to be
clearer and more concise — e.g., "3.5 Validate bundled rule resources: add a
pre-publish validation that confirms all templates/resources in
packages/specfact-pii-gdpr/ are included in the package, fail the release if any
are missing, and include the missing resource path in the error message."
Replace the existing line text with this simplified phrasing so the intent is
unambiguous.
In `@packages/specfact-code-review/src/specfact_code_review/run/findings.py`:
- Around line 59-62: The schema currently types evidence_refs as list[dict[str,
Any]] which allows arbitrary payloads; define a concrete typed model (e.g., a
Pydantic BaseModel named EvidenceRef) that captures the allowed keys (e.g.,
path, start_line, end_line, artifact_id, description) and replace evidence_refs:
list[dict[str, Any]] | None = Field(...) with evidence_refs: list[EvidenceRef] |
None = Field(...); update imports and any serialization/deserialization points
that construct or consume evidence_refs to instantiate/validate EvidenceRef
objects so the adapter boundary enforces a stable contract (refer to the
EvidenceRef model name and evidence_refs field to locate changes).
---
Duplicate comments:
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Line 65: The file ends without a trailing newline (violates markdownlint
MD047); open the file containing the line "Should interface diff support start
with one ecosystem or ship as a provider abstraction from the first
implementation?" and add a single newline character at the end of the file so
the file terminates with a trailing newline.
In `@openspec/changes/review-resiliency-01-module/proposal.md`:
- Line 75: The file ends without a proper single trailing newline (violates
MD047); open the document and ensure it ends with exactly one newline character
(no extra blank lines or missing newline). Remove any extra trailing blank lines
after the last content line (the current "**Sanitized**: false" line) and save
the file so the final byte is a single '\n' at EOF.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Line 68: The file triggers MD047 because it lacks a trailing newline at EOF;
update the file
"openspec/changes/security-02-module-license-compliance/proposal.md" so that it
ends with exactly one newline character (ensure there is a single '\n' as the
final character and no extra blank lines).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2f0a0b0-f733-4c17-9bf8-0c93268cc446
📒 Files selected for processing (5)
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdpackages/specfact-code-review/src/specfact_code_review/run/findings.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
packages/specfact-code-review/src/specfact_code_review/run/findings.py
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: Focus on adapter and bridge patterns: imports from specfact_cli (models, runtime, validators),
Typer/Rich command surfaces, and clear boundaries so core upgrades do not silently break bundles.
Flag breaking assumptions about registry loading, lazy imports, and environment/mode behavior.
Files:
packages/specfact-code-review/src/specfact_code_review/run/findings.py
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
openspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Applied to files:
openspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
openspec/changes/review-resiliency-01-module/proposal.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Treat the clean-code compliance gate as mandatory: the review surface enforces `naming`, `kiss`, `yagni`, `dry`, and `solid` categories and blocks regressions
Applied to files:
openspec/changes/review-resiliency-01-module/proposal.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: Applies to **/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h} : Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Applied to files:
openspec/changes/review-resiliency-01-module/proposal.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.md
📚 Learning: 2026-04-13T10:38:15.855Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.855Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Work belongs on feature/*, bugfix/*, hotfix/*, or chore/* branches, normally in a worktree rooted under ../specfact-cli-modules-worktrees/
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: This repository enforces the clean-code review gate through hatch run specfact code review run --json --out .specfact/code-review.json
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
🪛 LanguageTool
openspec/changes/security-02-module-license-compliance/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re security/license findings model. - NEW: Add policy-aware handling for allowe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...face explicit remediation guidance. - NEW: Define bundle manifest, registry, do...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
[style] ~20-~20: The double modal “required bundled” is nonstandard (only accepted in certain dialects). Consider “to be bundled”.
Context: ...alidation step that checks all required bundled templates/resources shipped with `packa...
(NEEDS_FIXED)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/review-resiliency-01-module/proposal.md
[warning] 75-75: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-02-module-license-compliance/proposal.md
[warning] 68-68: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
🔀 Multi-repo context nold-ai/specfact-cli
Linked repositories findings
nold-ai/specfact-cli
-
Generators expect default templates under resources/templates (potential consumer of new template resolution helpers / packaged templates):
- PlanGenerator default path logic: src/specfact_cli/generators/plan_generator.py:31–38 — falls back to Path(file).parent.parent.parent.parent / "resources" / "templates" when templates_dir is None. [::nold-ai/specfact-cli::]
- ProtocolGenerator default path logic: src/specfact_cli/generators/protocol_generator.py:30–34 — same resources/templates fallback. [::nold-ai/specfact-cli::]
- WorkflowGenerator default path logic: src/specfact_cli/generators/workflow_generator.py:36–40 — same resources/templates fallback. [::nold-ai/specfact-cli::]
- PersonaExporter expects persona templates under resources/templates/persona/default.md.j2: src/specfact_cli/generators/persona_exporter.py:50–90,387. [::nold-ai/specfact-cli::]
-
Init / resource resolution code references packaged and dev template locations:
- _resolve_templates_dir / _resolve_field_mapping_templates_dir use installed package paths and repo-local resources/templates paths: src/specfact_cli/modules/init/src/commands.py:87–108,271–279. Tests rely on this behavior. [::nold-ai/specfact-cli::]
-
Many tests assert presence/resolution of resources/templates and prompt templates (will exercise any change to template resolution):
- tests/unit/specfact_cli/registry/test_init_module_lifecycle_ux.py:125–133 — tests _resolve_templates_dir behavior. [::nold-ai/specfact-cli::]
- tests/unit/generators/test_persona_exporter.py:71–90 — asserts exporter.templates_dir exists and can be overridden. [::nold-ai/specfact-cli::]
- Multiple integration/e2e tests create resources/prompts or resources/templates and assert generators/init behavior (see tests/integration/* and tests/e2e/* references). [::nold-ai/specfact-cli::]
-
No matches found for the new import-discovery APIs or runtime patch names (discover_code_files, import_runtime_patches, ImportDiscoveryOptions, should_ignore_path) in this repo — import runtime hardening appears not to be directly consumed here. Tests and code are instead sensitive to template discovery/resolution semantics and the presence of packaged templates. [::nold-ai/specfact-cli::]
Implication summary:
- The modules changes that introduce packaged runtime templates and new resolver helpers (resolve_project_templates_dir / resolve_templates_dir) and add templates under packages/specfact-project/resources/templates must remain compatible with specfact-cli callers that expect resources/templates locations or allow overriding templates_dir. Tests in specfact-cli will exercise template resolution behavior and could break if packaged template lookup semantics change or if installed package resource locations differ. The import runtime patches / discovery logic added in modules do not appear to have direct consumers in this repository. [::nold-ai/specfact-cli::]
🔇 Additional comments (5)
packages/specfact-code-review/src/specfact_code_review/run/findings.py (1)
133-133: Blocking-findings predicate remains correct and aligned.Line 133 stays consistent with
ReviewFinding.is_blocking()and preserves existing gating behavior.openspec/changes/review-resiliency-01-module/proposal.md (1)
21-43: Adapter contract is now aligned with the canonical findings model.This mapping correctly anchors emissions to
ReviewFindingcore fields and canonical severities (error|warning|info), while keepingevidence_refssupplemental.As per coding guidelines: “
openspec/**/*.md: Specification truth ... drift vs. shipped modules or docs.”openspec/changes/security-03-module-pii-gdpr-eu/tasks.md (1)
25-27: Verification gates correctly protect cross-repo and signing boundaries.Including explicit
specfact-clicompatibility checks andverify-modules-signature --enforce-version-bumphere is the right boundary control before delivery.Based on learnings: “When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics” and “Signed module or manifest changes require version-bump review and verify-modules-signature.”
openspec/changes/architecture-02-module-well-architected/design.md (2)
48-61: Implementation blocker adequately addresses core contract dependency.The Migration Plan meta-note (line 50) and Implementation BLOCKER section (lines 56-61) clearly gate implementation work until the paired
architecture-02-well-architected-reviewcontract exists in specfact-cli. This addresses previous review concerns by:
- Distinguishing design-phase merge from implementation-phase work
- Explicitly naming the required contract and repository
- Providing a concrete checklist for verifying preconditions
The blocker adequately protects against premature module integration that would create cross-repo API mismatches.
However, the broader issue remains: this architecture-02 design file appears misaligned with PR
#243's import-hardening objectives (see separate critical comment). If this file was unintentionally included, the blocker's relevance to this PR is unclear.
1-65: Scope and sequencing are correct; blocker is properly structured.This design is formally sequenced in CHANGE_ORDER.md as
architecture-02-module-well-architected(#230) alongside other module changes. The blocker section (lines 50–61) correctly gates implementation: "Module integration and any references...are disallowed until the pairedspecfact-clichange exists and is marked stable," with three explicit gating conditions that implementation PRs must satisfy.The missing paired core contract (
architecture-02-well-architected-review) is confirmed absent from specfact-cli, but the design document explicitly allows design-artifact merge while blocking implementation. This is the correct sequencing.Adapter boundaries, registry, and rule enforcement are adequately designed.
Decision 2 specifies portable rule resources with three enforcement dimensions: dual-mode prefix matching (preserving
_is_allowed_prefixsemantics), MIGRATE-tier blocking markers, and directional cross-bundle allowlists. Decision 3 separates graph extraction from policy evaluation through analyzer adapters. The spec requires bundle registration with truthful dependencies andcore_compatibilitymetadata, and mandates translation to paired core findings contracts instead of bundle-local schemas.Implementation details for the rule parser/loader/matcher will be fleshed out in implementation PRs, which is appropriate for a design-phase document. The design provides sufficient contract specification for those PRs to proceed once the blocker conditions are met.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 5 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md (1)
30-30:⚠️ Potential issue | 🟡 MinorAppend a single trailing newline at EOF.
MD047is still triggered; please add one newline after Line 30.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md` at line 30, Append a single trailing newline at the end of the file so MD047 stops firing: open the tasks.md file and ensure there is one extra newline character after the final line containing "- [ ] 4.6 Open the modules PR to `dev`, cross-link paired privacy/security changes, and note any deferred detector providers as follow-up issues." (i.e., add one blank line at EOF).openspec/changes/security-02-module-license-compliance/proposal.md (1)
65-68:⚠️ Potential issue | 🟡 MinorAdd a single trailing newline at EOF (MD047).
The file still needs exactly one newline at end-of-file to satisfy markdownlint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/security-02-module-license-compliance/proposal.md` around lines 65 - 68, The file ends without a single trailing newline; open the markdown file containing the metadata block (look for the lines beginning with "**Issue URL**", "**Repository**", "**Last Synced Status**", "**Sanitized**") and add exactly one newline character at the end of the file (ensure there is no extra blank line beyond that) so the file ends with one trailing newline to satisfy MD047.openspec/changes/architecture-02-module-well-architected/design.md (2)
54-61: 🧹 Nitpick | 🔵 TrivialUnchecked blocker checkboxes may create merge ambiguity.
The "Implementation BLOCKER" section (lines 56-61) contains unchecked checkboxes for verifying the paired core contract. While the meta-note at line 50 clarifies this design can merge before implementation blockers are satisfied, the presence of unchecked boxes in a "BLOCKER" section may signal to reviewers that this design is not ready to merge.
Consider either:
- Adding a sentence to the section header clarifying these are implementation blockers verified during implementation PRs, not design-merge blockers, or
- Moving the checklist to a "Prerequisites for Implementation" or "Implementation Gates" section to reduce ambiguity
This would align with the meta-note's intent and make the design/implementation sequencing clearer to reviewers unfamiliar with the prior discussion.
Based on learnings, when a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/design.md` around lines 54 - 61, The "Implementation BLOCKER" checklist referencing the `architecture-02-well-architected-review` contract creates merge ambiguity; update the document by either (A) appending a clarifying sentence to the "Implementation BLOCKER" header stating these are implementation-time checks verified during paired implementation PRs (not design merge blockers), or (B) moving the checkbox list out of the blocker header into a new "Prerequisites for Implementation" or "Implementation Gates" section; ensure the text references the `architecture-02-well-architected-review` artifact and the requirement that the paired `specfact-cli` change be marked stable so reviewers understand sequencing.
65-65:⚠️ Potential issue | 🟡 MinorAdd trailing newline.
Static analysis (markdownlint MD047) requires files to end with a single newline character. This was flagged in a previous review but remains unresolved.
📝 Proposed fix
- Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation? +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/architecture-02-module-well-architected/design.md` at line 65, This file is missing a trailing newline (markdownlint MD047); open the markdown that contains the line "Should interface diff support start with one ecosystem or ship as a provider abstraction from the first implementation?" (design.md content) and ensure the file ends with exactly one newline character by adding a single newline at EOF so the file terminates with a single trailing newline.
🤖 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/architecture-02-module-well-architected/design.md`:
- Around line 24-28: Move the detailed implementation checklist out of the
high-level Decision 1 paragraph and into a dedicated implementation artifact
(e.g., tasks.md or spec.md) or a separate "Implementation Requirements" section:
keep the Decision 1 summary as the strategic requirement to create
packages/specfact-architecture/ with an architecture command and
manifest/lifecycle, and relocate the concrete module-package.yaml field list
(name, version, commands, tier, publisher, bundle_dependencies,
pip_dependencies, core_compatibility, integrity), the example core_compatibility
range, the requirement to ensure architecture-02-well-architected-review exists
in specfact-cli, and the workflow/registry verification steps
(sign-modules-on-approval, publish-modules) into that implementation artifact so
the design.md remains a high-level decision document.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Around line 9-13: The bullets repeat the same "- **NEW**:" prefix three times;
update the proposal so only the first truly new item uses "- **NEW**:" and
rephrase the following two similar bullets to varied lead-ins (e.g., "-
**INCLUDES**: Package SBOM ingestion..." and "- **ENABLES**: Add policy-aware
handling...") while keeping the same referenced symbols
(specfact-license-compliance, license command, SBOM, SPDX normalization
adapters, bundle manifest, registry, docs, signing expectations, compatibility
hooks, and security bundle) and preserving the original meaning and order.
In `@packages/specfact-code-review/src/specfact_code_review/run/findings.py`:
- Around line 34-49: EvidenceRef currently allows structurally invalid objects
(all locators null and inconsistent line ranges); add a cross-field model
validator on EvidenceRef (e.g., a `@model_validator`(mode="after") method named
_validate_invariants) that enforces: at least one locator is present (path or
artifact_id or start_line), if end_line is provided then start_line must be
provided, and if both start_line and end_line are provided then end_line >=
start_line; keep the existing field_validator _validate_non_empty_if_present for
string trimming and raise ValueError with clear messages when invariants fail.
---
Duplicate comments:
In `@openspec/changes/architecture-02-module-well-architected/design.md`:
- Around line 54-61: The "Implementation BLOCKER" checklist referencing the
`architecture-02-well-architected-review` contract creates merge ambiguity;
update the document by either (A) appending a clarifying sentence to the
"Implementation BLOCKER" header stating these are implementation-time checks
verified during paired implementation PRs (not design merge blockers), or (B)
moving the checkbox list out of the blocker header into a new "Prerequisites for
Implementation" or "Implementation Gates" section; ensure the text references
the `architecture-02-well-architected-review` artifact and the requirement that
the paired `specfact-cli` change be marked stable so reviewers understand
sequencing.
- Line 65: This file is missing a trailing newline (markdownlint MD047); open
the markdown that contains the line "Should interface diff support start with
one ecosystem or ship as a provider abstraction from the first implementation?"
(design.md content) and ensure the file ends with exactly one newline character
by adding a single newline at EOF so the file terminates with a single trailing
newline.
In `@openspec/changes/security-02-module-license-compliance/proposal.md`:
- Around line 65-68: The file ends without a single trailing newline; open the
markdown file containing the metadata block (look for the lines beginning with
"**Issue URL**", "**Repository**", "**Last Synced Status**", "**Sanitized**")
and add exactly one newline character at the end of the file (ensure there is no
extra blank line beyond that) so the file ends with one trailing newline to
satisfy MD047.
In `@openspec/changes/security-03-module-pii-gdpr-eu/tasks.md`:
- Line 30: Append a single trailing newline at the end of the file so MD047
stops firing: open the tasks.md file and ensure there is one extra newline
character after the final line containing "- [ ] 4.6 Open the modules PR to
`dev`, cross-link paired privacy/security changes, and note any deferred
detector providers as follow-up issues." (i.e., add one blank line at EOF).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d003493-8f65-43d0-9e1c-096c6c431bd2
📒 Files selected for processing (4)
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.mdpackages/specfact-code-review/src/specfact_code_review/run/findings.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
packages/specfact-code-review/src/specfact_code_review/run/findings.py
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: Focus on adapter and bridge patterns: imports from specfact_cli (models, runtime, validators),
Typer/Rich command surfaces, and clear boundaries so core upgrades do not silently break bundles.
Flag breaking assumptions about registry loading, lazy imports, and environment/mode behavior.
Files:
packages/specfact-code-review/src/specfact_code_review/run/findings.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-02-module-license-compliance/proposal.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.md
📚 Learning: 2026-04-13T10:38:15.855Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.855Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:29.399Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.399Z
Learning: Treat clean-code regressions (naming, kiss, yagni, dry, solid violations) as blocking until they are fixed or explicitly justified
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Require public GitHub metadata completeness before implementation when linked issue workflow applies: parent, labels, project assignment, blockers, and blocked-by relationships
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Read docs/agent-rules/05-non-negotiable-checklist.md before implementation
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.md
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Applied to files:
openspec/changes/architecture-02-module-well-architected/design.mdopenspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:43.535Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.535Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: Work belongs on feature/*, bugfix/*, hotfix/*, or chore/* branches, normally in a worktree rooted under ../specfact-cli-modules-worktrees/
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
📚 Learning: 2026-04-13T10:38:22.848Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.848Z
Learning: This repository enforces the clean-code review gate through hatch run specfact code review run --json --out .specfact/code-review.json
Applied to files:
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
🪛 LanguageTool
openspec/changes/security-02-module-license-compliance/proposal.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re security/license findings model. - NEW: Add policy-aware handling for allowe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...face explicit remediation guidance. - NEW: Define bundle manifest, registry, do...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/security-02-module-license-compliance/proposal.md
[warning] 65-65: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md
[warning] 30-30: Files should end with a single newline character
(MD047, single-trailing-newline)
🔀 Multi-repo context nold-ai/specfact-cli
Linked repositories findings
nold-ai/specfact-cli
-
Template/resource resolution and generators expect packaged or repo-local templates:
- _resolve_field_mapping_templates_dir — locates packaged or dev templates under resources/templates/backlog/field_mappings and falls back to importlib/resources or package spec lookup. [::nold-ai/specfact-cli::src/specfact_cli/modules/init/src/commands.py:36-110]
-
Tests rely on template discovery behavior:
- test_resolve_templates_dir_none_when_no_discoverable_prompts — expects _resolve_templates_dir to return None when no discoverable prompts/templates exist. [::nold-ai/specfact-cli::tests/unit/specfact_cli/registry/test_init_module_lifecycle_ux.py:125-141]
Notes:
- I did not find any usages of the new import-discovery APIs (discover_code_files, import_runtime_patches, ImportDiscoveryOptions, should_ignore_path, resolve_templates_dir aliases) in this repo; primary surface of potential impact here is template/resource resolution behavior and tests that assert resolution semantics. [::nold-ai/specfact-cli::]
🔇 Additional comments (5)
openspec/changes/security-03-module-pii-gdpr-eu/tasks.md (1)
21-21: Good clarification on signature handling.This wording correctly prevents manual signature commits and keeps pre-merge focus on enforcement gates.
Based on learnings, "module tarball signatures ... are generated by thepublish-modulesGitHub Actions runner during the publish workflow, not committed locally to the branch."openspec/changes/security-02-module-license-compliance/proposal.md (1)
15-54: Adapter boundary and compatibility direction look solid now.Good improvement: the contract is explicit (schema + SPDX mapping), and inter-bundle guidance cleanly separates
core_compatibilityfrom bundle dependency semantics.packages/specfact-code-review/src/specfact_code_review/run/findings.py (3)
6-6: No actionable issue in this import change.No functional or boundary-impacting problem found in Line 6.
150-150: No actionable issue in this return-line change.Behavior remains correct and aligned with report blocking semantics.
76-79: Good boundary upgrade with typedevidence_refs.This change materially improves contract stability versus raw dict payloads while keeping existing producers backward compatible.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
Author
|
@codex fix the module signature mismatch and verify quality gates are green and update the source branch directly |
Summary
Testing
|
Contributor
Author
|
@cursor please fix the signature mismatch and verify 2026-04-21T08:57:03.4014114Z FAIL packages/specfact-code-review/module-package.yaml: checksum mismatch |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
Contributor
Author
|
@codex please fix checksum mismatch and verify with unsigned modules: 2026-04-21T08:57:03.4014114Z FAIL packages/specfact-code-review/module-package.yaml: checksum mismatch |
|
Summary
Testing
Also completed: created the PR entry via |
…ain-24724901420 chore(registry): publish changed modules
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit 4b6e757. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR promotes the current
devdelta tomainafter PR #236 merged and the follow-up signing/publish automation completed.What changed
mainspecfact-projectdevWhy
#236merged todev, and this opens the corresponding promotion path tomainValidation
devorigin/devbefore opening this PRNote
Medium Risk
Touches CI workflows that can push commits back to PR branches and introduces runtime monkey-patching for
code importtraversal/ETA behavior, so failures could block PRs or change import performance/outputs. Changes are guarded for same-repo/non-bot actors and largely additive but still affect release/signing integrity paths.Overview
CI module signing now auto-remediates same-repo PRs.
sign-modules.ymlchecks out the PR head SHA, fetches the PR base, and (for same-repo, non-bot PRs) re-signs only changedpackages/*/module-package.yamlmanifests, commitschore(modules): ci sign changed modules, and pushes back to the PR branch;sign-modules-on-approval.ymlis tightened to approval-only gating with updated concurrency/permissions and safer env fallbacks.Project import/runtime hardening is promoted and packaged.
specfact-projectadds packaged Jinja2 templates and a shared resolver, bumps bundle versions/integrity, and applies import-runtime patches at command entrypoints to enforce a pruned traversal/ETA policy (with warnings) without directly rewriting the legacy import implementation.specfact-code-reviewextends findings with structuredevidence_refssupport.Reviewed by Cursor Bugbot for commit 61dc5ec. Bugbot is set up for automated code reviews on this repo. Configure here.