docs: fill 10 P2 gaps from exhaustive v0.68.4–v0.71.1 audit - #795
Conversation
Exhaustive Opus audit found 10 undocumented P2 features: Anti-patterns table: - merge-pull-request safe output (v0.70.0) — re-added, confirmed real in release notes and official safe-outputs-pull-requests docs - update-branch on update-pull-request (v0.69.0) Frontmatter features: - merge-pull-request safe output description - safe-outputs.needs for credential supply (v0.69.1) - engine.max-turns tuning knob - Available engines list: copilot, claude, codex, crush (v0.69.0), gemini — crush replaces OpenCode - MCP config at .github/mcp.json (v0.68.5, was .mcp.json) - vulnerability-alerts permission scope (v0.69.3) Breaking changes section: - network.firewall removed in v0.69.2 (gh aw fix --write to migrate) Version baseline updated to v0.71.1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expert Code Review — PR #795Methodology: 3 independent reviewers with adversarial consensus (+ 2 follow-up agents for disputed findings) 9 findings — 7 posted as inline comments (1 moderate-high, 4 moderate, 2 minor), 2 outside diff below. Findings outside the diffThese could not be posted inline because they reference files not changed in this PR.
CI Status
Test CoverageDocumentation-only PR — no test changes expected or needed. No code modified. Discarded FindingsNone — all findings passed consensus.
|
There was a problem hiding this comment.
Expert Code Review: 9 findings posted — 7 inline, 2 in summary (outside diff). See summary comment below for full details and methodology.
Generated by Expert Code Review (auto) for issue #795 · ● 9.7M
| **`on.needs:` (v0.70.0+)** — Express dependencies on custom `pre_activation`/`activation` jobs, enabling GitHub App credentials to be sourced from upstream job outputs. | ||
| **`on.needs:` (v0.70.0+)** — Express dependencies on custom `pre_activation`/`activation` jobs, enabling GitHub App credentials to be sourced from upstream job outputs. See also `safe-outputs.needs` (v0.69.1+) for credential-supply dependencies in the safe-outputs job. | ||
|
|
||
| **`merge-pull-request` safe output (v0.70.0+)** — Merge a PR directly as a safe output. Executes in the safe-outputs job with proper write permissions, not inside the agent container. |
There was a problem hiding this comment.
🔴 MODERATE · 2/3 reviewers · merge-pull-request lacks security guidance
merge-pull-request is a more powerful operation than submit-pull-request-review (which has extensive guards: allowed-events, supersede-older-reviews). This entry documents it only as "executes in the safe-outputs job with proper write permissions" without noting:
- What
permissions:block is required (contents: write?pull-requests: write?) - Whether an
allowed-events-style guard exists to restrict invocation - Risk if used on a low-integrity-triggered workflow (e.g., fork PR slash command)
Scenario: An author adds merge-pull-request to a slash-command workflow without additional guards, and an untrusted actor triggers a merge.
Suggestion: Add a security note analogous to the submit-pull-request-review guards — at minimum: required permissions and recommended pairing with manual-approval: or integrity constraints.
|
|
||
| ### Breaking Changes |
There was a problem hiding this comment.
🟡 MODERATE · 2/3 reviewers · "Supported runtimes" orphaned under Breaking Changes
The new ### Breaking Changes heading is inserted immediately before the Supported runtimes: line, making runtimes appear to be part of the breaking changes subsection. A reader scanning the doc structure will misinterpret the runtimes list.
Suggestion: Move Supported runtimes: above the ### Breaking Changes heading, or add a separator/heading between them.
| **`comment_memory` safe output (v0.69.2+)** — Agents can persist structured memory in a managed issue/PR comment. Memory files are materialized under `/tmp/gh-aw/comment-memory/` before the agent runs and synced back after. Enables stateful agents across runs without external storage. | ||
|
|
||
| **`on.needs:` (v0.70.0+)** — Express dependencies on custom `pre_activation`/`activation` jobs, enabling GitHub App credentials to be sourced from upstream job outputs. | ||
| **`on.needs:` (v0.70.0+)** — Express dependencies on custom `pre_activation`/`activation` jobs, enabling GitHub App credentials to be sourced from upstream job outputs. See also `safe-outputs.needs` (v0.69.1+) for credential-supply dependencies in the safe-outputs job. |
There was a problem hiding this comment.
🟢 MINOR · 2/3 reviewers · safe-outputs.needs buried as parenthetical
safe-outputs.needs is listed as one of the 10 features in the PR description but only appears as a cross-reference sentence appended to the on.needs: entry. A user on v0.69.1–v0.69.x will skip the on.needs: entry (v0.70.0+) and never see this.
Suggestion: Give it a dedicated bold entry: **safe-outputs.needs (v0.69.1+)** — Express credential-supply dependencies in the safe-outputs job... with a minimal YAML example, matching the style of other entries in this section.
| | No guard against agent approving PRs | `allowed-events: [COMMENT]` on `submit-pull-request-review`; or `[COMMENT, REQUEST_CHANGES]` with `supersede-older-reviews: true` to auto-dismiss stale blocking reviews | [Safe Outputs](https://github.github.com/gh-aw/reference/safe-outputs-pull-requests/) | | ||
| | Stale blocking reviews from previous `/review` runs | `supersede-older-reviews: true` on `submit-pull-request-review` — dismisses older same-workflow `REQUEST_CHANGES` reviews after posting replacement | [Safe Outputs](https://github.github.com/gh-aw/reference/safe-outputs-pull-requests/) | | ||
| | Merging PRs via shell `gh pr merge` in post-steps | Use `push-to-pull-request-branch` + branch protection auto-merge, or `dispatch-workflow` to trigger a merge workflow | [Safe Outputs](https://github.github.com/gh-aw/reference/safe-outputs/) | | ||
| | Merging PRs via shell `gh pr merge` in post-steps | `merge-pull-request` safe output (v0.70.0+) — executes in the safe-outputs job with proper permissions | [Safe Outputs](https://github.github.com/gh-aw/reference/safe-outputs/) | |
There was a problem hiding this comment.
🟡 MODERATE · 2/3 reviewers · Old workaround removed without fallback
The previous row recommended push-to-pull-request-branch + branch protection auto-merge, or dispatch-workflow — valid for users on v0.68.3–v0.69.x. This replacement makes the old (still working) approach undiscoverable.
Scenario: A user pinned to v0.69.x reads this table, tries merge-pull-request, gets a compile error, and has no fallback guidance — potentially falling back to the original anti-pattern (gh pr merge in shell).
Suggestion: Add a parenthetical: (pre-v0.70.0: use push-to-pull-request-branch + auto-merge or dispatch-workflow)
|
|
||
| ### Breaking Changes | ||
|
|
||
| **`network.firewall` removed (v0.69.2)** — This deprecated frontmatter key is now rejected by the compiler. Migrate with: `gh aw fix --write`. |
There was a problem hiding this comment.
🟡 MODERATE · 3/3 reviewers · network.firewall migration guidance incomplete
Two concerns:
-
Missing replacement: The entry says the key is "rejected by the compiler" and to run
gh aw fix --write, but doesn't explain what replaces the functionality. Did network egress control move to a different key, or was it removed entirely? A user with legitimate egress restrictions won't know if running the fix command silently drops their protections. -
gh aw fixnot in CLI reference: The CLI commands section (lines 37–43) listscompile,run,status,trial,audit,upgrade— but notfix. If this subcommand doesn't exist, the guidance will fail.
Suggestion: (1) Add one sentence about the replacement (e.g., "egress is now unrestricted" or "use network.allowed: instead"). (2) Add gh aw fix to the CLI commands section, or provide the manual migration (remove the key from frontmatter).
|
|
||
| **`checkout: false`** — Skip the default repository checkout when the workflow doesn't need source code (e.g., ChatOps commands that only call APIs via `web-fetch`). Saves ~10-30s of runner time. | ||
|
|
||
| **`engine.max-turns`** — Limit the number of turns the agent can take. Set in the engine block: `engine: { id: copilot, max-turns: 15 }`. Preserved through shared imports (fixed in v0.68.3). |
There was a problem hiding this comment.
🟡 MODERATE · 3/3 reviewers · engine.max-turns missing version tag
Every other entry in this section uses the (vX.Y.Z+) convention. This entry says "fixed in v0.68.3" but doesn't clarify when max-turns was introduced. A user can't tell if it's available on all versions or only post-v0.68.3.
Suggestion: Add an explicit version tag, e.g., **engine.max-turns (v0.68.3+)** — or clarify that it predates the baseline and the v0.68.3 note is only about the shared-imports fix.
|
|
||
| **MCP config location:** `.github/mcp.json` (v0.68.5+ — previously `.mcp.json` at repo root). Migrate existing configs manually. | ||
|
|
||
| **`vulnerability-alerts` permission (v0.69.3+)** — Available as a `GITHUB_TOKEN` permission scope for workflows that need to read security alerts. |
There was a problem hiding this comment.
🟢 MINOR · 2/3 reviewers · vulnerability-alerts lacks explicit permission level
GitHub permission scopes require an access level (read/write). While "workflows that need to read security alerts" implies read, showing the explicit YAML syntax (e.g., vulnerability-alerts: read) would prevent users from writing vulnerability-alerts: true (which may cause a compile error).
Suggestion: Add the explicit syntax: permissions: { vulnerability-alerts: read }
Drift workflow PR #794 found 9 undocumented integrity features: - allowed-repos scoping, approval-labels full docs - GH Actions expressions for blocked/trusted/approval-labels - integrity-proxy: false opt-out - Centralized GH_AW_GITHUB_* env vars - Effective integrity computation order - DIFC_FILTERED logging + gh aw logs --filtered-integrity - Public repos auto-apply approved, private default none - Reaction-based integrity (v0.68.2+) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Opus audit of all 11 releases found 10 undocumented author-facing features:
merge-pull-requestsafe output (v0.70.0) — re-added after confirming in official docsupdate-branchonupdate-pull-request(v0.69.0)safe-outputs.needsfor credential supply (v0.69.1)engine.max-turnstuning knob.github/mcp.json(v0.68.5, was.mcp.json)vulnerability-alertspermission scope (v0.69.3)network.firewallbreaking removal (v0.69.2) + migration guideAlso: drift workflow (PR #794) independently found 9 integrity filtering coverage gaps.