Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/source-control/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "source-control",
"version": "0.9.1",
"version": "0.9.2",
"description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention and babysit-prs config, or apply — interview the repo and write the tracked convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared.",
"author": {
"name": "Melodic Software",
Expand Down
17 changes: 17 additions & 0 deletions plugins/source-control/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@
All notable changes to the `source-control` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.9.2]

### Fixed

- **babysit-prs no longer re-dispatches a worker onto its own prior-round replies.** For a solo
maintainer whose `gh` login is the configured self-login (`gh api user` login plus any
`babysit_self_logins` extras), the delta engine counted the worker's own classification replies
and `Fixed in <sha>` follow-ups as new human-authored feedback, manufacturing a self-inflicted,
unsuppressible `new_human_blocking_feedback` dispatch that re-fired every cycle with zero real
work. The `new_human_blocking_feedback` and `new_human_feedback` deltas now exclude items
authored by the configured self-login(s) — the same self-reply exclusion `review-discipline.md`
§1 already mandates for the worker, and parity with the bot delta arms (self-filtered
structurally because the engine never comments as a bot). Scoped to the dispatch deltas only: a
self-authored item still classifies as human feedback, so a genuine "do not merge" comment the
maintainer posts under their own login keeps the human stop and triage blocker intact and still
halts the merge gate.

## [0.9.1]

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,14 @@ rather than recomputing it, so the untriaged-material clause applies there too.
`new_blocking_feedback`) — a human `CHANGES_REQUESTED`/blocking/unresolved-inline-thread item
with a new id. An *ordinary*, non-blocking new human comment does **not** set this — it is
handled per `feedback.md`'s Human Feedback section, surfaced directly by the main agent from the
snapshot without spending a worker on it.
snapshot without spending a worker on it. Excludes items authored by the configured
self-login(s): the worker posts its own prior-round classification replies and `Fixed in <sha>`
follow-ups under the operator's own login, so counting them would manufacture a self-inflicted
dispatch that re-fires every cycle. The bot arms get this self-filter structurally (the engine
never comments as a bot); the human arm needs it explicitly, matching the self-reply exclusion
`review-discipline.md` §1 already mandates for the worker. A self-authored item still sets the
human stop and triage blocker — only the worker-dispatch delta is suppressed, so a genuine
"do not merge" comment the maintainer posts under their own login still halts the merge gate.
- **`resolved_human_blocking`** (suppressible) — the PR previously required a human stop
(`CHANGES_REQUESTED` or a blocking/unresolved human item), and now requires none, with no other
delta. Symmetric to `resolved_blocking_feedback` below: without this arm a PR that just cleared
Expand Down
23 changes: 23 additions & 0 deletions plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,27 @@ def classify_pr(
human_feedback_ids = [
item["id"] for item in (*feedback["human_blocking"], *feedback["human"])
]
# Parity with the bot delta arms' *structural* self-filter: bot feedback can
# never contain this engine's own posts, because the engine comments under
# the operator's human login and `collect_feedback` only routes bot-authored
# items to `feedback["blocking"]`/`["material"]`. The human arms have no such
# structural guard, so without this an operator whose login is the configured
# self-login sees every prior-round worker reply (classification tables,
# "Fixed in <sha>" follow-ups) counted as new human-authored feedback --
# manufacturing a self-inflicted, unsuppressible `new_human_blocking_feedback`
# dispatch that re-fires forever with zero real work. This brings the
# deterministic delta to the same rule `reference/review-discipline.md` §1
# already mandates for the worker's own classification replies.
#
# Deliberately scoped to the new-feedback deltas only, NOT to
# `collect_feedback`'s classification: a self-authored item still lands in
# `feedback["human_blocking"]` above, so a genuine "don't merge, I found a
# problem" comment the maintainer posts under their own login keeps
# `human_stop`/the triage blocker intact and continues to halt the merge
# gate. Filtering it there instead would silently strip the solo maintainer's
# ability to human-stop their own PR. Here it only stops re-dispatching a
# worker onto the engine's own prior output.
self_logins = {login.casefold() for login in config.self_logins if login}
new_blocking_feedback = [
item for item in feedback["blocking"] if item["id"] not in prev_blocking_ids
]
Expand Down Expand Up @@ -383,6 +404,7 @@ def classify_pr(
item
for item in (*feedback["human_blocking"], *feedback["human"])
if item["id"] not in prev_human_ids
and str(item.get("author") or "").casefold() not in self_logins
Comment thread
kyle-sexton marked this conversation as resolved.
]
changed = bool(prev) and (
prev.get("head_sha") != head_sha or prev.get("updated_at") != updated_at
Expand Down Expand Up @@ -642,6 +664,7 @@ def classify_pr(
item
for item in feedback["human_blocking"]
if item["id"] not in prev_human_blocking_ids
and str(item.get("author") or "").casefold() not in self_logins
]
# Symmetric to `resolved_failing_checks`: a PR previously blocked only by
# human feedback (CHANGES_REQUESTED, an unresolved inline thread) that the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,21 @@ def _csv(value: str | None) -> frozenset[str]:

def build_config(args: argparse.Namespace) -> delta.ClassifyConfig:
owners = _csv(getattr(args, "owners", None))
resolved_authors = getattr(args, "resolved_authors", None)
if resolved_authors is None:
# Hand-built Namespaces (tests, callers) may skip resolution; derive
# the self set from the raw --author, dropping the unresolvable '@me'.
self_logins = frozenset(
login for login in _csv(getattr(args, "author", None)) if login != "@me"
)
resolved_self_logins = getattr(args, "resolved_self_logins", None)
if resolved_self_logins is not None:
self_logins = frozenset(resolved_self_logins)
else:
self_logins = frozenset(resolved_authors)
resolved_authors = getattr(args, "resolved_authors", None)
if resolved_authors is None:
# Hand-built Namespaces (tests, callers) may skip resolution; derive
# the self set from the raw --author, dropping the unresolvable '@me'.
self_logins = frozenset(
login
for login in _csv(getattr(args, "author", None))
if login != "@me"
)
else:
self_logins = frozenset(resolved_authors)
return delta.ClassifyConfig(
allowed_owners=owners,
self_logins=self_logins,
Expand All @@ -69,6 +75,24 @@ def build_config(args: argparse.Namespace) -> delta.ClassifyConfig:
)


def resolve_self_logins(authors: list[str]) -> list[str]:
"""Self-identity suppression must not ride on the discovery author filter.

Autopilot (and explicit widening) deliberately drops `--author`, so `authors`
is empty and `ClassifyConfig.self_logins` derived from it would be empty too —
leaving the worker's own comments free to re-fire `new_human_blocking_feedback`
every cycle. Resolve the authenticated posting identity independently and union
it with any discovery authors, so suppression holds regardless of scope.
"""
self_login = gh.resolve_author("@me")
resolved = list(authors)
if self_login and self_login.casefold() not in {
login.casefold() for login in resolved
}:
resolved.append(self_login)
return resolved


def resolve_scope_repos(
args: argparse.Namespace, owners: frozenset[str]
) -> tuple[str, ...]:
Expand Down Expand Up @@ -111,6 +135,7 @@ def build_snapshot(args: argparse.Namespace) -> dict[str, Any]:
# rather than require them.
authors = gh.resolve_authors(getattr(args, "author", None))
args.resolved_authors = authors
args.resolved_self_logins = resolve_self_logins(authors)
Comment thread
kyle-sexton marked this conversation as resolved.
Comment thread
kyle-sexton marked this conversation as resolved.
Comment thread
kyle-sexton marked this conversation as resolved.
config = build_config(args)
scope_repos = resolve_scope_repos(args, config.allowed_owners)
if scope_repos:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,96 @@ def test_became_ready_for_review_fires_even_when_clean(self) -> None:
self.assertTrue(result["needs_worker"])


class SelfLoginHumanFeedbackExclusionTests(unittest.TestCase):
"""The engine must not dispatch a worker onto its own prior-round replies.

A solo maintainer whose login is the configured self-login posts every
worker classification reply and `Fixed in <sha>` follow-up under that login.
Counting those as new human feedback manufactures a self-inflicted,
unsuppressible `new_human_blocking_feedback` dispatch that re-fires forever
(issue #473). The bot arms are self-filtered structurally; the human arm
needs an explicit self-login filter -- but only over the *new-feedback
deltas*, never the classification itself.
"""

SELF = frozenset({"solo"})
CONFIG = delta.ClassifyConfig(allowed_owners=frozenset({"owner"}),
self_logins=SELF)
SELF_BLOCKING = {"author": {"login": "solo", "__typename": "User"},
"body": "must fix this blocking issue"}
# GitHub logins are case-insensitive: a differently-cased self login must
# still be recognized, matching the casefold the engine applies.
SELF_BLOCKING_MIXED_CASE = {"author": {"login": "SoLo", "__typename": "User"},
"body": "must fix this blocking issue"}
OTHER_BLOCKING = {"author": {"login": "reviewer", "__typename": "User"},
"body": "must fix this blocking issue"}
SELF_NONBLOCKING = {"author": {"login": "solo", "__typename": "User"},
"body": "small stylistic note, take it or leave it"}
OTHER_NONBLOCKING = {"author": {"login": "reviewer", "__typename": "User"},
"body": "small stylistic note, take it or leave it"}

def test_self_login_reply_does_not_manufacture_dispatch(self) -> None:
# A recent worker check-in suppresses the bounded `quiet_recheck_due`
# fallback, so the only signal that could still dispatch is the
# (unsuppressible) new_human_blocking_feedback delta. Filtered out for a
# self-authored comment, it must leave the PR needing no worker.
result = classify(make_pr(comments=[self.SELF_BLOCKING]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
self.assertNotIn("new_human_blocking_feedback",
result["needs_worker_reasons"])
self.assertFalse(result["needs_worker"])

def test_self_login_match_is_case_insensitive(self) -> None:
result = classify(make_pr(comments=[self.SELF_BLOCKING_MIXED_CASE]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
self.assertNotIn("new_human_blocking_feedback",
result["needs_worker_reasons"])
self.assertFalse(result["needs_worker"])

def test_self_login_item_is_still_classified_human_blocking(self) -> None:
# Deliberate scope: the comment is excluded only from the dispatch
# delta, not from classification -- so a genuine "do not merge" comment
# the maintainer posts under their own login still halts the merge gate.
result = classify(make_pr(comments=[self.SELF_BLOCKING]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
authors = [item["author"]
for item in result["feedback"]["human_blocking"]]
self.assertEqual(authors, ["solo"])
self.assertTrue(result["human_stop"]["required"])
self.assertFalse(result["pr_clean_ready_for_direct_gate"])

def test_other_login_still_fires_under_same_config(self) -> None:
# The filter is login-specific, not a blanket human-feedback mute:
# genuine feedback from any other login must still dispatch a worker,
# even with a recent check-in that suppresses the quiet-recheck fallback.
result = classify(make_pr(comments=[self.OTHER_BLOCKING]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
self.assertIn("new_human_blocking_feedback",
result["needs_worker_reasons"])
self.assertTrue(result["needs_worker"])

def test_self_login_nonblocking_excluded_from_new_feedback_human(self) -> None:
# The self-login filter over the new-human-feedback delta also covers the
# non-blocking `human` arm surfaced in new_feedback["human"]: a self-authored
# non-blocking comment stays classified human -- so it never hollows out the
# maintainer's own signal -- but must not be re-surfaced to the orchestrator
# as new feedback every cycle.
result = classify(make_pr(comments=[self.SELF_NONBLOCKING]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
self.assertEqual([item["author"]
for item in result["feedback"]["human"]], ["solo"])
self.assertEqual(result["new_feedback"]["human"], [])

def test_other_login_nonblocking_still_surfaces_as_new_feedback(self) -> None:
# The non-blocking-arm filter is login-specific, not a blanket mute: a
# non-blocking comment from any other login must still surface as new
# human feedback for the orchestrator to relay.
result = classify(make_pr(comments=[self.OTHER_NONBLOCKING]),
make_prev(last_worker_checkin_at=OBS), self.CONFIG)
self.assertEqual([item["author"]
for item in result["new_feedback"]["human"]], ["reviewer"])


class DispatchPendingUnconfirmedTests(unittest.TestCase):
def test_pending_dispatch_survives_direct_gate_readiness(self) -> None:
prev = make_prev(pending_worker_dispatch_head_sha=HEAD,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
"""Snapshot config assembly: self-login resolution and ClassifyConfig mapping.

Covers the decoupling of the self-identity suppression set from the discovery
`--author` filter. `@me` resolution is stubbed by monkeypatching the `babysit_gh`
seam; no real gh process is spawned.
"""

from __future__ import annotations

import argparse
import pathlib
import sys
import unittest
from unittest import mock

sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))

import babysit_gh as gh # noqa: E402
import pr_queue_snapshot as snapshot # noqa: E402


class ResolveSelfLoginsTests(unittest.TestCase):
def test_autopilot_empty_authors_still_yields_authenticated_login(self) -> None:
with mock.patch.object(gh, "resolve_author", return_value="kyle-sexton"):
self.assertEqual(snapshot.resolve_self_logins([]), ["kyle-sexton"])

def test_discovery_authors_are_unioned_with_the_self_login(self) -> None:
with mock.patch.object(gh, "resolve_author", return_value="kyle-sexton"):
self.assertEqual(
snapshot.resolve_self_logins(["alice", "bob"]),
["alice", "bob", "kyle-sexton"],
)

def test_self_login_already_present_is_not_duplicated(self) -> None:
with mock.patch.object(gh, "resolve_author", return_value="Kyle-Sexton"):
self.assertEqual(
snapshot.resolve_self_logins(["alice", "kyle-sexton"]),
["alice", "kyle-sexton"],
)

def test_input_author_list_is_not_mutated(self) -> None:
authors = ["alice"]
with mock.patch.object(gh, "resolve_author", return_value="kyle-sexton"):
snapshot.resolve_self_logins(authors)
self.assertEqual(authors, ["alice"])

def test_unresolvable_self_login_leaves_discovery_authors_intact(self) -> None:
with mock.patch.object(gh, "resolve_author", return_value=None):
self.assertEqual(snapshot.resolve_self_logins(["alice"]), ["alice"])


class BuildConfigSelfLoginsTests(unittest.TestCase):
def test_resolved_self_logins_populate_config_self_logins(self) -> None:
args = argparse.Namespace(
owners="melodic-software",
resolved_self_logins=["kyle-sexton"],
)
config = snapshot.build_config(args)
self.assertEqual(config.self_logins, frozenset({"kyle-sexton"}))

def test_resolved_self_logins_take_precedence_over_resolved_authors(self) -> None:
args = argparse.Namespace(
owners="melodic-software",
resolved_authors=["alice"],
resolved_self_logins=["kyle-sexton"],
)
config = snapshot.build_config(args)
self.assertEqual(config.self_logins, frozenset({"kyle-sexton"}))

def test_raw_author_fallback_drops_me_when_unresolved(self) -> None:
args = argparse.Namespace(
owners="melodic-software",
author="@me,alice",
)
config = snapshot.build_config(args)
self.assertEqual(config.self_logins, frozenset({"alice"}))


if __name__ == "__main__":
unittest.main()
Loading