Repository navigation
Align blocked workflow publication with configured tool contracts - #68183
Conversation
Support configured safeoutputs CLI transport, keep review publishing stdout-only, and prepare local blog branches and commits before safe-output PR submission. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🔍 Design Decision Gate 🏗️ is checking for design decision records on this pull request... |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review. PR Code Quality ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
There was a problem hiding this comment.
🟡 Changes recommended
Two workflows mandate inline multiline JSON despite injected guidance warning that this publication pattern may be rejected by the shell guard.
2 open findings
What changed in this PR
Aligns four workflow prompts with their configured safe-output and committed-branch contracts.
Changes:
- Updates CLI-based publication guidance and failure handling.
- Adds local branch/commit preparation for blog PR workflows.
- Adds contract regressions and regenerates workflow locks.
Security review found no permission or credential expansion, but two long-body publication paths conflict with the injected shell-guard guidance.
| File | Description |
|---|---|
.github/workflows/daily-formal-spec-verifier.md |
Revises issue publication transport. |
.github/workflows/daily-formal-spec-verifier.lock.yml |
Regenerates compiled workflow. |
.github/workflows/pr-code-quality-reviewer.md |
Revises review publication commands. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Regenerates compiled workflow. |
.github/workflows/daily-agent-of-the-day-blog-writer.md |
Adds local PR branch preparation. |
.github/workflows/daily-agent-of-the-day-blog-writer.lock.yml |
Regenerates compiled workflow. |
.github/workflows/daily-ecosystem-explorer.md |
Adds local PR branch preparation. |
.github/workflows/daily-ecosystem-explorer.lock.yml |
Regenerates compiled workflow. |
pkg/workflow/prompts_test.go |
Adds workflow contract regressions. |
pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go |
Tests reviewer publication guidance. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| 2. Use the configured safe-output transport. With `cli-proxy`, invoke the `safeoutputs` CLI from `bash`; direct safe-output MCP tools are not exposed. Send a complete JSON object with `title` and `body` on stdin using `printf '%s' '<JSON>' | safeoutputs create_issue .`. JSON-escape the body and shell-quote the payload correctly; do not use shell expansion to construct it. If a direct safe-output tool is actually exposed instead, call it with the same arguments. | ||
| 3. Do **not** use `python3`, `node`, `jq`, or any other scripting runtime to construct or validate the issue payload — build the body string inline. Do not redirect it to a file. |
|
|
||
| Each comment must contain one visible sentence stating the issue and its impact, then a `<details><summary>💡 …</summary>` block with explanation, fix snippet, and rationale. | ||
|
|
||
| Keep publication commands stdout-only: construct JSON inline with `printf '%s'` and pipe it directly to `safeoutputs`. Do not write temporary payload files, use shell redirection or heredocs, or invoke `jq`, `python`, or `node`; they are not authorized by this workflow. JSON-escape content and shell-quote it correctly without shell expansion. Send each safe-output command separately from analysis commands. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd to the publication-contract fixes across four workflow prompts. This is a tightly-scoped, well-evidenced fix with solid regression coverage — no blocking issues found.
📋 Analysis
Root-cause verification
I independently verified the core technical claim behind the Agent of the Day / Ecosystem Explorer fix: create_pull_request (actions/setup/js/create_pull_request.cjs + generate_git_patch.cjs) builds its patch from a committed diff (GITHUB_SHA..HEAD via git format-patch), not from the working tree. The old prompts told the agent create_pull_request would "package the edited docs files" without any local commit — that was a real root-cause bug, not just a denial-message tweak. The new Step 6 instructions (branch via git checkout -b <name>-${{ github.run_id }}, exact-path git add --, local git commit, then branch: passed to the safe output) correctly match the tool's actual contract, and branch is a genuine, already-supported field on create_pull_request (confirmed in collect_ndjson_output.test.cjs). Using github.run_id in the branch name avoids collisions across reruns.
Regression tests
TestBlogWorkflowsPrepareCommittedSafeOutputBranches, TestDailyFormalSpecVerifierDefinesConfiguredSafeOutputContract, and TestPRCodeQualityReviewerWorkflowPublicationContract all pass against the committed PR state (verified by running them directly). They assert both the presence of the new guidance and the absence of the old contradictory guidance, which is good /tdd practice — these tests will catch any regression that reintroduces the old, blocking instructions.
Minor non-blocking observations
- The
daily-agent-of-the-day-blog-writer.md"Tool Restrictions" section (early in the file) and the Step 6 "Open PR" section both describe git usage; the early section is appropriately general (read-only + pointer to Step 6) and Step 6 carries the actual procedure, so this isn't duplication, just distributed documentation. No action needed. pr-code-quality-reviewer.md's stdout-onlyprintfconstraint (no temp files, nojq) is stricter than the general MCP CLI guidance that recommendsjq -Rsfor large bodies — that's intentional for this agent's smaller comment bodies and is explicitly tested, so it's consistent within the PR's own scope.
Positive highlights
- ✅ Each of the 4 workflow fixes is traceable to a specific failed run ID cited in the PR description, giving strong before/after evidence.
- ✅ Negative assertions (
NotContains) in the new tests actively guard against the contradictory instructions silently returning. - ✅ No permission, credential, sandbox, or network changes — scope stayed tight to prompt text + regression tests as described.
No actionable issues found; approving.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 115.3 AIC · ⌖ 15.6 AIC · ⊞ 10.3K
Comment /matt to run again
There was a problem hiding this comment.
💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · 69.7 AIC · ⌖ 6.8 AIC · ⊞ 878
Comment /smoke-claude to run again
|
|
||
| func TestPRCodeQualityReviewerWorkflowPublicationContract(t *testing.T) { | ||
| t.Parallel() | ||
| repoRoot, err := gitutil.FindGitRoot() |
There was a problem hiding this comment.
🔍 Good addition of contract tests here. Consider adding a test case for the error path when the safeoutputs CLI is unavailable, to ensure the workflow fails gracefully with a report_incomplete rather than a silent no-op.
| @@ -347,7 +347,7 @@ func TestDailyCavemanOptimizerUsesConcreteClaudeModelsForExperiment(t *testing.T | |||
| } | |||
There was a problem hiding this comment.
📝 The test name TestDailyFormalSpecVerifierDefinesDirectSafeOutputContract clearly conveys its purpose. Consider also adding an assertion that verifies the prohibition text is present — this would guard against future prompt edits accidentally removing the restriction.
|
🎉 This pull request is included in a new release. Release: |
Summary
Align four workflow-local publication instructions with their existing declared tools and safe-output implementation. Permissions, credential references, sandbox/network configuration, protected-file checks, and output limits are unchanged. Sources and regenerated locks are included together.
create_issuetool was exposed while the prompt explicitly forbade the availablesafeoutputsCLI. Use the configured transport with complete, quoted JSON on stdin; keep structured incomplete reporting and prohibit placeholder/probe calls.jq, not evidence of a missingsafeoutputsallowlist entry. Require separate stdout-onlyprintfpipelines, without broadening permissions, and preserve incomplete reporting if publication remains blocked.create_pull_requestonmainand were rejected because branch equaled base. Their prompts prohibited the local branch/stage/commit preparation required by the existing committed-change tool contract. Permit only local branch creation, exact-path staging, inspection, and commit; keep remote publication exclusively behind safe outputs.Add source-contract regressions that prevent reinstating these contradictory instructions.
Evidence and scope
Inspected existing runs 38271323683, 38265276416, 38263861724, and 38270292718 and compared their first blocked boundary with current main. No raw agent payloads or secret values are included here. No live run was dispatched, no credentials changed, and no issue was closed.
Refs #68041, #68029, #68024, #68035.
PureLock #68008's missing shell capability is handled separately in #68182. The PR review run also had an external AWF sub-agent Responses-to-Chat request-feature translation failure; that remains outside this patch. Hosted publication and approval outcomes remain unverified. This does not duplicate merged #68004, #68000, or #68003.
Validation
make recompile: 338/338 workflows compiled.make agent-report-progress: passed, including formatting/lint/build, impacted Go tests, schema freshness, and full workflow-lock drift.✨ PR Review Safe Output Test - Run 38326766031