<< All versions

Skill v1.0.0

currentAutomated scan100/100
lugassawan/swe-workbench/workflow-address-feedback
──Details
PublishedSeptember 27, 2026 at 11:07 PM
Content Hashsha256:b7d765a3f478883d...
Git SHAec10c6e51f09
──Files
Files (1 file, 19.6 KB)
SKILL.md19.6 KBactive
SKILL.md · 222 lines · 19.6 KB

version: "1.0.0" name: workflow-address-feedback description: Use when a PR owner wants to address review feedback — fetches outstanding threads and general comments, presents a per-item triage (ADDRESSED / CLARIFIED / DEFERRED), applies fixes via the Edit tool, commits via workflow-commit-and-pr, resolves review threads via GraphQL resolveReviewThread (those without a thread are never resolved), and syncs stale PR metadata when drift is detected. orchestrator: true


Workflow: Address Feedback

Announce at start: "I'm using the workflow-address-feedback skill to address review feedback on PR #N."

When to invoke

  • The user runs /swe-workbench:address-feedback <N>.
  • A PR owner wants to systematically work through review threads.
  • Phrases: "address the feedback on PR 123", "help me resolve review comments", "triage and fix the review threads on #456".

When NOT to invoke

  • The reviewer side of the loop → use swe-workbench:workflow-pr-review (first-pass or followup mode).
  • The user just wants to reply to a single comment without the full triage flow.
  • The PR is closed/merged.

Composition

This skill orchestrates:

  • swe-workbench:ticket-context — prepended context when PR references a ticket.
  • swe-workbench:workflow-commit-and-pr — invoked after all ADDRESSED fixes are applied to commit and push.

Phase flow

Phase 1 — Pre-flight + fetch

bash
command -v swe-workbench-address-feedback-fetch >/dev/null 2>&1 || {
echo "swe-workbench runtime commands not on PATH — reinstall or update the swe-workbench plugin." >&2
exit 1
}
RESULT=$(swe-workbench-address-feedback-fetch --pr "$PR" \
| swe-workbench-result-check swb.address-feedback-fetch/1) || exit 1
STATE=$(printf '%s' "$RESULT" | jq -r '.data.state')
JSON=$(printf '%s' "$RESULT" | jq -r '.data.pr_json_path')
[ "$STATE" = "OPEN" ] || { swe-workbench-clean-state-files "$JSON"; echo "PR #$PR is $STATE — address-feedback only applies to open PRs."; exit 1; }
OWNER=$(printf '%s' "$RESULT" | jq -r '.data.owner')
REPO=$(printf '%s' "$RESULT" | jq -r '.data.repo')
AUTHOR_LOGIN=$(printf '%s' "$RESULT" | jq -r '.data.author_login')
CURRENT_USER=$(printf '%s' "$RESULT" | jq -r '.data.current_user')
PR_BRANCH=$(printf '%s' "$RESULT" | jq -r '.data.pr_branch')
THREADS_PATH=$(printf '%s' "$RESULT" | jq -r '.data.threads_path')
PR_COMMENTS_PATH=$(printf '%s' "$RESULT" | jq -r '.data.pr_comments_path')
TRIAGE_PATH=$(printf '%s' "$RESULT" | jq -r '.data.triage_path')
RESUME_TRIAGE_PATH=$(printf '%s' "$RESULT" | jq -r '.data.resume_triage_path')
ELIGIBLE_THREADS=$(printf '%s' "$RESULT" | jq -r '.data.eligible_threads')
SKIPPED_THREADS_CLARIFIED=$(printf '%s' "$RESULT" | jq -r '.data.skipped_threads_clarified')
ELIGIBLE_PR_COMMENTS=$(printf '%s' "$RESULT" | jq -r '.data.eligible_pr_comments')
SKIPPED_PR_COMMENTS=$(printf '%s' "$RESULT" | jq -r '.data.skipped_pr_comments')
RUN_DIR=$(swe-workbench-new-run-dir address-feedback "$PR")

swe-workbench-address-feedback-fetch handles gh auth status, fetches the PR JSON to $JSON (via swe-workbench-preflight-pr), and — when the PR is OPEN — paginates review threads and PR-level conversation comments, projecting eligible/skip_reason onto each entry (resolved/already-clarified for threads; bot/owner/marker/manual-reply exclusion for PR comments) before writing them to $THREADS_PATH/$PR_COMMENTS_PATH. The [ "$STATE" = "OPEN" ] gate runs immediately after the fetch and before $RUN_DIR is allocated, so a rejected PR reaps $JSON inline via swe-workbench-clean-state-files rather than leaking it — $RUN_DIR never exists on this path, so there is nothing else to reap. new-run-dir.sh allocates $RUN_DIR — a mode-0700 scratch directory under /tmp/swe-workbench-run/ for this run's own ad-hoc bash artifacts, distinct from the deliberate PR-keyed state files above (including $TRIAGE_PATH, which is a cross-invocation resume point and must never move here). All four state paths are repo-scoped: the fetch resolves the owner-repo slug itself — explicit --repo when the invocation carried a full PR URL, else the checkout's origin remote, else legacy un-scoped names — so same-numbered PRs in different repositories never collide.

If CURRENT_USER != AUTHOR_LOGIN, warn:

"You are not the PR author (PR author: @AUTHOR_LOGIN, you: @CURRENT_USER). Address-feedback flows are typically owner-side. Continue anyway? Reply yes to proceed."

Wait for confirmation before continuing. If the user declines, run Phase 7 — Cleanup and exit.

If $ELIGIBLE_THREADS and $ELIGIBLE_PR_COMMENTS are both zero, nothing is left to triage — one merged check, replacing two separate early-exits the pre-runtime-command version had (this now runs before Phase 2 ever spins up a worktree, unlike before):

  • $SKIPPED_THREADS_CLARIFIED or $SKIPPED_PR_COMMENTS is non-zero: print "No new items to triage — N already clarified/handled." (N = their sum).
  • Otherwise, when some threads existed but were all resolved (nothing was skipped as already-clarified/-handled): print "No new items to triage."
  • Otherwise, when $ELIGIBLE_PR_COMMENTS is zero and no threads existed at all (jq 'length' "$THREADS_PATH" is zero): print "No open threads — nothing to address."

Then run Phase 7 — Cleanup and exit.

If the envelope's resume_available is true, offer to resume — reading the saved decisions from $RESUME_TRIAGE_PATH, which the fetch resolves to wherever the data actually lives (the scoped $TRIAGE_PATH, or a pre-upgrade session's legacy un-scoped /tmp/swe-workbench-address-feedback/${PR}-triage.json, dual-read). New saves always go to $TRIAGE_PATH, so a resumed session migrates to the scoped spelling on its next save.

Phase 2 — Worktree

Acquire the worktree via the runtime command — it owns the reuse-current / reuse-existing / create-via-rimba / create-via-git decision, the origin/$PR_BRANCH fetch, the fast-forward-or-diverged-warn reconcile, and rimba deps install:

bash
RESULT=$(swe-workbench-address-feedback-worktree acquire --pr "$PR" --branch "$PR_BRANCH" \
| swe-workbench-result-check swb.address-feedback-worktree-acquire/1) || exit 1
WT=$(printf '%s' "$RESULT" | jq -r '.data.path')
CREATED_WT=$(printf '%s' "$RESULT" | jq -r 'if .data.reused then "false" else "true" end')
printf '%s' "$RESULT" | jq -r '
if .data.reused then "Reusing worktree (" + .data.reuse_reason + ") at " + .data.path
else "Created worktree at " + .data.path end'
[ "$(printf '%s' "$RESULT" | jq -r '.data.dirty')" = "true" ] && \
echo "Note: working tree has uncommitted changes; the user may stash before Phase 4 commits to avoid sweeping unrelated edits into the feedback commit."
[ "$(printf '%s' "$RESULT" | jq -r '.status')" = "partial" ] && \
printf '%s' "$RESULT" | jq -r '.warnings[] | "⚠ " + .message'

This worktree is disposable but sits on the PR branch itself — Phase 4 commits and pushes from it update the PR directly, so the work lives on the PR branch, not a throwaway task branch. Phase 7 removes the worktree on every exit (success, Q-quit, or error) but keeps the local `$PR_BRANCH`: it is the owner's actual PR head branch, and keeping it preserves unpushed commits if a prior run crashed mid-Phase-4 (the next run's acquire checks that branch out instead of re-creating it). Never delete $PR_BRANCH (e.g. via git branch -D) — swe-workbench-address-feedback-worktree release never issues one. If the skill exits with an unrecoverable error at any point after this phase, run Phase 7 before stopping.

Phase 3 — Triage digest

Read $THREADS_PATH and $PR_COMMENTS_PATH (jq '[.[] | select(.eligible)]' on each) and render only the eligible == true entries, one by one — the fetch command already applied the resolved/already-clarified exclusion for threads and the bot/owner/marker/manual-reply exclusion for PR comments, so Phase 3 never re-implements those rules itself.

If $SKIPPED_THREADS_CLARIFIED or $SKIPPED_PR_COMMENTS is non-zero, print transparency notes before the digest — this dedup is lossy by construction, so a transparency note replaces silently dropping:

"(N thread(s) skipped — already clarified.)"
"(N PR comment(s) skipped — already handled.)"

For each remaining thread:

─────────────────────────────────────────────────
Thread #ID — {path}:{line} by @{author} [{Severity if parseable}]
─────────────────────────────────────────────────
> {first 200 chars of comment body}
[A]ddressed — fix + commit + reply + resolve
[C]larified — reply + resolve
[D]eferred — reply + resolve (acknowledged, not fixed now)
[Q]uit — save progress and exit

Parse severity from the comment's leading **<Severity>** headline (the layout swe-workbench-pr-review-submit renders) or a legacy Severity: <level> prefix in the comment body if present; otherwise label Unknown.

Capture: triage[<thread_id>] = A|C|D.

For each remaining PR comment (no path:line, no resolve state), key as triage["prcomment:<comment.id>"] (namespaced against review-thread node IDs in the same flat map; carries the id needed for the Phase 5 marker; both key kinds round-trip through Q-quit save/resume unchanged):

PR comment by @{author}
> {first 200 chars of comment body}
[A]ddressed — fix + commit + reply (PR comments have no thread to resolve)
[C]larified — reply only
[D]eferred — skip this comment
[Q]uit — save progress and exit

If the owner replies Q at any point in either loop, save triage state to $TRIAGE_PATH, then run Phase 7 — Cleanup, and exit. Re-invocation resumes from this file (Phase 2 re-creates the worktree).

Phase 4 — Implement + commit

For each ADDRESSED review thread or PR comment (in order — both sources share this loop, since the commit step is source-agnostic):

  1. Show the finding and the relevant file/line context (PR comments have no path:line; show the comment body instead).
  2. Ask the owner for the fix approach (free-text). If the comment already carries a suggested fix — a **Suggested fix:** paragraph (the layout swe-workbench-pr-review-submit renders; the fix is the rest of that paragraph — or, when a fenced block opens the value, the paragraph after the label — code fences included, up to the next label or the end of the comment) or a legacy ### Suggested fix block — offer to apply it automatically via the Edit tool.
  3. Apply edits using the Edit tool.

After all ADDRESSED fixes are applied, invoke swe-workbench:workflow-commit-and-pr with the prompt:

"commit and push these fixes addressing review feedback on PR #N"

This reuses the existing [type] commit format, branch-naming check, and push logic. After the skill returns, capture the resulting commit SHA:

bash
FIX_SHA=$(git -C "$WT" rev-parse HEAD)

Phase 5 — Reply + resolve

For each ADDRESSED, CLARIFIED, or DEFERRED review thread, post a reply via REST then resolve via GraphQL resolveReviewThread by calling swe-workbench-reply-and-resolve with the triage-mapped args — all three dispositions now resolve the thread, since an open thread blocks /swe-workbench:review's approval gate regardless of why it was left open. For each ADDRESSED or CLARIFIED PR comment, post a reply via REST — PR comments have no thread, so resolve is always suppressed and KIND=issue is passed explicitly, with a hidden swe-workbench:handled:{id} marker embedded in the reply body for Phase 1's re-run dedup; DEFERRED PR comments are skipped entirely, unchanged. Reply targets the thread root comment (comments.nodes[0].databaseId), never a subsequent reply. Full reply-body templates, exact invocation args, and the PR-comment quoting/escaping caveat live in reference/resolve-review-threads.md.

After all replies and resolutions land, emit the follow-up CTA:

"Want me to ping the reviewer to re-check? Reply yes to run /swe-workbench:review --check-followup <N>."

On the Phase 5 success path, delete the triage resume-point file. The three run-scoped state files ($JSON, $THREADS_PATH, $PR_COMMENTS_PATH — repo-scoped per) are reaped by Phase 7 on every exit instead — $TRIAGE_PATH is reaped here specifically, because completion is what makes the resume point spent, and Phase 7 must never touch it (see Phase 7). The reap runs foreground; failures surface (no 2>/dev/null):

bash
swe-workbench-clean-state-files "$TRIAGE_PATH"
[ -e "$TRIAGE_PATH" ] \
&& echo "⚠ state file NOT reaped: $TRIAGE_PATH" >&2 \
|| echo "✓ state file reaped: $TRIAGE_PATH"

Then run Phase 6 — Sync PR metadata.

Phase 6 — Sync PR metadata (when fixes were committed)

Skipped when $FIX_SHA is unset (no fixes committed in Phase 4). Otherwise fetches the live PR title/body, commit subjects, and diff stat, judges drift against the title and ## Summary section, and — on detected drift, after a Reply \yes\` preview gate — completely re-writes/rephrases the PR title (if needed) and the ## Summary section to keep them clean (instead of just appending updates) via swe-workbench-sync-pr-metadata while preserving ## Test Plan, the Closes #/Fixes #/Issue: N/A trailer, and all other sections. Full fetch commands, drift-judging criteria, and apply mechanics live in reference/sync-pr-metadata.md`. Then run Phase 7 — Cleanup.

Phase 7 — Cleanup (always)

Run on every exit after the Phase 1 preflight line. Skip the worktree-removal block on Phase 1 early-exits (before any worktree exists) — $WT is unset there since Phase 2 never ran. Otherwise call release, which itself no-ops when $CREATED_WT is "false" (a reused worktree was never created, so there is nothing for it to remove) — the runtime command's own --created flag carries that distinction now, replacing the old skill-level REUSED_WT variable.

bash
if [ -z "${WT:-}" ]; then
echo "No worktree was created this run — skipping worktree cleanup."
else
RELEASE_RESULT=$(swe-workbench-address-feedback-worktree release \
--pr "$PR" --path "$WT" --branch "$PR_BRANCH" --created "$CREATED_WT" \
| swe-workbench-result-check swb.address-feedback-worktree-release/1) || RELEASE_RESULT=""
if [ -z "$RELEASE_RESULT" ]; then
echo "⚠ swe-workbench-address-feedback-worktree release failed — worktree at $WT may need manual cleanup."
elif [ "$(printf '%s' "$RELEASE_RESULT" | jq -r '.data.removed')" = "true" ]; then
echo "Cleaned up worktree at $WT (local branch kept)."
elif [ "$(printf '%s' "$RELEASE_RESULT" | jq -r '.status')" = "ok" ]; then
echo "Reused existing worktree at $WT — skipping cleanup (nothing was created)."
else
echo "⚠ release did not remove $WT — see warnings below."
printf '%s' "$RELEASE_RESULT" | jq -r '.warnings[] | "⚠ " + .message'
fi
fi
[ -n "${RUN_DIR:-}" ] && { swe-workbench-reap-run-dir "$RUN_DIR"; [ -e "$RUN_DIR" ] && echo "⚠ run dir NOT reaped: $RUN_DIR" >&2 || echo "✓ run dir reaped: $RUN_DIR"; }
swe-workbench-clean-state-files "$JSON" "$THREADS_PATH" "$PR_COMMENTS_PATH"
for f in "$JSON" "$THREADS_PATH" "$PR_COMMENTS_PATH"; do
[ -e "$f" ] && echo "⚠ state file NOT reaped: $f" >&2 || echo "✓ state file reaped: $f"
done

Cleanup is failure-tolerant: release always exits 0, and a genuine removal failure surfaces as a warning rather than blocking completion. release is path-keyed and never issues a branch-deleting command, so $PR_BRANCH — the owner's actual PR head branch — can never be destroyed by this step. $RUN_DIR is reaped unconditionally whenever it exists, independent of whether Phase 2 reused an existing worktree. It is guarded with ${RUN_DIR:-} because the Phase 1 STATE-gate rejection is the one exit that precedes $RUN_DIR's allocation entirely — that path reaps $JSON inline instead of routing through this phase (see Phase 1) and never reaches this guard. $JSON/$THREADS_PATH/$PR_COMMENTS_PATH are the paths the Phase 1 fetch envelope named — reaping whatever the manifest names, rather than re-hardcoding the three literals, keeps this list and swe-workbench-address-feedback-fetch's own output in permanent sync. These are the state files whose lifetime ends with the run itself; the resume-point file created on Q-quit is reaped separately, on completion, in Phase 5 — never here, since a Q-quit into this phase must leave it intact for the next invocation to resume from.

Failure modes

FailureSignalAction
gh auth failsNon-zero exitAbort.
PR not foundgh pr view failsAbort.
CURRENT_USER != AUTHOR_LOGINJSON mismatchWarn + ask to continue.
No outstanding threadsGraphQL returns 0 unresolvedPrint "No open threads — nothing to address." Exit.
Owner picks Q mid-triageLoop exitSave triage state to $TRIAGE_PATH, run Phase 7 cleanup, then exit.
Worktree removal failsrelease's data.removed is falserelease still exits 0 with status: "partial" and a warnings entry — log it; do not block.
Reply REST fails (404 — comment deleted)HTTP 404Skip that thread, log "skipped (comment deleted)".
Resolve mutation failsGraphQL errorReply already posted — log "reply posted but resolve failed". Continue. Do not roll back the reply.
rimba re-prefixes a non-conventional PR branch name (worktree on feature/<name> ≠ $PR_BRANCH)acquire's post-create branch verification mismatchHandled internally by acquire: removes the mis-branched worktree and falls back to a plain git worktree add.
Fork PR — branch exists only on the fork remotegit fetch origin "$PR_BRANCH" fails / origin/$PR_BRANCH missingOut of scope: add the fork remote manually (git remote add gh-fork-<owner> …) and check out the PR head by hand, then re-run.

Common mistakes

MistakeFix
Create a new worktree when already on the PR branch or when one already existsacquire runs the reuse-current and reuse-existing checks internally before ever creating anything — Phase 2 never needs to reimplement them.
Re-implement --skip-deps/--skip-hooks handling in Phase 2acquire never exposes either flag — it always installs deps so the worktree is fully initialized with no separate bootstrap step.
Create the worktree on a throwaway task branchacquire always creates on $PR_BRANCH itself, never a throwaway task branch — Phase 4 pushes update the PR directly.
Leave the worktree behind after skill exitsPhase 7 runs on every exit past the Phase 1 preflight line — including exits before Phase 2 ever ran — but its worktree-removal block only fires when $WT is actually set; a Phase 1 early exit ($WT unset) skips removal, and release --created "$CREATED_WT" itself no-ops for a reused worktree.
Deleting $PR_BRANCH directly in Phase 7 cleanuprelease is path-keyed and never issues a branch-deleting command — $PR_BRANCH is the real PR head branch and can never be destroyed by this step.
Post the reply before the commitAlways commit first (Phase 4) so $FIX_SHA is available for the ADDRESSED reply template.
Leave a CLARIFIED/DEFERRED thread open after replyAll three review-thread dispositions now resolve — an open thread blocks /swe-workbench:review's approval gate. PR comments are unaffected (no thread to resolve).
Try to resolve via RESTThread resolution is GraphQL-only (resolveReviewThread mutation). REST has no equivalent endpoint.
Re-present a thread the owner already clarifiedOn re-runs, skip unresolved threads that already have a comment authored by $CURRENT_USER. Detect via comments.nodes[*].author.login.
All versions