Skip to content

Review agent should detect cross-variant drift for forge-specific skills #808

Description

@fullsend-ai-retro

What happened

On PR #752, ralphbean added a "Use when..." trigger phrase to the issue-labels skill's description. The PR initially modified only the GitHub variant (skills/issue-labels/github/SKILL.md) after the flat SKILL.md had been split into forge-specific variants on main. The GitLab variant (skills/issue-labels/gitlab/SKILL.md) was left without the trigger phrase.

The review agent ran 6 successful review passes (runs 31515308320, 31747275331, 31756076411, 31815407032, 31818000451, 31826927290) and never flagged the cross-variant inconsistency. Human reviewer waynesun09 caught the gap as a MEDIUM finding on Aug 14, noting that the GitLab variant would remain unfixed. This required a third /fs-fix iteration to replicate the change.

The skills/ directory in this repo has a consistent pattern of forge-specific variants: skills like issue-labels, filing-issues, and others maintain parallel github/ and gitlab/ subdirectories with analogous SKILL.md files.

What could go better

The review agent should structurally detect when a PR changes one forge variant of a skill without updating its sibling variant(s). This is a mechanical check — if the PR diff includes skills/<name>/github/SKILL.md but not skills/<name>/gitlab/SKILL.md (or vice versa), and both variants exist, flag it as a potential cross-variant drift finding.

This is high-confidence: the forge-variant directory structure is a well-established convention in the repo (each forge variant has its own subdirectory under the skill name). The check is analogous to the existing protected-path detection — it's structural, deterministic, and doesn't require LLM judgment.

The check should be scoped to avoid false positives: some changes are legitimately forge-specific (e.g., a GitLab API endpoint fix). The finding should be informational (MEDIUM severity) — "This PR modifies the GitHub variant of skill X but does not update the GitLab variant. If this change applies to both forges, update the sibling variant to prevent drift."

Proposed change

Add a cross-variant parity check to the review orchestrator in skills/pr-review/SKILL.md. This should be an orchestrator-level structural check (similar to the existing protected-path detection in step 6e), not a sub-agent responsibility:

  1. In the changed-files analysis (step 2), detect when the PR modifies files matching skills/<name>/<forge>/ where sibling forge directories exist.
  2. If a sibling variant exists but is not included in the PR's changed files, emit a MEDIUM finding noting potential cross-variant drift.
  3. The finding text should note both variants and suggest reviewing whether the change applies to the sibling.

This check belongs in the orchestrator because it is deterministic and structural — no LLM judgment needed to detect the pattern.

Validation criteria

On the next 3 PRs to this repo that modify a single forge variant of a skill (e.g., only github/SKILL.md), the review agent should emit a cross-variant drift finding when a sibling variant exists but is not modified. Verify by checking the review agent's posted findings for the MEDIUM cross-variant finding. False positive rate should be low — the check only triggers when sibling forge directories exist.


Generated by retro agent from #752

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions