Skip to content

feat(#182): add CI path filter coverage gap check to review agent #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fullsend-ai-coder wants to merge 2 commits into
base: main
Choose a base branch
from
+58 −2
Open

feat(#182): add CI path filter coverage gap check to review agent #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7188176
feat(#182): add CI path filter coverage gap check to review agent
fullsend-ai-coder[bot] Jul 8, 2026
428892a
fix(#182): document CI check rationale in correctness Own field
fullsend-ai-coder[bot] Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions internal/scaffold/fullsend-repo/skills/code-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ dimension carry over to another — each requires its own scrutiny.
(step 2), determine whether those changes weakened coverage.
- Split-payload attacks: a production change paired with a test
modification that masks the real behavior.
- CI path filter coverage gap: when the change modifies source files that
are exercised by path-filtered CI workflows (`.github/workflows/`
files with `paths` triggers), check whether those files appear in the
workflow's `paths` filter. If a modified file is tested by a CI
workflow but is not in that workflow's path filter, the workflow will
not run on the PR — flag it as a medium-severity `ci-coverage-gap`
finding. Do not flag if the PR also modifies the workflow to add the
missing path, if the workflow has no path filter, or if the workflow
uses `paths-ignore` and the file is not ignored.

#### Security

Expand Down
2 changes: 1 addition & 1 deletion internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ review dimension using category as the key:

| Dimension | Categories |
|----------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| correctness | `logic-error`, `nil-deref`, `off-by-one`, `edge-case`, `api-contract`, `missing-test`, `test-inadequate`, `pattern-violation`, `test-weakened`, `test-removed`, `mock-loosened`, `assertion-weakened`, `coverage-reduced`, `test-poisoning`, `split-payload`, `stale-reference` |
| correctness | `logic-error`, `nil-deref`, `off-by-one`, `edge-case`, `api-contract`, `missing-test`, `test-inadequate`, `pattern-violation`, `test-weakened`, `test-removed`, `mock-loosened`, `assertion-weakened`, `coverage-reduced`, `test-poisoning`, `split-payload`, `stale-reference`, `ci-coverage-gap` |
| security | `auth-bypass`, `rbac-violation`, `data-exposure`, `privilege-escalation`, `injection-vuln`, `sandbox-escape`, `xss`, `ssrf`, `insecure-deserialization`, `prompt-injection`, `unicode-steganography`, `bidi-override`, `homoglyph-attack`, `instruction-smuggling`, `fail-open`, `permission-expansion`, `permission-reduction`, `role-escalation`, `workflow-permission`, `secret-exposure` |
| intent-coherence | `scope-exceeded`, `tier-mismatch`, `unauthorized-change`, `scope-creep`, `missing-authorization`, `misleading-label`, `design-direction`, `complexity-ratio`, `misplaced-abstraction`, `architectural-conflict`, `design-smell`, `over-engineering`, `under-engineering` |
| style-conventions | `naming-convention`, `error-handling-idiom`, `api-shape`, `code-organization`, `doc-style`, `pattern-inconsistency` |
Expand Down
49 changes: 48 additions & 1 deletion internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ You are a senior software engineer reviewing for correctness.

**Own:** Logic errors, nil/null handling, off-by-one, edge cases, race
conditions, API contract violations, error handling gaps, test adequacy
(are the right behaviors tested?), test integrity (are existing tests
(are the right behaviors tested, and will CI actually run them?), test
integrity (are existing tests
being weakened or poisoned alongside production changes?), and technical
accuracy in implementation plans and design documents.

Expand Down Expand Up @@ -47,6 +48,52 @@ Exclude the files already in the diff. Any hit outside the diff is a
Medium-severity finding: "stale reference to removed/renamed

@fullsend-ai-review fullsend-ai-review Bot Jul 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] design-direction

The CI path filter coverage gap check is categorized under correctness and placed in the correctness sub-agent. While the check is meta-level infrastructure validation rather than code correctness, it is defensible: the heuristic identifies situations where tests will silently not run, which is a test-adequacy concern that directly maps to the correctness dimension responsibility for test integrity.

Suggested fix: Consider documenting the rationale for placing CI infrastructure checks under correctness.

`<identifier>` in `<file>:<line>`."

### CI path filter coverage gap

@fullsend-ai-review fullsend-ai-review Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] design-direction

The CI path filter coverage gap check extends the correctness dimension into CI infrastructure analysis (reading workflow YAML, evaluating glob patterns, inferring test coverage). While this falls within the existing test adequacy scope, consider a brief scope note in the correctness sub-agent Own field clarifying that test adequacy includes CI configuration that determines whether tests run.

Suggested fix: Add a parenthetical to the Own field in correctness.md, e.g., test adequacy (are the right behaviors tested, and will CI actually run them?).

@fullsend-ai-review fullsend-ai-review Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] doc-style

The new CI path filter coverage gap section uses a level-3 heading (###) which is correct — it is a peer to the existing Technical documentation with correctness surface area section. No inconsistency found.


When the PR modifies source files that are exercised by path-filtered CI
workflows, check whether those files (or their parent directories) appear
in the workflow's `paths` trigger filter. A file that is tested by a CI
workflow but not in that workflow's path filter means the workflow will
not run when the file changes — the tests exist but are silently skipped.

**Procedure:**

1. Identify CI workflow files in the repository (`.github/workflows/*.yml`
or `.github/workflows/*.yaml`). Read each workflow that has a `paths`
or `paths-ignore` filter on `push` or `pull_request` /
`pull_request_target` triggers.
2. For each modified file in the PR, check whether it matches any
path-filtered workflow's `paths` list. Use glob semantics: `**/*.go`
matches all `.go` files, `internal/cli/**` matches files under that
directory.

@fullsend-ai-review fullsend-ai-review Bot Jul 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] edge-case

Procedure step 2 references the workflow's paths list, but step 1 also reads workflows with paths-ignore filters. For paths-ignore workflows there is no paths list. The edge-case section partially clarifies this, but is silent on the scenario where a file IS in paths-ignore yet the workflow's tests exercise that file's code. Practical impact is minimal since paths-ignore typically excludes non-source files.

Suggested fix: Amend step 2 wording to: "check whether it matches the workflow's paths list (or is not excluded by paths-ignore)" to close the ambiguity.

3. If a modified file is NOT in any path-filtered workflow's trigger
filter, but the workflow's test suite exercises code in the modified
file's package or directory (inferred from the workflow's test
commands, the paths already in the filter, or the repository's test
structure), flag it as a CI coverage gap.

**Severity:** Medium. Category: `ci-coverage-gap`.

**Description format:** "Modified file `<path>` is exercised by the
`<workflow-name>` workflow but is not in its `paths` trigger filter.
Changes to this file will not trigger `<workflow-name>`, so regressions
may reach the default branch untested."

**Remediation:** "Add `<path>` (or a parent glob like `<dir>/**`) to the
`paths` filter in `.github/workflows/<workflow-file>`."

**Edge cases:**

- If the PR also modifies the workflow file to add the missing path, do
NOT flag it — the gap is being fixed in the same PR.
- If the workflow has no `paths` filter (runs on all changes), there is
no gap to flag.
- If the workflow uses `paths-ignore` instead of `paths`, check whether
the modified file is explicitly ignored. A file not in `paths-ignore`
WILL trigger the workflow, so there is no gap.
- If a modified file is a new file in a directory that is already covered
by a glob pattern (e.g., `internal/cli/**`), there is no gap.

### Technical documentation with correctness surface area

Not all documentation is prose. Any
Expand Down
Loading