feat(skill): add PR review skill (#88) by diiviikk5 · Pull Request #90 · mofa-org/mofaclaw

diiviikk5 · 2026-03-18T14:02:09Z

Summary

Add a new builtin PR review skill at skills/pr-review/SKILL.md that helps reviewers inspect pull requests with a structured workflow covering:

PR overview and template compliance
code change analysis
security checks
quality suggestions
CI status checks
review comment generation

This closes issue #88.

What Changed

Added skills/pr-review/SKILL.md
- YAML frontmatter with metadata and required binaries
- trigger phrases for PR review requests
- step-by-step review workflow
- Windows and Unix/Mac command variants
- security / quality / CI checklists
- structured review comment template
- examples based on real mofaclaw PRs
Updated skills/README.md
- added pr-review to the builtin skills index
Added prompt-loading coverage in core/src/agent/context.rs
- verifies pr-review appears in the skills summary
- verifies the requested skill body is loaded into context
Fixed Windows local-testing blocker in core/src/session/mod.rs
- sanitize session storage keys before they reach the underlying JSONL session store
- this prevents Windows path failures for keys like cli:default

Tests Verification

1. Direct command verification against real mofaclaw PRs

All core commands documented by the skill were tested locally with gh against real PRs in mofa-org/mofaclaw.

PR #78 - feature PR

Commands tested:

gh pr view 78 --repo mofa-org/mofaclaw --json title,body,author,files,additions,deletions,changedFiles
gh pr diff 78 --repo mofa-org/mofaclaw --name-only
gh pr checks 78 --repo mofa-org/mofaclaw
gh api repos/mofa-org/mofaclaw/pulls/78/commits --jq '.[].commit.message'
gh run list --repo mofa-org/mofaclaw --branch feat/permission-based-tool-registry --limit 3

Verified results:

PR metadata returned successfully
changed files returned successfully
CI checks returned successfully
commit history returned successfully
Actions runs returned successfully

Observed CI summary for PR #78:

Lint: pass
Test (macos-latest): pass
Test (ubuntu-latest): pass
Test (windows-latest): pass
rbac-tests: pass

PR #89 - docs PR

Commands tested:

gh pr view 89 --repo mofa-org/mofaclaw --json title,body,author,files,additions,deletions,changedFiles
gh pr diff 89 --repo mofa-org/mofaclaw --name-only
gh pr checks 89 --repo mofa-org/mofaclaw

Verified results:

docs-only PR metadata returned successfully
changed file listing returned successfully
CI checks returned successfully

PR #80 - medium feature PR

Commands tested:

gh pr view 80 --repo mofa-org/mofaclaw --json title,body,author,files,additions,deletions,changedFiles
gh pr checks 80 --repo mofa-org/mofaclaw

Verified results:

PR metadata returned successfully
CI checks returned successfully

PRs #3 and #4 - older PRs with no checks

Commands tested:

gh pr view 3 --repo mofa-org/mofaclaw --json title,body,author,state,baseRefName,headRefName,additions,deletions,changedFiles,files
gh pr checks 3 --repo mofa-org/mofaclaw
gh pr diff 3 --repo mofa-org/mofaclaw --name-only

gh pr view 4 --repo mofa-org/mofaclaw --json title,body,author,state,baseRefName,headRefName,additions,deletions,changedFiles,files
gh pr checks 4 --repo mofa-org/mofaclaw
gh pr diff 4 --repo mofa-org/mofaclaw --name-only

Verified results:

metadata and changed file listing worked
gh pr checks correctly reported no checks for those older branches
this validated the skill guidance that missing checks should be stated explicitly rather than assumed to be passing

2. End-to-end local mofaclaw verification

After fixing the Windows session-key issue, the skill was also exercised through a real local mofaclaw CLI run, not just raw gh commands.

Local setup used for verification:

workspace moved to D:\mofaclaw-workspace
Cargo target dir moved to D:\cargo-targets\mofaclaw
a locally configured LLM provider was used for end-to-end agent verification; provider choice was only for local testing and is not part of this PR

Command used:

cargo run --quiet -- agent -s upstream-pr-test -m "Review PR #78 in mofa-org/mofaclaw using the pr-review skill. Do not use my fork. Check scope, security, tests, and CI."

Verified result:

mofaclaw discovered pr-review
the agent produced a structured review using real PR data from mofa-org/mofaclaw
the output included PR overview, changed files, CI status, security checks, quality suggestions, and a recommendation

3. Automated verification

Rust tests run locally:

cargo test -p mofaclaw-core test_skills_summary_includes_pr_review -- --nocapture
cargo test -p mofaclaw-core test_requested_pr_review_skill_is_loaded -- --nocapture

Verified results:

test_skills_summary_includes_pr_review passed
test_requested_pr_review_skill_is_loaded passed

Acceptance Criteria

SKILL.md created with proper YAML frontmatter
Skill triggers on phrases like "review pr", "check this pr", "pr review for"
Clear workflow for analyzing PRs step-by-step
Security checklist included
Template for review comments
Examples for different PR sizes/types
Tested against real PRs in mofaclaw
Human-reviewed and locally verified before merge

Notes

The skill intentionally assists reviewers rather than auto-approving PRs.
Examples and commands were validated on Windows and use Windows-safe alternatives where needed (findstr, PowerShell examples).
The local agent test used the upstream repo mofa-org/mofaclaw explicitly because the local git clone had a fork remote as origin.

diiviikk5 · 2026-03-18T14:19:15Z

@lijingrs , @Nixxx19 , done with it , im sure there would be improvements but im quite excited to work on mofaclaw and make more skills for it , also i couldnt use open router properly from my end so i tested it with my own minimax m2.5 key , and the screenshots are a result that , im happy to keep iterating on this , its good to work on such stuff

Nixxx19 · 2026-03-18T14:56:47Z

@lijingrs , @Nixxx19 , done with it , im sure there would be improvements but im quite excited to work on mofaclaw and make more skills for it , also i couldnt use open router properly from my end so i tested it with my own minimax m2.5 key , and the screenshots are a result that , im happy to keep iterating on this , its good to work on such stuff

nice one, i will try to review it as soon as possible, thanks!!

Copilot

Pull request overview

Adds a new built-in pr-review skill to the skills catalog and integrates it into prompt construction/testing, while also changing session key sanitization to produce filesystem-safe storage keys.

Changes:

Added pr-review to the skills list and introduced skills/pr-review/SKILL.md with a structured PR-review workflow and gh CLI commands.
Updated SessionManager to use a sanitized storage_key() when creating/deleting/loading sessions, plus added tests around the new key behavior.
Added ContextBuilder tests to assert the pr-review skill appears in the skills summary and loads when explicitly requested.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
skills/README.md	Adds `pr-review` to the documented built-in skills list.
skills/pr-review/SKILL.md	New skill documentation and command-driven workflow for structured PR reviews.
core/src/session/mod.rs	Routes session operations through a filesystem-safe storage key and adds related tests.
core/src/agent/context.rs	Adds tests asserting `pr-review` appears in the built system prompt output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    requires:
+      bins: ["gh", "git"]
+    install:
+      - id: brew
+        kind: brew
+        formula: gh
+        bins: ["gh"]


+# Check for SQL injection patterns (Unix/Mac)
+gh pr diff <PR_NUMBER> --repo <OWNER/REPO> | grep -iE '(SELECT|INSERT|UPDATE|DELETE).*\+.*'
+
+# Check for changed workflow, deployment, or secret-related files


    pub async fn get_or_create(&self, key: impl Into<String>) -> Session {
-        self.inner.get_or_create(&key.into()).await
+        let key = key.into();
+        self.inner.get_or_create(&storage_key(&key)).await


+    #[test]
+    fn test_storage_key_replaces_windows_unsafe_separators() {
+        assert_eq!(storage_key("cli:default"), "cli_default");
+        assert_eq!(storage_key("discord:general/chat"), "discord_general_chat");


+    #[tokio::test]
+    async fn test_skills_summary_includes_pr_review() {
+        let temp = tempdir().expect("tempdir");
+        let mut config = default_config();
+        config.agents.defaults.workspace = temp.path().display().to_string();
+
+        let context = ContextBuilder::new(&config);
+        let prompt = context
+            .build_system_prompt(None)
+            .await
+            .expect("system prompt");
+
+        assert!(
+            prompt.contains("pr-review"),
+            "prompt did not include pr-review skill summary"
+        );


diiviikk5 · 2026-03-18T15:08:04Z

will have a look at the suggestions , also could you check the #78 i have done the changes as requested

Nixxx19 · 2026-03-19T13:24:11Z

looks good overall, nice one!

nice addition of the pr-review skill. the workflow and command guidance are practical, and the skill is well structured for real review tasks.

one small follow-up note: this pr also includes session key normalization changes in core/src/session/mod.rs, which is broader than a pure skill addition. not blocking for me, but please mention this explicitly in the pr description or split it next time for easier review.

thanks!!

diiviikk5 · 2026-03-19T14:03:41Z

thanks for reviewing , i mustve forgotten to edit the pr description , i was aware of it , thats on me , will take care next time
thanks for the work

feat(skill): add PR review skill (mofa-org#88)

abd5aae

diiviikk5 force-pushed the feat/pr-review-skill-88 branch from f44b325 to abd5aae Compare March 18, 2026 14:14

Nixxx19 requested a review from Copilot March 18, 2026 14:54

Copilot started reviewing on behalf of Nixxx19 March 18, 2026 14:54 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

Nixxx19 merged commit e3edc98 into mofa-org:main Mar 19, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(skill): add PR review skill (#88)#90

feat(skill): add PR review skill (#88)#90
Nixxx19 merged 1 commit into
mofa-org:mainfrom
diiviikk5:feat/pr-review-skill-88

diiviikk5 commented Mar 18, 2026

Uh oh!

diiviikk5 commented Mar 18, 2026

Uh oh!

Nixxx19 commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

diiviikk5 commented Mar 18, 2026 •

edited

Loading

Uh oh!

Nixxx19 commented Mar 19, 2026

Uh oh!

Uh oh!

diiviikk5 commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

diiviikk5 commented Mar 18, 2026

Summary

What Changed

Tests Verification

1. Direct command verification against real mofaclaw PRs

PR #78 - feature PR

PR #89 - docs PR

PR #80 - medium feature PR

PRs #3 and #4 - older PRs with no checks

2. End-to-end local mofaclaw verification

3. Automated verification

Acceptance Criteria

Notes

Uh oh!

diiviikk5 commented Mar 18, 2026

Uh oh!

Nixxx19 commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

diiviikk5 commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nixxx19 commented Mar 19, 2026

Uh oh!

Uh oh!

diiviikk5 commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

diiviikk5 commented Mar 18, 2026 •

edited

Loading