Repository navigation
Align workflow tool allowlists with required operations - #67472
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The integrity-filtered fallback still requires issue numbers that the linked failure confirms are withheld.
1 open finding
What changed in this PR
Aligns workflow allowlists and prompts with required review writes, shell commands, Actions reads, and integrity-filtered results.
Changes:
- Enables
safeoutputs,cut, and GitHub Actions tools. - Replaces unauthenticated Actions CLI reads with MCP calls.
- Preserves integrity filtering, though the fallback still incorrectly expects unavailable issue numbers.
| File | Description |
|---|---|
.github/workflows/pr-code-quality-reviewer.md |
Routes review writes through the safeoutputs CLI. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Regenerates the reviewer workflow. |
.github/workflows/daily-compiler-threat-spec-optimizer.md |
Allows cut and improves denial reporting. |
.github/workflows/daily-compiler-threat-spec-optimizer.lock.yml |
Regenerates the optimizer workflow. |
.github/workflows/codex-github-remote-mcp-test.md |
Handles integrity-filtered issue results. |
.github/workflows/codex-github-remote-mcp-test.lock.yml |
Regenerates the Codex test workflow. |
.github/workflows/avenger.md |
Enables Actions MCP reads for CI diagnosis. |
.github/workflows/avenger.lock.yml |
Regenerates Avenger with the Actions toolset. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| - Number of issues retrieved | ||
| - Sample issue numbers and titles | ||
|
|
||
| When contents are filtered, say that sample titles are unavailable rather than inventing or bypassing the filter. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed for PR #67472: the PR does not carry the 'implementation' label (has_implementation_label=false) and adds 0 new lines in default business logic directories (src/, lib/, pkg/, internal/, app/, core/, domain/, services/, api/), well under the 100-line threshold. No custom .design-gate.yml config is present (has_custom_config=false), so default rules apply. 8 files changed.
|
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. Changes are limited to workflow markdown files (.github/workflows/*.md) and their compiled lock files (.lock.yml).
|
|
✅ PR Code Quality Reviewer completed the code quality review. no action
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes Two workflow-contract regressions still block this from doing what the PR claims.Blocking themes
- Avenger Actions triage: the prompt replaces unauthenticated
gh usage with an Actions MCP call signature that does not appear to match the actual tool surface, so the fixer can still fail before reading the first bad job log. - PR reviewer write path: the new safeoutputs example inlines a multiline review body into JSON with
printf; that is the exact quoting path the runtime warns against, and jq is not allowlisted to do this safely.
Those both leave the workflows broken at runtime, not just under-documented.
Blocking themes
- Avenger Actions triage: the prompt replaces unauthenticated
ghusage with an Actions MCP call signature that does not appear to match the actual tool surface, so the fixer can still fail before reading the first bad job log. - PR reviewer write path: the new safeoutputs example inlines a multiline review body into JSON with
printf; that is the exact quoting path the runtime warns against, andjqis not allowlisted to do this safely.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 56.8 AIC · ⌖ 5.61 AIC · ⊞ 19.9K
Comment /review to run again
|
|
||
| Use the CI Run ID from pre-check and identify the first failed job and failing signal before running any local validation: | ||
| Use the GitHub Actions MCP tools, not `gh run view` (the agent's `gh` CLI is unauthenticated), and identify the first failed job and failing signal before running any local validation: | ||
|
|
There was a problem hiding this comment.
This swaps out the unauthenticated gh call for an Actions MCP path, but the prompt now hard-codes a tool contract (actions_list with method: list_workflow_jobs and resource_id) that does not match the Actions tools gh-aw exposes, so Avenger can still fail before it ever reaches the job logs.
💡 Why this blocks the workflow
There is no prior-art usage ofmethod: list_workflow_jobs or resource_id in this repo, while generated manifests expose Actions reads as dedicated MCP tools such as mcp__github__actions_list and mcp__github__get_job_logs. If the agent follows the literal contract here, it will burn a denial or validation error and noop instead of triaging CI. Please describe the actual tool shape the runtime exposes, or keep the step tool-agnostic instead of inventing parameter names.| @@ -154,7 +155,13 @@ You may use compact pseudo-language/encoding during private reasoning (examples: | |||
|
|
|||
| ### Step 4: Write Review Comments | |||
|
|
|||
There was a problem hiding this comment.
This sample command tells the agent to inline the review body directly into JSON with printf, which is exactly the fragile path that breaks once the body contains the required <details> block, quotes, or code fences, so the reviewer can still fail when it tries to post real findings.
💡 Why this is still broken
The new prompt requires every inline review comment to be multi-line and to include structured markdown. The sandbox instructions explicitly warn against building longbody strings inline because shell and JSON quoting become error-prone. The robust pattern is a heredoc plus jq -Rs, but this workflow does not allow jq in tools.bash, so the agent has no reliable way to follow the safe-output guidance you just moved it onto. Please switch the example to the heredoc-plus-jq -Rs pattern and add jq to the allowlist.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — one blocking concern about whether the Avenger fix addresses the actual root cause; everything else is solid.
📋 Key Themes & Highlights
Key Themes
- Avenger root-cause risk:
tools.github.toolsetsnow correctly includesactions(matches theactions: readpermission already granted), but Step 3's body text still instructsgh run view ... --json jobsusing the repo'sghCLI, while the PR description itself says "the agent'sghCLI is unauthenticated" for this workflow. If that's accurate, the repair step will keep failing the same way; if it's not accurate, the stated rationale for other changes in this PR is misleading. Also, the specific "name the exact allowlist entry" fallback text required by issue #67450's acceptance criteria was deleted from Step 3. - safeoutputs CLI wiring (pr-code-quality-reviewer): Correctly follows the established repo convention (seen in
code-scanning-fixer.md,daily-code-debt-aider.md) of invokingsafeoutputsvia the bash CLI rather than the MCP tool directly, and thetools.bashallowlist addition matches. - threat-spec-optimizer / codex-mcp-test: Both changes are narrowly scoped, map directly to the named failure (missing
cut, integrity-filter false-negative), and preserve existing guardrails (no broadening beyond what's needed, filter policy kept intact).
Positive Highlights
- ✅
daily-compiler-threat-spec-optimizer.md:cutadded to both the tool allowlist and the prose list, with a sensible instruction to preserve denial text rather than broadening the allowlist further. - ✅
codex-github-remote-mcp-test.md: Correctly treats integrity-filtered content as an expected policy outcome rather than a test failure, without bypassing the trust filter. - ✅ Compiled lock files were verified via local
go run ./cmd/gh-aw compile— the diffs match source changes with no stray drift.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 129.9 AIC · ⌖ 15.8 AIC · ⊞ 10.3K
Comment /matt to run again
| @@ -30,7 +30,7 @@ tools: | |||
| cli-proxy: true | |||
There was a problem hiding this comment.
[/diagnosing-bugs] Good: toolsets: [default, actions] now matches the actions: read permission already granted at the top of this file, closing the denial. One gap remains — the "If an Actions tool is unavailable" fallback text (previously at the end of Step 3) that named the exact missing allowlist entry was deleted in this PR, even though the issue's "Done when" criterion requires "Missing tool reports name the exact allowlist entry to add." Consider keeping a short fallback note for future regressions (e.g. if actions toolset access is revoked again).
@copilot please address this.
| ``` | ||
| 1. Call `actions_list` with `method: list_workflow_jobs` and `resource_id` set to the CI Run ID from pre-check. | ||
| 2. Identify the first failed job. | ||
| 3. Call `get_job_logs` with that job's ID and inspect the failing output. |
There was a problem hiding this comment.
[/diagnosing-bugs] This mandatory step still invokes gh run view "${{ needs.check_ci_status.outputs.ci_run_id }}" --json jobs using the agent's gh CLI, but line 87 of this same PR's description states "the agent's gh CLI is unauthenticated" and the companion Step-0 fallback at line 161 also reverts to raw gh run list. If that premise is true, this gh run view call will fail the same way the original bug (#67444/#67431) did, and the "If an Actions tool is unavailable" fallback guidance that named the exact tools.github.toolsets: [default, actions] entry was removed, so a future denial will no longer self-document the fix.
💡 Root cause check
Compare with the compiled lock file: toolsets: [default, actions] was reverted to [default] and GITHUB_TOOLSETS dropped actions, yet the body text keeps a gh run view/gh run list based repair flow instead of the actions_list/get_job_logs MCP calls that were in the issue's intended fix. If gh really is unauthenticated for this workflow, Step 0's and Step 3's gh run list/gh run view calls are dead code that will always fail, silently defeating the "CI self-healed" and "inspect failing run" branches. Verify whether gh is authenticated here (it may be, via GH_TOKEN as seen at line 63) — if so, document that explicitly instead of claiming otherwise elsewhere in the PR, since the inconsistency between the PR description and the body text is itself a latent regression risk.
@copilot please address this.
Several workflows were denied required shell, Actions-read, or review-write tools. The Codex MCP test also treated integrity-filtered issue data as a tool failure.
safeoutputsin the PR reviewer and direct it to submit review comments and summaries through the CLI.cutto the threat-spec optimizer’s allowlist and require exact missing-entry details in denial reports.actionstoolset for Avenger and use MCP tools instead of unauthenticatedghcommands.Example review submission: