docs: add supersede-older-reviews, review safe outputs, version baseline#789
Conversation
Major additions to the gh-aw skill: SKILL.md: - Added version baseline note (v0.68.3 is the default installed) - Added supersede-older-reviews: true to security patterns and anti-patterns table — solves the stale blocking review problem - Added Submit PR Reviews section with full YAML example covering submit-pull-request-review, create-pull-request-review-comment, reply-to-pull-request-review-comment, resolve-pull-request-review-thread - Added reply_to_id documentation for add-comment - Added reply-to/resolve review thread anti-pattern rows - Updated allowed-events guidance to present REQUEST_CHANGES as viable when paired with supersede-older-reviews architecture.md: - Added submit-pull-request-review notable options (supersede, footer: if-body) - Added create-pull-request missing options (allowed-base-branches, preserve-branch-name, fallback-as-issue, team-reviewers, max) - Added reply_to_id to add-comment options - Removed stale 'no dismiss-pull-request-review' limitation (supersede-older-reviews solves this) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewer 1 summary — 2 findings, both MODERATE
-
allowed-base-branchesmissing(v0.70.0+)tag (architecture.md:348) — The Limitations table at L385 in the same file says this field failsgh aw compileuntil v0.70.0. Without a version tag, users on the v0.68.3 baseline will hit a compile error. -
Limitations (L375) and Troubleshooting (L417) tables contradict the new
supersede-older-reviewscontent (architecture.md:355–358) — The new section sayssupersede-older-reviews: truesolves stale blocking reviews. Two pre-existing tables still say they "can't be dismissed" and recommend avoidingREQUEST_CHANGESentirely. Agents will generate contradictory guidance.
No other issues found. No security, data-loss, or race-condition concerns.
Generated by Expert Code Review (auto) for issue #789 · ● 19.1M
| - `protected-files: fallback-to-issue` — Create issue instead of failing when protected files modified; optionally add `fallback-labels:` to tag that issue | ||
| - `protected-files: fallback-to-issue` — Create issue instead of failing when protected files modified; optionally add `fallback-labels:` to tag that issue (v0.70.0+) | ||
| - `base-branch: "vnext"` — Target non-default branch | ||
| - `allowed-base-branches: [main, "release/*"]` — Allow agent to override base branch at runtime (glob patterns) |
There was a problem hiding this comment.
🟡 Version-gating omission — will cause compile failure on v0.68.3 baseline
This bullet lists allowed-base-branches with no version tag, but line 385 (Limitations table, same file) says:
allowed-base-branchescompile validation —gh aw compileincorrectly reported ... as unknown field — Fixed in v0.70.0
The skill declares a v0.68.3 baseline (SKILL.md line 17). An agent or user targeting v0.68.3 who adds allowed-base-branches will get a compile error. Every other post-baseline feature is tagged ((v0.70.0+) on gh-proxy, fallback-labels), but this one is not.
Suggested fix: Add (v0.70.0+) — e.g.:
- `allowed-base-branches: [main, "release/*"]` (v0.70.0+) — Allow agent to override base branch at runtime (glob patterns)
| **`submit-pull-request-review` notable options:** | ||
| - `allowed-events: [COMMENT]` — Blocks APPROVE; non-blocking reviews. **Always use this** unless you need merge-gating. | ||
| - `allowed-events: [COMMENT, REQUEST_CHANGES]` — Blocking reviews. Pair with `supersede-older-reviews: true`. | ||
| - `supersede-older-reviews: true` — Auto-dismiss older blocking reviews from the same workflow after posting replacement. Solves the stale-review problem without needing `dismiss-pull-request-review` (which doesn't exist as a safe output). |
There was a problem hiding this comment.
🟡 Internal contradiction — Limitations table (L375) and Troubleshooting table (L417) still say stale reviews "can't be dismissed"
This new section says supersede-older-reviews: true "Solves the stale-review problem." But two pre-existing tables in this same file were not updated and now contradict it:
- L375 (Limitations): "Agent reviews with
REQUEST_CHANGESblock PRs and can't be dismissed" → Workaround: "Useallowed-events: [COMMENT]" - L417 (Troubleshooting): "Agent posted
REQUEST_CHANGESbut can't dismiss it" → Fix: "Switch toallowed-events: [COMMENT]"
An agent consulting this skill will get opposite guidance depending on which section it reads. Since context windows often include partial files, this creates a real risk of contradictory output.
Suggested fix: Update L375 workaround to: "Use supersede-older-reviews: true to auto-dismiss stale reviews, or switch to allowed-events: [COMMENT]". Make an analogous update to L417.
| allowed-events: [COMMENT] # Blocks APPROVE; non-blocking | ||
| # allowed-events: [COMMENT, REQUEST_CHANGES] # Blocking reviews | ||
| # supersede-older-reviews: true # Auto-dismiss stale blocking reviews from same workflow | ||
| footer: "if-body" # Omit footer on approval-only reviews |
There was a problem hiding this comment.
🟢 Minor · 2/3 consensus
The inline comment # Omit footer on approval-only reviews is misleading: the active config on the line above (allowed-events: [COMMENT]) blocks APPROVE events entirely, so "approval-only reviews" are unreachable in this configuration.
footer: "if-body" omits the AI-generated footer when the review body is empty — it has nothing to do with APPROVE events.
Suggestion: # Omit AI footer when review body is empty
| - `protected-files: fallback-to-issue` — Create issue instead of failing when protected files modified; optionally add `fallback-labels:` to tag that issue | ||
| - `protected-files: fallback-to-issue` — Create issue instead of failing when protected files modified; optionally add `fallback-labels:` to tag that issue (v0.70.0+) | ||
| - `base-branch: "vnext"` — Target non-default branch | ||
| - `allowed-base-branches: [main, "release/*"]` — Allow agent to override base branch at runtime (glob patterns) |
There was a problem hiding this comment.
🟢 Minor · 3/3 consensus
allowed-base-branches has no version tag. Per the version baseline note added in this PR, features introduced after v0.68.3 should be tagged (e.g., (v0.70.0+)). Other options in this same diff — protected-files: fallback-to-issue at line 346 — were correctly tagged.
If allowed-base-branches was added after v0.68.3, add the appropriate version tag. The same applies to preserve-branch-name, fallback-as-issue, team-reviewers, and max below if they're also post-baseline features.
| - `allowed-events: [COMMENT]` — Blocks APPROVE; non-blocking reviews. **Always use this** unless you need merge-gating. | ||
| - `allowed-events: [COMMENT, REQUEST_CHANGES]` — Blocking reviews. Pair with `supersede-older-reviews: true`. | ||
| - `supersede-older-reviews: true` — Auto-dismiss older blocking reviews from the same workflow after posting replacement. Solves the stale-review problem without needing `dismiss-pull-request-review` (which doesn't exist as a safe output). | ||
| - `footer: "if-body"` — Omit AI footer on approval-only reviews (cleaner) |
There was a problem hiding this comment.
🟢 Minor · 2/3 consensus (same root cause as SKILL.md line 138)
"Omit AI footer on approval-only reviews (cleaner)" — same misleading description. footer: "if-body" controls whether the footer is appended when the review body is empty, not specifically for approval events.
Suggestion: - \footer: "if-body"` — Omit AI footer when review body is empty (cleaner)`
| footer: "if-body" # Omit footer on approval-only reviews | ||
| create-pull-request-review-comment: | ||
| max: 30 # Inline annotations on diff lines | ||
| side: "RIGHT" |
There was a problem hiding this comment.
🟢 Minor · 2/3 consensus (disputed)
side: "RIGHT" is shown as a YAML-level safe-outputs: config, but one reviewer found no source .md files using it (suggesting it may be a per-call MCP tool parameter only), while another found it present in compiled lock files (suggesting the compiler does accept it).
Worth verifying this is a valid compile-time config rather than only a per-call parameter. If side is only meaningful at call time, removing it from this example avoids confusion.
Manual audit of all 11 releases (v0.68.4 through v0.71.1) found these undocumented features in our skill: New frontmatter features: - pre-agent-steps: (v0.68.5) — steps before agent, after checkout - engine.bare: true (v0.68.6) — skip AGENTS.md for non-code workflows - sandbox.agent.version (v0.69.1) — pin AWF version - redirect: (v0.69.2) — workflow moved/renamed canonical location - comment_memory safe output (v0.69.2) — persistent memory in comments Updated patterns: - protected-files object form with exclude: (v0.68.4) — exclude specific paths like AGENTS.md from default protected set - protected-files object form compilation fix (v0.71.1) Updated watermark: v0.71.0 → v0.71.1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Adds missing features to the gh-aw skill found during review:
supersede-older-reviews — the solution to stale blocking reviews (gh-aw#27655). When using
REQUEST_CHANGES, setsupersede-older-reviews: trueto auto-dismiss older blocking reviews from the same workflow. This makes[COMMENT, REQUEST_CHANGES]viable alongside[COMMENT]-only.Review safe outputs — added full YAML examples and anti-pattern rows for:
submit-pull-request-review(with supersede, footer: if-body)create-pull-request-review-comment(inline annotations)reply-to-pull-request-review-comment(thread replies)resolve-pull-request-review-thread(mark resolved)Version baseline — added note that the skill targets v0.68.3 (default installed version). Features from later versions are tagged.
create-pull-request — added missing options:
allowed-base-branches,preserve-branch-name,fallback-as-issue,team-reviewers,max(multiple PRs per run).