Skip to content

[#3145] Kept 'review-code' read-only after the report and documented its external and merged pull request cases. - #3257

Merged
AlexSkrypnyk merged 2 commits into
mainfrom
feature/3145-review-skill-fixes
Oct 30, 2026
Merged

AlexSkrypnyk merged 2 commits into
mainfrom
feature/3145-review-skill-fixes

Conversation

@AlexSkrypnyk

Copy link
Copy Markdown
Member

Closes #3145

Summary

The review-code skill's closing step no longer offers to act. After the report it notes that fixing the findings or posting them to the pull request is a separate request, and the review itself does neither. The "Review code" section of development/ai.mdx now documents the cases where the 3-source order and the verdict don't apply as written.

#3256 merged before these 2 review fixes reached it. The skill opens by saying it never edits project files or posts to GitHub, but its last step said "offer to fix them or post them to the pull request. Do neither unless the developer asks". An agent that got a "yes" would then edit files or post comments from inside a skill that promised not to. Separately, the docs described the rule order and the verdict as unconditional. In fact the skill uses another repository's rules (and no checklist) for that repository's PRs, drops the verdict for merged or closed PRs, and skips the lint commands AGENTS.md lists as well as ahoy lint.

After merge, a review run never changes anything, even when the developer asks for a fix afterwards; that becomes a normal follow-up request outside the skill. The procedure and checklist are otherwise unchanged.

Before / After

BEFORE                                   AFTER
──────                                   ─────
SKILL.md, after the report               SKILL.md, after the report
┌────────────────────────────────┐       ┌────────────────────────────────┐
│ "offer to fix them or post     │       │ "fixing them or posting them   │
│  them ... Do neither unless    │  ──►  │  is a separate request. The    │
│  the developer asks"           │       │  review itself does neither."  │
│ → can edit/post from a         │       │ → always read-only             │
│   read-only skill              │       │                                │
└────────────────────────────────┘       └────────────────────────────────┘

ai.mdx "Review code"                     ai.mdx "Review code"
  rule order + verdict always apply        + another repo's PR: its rules,
  skips `ahoy lint` checks                   no checklist
                                           + merged/closed PR: follow-ups,
                                             no verdict
                                           + skips `ahoy lint` and other
                                             lint commands in AGENTS.md

Changes

  • .claude/skills/review-code/SKILL.md: reworded the closing line so the follow-up is a separate request.
  • .vortex/docs/content/development/ai.mdx: documented the other-repository and merged-PR cases and the AGENTS.md lint commands. Passes yarn lint-prose with 0 warnings.
  • _baseline/.claude/skills/review-code/SKILL.md: regenerated snapshot. All 163 install scenarios pass.

…d-only review and documented the external and merged pull request cases.
@AlexSkrypnyk AlexSkrypnyk added the A2 Working clone index A2 label Oct 30, 2026
@github-project-automation github-project-automation Bot moved this to BACKLOG in Vortex 1.x Oct 30, 2026
@AlexSkrypnyk AlexSkrypnyk added this to the 1.43.0 milestone Oct 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 9 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available. Your 62 included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between f563234 and 45bf9d0.

Files ignored due to path filters: 1
  • .vortex/installer/tests/Fixtures/handler_process/_baseline/.claude/skills/review-code/SKILL.md is excluded by !.vortex/installer/tests/Fixtures/**
Files selected for processing: 2
  • .claude/skills/review-code/SKILL.md
  • .vortex/docs/content/development/ai.mdx
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

This comment has been minimized.

@AlexSkrypnyk

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   100.00% (233/233)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

📖 Documentation preview for this pull request has been deployed to Netlify:

https://6ae419e8789e54c5564e3e99--vortex-docs.netlify.app

This preview is rebuilt on every commit and is not the production documentation site.

@AlexSkrypnyk

Copy link
Copy Markdown
Member Author

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   100.00% (233/233)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.28%. Comparing base (f563234) to head (45bf9d0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3257      +/-   ##
==========================================
- Coverage   92.89%   92.28%   -0.62%     
==========================================
  Files         102      108       +6     
  Lines        4702     5053     +351     
  Branches       49        3      -46     
==========================================
+ Hits         4368     4663     +295     
- Misses        334      390      +56     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk
AlexSkrypnyk enabled auto-merge (squash) October 30, 2026 04:22
@AlexSkrypnyk AlexSkrypnyk added the AUTOMERGE Pull request has been approved and set to automerge label Oct 30, 2026
@AlexSkrypnyk
AlexSkrypnyk merged commit 0d8e688 into main Oct 30, 2026
48 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/3145-review-skill-fixes branch October 30, 2026 04:24
@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Oct 30, 2026
@github-project-automation github-project-automation Bot moved this from BACKLOG to Release queue in Vortex 1.x Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A2 Working clone index A2 AUTOMERGE Pull request has been approved and set to automerge Needs review Pull request needs a review from assigned developers

Projects

Status: Release queue

Development

Successfully merging this pull request may close these issues.

Ship an AI code-review configuration as a selectable tool

1 participant