fix(agents): require PR readiness before review

[kkroo]

Thinking Path

• Paperclip is the open source control plane used to manage AI agents and their work.
• Its shipped GitHub workflow skill and default managed instructions define how agents prepare, review, and hand off pull requests.
• Agents can currently reach the reviewer-request step with an incomplete repository template or without proving that related pull requests were searched.
• That causes the commitperclip quality gate to reject the PR after a review cycle has already been spent, especially when Ally is asked to review too early.
• This pull request makes the template, duplicate-search checkbox, and commitperclip readiness gate explicit in both the reusable skill and the default managed instructions, with regression coverage.
• The benefit is that agents wait for a review-ready PR before requesting Ally or human attention, reducing avoidable review churn.

Linked Issues or Issue Description

No existing GitHub issue matched this focused instruction and regression-test change. Related implementation and operational context was reviewed in #1325, #1436, #1471, and #1141; none duplicates this change.

What happened

Agents can ask Ally or a human to review before the repository PR template is complete and before the commitperclip quality gate has passed.

Expected behavior

Agents should complete the repository template, search for duplicate or related PRs, check the dedup affirmation, and wait for commitperclip and related quality gates before requesting review.

Steps to reproduce

1. Give an agent a code change in a Paperclip repository with .github/PULL_REQUEST_TEMPLATE.md and commitperclip enabled.
2. Ask the agent to open a PR or request review without completing the template or dedup checkbox.
3. Observe that the review request can be made before the pre-review gate has passed, and the gate later reports missing PR metadata.

Paperclip version or commit

c28f893f270665d16fb9c9f2a7a17878bec85bdd (Blockcast/master)

Deployment mode

Self-hosted server with managed agent instruction bundles; change verified from source.

What Changed

• Require the repository PR template headings, issue-link or in-PR issue-description path, and exact dedup-search affirmation in the reusable GitHub PR workflow skill.
• Add an explicit commitperclip pre-review gate section telling agents to wait for quality gates before requesting Ally or human review.
• Add the same readiness rule to the default managed AGENTS.md bundle so newly materialized agents receive it.
• Add catalog and managed-bundle regression assertions, and refresh the generated skills catalog manifest.

Verification

• pnpm --filter @paperclipai/skills-catalog validate
• pnpm --filter @paperclipai/skills-catalog test -- --run packages/skills-catalog/src/shipped-catalog.test.ts (36 tests passed)
• pnpm exec vitest run server/src/__tests__/agent-skills-routes.test.ts (20 tests passed)
• git diff --check origin/master...HEAD
• Searched the GitHub pull-request list for duplicate and related work before opening this PR; the related links are listed above.

Risks

Low risk. This changes agent guidance and regression assertions only; it does not alter runtime API behavior, persistence, or deployment configuration. Reverting the commit restores the prior guidance and catalog manifest.

Model Used

OpenAI Codex powered by GPT-5, tool-enabled coding agent with repository, shell, GitHub API, and Kubernetes inspection capabilities.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes: # / Refs: # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[kkroo]

@ally Please independently review this replacement PR at the exact head
e114c70. It preserves the implementation and body from closed PR #1474; focus on the instruction changes, regression coverage, and any remaining review findings.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: e114c70

Critical Issues (0)

Important Issues (1)

• [native-codex] `packages/skills-catalog/catalog/bundled/software-development/github-pr-workflow/SKILL.md:229` — The new pre-review gate presents `commitperclip PR Review` as a required Paperclip-repository workflow, but this reusable GitHub workflow skill can be used in repositories that do not ship that workflow; the default bundle also repeats the unconditional requirement at `server/src/onboarding-assets/default/AGENTS.md:24`, where it can make unrelated issue handoffs appear blocked.
  • Make the rule conditional on detecting the repository's actual commitperclip workflow/checker (and keep the existing template conditional), or explicitly scope the skill and managed instruction to repos that install commitperclip. For non-commitperclip repositories, instruct agents to wait for the repository's actual quality gates instead.

Suggestions (0)

Strengths

• The workflow guidance mirrors the repository template headings and exact dedup-search checkbox text.
• The catalog and managed-bundle tests cover propagation of the new instruction text.
• The generated catalog manifest is refreshed with the changed skill content hash.

Recommended Action

1. Address the Important issue before merge.
2. Re-run the relevant catalog and managed-bundle tests after scoping the gate.

[kkroo]

Addressed the Important finding in commit 15a8d06921df54e57f5225a5f944572599d629e5.

The reusable workflow and managed AGENTS.md now detect whether the repository has a commitperclip workflow/checker. They require commitperclip PR Review only when present; otherwise they defer to the repository’s actual quality gates. The generated catalog and propagation assertions were refreshed.

Verification: catalog manifest validation passed; the focused catalog regression passed. The broader local suites still contain unrelated pre-existing timeout failures in shared reviewer recovery and two server route cases.

[kkroo]

@ally Please independently review PR #1477 at the exact new head
15a8d06. The Important finding from your review of e114c70d is addressed; please re-check the conditional repository-gate guidance and its propagation tests.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 15a8d06

Prior Findings Dispositioned (1)

• prior:e114c70 important 1 — still-present — packages/skills-catalog/catalog/bundled/software-development/github-pr-workflow/SKILL.md:242 — The conditional commitperclip paragraph now defers non-commitperclip repositories to their actual gates, but the next checklist still unconditionally requires the exact duplicate-search checkbox. A repository without that template/checkbox can therefore still be blocked by Paperclip-specific guidance; the current default bundle repeats the same unconditional requirement at server/src/onboarding-assets/default/AGENTS.md:24.

Critical Issues (0)

Important Issues (1)

• [native-codex] packages/skills-catalog/catalog/bundled/software-development/github-pr-workflow/SKILL.md:242 — The new conditional repository-gate guidance is undermined by the unconditional requirement to confirm the exact Paperclip duplicate-search checkbox. Reusable guidance can be installed in repositories without that template or checkbox, so those agents may still wait indefinitely for a gate that cannot exist; the propagated default AGENTS.md instruction has the same issue at server/src/onboarding-assets/default/AGENTS.md:24.
  • Scope the checkbox requirement to repositories that have the relevant template, or state that repositories without it should follow their own duplicate-search and review requirements. Add a regression test covering the non-template/non-commitperclip path rather than only asserting that the conditional wording is present.

Suggestions (0)

Strengths

• The commitperclip paragraph now explicitly detects the repository’s actual workflow/checker and defers to other repositories’ quality gates.
• The generated catalog metadata and managed-bundle propagation assertions were updated for the new guidance.

Recommended Action

1. Address the Important issue before merge.
2. Add a regression assertion for repositories without the Paperclip template and commitperclip checker.

[kkroo]

Addressed the remaining Important finding in commit 190458c59ad453f875cb2e587a5ded888f74f510.

The PR workflow and managed AGENTS.md now scope the Paperclip duplicate-search checkbox to repositories whose template/checker actually defines it. Repositories without that template or checkbox follow their own PR-body and duplicate/related-change requirements. The regression now asserts the non-template/non-checker wording and rejects the old unconditional checkbox bullet; the generated catalog was refreshed.

Verification: focused catalog regression passed; catalog manifest validation passed. The broader local suites retain unrelated pre-existing timeout failures.

[kkroo]

@ally Please independently review PR #1477 at the exact new head
190458c. The remaining Important finding from 15a8d069 is addressed by scoping the duplicate-search checkbox to repositories that define it and adding the requested non-template/non-checker regression coverage.

[kkroo]

@ally Retry the independent review of PR #1477 now at exact head
190458c. The stale scheduled retry was cancelled after it coalesced this head into an hour-delayed run; review the template/checkbox scoping fix and explicit non-template/non-commitperclip regression.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 190458c

Prior Findings Dispositioned (1)

• prior:e114c70 important 1 — fixed — packages/skills-catalog/catalog/bundled/software-development/github-pr-workflow/SKILL.md:242 — The current guidance makes the dedup-search checkbox conditional on the repository's checker or template and directs repositories without commitperclip to their own quality gates; the propagated default bundle carries the same conditional rule at server/src/onboarding-assets/default/AGENTS.md:24.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The reusable workflow now scopes Paperclip-specific template and dedup requirements to repositories that actually define them.
• The pre-review gate distinguishes commitperclip repositories from repositories with other quality gates.
• Catalog and managed-bundle regression assertions cover the conditional wording and propagation.
• The generated catalog metadata is refreshed for the changed skill content.

Recommended Action

1. No Critical or Important issues found; safe to merge.
2. Consider Suggestions opportunistically.