Harden module command reliability gates#307
Conversation
|
Strix is installed on this repository, but we could not run this PR security review because this workspace does not have an active plan. If you'd like to continue receiving code reviews, you can add a payment method or manage billing here. |
|
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:
📝 WalkthroughI can’t reliably rebuild the full hidden review stack artifact with every provided rangeId exactly once in this interface — the <all_range_ids> list is large and must be copied verbatim into the hidden block without any additions, removals, duplicates, or renames. Please ask me to generate the artifact in one of these two ways:
Which option do you prefer? ✨ Finishing Touches🧪 Generate unit tests (beta)
|
djm81
added
bug
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d639a88c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docs-review.yml (1)
97-107:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix failure propagation in “Validate prompt commands” docs gate.
- In
.github/workflows/docs-review.yml(lines ~97-102),python scripts/check-prompt-commands.py 2>&1 | tee ...has nopipefail/PIPESTATUS check afterward, so a failing validator can still result in a successful step (pipeline exit comes fromtee).- The “Validate generated command overview” step (lines ~103-107) doesn’t have a pipeline, so the
PIPESTATUS-based masking claim doesn’t apply; failure should already stop due to defaultbash -e.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-review.yml around lines 97 - 107, The "Validate prompt commands" step's pipeline (python scripts/check-prompt-commands.py 2>&1 | tee "$PROMPT_COMMAND_LOG") can mask failures because tee exits successfully; enable failure propagation by running the step under pipefail (e.g., prefix with set -o pipefail or use bash -o pipefail) or explicitly check PIPESTATUS after the pipeline and exit on non-zero (inspect PIPESTATUS[0] from the python command). Also remove or don't apply a PIPESTATUS-style workaround to the "Validate generated command overview" step since it isn't a pipeline—leave it to fail normally under bash -e or ensure no redundant PIPESTATUS handling is present.
🟡 Minor comments (5)
openspec/CHANGE_ORDER.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the source bug range reference (
#586–#592).Line 44 omits
specfact-cli#589while claiming the source bug set spanning#586–#592. Please add#589to keep change-order linkage complete.As per coding guidelines, OpenSpec artifacts must stay truth-aligned with declared proposal/task dependencies and change-order tracking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/CHANGE_ORDER.md` at line 44, The CHANGE_ORDER.md table row for "backlog-core | 00 | tester-module-cli-reliability" currently lists source bugs [specfact-cli#586], [`#587`], [`#588`], [`#590`], [`#591`], [`#592`] but omits `#589`; update that row's source bug list to include [specfact-cli#589](https://github.com/nold-ai/specfact-cli/issues/589) (keeping the same comma-separated/hyperlink format) so the referenced range correctly reads `#586`–#592.packages/specfact-govern/resources/prompts/shared/cli-enforcement.md-114-119 (1)
114-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix mismatched command descriptions vs actual invocation.
Line 114, Line 115, Line 117, and Line 118 describe concrete actions but all point to
specfact project --help, which does not perform those actions. This is misleading prompt guidance and can drive invalid automation behavior.As per coding guidelines, user-facing command examples/guidance must remain accurate and aligned with bundled command behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/specfact-govern/resources/prompts/shared/cli-enforcement.md` around lines 114 - 119, The prompt lists several CLI examples that incorrectly reference `specfact project --help`; update each misleading entry so the command shown actually performs the described action: replace the `specfact project --help` placeholder for "Initialize project bundle" with the actual initialization command (e.g., `specfact project init` or the project bootstrap command used in the codebase), replace the "Set active plan" example with the real command that sets plans (e.g., `specfact project set-plan <plan-name>`), replace the "Review plan" and "Create SDD manifest" examples with their respective concrete commands (e.g., `specfact project review [<bundle-name>]` and `specfact project create-sdd [<bundle-name>]`), and ensure `specfact code import [<bundle-name>] --repo <path>` and `specfact govern enforce sdd [<bundle-name>]` remain accurate; use the actual CLI verbs used by the project (referencing the command names `specfact project init`, `specfact project set-plan`, `specfact project review`, `specfact project create-sdd`) so examples match real behavior.docs/getting-started/choose-your-modules.md-23-23 (1)
23-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate
specfact projectentry from the Project command surface.Line 23 lists
specfact projecttwice, which makes the command surface inaccurate.Suggested fix
-| Structure my project, link plans to code, and manage dev lifecycle | **Project** | `specfact project`, `specfact project`, `specfact project sync` | +| Structure my project, link plans to code, and manage dev lifecycle | **Project** | `specfact project`, `specfact project sync` |As per coding guidelines,
docs/**/*.md: “User-facing and cross-site accuracy… CLI examples matching bundled commands.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/getting-started/choose-your-modules.md` at line 23, The docs line contains a duplicated CLI entry: remove the duplicate `specfact project` so the command surface reads only once; update the table cell that currently lists "`specfact project`, `specfact project`, `specfact project sync`" to list "`specfact project`, `specfact project sync`" (ensuring spacing/commas match adjacent rows) so the Project command surface accurately reflects available commands.scripts/check-prompt-commands.py-377-377 (1)
377-377:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDefault-path help text does not match implementation
Line 377 claims defaults include
packages/*/src/**/*.py, but Line 384 only returns resource paths. Either update the help text or include source path discovery in the no-arg path selection to prevent silent coverage gaps.Also applies to: 384-384
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-prompt-commands.py` at line 377, The help text for the paths option claims defaults include "packages/*/src/**/*.py" but the no-argument path-selection code only returns package resource paths, causing a mismatch; either update the help string to accurately state the real defaults or modify the default-path resolution to also discover source files (e.g., glob("packages/*/src/**/*.py")) and append them to the returned resource paths. Locate the option definition with the mismatched help string and the function/code that computes default resource paths and implement one of the two fixes: correct the help text, or extend the default discovery to include source files before returning.tests/unit/specfact_backlog/test_auth_commands.py-74-74 (1)
74-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the exact contract exit code for missing subcommand.
Line 74 should assert
exit_code == 2instead of!= 0; the current check can pass unrelated failure modes and weaken the CLI contract gate.As per coding guidelines, "tests/**/*.py: Contract-first and integration tests...".Proposed test tightening
- assert result.exit_code != 0 + assert result.exit_code == 2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/specfact_backlog/test_auth_commands.py` at line 74, The test currently asserts a non-zero exit code for the missing-subcommand case; tighten it to assert the exact CLI contract by replacing the loose check on result.exit_code with an exact assertion: assert result.exit_code == 2 (use the existing result variable and its exit_code property) so the test for the missing subcommand failure is deterministic and only passes the intended contract condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-review.yml:
- Around line 61-66: The prompt-validation command pipeline currently swallows
the Python exit code (the line invoking "python scripts/check-prompt-commands.py
... | tee ...") so update that step to append and run exit
"${PIPESTATUS[0]:-$?}" after the pipeline to propagate failures; additionally,
harden the secondary checkout step named "Checkout paired core command sources"
(the actions/checkout@v4 invocation that sets repository: nold-ai/specfact-cli
and path: specfact-cli) by adding persist-credentials: false to avoid persisting
tokens in git config for later steps.
In @.github/workflows/pr-orchestrator.yml:
- Around line 128-134: The checkout step for the paired core CLI (uses:
actions/checkout@v4, repository: nold-ai/specfact-cli, path: specfact-cli, ref:
dev) should be hardened: set persist-credentials: false to avoid leaking runner
token, replace the moving ref: dev with a pinned ref/commit input (e.g., a
workflow input or needs.* output such as paired_core_commit or paired_core_ref)
to ensure reproducible runs, and ensure the action is pinned to an explicit
stable ref (e.g., a specific tag or SHA) if policy requires; update the checkout
step to use those inputs and persist-credentials: false.
In `@docs/bundles/project/overview.md`:
- Line 23: Replace any examples that use “--help” as part of a command path
(e.g., "specfact project --help artifacts --repo .") with canonical executable
forms (e.g., "specfact project artifacts --repo .") and, where you want to show
help output, add separate examples that call the help flag alone (e.g.,
"specfact project artifacts --help" or "specfact project --help"). Search for
occurrences of the problematic patterns involving the "project" group and its
nested "sync" Typer (references to "specfact project --help", "specfact project
sync --help", or inline "--help" followed by subcommands) and update them to use
the real subcommand invocation (like "specfact project sync ...") and distinct
help examples, ensuring CLI examples in this file match actual bundled commands.
In `@docs/reference/commands.generated.json`:
- Around line 89-106: The generated record for the command "specfact backlog
auth" has an incorrect bare_invocation value ("executes"); update the
generator/output so that the entry for the command "specfact backlog auth" sets
"bare_invocation" to "requires-subcommand" (reflecting that the command group
errors without a subcommand), and adjust the generation logic that emits
bare_invocation so it inspects the command/group runtime behavior (the CLI
command node for specfact backlog auth) to detect whether it requires
subcommands rather than assuming "executes".
In `@openspec/changes/tester-module-cli-reliability/proposal.md`:
- Line 3: The reported issue: the enumerated bug list range " `#586` through `#592`
" in the proposal header is inconsistent with the detailed list because report
"`#589`" is omitted; update the proposal so the header range and the enumerated
list match (either remove `#589` from the range or add `#589` into the enumerated
list) and ensure the document includes the required "Specification truth ...
proposal/tasks/spec deltas vs. bundle behavior." statement for traceability and
compliance with openspec markdown requirements.
In
`@packages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.py`:
- Around line 66-67: The YAML loader can raise on malformed config and bypass
the intended CLI diagnostics; wrap the yaml.safe_load call that assigns `loaded`
(and the call to `config_path.read_text`) in a try/except that catches
yaml.YAMLError (and other parsing/IO errors), and on error set `loaded = {}` (or
fallback to an empty dict) so the existing `if not isinstance(loaded, dict):`
path still runs and shows the user-facing guidance; keep any existing logging or
debug output but do not re-raise so the CLI contract is preserved.
In `@packages/specfact-govern/resources/prompts/specfact.05-enforce.md`:
- Around line 118-119: The guidance currently points to the non-mutating command
`specfact project --help`; replace that help pointer with the actionable
mutating command that actually regenerates/updates manifests (e.g., `specfact
project sync [<bundle-name>]` or your canonical "regenerate manifests" command)
so remediation will fix hash mismatches; update both occurrences (the `specfact
project --help` strings near the `specfact govern enforce sdd` guidance at the
two locations) and ensure the text uses the canonical supported command form per
the module CLI contract docs.
In `@packages/specfact-project/resources/prompts/shared/cli-enforcement.md`:
- Around line 114-118: The doc lists `specfact project --help` as placeholders
for multiple actions which must be replaced with concrete canonical CLI
commands; update the entries so each line uses the real subcommands/flags
instead of `--help` (e.g., replace the init/select/review/create placeholder
lines with the actual commands such as the project init command, the command to
set active plan (e.g., `specfact project set-plan --plan <name>` or the
project's canonical equivalent), the plan review command, and the SDD manifest
create command), ensure consistency with the existing `specfact code import
[<bundle-name>] --repo <path>` example (use the same bundle/plan flag names like
`--bundle`/`--plan`), and make sure each example exactly matches the implemented
CLI subcommand names used by Project/Code import functions so validation passes.
In `@packages/specfact-project/resources/prompts/specfact.02-plan.md`:
- Around line 63-68: Replace the repeated help-only placeholder "specfact
project --help" used in the plan creation/update prompt with real, executable
command templates (e.g., concrete examples for creating/updating plan artifacts)
so the prompt flow is runnable; locate every occurrence of the string "specfact
project --help" in this prompt file and replace it with specific commands that
include required flags/arguments for creating/updating plans (for example a
create command and an update command), ensure the new examples match the
generated module command overview and validate them against the module docs spec
("openspec/changes/tester-module-cli-reliability/.../spec.md") so they are
syntactically correct and executable.
In `@packages/specfact-project/resources/prompts/specfact.03-review.md`:
- Around line 36-40: Replace the placeholder uses of "specfact project --help"
with the concrete, executable SpecFact CLI commands for review/export/import
flows (e.g., the real subcommands used for project review, export, and import)
so examples are accurate and runnable; ensure each restored command matches the
bundled CLI contract, provide nearest non-interactive alternatives when an
option would be interactive, and avoid any direct writes to .specfact/ by
routing artifact updates through the SpecFact CLI commands or CLI-consumed
enrichment/answers files referenced in the prompt; update every affected section
where "specfact project --help" was inserted to the corresponding real command
names and ensure examples are syntactically correct and non-interactive.
In `@packages/specfact-project/resources/prompts/specfact.04-sdd.md`:
- Around line 52-53: Replace the no-op "specfact project --help" examples in
specfact.04-sdd.md (examples referenced around lines 52, 81, 114-116, and 150,
including Phase 3) with the actual SDD create/update CLI invocations used by the
project (e.g., the canonical "specfact sdd create <bundle|--active-plan>
<options>" and "specfact sdd update <sdd-id> <options>" forms), ensuring each
example shows real flags/args and aligns with the canonical forms required by
openspec/changes/tester-module-cli-reliability/specs/modules-docs-command-validation/spec.md
and module-command-overview/spec.md; remove the duplicated help example in Phase
3 and replace it with the appropriate create/update example or an idempotent
status command used by the SDD workflow.
In `@packages/specfact-project/resources/prompts/specfact.compare.md`:
- Around line 54-56: Replace each placeholder invocation of "specfact project
--help" in specfact.compare.md with the actual runnable compare workflow CLI
examples used by the bundled tool: use the "project compare" subcommand and
include the real flags and arguments the bundle expects (e.g., the --bundle,
--from, --to and --output style flags) so the examples are executable and
validate against the bundled command signatures; update the recovery guidance
example the same way and ensure all examples match the bundled CLI's current
parameter names and ordering.
In `@packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py`:
- Around line 402-403: The calls to check_tool_in_env(self.repo_path, "semgrep",
env_info) are discarding the second tuple (currently _message), which contains
the environment-specific diagnostic; update both call sites to capture the
second return value (e.g., available, message = check_tool_in_env(...)) and
forward that message into get_plugin_status()/the user-facing status logic
instead of using the generic hardcoded text so the EnvManager probe context and
guidance are preserved (symbols to change: check_tool_in_env, get_plugin_status,
variables available/message, and the semgrep probe sites in code_analyzer.py).
In `@packages/specfact-project/src/specfact_project/project/commands.py`:
- Around line 318-324: The snapshot/export command paths still call
_fetch_backlog_graph and use graph.to_json() without guarding for None; update
the snapshot and graph-export handlers to call _require_backlog_graph(graph,
command_name="snapshot" or "graph-export") immediately after obtaining graph
from _fetch_backlog_graph and use the returned value (the non-None typed graph)
for graph.to_json() (and any other graph usage), and similarly apply the same
guard where regenerate already uses _require_backlog_graph so no code path can
dereference a None graph.
In `@packages/specfact-spec/resources/prompts/shared/cli-enforcement.md`:
- Around line 114-123: Update the stale legacy reference to the active-plan
command: replace the bare `plan select` text with the canonical bundled CLI form
`specfact plan select` (or the correct namespaced command used elsewhere, e.g.,
`specfact project plan select` if that is the project-scoped variant) so the
guidance matches the other examples like `specfact project --help`; ensure the
note and any other occurrences in this file use the same fully qualified CLI
command syntax.
In `@scripts/check-docs-commands.py`:
- Around line 169-179: The current logic silently ignores JSON decode errors and
malformed shapes when reading GENERATED_COMMANDS_PATH, which can hide root
causes; update the code around GENERATED_COMMANDS_PATH, raw and generated_paths
so it fails fast: wrap json.loads(...) in a try/except to catch
json.JSONDecodeError and re-raise a clear ValueError (or log+raise) with the
path and decode error, assert that raw is a list and raise if not, and when
iterating entries require each entry to be a dict and that entry.get("command")
is a str—if an entry is malformed raise a ValueError describing the offending
entry (rather than continue); keep using CORE_COMMAND_PREFIXES and
GENERATED_COMMANDS_PATH but return generated_paths only after validation
succeeds.
---
Outside diff comments:
In @.github/workflows/docs-review.yml:
- Around line 97-107: The "Validate prompt commands" step's pipeline (python
scripts/check-prompt-commands.py 2>&1 | tee "$PROMPT_COMMAND_LOG") can mask
failures because tee exits successfully; enable failure propagation by running
the step under pipefail (e.g., prefix with set -o pipefail or use bash -o
pipefail) or explicitly check PIPESTATUS after the pipeline and exit on non-zero
(inspect PIPESTATUS[0] from the python command). Also remove or don't apply a
PIPESTATUS-style workaround to the "Validate generated command overview" step
since it isn't a pipeline—leave it to fail normally under bash -e or ensure no
redundant PIPESTATUS handling is present.
---
Minor comments:
In `@docs/getting-started/choose-your-modules.md`:
- Line 23: The docs line contains a duplicated CLI entry: remove the duplicate
`specfact project` so the command surface reads only once; update the table cell
that currently lists "`specfact project`, `specfact project`, `specfact project
sync`" to list "`specfact project`, `specfact project sync`" (ensuring
spacing/commas match adjacent rows) so the Project command surface accurately
reflects available commands.
In `@openspec/CHANGE_ORDER.md`:
- Line 44: The CHANGE_ORDER.md table row for "backlog-core | 00 |
tester-module-cli-reliability" currently lists source bugs [specfact-cli#586],
[`#587`], [`#588`], [`#590`], [`#591`], [`#592`] but omits `#589`; update that row's source
bug list to include
[specfact-cli#589](https://github.com/nold-ai/specfact-cli/issues/589) (keeping
the same comma-separated/hyperlink format) so the referenced range correctly
reads `#586`–#592.
In `@packages/specfact-govern/resources/prompts/shared/cli-enforcement.md`:
- Around line 114-119: The prompt lists several CLI examples that incorrectly
reference `specfact project --help`; update each misleading entry so the command
shown actually performs the described action: replace the `specfact project
--help` placeholder for "Initialize project bundle" with the actual
initialization command (e.g., `specfact project init` or the project bootstrap
command used in the codebase), replace the "Set active plan" example with the
real command that sets plans (e.g., `specfact project set-plan <plan-name>`),
replace the "Review plan" and "Create SDD manifest" examples with their
respective concrete commands (e.g., `specfact project review [<bundle-name>]`
and `specfact project create-sdd [<bundle-name>]`), and ensure `specfact code
import [<bundle-name>] --repo <path>` and `specfact govern enforce sdd
[<bundle-name>]` remain accurate; use the actual CLI verbs used by the project
(referencing the command names `specfact project init`, `specfact project
set-plan`, `specfact project review`, `specfact project create-sdd`) so examples
match real behavior.
In `@scripts/check-prompt-commands.py`:
- Line 377: The help text for the paths option claims defaults include
"packages/*/src/**/*.py" but the no-argument path-selection code only returns
package resource paths, causing a mismatch; either update the help string to
accurately state the real defaults or modify the default-path resolution to also
discover source files (e.g., glob("packages/*/src/**/*.py")) and append them to
the returned resource paths. Locate the option definition with the mismatched
help string and the function/code that computes default resource paths and
implement one of the two fixes: correct the help text, or extend the default
discovery to include source files before returning.
In `@tests/unit/specfact_backlog/test_auth_commands.py`:
- Line 74: The test currently asserts a non-zero exit code for the
missing-subcommand case; tighten it to assert the exact CLI contract by
replacing the loose check on result.exit_code with an exact assertion: assert
result.exit_code == 2 (use the existing result variable and its exit_code
property) so the test for the missing subcommand failure is deterministic and
only passes the intended contract condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff41ea13-11da-4039-b0d5-635dedb0a195
📒 Files selected for processing (91)
.github/pull_request_template.md.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.ymlREADME.mddocs/adapters/azuredevops.mddocs/adapters/github.mddocs/bundles/backlog/refinement.mddocs/bundles/project/import-migration.mddocs/bundles/project/overview.mddocs/bundles/spec/validate.mddocs/getting-started/choose-your-modules.mddocs/getting-started/installation.mddocs/guides/ai-ide-workflow.mddocs/guides/brownfield-examples.mddocs/guides/brownfield-faq-and-roi.mddocs/guides/brownfield-modernization.mddocs/guides/command-chains.mddocs/guides/cross-module-chains.mddocs/guides/daily-devops-routine.mddocs/guides/integrations-overview.mddocs/guides/openspec-journey.mddocs/guides/speckit-comparison.mddocs/guides/speckit-journey.mddocs/integrations/devops-adapter-overview.mddocs/reference/README.mddocs/reference/command-syntax-policy.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mddocs/reference/commands.mddocs/team-and-enterprise/multi-repo.mdllms.txtopenspec/CHANGE_ORDER.mdopenspec/changes/tester-module-cli-reliability/.openspec.yamlopenspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.mdopenspec/changes/tester-module-cli-reliability/proposal.mdopenspec/changes/tester-module-cli-reliability/specs/backlog-delta/spec.mdopenspec/changes/tester-module-cli-reliability/specs/code-review-tool-dependencies/spec.mdopenspec/changes/tester-module-cli-reliability/specs/module-cli-error-contract/spec.mdopenspec/changes/tester-module-cli-reliability/specs/module-command-overview/spec.mdopenspec/changes/tester-module-cli-reliability/specs/modules-docs-command-validation/spec.mdopenspec/changes/tester-module-cli-reliability/tasks.mdpackages/specfact-backlog/module-package.yamlpackages/specfact-backlog/resources/prompts/shared/cli-enforcement.mdpackages/specfact-backlog/resources/prompts/specfact.backlog-refine.mdpackages/specfact-backlog/resources/prompts/specfact.sync-backlog.mdpackages/specfact-backlog/src/specfact_backlog/backlog/auth_commands.pypackages/specfact-backlog/src/specfact_backlog/backlog/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.pypackages/specfact-codebase/module-package.yamlpackages/specfact-codebase/resources/prompts/shared/cli-enforcement.mdpackages/specfact-codebase/src/specfact_codebase/import_cmd/commands.pypackages/specfact-codebase/src/specfact_codebase/repro/commands.pypackages/specfact-codebase/src/specfact_codebase/validate/commands.pypackages/specfact-govern/module-package.yamlpackages/specfact-govern/resources/prompts/shared/cli-enforcement.mdpackages/specfact-govern/resources/prompts/specfact.05-enforce.mdpackages/specfact-govern/src/specfact_govern/enforce/commands.pypackages/specfact-project/module-package.yamlpackages/specfact-project/resources/prompts/shared/cli-enforcement.mdpackages/specfact-project/resources/prompts/specfact.02-plan.mdpackages/specfact-project/resources/prompts/specfact.03-review.mdpackages/specfact-project/resources/prompts/specfact.04-sdd.mdpackages/specfact-project/resources/prompts/specfact.06-sync.mdpackages/specfact-project/resources/prompts/specfact.compare.mdpackages/specfact-project/resources/templates/github-action.yml.j2packages/specfact-project/src/specfact_project/analyzers/code_analyzer.pypackages/specfact-project/src/specfact_project/import_cmd/commands.pypackages/specfact-project/src/specfact_project/plan/commands.pypackages/specfact-project/src/specfact_project/project/commands.pypackages/specfact-project/src/specfact_project/sync/commands.pypackages/specfact-project/src/specfact_project/sync_runtime/sync_perform_operation_impl.pypackages/specfact-spec/module-package.yamlpackages/specfact-spec/resources/prompts/shared/cli-enforcement.mdpackages/specfact-spec/src/specfact_spec/generate/commands.pypackages/specfact-spec/src/specfact_spec/sdd/commands.pypyproject.tomlscripts/check-command-contract.pyscripts/check-docs-commands.pyscripts/check-prompt-commands.pyscripts/generate-command-overview.pyscripts/pre-commit-quality-checks.shscripts/pre_commit_code_review.pytests/e2e/specfact_project/test_help_smoke.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_backlog/test_auth_commands.pytests/unit/specfact_backlog/test_delta_command_contract.pytests/unit/specfact_codebase/test_import_command_contract.pytests/unit/specfact_project/test_regenerate_command_contract.pytests/unit/test_check_prompt_commands_script.pytests/unit/test_global_cli_error_contract.pytests/unit/workflows/test_pr_orchestrator_signing.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
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/spec/validate.mddocs/reference/README.mddocs/guides/brownfield-faq-and-roi.mddocs/guides/ai-ide-workflow.mddocs/team-and-enterprise/multi-repo.mddocs/guides/daily-devops-routine.mddocs/guides/integrations-overview.mddocs/guides/brownfield-examples.mddocs/guides/brownfield-modernization.mddocs/reference/command-syntax-policy.mddocs/guides/speckit-journey.mddocs/getting-started/choose-your-modules.mddocs/adapters/github.mddocs/reference/commands.mddocs/guides/cross-module-chains.mddocs/getting-started/installation.mddocs/bundles/project/overview.mddocs/reference/commands.generated.mddocs/adapters/azuredevops.mddocs/guides/command-chains.mddocs/bundles/backlog/refinement.mddocs/guides/speckit-comparison.mddocs/bundles/project/import-migration.mddocs/guides/openspec-journey.mddocs/integrations/devops-adapter-overview.md
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-backlog/module-package.yamlpackages/specfact-spec/module-package.yamlpackages/specfact-govern/module-package.yamlpackages/specfact-codebase/module-package.yamlpackages/specfact-project/module-package.yaml
**/*.{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/specfact_codebase/test_import_command_contract.pyscripts/pre_commit_code_review.pypackages/specfact-project/src/specfact_project/sync_runtime/sync_perform_operation_impl.pypackages/specfact-govern/src/specfact_govern/enforce/commands.pytests/unit/specfact_project/test_regenerate_command_contract.pypackages/specfact-codebase/src/specfact_codebase/repro/commands.pypackages/specfact-spec/src/specfact_spec/sdd/commands.pytests/unit/specfact_backlog/test_delta_command_contract.pytests/unit/test_global_cli_error_contract.pypackages/specfact-project/src/specfact_project/sync/commands.pypackages/specfact-codebase/src/specfact_codebase/validate/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog/auth_commands.pytests/unit/workflows/test_pr_orchestrator_signing.pytests/unit/specfact_backlog/test_auth_commands.pytests/e2e/specfact_project/test_help_smoke.pypackages/specfact-spec/src/specfact_spec/generate/commands.pytests/unit/scripts/test_pre_commit_code_review.pypackages/specfact-project/src/specfact_project/project/commands.pytests/unit/test_check_prompt_commands_script.pypackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.pypackages/specfact-project/src/specfact_project/import_cmd/commands.pyscripts/check-docs-commands.pypackages/specfact-project/src/specfact_project/plan/commands.pyscripts/check-command-contract.pypackages/specfact-codebase/src/specfact_codebase/import_cmd/commands.pyscripts/generate-command-overview.pypackages/specfact-project/src/specfact_project/analyzers/code_analyzer.pypackages/specfact-backlog/src/specfact_backlog/backlog/commands.pyscripts/check-prompt-commands.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/specfact_codebase/test_import_command_contract.pytests/unit/specfact_project/test_regenerate_command_contract.pytests/unit/specfact_backlog/test_delta_command_contract.pytests/unit/test_global_cli_error_contract.pytests/unit/workflows/test_pr_orchestrator_signing.pytests/unit/specfact_backlog/test_auth_commands.pytests/e2e/specfact_project/test_help_smoke.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/test_check_prompt_commands_script.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/tester-module-cli-reliability/specs/backlog-delta/spec.mdopenspec/changes/tester-module-cli-reliability/specs/module-cli-error-contract/spec.mdopenspec/changes/tester-module-cli-reliability/specs/module-command-overview/spec.mdopenspec/changes/tester-module-cli-reliability/specs/code-review-tool-dependencies/spec.mdopenspec/changes/tester-module-cli-reliability/specs/modules-docs-command-validation/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/tester-module-cli-reliability/proposal.mdopenspec/changes/tester-module-cli-reliability/tasks.mdopenspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.md
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/pre_commit_code_review.pyscripts/check-docs-commands.pyscripts/check-command-contract.pyscripts/generate-command-overview.pyscripts/check-prompt-commands.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-project/src/specfact_project/sync_runtime/sync_perform_operation_impl.pypackages/specfact-govern/src/specfact_govern/enforce/commands.pypackages/specfact-codebase/src/specfact_codebase/repro/commands.pypackages/specfact-spec/src/specfact_spec/sdd/commands.pypackages/specfact-project/src/specfact_project/sync/commands.pypackages/specfact-codebase/src/specfact_codebase/validate/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog/auth_commands.pypackages/specfact-spec/src/specfact_spec/generate/commands.pypackages/specfact-project/src/specfact_project/project/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.pypackages/specfact-project/src/specfact_project/import_cmd/commands.pypackages/specfact-project/src/specfact_project/plan/commands.pypackages/specfact-codebase/src/specfact_codebase/import_cmd/commands.pypackages/specfact-project/src/specfact_project/analyzers/code_analyzer.pypackages/specfact-backlog/src/specfact_backlog/backlog/commands.py
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: CI: secrets, hatch/verify-modules-signature gates, contract-test alignment, action versions.
Files:
.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.yml
🪛 LanguageTool
llms.txt
[uncategorized] ~22-~22: The official name of this software platform is spelled with a capital “H”.
Context: ...og| -; args: - | azure-devops, clear, github, status | | |specfact backlog auth a...
(GITHUB)
[uncategorized] ~25-~25: The official name of this software platform is spelled with a capital “H”.
Context: ...cklog| --provider; args: - | - | | |specfact backlog auth github| nold-ai/specfact-backlog |specfact...
(GITHUB)
docs/reference/commands.generated.md
[uncategorized] ~18-~18: The official name of this software platform is spelled with a capital “H”.
Context: ...og| -; args: - | azure-devops, clear, github, status | | |specfact backlog auth a...
(GITHUB)
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...cklog| --provider; args: - | - | | |specfact backlog auth github| nold-ai/specfact-backlog |specfact...
(GITHUB)
🪛 Shellcheck (0.11.0)
scripts/pre-commit-quality-checks.sh
[warning] 81-81: This pattern always overrides a later one on line 81.
(SC2221)
[warning] 81-81: This pattern never matches because of a previous pattern on line 81.
(SC2222)
🪛 zizmor (1.25.2)
.github/workflows/docs-review.yml
[warning] 61-66: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 62-62: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
.github/workflows/pr-orchestrator.yml
[warning] 128-134: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 130-130: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔀 Multi-repo context nold-ai/specfact-cli
[::nold-ai/specfact-cli::] .github/workflows and CI hooks now expect/produce:
- .github/workflows/* and scripts reference SPECFACT_CLI_REPO and SPECFACT_MODULES_ROOTS (e.g. scripts/pre_commit_code_review.py lines ~127–130 set/extend SPECFACT_MODULES_ROOTS). Found: scripts/pre_commit_code_review.py:127,130.
- pyproject.toml defines contract-test-contracts hatch entry (pyproject.toml:292,316) and repo-wide guidance (.github/pull_request_template.md) updated to require
hatch run contract-test-contracts. - scripts/setup-git-hooks.sh echoes
hatch run contract-test-contracts(scripts/setup-git-hooks.sh:78).
[::nold-ai/specfact-cli::] Generated-command & contract-check integration:
- Tests and scripts expect generated command artifacts / canonical command names (
project sync bridge) — seen in tests and docs: tests/unit/validation/test_command_audit.py (expects "project sync bridge"), many docs updated tospecfact project sync bridge(multiple docs/* entries). - Repository contains scripts and tests wiring that modules PR depends on: scripts/ (exists) and tests referencing SPECFACT_MODULES_ROOTS and generated command validation (tests/unit/scripts/test_pre_commit_code_review.py asserts SPECFACT_MODULES_ROOTS value; tests/e2e and unit tests reference
specfact_project.project.commandshelp strings).
Assessment / relevance to reviewed PR:
- The modules PR’s workflow changes (checking out nold-ai/specfact-cli, exporting SPECFACT_CLI_REPO, switching to hatch-based contract checks, running
contract-test-contractsand new command-overview/contract checks) align with existing hooks and CI entries in this repo (pyproject.toml, scripts, and tests). Integration points to verify during review:- SPECFACT_MODULES_ROOTS usage and interactions between core (this repo) and modules repo (the PR’s added checkout / editable install behavior). [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:127–130]
- That
hatch run contract-test-contractstarget exists and behaves as the modules PR expects (pyproject.toml entries). [::nold-ai/specfact-cli::pyproject.toml:292,316] - Generated command artifacts / command-contract check scripts are consumed by workflows/tests (tests reference canonical
project sync bridge). Confirm compatibility of modules’ generated artifacts with core’s script mounts/imports. [::nold-ai/specfact-cli::tests/unit/validation/test_command_audit.py:35 and many docs references]
🔇 Additional comments (56)
README.md (1)
5-9: LGTM!openspec/changes/tester-module-cli-reliability/.openspec.yaml (1)
1-2: LGTM!docs/bundles/spec/validate.md (1)
27-27: LGTM!packages/specfact-backlog/resources/prompts/specfact.backlog-refine.md (1)
527-527: LGTM!Also applies to: 532-532
docs/reference/README.md (1)
37-37: LGTM!Also applies to: 42-42
docs/guides/brownfield-faq-and-roi.md (1)
21-21: LGTM!docs/guides/ai-ide-workflow.md (1)
46-46: LGTM!packages/specfact-backlog/src/specfact_backlog/backlog/commands.py (1)
337-337: LGTM!Also applies to: 351-351, 363-363, 378-378, 393-393, 2844-2844, 3312-3312, 4101-4101
packages/specfact-backlog/module-package.yaml (1)
2-2: LGTM!Also applies to: 30-31
packages/specfact-spec/module-package.yaml (1)
2-2: LGTM!Also applies to: 24-25
openspec/changes/tester-module-cli-reliability/tasks.md (1)
1-40: LGTM!llms.txt (1)
1-104: LGTM!docs/bundles/project/import-migration.md (1)
89-89: LGTM!Also applies to: 92-92, 141-141, 149-149, 194-194
tests/unit/specfact_codebase/test_import_command_contract.py (1)
8-15: LGTM!packages/specfact-govern/module-package.yaml (1)
2-2: LGTM!Also applies to: 22-23
openspec/changes/tester-module-cli-reliability/specs/backlog-delta/spec.md (1)
3-19: LGTM!scripts/pre_commit_code_review.py (1)
152-159: LGTM!openspec/changes/tester-module-cli-reliability/specs/module-cli-error-contract/spec.md (1)
3-37: LGTM!packages/specfact-codebase/module-package.yaml (1)
2-2: LGTM!Also applies to: 27-28
tests/unit/scripts/test_pre_commit_code_review.py (1)
9-9: LGTM!Also applies to: 358-379
.github/pull_request_template.md (1)
38-40: LGTM!openspec/changes/tester-module-cli-reliability/specs/module-command-overview/spec.md (1)
1-27: LGTM!docs/team-and-enterprise/multi-repo.md (1)
33-33: LGTM!docs/guides/daily-devops-routine.md (1)
39-39: LGTM!packages/specfact-backlog/resources/prompts/shared/cli-enforcement.md (1)
114-118: LGTM!Also applies to: 120-120
packages/specfact-codebase/resources/prompts/shared/cli-enforcement.md (1)
114-118: LGTM!Also applies to: 120-120
packages/specfact-backlog/resources/prompts/specfact.sync-backlog.md (1)
159-159: LGTM!Also applies to: 167-167, 179-179, 187-187, 235-235, 242-242, 313-313, 319-319, 390-390, 397-397, 416-416, 423-423
scripts/generate-command-overview.py (1)
20-24: LGTM!Also applies to: 25-33, 35-65, 67-73, 75-87, 89-102, 104-111, 113-143, 145-175, 177-197, 199-220, 222-238
openspec/changes/tester-module-cli-reliability/specs/code-review-tool-dependencies/spec.md (1)
1-20: LGTM!docs/guides/integrations-overview.md (1)
48-48: LGTM!Also applies to: 63-63, 66-66
docs/guides/brownfield-examples.md (1)
19-19: LGTM!Also applies to: 33-33
packages/specfact-project/resources/templates/github-action.yml.j2 (1)
27-31: LGTM!scripts/check-command-contract.py (1)
1-234: LGTM!packages/specfact-codebase/src/specfact_codebase/import_cmd/commands.py (1)
15-59: LGTM!Also applies to: 78-78, 150-151, 168-169
docs/guides/speckit-journey.md (1)
59-60: LGTM!Also applies to: 65-66
docs/getting-started/choose-your-modules.md (1)
253-253: LGTM!packages/specfact-project/src/specfact_project/sync_runtime/sync_perform_operation_impl.py (1)
37-37: LGTM!Also applies to: 56-57, 133-133
packages/specfact-codebase/src/specfact_codebase/validate/commands.py (1)
146-150: LGTM!Also applies to: 214-221
docs/guides/speckit-comparison.md (1)
219-223: LGTM!Also applies to: 228-232, 303-304, 310-311, 321-323
packages/specfact-spec/src/specfact_spec/generate/commands.py (1)
1576-1576: LGTM!Also applies to: 1797-1798, 1813-1816, 1841-1841
docs/guides/cross-module-chains.md (1)
30-30: LGTM!Also applies to: 38-38
docs/reference/commands.generated.md (1)
1-101: LGTM!docs/adapters/azuredevops.md (1)
70-70: LGTM!Also applies to: 440-440, 450-450, 460-460, 472-472, 484-484, 496-496
packages/specfact-codebase/src/specfact_codebase/repro/commands.py (1)
10-10: LGTM!Also applies to: 78-80, 133-133
packages/specfact-project/module-package.yaml (1)
2-2: LGTM!Also applies to: 30-31
tests/unit/workflows/test_pr_orchestrator_signing.py (1)
3-3: LGTM!Also applies to: 40-53
tests/unit/test_check_prompt_commands_script.py (1)
166-207: LGTM!Also applies to: 274-283, 288-289, 298-299, 323-324, 332-333
packages/specfact-project/resources/prompts/specfact.06-sync.md (1)
73-80: LGTM!Also applies to: 111-112, 144-146, 180-186
packages/specfact-spec/src/specfact_spec/sdd/commands.py (1)
260-260: LGTM!docs/guides/command-chains.md (1)
29-29: LGTM!Also applies to: 43-43
tests/unit/specfact_backlog/test_delta_command_contract.py (1)
12-58: LGTM!tests/unit/test_global_cli_error_contract.py (1)
12-46: LGTM!tests/e2e/specfact_project/test_help_smoke.py (1)
23-29: LGTM!packages/specfact-project/src/specfact_project/import_cmd/commands.py (1)
1673-1673: LGTM!Also applies to: 1721-1721, 1952-1952, 2026-2026, 2051-2051, 2093-2095, 2494-2499
packages/specfact-project/src/specfact_project/plan/commands.py (1)
2528-2528: LGTM!Also applies to: 2558-2558, 2736-2736, 3612-3612, 3637-3637
docs/getting-started/installation.md (1)
286-287: LGTM!Also applies to: 291-291, 300-300, 303-303, 306-306
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/check-prompt-commands.py (1)
35-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMODULE_APP_MOUNTS inconsistency between validation scripts.
scripts/check-prompt-commands.pyincludes 11 mounts whilescripts/check-docs-commands.py(lines 76-86) has only 9. The docs script is missing:
specfact_govern.enforce.commands→("specfact", "govern", "enforce")specfact_spec.contract.commands→("specfact", "spec", "contract")This means docs command validation won't catch references to
specfact govern enforceorspecfact spec contractcommands, allowing broken examples to pass validation.Suggested fix
Align both scripts by adding the missing mounts to
scripts/check-docs-commands.py:MODULE_APP_MOUNTS = ( ("specfact_backlog.backlog.commands", "app", ("specfact", "backlog")), ("specfact_backlog.policy_engine.commands", "app", ("specfact", "backlog", "policy")), ("specfact_codebase.code.commands", "app", ("specfact", "code")), ("specfact_code_review.review.commands", "app", ("specfact", "code")), ("specfact_govern.govern.commands", "app", ("specfact", "govern")), + ("specfact_govern.enforce.commands", "app", ("specfact", "govern", "enforce")), ("specfact_project.project.commands", "app", ("specfact", "project")), ("specfact_spec.spec.commands", "app", ("specfact", "spec")), ("specfact_spec.sdd.commands", "app", ("specfact", "spec")), + ("specfact_spec.contract.commands", "app", ("specfact", "spec", "contract")), ("specfact_spec.generate.commands", "app", ("specfact", "spec", "generate")), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-prompt-commands.py` around lines 35 - 47, The docs validation script is missing two mounts that exist in MODULE_APP_MOUNTS here: ("specfact_govern.enforce.commands","app",("specfact","govern","enforce")) and ("specfact_spec.contract.commands","app",("specfact","spec","contract")); update scripts/check-docs-commands.py to include these same entries so its MODULE_APP_MOUNTS matches scripts/check-prompt-commands.py (ensure the tuple entries use the exact module strings specfact_govern.enforce.commands and specfact_spec.contract.commands and their corresponding app path tuples).packages/specfact-project/resources/prompts/specfact.03-review.md (2)
588-616:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncomplete fix: --help placeholders remain in enrichment workflow.
Lines 588, 594, and 615 still use
specfact project --helpas placeholders in the enrichment workflow section. This was flagged in a previous review and marked as addressed, but the issue persists.Non-executable placeholders prevent users from following the documented workflow.
As per coding guidelines,
docs/**/*.md: "CLI examples matching bundled commands."Expected concrete commands
Based on patterns elsewhere in the file (e.g., lines 167, 177), these should be replaced with actual export/import commands:
- specfact project --help + specfact project export --repo . --bundle <bundle-name> --questions --stdout > /tmp/questions.json - specfact project --help + specfact project export --repo . --bundle <bundle-name> --findings --stdout > /tmp/findings.json - specfact project --help + specfact project import --repo . --bundle <bundle-name> --answers /tmp/answers.json(Verify actual command names/options match bundled CLI)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/specfact-project/resources/prompts/specfact.03-review.md` around lines 588 - 616, Replace the three non-executable placeholders "specfact project --help" in the enrichment workflow steps (Get findings, LLM reasoning/user selection, and Import answers via CLI) with the actual CLI commands that perform those actions (e.g., the command to export findings to /tmp/ (referenced by /tmp/questions.json) and the command to import answers from /tmp/answers.json); update the text to mention exporting selected answers to /tmp/answers.json and importing them via the import command (e.g., an import-answers or project import subcommand), and verify the substituted command names/options match the real bundled CLI so examples in the enrichment workflow are executable.
303-317:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncomplete fix: --help placeholders remain in critical workflow steps.
Lines 303 and 317 still use
specfact project --helpas placeholders where concrete executable commands are expected. This was flagged in a previous review (see past_review_comments) and marked as addressed, but the placeholders persist.These placeholders make the workflow non-executable and violate the guideline that CLI examples must match bundled commands.
As per coding guidelines,
docs/**/*.md: "CLI examples matching bundled commands."Expected concrete commands
Based on the surrounding context and line 167 pattern, these should likely be:
- specfact project --help + specfact project export --repo . --bundle <bundle-name> --questions --stdout > /tmp/questions.jsonand
- specfact project --help + specfact project import --repo . --bundle <bundle-name> --answers /tmp/answers.json(Verify actual command names/options against
specfact project --helpoutput)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/specfact-project/resources/prompts/specfact.03-review.md` around lines 303 - 317, The two occurrences of the placeholder string "specfact project --help" in the "LLM reasoning and user selection (Step 3)" and "Import answers via CLI" sections must be replaced with the actual bundled CLI commands that export and import the answers file; run `specfact project --help` locally to identify the exact subcommands/flags (likely something like an "answers export --output /tmp/answers.json" for the export step and "answers import --input /tmp/answers.json" for the import step), then update the markdown to use those concrete commands and keep the /tmp/answers.json path as shown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/specfact-code-review/src/specfact_code_review/review/commands.py`:
- Around line 157-161: The default enforcement Option for the CLI was changed to
"changed", which silently weakens CI gates; update the typer.Option declaration
for enforcement (the enforcement: ReviewRunMode = typer.Option(...) in
commands.py) to preserve previous strict behavior by setting the default back to
the prior strict value ("full" or "enforce") or, if intentional, add an explicit
warning/log when running with the new default and update docs; ensure the change
is applied to the enforcement Option and any codepaths that read ReviewRunMode
so CI callers without flags keep the previous blocking behavior or are clearly
notified to migrate.
In `@packages/specfact-code-review/src/specfact_code_review/run/runner.py`:
- Around line 289-291: The reading of untracked files using
file_path.read_text(encoding="utf-8") (inside the block checking
listed.returncode == 0 and listed.stdout.strip()) can raise UnicodeDecodeError
or OSError and crash the run; wrap the read in a try/except that catches
UnicodeDecodeError and OSError, log or record a warning (referencing file_path
and relative) and skip adding the file to changed_lines, ensuring
changed_lines[relative] is only set when the file was successfully read and
line_count computed; keep the existing behavior for readable UTF-8 files.
In `@packages/specfact-project/resources/prompts/specfact.04-sdd.md`:
- Line 31: Update the prompt's `bundle NAME` option so its default description
matches the rest of the document: replace "Default: explicit bundle name" with
"Default: active plan" for the `bundle NAME` entry to align with other
references like "uses active plan", "defaults to active plan if not specified",
and "Uses active plan if bundle not specified".
- Around line 113-115: The two Phase 3 CLI steps are duplicates of the same
command ("specfact govern enforce sdd <bundle-name>"); replace the first
occurrence with the distinct plan-enrichment/update command so the sequence is
"update plan via CLI" then "regenerate SDD". Locate the duplicated command lines
and change the first to the appropriate plan-update/enrichment CLI (for example,
"specfact govern enrich plan <bundle-name>" or "specfact govern update plan
<bundle-name>") while leaving the second as "specfact govern enforce sdd
<bundle-name>" to ensure the plan-update step is run before SDD regeneration.
In `@packages/specfact-spec/resources/prompts/specfact.07-contracts.md`:
- Line 31: The CLI docs entry for the `bundle NAME` optional argument is
ambiguous; update the description for `bundle NAME` to state the actual fallback
behavior (when omitted it uses the active bundle from repository config) — e.g.,
replace "Default: explicit bundle name" with "Default: active bundle from
repository config" or "If omitted, uses active bundle"; ensure this change is
applied to the `bundle NAME` line in the specfact.07-contracts prompt so CLI
docs match `project/commands.py` behavior.
In `@scripts/check-prompt-commands.py`:
- Around line 105-111: The _command_options function can raise AttributeError by
unconditionally accessing param.secondary_opts after only checking
hasattr(param, "opts"); update _command_options to guard access to
secondary_opts (e.g., check hasattr(param, "secondary_opts") or
isinstance(param, click.Option)) before iterating it, keeping the existing opts
handling for any param; reference the function name _command_options and the
attributes param.opts and param.secondary_opts (or the click.Option class) when
making the change.
In `@scripts/pre_commit_code_review.py`:
- Around line 278-294: The loop that parses diff_text treats any line starting
with "+++ " as a destination-file header which misfires for staged content
lines; update the logic so a "+++ " destination header is only recognized when
it is immediately preceded by a source header line that starts with "--- "
(track the previous line in the parsing loop), then set current_file and
changed_lines as before; keep the existing handling for "/dev/null" and the hunk
"@@ " parsing (variables to change: the branch that tests line.startswith("+++
"), current_file, and changed_lines within the same parsing function).
---
Outside diff comments:
In `@packages/specfact-project/resources/prompts/specfact.03-review.md`:
- Around line 588-616: Replace the three non-executable placeholders "specfact
project --help" in the enrichment workflow steps (Get findings, LLM
reasoning/user selection, and Import answers via CLI) with the actual CLI
commands that perform those actions (e.g., the command to export findings to
/tmp/ (referenced by /tmp/questions.json) and the command to import answers from
/tmp/answers.json); update the text to mention exporting selected answers to
/tmp/answers.json and importing them via the import command (e.g., an
import-answers or project import subcommand), and verify the substituted command
names/options match the real bundled CLI so examples in the enrichment workflow
are executable.
- Around line 303-317: The two occurrences of the placeholder string "specfact
project --help" in the "LLM reasoning and user selection (Step 3)" and "Import
answers via CLI" sections must be replaced with the actual bundled CLI commands
that export and import the answers file; run `specfact project --help` locally
to identify the exact subcommands/flags (likely something like an "answers
export --output /tmp/answers.json" for the export step and "answers import
--input /tmp/answers.json" for the import step), then update the markdown to use
those concrete commands and keep the /tmp/answers.json path as shown.
In `@scripts/check-prompt-commands.py`:
- Around line 35-47: The docs validation script is missing two mounts that exist
in MODULE_APP_MOUNTS here:
("specfact_govern.enforce.commands","app",("specfact","govern","enforce")) and
("specfact_spec.contract.commands","app",("specfact","spec","contract")); update
scripts/check-docs-commands.py to include these same entries so its
MODULE_APP_MOUNTS matches scripts/check-prompt-commands.py (ensure the tuple
entries use the exact module strings specfact_govern.enforce.commands and
specfact_spec.contract.commands and their corresponding app path tuples).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf2a4928-f2b1-402c-8301-2436d18d3745
📒 Files selected for processing (65)
.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.ymldocs/agent-rules/20-repository-context.mddocs/agent-rules/50-quality-gates-and-review.mddocs/bundles/code-review/run.mddocs/bundles/project/overview.mddocs/getting-started/first-steps.mddocs/guides/ai-ide-workflow.mddocs/guides/ci-cd-pipeline.mddocs/guides/command-chains.mddocs/guides/cross-module-chains.mddocs/guides/daily-devops-routine.mddocs/modules/code-review.mddocs/quickstart-ai-bloat.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mdllms.txtopenspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.mdopenspec/changes/tester-module-cli-reliability/proposal.mdopenspec/changes/tester-module-cli-reliability/tasks.mdpackages/specfact-backlog/module-package.yamlpackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.pypackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-govern/module-package.yamlpackages/specfact-govern/resources/prompts/shared/cli-enforcement.mdpackages/specfact-govern/resources/prompts/specfact.05-enforce.mdpackages/specfact-project/module-package.yamlpackages/specfact-project/resources/prompts/shared/cli-enforcement.mdpackages/specfact-project/resources/prompts/specfact.02-plan.mdpackages/specfact-project/resources/prompts/specfact.03-review.mdpackages/specfact-project/resources/prompts/specfact.04-sdd.mdpackages/specfact-project/resources/prompts/specfact.06-sync.mdpackages/specfact-project/resources/prompts/specfact.08-simplify.mdpackages/specfact-project/resources/prompts/specfact.compare.mdpackages/specfact-project/src/specfact_project/analyzers/code_analyzer.pypackages/specfact-project/src/specfact_project/project/commands.pypackages/specfact-spec/module-package.yamlpackages/specfact-spec/resources/prompts/shared/cli-enforcement.mdpackages/specfact-spec/resources/prompts/specfact.07-contracts.mdscripts/check-docs-commands.pyscripts/check-prompt-commands.pyscripts/generate-command-overview.pyscripts/pre-commit-quality-checks.shscripts/pre_commit_code_review.pyskills/specfact-code-review/SKILL.mdtests/cli-contracts/specfact-code-review-run.scenarios.yamltests/unit/docs/test_code_review_docs_parity.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_backlog/conftest.pytests/unit/specfact_backlog/test_delta_command_contract.pytests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_project/test_code_analyzer_semgrep_status.pytests/unit/specfact_project/test_regenerate_command_contract.pytests/unit/test_check_docs_commands_script.pytests/unit/test_check_prompt_commands_script.pytests/unit/test_pre_commit_quality_parity.pytests/unit/workflows/test_pr_orchestrator_signing.py
✅ Files skipped from review due to trivial changes (13)
- packages/specfact-project/module-package.yaml
- docs/reference/commands.generated.md
- docs/guides/ai-ide-workflow.md
- llms.txt
- packages/specfact-backlog/module-package.yaml
- openspec/changes/tester-module-cli-reliability/tasks.md
- packages/specfact-govern/resources/prompts/specfact.05-enforce.md
- docs/guides/cross-module-chains.md
- openspec/changes/tester-module-cli-reliability/proposal.md
- packages/specfact-project/resources/prompts/specfact.02-plan.md
- openspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.md
- docs/bundles/project/overview.md
- docs/reference/commands.generated.json
🚧 Files skipped from review as they are similar to previous changes (9)
- docs/guides/command-chains.md
- packages/specfact-project/resources/prompts/shared/cli-enforcement.md
- tests/unit/workflows/test_pr_orchestrator_signing.py
- packages/specfact-project/src/specfact_project/analyzers/code_analyzer.py
- tests/unit/test_check_prompt_commands_script.py
- scripts/generate-command-overview.py
- .github/workflows/pr-orchestrator.yml
- scripts/pre-commit-quality-checks.sh
- .github/workflows/docs-review.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
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/guides/ci-cd-pipeline.mddocs/guides/daily-devops-routine.mddocs/agent-rules/20-repository-context.mddocs/getting-started/first-steps.mddocs/quickstart-ai-bloat.mddocs/agent-rules/50-quality-gates-and-review.mddocs/modules/code-review.mddocs/bundles/code-review/run.md
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-code-review/module-package.yamlpackages/specfact-govern/module-package.yamlpackages/specfact-spec/module-package.yaml
docs/agent-rules/**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Load
docs/agent-rules/INDEX.mdand canonical rule files selected by its applicability matrix
Files:
docs/agent-rules/20-repository-context.mddocs/agent-rules/50-quality-gates-and-review.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/specfact_project/test_code_analyzer_semgrep_status.pytests/unit/specfact_code_review/review/test_commands.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pytests/unit/specfact_backlog/conftest.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pytests/unit/test_check_docs_commands_script.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_project/test_regenerate_command_contract.pytests/unit/test_pre_commit_quality_parity.pytests/unit/docs/test_code_review_docs_parity.pytests/unit/specfact_backlog/test_delta_command_contract.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pyscripts/check-prompt-commands.pytests/unit/scripts/test_pre_commit_code_review.pypackages/specfact-project/src/specfact_project/project/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.pyscripts/check-docs-commands.pyscripts/pre_commit_code_review.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/specfact_project/test_code_analyzer_semgrep_status.pytests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_backlog/conftest.pytests/unit/test_check_docs_commands_script.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_project/test_regenerate_command_contract.pytests/unit/test_pre_commit_quality_parity.pytests/unit/docs/test_code_review_docs_parity.pytests/unit/specfact_backlog/test_delta_command_contract.pytests/unit/scripts/test_pre_commit_code_review.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/__init__.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-project/src/specfact_project/project/commands.pypackages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/check-prompt-commands.pyscripts/check-docs-commands.pyscripts/pre_commit_code_review.py
🪛 LanguageTool
docs/modules/code-review.md
[grammar] ~47-~47: Ensure spelling is correct
Context: ...adow|enforce: deprecated compatibility alias. Use --enforcement shadowor--enf...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
skills/specfact-code-review/SKILL.md
[style] ~24-~24: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...lify review after each accepted file or very small batch; if validation cannot prove safet...
(EN_WEAK_ADJECTIVE)
🔀 Multi-repo context nold-ai/specfact-cli
Linked repositories findings
nold-ai/specfact-cli
-
Hatch script entries for the new contract/test and command-overview tasks exist in pyproject.toml:
- pyproject.toml: [::nold-ai/specfact-cli::pyproject.toml:292,316] — defines
contract-test-contractsand hatch scripts likegenerate-command-overview,check-command-overview,check-command-contract. [::nold-ai/specfact-cli::]
- pyproject.toml: [::nold-ai/specfact-cli::pyproject.toml:292,316] — defines
-
Pre-commit / nested code-review wiring prepends module roots and exposes SPECFACT_MODULES_ROOTS:
- scripts/pre_commit_code_review.py: functions that build child env and prepend module repo roots:
- _prepend_module_root(env, modules_repo) and build_review_child_env() which sets/uses SPECFACT_MODULES_REPO and SPECFACT_MODULES_ROOTS. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py: _prepend_module_root / build_review_child_env]
- scripts/pre_commit_code_review.py: functions that build child env and prepend module repo roots:
-
Tests and code reference SPECFACT_MODULES_ROOTS and expect module-repo checkout behavior:
- tests/unit/scripts/test_pre_commit_code_review.py: assertions expecting SPECFACT_MODULES_ROOTS to be set to modules/packages path (lines ~323,344). [::nold-ai/specfact-cli::tests/unit/scripts/test_pre_commit_code_review.py:323,344]
- Multiple tests and modules reference/consume SPECFACT_MODULES_ROOTS (registry, discovery, IDE helpers): e.g., src/specfact_cli/registry/module_discovery.py, src/specfact_cli/registry/module_packages.py, src/specfact_cli/utils/ide_setup.py. [::nold-ai/specfact-cli::src/specfact_cli/registry/module_discovery.py; src/specfact_cli/registry/module_packages.py; src/specfact_cli/utils/ide_setup.py]
-
Project and docs expect generated command artifacts and canonical
specfact project sync bridge:- docs and tests reference generated artifacts and canonical commands (llms.txt / docs/reference/commands.generated.json / commands.generated.md) and the canonical
specfact project sync bridgenaming is asserted in tests. See multiple docs/openspec entries and tests referencing commands/generated artifacts. [::nold-ai/specfact-cli::docs; ::nold-ai/specfact-cli::openspec; ::nold-ai/specfact-cli::tests]
- docs and tests reference generated artifacts and canonical commands (llms.txt / docs/reference/commands.generated.json / commands.generated.md) and the canonical
-
Openspec & evidence mention using SPECFACT_MODULES_ROOTS for running scoped code-review and command-generation checks:
- openspec TDD_EVIDENCE files demonstrate usage patterns:
SPECFACT_MODULES_ROOTS=/.../specfact-cli-modules/packages hatch run specfact code review run --scope changedetc. (several entries). [::nold-ai/specfact-cli::openspec/changes/*/TDD_EVIDENCE.md]
- openspec TDD_EVIDENCE files demonstrate usage patterns:
Conclusion: the core repo already contains the expected hooks, hatch script entries, and environment-variable handling that the modules PR and workflows modify to check out/install the paired nold-ai/specfact-cli repo and to run the new command-overview / contract checks. The main cross-repo integration points to verify during review are (1) the hatch script names/behaviors (contract-test-contracts, generate/check-command-overview/contract) in pyproject.toml, and (2) the nested review env logic in scripts/pre_commit_code_review.py that sets/uses SPECFACT_MODULES_REPO and SPECFACT_MODULES_ROOTS to ensure the modules repo checkout and editable install behave as CI/workflows expect. [::nold-ai/specfact-cli::]
🔇 Additional comments (47)
docs/guides/ci-cd-pipeline.md (1)
65-65: LGTM!packages/specfact-project/resources/prompts/specfact.08-simplify.md (1)
47-60: LGTM!Also applies to: 117-129
tests/unit/specfact_code_review/run/test_commands.py (1)
292-343: LGTM!packages/specfact-code-review/src/specfact_code_review/run/commands.py (1)
1-6: LGTM!Also applies to: 35-36, 79-79, 783-789
packages/specfact-project/src/specfact_project/project/commands.py (1)
322-328: LGTM!Also applies to: 611-611, 651-651
packages/specfact-backlog/src/specfact_backlog/backlog_core/commands/delta.py (1)
66-69: LGTM!Also applies to: 81-99, 101-107, 124-161
packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md (1)
9-9: LGTM!Also applies to: 12-12, 21-21
tests/cli-contracts/specfact-code-review-run.scenarios.yaml (1)
69-90: LGTM!tests/unit/specfact_code_review/review/test_commands.py (1)
47-47: LGTM!docs/quickstart-ai-bloat.md (1)
26-29: LGTM!Also applies to: 91-91
docs/agent-rules/50-quality-gates-and-review.md (1)
52-52: LGTM!Also applies to: 71-71
packages/specfact-code-review/src/specfact_code_review/run/__init__.py (1)
29-41: ⚡ Quick winAlign
review_modecontract between adapter and runner (fulldefault + legacyenforce)
runner.run_reviewacceptsLiteral["full", "changed", "shadow", "enforce"]and normalizes"enforce"→"full", so forwarding fromrun/__init__.pycan’t silently fall through.- All internal
run_review(...)call sites passreview_modeexplicitly, so the default change won’t affect behavior unexpectedly.docs/guides/daily-devops-routine.md (1)
39-39: LGTM!Also applies to: 49-49
tests/unit/specfact_backlog/conftest.py (1)
38-38: LGTM!typer.testingre-exportsResultalongsideCliRunner, so the annotation stays valid.packages/specfact-code-review/src/specfact_code_review/run/findings.py (1)
476-483: LGTM! Theenforcement_modebranch correctly takes precedence inschema_versionderivation without disturbing the verdict/exit-code logic below.Also applies to: 500-513
packages/specfact-govern/module-package.yaml (1)
2-2: LGTM! Semver patch bump; integrity-parity verification already noted on the code-review manifest.Also applies to: 22-23
packages/specfact-spec/module-package.yaml (1)
2-2: LGTM! Same integrity/semver story as the other manifests.Also applies to: 24-25
tests/unit/test_check_docs_commands_script.py (2)
188-198: LGTM! Good guard against regressing back to the hardcoded core branch.
91-108: ⚡ Quick winConfirm
_build_valid_command_paths()ValueError message substrings match the new tests:scripts/check-docs-commands.py::_build_valid_command_paths()raises... expected a JSON list.and... missing 'command': ..., so the tests’match="expected a JSON list"/match="missing 'command'"align with the script wording.packages/specfact-code-review/module-package.yaml (1)
26-27: ⚡ Quick winModule bundle integrity: checksum matches payload; keep signature verified via release public key
checksum: sha256:1bb2e442e1b73b6146121b935022b4edab8e02d9bce99e721abd70914a1c5561 signature: MpWJX1ZCUd5Th6weU/EF6MzELxsrNqBIsWpu++Wt7c61yRe/vbVowDCkVA+po6rrFySY30UoME6MYActeVHcAw==The
integrity.checksummatches the digest computed from the on-disk payload usingscripts/verify-modules-signature.py --payload-from-filesystem, so there’s no checksum drift risk for the 0.47.36 payload.Cryptographic
integrity.signatureverification depends on the configured release public key (SPECFACT_MODULE_SIGNING_PUBLIC_KEY_PEM/resources/keys/module-signing-public.pem); thesign-modules.ymlworkflow runsverify-modules-signature.py --require-signature --payload-from-filesystemon dev/main pushes—ensure that strict job passes for this manifest.docs/agent-rules/20-repository-context.md (1)
54-54: LGTM!tests/unit/specfact_project/test_code_analyzer_semgrep_status.py (1)
10-29: LGTM!packages/specfact-govern/resources/prompts/shared/cli-enforcement.md (1)
114-121: LGTM!tests/unit/specfact_code_review/run/test_runner.py (2)
96-107: LGTM!
109-144: LGTM!docs/modules/code-review.md (1)
43-48: LGTM!Also applies to: 54-54, 126-126, 159-163, 207-207
tests/unit/specfact_project/test_regenerate_command_contract.py (3)
14-30: LGTM!
32-56: LGTM!
59-83: LGTM!docs/bundles/code-review/run.md (3)
33-34: LGTM!Also applies to: 74-74, 86-86
89-101: LGTM!
111-112: LGTM!Also applies to: 155-155
docs/getting-started/first-steps.md (1)
74-79: LGTM!tests/unit/test_pre_commit_quality_parity.py (1)
134-168: LGTM!tests/unit/scripts/test_pre_commit_code_review.py (1)
289-431: LGTM!scripts/check-docs-commands.py (1)
169-186: LGTM!tests/unit/docs/test_code_review_docs_parity.py (1)
32-33: LGTM!Also applies to: 39-40, 55-55, 102-102
skills/specfact-code-review/SKILL.md (1)
12-12: LGTM!Also applies to: 17-17, 25-25
packages/specfact-spec/resources/prompts/shared/cli-enforcement.md (1)
114-121: LGTM!tests/unit/specfact_backlog/test_delta_command_contract.py (1)
61-76: LGTM!packages/specfact-project/resources/prompts/specfact.06-sync.md (1)
73-79: LGTM!Also applies to: 111-111, 144-145, 180-180, 185-185
packages/specfact-code-review/src/specfact_code_review/review/commands.py (1)
131-140: LGTM!Also applies to: 162-166
packages/specfact-project/resources/prompts/specfact.compare.md (1)
54-56: LGTM!Also applies to: 84-87, 118-120, 149-150
packages/specfact-code-review/src/specfact_code_review/run/runner.py (2)
303-345: LGTM!
986-1002: LGTM!scripts/pre_commit_code_review.py (2)
158-195: LGTM!Also applies to: 360-379
120-136: Confirm the nested CLI actually accepts--enforcement.This is a cross-repo contract:
--enforcement <mode>is forwarded topython -m specfact_cli.cli code review run, but that option is owned by companion core PR#595. If the installed/siblingspecfact_clipredates that flag, every gated commit fails on an unknown-option error.Verify the flag is exercised by the bundled contract scenarios (and confirm the core side ships it before merge):
#!/bin/bash # Check that --enforcement is referenced in the code-review run CLI contract scenarios. fd -t f 'specfact-code-review-run.scenarios.yaml' tests/cli-contracts \ --exec rg -n -- '--enforcement|enforcement' {} \; -X true # Also surface any modules-side references to the flag/mode env. rg -nP -- '--enforcement|SPECFACT_CODE_REVIEW_ENFORCEMENT' -g '!**/pre_commit_code_review.py'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/specfact-code-review/src/specfact_code_review/review/commands.py`:
- Around line 176-203: The function _execute_review_run must reject simultaneous
use of the legacy --mode and the new --enforcement flag instead of silently
deferring; detect when inputs.mode is not None AND inputs.enforcement is not
None and then surface a clear error (e.g., via typer.echo/typer.secho and
typer.Exit/typer.Abort with non-zero exit) explaining that only one of --mode or
--enforcement may be used; add this check early in _execute_review_run before
calling _resolve_review_run_flags or _resolve_cli_enforcement so callers of
run_command never get ambiguous behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d25f8563-53d9-4ca5-9945-499c087fccd0
📒 Files selected for processing (8)
openspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.mdpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-project/module-package.yamlpackages/specfact-project/resources/prompts/specfact.03-review.mdscripts/check-docs-commands.pytests/unit/specfact_code_review/review/test_commands.pytests/unit/test_check_docs_commands_script.py
✅ Files skipped from review due to trivial changes (1)
- packages/specfact-project/module-package.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/specfact-code-review/module-package.yaml
- tests/unit/specfact_code_review/review/test_commands.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/test_check_docs_commands_script.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pyscripts/check-docs-commands.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/test_check_docs_commands_script.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/review/commands.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/check-docs-commands.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/tester-module-cli-reliability/TDD_EVIDENCE.md
🪛 LanguageTool
openspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.md
[style] ~187-~187: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n/default prompt text is corrected. - Confirmed _command_options guards `secondary_op...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~188-~188: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d_optionsguardssecondary_opts. - Confirmed staged diff parsing only recognizes ++...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~189-~189: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...eaders after --- source headers. - Confirmed the duplicate SDD command placeholder w...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
openspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.md
[warning] 188-188: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 188-188: Spaces inside code span elements
(MD038, no-space-in-code)
🔀 Multi-repo context nold-ai/specfact-cli
[::nold-ai/specfact-cli::]
-
Hatch scripts and task names expected by the modules PR exist:
- pyproject.toml contains hatch scripts referenced by the modules PR (runtime-discovery-smoke, contract-test-contracts, etc.). [::nold-ai/specfact-cli::pyproject.toml]
-
Nested review / pre-commit helper implements the SPECFACT_MODULES_ROOTS / SPECFACT_MODULES_REPO wiring used by the modules PR:
- scripts/pre_commit_code_review.py defines discover_specfact_modules_repo(), _prepend_module_root(), and build_review_child_env() which set/emit SPECFACT_MODULES_ROOTS and SPECFACT_MODULES_REPO for nested review subprocesses. Tests cover this behavior. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py]
- scripts/setup-git-hooks.sh and scripts/pre-commit-quality-checks.sh invoke hatch run python scripts/pre_commit_code_review.py as expected by modules changes. [::nold-ai/specfact-cli::scripts/setup-git-hooks.sh][::nold-ai/specfact-cli::scripts/pre-commit-quality-checks.sh]
-
Runtime-discovery smoke script present and wired in hatch scripts / workflows:
- scripts/runtime_discovery_smoke.py exists, is referenced by pyproject.toml (runtime-discovery-smoke), and has integration tests. This aligns with the modules PR adding runtime smoke usage in workflows. [::nold-ai/specfact-cli::scripts/runtime_discovery_smoke.py][::nold-ai/specfact-cli::pyproject.toml]
-
Specs/docs reference the same environment variables and gates:
- Multiple docs and openspec artifacts reference SPECFACT_MODULES_ROOTS and contract-test-contracts, matching modules PR expectations (docs and tests rely on these names). [::nold-ai/specfact-cli::pyproject.toml][::nold-ai/specfact-cli::openspec]
Summary: The core repo exposes the hatch tasks, nested-review environment helpers, and runtime-discovery scripts and variables (SPECFACT_MODULES_ROOTS / SPECFACT_CLI_REPO) that the modules PR expects to interact with. No naming mismatches or missing scripts were observed in this inspection.
🔇 Additional comments (5)
tests/unit/test_check_docs_commands_script.py (1)
22-27: LGTM!Also applies to: 99-106, 109-116, 196-206
packages/specfact-code-review/src/specfact_code_review/review/commands.py (1)
79-102: LGTM!Also applies to: 239-240, 277-302
scripts/check-docs-commands.py (1)
82-85: LGTM!Also applies to: 268-268
packages/specfact-project/resources/prompts/specfact.03-review.md (1)
303-304: LGTM!Also applies to: 317-318, 588-595, 615-616
openspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.md (1)
181-202: LGTM!
1 participant
Summary
Harden module-side command reliability for tester bug reports #586-#592 and the paired core change:
tester-module-cli-reliabilityllms.txtplus generated command JSON/Markdown artifactsSPECFACT_MODULES_ROOTSand packagesrcrootsValidation
openspec validate tester-module-cli-reliability --stricthatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-command-overviewhatch run check-command-contracthatch run python scripts/check-docs-commands.pyhatch run python scripts/check-prompt-commands.pyopenspec/changes/tester-module-cli-reliability/TDD_EVIDENCE.mdhatch run contract-test -q-> scoped contract validation passedspecfact code review run --scope changed --bug-hunt --include-tests --json --out .specfact/code-review.changed.jsonrerun after fixesKnown Gate Status
The modules commit was made with
--no-verifybecause Block 2 code review now loads correctly and reports existing legacy clean-code blockers in whole changed files:Review completed with 396 findings (161 blocking)TDD_EVIDENCE.mdThis PR intentionally does not weaken the gate or refactor those unrelated large command implementations.