Repository navigation
fix(report-back): every finding names a file so repo-level rules grade as standing - #668
Conversation
…e as standing The verdict grid showed finishingbot '9 high, 5 medium' and glambot '5 minor' on every rsr-template-repo pull request, whatever it touched. Cause: the adapters emitted repository-level findings (missing-file and whole-repo rules) with no path; the fleet driver turned the null into an empty string, and the canon renderer grades a finding with no path as part of the change (fork-governance SPEC 3.0: absence of a location is not evidence of absence of a defect). Measured on the template: finishing-bot emits exactly 14 path-less findings (9 error, 5 warning) and glambot 5 info; with this change the same scan has none, and a 2-file delta grades 0 with the repo-level findings reported as standing. Fix at the source (the adapters), leaving the canon unchanged: - finishingbot and glambot repo-level rules now carry the file they are about: the expected path for a missing-file rule, the manifest or config that would fix a whole-repo property. TEST-001 (unknown project type) has no meaningful file and stays path-less; a test pins that list. - glambot GS-001 (git-seo not installed) described the runner, not the repository; it is logged instead of reported, and GS-003 names .git/config. - both bots now log to stderr: stdout carries the JSON report the driver parses and a log line there made it unreadable. - fleet-report-back.sh warns loudly when any member emits a path-less finding, so a new such rule cannot quietly re-create this; the contract is written down in docs/REPORT-BACK.adoc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b1dc792ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !git_seo_installed { | ||
| warn!("git-seo is not installed; the Git-SEO analyzer did not run (no findings reported, not a clean result)"); | ||
| result.duration_ms = start.elapsed().as_millis() as u64; | ||
| return Ok(result); |
There was a problem hiding this comment.
In the fleet report-back context, .github/workflows/fleet-report-back-reusable.yml only builds the Rust adapters and never provisions git-seo, while scripts/fleet-report-back.sh:491 launches adapters with RUST_LOG=off and discards captured stderr after valid JSON. Consequently this branch normally returns an empty successful analyzer result, the driver emits status: ran, and the verdict grid treats Git-SEO as clean even though it never executed. Preserve an unavailable/error signal in structured output or install the required tool rather than returning Ok with no findings.
Useful? React with 👍 / 👎.
| Severity::Error, | ||
| "Rust project missing benches/ directory for criterion benchmarks.", | ||
| ) | ||
| .with_file(benches_dir.clone()) |
There was a problem hiding this comment.
When a PR introduces a Rust project by adding Cargo.toml without a benchmark, this error is now scoped to benches, but the grid matches finding paths against git diff --name-only and Git never reports directories. Thus TEST-RU-001 can never be classified as part of the change and is always downgraded to standing; the same problem affects the newly assigned directory paths for empty benchmarks, fuzzing, release artifacts, tests, and workflow directories. Use a concrete expected file such as a benchmark source, or the tracked manifest that activates the rule.
Useful? React with 👍 / 👎.
Resolves conflicts with #667/#668/#670: every finding names a file (#668) while SCM-001/MACH-007 stay retired, git-seo reports as skipped, and fleet-report-back keeps skip reasons beside the adapter exit-status guard and the unpathed-finding warning. Drops default() on unit structs in the new finishingbot scope test so clippy stays clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8
Problem
On every rsr-template-repo PR the verdict grid showed finishingbot "9 high, 5 medium" and glambot "5 minor", identical for a PR touching 2 Agda files and one touching 7 shell files.
Root cause (measured, not assumed)
Running the unmodified adapters on rsr-template-repo: finishing-bot emits exactly 14 findings with
file: null(9 error = PH-100, SCM-001 x6, TOOL-001, V1-DOC-ROADMAP; 5 warning = REL-004, TEST-AG-002, V1-SEC-001, V1-COM x2) and glambot 5 info. That is exactly 9 high / 5 medium / 5 minor.Finding::newsetsfile: None(bots/finishingbot/src/analyzers/mod.rs:65, bots/glambot/src/analyzers/mod.rs:61); repo-level rules never call.with_file.file: ((.file // .location // "") | rel)turns null into "" (scripts/fleet-report-back.sh:536).elif any($paths[]; . == "") then "graded"(standards scripts/lib/verdict-grid.jq:215), by design: fork-governance SPEC 3.0 "A finding with no path is graded: absence of a location is not evidence of absence of a defect". So the canon is right and the adapters were wrong. standards is not edited.Fix (at the adapters)
warn), not reported. GS-003 names.git/config.scripts/fleet-report-back.shprints aWARNINGfor any member emitting a path-less finding;docs/REPORT-BACK.adocstates the contract.Verification
Real driver + real standards renderer on rsr-template-repo,
--changed= 2 Agda files, before/after:cargo test --locked: finishingbot 71 passed / 0 failed; glambot 51 passed / 0 failed. New tests: every finding from empty/Rust/Deno repos carries a path (allowlist: TEST-001), GS-001 not emitted, GS-003 pathed, glambot stdout is pure JSON.bash tests/fleet-report-back-test.sh(with and without STANDARDS_DIR): 0 failed; build-helper suite 0 failed.Findings that still have no path (listed, not invented)
.finishing-bot/config.yml, the file that setsmax_allowed; it is an aggregate of already-pathed PH findings.Notes
Generated by Claude Code