Skip to content

dev-lead: resolve review threads it has addressed — unresolved 'Applied in…' threads silently block merge via required_review_thread_resolution #1547

Closed

Description

@don-petry

Problem

Every pr-quality ruleset sets required_review_thread_resolution: true, so a PR cannot merge while any review thread is unresolved. The dev-lead review-changes flow applies fixes for bot review comments (gemini-code-assist, codeant-ai, CodeRabbit) and replies in-thread with "Applied in …" / "Verified and confirmed …" — but never resolves the thread. The PR then sits fully green + approved yet unmergeable, with nothing surfacing why.

During the 2026-08-18 compliance-PR sweep, 20 such threads had to be hand-resolved across ContentTwin #371/#424/#425, broodly #432/#483, bmad-bgreat-suite #429, TalkTerm #426/#427, and .github #970. In every case the fix was already live at head and the agent had already replied that it was addressed — resolution was the only missing step.

Requested fix

After the dev-lead posts an "addressed" reply to a review thread (and the fix commit is pushed), it should call the GraphQL resolveReviewThread mutation on that thread. Alternatively (or additionally) the pr-review cascade could resolve threads whose last reply is an agent addressed-marker when it verifies the fix at head.

Evidence

Activity

  1. don-petry commented on Aug 20, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1547 — dev-lead: resolve review threads it has addressed — unresolved 'Applied in…' threads silently block merge via required_review_thread_resolution

    Scope

    Add a deterministic, script-side safety net so that after the engine session, dev-lead resolves bot-originated review threads it has already addressed in-thread (the fix is at head, the agent replied but forgot to resolve) — the exact class of thread that sat unresolved and silently blocked merge under required_review_thread_resolution: true during the 2026-08-18 sweep. The existing resolve_*_outdated_threads nets only cover isOutdated threads; an addressed-at-head thread that GitHub has not marked outdated stays open. Resolution keys on a stable addressed-marker (<!-- dev-lead:addressed -->) so a skipped thread (no marker) is never resolved, and only bot-originated threads are touched — never a marker-less maintainer finding (respects the #1415 resolve-guard).

    Implementation Checklist

    • scripts/lib/maintainer-review-thread-gate.sh — add _DEV_LEAD_ADDRESSED_MARKER constant + review_reply_is_addressed_marker <body> classifier (0 = carries the addressed-marker)
    • scripts/dev-lead-fix-reviews.sh — add resolve_addressed_bot_threads <intent>: paginate review threads, resolve every unresolved bot-originated thread whose last comment carries the addressed-marker; dry-run + PR_NUMBER guards mirroring resolve_bot_outdated_threads
    • Wire resolve_addressed_bot_threads into the fix-reviews, fix-bot-comment, and review-changes intents alongside the existing resolve calls (applied + no-changes paths; skipped on the dev-lead fix-bot reverts its own fix on neutral Copilot 'overview' comments → net no-op PRs that falsely close compliance issues #1340 net-zero path)
    • prompts/dev-lead/fix-reviews.md, fix-bot-comment.md, review-changes.md — instruct the agent to append <!-- dev-lead:addressed --> to every addressed reply

    Tests to Write

    • tests/dev-lead/unit/test_maintainer_review_thread_gate.bats — review_reply_is_addressed_marker: marker present → 0; marker-less / skip reply / empty → 1
    • tests/dev-lead/unit/test_fix_reviews.bats — resolve_addressed_bot_threads: dry-run announces + skips API; resolves an addressed bot thread (non-outdated) at head; does NOT resolve a marker-less bot thread; does NOT resolve a maintainer-originated thread
    • tests/dev-lead/integration/test_prompt_coverage.sh stays green (marker literal adds no undeclared ${VAR})

    Test Command

    bats tests/dev-lead/unit/test_fix_reviews.bats tests/dev-lead/unit/test_maintainer_review_thread_gate.bats + bash tests/dev-lead/integration/test_prompt_coverage.sh + bash .dev-lead/scripts/dev-lead-lint.sh

  2. don-petry commented on Aug 20, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Progress (in progress — not yet durable)

    Provisional. Nothing is pushed yet; the automation posts the authoritative
    completion record (with PR number + head SHA) after the push. Treat the notes
    below as intent, not delivered results.

    Plan Execution (intended)

    • Added a deterministic safety net so dev-lead resolves bot-originated review threads it addressed in-thread but left unresolved — the class of thread that silently blocks merge under required_review_thread_resolution: true (the 2026-08-18 sweep hand-resolved 20).

    Files touched (this workspace, not yet pushed)

    • scripts/lib/maintainer-review-thread-gate.sh: added the <!-- dev-lead:addressed --> addressed-marker constant + review_reply_is_addressed_marker classifier (single source of truth for the marker).
    • scripts/dev-lead-fix-reviews.sh: added resolve_addressed_bot_threads (paginated; resolves an unresolved bot-originated thread whose last reply carries the addressed-marker; dry-run + PR_NUMBER guards) and wired it into the fix-reviews, fix-bot-comment, and review-changes intents alongside the existing outdated-thread nets (skipped on the dev-lead fix-bot reverts its own fix on neutral Copilot 'overview' comments → net no-op PRs that falsely close compliance issues #1340 net-zero path).
    • prompts/dev-lead/{fix-reviews,fix-bot-comment,review-changes}.md: instruct the agent to end each addressed reply with <!-- dev-lead:addressed --> (only on genuine addressed replies, never on skips).
    • Tests: tests/dev-lead/unit/test_maintainer_review_thread_gate.bats (classifier) and tests/dev-lead/unit/test_fix_reviews.bats (dry-run announce; resolves a non-outdated addressed bot thread; leaves a marker-less bot thread open; leaves a maintainer-originated thread open).

    Notes

  3. don-petry commented on Aug 20, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #1555

    The implementation for issue #1547 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

  4. added a commit that references this issue on Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dev-leadFor dev-lead agent pickup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions