Skip to content

Make the PR follow-through loop act on rule violations - #76

Merged
rob-archastro merged 2 commits into
mainfrom
skill/pr-loop-rule-violations
Oct 20, 2026
Merged

rob-archastro merged 2 commits into
mainfrom
skill/pr-loop-rule-violations

Conversation

@rob-archastro

@rob-archastro rob-archastro commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review on ArchCode

Epic: Repo rules: instruction-file verdicts on PRs and mined rule candidates
Task: The PR follow-through loop acts on rule violations

Dependencies met. ArchDev CLI v0.54.8 (released 2026-10-20 23:00Z) ships firstlanding #19177 and #19197. This branch is rebased onto #73, merged at 23:51Z.

Problem and author intent

Bruno's #68 added the Pull Requests loop. After each push the agent reads CI and the server risk assessments, then fixes or explains what it finds. The server also runs a repository-rule check on every PR head, but agents can't use it:

  • inspect metadata returned no rule results until firstlanding #19177 / #19197, which are not released yet.
  • monitor.md says a PR's per-rule pass/fail verdict is unavailable and tells agents to report it that way.
agent pushes head → server rule check stores verdicts → agent reads risk only → violations sit unseen until a human opens the PR page

The goal is for the loop to treat rule violations the way it treats risk concerns:

  • read them for the exact head;
  • check each one, because the checker has false positives;
  • fix it or say why it was left;
  • never call an unchecked head passing.

What changed

  • archdev/SKILL.md → Pull Requests. A new step 3: read the rules violations, check each against the rule file and the cited lines, fix it if it holds and is in scope, otherwise explain it in the summary, and never disable a rule yourself. The loop now covers CI, risk and rules. The finish line now includes the violations that were fixed or left, with reasons. not_checked, unreadable, or a CLI with no rules key all count as unassessed, never passing.

  • archdev/SKILL.md → Read PR risk, scope, and rules. It now says inspect metadata carries the head's rules verdicts, and that docs(archdev): rules are added from a session, not proposed #73's rules --repository listing does not hold PR results.

  • archdev/references/monitor.md. A new "Rule verdicts for a PR head" subsection sits before docs(archdev): rules are added from a session, not proposed #73's "Repository rules, not PR verdicts". It documents:

    • the three rules states (checked, not_checked, unreadable) and the fields of a violation;
    • partial and invalid_rules;
    • the disabled_filter states (applied, none, and unavailable with a reason);
    • where to read the rule: a repository file's source_file, or the rules --repository listing for a stored rl_ rule, whose source_file is "ArchDev rules";
    • what to do when an older CLI returns no rules key: report it as unavailable and ask before upgrading.

    docs(archdev): rules are added from a session, not proposed #73's closing paragraph now sends a PR's rule results to inspect metadata instead of saying they are unavailable.

  • plugins/*/skills/archdev/**. Regenerated with npm run build.

  • tools/plugin-gen/test/generate.test.ts. Both contract tests now pin the rules guidance: the loop's rules step, never disabling a rule, unassessed states, each documented state, and the removal of the old "unavailable" wording.

The checker's measured precision is deliberately left out of the text. It is an internal number, and this skill ships to customers.

Scope

The skill text and its generated plugin copies only. There are no code or CLI changes.

Risk

Low: prose and tests only. Agents follow this text, though, so the main risk is behavioral. Overstating violations would make agents churn on false flags. The text guards against that by calling violations claims to check, requiring the rule and cited lines to be read first, and allowing explain-and-leave.

User impact

After the CLI release, coding agents that use the ArchDev skill will act on rule violations for the PRs they push. With an older CLI they'll report rule results as unavailable instead of guessing.

Testing

  • Canonical proof: tools/plugin-gen/test/generate.test.ts, tests "PR completion requires current-head CI, server risk and rule follow-through in the existing skill" and "review queries expose exact-head risk and scope without making reporting wait". They assert the new rules guidance in archdev/SKILL.md and monitor.md, and that every harness's generated copy is byte-identical to the source. npm test: 116 passed. npm run check: 140 generated files match. npm run typecheck: clean. These run in the plugins.yml workflow on PRs.

  • No honest end-to-end test exists for skill prose. Whether an agent follows the text can't be asserted mechanically.

  • Manual check: the rules JSON shape in the text was compared with the Rust CLI on firstlanding branch fix/inspect-metadata-rules-timeout (#19197, head 550e67d168). That CLI was also run against real heads:

    • #19177's first head flagged a missing-rustdoc violation, which held up and was fixed.
    • #19197 flagged a tagged-union violation, which was fixed, and a synthetics-coverage violation, which was left with a reason.

    That is the loop this PR writes down.

  • Checked against the released CLI: the v0.54.8 release binary was run on #19197 ({state: "checked"}, 1 violation, disabled_filter: {state: "none"}) and on the release PR #19221 (1 violation, with source_file and cited_lines). Both match the text.

Follow-ups

  • None for this PR. The rule check's synthetics flag on firstlanding #19197 is tracked there.

🤖 Generated with Claude Code

rob-archastro and others added 2 commits October 20, 2026 16:53
After each push the agent reads the head's `rules` section from
`inspect metadata`, checks each violation against the rule and cited
lines, and fixes it or says why it was left. not_checked, unreadable, or
an older CLI without `rules` are unassessed, never passing. Agents never
disable rules. Replaces the "pass/fail unavailable" wording.

Task: tsk_034jIRXBetVPbMSHo3Votz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stored rule's verdict names "ArchDev rules" as its source file, so the
agent reads it from `rules --repository` rather than a repository file.

Task: tsk_034jIRXBetVPbMSHo3Votz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rob-archastro
rob-archastro force-pushed the skill/pr-loop-rule-violations branch from 052c982 to 889f275 Compare October 20, 2026 23:53
@rob-archastro
rob-archastro marked this pull request as ready for review October 20, 2026 23:55
@rob-archastro
rob-archastro merged commit 80f14fe into main Oct 20, 2026
20 checks passed
@rob-archastro
rob-archastro deleted the skill/pr-loop-rule-violations branch October 20, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant