Release v1.10.0: safe unassign (no destructive clear-all default) - #83
Merged
Conversation
… verdict (#73) The verdict-extractor posts a machine `MERGE-VERDICT-GATE VERDICT=...` PR comment so the gate can read it. That plumbing was visible to every reader (contributors/students), alongside the human-readable "### Review Gate" verdict. After the gate parses the extractor's comment it now deletes it (scoped to the review-bot author AND the marker, so it never touches the review), leaving exactly one clean gate verdict at the bottom of the thread. Adds issues:write to the gate job so it can delete that cross-author conversation comment. CI-only; observe-only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…progress (#74) Real bug surfaced by the rile-connect team (2026-06-18): the review gate intermittently reported INCONCLUSIVE ("No parseable MERGE-VERDICT-GATE line") on perfectly good PRs, across multiple repos. Root cause, confirmed from the verdict-extract job log: with track_progress:true the action posts a placeholder comment ("I'll analyze this and get back to you.") and instructs the agent to UPDATE it with the result. On this tiny one-shot the Haiku extractor intermittently never performs that update, so the placeholder survives, the gate finds no verdict line, and fails closed to INCONCLUSIVE. Fix: set track_progress:false on the verdict-extract step. The action then posts the agent's final message directly as the comment (no update step to drop), which is exactly the one line the gate parses. Prompt also hardened to forbid the "I'll analyze this" acknowledgement. The gate's fail-closed behavior was correct; the extractor was the bug. CI only; observe-only everywhere except zenhub-cli. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the track_progress fix: that alone wasn't enough. With track_progress:false the Haiku extractor reliably EMITS the verdict (confirmed in its execution log) but posts no PR comment at all, so the gate (which read a comment) still saw nothing -> INCONCLUSIVE. And the earlier clean-thread cleanup deleted any claude[bot] comment containing the token, which on a gate-related PR matched — and removed — the review itself. Redesign so the verdict never round-trips through a PR comment: - verdict-extract parses the agent's emitted line straight from claude-code-action's execution log and exposes it as a JOB OUTPUT (verdict_line). Schema-tolerant: jq the final result text, fall back to a raw scan. The prompt's format example now uses <placeholders> so it can't false-match the regex. - review-gate reads needs.verdict-extract.outputs.verdict_line directly; no comment fetch. Dropped the extractor-comment cleanup (and its issues:write), which also fixes the over-broad delete that ate reviews on gate-related PRs. Visible PR comments are unchanged: the review, semgrep, CI, and the single '### Review Gate' verdict. INCONCLUSIVE stays fail-closed. CI only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ead SHA) (#77) Found while validating the extractor fix: the gate's own INCONCLUSIVE advice ("Re-run via Actions → Run workflow") was broken. On a workflow_dispatch run there is no pull_request context, so github.event.pull_request.head.sha is empty; await-tests then can't find the CI run, fails, and drags claude-review (needs await-tests) → skipped → verdict-extract skipped → gate INCONCLUSIVE. So the documented re-run made things worse, not better. Fix: in await-tests and review-gate, when SHA is empty (dispatch), resolve it from the PR number via `gh pr view --json headRefOid`. claude-review already checks out the PR by inputs.pr_number, so with await-tests fixed the whole manual re-run path works. CI only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tch (#78) The dispatch re-run path can't work with this review flow: claude-code-action rejects track_progress on workflow_dispatch events ("track_progress is only supported for events: pull_request, ..."), which both the review and (indirectly) the extractor rely on. So the gate's old advice to "Re-run via Actions -> Run workflow" always failed. Pushing a commit re-runs the whole pipeline reliably (pull_request synchronize), so the INCONCLUSIVE message now says that. CI only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion bumps) (#79) #231 bumps claude-code-action to v1.0.152, which broke the gate: the verdict-extract execution-log parsing was coupled to v1.0.133's log path (execution_file came back empty and the hardcoded fallback path no longer exists in v1.0.152), so the gate read no verdict -> INCONCLUSIVE on every PR running the new action. Fix: use v1.0.152's purpose-built structured-output feature. The extractor passes a --json-schema in claude_args and the action returns schema- validated JSON in steps.extract.outputs.structured_output ({"verdict": "PASS|BLOCK","count":N}). The parse step reconstructs the same MERGE-VERDICT-GATE line the gate already keys on, so review-gate is unchanged. No PR comment (no track_progress flakiness) and no execution-log path coupling (no breakage on the next action bump). Also bumps the action to v1.0.152 across the file. CI only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#81) (#82) `zh unassign <issue>` with no target used to remove EVERY assignee. That was reachable from the MCP layer: `unassign(user="")` defaulted to empty and a misnamed kwarg was silently dropped, so a missing/mistyped target silently un-assigned teammates on a shared issue (observed: removing one of two assignees cleared both). Audited the whole command layer for the same empty-arg->destructive pattern: unassign was the ONLY case — unblock, sprint remove, subissue remove, and the hierarchy remove verbs all already require their target and hard-error; all MCP *_remove_* tools take required lists. Fix: - bash cmd_unassign: removes ONLY the named user(s). Clearing everyone now requires an explicit `--all`. A target-less call is a hard error, never a silent clear-all. A named user who isn't assigned errors (no silent no-op). - bash cmd_assign / cmd_unassign: accept multiple usernames (symmetric). - MCP assign/unassign: canonical `assignees: list[str]` param (+ `user` alias); unassign gains `clear_all: bool` and NEVER clears all without it; calling unassign with no target returns an error WITHOUT invoking zh (no destructive fallback), and passing both assignees and clear_all errors. BREAKING: `zh unassign <issue>` with no user now errors instead of removing all assignees; use `--all` to clear everyone. Safety-motivated. Tests: tests/test_assign_unassign_safety.py (15) — only-named-user removed, no-target refuses, --all clears, unknown-user errors, multi-user, and the MCP guards (no-target/both-args error without calling zh). Suite: 597 passing. Closes #80 Closes #81 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Semgrep (OSS): no findingsClean scan. Posted before the Claude review. |
CI Tests✅ Tests passed: the pytest suite across Python 3.10/3.11/3.12 (and bash syntax checks) are green on Posted before the Claude review so the test outcome is folded into the review. |
Review Gate
Observe-only until |
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
1 participant
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.
Release v1.10.0. Validated on #82 (real code PR through the now-required gate → PASS; 597 tests).
Highlights
zh unassign <issue>no longer removes ALL assignees when no target is given — it now removes only the named user(s). Clearing everyone requires explicit--all. Both the bash and MCP layers refuse a target-less unassign rather than falling through to a destructive clear-all.assign/unassignare now symmetric and multi-user, with a canonicalassignees: list[str]MCP param (userkept as alias). The MCPunassigngainsclear_alland errors (without invoking zh) on no-target or conflicting args.unassignwas the only empty-arg→destructive footgun in the command layer; all other remove/unblock/remove-children/sprint-remove paths already require their target.zh unassign <issue>with no user now errors instead of clearing all assignees. Use--all.🤖 Generated with Claude Code