Skip to content

feat(source-control): add the pull-request evidence posting step - #11854

Merged
melodic-automation-lanes[bot] merged 4 commits into
mainfrom
work-resolve-issue/issue-11391-41278026771-1
Nov 1, 2026
Merged

melodic-automation-lanes[bot] merged 4 commits into
mainfrom
work-resolve-issue/issue-11391-41278026771-1

Conversation

@melodic-automation-lanes

@melodic-automation-lanes melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Closes #11391

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Adds /source-control:pull-request evidence. Once the verifier has recorded CONFIRMED for the PR head (confirm-verdict.sh), it posts one evidence comment marked ``, or updates it in place, with gh pr comment --edit-last --create-if-none --attach. The comment holds the text summary, the publishable captures in collapsed `<details>` sections laid out by `templates/evidence-comment.md`, and the `pr-capture-<pr>-<head>` artifact link with its expiry, or `Media not uploaded: <reason>`. It posts nothing in these cases: a file outside the run's `publishable` list, a leak-scan hit (drive's Mask post-check values and pattern), or a login that has other comments on the PR. A video is attached only when its every-frame sidecar passed. When gh rejects `--attach` beside `--edit-last`, it falls back to delete and repost. Design choices, each on evidence: the publishable list is read from run-e2e's `e2e-summary.json` or drive's `report.json`, the inputs #11387/#11388 shipped. The verifier gate reuses the plugin's existing confirm-verdict record. Captures are placed inside the sections by markdown image references to their absolute paths, which gh rewrites in place (cli/cli internal/attachments/references.go). The playwright `posting.md` text is copied into the new reference; trimming the playwright file and having demo-video call this step is sibling #11392's work, so this PR keeps to one plugin.

Not verified

  • /skill-quality:check pull-request (the issue's Sanity Check) is not loaded in this lane, and check-affected skips changed-skills because it runs the checked tree's own code. check-evals-quality passed as a partial stand-in. Left to the PR's CI
  • Running evals 61 to 63 against a model (at most 2 runs per case per the issue's run budget): a test run this lane cannot execute. Left to the PR's CI or a person
  • Live probe that gh pr comment --edit-last accepts --attach (the owner's 'first task'): this lane has no gh and no PR. gh trunk source does not reject the pair, and the step carries the delete-and-repost fallback either way. evidence.md's As-of line records that no live probe has run
  • ci-status green on the PR head: a PR check, left to the PR's CI
  • plugins/playwright/skills/demo-video/reference/posting.md still holds its text and artifact-link sections. Their content is now in evidence.md; trimming posting.md and having demo-video call evidence is sibling #11392's work, left out to keep this PR to one plugin
  • The verifier gate reads the local /verification:confirm record (confirm-verdict.sh). A CI caller gating on the pr-verify lane's PASS (the merge-gate posting in sibling #11393) needs its own gate input, which this PR does not add
  • tidy recorded skipped: passes 1 to 4 ran with no edits. Pass 5 (ai-slop fix) could not run its detect.sh, digest guard or semantic-diff scripts, because this lane's Bash runs only its named wrappers. A Grep for dashes and catalog vocabulary plus a rubric read stood in for it, with no edits
  • verify recorded skipped: the change is markdown and eval JSON only, with no build and no script test that covers these files
  • typos check skipped by check-affected (typos not installed)
  • The checkout root holds untracked files this run did not create (.bash_profile, .bashrc, .gitconfig, .idea, .mcp.json, .profile, .ripgreprc, .vscode, .zprofile, .zshrc). They are environment files, not part of this change, and should stay out of the patch

Fix

  • plugins/source-control/skills/pull-request/reference/evidence.md: new phase file for the evidence action: inputs, the verifier gate, the publishable-list refusal, the video frame-masking rule, template rendering, the leak scan (trusted_root hub), the single-identity guard, the gh --attach version
  • plugins/source-control/skills/pull-request/templates/evidence-comment.md: new comment template: the marker line, the head, the summary, repeated collapsed &lt;details&gt; sections with capture references, the artifact line
  • plugins/source-control/skills/pull-request/SKILL.md: evidence added to the argument-hint (|view dropped to keep it within the 100-character budget), the Arguments line, the Arguments table and the phase table
  • plugins/source-control/skills/pull-request/reference/create.md: the Visual evidence paragraph no longer says there is no upload option or asks the person to drag files; it points at the evidence step as the attach path, with an updated pointer, as-of and recheck trigger
  • plugins/source-control/reference/providers/github/pull-request/create.md: the stale 'gh pr create has no option that uploads a file' line now says create passes no --attach and captures go through the evidence action
  • plugins/source-control/skills/pull-request/evals/evals.json: evals 61 (a file not in the publishable list is refused, nothing posted), 62 (a second head edits the existing comment, not a second one) and 63 (no artifact gives the text summary and Media not uploaded:)
  • .changes/source-control/work-resolve-issue-issue-11391-8d3f1a64.md: minor changelog fragment (Added: evidence action; Changed: create's Visual evidence)

Verification

Evidence: both review threads are settled at e12b695 (evidence.md:69-75 numbered copy subdirectories; evidence.md:103-107 pins gh@2.102.0), and pr-require-checks run 41286632888 passed at this head.

The runner ran each check from the base copy against 1ad2b59:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • read https: //cli.github.com/manual/gh_pr_comment: lists --attach (image or video, &lt;file&gt;#&lt;alt&gt;, up to 50 files, unreferenced attachments appended), --edit-last, --create-if-none ('Can be used only with --edit-last'), --delete-last and --yes for the delete prompt; says nothing about wh
  • read https: //raw.githubusercontent.com/cli/cli/trunk/pkg/cmd/pr/shared/commentable.go: --attach is rejected only with --web and --delete-last, not with --edit-last; 'last comment' is the last of CurrentUserComments() (the logged-in user's); the body is uploaded and attached before CommentUpdate
  • read https: //github.com/cli/cli/releases/tag/v2.99.0: --attach was added to issue/pr create, edit and comment; a body reference to the local path is replaced with the uploaded URL, else the attachment is appended
  • read https: //github.com/cli/cli/tree/trunk/internal/attachments: directory listing (attach.go, references.go, doc.go, and others)
  • read https: //raw.githubusercontent.com/cli/cli/trunk/internal/attachments/references.go: inline and reference-style markdown image and link destinations are matched by absolute path and rewritten; raw HTML img tags and code spans are not; a video alone in its paragraph becomes the bare player URL
  • read https: //raw.githubusercontent.com/cli/cli/trunk/internal/attachments/doc.go: an upload cannot be undone; when nothing uploads, nothing is written
  • check-changelog-fragments: all 32 fragments valid (before and after the review fixes)
  • check-affected: first run failed validate-plugins (argument-hint 105 characters, over the 100 budget); after shortening it, 11 passed, 7 skipped over 7 changed files, and the same again after the review fixes
  • check-evals-quality plugins/source-control/skills/pull-request/evals/evals.json: PASS, 5 warnings, all on earlier cases (ids 9, 12, 17, 18, 20), none on 61 to 63
  • fresh-context review subagent over the whole diff: 8 findings. Fixed: the GH -cannot-attach path now re-renders without capture references and keeps the artifact link; the fallback posts a fresh comment when none exists; more than one marker comment is refused; the default summary from report.json n
  • Grep over the new markdown for em and en dashes and the ai-slop vocabulary list: no hits
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41278026771

Left out: the items under Not verified.

Related

#11391

Lane-Activity: work-resolve-issue/resolve-issue
@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head e12b6954266e1c252a4e63a0c2c4a2493753ca07. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:missing)

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed e12b6954266e1c252a4e63a0c2c4a2493753ca07: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental, fe40ed9 → e12b695): no security issues found.

Scope: the two files changed since the last review, plugins/source-control/skills/pull-request/reference/evidence.md and plugins/source-control/skills/pull-request/templates/evidence-comment.md. I read every hunk of both in .claude-lane/incremental.diff.

What changed: the template's ![{{capture_alt}}]({{capture_path}}) became a single {{capture_image}} placeholder. The reference now describes that placeholder as the whole ![<alt>](<path>) reference, with the same absolute path and the file name as alt text. The stated reason is that the offline link check reads a placeholder link target as a missing local file.

What I checked:

  • Path handling: the path character rule (ASCII letters, digits and /._- only) and the scratchpad-copy fallback are unchanged (evidence.md L65-L76). So the unparsed-reference risk this rule guards against, a local path ending up in the published comment, is handled exactly as before.
  • Leak scan: step 5 still scans the fully rendered body before anything is posted (evidence.md L82-L90), so the combined placeholder sends nothing past it.
  • Other gates: the verifier gate, the publishable-list refusal and the single-identity guard are untouched.
  • Instructions: no rule was removed or weakened. The rewrite rewords how the placeholder is filled without changing what is forbidden.

No trusted-path files are in this increment. There are no manifest changes, so no manifest lines apply.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 3 inline finding(s) on 1ad2b59e03644ca042be03d25be20eabe4f8f97c; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed e12b6954266e1c252a4e63a0c2c4a2493753ca07: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental since fe40ed9)

Scope: plugins/source-control/skills/pull-request/reference/evidence.md and plugins/source-control/skills/pull-request/templates/evidence-comment.md. I read every hunk of both files in .claude-lane/incremental.diff. I also read the surrounding head text of evidence.md (steps 4 to 9) for context.

What changed: The template no longer builds the image reference from two placeholders (![{{capture_alt}}]({{capture_path}})). It now uses one placeholder, {{capture_image}}. evidence.md step 4 now defines {{capture_image}} as the whole ![<alt>](<path>) reference. It gives the reason for the change: the offline link check reads a placeholder link target as a missing local file. The rules that came with the old placeholders are unchanged: the ASCII-only path restriction, the scratchpad copy into numbered subdirectories, and the use of the copy's path in both the reference and its --attach.

Checks:

  • The template and the reference now match. No capture_alt or capture_path is left anywhere under plugins/source-control.
  • The step 7 fallback ("render the body again from step 4 with no captures (no {{#capture}} blocks…)") still works. It removes the whole {{#capture}} block, so no local path reaches the comment under the new placeholder either.
  • The step 5 leak scan runs on the rendered body. It is unaffected because the reference is filled in before the scan.
  • The change keeps to the owner's Q12 decision: the layout still comes from a template file the skill reads, with captures inside collapsed <details> sections.

Findings: None. One minor formatting point that doesn't count as a finding: the reflow leaves Copy a capture alone on a short line (evidence.md:68). It renders the same in Markdown.

Not verified by this lane: I did not run the offline link check or render the template. The claim that the old placeholders tripped the link check comes from the author and I have not verified it.

Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6yXx3P
Lane-Thread: PRRT_kwDOTCGFQM6yXx3Q
Lane-Thread: PRRT_kwDOTCGFQM6yXx3R

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on f7638c6e8ec5296ec6ca521fd2a5e15a077a95f9; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on f7638c6e8ec5296ec6ca521fd2a5e15a077a95f9; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on f7638c6.

Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6yX1ht
Lane-Thread: PRRT_kwDOTCGFQM6yX1jT
@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 1, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=11854 -f activity=address-feedback

@kyle-sexton kyle-sexton removed the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 1, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head e12b6954266e1c252a4e63a0c2c4a2493753ca07.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 379109c Nov 1, 2026
122 of 126 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-11391-41278026771-1 branch November 1, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(source-control): add the pull-request evidence posting step

1 participant