Skip to content

bug(gardener): self-loop in new CLI — comment reviews sync PRs, respond acts on its own review #134

@serenakeyitan

Description

@serenakeyitan

Problem

Same self-loop bug as repo-gardener#22, but in the new TypeScript CLI being added under #130 (phases #131, #132).

Two scenarios:

  1. first-tree gardener comment may review sync PRs — it scans open PRs, and without a label filter it will comment on gardener's own sync PRs (tree PRs labeled first-tree:sync).
  2. first-tree gardener respond may act on its own review — when third-party reviewer agents are absent, respond sees gardener-sync's review-pass comment as the only feedback and tries to fix → pushes commit → triggers re-review → loop.

Fix (in the new CLI)

src/products/gardener/engine/comment.ts

Add label filter early (both in single-PR mode and scan mode):

// Single-PR mode
if (pr.labels?.some(l => l.name === "first-tree:sync")) {
  log(\` PR #\${prNumber} has first-tree:sync label  gardener-comment skips sync PRs to avoid self-loops\`);
  emitBreezeResult("skipped", "sync PR not handled by comment module");
  return 0;
}

src/products/gardener/engine/respond.ts

When counting CHANGES_REQUESTED reviews and parsing @gardener fix comments, exclude gardener-authored entries. Two checks OR'd:

  • Login match: review.user.login !== gardener_user (gardener_user from gh api user --jq .login at startup)
  • HTML marker fallback: !review.body.includes(\"<!-- gardener:\")

If the filtered list is empty → log `"⏭ no non-gardener feedback on # — skipping to avoid self-loop"` and emit BREEZE_RESULT: status=skipped summary=.... Do not modify any breeze:* label in this path.

Breeze compatibility

This CLI can be invoked by breeze-runner (stacked on #131, #132). The skip path must:

  • Exit 0 (not an error)
  • Emit BREEZE_RESULT: status=skipped summary=... as the last stdout line
  • Not touch breeze:wip/human/done labels
  • Let the breeze runner's own label-management prompt handle the outer state

Labeling rules (unchanged, documented here for clarity)

  • Gardener never sets breeze:wip (that's the reviewer's job, per breeze DESIGN.md)
  • Gardener sets breeze:human after 5 failed respond attempts (per issue docs: clarify who sets breeze:human (responder-elevated pattern) breeze#13 — responder-elevated pattern)
  • When gardener skips due to self-loop guard → no label change
  • Merge / close → breeze:done (auto-handled by PR close in breeze's poller)

Tests

Add to tests/gardener-comment.test.ts:

  • PR has first-tree:sync label → CLI exits 0 with skip message, no classify/post called
  • PR has other labels → normal path

Add to tests/gardener-respond.test.ts:

  • Reviews list contains only gardener-authored CHANGES_REQUESTED → skip, no commit pushed, no label changed
  • Reviews contains at least one non-gardener CHANGES_REQUESTED → fix path triggered
  • Comment threads contains only gardener's own @gardener fix (edge case — unlikely but possible) → skip
  • Login-match fallback: review body contains <!-- gardener: marker but login differs from current user → still skip (B-path)

Related

This fix should land in #131 and #132 themselves (as additional commits) rather than in a follow-up PR, so the phase-1/2 code is never shipped with the self-loop bug.

Acceptance

  • gardener comment guards against first-tree:sync labeled PRs
  • gardener respond computes feedback-present from non-gardener reviewers only
  • Login match + marker fallback both implemented
  • Skip path: clean exit, BREEZE_RESULT: skipped, no label mutations
  • Tests added in both test files
  • E2E verify: synthetic tree with gardener-only review → CLI skips; add non-gardener review → CLI fixes

🌱 Issue created by Claude on behalf of @serenakeyitan via Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions