Fix PR Review Panel: emit advisory comment via add_comment tool, not a shell heredoc - #1844
Fix PR Review Panel: emit advisory comment via add_comment tool, not a shell heredoc#1844Daniel Meppiel (danielmeppiel) with Copilot wants to merge 3 commits into
Conversation
Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com>
…edoc Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com>
Carry the distinct #1844 safeguard into the canonical panel package and deployed ledger without recreating historical mirrors. Keep safe outputs fail-closed, prove transport clauses and provenance, and distinguish legacy unpack from plugin install in the actual pack handoff. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Superseded by #2897, which preserves this proposal's distinct structured-comment safeguard. At replacement commit The deployed skill, canonical and compatibility hashes in Replacement evidence: the final current-head advisory, exact-head CI, and published APM 0.30.0 panel-resource roundtrip. The same nine retained reviewers and final CEO synthesis are complete. Local real-process restoration preserved all 12 actual package files; hosted deterministic compatibility covers four required files. Neither local CLI advisory delivery nor those checks is presented as hosted AI safe-output execution. Closing this superseded proposal only, under the recovered preservation-and-linking authority. #2897 is ready for human review, not accepted or merged: CODEOWNER and independent last-push approvals remain outstanding. The fresh trusted-main #2741 AI exercise remains a post-merge human rollout obligation. Original branch |
Description
The
PR Review Panelworkflow's agent job terminated unexpectedly (run 27815857237, 33m / 144 turns, no comment posted).gh aw auditshows the CEO synthesizer produced a validneeds_reworkrecommendation, but the orchestrator then staged the rendered comment body in a temp file with a shell heredoc:The copilot engine's bash command-safety parser scans command text (heredoc bodies included) and read a word-wrapped line beginning with
killas a real command, refusing it: "Command not executed. The 'kill' command must specify at least one numeric PID." The comment never emitted and the run failed. Theadd_commentsafe-output tool already takes the body as a structured argument, so panel prose never needs to touch the shell.Changes (prose-only in the skill):
apm-review-panelSKILL step 7 — mandate emitting the single comment by passing the rendered markdown directly to theadd_commenttool'sbodyargument; forbid staging/piping it through the shell (cat <<EOF,printf,echo, temp.mdfiles), with thekill/rm/sudoparser hazard called out..agents/skills/apm-review-panel/SKILL.mdcopy (kept byte-identical).gh aw auditartifacts under.github/aw/logs/and gitignored that path.No workflow recompile needed — the workflow imports the skill from
microsoft/apm#mainat runtime, so.lock.ymlis unaffected.Type of change
Testing
Spec conformance (OpenAPM v0.1)
If this PR changes behaviour that an OpenAPM v0.1
req-XXXcovers,confirm the three-step ritual (see CONTRIBUTING.md "Adding or
changing a normative requirement"):
docs/src/content/docs/specs/openapm-v0.1.mdupdated(new/changed
<a id="req-XXX"></a>anchor + prose + Appendix Crow).
docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.ymlupdated.
@pytest.mark.req("req-XXX")test undertests/spec_conformance/added or extended.CONFORMANCE.{md,json}regenerated viauv run --extra dev python -m tests.spec_conformance.gen_statementand committed.