diff --git a/docs/agent-rules/20-repository-context.md b/docs/agent-rules/20-repository-context.md index d771d9ee..2cce7f08 100644 --- a/docs/agent-rules/20-repository-context.md +++ b/docs/agent-rules/20-repository-context.md @@ -13,7 +13,7 @@ tracks: - packages/** - registry/index.json - pyproject.toml -last_reviewed: 2026-04-12 +last_reviewed: 2026-04-16 exempt: false exempt_reason: "" id: agent-rules-repository-context @@ -50,7 +50,8 @@ hatch run verify-modules-signature --payload-from-filesystem --enforce-version-b hatch run contract-test hatch run smart-test hatch run test -hatch run specfact code review run --json --out .specfact/code-review.json +# manual code review: always include --bug-hunt (CrossHair longer budgets; see bundle docs) +hatch run specfact code review run --bug-hunt --json --out .specfact/code-review.json ``` ## Architecture diff --git a/docs/agent-rules/50-quality-gates-and-review.md b/docs/agent-rules/50-quality-gates-and-review.md index 8582b653..38c2cc35 100644 --- a/docs/agent-rules/50-quality-gates-and-review.md +++ b/docs/agent-rules/50-quality-gates-and-review.md @@ -13,7 +13,7 @@ tracks: - scripts/pre_commit_code_review.py - scripts/verify-modules-signature.py - docs/agent-rules/** -last_reviewed: 2026-04-12 +last_reviewed: 2026-04-16 exempt: false exempt_reason: "" id: agent-rules-quality-gates-and-review @@ -47,7 +47,7 @@ depends_on: 7. `hatch run contract-test` 8. `hatch run smart-test` 9. `hatch run test` -10. `hatch run specfact code review run --json --out .specfact/code-review.json` (full-repo scope when required: add `--scope full`; machine-readable evidence lives at `.specfact/code-review.json` and unresolved findings block merge unless an explicit exception is documented) +10. `hatch run specfact code review run --bug-hunt --json --out .specfact/code-review.json` (always pass **`--bug-hunt`** on manual runs so CrossHair uses bug-hunt timeouts; full-repo scope when required: add **`--scope full`**; machine-readable evidence lives at `.specfact/code-review.json` and unresolved findings block merge unless an explicit exception is documented) ## Pre-commit order @@ -66,7 +66,7 @@ Run the full pipeline manually with `./scripts/pre-commit-quality-checks.sh` or ## Clean-code review gate -The repository enforces the clean-code charter through `specfact code review run`. Zero regressions in `naming`, `kiss`, `yagni`, `dry`, and `solid` are required before merge. +The repository enforces the clean-code charter through `specfact code review run`. When agents or developers invoke the review manually (outside the pre-commit helper), include **`--bug-hunt`** so the contract runner gives CrossHair the longer bug-hunt budgets documented in the code-review bundle. Zero regressions in `naming`, `kiss`, `yagni`, `dry`, and `solid` are required before merge. ## Module signature gate diff --git a/docs/bundles/code-review/overview.md b/docs/bundles/code-review/overview.md index 2df3a93a..1c8f42a7 100644 --- a/docs/bundles/code-review/overview.md +++ b/docs/bundles/code-review/overview.md @@ -12,7 +12,7 @@ expertise_level: [beginner, intermediate] The **Code Review** bundle (`nold-ai/specfact-code-review`) extends the shared **`specfact code`** command group with **`review`** workflows: governed review runs, **reward ledger** history, and **house-rules** skill management. -Use it together with the [Codebase](../codebase/overview/) bundle (`import`, `analyze`, `drift`, `validate`, `repro`) on the same `code` surface. +Use it together with the [Codebase](/bundles/codebase/overview/) bundle (`import`, `analyze`, `drift`, `validate`, `repro`) on the same `code` surface. ## Prerequisites @@ -63,5 +63,5 @@ specfact code review rules show --help - [Code review run](../run/) - [Code review ledger](../ledger/) - [Code review rules](../rules/) -- [Code review module](../../modules/code-review/) -- [Codebase bundle overview](../codebase/overview/) — import, drift, validation, repro +- [Code review module](/modules/code-review/) +- [Codebase bundle overview](/bundles/codebase/overview/) — import, drift, validation, repro diff --git a/docs/bundles/code-review/run.md b/docs/bundles/code-review/run.md index e1b3cada..e8b406b6 100644 --- a/docs/bundles/code-review/run.md +++ b/docs/bundles/code-review/run.md @@ -16,6 +16,8 @@ expertise_level: [intermediate, advanced] The command prints **progress** to the terminal (spinner/status while the pipeline prepares and runs). With **`--json`**, it writes a machine-readable **`ReviewReport`** JSON file (defaulting to **`review-report.json`** in the working directory when **`--out`** is omitted). +The pipeline reviews **`.py`** and **`.pyi`** only. The **`--focus docs`** facet selects Python files whose path contains a **`docs/`** directory segment (for example tooling beside the Jekyll site), not Markdown documentation pages. For published-site link, front matter, and command-example checks on the modules docs tree, run **`python scripts/check-docs-commands.py`** in this repository (see CI and contributing docs). + ## Command - `specfact code review run [FILES...]` @@ -29,7 +31,7 @@ The command prints **progress** to the terminal (spinner/status while the pipeli | `--include-tests`, `--exclude-tests` | Control whether changed test files participate in auto-scope review | | `--focus ` | Limit auto-discovered scope to **`source`**, **`tests`**, and/or **`docs`** (repeatable); mutually exclusive with `--include-tests` / `--exclude-tests` | | `--mode shadow\|enforce` | **`shadow`** surfaces findings without failing the exit code for policy violations; **`enforce`** applies normal gating (default **`enforce`**) | -| `--level error\|warning` | Optional reporting level override for the review run | +| `--level error\|warning` | Optional reporting level override before scoring: **`error`** keeps errors only (drops warnings and info); **`warning`** keeps errors and warnings (drops info only); omit to keep all severities (JSON, verdict, and `ci_exit_code` use the filtered list) | | `--bug-hunt` | Enable exploratory / bug-hunt style heuristics in the review pipeline | | `--include-noise`, `--suppress-noise` | Keep or suppress known low-signal findings | | `--json` | Emit a `ReviewReport` JSON file | @@ -55,12 +57,63 @@ The Typer entrypoint validates **review flags** first: it raises **`typer.BadPar ## Examples +### Auto-discovered scope (omit positional files) + ```bash +# Tracked + untracked changes; tests excluded by default for auto-scope specfact code review run --scope changed + +# Same, with bug-hunt heuristics on the discovered file set +specfact code review run --scope changed --bug-hunt + +# Full index, limited to one package (repeat --path for more repo-relative prefixes) specfact code review run --scope full --path packages/specfact-code-review + +# Package sources plus that package’s unit tests +specfact code review run --scope full --path packages/specfact-code-review --path tests/unit/specfact_code_review + +# Errors only before scoring — warnings and info omitted from JSON, verdict, and ci_exit_code +specfact code review run --scope changed --level error + +# Longer CrossHair budgets for exploratory bug-hunt pass (with explicit files) +specfact code review run --bug-hunt --json --out /tmp/review-bughunt.json packages/specfact-code-review/src/specfact_code_review/run/commands.py +``` + +### Shadow mode and JSON to a file + +**`--mode shadow`** runs the full toolchain but forces process exit code **`0`** and JSON **`ci_exit_code`** **`0`** so callers can ingest reports without failing a step; **`overall_verdict`** still reflects the real outcome. + +```bash +specfact code review run --scope changed --mode shadow --json --out /tmp/review-report.json +``` + +### `--focus` facets (repeatable) + +Use **`--focus`** with **`source`**, **`tests`**, and/or **`docs`** (union of facets, then intersect with scope). Do not combine **`--focus`** with **`--include-tests`** or **`--exclude-tests`**. + +```bash +specfact code review run --scope changed --focus tests +specfact code review run --scope full --path packages/specfact-code-review --focus source +specfact code review run --scope full --focus docs +``` + +### Positional files (explicit Python paths) + +Do not pass **`--scope`** or **`--path`** when **`FILES...`** are present. + +```bash specfact code review run --json --out /tmp/review-report.json packages/specfact-code-review/src/specfact_code_review/run/commands.py specfact code review run --score-only packages/specfact-code-review/src/specfact_code_review/run/commands.py specfact code review run --fix packages/specfact-code-review/src/specfact_code_review/run/commands.py +specfact code review run --no-tests packages/specfact-code-review/src/specfact_code_review/run/commands.py +``` + +### Noise and interactive test inclusion + +```bash +specfact code review run --scope changed --include-noise +specfact code review run --scope changed --suppress-noise +specfact code review run --scope changed --interactive ``` ## Bundle-owned resources @@ -71,4 +124,4 @@ The review pipeline uses rules, skills, and policy payloads shipped with the ins - [Code review ledger](/bundles/code-review/ledger/) - [Code review rules](/bundles/code-review/rules/) -- [Code review module guide](../../modules/code-review/) +- [Code review module guide](/modules/code-review/) diff --git a/docs/bundles/codebase/overview.md b/docs/bundles/codebase/overview.md index b961d5c1..625abec5 100644 --- a/docs/bundles/codebase/overview.md +++ b/docs/bundles/codebase/overview.md @@ -12,7 +12,7 @@ expertise_level: [beginner, intermediate] The **Codebase** bundle (`nold-ai/specfact-codebase`) mounts under `specfact code` alongside the Code Review bundle. It focuses on **brownfield import**, **contract coverage analysis**, **drift detection**, **sidecar validation**, and **reproducible validation suites**. -For automated review runs (Ruff, Semgrep, ledger, rules), see [Code Review](../code-review/overview/) — also on the `code` command group. +For automated review runs (Ruff, Semgrep, ledger, rules), see [Code Review](/bundles/code-review/overview/) — also on the `code` command group. ## Prerequisites @@ -28,7 +28,7 @@ For automated review runs (Ruff, Semgrep, ledger, rules), see [Code Review](../c | `specfact code import` (default) | Import a repository into a project bundle (`from-code` behavior; see `--help`) | | `specfact code import from-bridge` | Import from an external bridge/export flow | -Advanced import topics: [Project import command features](../project/import-migration/) (cross-bundle). +Advanced import topics: [Project import command features](/bundles/project/import-migration/) (cross-bundle). ### `analyze` — structure and contracts diff --git a/docs/bundles/govern/overview.md b/docs/bundles/govern/overview.md index add1fee4..3e7675b8 100644 --- a/docs/bundles/govern/overview.md +++ b/docs/bundles/govern/overview.md @@ -47,4 +47,4 @@ specfact govern patch apply --help - [Govern enforce](../enforce/) - [Govern patch](../patch/) -- [Command reference](../../reference/commands/) — nested `govern` commands +- [Command reference](/reference/commands/) — nested `govern` commands diff --git a/docs/bundles/project/overview.md b/docs/bundles/project/overview.md index 18e1f8bd..fe8d16af 100644 --- a/docs/bundles/project/overview.md +++ b/docs/bundles/project/overview.md @@ -16,7 +16,7 @@ The **Project** bundle (`nold-ai/specfact-project`) manages SpecFact **project b - SpecFact CLI and a repository with `.specfact/` layout - Bundle installed: `specfact module install nold-ai/specfact-project` -- For backlog-linked flows: install [Backlog](../backlog/overview/) and link a provider +- For backlog-linked flows: install [Backlog](/bundles/backlog/overview/) and link a provider ## Command families @@ -76,7 +76,7 @@ Use the top-level group (`specfact sync --help`). ## Related: codebase import -Brownfield **code import** (`specfact code import`, `specfact import …`) lives in the [Codebase](../codebase/overview/) bundle; it often feeds project bundles. See [Import command features](../import-migration/) for behavior that spans both bundles. +Brownfield **code import** (`specfact code import`, `specfact import …`) lives in the [Codebase](/bundles/codebase/overview/) bundle; it often feeds project bundles. See [Import command features](../import-migration/) for behavior that spans both bundles. ## Bundle-owned prompts and plan templates diff --git a/docs/bundles/spec/overview.md b/docs/bundles/spec/overview.md index 2e7f220c..8e91bb78 100644 --- a/docs/bundles/spec/overview.md +++ b/docs/bundles/spec/overview.md @@ -71,8 +71,8 @@ specfact spec generate contracts --help ## See also -- [Command reference](../../reference/commands/) — bundle-to-command mapping +- [Command reference](/reference/commands/) — bundle-to-command mapping - [Spec validate and backward compatibility](/bundles/spec/validate/) - [Generate Specmatic tests](/bundles/spec/generate-tests/) - [Run a mock server](/bundles/spec/mock/) -- [Contract testing workflow](../../guides/contract-testing-workflow/) +- [Contract testing workflow](/contract-testing-workflow/) diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index ce498abe..8661a851 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -44,9 +44,10 @@ Options (aligned with `specfact code review run --help`): full toolchain and preserves `overall_verdict` in JSON, but forces `ci_exit_code` and the process exit code to `0` so CI or hooks can log signal without blocking -- `--level error|warning`: drop findings below the chosen severity **before** - scoring and report construction so JSON, tables, score, verdict, and - `ci_exit_code` all match the filtered list. Omit to keep all severities +- `--level error|warning`: filter findings **before** scoring so JSON, tables, + score, verdict, and `ci_exit_code` match the filtered list: **`error`** + keeps errors only (warnings and info dropped); **`warning`** keeps errors and + warnings (info dropped). Omit to keep all severities - `--bug-hunt`: enable the bug-hunt pass (CrossHair uses longer budgets: per-path timeout **10s**, subprocess budget **120s**; other tools keep normal speed) - `--include-noise` / `--suppress-noise`: include or suppress known low-signal @@ -101,6 +102,11 @@ specfact code review run --scope full --path packages/specfact-code-review specfact code review run --scope changed --path packages/specfact-code-review --path tests/unit/specfact_code_review ``` +Copy-pastable recipes for **shadow mode**, **JSON `--out`**, **`--focus`** +(`source` / `tests` / `docs` Python only), **noise flags**, and **interactive** +test prompts live in the [Code review run](/bundles/code-review/run/) bundle +guide (same Typer surface as this section). + Positional `FILES...` cannot be mixed with **`--scope`** or **`--path`** (see **Invalid combinations** above). diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/TDD_EVIDENCE.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/TDD_EVIDENCE.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/TDD_EVIDENCE.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/TDD_EVIDENCE.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/design.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/design.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/design.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/design.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/proposal.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/proposal.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/proposal.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/proposal.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-tool-dependencies/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-tool-dependencies/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-tool-dependencies/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-tool-dependencies/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-cli-contracts/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/review-cli-contracts/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-cli-contracts/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/review-cli-contracts/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md diff --git a/openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/tasks.md b/openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/tasks.md similarity index 100% rename from openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/tasks.md rename to openspec/changes/archive/2026-04-16-code-review-bug-finding-and-sidecar-venv-fix/tasks.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/.openspec.yaml b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/.openspec.yaml similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/.openspec.yaml rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/.openspec.yaml diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/design.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/design.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/design.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/design.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/proposal.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/proposal.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/proposal.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/proposal.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/specs/github-hierarchy-cache/spec.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/specs/github-hierarchy-cache/spec.md similarity index 100% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/specs/github-hierarchy-cache/spec.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/specs/github-hierarchy-cache/spec.md diff --git a/openspec/changes/governance-04-deterministic-agent-governance-loading/tasks.md b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/tasks.md similarity index 95% rename from openspec/changes/governance-04-deterministic-agent-governance-loading/tasks.md rename to openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/tasks.md index fb59c887..ba39e7d2 100644 --- a/openspec/changes/governance-04-deterministic-agent-governance-loading/tasks.md +++ b/openspec/changes/archive/2026-04-16-governance-04-deterministic-agent-governance-loading/tasks.md @@ -7,7 +7,7 @@ - [x] 1.3 In the worktree: `hatch env create` and `hatch run dev-deps` so `specfact` CLI is available for code-review dogfood tasks. - [x] 1.4 Pre-flight from worktree: `hatch run smart-test-status` and `hatch run contract-test-status` (or full quick sanity per AGENTS.md if those targets differ). - [x] 1.5 Run `openspec validate governance-04-deterministic-agent-governance-loading --strict` and capture output in `CHANGE_VALIDATION.md`; fix artifact issues until green. -- [ ] 1.6 After PR merges: `git worktree remove`, `git branch -d`, `git worktree prune` for the feature branch; remove worktree-local `.venv` if unused. +- [x] 1.6 After PR merges: `git worktree remove`, `git branch -d`, `git worktree prune` for the feature branch; remove worktree-local `.venv` if unused. ## 2. Spec-first and test-first preparation @@ -28,12 +28,12 @@ ## 4. Validation and documentation - [x] 4.1 Run quality gates from the worktree until green: `hatch run format`, `hatch run type-check`, `hatch run lint`, `hatch run yaml-lint`, `hatch run contract-test`, `hatch run smart-test`, `hatch run test` (add signature verify if any `module-package.yaml` / registry payload changes). -- [ ] 4.2 **SpecFact code review JSON**: ensure `.specfact/code-review.json` exists and is fresh per `openspec/config.yaml` rules; remediate all findings or document a rare justified exception in the proposal; record commands and timestamp in `TDD_EVIDENCE.md`. +- [x] 4.2 **SpecFact code review JSON**: ensure `.specfact/code-review.json` exists and is fresh per `openspec/config.yaml` rules; remediate all findings or document a rare justified exception in the proposal; record commands and timestamp in `TDD_EVIDENCE.md`. - [x] 4.3 If contributor-facing docs under `docs/` must mention the new layout (e.g. onboarding, nav, frontmatter schema), update them without breaking Jekyll front matter or `documentation-url-contract.md` permalinks. - [x] 4.4 Re-run `openspec validate governance-04-deterministic-agent-governance-loading --strict` and update `CHANGE_VALIDATION.md`. ## 5. Delivery - [x] 5.1 Refresh `TDD_EVIDENCE.md` with passing-after commands and timestamps. -- [ ] 5.2 Open a PR from `feature/governance-04-deterministic-agent-governance-loading` to `dev` with summary linking modules issue, #163, #494, and #178. -- [ ] 5.3 After merge, run `openspec archive governance-04-deterministic-agent-governance-loading` from repo root (no manual folder moves) and confirm **openspec/CHANGE_ORDER.md** reflects archived status. +- [x] 5.2 Open a PR from `feature/governance-04-deterministic-agent-governance-loading` to `dev` with summary linking modules issue, #163, #494, and #178. +- [x] 5.3 After merge, run `openspec archive governance-04-deterministic-agent-governance-loading` from repo root (no manual folder moves) and confirm **openspec/CHANGE_ORDER.md** reflects archived status. diff --git a/openspec/changes/marketplace-06-ci-module-signing/.openspec.yaml b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/.openspec.yaml similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/.openspec.yaml rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/.openspec.yaml diff --git a/openspec/changes/marketplace-06-ci-module-signing/TDD_EVIDENCE.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/TDD_EVIDENCE.md similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/TDD_EVIDENCE.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/TDD_EVIDENCE.md diff --git a/openspec/changes/marketplace-06-ci-module-signing/design.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/design.md similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/design.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/design.md diff --git a/openspec/changes/marketplace-06-ci-module-signing/proposal.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/proposal.md similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/proposal.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/proposal.md diff --git a/openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/specs/ci-integration/spec.md similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/specs/ci-integration/spec.md diff --git a/openspec/changes/marketplace-06-ci-module-signing/specs/ci-module-signing-on-approval/spec.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/specs/ci-module-signing-on-approval/spec.md similarity index 100% rename from openspec/changes/marketplace-06-ci-module-signing/specs/ci-module-signing-on-approval/spec.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/specs/ci-module-signing-on-approval/spec.md diff --git a/openspec/changes/marketplace-06-ci-module-signing/tasks.md b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/tasks.md similarity index 96% rename from openspec/changes/marketplace-06-ci-module-signing/tasks.md rename to openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/tasks.md index 5a7fff30..93cc1279 100644 --- a/openspec/changes/marketplace-06-ci-module-signing/tasks.md +++ b/openspec/changes/archive/2026-04-16-marketplace-06-ci-module-signing/tasks.md @@ -5,7 +5,7 @@ - [x] 1.1 Create `feature/marketplace-06-ci-module-signing` in a dedicated worktree from `origin/dev`; run pre-flight status checks. - [x] 1.2 ~~Create a GitHub User Story issue~~ Issue created: [specfact-cli-modules#185](https://github.com/nold-ai/specfact-cli-modules/issues/185); `proposal.md` Source Tracking updated. Paired core issue: [specfact-cli#500](https://github.com/nold-ai/specfact-cli/issues/500). *(done)* -- [ ] 1.3 Confirm `SPECFACT_MODULE_PRIVATE_SIGN_KEY` and `SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE` +- [x] 1.3 Confirm `SPECFACT_MODULE_PRIVATE_SIGN_KEY` and `SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE` are set as repository secrets in `specfact-cli-modules` (should already be present via publish-modules.yml). *(human)* @@ -73,5 +73,5 @@ require `--require-signature`). PR: [specfact-cli-modules#188](https://github.com/nold-ai/specfact-cli-modules/pull/188). - [x] 6.2 Link the PR to the GitHub issue created in 1.2 and to the paired specfact-cli PR. *(Closes #185 in PR body; link specfact-cli PR manually when it exists.)* -- [ ] 6.3 After merge: remove the worktree, delete the local branch, run `git worktree prune`. -- [ ] 6.4 Record cleanup completion in `TDD_EVIDENCE.md`. +- [x] 6.3 After merge: remove the worktree, delete the local branch, run `git worktree prune`. +- [x] 6.4 Record cleanup completion in `TDD_EVIDENCE.md`. diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.md b/openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.md index ec711534..a5ea8e80 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.md @@ -9,6 +9,7 @@ ## OpenSpec - `openspec validate docs-15-code-review-validation-guardrails --strict` — passing. +- 2026-04-16: Spec delta headers normalized (`## MODIFIED Requirements`, `### Requirement:`, `#### Scenario:`) under `openspec/changes/docs-15-code-review-validation-guardrails/specs/**/spec.md` so strict validation and future `openspec archive` succeed; `openspec validate docs-15-code-review-validation-guardrails --strict` — passing. ## Format / lint @@ -17,3 +18,16 @@ ## SpecFact code review - `hatch run specfact code review run --json --out .specfact/code-review.json --scope changed` — passing (2026-04-15); evidence at `.specfact/code-review.json`. + +## Code Review run guide examples (2026-04-16) + +- Expanded `docs/bundles/code-review/run.md` with worked examples (auto-scope, `--path`, shadow + `--json` + `--out`, `--focus`, positional files, `--level`, `--bug-hunt`, noise/interactive); clarified Python-only scope vs Markdown docs validation (`scripts/check-docs-commands.py`). Cross-links on bundle overview fixed to root-absolute routes. `docs/modules/code-review.md` points readers to the bundle guide for recipes. +- `hatch run pytest tests/unit/docs/test_code_review_docs_parity.py …` and `python scripts/check-docs-commands.py` — passing. + +## Follow-up: bundle permalink vs. `..` links (2026-04-16) + +- `hatch run pytest tests/unit/scripts/test_docs_site_validation_link_agreement.py tests/unit/docs/test_docs_review.py::test_authored_internal_docs_links_resolve_to_published_docs_targets -q` — passing. +- `python scripts/check-docs-commands.py` — passing (no findings). +- `hatch run format`, `hatch run lint`, `hatch run type-check` — passing. +- `hatch run contract-test` — passing (624 tests). +- `hatch run specfact code review run --json --out /tmp/code-review-docs15.json scripts/docs_site_validation.py tests/unit/scripts/test_docs_site_validation_link_agreement.py` — **PASS** (`overall_verdict` PASS, `ci_exit_code` 0; report outside gitignored `.specfact/` for inspection). diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md b/openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md index 556ca3b0..71c0dd67 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md @@ -1,26 +1,28 @@ -# ADDED Requirements +# Bundle overview pages -## Requirement: Bundle overview links SHALL resolve as published URLs +## MODIFIED Requirements + +### Requirement: Bundle overview links SHALL resolve as published URLs Bundle overview pages SHALL use links that resolve correctly from the published overview permalink route, including "See also", prerequisite, deep-dive, and related-bundle links. -### Scenario: Code Review overview links to run page +#### Scenario: Code Review overview links to run page - **WHEN** the Code Review overview page is published at `/bundles/code-review/overview/` - **THEN** its "Code review run" link resolves to `/bundles/code-review/run/` - **AND** the link does not resolve to `/bundles/code-review/overview/run/` -### Scenario: Cross-bundle overview link resolves +#### Scenario: Cross-bundle overview link resolves - **WHEN** a bundle overview page links to another bundle overview page - **THEN** the link resolves to the target bundle's canonical published overview route - **AND** docs validation fails if the link resolves to a route nested under the source overview page -## Requirement: Bundle overview-related links SHALL be covered by docs validation tests +### Requirement: Bundle overview-related links SHALL be covered by docs validation tests The bundle overview docs test suite SHALL include coverage that fails when any overview page contains a body link that is valid by source-file path but broken under published permalink semantics. -### Scenario: Source-valid but published-broken link is rejected +#### Scenario: Source-valid but published-broken link is rejected - **WHEN** an overview page links to a sibling page using a source-file-relative shorthand that would publish below the overview permalink - **THEN** the overview link test reports the generated public route mismatch diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.md b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.md index ecb7a834..62cddbcf 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.md @@ -1,54 +1,67 @@ -# ADDED Requirements +# Modules docs command validation -## Requirement: Docs validation SHALL validate published-route body links +## MODIFIED Requirements + +### Requirement: Docs validation SHALL validate published-route body links The modules docs validation command SHALL validate internal links in authored page bodies using the page's published permalink route as the URL base, and SHALL fail when a link resolves to a route that is not backed by a published page or an accepted redirect route. -### Scenario: Overview relative link fails under published route semantics +#### Scenario: Overview relative link fails under published route semantics - **WHEN** a page with permalink `/bundles/code-review/overview/` contains a body link `run/` - **THEN** docs validation resolves the link as `/bundles/code-review/overview/run/` - **AND** docs validation reports a `published-link` finding when that route is not published or redirected - **AND** the validation command exits non-zero -### Scenario: Published-route-safe link passes +#### Scenario: Published-route-safe link passes - **WHEN** a page with permalink `/bundles/code-review/overview/` links to `/bundles/code-review/run/` - **THEN** docs validation resolves the link to the published Code Review run page - **AND** no `published-link` finding is emitted for that link -## Requirement: Docs validation SHALL reject incomplete published page front matter +### Requirement: Docs validation SHALL reject incomplete published page front matter The modules docs validation command SHALL reject published Markdown pages whose front matter is missing required route and display metadata, including `layout`, `title`, and `permalink`, unless the page has an explicit documented exemption recognized by the validator. -### Scenario: Redirect page missing title fails +#### Scenario: Redirect page missing title fails - **WHEN** a published Markdown redirect page has `layout` and `permalink` but no `title` - **THEN** docs validation reports a `frontmatter` finding for the missing `title` - **AND** the validation command exits non-zero -### Scenario: Complete published page passes front matter validation +#### Scenario: Complete published page passes front matter validation - **WHEN** a published Markdown page defines `layout`, `title`, and `permalink` - **THEN** docs validation accepts the page front matter - **AND** no `frontmatter` finding is emitted for that page -## Requirement: Docs validation SHALL expose stable finding categories +### Requirement: Docs validation SHALL expose stable finding categories The modules docs validation command SHALL emit stable category names for each class of documentation defect so CI logs, pre-commit output, and tests can assert category coverage without matching brittle prose. -### Scenario: Multiple docs defect categories are reported together +#### Scenario: Multiple docs defect categories are reported together - **WHEN** docs validation finds an unknown command example, a broken published route link, and incomplete front matter - **THEN** the output includes `command`, `published-link`, and `frontmatter` categories - **AND** the validation command exits non-zero after reporting all discovered docs findings -## Requirement: Docs validation SHALL detect docs build dependency drift +### Requirement: Docs validation SHALL detect docs build dependency drift The modules docs validation workflow SHALL include a docs build dependency health check that fails when the checked-in Jekyll dependency lock cannot be installed for the docs site. -### Scenario: Stale Gemfile lock fails docs dependency validation +#### Scenario: Stale Gemfile lock fails docs dependency validation - **WHEN** the docs dependency install command cannot resolve a locked gem version from the configured sources - **THEN** the docs workflow reports a `docs-build-dependency` failure - **AND** Pages publication does not proceed as healthy + +### Requirement: Bundle permalink pages SHALL validate parent-segment links against browser routes + +For pages whose canonical published route is under `/bundles/`, docs validation SHALL treat Markdown links whose path contains parent-directory segments (`..`) as unsafe unless the filesystem-resolved target file matches the target resolved from the page permalink using browser URL rules. + +#### Scenario: Deep bundle overview rejects filesystem-only match for `../../` links + +- **WHEN** a bundle overview page is published under `/bundles//overview/` (or another deep `/bundles/` permalink) +- **AND** its body uses a `../` or `../../` link that reaches a markdown file on disk but resolves to a different or missing public route +- **THEN** docs validation reports a `published-link` finding (missing route or route mismatch) +- **AND** the validation command exits non-zero diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.md b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.md index c067c0fc..1e305958 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.md @@ -1,26 +1,28 @@ -# ADDED Requirements +# Modules docs publishing -## Requirement: Docs publishing SHALL validate generated-site readiness before deploy +## MODIFIED Requirements + +### Requirement: Docs publishing SHALL validate generated-site readiness before deploy The docs publishing workflow SHALL run docs dependency installation, Jekyll build, and generated-site validation before uploading or deploying the Pages artifact. -### Scenario: Dependency install failure blocks Pages artifact +#### Scenario: Dependency install failure blocks Pages artifact - **WHEN** `bundle install` fails for the docs site - **THEN** the docs publishing workflow fails before `jekyll build` - **AND** no Pages artifact is uploaded from that run -### Scenario: Generated site contains broken internal link +#### Scenario: Generated site contains broken internal link - **WHEN** the generated `_site` HTML contains an internal `modules.specfact.io` link whose route is not present in the generated site or redirect set - **THEN** generated-site validation reports the broken route - **AND** the docs publishing workflow fails before deployment -## Requirement: Docs review CI SHALL run the same deterministic docs validators as local checks +### Requirement: Docs review CI SHALL run the same deterministic docs validators as local checks The docs review workflow SHALL run the deterministic docs validators used by local pre-commit, plus the docs unit tests, so PR and local validation enforce the same defect categories. -### Scenario: Docs-only pull request has broken published link +#### Scenario: Docs-only pull request has broken published link - **WHEN** a pull request changes only Markdown files under `docs/` - **THEN** the docs review workflow runs published-route link validation diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md index 8d74260f..e467772a 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md @@ -1,26 +1,28 @@ -# ADDED Requirements +# Modules pre-commit quality parity -## Requirement: Docs-only pre-commit changes SHALL run docs validation before safe bypass +## MODIFIED Requirements + +### Requirement: Docs-only pre-commit changes SHALL run docs validation before safe bypass The modules repo pre-commit helper SHALL run deterministic docs validation for staged docs-only changes before skipping code-specific review and contract-test stages. -### Scenario: Docs-only commit with broken link fails pre-commit +#### Scenario: Docs-only commit with broken link fails pre-commit - **WHEN** only docs files are staged and one staged docs page introduces a broken published-route link - **THEN** pre-commit runs docs validation - **AND** pre-commit fails before reporting the change as safe -### Scenario: Docs-only commit with valid docs skips code-specific checks +#### Scenario: Docs-only commit with valid docs skips code-specific checks - **WHEN** only docs files are staged and docs validation passes - **THEN** pre-commit may skip code review and contract-test stages - **AND** pre-commit reports that docs validation passed before applying the safe-change bypass -## Requirement: Pre-commit and CI docs gates SHALL share validation categories +### Requirement: Pre-commit and CI docs gates SHALL share validation categories The local pre-commit docs gate and CI docs review workflow SHALL report the same docs validation categories for matching defects. -### Scenario: Same broken docs route reports same category locally and in CI +#### Scenario: Same broken docs route reports same category locally and in CI - **WHEN** a docs change introduces a broken generated public route - **THEN** local pre-commit reports a `published-link` finding diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md b/openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md index a92c3afd..e0fb1744 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md @@ -1,26 +1,28 @@ -# ADDED Requirements +# Review run command (docs) -## Requirement: Code Review run docs SHALL cover the public option surface +## MODIFIED Requirements + +### Requirement: Code Review run docs SHALL cover the public option surface The Code Review run documentation SHALL describe every supported public `specfact code review run` option that affects targeting, output, exit behavior, analysis depth, or filtering. -### Scenario: Newly added review options are documented +#### Scenario: Newly added review options are documented - **WHEN** the `specfact code review run` Typer command exposes `--bug-hunt`, `--mode`, `--focus`, and `--level` - **THEN** the Code Review run guide documents those options in its key option table or equivalent option section - **AND** docs validation fails if any of those public options are missing from the run guide -### Scenario: Invalid option combinations are documented +#### Scenario: Invalid option combinations are documented - **WHEN** the command rejects combinations such as positional files with `--scope` or `--path`, or `--focus` with `--include-tests` - **THEN** the Code Review docs describe the invalid combination behavior - **AND** the docs include a user-facing alternative for the supported targeting style aligned with the public **`run`** signature (**`files: list[Path]`**): pass explicit **positional files** (file paths) for a fixed review set, or use **`--scope`** / **`--path`** (without positional files) to auto-discover targets from the repo -## Requirement: Code Review docs SHALL stay aligned with review behavior +### Requirement: Code Review docs SHALL stay aligned with review behavior The Code Review docs SHALL describe current review run behavior for JSON output, shadow/enforce mode, progress output, focus filtering, severity filtering, bug-hunt budgets, and test inclusion semantics. -### Scenario: Docs parity check detects missing behavior section +#### Scenario: Docs parity check detects missing behavior section - **WHEN** the command implementation includes a public behavior that affects output, exit code, target selection, or analysis cost - **THEN** docs parity validation checks that the behavior is represented in the Code Review run docs diff --git a/openspec/specs/agent-governance-loading/spec.md b/openspec/specs/agent-governance-loading/spec.md new file mode 100644 index 00000000..05767591 --- /dev/null +++ b/openspec/specs/agent-governance-loading/spec.md @@ -0,0 +1,117 @@ +# agent-governance-loading Specification + +## Purpose +TBD - created by archiving change governance-04-deterministic-agent-governance-loading. Update Purpose after archive. +## Requirements +### Requirement: Compact AGENTS bootstrap contract + +The repository SHALL keep `AGENTS.md` as the mandatory bootstrap governance surface, but it SHALL remain compact and SHALL delegate long-form operational detail to canonical rule artifacts. + +#### Scenario: Session bootstrap reads compact governance contract + +- **WHEN** an agent starts work in the repository +- **THEN** it reads `AGENTS.md` first +- **AND** `AGENTS.md` defines a mandatory bootstrap sequence rather than embedding the full long-form governance corpus +- **AND** the bootstrap sequence requires loading `docs/agent-rules/INDEX.md` +- **AND** the bootstrap sequence requires loading the canonical non-negotiable checklist before code implementation work + +#### Scenario: AGENTS stays compact while preserving enforcement + +- **WHEN** repository governance is updated +- **THEN** `AGENTS.md` SHALL retain only the bootstrap contract, invariant governance rules, and canonical references needed for startup +- **AND** detailed workflow, validation, or finalization guidance SHALL live in dedicated rule artifacts rather than being duplicated inline + +### Requirement: Deterministic rule index and loading semantics + +The repository SHALL provide a canonical rule index that deterministically decides which governance rule files must be loaded for a given task. + +#### Scenario: Always-load rule subset + +- **WHEN** an agent loads the governance rule index +- **THEN** the index SHALL identify a mandatory always-load subset +- **AND** that subset SHALL include the non-negotiable checklist +- **AND** the index SHALL define the order in which always-load rules are processed + +#### Scenario: Applicability-based rule loading + +- **WHEN** a task involves worktree management, OpenSpec change selection, GitHub issue coordination, TDD gating, quality verification, module signature or registry release work, or finalization +- **THEN** the index SHALL map those task signals to specific `docs/agent-rules/*.md` files +- **AND** the index SHALL define which rule files are required versus optional for that task class +- **AND** the loading decision SHALL not depend on heuristic file-name guessing alone + +#### Scenario: Deterministic precedence + +- **WHEN** governance instructions overlap across `AGENTS.md`, the rule index, rule files, and change-local artifacts +- **THEN** the repository SHALL define a single precedence order for which instruction wins +- **AND** the precedence order SHALL include explicit handling for direct user override where repository governance permits it + +### Requirement: Governance rule files use machine-readable frontmatter + +Every dedicated governance rule artifact SHALL include machine-readable frontmatter that defines how and when the rule applies. + +#### Scenario: Required frontmatter fields are present + +- **WHEN** a file under `docs/agent-rules/` is intended to govern agent behavior +- **THEN** it SHALL include frontmatter fields for rule identity, applicability, priority, blocking semantics, and stop conditions +- **AND** it SHALL declare whether the file is always loaded +- **AND** it SHALL declare whether user interaction is required when the rule blocks progress + +#### Scenario: Frontmatter drives deterministic behavior + +- **WHEN** an agent evaluates a rule file with frontmatter +- **THEN** it can determine from metadata whether the rule is mandatory for the current task +- **AND** it can determine whether the rule requires a hard stop, read-only continuation, or interactive clarification +- **AND** it does not need to infer those semantics solely from unstructured prose + +### Requirement: Governance must define explicit stop and continue behavior + +The governance system SHALL define explicit blocking behavior for stale changes, concurrency ambiguity, missing metadata, and TDD gate violations. + +#### Scenario: Blocking ambiguity requires user clarification + +- **WHEN** a selected change is stale, superseded, ambiguous, or linked to GitHub work already in progress +- **THEN** the applicable rule SHALL require the agent to stop implementation work +- **AND** the rule SHALL state that the ambiguity must be surfaced to the user for clarification +- **AND** the rule SHALL define whether read-only investigation may continue while implementation remains blocked + +#### Scenario: TDD gate remains non-bypassable in compact governance + +- **WHEN** a task changes behavior in code +- **THEN** the applicable rule SHALL still require spec updates, test creation, failing-test evidence, implementation, and passing evidence in that order +- **AND** compact governance SHALL not weaken or omit that sequence + +### Requirement: Public GitHub work must pass metadata completeness checks + +The governance system SHALL define explicit readiness checks for linked GitHub change issues before implementation proceeds for public repository work. + +#### Scenario: Parent and dependency metadata must be complete + +- **WHEN** an agent prepares to implement a publicly tracked change with a linked GitHub issue +- **THEN** the applicable governance rules SHALL require verifying the issue's parent relationship, blockers, and blocked-by relationships against current repository GitHub reality +- **AND** the parent lookup SHALL use the local hierarchy cache first and refresh the cache when repository-defined freshness rules require it +- **AND** implementation SHALL not proceed if the required parent or dependency metadata is missing or ambiguous + +#### Scenario: Labels and project assignment must be complete + +- **WHEN** an agent prepares to implement a publicly tracked change with a linked GitHub issue +- **THEN** the applicable governance rules SHALL require verifying that the issue has the required labels and project assignment for repository workflow completeness +- **AND** implementation SHALL not proceed until that metadata is complete or the user explicitly directs an allowed exception path + +#### Scenario: Live GitHub issue state can block implementation + +- **WHEN** an agent prepares to implement a publicly tracked change with a linked GitHub issue +- **AND** the issue is already marked `in progress` +- **THEN** the governance rules SHALL treat that state as a concurrency ambiguity +- **AND** the agent SHALL stop implementation work and ask the user to clarify whether the change is already being worked in another session +- **AND** the rules SHALL define whether only read-only investigation may continue while implementation remains blocked + +### Requirement: Canonical aliases prevent instruction drift + +Repository instruction surfaces other than `AGENTS.md` SHALL reference the canonical governance rule system instead of embedding duplicate long-form policy text. + +#### Scenario: Alias instruction surfaces stay synchronized + +- **WHEN** a contributor reads another repository instruction surface such as `CLAUDE.md`, `.cursorrules`, `.github/copilot-instructions.md`, or generated IDE guidance +- **THEN** the surface SHALL reference the canonical rule system for governance semantics +- **AND** it SHALL avoid copying long-form governance content that could drift from the canonical source + diff --git a/openspec/specs/ci-integration/spec.md b/openspec/specs/ci-integration/spec.md new file mode 100644 index 00000000..f1109782 --- /dev/null +++ b/openspec/specs/ci-integration/spec.md @@ -0,0 +1,55 @@ +# ci-integration Specification + +## Purpose +TBD - created by archiving change marketplace-06-ci-module-signing. Update Purpose after archive. +## Requirements +### Requirement: pr-orchestrator skips signature requirement for dev-targeting events + +The `verify-module-signatures` job in `pr-orchestrator.yml` SHALL NOT enforce `--require-signature` +for pull requests or pushes targeting `dev`; it SHALL enforce `--require-signature` only for +`main`-targeting events. + +#### Scenario: Feature-to-dev PR with unsigned package manifests + +- **WHEN** a pull request targets `dev` +- **AND** the PR contains package manifest changes with checksum-only integrity blocks +- **THEN** the `verify-module-signatures` job SHALL pass without `--require-signature` +- **AND** all downstream jobs (`quality`, `contract-tests`, etc.) SHALL not be blocked + +#### Scenario: Dev-to-main PR with unsigned manifests (before approval) + +- **WHEN** a pull request targets `main` +- **AND** one or more `packages/*/module-package.yaml` files lack a valid signature +- **THEN** the `verify-module-signatures` job SHALL fail with `--require-signature` +- **AND** the PR SHALL be blocked from merging + +#### Scenario: Dev-to-main PR after CI signing commit + +- **WHEN** a pull request targets `main` +- **AND** the CI `sign-modules-on-approval` workflow has committed signed manifests to the PR branch +- **THEN** the `verify-module-signatures` job SHALL pass with `--require-signature` +- **AND** the PR SHALL be unblocked (subject to other required checks) + +#### Scenario: Push to main post-merge + +- **WHEN** a commit is pushed to `main` (post-merge) +- **THEN** the `verify-module-signatures` job SHALL run with `--require-signature` +- **AND** fail if any `packages/*/module-package.yaml` lacks a valid signature + +### Requirement: pre-commit verify mirrors pr-orchestrator signature policy + +The repository pre-commit hook that runs `verify-modules-signature.py` SHALL apply the same branch rule as the `verify-module-signatures` CI job: omit `--require-signature` unless the integration target is `main` (local branch `main`, or `GITHUB_BASE_REF=main` on pull request events in Actions). + +#### Scenario: Commit on a feature branch without a signing key + +- **WHEN** a developer commits on a branch other than `main` +- **AND** manifests satisfy checksum and version-bump policy but lack a valid signature +- **THEN** the pre-commit signature hook SHALL pass (no `--require-signature`) +- **AND** the developer SHALL remain unblocked until a `main`-targeting flow enforces signatures in CI + +#### Scenario: Commit on main requires signatures + +- **WHEN** a developer commits on branch `main` +- **AND** any `packages/*/module-package.yaml` lacks a valid signature under `--require-signature` +- **THEN** the pre-commit signature hook SHALL fail + diff --git a/openspec/specs/ci-module-signing-on-approval/spec.md b/openspec/specs/ci-module-signing-on-approval/spec.md new file mode 100644 index 00000000..1448dc9e --- /dev/null +++ b/openspec/specs/ci-module-signing-on-approval/spec.md @@ -0,0 +1,68 @@ +# ci-module-signing-on-approval Specification + +## Purpose +TBD - created by archiving change marketplace-06-ci-module-signing. Update Purpose after archive. +## Requirements +### Requirement: Sign packages manifests on PR approval + +The system SHALL automatically sign changed `packages/*/module-package.yaml` manifests using CI +secrets when a pull request targeting `dev` or `main` is approved, and SHALL commit the signed +manifests back to the PR branch. + +#### Scenario: PR to dev approved with package module changes + +- **WHEN** a pull request targeting `dev` is approved by a reviewer +- **AND** the PR contains changes to one or more files under `packages/` +- **THEN** the CI signing workflow SHALL discover all `packages/*/module-package.yaml` manifests + whose payload changed on the PR branch since the merge-base with `origin/dev` (not merely + divergent from the moving `origin/dev` tip) +- **AND** SHALL sign them using `SPECFACT_MODULE_PRIVATE_SIGN_KEY` and + `SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE` +- **AND** SHALL commit the updated manifests back to the PR branch + +#### Scenario: PR to main approved with package module changes + +- **WHEN** a pull request targeting `main` is approved +- **AND** the PR contains changes to one or more files under `packages/` +- **THEN** the CI signing workflow SHALL sign all changed manifests relative to the merge-base + between the PR head and `origin/main` +- **AND** SHALL commit the signed manifests back to the PR branch before merge + +#### Scenario: PR approved with no package changes + +- **WHEN** a pull request is approved +- **AND** no files under `packages/` have changed relative to the base branch +- **THEN** the CI signing workflow SHALL exit cleanly with no commit + +#### Scenario: Missing signing secret + +- **WHEN** the signing workflow triggers on approval +- **AND** `SPECFACT_MODULE_PRIVATE_SIGN_KEY` is empty or unset, or `SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE` is empty or unset +- **THEN** the workflow SHALL fail before checkout/signing with a clear error naming which secret(s) are missing +- **AND** SHALL NOT commit partial changes + +#### Scenario: Fork PR is out of scope for automated signing + +- **WHEN** a pull request targets `dev` or `main` but the head branch lives in a fork + (`head.repo` differs from the base repository) +- **THEN** the signing workflow SHALL NOT run (the default `GITHUB_TOKEN` cannot push to the + contributor fork; maintainers sign or merge via same-repo branches instead) + +### Requirement: Manifest discovery covers packages directory + +The signing workflow SHALL discover module manifests from the `packages/` directory tree, not only +from `src/specfact_cli/modules/` or `modules/` (which do not exist in this repository). + +#### Scenario: Sign only changed packages manifests + +- **WHEN** the signing workflow runs with changes across multiple packages +- **AND** only a subset of packages have payload changes +- **THEN** only the changed `packages/*/module-package.yaml` files SHALL be signed and committed +- **AND** unchanged package manifests SHALL NOT be modified + +#### Scenario: Sign workflow produces idempotent output + +- **WHEN** the signing workflow runs twice on the same package payload +- **THEN** the resulting `integrity:` block SHALL be byte-for-byte identical +- **AND** the second run SHALL produce no git diff and SHALL skip the commit + diff --git a/openspec/specs/code-review-bug-finding/spec.md b/openspec/specs/code-review-bug-finding/spec.md new file mode 100644 index 00000000..28f5b13e --- /dev/null +++ b/openspec/specs/code-review-bug-finding/spec.md @@ -0,0 +1,57 @@ +# code-review-bug-finding Specification + +## Purpose +TBD - created by archiving change code-review-bug-finding-and-sidecar-venv-fix. Update Purpose after archive. +## Requirements +### Requirement: Semgrep bug-finding rules pass + +The system SHALL run a second semgrep pass using a `bugs.yaml` config alongside +the existing `clean_code.yaml` pass. The `bugs.yaml` config SHALL cover Python +security and correctness patterns. When `bugs.yaml` is absent the pass SHALL be +silently skipped without error. + +#### Scenario: bugs.yaml present — security findings emitted + +- **WHEN** `.semgrep/bugs.yaml` exists in the bundle +- **AND** `run_semgrep_bugs` is called on Python files matching a bug rule +- **THEN** `ReviewFinding` records are returned with `category="security"` or `category="clean_code"` +- **AND** findings reference the matched rule id from `bugs.yaml` + +#### Scenario: bugs.yaml absent — pass is a no-op + +- **WHEN** no `.semgrep/bugs.yaml` file is discoverable +- **AND** `run_semgrep_bugs` is called +- **THEN** no finding is returned for the missing bugs pass +- **AND** no exception propagates to the caller + +#### Scenario: bugs.yaml findings are included in the JSON report + +- **WHEN** `specfact code review run --json` is executed on a file matching a bug rule +- **THEN** the output JSON contains findings from both the clean-code and bug-finding passes +- **AND** `run_review` wires `run_semgrep_bugs` alongside the existing `run_semgrep` pass + +### Requirement: CrossHair bug-hunt mode + +The system SHALL support a `--bug-hunt` flag on `specfact code review run` that +increases the CrossHair per-path timeout to 10 seconds and the total CrossHair +timeout to 120 seconds. Without `--bug-hunt` the existing timeouts SHALL remain +unchanged. + +#### Scenario: --bug-hunt increases CrossHair timeouts + +- **WHEN** `specfact code review run --bug-hunt --json ` is executed +- **THEN** CrossHair runs with `--per_path_timeout 10` +- **AND** the total CrossHair subprocess timeout is 120 seconds + +#### Scenario: Default run uses original CrossHair timeouts + +- **WHEN** `specfact code review run --json ` is executed without `--bug-hunt` +- **THEN** CrossHair runs with `--per_path_timeout 2` +- **AND** the total CrossHair subprocess timeout is 30 seconds + +#### Scenario: --bug-hunt is composable with --scope and --out + +- **WHEN** `specfact code review run --bug-hunt --scope full --json --out report.json` is executed +- **THEN** the command completes without error +- **AND** the output JSON is written to `report.json` + diff --git a/openspec/specs/code-review-tool-dependencies/spec.md b/openspec/specs/code-review-tool-dependencies/spec.md new file mode 100644 index 00000000..04b4ac46 --- /dev/null +++ b/openspec/specs/code-review-tool-dependencies/spec.md @@ -0,0 +1,70 @@ +# code-review-tool-dependencies Specification + +## Purpose +TBD - created by archiving change code-review-bug-finding-and-sidecar-venv-fix. Update Purpose after archive. +## Requirements +### Requirement: pip_dependencies cover all external review tools + +The `specfact-code-review` bundle manifest (`packages/specfact-code-review/module-package.yaml`) SHALL declare, under `pip_dependencies`, every PyPI distribution required so that **all external** tools invoked by the default `run_review` pipeline (including the TDD gate and CrossHair) can run in a normal bundle install. + +#### Scenario: Manifest includes core CLI tools + +- **WHEN** a maintainer inspects `module-package.yaml` +- **THEN** `pip_dependencies` includes packages that provide the `ruff`, `radon`, `semgrep`, `basedpyright`, `pylint`, and `crosshair` CLIs on `PATH` after install +- **AND** `pytest` and `pytest-cov` are listed for the targeted test / coverage subprocess + +#### Scenario: New runner adds a new external executable + +- **WHEN** a new subprocess-backed review step is added to the pipeline +- **THEN** the change updates `pip_dependencies` and the canonical tool map (see design D9) in the same delivery + +### Requirement: Runtime detection skips missing tools with a clear tool_error + +Before each external tool subprocess runs, the implementation SHALL verify the tool is available. If the executable is not found on `PATH` (or pytest cannot be launched as today’s gate requires), the implementation SHALL **not** invoke that tool and SHALL return **exactly one** `ReviewFinding` per skipped tool. + +#### Scenario: Missing Ruff executable produces a skip finding + +- **GIVEN** `ruff` is not on `PATH` +- **WHEN** `run_ruff` is executed for a non-empty file list +- **THEN** no `ruff` subprocess is started +- **AND** exactly one finding is returned with `category="tool_error"` and `tool="ruff"` +- **AND** the message states that review checks for `ruff` were **skipped** because it is **not installed** or unavailable, and names the pip package to install (e.g. `ruff`) + +#### Scenario: Missing Semgrep does not raise + +- **GIVEN** `semgrep` is not on `PATH` +- **WHEN** the semgrep review step runs +- **THEN** the step returns a single skip finding as above for `semgrep` +- **AND** no uncaught exception propagates + +#### Scenario: AST clean-code pass requires no extra CLI dependency + +- **WHEN** `run_ast_clean_code` runs +- **THEN** it does not depend on a `pip_dependencies` CLI entry (stdlib / in-package Python only) + +#### Scenario: TDD gate reports pytest unavailable clearly + +- **GIVEN** `pytest` (or coverage support) is missing such that the targeted test subprocess cannot run +- **WHEN** `_evaluate_tdd_gate` would run tests +- **THEN** findings include a `tool_error` for `pytest` (or the agreed tool id) with a skip / not-installed style message referencing `pytest` and/or `pytest-cov` + +### Requirement: No misleading errors when the binary is absent + +If a tool executable is missing, runners SHALL NOT surface generic failures such as “Unable to parse JSON output” that imply the tool ran but returned bad data. + +#### Scenario: Ruff missing is not reported as a parse failure + +- **GIVEN** `ruff` is absent +- **WHEN** `run_ruff` completes +- **THEN** the user-visible message indicates the tool was **skipped** / **not installed**, not a JSON parse error from Ruff output + +### Requirement: Automated guard for manifest vs tool map + +The repository SHALL include an automated check (unit test or small validation script invoked by the normal test suite) that fails if `module-package.yaml` `pip_dependencies` omits any package from the canonical tool → pip map for the default pipeline. + +#### Scenario: Drift fails CI + +- **GIVEN** the canonical map lists a pip package for an active runner +- **WHEN** that package is removed from `pip_dependencies` without updating the map +- **THEN** `hatch run test` (or the chosen gate) fails with a clear assertion message + diff --git a/openspec/specs/contract-runner/spec.md b/openspec/specs/contract-runner/spec.md index 2b0da54c..56f31ec5 100644 --- a/openspec/specs/contract-runner/spec.md +++ b/openspec/specs/contract-runner/spec.md @@ -5,17 +5,28 @@ TBD - created by archiving change code-review-04-contract-test-runners. Update P ## Requirements ### Requirement: icontract Decorator AST Scan and CrossHair Fast Pass -The system SHALL AST-scan changed Python files for public functions missing -`@require` / `@ensure` decorators, and run CrossHair with a 2-second per-path timeout -for counterexample discovery. +The system SHALL AST-scan reviewed Python files for public functions missing +`@require` / `@ensure` decorators, and run CrossHair with a configurable +per-path timeout for counterexample discovery. When no icontract usage is +detected in the reviewed files' package/repo scan roots, `MISSING_ICONTRACT` findings SHALL be +suppressed entirely for that run. -#### Scenario: Public function without icontract decorators produces a contracts finding +#### Scenario: Public function without icontract decorators produces a contracts finding when icontract is in use - **GIVEN** a Python file with a public function lacking icontract decorators +- **AND** at least one file in the reviewed batch's package/repo scan roots imports from `icontract` - **WHEN** `run_contract_check(files=[...])` is called - **THEN** a `ReviewFinding` is returned with `category="contracts"` and `severity="warning"` +#### Scenario: MISSING_ICONTRACT suppressed when no icontract usage detected + +- **GIVEN** the package/repo scan roots for the reviewed Python files contain no `from icontract import` or + `import icontract` statements +- **WHEN** `run_contract_check(files=[...])` is called +- **THEN** no `MISSING_ICONTRACT` findings are returned +- **AND** CrossHair still runs on the files + #### Scenario: Decorated public function produces no contracts finding - **GIVEN** a Python file with a public function decorated with both `@require` and @@ -45,3 +56,10 @@ for counterexample discovery. - **AND** no exception propagates to the caller - **AND** CrossHair unavailability does not produce a blocking finding +#### Scenario: Bug-hunt mode uses extended CrossHair timeouts + +- **GIVEN** `run_contract_check(files=[...], bug_hunt=True)` is called +- **WHEN** CrossHair executes +- **THEN** CrossHair runs with `--per_path_timeout 10` +- **AND** the subprocess timeout is 120 seconds + diff --git a/openspec/specs/github-hierarchy-cache/spec.md b/openspec/specs/github-hierarchy-cache/spec.md index 2a13536e..8c8c1032 100644 --- a/openspec/specs/github-hierarchy-cache/spec.md +++ b/openspec/specs/github-hierarchy-cache/spec.md @@ -3,9 +3,7 @@ ## Purpose This capability standardizes a **repo-local, deterministic GitHub Epic/Feature hierarchy cache** for SpecFact modules and paired `specfact-cli` workflows. It defines how contributors sync hierarchy metadata from GitHub into ignored `.specfact/backlog/` files, how fingerprints detect unchanged trees, and how governance (for example `AGENTS.md` and `docs/agent-rules/`) MUST consult that cache before manual GitHub parent lookups. It builds on the archived OpenSpec change **`governance-03-github-hierarchy-cache`**; shipped behavior and drift against `openspec/**/*.md` remain governed by `openspec/CHANGE_ORDER.md` and the modules release checklist. - ## Requirements - ### Requirement: Repository hierarchy cache sync The repository SHALL provide a deterministic sync mechanism that retrieves GitHub Epic and Feature issues for the current repository and writes a local hierarchy cache under ignored `.specfact/backlog/`. @@ -29,7 +27,28 @@ Repository governance instructions SHALL direct contributors and agents to consu #### Scenario: Cache-first governance guidance -- **WHEN** a contributor reads `AGENTS.md` for GitHub issue setup guidance +- **WHEN** a contributor reads `AGENTS.md` or `openspec/config.yaml` for GitHub issue setup guidance - **THEN** the instructions tell them to consult the local hierarchy cache first - **AND** the instructions define when the sync script must be rerun to refresh stale hierarchy metadata - **AND** the instructions state that the cache is local ephemeral state and must not be committed + +#### Scenario: Session bootstrap refreshes missing or stale cache + +- **WHEN** an agent starts a governance-sensitive session that depends on GitHub hierarchy metadata +- **AND** the local hierarchy cache is missing or stale according to repository-defined freshness rules +- **THEN** the bootstrap guidance SHALL require rerunning the hierarchy cache sync script before continuing with issue-parenting or blocker-resolution work +- **AND** the compact governance flow SHALL treat the refresh as part of deterministic startup rather than an optional later reminder + +#### Scenario: State reuse is scoped to the current repository + +- **WHEN** the local hierarchy cache state file contains a matching hierarchy fingerprint +- **BUT** the state metadata belongs to a different repository or does not identify the repository at all +- **THEN** the sync logic SHALL regenerate the markdown cache instead of incorrectly short-circuiting on fingerprint equality alone +- **AND** the resulting cache SHALL render the current repository identity deterministically + +#### Scenario: CLI reports refresh failures clearly + +- **WHEN** the hierarchy cache sync script encounters a runtime or filesystem error during refresh +- **THEN** the CLI entrypoint SHALL emit a clear failure message to stderr +- **AND** it SHALL exit non-zero so bootstrap and governance flows do not silently continue on an untrusted cache state + diff --git a/openspec/specs/review-cli-contracts/spec.md b/openspec/specs/review-cli-contracts/spec.md index 22627efe..8230a8e1 100644 --- a/openspec/specs/review-cli-contracts/spec.md +++ b/openspec/specs/review-cli-contracts/spec.md @@ -38,3 +38,42 @@ The modules repository SHALL keep CLI review scenarios aligned with the expanded - **THEN** advisory checklist findings are represented without changing the base report schema - **AND** unknown clean-code category names are rejected by the scenario contract layer +### Requirement: Review-run CLI scenarios cover enforcement mode, bug-hunt, focus facets, and severity level + +The modules repository SHALL extend `tests/cli-contracts/specfact-code-review-run.scenarios.yaml` so contract tests exercise the new `specfact code review run` flags together with existing scope and JSON output behaviour. + +#### Scenario: Scenarios cover shadow versus enforce exit behaviour + +- **GIVEN** `tests/cli-contracts/specfact-code-review-run.scenarios.yaml` +- **WHEN** it is validated after this change +- **THEN** it includes at least one scenario asserting `--mode shadow` yields process success (exit `0`) while JSON still reports a failing verdict when findings warrant it +- **AND** it includes a control scenario showing `--mode enforce` (or default) preserves non-zero exit on blocking failures + +#### Scenario: Scenarios cover --bug-hunt in shadow and enforce modes + +- **GIVEN** `tests/cli-contracts/specfact-code-review-run.scenarios.yaml` +- **WHEN** it is validated after this change +- **THEN** it includes at least one scenario with `--bug-hunt --mode shadow` +- **AND** it includes at least one scenario with `--bug-hunt --mode enforce` + +#### Scenario: Scenarios cover --focus facets + +- **GIVEN** the same scenario file +- **WHEN** it is validated +- **THEN** it includes coverage for `--focus` union behaviour (e.g. `source` + `docs`) and for `--focus tests` narrowing the file set +- **AND** it includes coverage for `--bug-hunt` composed with `--focus` + +#### Scenario: Scenarios cover --level filtering + +- **GIVEN** the same scenario file +- **WHEN** it is validated +- **THEN** it includes at least one scenario where `--level error` removes warnings from the JSON `findings` list +- **AND** it includes coverage for `--bug-hunt` composed with `--level error` + +#### Scenario: Scenarios cover invalid flag combinations + +- **GIVEN** the same scenario file +- **WHEN** it is validated +- **THEN** it includes an error-path scenario for `--focus` combined with `--include-tests` or `--exclude-tests` +- **AND** `--bug-hunt` remains composable with test-selection flags + diff --git a/openspec/specs/review-run-command/spec.md b/openspec/specs/review-run-command/spec.md index a65bdf8c..09dc3a88 100644 --- a/openspec/specs/review-run-command/spec.md +++ b/openspec/specs/review-run-command/spec.md @@ -82,3 +82,107 @@ The bundle SHALL run the expanded clean-code analysis set as part of the governe - **THEN** the report includes an advisory checklist finding - **AND** the checklist finding does not create a new command surface +### Requirement: --bug-hunt flag on review run command + +The `specfact code review run` command SHALL accept a `--bug-hunt` flag that +enables extended CrossHair timeouts and is composable with all existing flags. + +#### Scenario: --bug-hunt flag accepted without error + +- **GIVEN** `specfact code review run --bug-hunt --json ` is executed +- **WHEN** the command parses its arguments +- **THEN** the command proceeds without a CLI argument error +- **AND** `ReviewRunRequest.bug_hunt` is `True` + +#### Scenario: --bug-hunt flag absent defaults to False + +- **GIVEN** `specfact code review run --json ` is executed without `--bug-hunt` +- **WHEN** the command parses its arguments +- **THEN** `ReviewRunRequest.bug_hunt` is `False` +- **AND** CrossHair uses the standard 2-second per-path timeout + +### Requirement: --mode shadow and --mode enforce + +The `specfact code review run` command SHALL accept `--mode shadow` or `--mode enforce`. + +#### Scenario: Default mode is enforce + +- **GIVEN** `specfact code review run` is invoked without `--mode` +- **WHEN** the command parses its arguments +- **THEN** enforcement behaves as today: `ci_exit_code` reflects blocking findings + +#### Scenario: Shadow mode never returns a failing process exit + +- **GIVEN** a review run that would yield `ci_exit_code == 1` under enforce semantics +- **WHEN** `specfact code review run --mode shadow` completes +- **THEN** the process exit code is `0` +- **AND** `ReviewReport.ci_exit_code` in JSON is `0` +- **AND** `overall_verdict` still reflects the computed verdict (including `FAIL` when applicable) + +#### Scenario: Enforce mode matches legacy exit behaviour + +- **GIVEN** the same findings payload as today for a failing run +- **WHEN** `specfact code review run --mode enforce` completes +- **THEN** process exit and `ci_exit_code` match the pre-change `enforce` default + +#### Scenario: --mode composes with --bug-hunt and --json + +- **WHEN** `specfact code review run --bug-hunt --mode shadow --json --out report.json` is executed +- **THEN** the command parses successfully +- **AND** CrossHair uses bug-hunt timeouts +- **AND** the process exits `0` even if findings would fail under enforce semantics + +### Requirement: Repeatable --focus for source, tests, and docs + +The command SHALL accept repeated `--focus` options with values `source`, `tests`, and `docs`. When at least one `--focus` is present, the reviewed Python file set SHALL be the intersection of the scope-resolved files with the **union** of the selected facets: + +- `tests`: files where `tests` appears in the path’s directory components (same rule as existing test detection). +- `docs`: Python files where `docs` appears in the path’s directory components. +- `source`: Python files that match neither the `tests` nor the `docs` facet. + +#### Scenario: --focus tests restricts to test paths + +- **GIVEN** a repository with both `src/app.py` and `tests/test_app.py` in scope +- **WHEN** `specfact code review run --scope full --focus tests --json` runs +- **THEN** only files under the `tests` facet are analyzed + +#### Scenario: Union of multiple focuses + +- **GIVEN** scope includes `src/a.py`, `tests/t.py`, and `docs/conf.py` +- **WHEN** `specfact code review run --scope full --focus source --focus docs --json` runs +- **THEN** `tests/t.py` is excluded +- **AND** `src/a.py` and `docs/conf.py` are included + +#### Scenario: --focus conflicts with --include-tests + +- **WHEN** `specfact code review run --focus source --include-tests` is parsed +- **THEN** the CLI rejects the combination with a clear error + +#### Scenario: --focus conflicts with --exclude-tests + +- **WHEN** `specfact code review run --focus tests --exclude-tests` is parsed +- **THEN** the CLI rejects the combination with a clear error + +### Requirement: --level error and --level warning + +The command SHALL accept `--level error` or `--level warning` to filter findings **before** scoring and verdict. + +#### Scenario: --level error drops warnings and info + +- **GIVEN** a run that produces both `warning` and `error` severity findings +- **WHEN** `specfact code review run --level error --json` completes +- **THEN** the JSON `findings` list contains only `severity == "error"` items +- **AND** score and verdict are computed from that filtered list + +#### Scenario: --level warning retains errors and warnings + +- **GIVEN** a run that produces `info`, `warning`, and `error` findings +- **WHEN** `specfact code review run --level warning --json` completes +- **THEN** the JSON `findings` list contains no `severity == "info"` items +- **AND** score and verdict are computed from the filtered list + +#### Scenario: Omitted --level keeps all severities + +- **WHEN** `specfact code review run --json` runs without `--level` +- **THEN** all severities appear in output as they do today + diff --git a/openspec/specs/sidecar-route-extraction/spec.md b/openspec/specs/sidecar-route-extraction/spec.md new file mode 100644 index 00000000..502df1a4 --- /dev/null +++ b/openspec/specs/sidecar-route-extraction/spec.md @@ -0,0 +1,43 @@ +# sidecar-route-extraction Specification + +## Purpose +TBD - created by archiving change code-review-bug-finding-and-sidecar-venv-fix. Update Purpose after archive. +## Requirements +### Requirement: Framework extractors exclude .specfact from scan paths + +All sidecar framework extractors (FastAPI, Flask, Django, DRF) SHALL exclude +`.specfact/` directories from Python file discovery. This prevents the sidecar's +own installed venv and workspace artefacts from being scanned as application +source. + +#### Scenario: FastAPI extractor ignores .specfact/venv + +- **GIVEN** a repo with a `.specfact/venv/` directory containing FastAPI source +- **WHEN** the FastAPI extractor runs route extraction on that repo +- **THEN** no routes are extracted from files under `.specfact/` +- **AND** only routes from application source files are returned + +#### Scenario: Flask extractor ignores .specfact/venv + +- **GIVEN** a repo with a `.specfact/venv/` directory containing Flask source +- **WHEN** the Flask extractor runs route extraction on that repo +- **THEN** no routes are extracted from files under `.specfact/` + +#### Scenario: Django extractor ignores .specfact/venv + +- **GIVEN** a repo with a `.specfact/venv/` directory containing Django source +- **WHEN** the Django extractor runs route extraction on that repo +- **THEN** no routes are extracted from files under `.specfact/` + +#### Scenario: Other standard exclusions also apply + +- **WHEN** any framework extractor scans a repo +- **THEN** files under `.git/`, `__pycache__/`, and `node_modules/` are also excluded +- **AND** legitimate application source outside these directories is not affected + +#### Scenario: Route count reflects real application routes only + +- **GIVEN** gpt-researcher repo with 19 real FastAPI routes and `.specfact/venv` installed +- **WHEN** sidecar validation runs route extraction +- **THEN** routes extracted is approximately 19, not 25,947 + diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 7483d4f0..48e91a00 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -1,5 +1,5 @@ name: nold-ai/specfact-code-review -version: 0.47.9 +version: 0.47.13 commands: - code tier: official @@ -23,5 +23,5 @@ description: Official SpecFact code review bundle package. category: codebase bundle_group_command: code integrity: - checksum: sha256:64d68f426a6140fdff3ceffd51c3960fb36745a040068722ae37271dc378659e - signature: PcP7OeNMkZqTtRpRBkpr2v2od0RX7fM4VCKXyWAYyP7bBRsKyqyRJI5oxX/0BEIWPt29o6y4t93WyJisWgWKBQ== + checksum: sha256:5345c5f7d453481edc033ebb64f5b38d3d8ebdcd8a8c6ae1fcd879d5136dc919 + signature: UyRN+72IHFSxnU1woW5XJwuBRd1eZQrgKT4XODi2LAhI7G/RgY2SRCrde1fugYuE9Rc4Wx8kNMdjzah01DoPAA== diff --git a/packages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.py b/packages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.py index af333ff4..d2a0a9db 100644 --- a/packages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.py +++ b/packages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.py @@ -44,6 +44,25 @@ def _category_for_message_id(message_id: str) -> Literal["architecture", "style" return "style" +def _coerce_pylint_line(raw: object) -> int: + """Pylint may emit ``0`` for file- or config-scoped messages; ``ReviewFinding`` requires ``line >= 1``.""" + if raw is None or isinstance(raw, bool): + return 1 + if isinstance(raw, int): + return raw if raw >= 1 else 1 + if isinstance(raw, float): + as_int = int(raw) + return as_int if as_int >= 1 else 1 + return 1 + + +def _coerce_pylint_message(raw: object) -> str: + """Pylint occasionally emits an empty ``msg``; governed findings require non-blank text.""" + if isinstance(raw, str) and raw.strip(): + return raw + return "(pylint provided no message text)" + + def _finding_from_item(item: object, *, allowed_paths: set[str]) -> ReviewFinding | None: if not isinstance(item, dict): raise ValueError("pylint finding must be an object") @@ -57,12 +76,8 @@ def _finding_from_item(item: object, *, allowed_paths: set[str]) -> ReviewFindin message_id = item["message-id"] if not isinstance(message_id, str): raise ValueError("pylint message-id must be a string") - line = item["line"] - if not isinstance(line, int): - raise ValueError("pylint line must be an integer") - message = item["message"] - if not isinstance(message, str): - raise ValueError("pylint message must be a string") + line = _coerce_pylint_line(item.get("line")) + message = _coerce_pylint_message(item.get("message")) return ReviewFinding( category=_category_for_message_id(message_id), @@ -76,8 +91,19 @@ def _finding_from_item(item: object, *, allowed_paths: set[str]) -> ReviewFindin ) -def _payload_from_output(stdout: str) -> list[object]: - payload = json.loads(stdout) +def _payload_from_output(stdout: str, *, stderr: str, returncode: int | None) -> list[object]: + stripped = stdout.strip() + if not stripped: + out = stdout + if len(out) > 4096: + out = f"{out[:4096]}... ({len(stdout)} chars total)" + err = stderr + if len(err) > 4096: + err = f"{err[:4096]}... ({len(stderr)} chars total)" + raise ValueError( + f"pylint produced no JSON on stdout; stdout={out!r}, stderr={err!r}, {returncode=}", + ) + payload = json.loads(stripped) if not isinstance(payload, list): raise ValueError("pylint output must be a list") return payload @@ -119,7 +145,11 @@ def run_pylint(files: list[Path]) -> list[ReviewFinding]: check=False, timeout=30, ) - payload = _payload_from_output(result.stdout) + payload = _payload_from_output( + result.stdout, + stderr=result.stderr, + returncode=result.returncode, + ) except (OSError, ValueError, json.JSONDecodeError, subprocess.TimeoutExpired) as exc: return [tool_error(tool="pylint", file_path=files[0], message=f"Unable to parse pylint output: {exc}")] diff --git a/registry/index.json b/registry/index.json index f9461119..017606a3 100644 --- a/registry/index.json +++ b/registry/index.json @@ -78,9 +78,9 @@ }, { "id": "nold-ai/specfact-code-review", - "latest_version": "0.47.9", - "download_url": "modules/specfact-code-review-0.47.9.tar.gz", - "checksum_sha256": "8471e41b3567021834229c970365f537e1ac1ebe3c174c64a5a21304105eacd9", + "latest_version": "0.47.13", + "download_url": "modules/specfact-code-review-0.47.13.tar.gz", + "checksum_sha256": "618fa5dea59e1a38bb7173b2d906d55897c896af8f6a97ce1d502def0afa5f9e", "core_compatibility": ">=0.44.0,<1.0.0", "tier": "official", "publisher": { diff --git a/registry/modules/specfact-code-review-0.47.12.tar.gz b/registry/modules/specfact-code-review-0.47.12.tar.gz new file mode 100644 index 00000000..3b8e47ad Binary files /dev/null and b/registry/modules/specfact-code-review-0.47.12.tar.gz differ diff --git a/registry/modules/specfact-code-review-0.47.12.tar.gz.sha256 b/registry/modules/specfact-code-review-0.47.12.tar.gz.sha256 new file mode 100644 index 00000000..2bee1d4c --- /dev/null +++ b/registry/modules/specfact-code-review-0.47.12.tar.gz.sha256 @@ -0,0 +1 @@ +8ff0261849057113629289b82c674ae20594d3df21c33c5932468a8169c9274b diff --git a/registry/modules/specfact-code-review-0.47.13.tar.gz b/registry/modules/specfact-code-review-0.47.13.tar.gz new file mode 100644 index 00000000..789ae1c6 Binary files /dev/null and b/registry/modules/specfact-code-review-0.47.13.tar.gz differ diff --git a/registry/modules/specfact-code-review-0.47.13.tar.gz.sha256 b/registry/modules/specfact-code-review-0.47.13.tar.gz.sha256 new file mode 100644 index 00000000..7e4b65ef --- /dev/null +++ b/registry/modules/specfact-code-review-0.47.13.tar.gz.sha256 @@ -0,0 +1 @@ +618fa5dea59e1a38bb7173b2d906d55897c896af8f6a97ce1d502def0afa5f9e diff --git a/registry/signatures/specfact-code-review-0.47.12.tar.sig b/registry/signatures/specfact-code-review-0.47.12.tar.sig new file mode 100644 index 00000000..04ac39f2 --- /dev/null +++ b/registry/signatures/specfact-code-review-0.47.12.tar.sig @@ -0,0 +1 @@ +8ZIQZvhMaaaXvEJwjePJu4lgWb2YBsDmXe3edwWbboa/a+oxkyv5NYuJWeojEvBx+pmxq2RMerDR/QvuZYMjDQ== diff --git a/registry/signatures/specfact-code-review-0.47.13.tar.sig b/registry/signatures/specfact-code-review-0.47.13.tar.sig new file mode 100644 index 00000000..fdc29ce7 --- /dev/null +++ b/registry/signatures/specfact-code-review-0.47.13.tar.sig @@ -0,0 +1 @@ +UyRN+72IHFSxnU1woW5XJwuBRd1eZQrgKT4XODi2LAhI7G/RgY2SRCrde1fugYuE9Rc4Wx8kNMdjzah01DoPAA== diff --git a/scripts/docs_site_validation.py b/scripts/docs_site_validation.py index a11c3858..4ed87041 100644 --- a/scripts/docs_site_validation.py +++ b/scripts/docs_site_validation.py @@ -3,6 +3,11 @@ Used by ``scripts/check-docs-commands.py``, unit tests, and CI. Resolves Markdown links relative to each page's *published* permalink (browser semantics), not the repository source path. + +For pages whose ``permalink`` is under ``/bundles/``, links whose path +contains parent segments (``..``) are validated against both filesystem +resolution and browser URL resolution, because bundle permalinks do not +mirror the ``docs/bundles//`` directory depth. """ from __future__ import annotations @@ -345,6 +350,30 @@ def _resolve_http_or_opaque(parsed, ctx: DocsLinkResolutionContext) -> tuple[Pat return None +def _published_route_label(ctx: DocsLinkResolutionContext, doc_path: Path) -> str: + """Human-readable route or path for mismatch diagnostics.""" + resolved = doc_path.resolve() + route = ctx.path_to_route.get(resolved) + if route is not None: + return route + try: + return str(resolved.relative_to(ctx.docs_root)) + except ValueError: + return str(resolved) + + +def _link_path_has_parent_traversal(raw_link: str) -> bool: + """True when the link path walks up with ``..`` (repo vs browser divergence risk).""" + stripped = _normalize_jekyll_relative_url(raw_link.strip()) + path_only = urlparse(stripped).path + return ".." in Path(unquote(path_only)).parts + + +def _published_route_is_under_bundles(published_page_route: str) -> bool: + """Bundle overview pages use ``permalink`` paths that diverge from the ``docs/`` tree.""" + return normalize_route(published_page_route).startswith("/bundles/") + + @beartype @require( lambda source, ctx, published_page_route, raw_link: ( @@ -381,6 +410,20 @@ def resolve_internal_link_hybrid( if fs_first: fs_target, fs_err = _resolve_filesystem_relative(source, ctx, raw_link) if fs_target is not None and fs_err is None: + if _link_path_has_parent_traversal(raw_link) and _published_route_is_under_bundles(published_page_route): + pub_target, pub_err = _resolve_published_relative_url(published_page_route, target_path, fragment, ctx) + if pub_err is not None: + return None, pub_err + if pub_target is None: + return fs_target, None + if pub_target.resolve() != fs_target.resolve(): + want = _published_route_label(ctx, fs_target) + got = _published_route_label(ctx, pub_target) + return None, ( + "repository-relative link matches a markdown file on disk, but browsers " + f"resolve it to a different site route than `{want}` (permalink-relative " + f"navigation targets `{got}`); use a root-absolute path such as `{want}`" + ) return fs_target, None pub_target, pub_err = _resolve_published_relative_url(published_page_route, target_path, fragment, ctx) diff --git a/tests/unit/scripts/test_docs_site_validation_link_agreement.py b/tests/unit/scripts/test_docs_site_validation_link_agreement.py new file mode 100644 index 00000000..174bc797 --- /dev/null +++ b/tests/unit/scripts/test_docs_site_validation_link_agreement.py @@ -0,0 +1,162 @@ +"""Tests for published vs filesystem agreement in ``docs_site_validation``.""" + +from __future__ import annotations + +import sys +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[3] +_SCRIPTS_DIR = REPO_ROOT / "scripts" +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +import docs_site_validation as dsv # noqa: E402 + + +def _ctx_for(docs: Path) -> dsv.DocsLinkResolutionContext: + route_to_path, path_to_route = dsv.build_route_mappings(docs) + return dsv.DocsLinkResolutionContext(docs, route_to_path, path_to_route, dsv.MODULES_DOCS_HOST) + + +def test_hybrid_rejects_repo_relative_when_disk_and_browser_targets_diverge(tmp_path: Path) -> None: + """``../..`` from a deep ``/bundles/.../`` permalink must not pass on filesystem match alone.""" + docs = tmp_path / "docs" + (docs / "bundles" / "deep").mkdir(parents=True) + (docs / "guides").mkdir(parents=True) + (docs / "bundles" / "deep" / "page.md").write_text( + """--- +layout: default +title: Deep page +permalink: /bundles/deep/here/ +--- +[bad](../../guides/target/) +""", + encoding="utf-8", + ) + (docs / "guides" / "target.md").write_text( + """--- +layout: default +title: Target +permalink: /guides/target/ +--- +""", + encoding="utf-8", + ) + ctx = _ctx_for(docs) + source = docs / "bundles" / "deep" / "page.md" + target, err = dsv.resolve_internal_link_hybrid( + source=source, + ctx=ctx, + published_page_route="/bundles/deep/here/", + raw_link="../../guides/target/", + ) + assert target is None + assert err is not None + assert "missing published route" in err or "root-absolute" in err + + +def test_hybrid_accepts_root_absolute_when_target_exists(tmp_path: Path) -> None: + docs = tmp_path / "docs" + (docs / "bundles" / "deep").mkdir(parents=True) + (docs / "guides").mkdir(parents=True) + (docs / "bundles" / "deep" / "page.md").write_text( + """--- +layout: default +title: Deep page +permalink: /bundles/deep/here/ +--- +[ok](/guides/target/) +""", + encoding="utf-8", + ) + (docs / "guides" / "target.md").write_text( + """--- +layout: default +title: Target +permalink: /guides/target/ +--- +""", + encoding="utf-8", + ) + ctx = _ctx_for(docs) + source = docs / "bundles" / "deep" / "page.md" + target, err = dsv.resolve_internal_link_hybrid( + source=source, + ctx=ctx, + published_page_route="/bundles/deep/here/", + raw_link="/guides/target/", + ) + assert err is None + assert target == (docs / "guides" / "target.md").resolve() + + +def test_parent_traversal_outside_bundles_keeps_filesystem_first_semantics(tmp_path: Path) -> None: + """``../`` from non-``/bundles/`` permalinks must not require published agreement (legacy).""" + docs = tmp_path / "docs" + (docs / "adapters").mkdir(parents=True) + (docs / "guides").mkdir(parents=True) + (docs / "adapters" / "page.md").write_text( + """--- +layout: default +title: Adapter page +permalink: /adapters/page/ +--- +[guide](../guides/target.md) +""", + encoding="utf-8", + ) + (docs / "guides" / "target.md").write_text( + """--- +layout: default +title: Guide +permalink: /guides/target/ +--- +""", + encoding="utf-8", + ) + ctx = _ctx_for(docs) + source = docs / "adapters" / "page.md" + target, err = dsv.resolve_internal_link_hybrid( + source=source, + ctx=ctx, + published_page_route="/adapters/page/", + raw_link="../guides/target.md", + ) + assert err is None + assert target == (docs / "guides" / "target.md").resolve() + + +def test_hybrid_uses_published_semantics_when_filesystem_misses(tmp_path: Path) -> None: + """Sibling ``../run/`` style links often miss on disk but are valid on the site URL.""" + docs = tmp_path / "docs" + (docs / "bundles" / "cr").mkdir(parents=True) + (docs / "bundles" / "cr" / "overview.md").write_text( + """--- +layout: default +title: Overview +permalink: /bundles/cr/here/ +--- +[run](../run/) +""", + encoding="utf-8", + ) + (docs / "bundles" / "cr" / "run.md").write_text( + """--- +layout: default +title: Run +permalink: /bundles/cr/run/ +--- +""", + encoding="utf-8", + ) + ctx = _ctx_for(docs) + source = docs / "bundles" / "cr" / "overview.md" + target, err = dsv.resolve_internal_link_hybrid( + source=source, + ctx=ctx, + published_page_route="/bundles/cr/here/", + raw_link="../run/", + ) + assert err is None + assert target == (docs / "bundles" / "cr" / "run.md").resolve() diff --git a/tests/unit/specfact_code_review/tools/test_pylint_runner.py b/tests/unit/specfact_code_review/tools/test_pylint_runner.py index b981f14a..58bcf83e 100644 --- a/tests/unit/specfact_code_review/tools/test_pylint_runner.py +++ b/tests/unit/specfact_code_review/tools/test_pylint_runner.py @@ -34,6 +34,7 @@ def test_run_pylint_maps_bare_except_to_architecture(tmp_path: Path, monkeypatch assert findings[0].category == "architecture" assert findings[0].severity == "warning" assert findings[0].rule == "W0702" + assert findings[0].line == 7 assert_tool_run(run_mock, ["pylint", "--output-format", "json", str(file_path)]) @@ -97,6 +98,169 @@ def test_run_pylint_returns_tool_error_on_parse_error(tmp_path: Path, monkeypatc assert findings[0].tool == "pylint" +def test_run_pylint_empty_stdout_is_tool_error(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + monkeypatch.setattr( + subprocess, + "run", + Mock( + return_value=completed_process( + "pylint", + stdout="", + stderr="config error: missing plugin", + returncode=2, + ), + ), + ) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].category == "tool_error" + assert findings[0].tool == "pylint" + assert "stdout=''" in findings[0].message + assert "config error" in findings[0].message + assert "returncode=2" in findings[0].message + + +def test_run_pylint_whitespace_only_stdout_is_tool_error(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + monkeypatch.setattr( + subprocess, + "run", + Mock(return_value=completed_process("pylint", stdout=" \n\t ", stderr="", returncode=1)), + ) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].category == "tool_error" + assert "pylint produced no JSON on stdout" in findings[0].message + + +def test_run_pylint_empty_stdout_truncates_long_stderr(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + unique_tail = "UNIQUE_STDERR_TAIL_FOR_TRUNCATION_TEST" + long_stderr = "e" * 4096 + unique_tail + assert len(long_stderr) > 4096 + monkeypatch.setattr( + subprocess, + "run", + Mock( + return_value=completed_process( + "pylint", + stdout="", + stderr=long_stderr, + returncode=3, + ), + ), + ) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].category == "tool_error" + assert findings[0].tool == "pylint" + assert unique_tail not in findings[0].message + assert "... (" in findings[0].message + assert "chars total)" in findings[0].message + assert f"... ({len(long_stderr)} chars total)" in findings[0].message + + +def test_run_pylint_coerces_line_zero_to_one(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + payload = [ + { + "message-id": "C0301", + "path": str(file_path), + "line": 0, + "message": "line too long", + } + ] + monkeypatch.setattr(subprocess, "run", Mock(return_value=completed_process("pylint", stdout=json.dumps(payload)))) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].line == 1 + + +def test_run_pylint_coerces_empty_message_text(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + payload = [ + { + "message-id": "C0301", + "path": str(file_path), + "line": 3, + "message": "", + } + ] + monkeypatch.setattr(subprocess, "run", Mock(return_value=completed_process("pylint", stdout=json.dumps(payload)))) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].message == "(pylint provided no message text)" + + +def test_run_pylint_coerces_negative_line_to_one(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + payload = [ + { + "message-id": "C0301", + "path": str(file_path), + "line": -5, + "message": "line too long", + } + ] + monkeypatch.setattr(subprocess, "run", Mock(return_value=completed_process("pylint", stdout=json.dumps(payload)))) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].line == 1 + + +def test_run_pylint_coerces_whitespace_only_message(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + payload = [ + { + "message-id": "C0301", + "path": str(file_path), + "line": 3, + "message": " \t\n ", + } + ] + monkeypatch.setattr(subprocess, "run", Mock(return_value=completed_process("pylint", stdout=json.dumps(payload)))) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].message == "(pylint provided no message text)" + + +def test_run_pylint_parses_json_with_surrounding_whitespace(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: + file_path = tmp_path / "target.py" + payload = [ + { + "message-id": "W0702", + "path": str(file_path), + "line": 7, + "message": "No exception type(s) specified", + } + ] + stdout = "\n " + json.dumps(payload) + " \n" + monkeypatch.setattr(subprocess, "run", Mock(return_value=completed_process("pylint", stdout=stdout, returncode=16))) + + findings = run_pylint([file_path]) + + assert len(findings) == 1 + assert findings[0].rule == "W0702" + assert findings[0].line == 7 + assert findings[0].message == "No exception type(s) specified" + assert findings[0].category == "architecture" + + def test_run_pylint_returns_tool_error_for_invalid_payload_item(tmp_path: Path, monkeypatch: MonkeyPatch) -> None: file_path = tmp_path / "target.py" payload = [{"path": str(file_path), "line": 7, "message": "No exception type(s) specified"}]