Extract /swarm:review --pr publish path into a deterministic script#30
Merged
Conversation
The step-5 publish path of `/swarm:review --pr` was model-interpreted
prose shell, and every prose fix spawned a new prose-shell bug (a
forgeable heredoc terminator, an advisory-echo "stale check" that posted
anyway, cross-tool-call shell state, per-cell sanitize rules with no
enforcer). Move it into a testable helper.
- Add plugins/swarm/scripts/pr-post.py:
- build — render the exact comment body from structured gated rows +
balance + PR meta through a deterministic per-cell sanitizer
(escape |/backtick/newline; @->@ and ://,www.->entities
to kill mentions + bare-URL autolinks anywhere in a cell;
<>->entities to strip raw HTML; []->escaped to break link
syntax; ort as an inert code span). The PR title (untrusted
contributor input) runs the same sanitizer.
- post — a real stale-head gate (re-read the live head; mismatch
returns SWARM_PR_STALE and does NOT post) then
gh pr comment --body-file with a self-cleaning temp file,
rebuilt from the same JSON so what was shown is what is sent.
Pure functions (sanitize_prose/sanitize_code/stale_gate/render_body)
keep it unit-testable; two-tier exit convention like loop-closeout.py.
- Add plugins/swarm/scripts/test_pr_post.py covering the sanitizer
(mention/pipe/newline/link/URL/HTML/entity-injection/backtick), the
code-span sanitizer, the stale-head gate (match/mismatch/empty-live/
empty-reviewed), body assembly, and a build subprocess round-trip.
- Shrink SKILL.md step 5 to: assemble JSON (cells raw, script sanitizes)
-> build -> human injection-scan + confirm once -> post, branching on
one token. PR_NUM/PR_HEAD_OID passed as explicit args, never via
cross-call shell state. Remove the residual note (now implemented).
- Document the extracted publish path in the knowledge base.
- Bump swarm 0.4.0 -> 0.4.1 (hardening, no new surface).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016HpkMdwQfGsYzk1Vz3Vub4
An "ohne opus" swarm review (codex + grok-build + composer) of the new publish helper surfaced seven confirmed findings; address them: - Sanitizer: entity-encode markdown-active chars instead of backslash- escaping. `\|` became `\\|` under escaping and freed a live table pipe; `\[..\](url)` re-opened a link. A numeric entity (`|`, ...) carries no literal metacharacter, so the delimiter/link/mention can never re-form. Also encode `* _ ~` (emphasis/strike). sanitize_code now STRIPS backtick + pipe (entities don't decode in a code span and escaping is bypassable there too). - render_body validates header pr_num (digits) and head_oid (hex-only), not just the gh-target seam — a JSON pr_num like "29\n\n**evil**" otherwise injects markdown past the cell sanitizer. - cmd_post threads the resolved --pr/--head-oid overrides back into the body so the header labels the PR/revision that is actually gated+posted. - Stale gate fails CLOSED: an unreadable live head now blocks the post (SWARM_PR_HEAD_UNVERIFIED) instead of posting a possibly-stale review; body is built last so the gate is the final step before the comment (minimal TOCTOU window; residual race documented). - _load_input validates rows is a list of dicts -> ValueError -> exit 2, instead of an uncaught AttributeError/TypeError + traceback + exit 1. - Fix the "6-col omits Source" test to locate the table header by content (the added title line shifted the fixed index onto the title). - Quote --input "<json>" in the SKILL examples (defensive). - test_pr_post.py: add backslash-bypass, emphasis, header-injection, hex-only short-oid, and non-dict-rows cases. Update knowledge entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016HpkMdwQfGsYzk1Vz3Vub4
A follow-up external-only swarm review (codex; grok+composer errored this
round) confirmed two misuse-contract edge cases the previous hardening
left open:
- render_body crashed on a non-string cell value: has_quelle detection
ran `.strip()` on a raw `{"quelle": 1}`, and _short ran re.sub on a
non-string head_oid — both raised past the ValueError handler and
exited 1 with a traceback instead of the documented clean exit 2 /
clean render. Coerce to str at both spots.
- An OSError from tempfile creation/write in cmd_post (full/read-only
temp dir), after the stale gate passed, escaped uncaught -> exit 1 +
traceback with no SWARM_PR_POST_ERR token. Wrap it in the operational
error path so it emits the token the skill branches on.
Add tests for both. The prior round's seven findings did not recur.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016HpkMdwQfGsYzk1Vz3Vub4
0f3786c to
674eb0a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/swarm:review --pr's step-5 publish path with a testablescripts/pr-post.pyhelper.$PR_NUM/$PR_HEAD_OIDreferenced across tool calls that don't share shell state, per-cell sanitize rules with no enforcer). Moving it to code makes correctness mechanical + unit-tested.--loopreview of PR Add /swarm:review --pr: review a PR diff and post the result #29.Changes
plugins/swarm/scripts/pr-post.pybuild— renders the exact comment body from structured gated rows + balance + PR meta through a deterministic per-cell sanitizer (escape|/backtick/newline;@→@and:///www.→entities to kill mentions + bare-URL autolinks anywhere in a cell;<>→entities to strip raw HTML;[/]to break link syntax;ortas an inert code span). The PR title (untrusted contributor input) runs the same sanitizer.post— a real stale-head gate (re-reads the live head; a mismatch returnsSWARM_PR_STALEand does not post) thengh pr comment --body-filewith a self-cleaning temp file, rebuilt from the same JSON so what the confirm gate showed is what is sent.sanitize_prose/sanitize_code/stale_gate/render_body); two-tier exit convention likeloop-closeout.py.plugins/swarm/scripts/test_pr_post.py— covers the sanitizer (mention/pipe/newline/link/URL/HTML/entity-injection/backtick), the code-span sanitizer, the stale-head gate (match/mismatch/empty-live/empty-reviewed), body assembly, and abuildsubprocess round-trip.SKILL.mdstep 5 shrinks to: assemble JSON (cells raw — script sanitizes) →build→ human injection-scan + confirm once →post, branching on one token.PR_NUM/PR_HEAD_OIDpassed as explicit args, never via cross-call shell state. Residual note removed (now implemented).swarm-review-pipeline.md+_index.md).Readiness
--pralready documented; helper scripts not enumerated per convention)check-structure.py0 errors (plugin testspr-post+loop-closeoutgreen)Test plan
python3 plugins/swarm/scripts/test_pr_post.pypassespython3 scripts/check-structure.pyreports 0 errorspython3 plugins/swarm/scripts/pr-post.py build --input <json>renders a sanitized body (mentions/URLs/HTML/pipes neutralized)/swarm:review --pr <n>posts a comment viaghunder the user's identity and honors the stale-head gate🤖 Generated with Claude Code