Skip to content

Commit 7916a40

Browse files
committed
feat(skill): draft truthful replies in pr-feedback-quality-gate (#6317)
The bundled pr-feedback-quality-gate skill inspected PR feedback, applied review fixes, validated them, and ran a read-only cross-review before pushing follow-up commits. It had no advisory path for requests like "continue PR; new responses appeared; tell me how to answer them," which left two risks: the agent could miss a still-open request after a later approval, or imply a change had happened when it hadn't. Add a read-only reply-advice mode: deterministic target resolution, full feedback-surface inspection (issue comments, review summaries, inline threads, checks, local worktree), checkpoint/freshness semantics that call the result "current feedback" rather than "new feedback" without a baseline, five-way classification (action required / question / acknowledgement / informational-approval / stale), truthful drafting rules that forbid claiming performed changes that haven't happened, and the read-only authority boundary (no edits, comments, thread resolutions, commits, pushes, or check reruns without separate user authorization). The mutating fix workflow stays opt-in below reply-advice so advisory intent is never mistaken for authorization. Add English (`continue PR`, `reply to PR feedback`, `answer PR feedback`) and Polish (`kontynuuj PR`, `odpowiedz na uwagi do PR`) continuation/reply triggers and an `od.example_prompt` that demonstrates a read-only "inspect and draft, do not post" request, all surfaced through the runtime skill registry. Add a focused `listSkills(skillsRoot)` contract test in `apps/daemon/tests/skills.test.ts` that verifies the triggers and example_prompt are exposed as runtime metadata and that the composed body contains the classification rules, truthful-draft markers, full review-surface inspection, the read-only authority boundary, and the preserved mutating fix/cross-review workflow. All 22 skills tests pass; `pnpm guard` stays green. Closes #6317. Signed-off-by: xxiaoxiong <2482929840@qq.com>
1 parent 517f39a commit 7916a40

2 files changed

Lines changed: 178 additions & 4 deletions

File tree

apps/daemon/tests/skills.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,70 @@ describe('listSkills', () => {
298298
expect(skill.body).toContain('pkill -f -- "--user-data-dir=${CHROME_USER_DATA_DIR}"');
299299
});
300300

301+
it('exposes reply-advice mode on the pr-feedback-quality-gate skill (#6317)', async () => {
302+
const skills = await listSkills(skillsRoot);
303+
const skill = skills.find((entry: { id: string }) => entry.id === 'pr-feedback-quality-gate');
304+
305+
if (!skill) throw new Error('pr-feedback-quality-gate skill not found');
306+
307+
// English and Polish continuation/reply triggers surfaced via the runtime
308+
// registry so the user-facing skill picker actually offers them.
309+
expect(skill.triggers).toContain('continue PR');
310+
expect(skill.triggers).toContain('reply to PR feedback');
311+
expect(skill.triggers).toContain('answer PR feedback');
312+
expect(skill.triggers).toContain('kontynuuj PR');
313+
expect(skill.triggers).toContain('odpowiedz na uwagi do PR');
314+
315+
// An explicit English `od.example_prompt` is advertised as `examplePrompt`
316+
// and demonstrates a read-only "inspect and draft, do not post" request.
317+
expect(skill.examplePrompt).toBeTruthy();
318+
expect(typeof skill.examplePrompt).toBe('string');
319+
expect(skill.examplePrompt).toContain('Continue PR');
320+
expect(skill.examplePrompt).toContain('draft');
321+
expect(skill.examplePrompt).toContain('Do not post');
322+
323+
// The composed skill body exposes the reply-advice classification rules.
324+
const body = skill.body ?? '';
325+
expect(body).toContain('Reply-advice mode (read-only)');
326+
expect(body).toContain('action required before replying');
327+
expect(body).toContain('question requiring an answer');
328+
expect(body).toContain('acknowledgement after a completed change');
329+
expect(body).toContain('informational / approval / automated — no reply required');
330+
expect(body).toContain('stale, outdated, or already resolved');
331+
332+
// Truthful drafting rules — never claim a change has been made when it
333+
// has not; never claim new validation has run when it has not.
334+
expect(body).toContain('Never claims a change has been made when it has not');
335+
expect(body).toContain('Never claims new validation has run when it has not');
336+
expect(body).toContain('current feedback');
337+
expect(body).toContain('rather than');
338+
expect(body).toMatch(/never\s+`new feedback`|not.*`new feedback`|call the result `current feedback`/i);
339+
340+
// Full review-surface inspection — every surface enumerated, and a surface
341+
// with no items is itself an observation rather than something to skip.
342+
expect(body).toContain('Issue (timeline) comments');
343+
expect(body).toContain('Review summaries');
344+
expect(body).toContain('Inline review threads');
345+
expect(body).toContain('Status checks');
346+
expect(body).toContain('Local worktree status');
347+
expect(body).toContain('A surface with no items is itself an observation');
348+
349+
// Read-only authority boundary in reply-advice mode — no edits, comments,
350+
// thread resolutions, commits, pushes, or check reruns without separate
351+
// authorization.
352+
expect(body).toContain('Reply-advice mode is the default path');
353+
expect(body).toContain('NEVER edits PRs');
354+
expect(body).toContain('resolves threads');
355+
expect(body).toContain('commits, pushes');
356+
expect(body).toContain('reruns checks');
357+
358+
// Existing minimal-fix, validation, and read-only cross-review rules
359+
// remain present in the mutating workflow below reply-advice mode.
360+
expect(body).toContain('mutating, opt-in only');
361+
expect(body).toContain('read-only cross-review');
362+
expect(body).toContain('pnpm guard');
363+
});
364+
301365
it('keeps html-ppt PNG export on one managed Chromium screenshot path', async () => {
302366
const skills = await listSkills(designTemplatesRoot);
303367
const skill = skills.find((entry: { id: string }) => entry.id === 'html-ppt');

skills/pr-feedback-quality-gate/SKILL.md

Lines changed: 114 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,133 @@
11
---
22
name: pr-feedback-quality-gate
33
description: |
4-
Safely track pull request feedback, resolve review comments or merge conflicts, validate fixes, and use a read-only cross-review before committing or pushing follow-up changes.
4+
Safely track pull request feedback, resolve review comments or merge conflicts, validate fixes, and use a read-only cross-review before committing or pushing follow-up changes. Also drafts truthful, ready-to-copy replies when the user asks how to answer new or current PR feedback without authorizing mutations.
55
triggers:
66
- "PR feedback"
77
- "review comments"
88
- "merge conflicts"
99
- "cross-review"
1010
- "Claude CLI review"
1111
- "monitor PR"
12+
- "continue PR"
13+
- "reply to PR feedback"
14+
- "answer PR feedback"
15+
- "kontynuuj PR"
16+
- "odpowiedz na uwagi do PR"
1217
od:
1318
mode: utility
19+
example_prompt: "Continue PR #6016 — inspect current feedback, classify each item, and draft truthful copy-paste replies for anything actionable. Do not post, resolve threads, push commits, or edit the PR; only read and advise."
1420
---
1521

1622
# PR Feedback Quality Gate
1723

1824
Use this when a PR has review feedback, merge conflicts, pending checks, or
19-
needs a monitored follow-up after a fix.
25+
needs a monitored follow-up after a fix, and use this when the user asks how to
26+
answer PR feedback (continue PR, reply to reviewer, kontynuuj PR, odpowiedz na
27+
uwagi do PR) without authorizing mutations.
2028

21-
## Workflow
29+
## Reply-advice mode (read-only)
30+
31+
Reply-advice mode is the default path whenever the user asks to continue a PR,
32+
review feedback, or draft replies, AND has not separately authorized the
33+
mutating fix workflow below. Reply-advice NEVER edits PRs, posts comments,
34+
resolves threads, commits, pushes, or reruns checks. It only inspects and
35+
advises. The mutating fix workflow runs only on an explicit separate request
36+
("apply the fix", "commit", "push", "resolve the thread"); until then, stay
37+
read-only.
38+
39+
### Target resolution precedence
40+
41+
Resolve the target PR deterministically before any inspection:
42+
43+
1. Explicit URL or number supplied by the user.
44+
2. The PR open on the current git branch (`gh pr view` from the worktree).
45+
3. A single open PR authored by the current user in the current repository.
46+
47+
If two or more candidates remain, list them with PR number, title, head SHA,
48+
author, and current review decision; then STOP. Do not pick silently. Let the
49+
user disambiguate before any inspection.
50+
51+
### Feedback surfaces to inspect
52+
53+
Read every surface that can carry actionable feedback on the resolved PR:
54+
55+
- PR body, current head SHA, and mergeable state.
56+
- Issue (timeline) comments on the PR.
57+
- Review summaries (approve / request changes / comment) and reviewer bodies.
58+
- Inline review threads, including resolved and outdated state per thread.
59+
- Status checks (`gh pr checks`) and mergeability (`gh pr view --json
60+
mergeable,mergeStateStatus`).
61+
- Local worktree status (`git status`, `git log`) when relevant.
62+
63+
A surface with no items is itself an observation ("no inline review threads
64+
present on this PR"), not a gap to skip.
65+
66+
### Freshness and checkpoint semantics
67+
68+
Compare against a prior report or conversation checkpoint when one exists in
69+
this session. Without a checkpoint, call the result `current feedback` — never
70+
`new feedback`. Saying `new` without a comparison baseline is a fabrication
71+
risk; only the delta over a recorded observation is `new`.
72+
73+
### Classification
74+
75+
Classify every relevant item on every inspected surface as exactly one of:
76+
77+
- **action required before replying** — a request that must be addressed
78+
(code change, PR body update, validation, doc edit) before any truthful
79+
reply can claim it is done.
80+
- **question requiring an answer** — a reviewer question that needs a
81+
response, with no implicit code change required.
82+
- **acknowledgement after a completed change** — a reviewer note reacting to
83+
an already-completed fix; reply optionally, briefly.
84+
- **informational / approval / automated — no reply required** — CI runs,
85+
Looper approvals, status updates, automation logs. These are not human
86+
feedback. Do not draft a substantive reply; an optional courtesy `thanks for
87+
the review and approval` is the maximum allowed.
88+
- **stale, outdated, or already resolved** — threads marked resolved/outdated
89+
by GitHub, or items superseded by a newer change. Acknowledge their state
90+
once; do not relitigate.
91+
92+
An item may have a single classification. If two classifications apply, the
93+
most actionable one wins (a question that is also an approval is still a
94+
question).
95+
96+
### Truthful drafting rules
97+
98+
For every actionable item, write a ready-to-copy reply that:
99+
100+
- States the smallest required action and why, before any draft reply.
101+
- Never claims a change has been made when it has not. If the user has not
102+
separately run the fix, the draft must say `Once X is done, the suggested
103+
reply is:` and present the reply as conditional, not past tense.
104+
- Never claims new validation has run when it has not. If the user has not run
105+
tests, say `after you run \`pnpm X\`` rather than `I ran \`pnpm X\``.
106+
- Mirrors the reviewer's language register and addresses the reviewer by name
107+
when known.
108+
- Is concise — one short paragraph per actionable item, not a wall of text.
109+
110+
### Output shape
111+
112+
Reply-advice output, in order:
113+
114+
1. PR state and freshness — number, title, head SHA, review decision,
115+
mergeable state, last update timestamp, and the comparison baseline (or
116+
`no prior checkpoint — current feedback, not new`).
117+
2. Feedback item — author, source (issue comment / review summary / inline
118+
thread / check), timestamp, classification.
119+
3. Required action and why (when classification is `action required` or
120+
`question`).
121+
4. Suggested reply, or explicit `no reply needed` rationale. Conditional
122+
language if the underlying action has not yet been performed.
123+
5. Checks and mergeability — label unknown causes as `unknown`; do not
124+
invent a blocker.
125+
6. Remaining risk and next step — the smallest action that unblocks the PR.
126+
127+
## Workflow (mutating, opt-in only)
128+
129+
This workflow runs only on an explicit mutating request from the user. It does
130+
not run inside reply-advice mode.
22131

23132
1. Inspect PR state first: comments, reviews, mergeability, checks, branch, and
24133
local worktree status. Keep unrelated local changes out of the PR.
@@ -50,4 +159,5 @@ needs a monitored follow-up after a fix.
50159

51160
Always report PR state, actions taken, cross-review verdict, accepted or
52161
rejected findings, validation run, commits pushed, skipped checks with reasons,
53-
remaining risks, and next step.
162+
remaining risks, and next step. In reply-advice mode, replace `actions taken`
163+
with `actions advised` and `commits pushed` with `no mutations performed`.

0 commit comments

Comments
 (0)