Skip to content

dev-lead unminimizes every informational-dispositioned bot comment on its next pass: maintainer-comment-gate.sh resolves its lib dir after the cd into the PR worktree (#2152 again, second file) #2208

Closed

Description

@don-petry

Problem

dev-lead minimizes a bot comment RESOLVED on one pass and unminimizes it on the next, so pr-review's undispositioned-pr-comment gate can never settle on a PR that needs an informational disposition.

scripts/lib/maintainer-comment-gate.sh resolves its own directory lazily, at call time, in two functions:

# _maintainer_gate_info_patterns_json (line 92) and _maintainer_gate_registry_value (line 127)
lib_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

dev-lead-fix-reviews.sh is run as bash .dev-lead/scripts/dev-lead-fix-reviews.sh, so BASH_SOURCE[0] is the relative path .dev-lead/scripts/lib/maintainer-comment-gate.sh. By the time resolve_dispositioned_comments calls maintainer_gate_reopen_candidates, the script has already cd'd into the PR worktree (checkout_pr_in_worktree). The cd fails, the registry cannot be sourced, and every lookup takes its fail-closed default:

Lookup Value after the cd Effect
_maintainer_gate_finding_re [\s\S] every body is "finding-bearing"
_maintainer_gate_registered_logins_json null the re-open check applies to every RESOLVED comment
_maintainer_gate_info_patterns_json {} no clean-status pattern applies

resolved_verdict then returns block for any RESOLVED bot comment whose covering disposition is informational, and the harness unminimizes it. The stderr is discarded (2>/dev/null on the call), so the run log shows only the unminimize notice.

This is the same defect as #2152 (fixed for comment-disposition-verify.sh by #2154, which captures _CDV_LIB_DIR at source time). This file was missed.

Evidence

petry-projects/.github PR 1276 (fix(canary): fail closed on a non-array run list), dev-lead v139-ring0 at 6fbedea, 2026-10-10. Comments IC_kwDORyesfc8AAAABayritw (Codex usage-limit notice) and IC_kwDORyesfc8AAAABayrkCg (Qodo trial-ended notice) were never edited (lastEditedAt is null) and each has one trusted informational disposition posted at 11:37Z.

  • Run 38055941317, 14:05Z: minimized comment IC_kwDORyesfc8AAAABayritw RESOLVED (disposition=informational is_human=false) and the same for …ABayrkCg.
  • Run 38067423978, 16:24Z, same tag: unminimized comment IC_kwDORyesfc8AAAABayritw — edited after its latest disposition (or its disposition cannot cover a finding-bearing body) and the same for …ABayrkCg. Nothing about either comment had changed in between.

Reproduction on main at aef61164:

bash -c 'source scripts/lib/maintainer-comment-gate.sh; cd /tmp
         _maintainer_gate_finding_re; echo; _maintainer_gate_registered_logins_json; echo; _maintainer_gate_info_patterns_json'
# cd: scripts/lib: No such file or directory   (three times)
# [\s\S]
# null
# {}

Without the cd /tmp the same three calls return the registry pattern, a 9-element array and a 4-key object.

Acceptance criteria

  • scripts/lib/maintainer-comment-gate.sh captures its library directory once, at source time, in a readonly variable (as _CDV_LIB_DIR does), and both functions above use it. No function in this file calls dirname "${BASH_SOURCE[0]}".
  • A test sources the gate by a relative path, changes to an unrelated directory, and asserts: _maintainer_gate_finding_re equals reviewer_sources_finding_section_pattern (not [\s\S]), _maintainer_gate_registered_logins_json is a non-empty array, and _maintainer_gate_info_patterns_json is a non-empty object.
  • A test asserts that, after the same directory change, maintainer_gate_reopen_candidates returns [] for a never-edited bot comment that is minimized RESOLVED, has a clean body, and is covered by one trusted informational disposition. The test fails on main before the fix.
  • The fail-closed defaults stay as they are for a registry that is really missing (existing tests still pass).

Out of scope

scripts/maintainer-resolve-comment.sh (lines 187 and 449) uses the same call-time pattern but is run standalone, not from a PR worktree. Leave it unless a test shows it failing.

Related: #2152, #2154, #2008, #2166, #1813.

Filed from a Claude Code session diagnosing why pr-review does not review petry-projects/.github PR 1276, at the maintainer's request. All times UTC.

Activity

  1. github-actions commented on Oct 10, 2026

    @github-actions
    Contributor

    Thanks for the very thorough report, including the run evidence and the one-line reproduction. This looks like a clear repeat of #2152 in a second file, and the root cause you identified (resolving the lib dir lazily after the cd into the PR worktree) matches what the reproduction shows. One clarifying question: does the unminimize also happen on a PR where the disposition is something other than informational, or is it limited to that disposition?

  2. don-petry commented on Oct 10, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #2208 — maintainer-comment-gate.sh resolves its lib dir after the cd into the PR worktree

    Scope

    Find the gate library's directory once, when the file is sourced, and store it in a readonly variable (_MAINTAINER_GATE_LIB_DIR, same approach as _CDV_LIB_DIR in #2154). Both registry readers will use it.

    Implementation Checklist

    • scripts/lib/maintainer-comment-gate.sh: add _MAINTAINER_GATE_LIB_DIR, set when the file is sourced; _maintainer_gate_info_patterns_json and _maintainer_gate_registry_value use it in place of dirname "${BASH_SOURCE[0]}"
    • scripts/maintainer-resolve-comment.sh: not changing it (out of scope per the issue)

    Tests to Write (tests/dev-lead/unit/test_maintainer_comment_gate.bats)

    • Structural: no function in the gate calls dirname "${BASH_SOURCE[0]}"
    • Source by a relative path and cd elsewhere. Then the finding regex equals the registry pattern, the logins are a non-empty array and the info patterns are a non-empty object
    • Same cd. A never-edited, RESOLVED-minimized Codex notice covered by one trusted informational disposition gives maintainer_gate_reopen_candidates → [] (fails on main)
    • A gate copied without its registry still falls back to [\s\S] / null / {}

    Test Command

    bats tests/dev-lead/unit/test_maintainer_comment_gate.bats (plus the sibling gate suites), bash .dev-lead/scripts/dev-lead-lint.sh

  3. don-petry commented on Oct 10, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #2210

    The implementation for issue #2208 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 2 commits that reference this issue on Oct 10, 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

    bugBug reportsdev-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