fix(source-control): exclude self-login from babysit new_human_blocking_feedback

[kyle-sexton]

Summary

source-control:babysit-prs re-dispatched a worker onto its own prior-round output for a solo maintainer. The pr_queue_snapshot.py delta engine (babysit_delta.py) classified every non-bot comment as human feedback with no exclusion for the login the babysit worker posts under. When the PR author login == the gh auth identity == the configured self-login, the worker's own classification replies and Fixed in <sha> follow-ups — each a fresh comment id — manufactured a new_human_blocking_feedback delta every next cycle. That arm is unsuppressible, so it re-fired forever (confirmed on PR #377, two consecutive cycles, zero real work).

Fix

Filter items authored by the configured self-login(s) out of both new-human delta arms — new_human_blocking_feedback and new_human_feedback — in classify_pr.

• Reuses the existing seam, no new config. config.self_logins already exists on ClassifyConfig, is threaded from pr_queue_snapshot.py's --author resolution (@me → gh api user --jq .login, plus babysit_self_logins extras), and is already consumed by detect_foreign_activity and the merge-gate self-exemption. The filter mirrors that module's {login.casefold() for login in config.self_logins if login} exactly. Empty self_logins is a no-op, preserving current behavior for unconfigured/test callers.
• Mirrors existing discipline. This is the same self-reply exclusion reference/review-discipline.md §1 already mandates for the worker's own classification replies, and brings the human delta to parity with the bot arms — which are self-filtered structurally (the engine comments under a human login and never enters feedback["blocking"]).
• Deliberately scoped to the dispatch deltas, not collect_feedback classification. A self-authored item still classifies as human_blocking, so human_stop["required"] and the triage blocker stay intact — a genuine "do not merge, I found a problem" comment the maintainer posts under their own login still halts the merge gate. Filtering at classification instead would silently strip the solo maintainer's ability to human-stop their own PR (a strictly worse regression). The adjacent human_stop-persistence behavior for the worker's own reply is left as-is for that same safety reason; distinguishing a classification reply from a genuine self-block there would require content heuristics out of scope for this deterministic fix.

reference/orchestration.md's new_human_blocking_feedback arm doc is updated to describe the exclusion, preventing drift.

Scope: fixes the --queue fleet-loop path (the reported reproduction)

This closes #473 as scoped — a deterministic engine-level fix. It is live in the --queue fleet-loop invocation the issue documents (PR #377 across cycles): pr_queue_snapshot.py's discovery branch resolves @me → gh login and rebuilds ClassifyConfig with self_logins populated before classifying, so the filter has its input.

Deliberately not bundled — tracked in #497: the single---pr snapshot path (/source-control:babysit-prs owner/repo#N) builds its config before author resolution and never rebuilds it, leaving config.self_logins empty — so this filter is dormant there. That is a separate, pre-existing orchestrator seam gap, not an engine defect, and because self_logins is the same field detect_foreign_activity reads, populating it in --pr mode also activates foreign-activity detection there — a shared-seam behavior change with wider blast radius than this engine fix warrants. Filed as #497 with the fix direction and an integration-test caveat.

Verification

• Engine suite (scripts/engine.test.sh) — full unittest discover, ruff, and the guarded-wrapper bash checks, all green:

  == unittest suite ==
  Ran 184 tests in 1.126s
  OK
  == ruff ==
  All checks passed!
  == guarded-wrapper behavior ==
  PASS: merge wrapper rejects --allow-unpinned-head
  PASS: merge wrapper reaches fail-closed CLI (no allowlist)
  PASS: resolve wrapper reaches fail-closed CLI (no allowlist)
  

  184 tests = the prior 180 plus the 4 new regression tests below.
• New regression tests (SelfLoginHumanFeedbackExclusionTests in test_babysit_delta.py), all pass:
  • test_self_login_reply_does_not_manufacture_dispatch — with a recent worker check-in (quiet-recheck fallback suppressed), a self-login blocking comment yields no new_human_blocking_feedback reason and needs_worker is False.
  • test_other_login_still_fires_under_same_config — the same config, a comment from another login, still fires new_human_blocking_feedback and dispatches (the filter is login-specific, not a blanket mute).
  • test_self_login_item_is_still_classified_human_blocking — the self item stays in feedback["human_blocking"], human_stop["required"] is True, and pr_clean_ready_for_direct_gate is False (locks in the deliberate scope: the halt is respected, only the dispatch is suppressed).
  • test_self_login_match_is_case_insensitive — a differently-cased self login is still recognized.
• Linters on changed files — markdownlint-cli2 (CHANGELOG.md, orchestration.md): 0 issues; typos (all changed files): exit 0.

Related

Closes #473

• Source issue: source-control:babysit-prs: needs_worker's new_human_blocking_feedback fires on the worker's own self-authored replies (same-login false positive) #473
• Follow-up: babysit-prs: single---pr snapshot mode leaves self_logins empty (self-filter + foreign-activity dormant) #497 — single---pr mode leaves self_logins empty (pre-existing orchestrator gap; deliberately not bundled here).
• Reuses the config.self_logins / babysit_self_logins seam and detect_foreign_activity self-filter introduced in the babysit-prs engine convergence (feat(source-control): converge babysit-prs tiers + engine (0.8.0) #322).

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 51s —— View job

---

PR Review — fix(source-control): exclude self-login from babysit new_human_blocking_feedback

• Load tools
• Read changed files
• Review babysit_delta.py core logic
• Review test coverage
• Review docs/changelog/manifest

---

The fix is correct, well-scoped, and the deliberate scope decision (filtering at the dispatch delta, not at classification) is the right call. A few observations below.

---

Correctness

The core logic (lines 364, 407, 667) is correct:

• self_logins is built once before both comprehensions — no per-item reconstruction.
• casefold() mirrors detect_foreign_activity's own {login.casefold() for login in config.self_logins if login} exactly (babysit_delta.py:205).
• str(item.get("author") or "") is correct here: collect_feedback normalizes the author field to a plain string login (babysit_feedback.py:306: "author": login), unlike raw GitHub comment objects where author_login() would be needed. The test test_self_login_item_is_still_classified_human_blocking asserts item["author"] == "solo" which locks this in.
• Empty self_logins (frozenset() default) produces an empty set — filter is a no-op, preserving existing behavior for unconfigured callers.
• Deliberate non-application to collect_feedback / human_feedback_ids: self-authored items still land in feedback["human_blocking"] and their IDs are still tracked in human_feedback_ids. This is the right call: human_stop["required"] and the triage blocker stay intact, and the IDs accumulate in prev_human_ids so a same-ID won't re-fire even hypothetically. The fix only stops the re-dispatch loop; it doesn't hollow out the solo maintainer's ability to self-stop.

---

One observation: new_human_feedback (non-blocking) arm has no dedicated test

The filter at line 407 covers new_human_feedback (the non-blocking arm stored in new_feedback.human and surfaced as a material_findings entry). All four new tests exercise only new_human_blocking_feedback. The non-blocking arm doesn't feed unsuppressible_delta (so it cannot cause infinite dispatch on its own), but a self-authored comment appearing in new_feedback.human would still mislead the orchestrator into surfacing "new human feedback" to the user on every cycle until the ID is absorbed into prev_human_ids. A short test confirming a self-login comment doesn't appear in result["new_feedback"]["human"] would complete the coverage for line 407.

This is low-priority — the critical infinite-dispatch vector (new_human_blocking_feedback) is fully tested — but worth a follow-up.

---

Everything else looks good

• Version bump 0.9.0 → 0.9.1 (patch, correct for a bug fix).
• CHANGELOG entry is accurate and descriptive.
• orchestration.md update at new_human_blocking_feedback's doc block precisely describes the exclusion and its safety guarantee — no drift risk.
• Tests are well-structured: the four cases cover the suppression itself, case-insensitivity, the preserved classification path (the key safety guarantee), and the non-self login negative case.

Verdict: approve with the above noted. The minor missing test for the non-blocking arm is a follow-up item, not a blocker.

[kyle-sexton]

Babysit worker (login kyle-sexton) — classification of the claude[bot] PR Review (run 29694189284).

Validated each observation against the head-branch code at d4cb160. The review is an approve-with-note; the single actionable item is the coverage gap for the non-blocking arm.

#	Finding	Classification	Evidence	Reacted
1	Core suppression logic (babysit_delta.py L364/407/667, casefold() mirror of L205, str(item.get("author") or "") normalization, empty-self_logins no-op, deliberate non-application to collect_feedback/human_feedback_ids) is correct	VALID — no change (affirmation)	Confirmed L364 self_logins = {login.casefold() …}; L403–408 filter applies to (*human_blocking, *human); collect_feedback L306 sets "author": login; solo-maintainer human-stop stays intact via feedback["human_blocking"]	👍
2	new_human_feedback (non-blocking) arm at L407 has no dedicated test — all four new tests exercise only new_human_blocking_feedback; suggests a test that a self-login comment does not appear in result["new_feedback"]["human"]	VALID — fixed	Verified: new_feedback["human"] (L1059) = new_human_feedback, filtered at L407; prior tests only asserted needs_worker_reasons. Added test_self_login_nonblocking_excluded_from_new_feedback_human (asserts the comment stays in feedback["human"] yet is absent from new_feedback["human"]) + test_other_login_nonblocking_still_surfaces_as_new_feedback (login-specific, not a blanket mute). Both green; full suite 44/44 passing. Fixed in 9a9c991	👍
3	Version bump 0.9.0→0.9.1, CHANGELOG, orchestration.md doc block, and the four blocking-arm tests all accurate	VALID — no change (affirmation)	Confirmed in diff	👍

Finding 2's test was a test-only addition (no behavior change), so no further version/CHANGELOG bump. Head is now 9a9c991.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a9c991370

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 39s —— View job

---

PR Review — fix(source-control): exclude self-login from babysit new_human_blocking_feedback

• Load tools
• Read changed files (git diff)
• Review babysit_delta.py core filter logic
• Review pr_queue_snapshot.py — resolve_self_logins() addition
• Review test coverage (test_babysit_delta.py, test_pr_queue_snapshot.py)
• Review docs / CHANGELOG / plugin.json
• Compose and post final review

---

The fix is correct and the Codex P1 finding (autopilot/widened discovery drops --author, leaving self_logins empty) is properly addressed by resolve_self_logins. A few observations below.

---

Core filter logic (babysit_delta.py)

babysit_delta.py:364 — self_logins is built once before both comprehensions, so there's no per-item reconstruction. Good.

Lines 407 and 667 — both arms (new_human_feedback non-blocking and new_human_blocking_feedback) apply the same guard.

str(item.get("author") or "").casefold() is correct: collect_feedback normalizes author to a plain string login (babysit_feedback.py:306), and test_self_login_item_is_still_classified_human_blocking asserts item["author"] == "solo" which locks that invariant in.

The deliberate non-application to collect_feedback / human_feedback_ids is correct — self-authored items still land in feedback["human_blocking"] so human_stop["required"] and the triage blocker stay intact. IDs also accumulate in prev_human_ids normally, so even hypothetically the same ID wouldn't re-fire.

---

resolve_self_logins (pr_queue_snapshot.py)

pr_queue_snapshot.py:78 — the fix is clean. resolved = list(authors) creates a copy so the input list is never mutated (covered by test_input_author_list_is_not_mutated). Case-insensitive dedup with casefold() matches the engine's own convention throughout.

The build_config priority chain at pr_queue_snapshot.py:37-51 — resolved_self_logins → resolved_authors → raw --author fallback — is clearly structured. build_snapshot's else branch correctly sets args.resolved_self_logins before the second build_config call, so the fleet-loop path gets the right config.

One observation — test_unresolvable_self_login_leaves_discovery_authors_intact tests a path resolve_author doesn't reach in production. The test mocks resolve_author to return None, but the real resolve_author("@me") raises RuntimeError rather than returning None when the login can't be resolved (the return None path is guarded by if not author — i.e., None/empty input, not a failed @me lookup). The resolve_self_logins function itself has no try/except, so a failed @me lookup would propagate as a RuntimeError from build_snapshot. This doesn't change runtime behavior (a broken gh auth already breaks the fleet-loop at resolve_authors earlier), and the defensive guard is fine to have documented — but the test doesn't exercise a real error path. Low priority, worth a comment in the test body.

---

Known scope gap (babysit_self_logins extras in autopilot)

The PR notes this correctly: in autopilot mode, babysit_self_logins extras (which ride on --author) are absent from resolved_self_logins since authors = []. The @me posting identity is covered; a secondary posting identity is not. The fix closes the reported reproduction (#473); the gap is tracked in the follow-up.

---

Everything else looks good

• test_pr_queue_snapshot.py (new file): 8 tests, full coverage of resolve_self_logins and the build_config priority chain. test_self_login_already_present_is_not_duplicated correctly verifies case-insensitive dedup (mock returns "Kyle-Sexton", input has "kyle-sexton" — no append, original casing preserved). ✓
• test_babysit_delta.py additions: 6 tests across both arms. test_self_login_nonblocking_excluded_from_new_feedback_human and its companion close the non-blocking arm gap noted in the prior review cycle. ✓
• Version bump 0.9.1 → 0.9.2 — correct for a bug fix landing after the 0.9.1 slot.
• CHANGELOG entry is accurate. Orchestration.md update at new_human_blocking_feedback is precise and prevents drift.

Verdict: approve. The test_unresolvable_self_login_leaves_discovery_authors_intact observation is worth tracking but not a blocker — the defensive guard is safe and the real error mode still propagates correctly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ddfd248f4f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 5m 25s —— View job

---

PR Review — fix(source-control): exclude self-login from babysit new_human_blocking_feedback

• Load tools
• Read changed files (git diff origin/main...HEAD)
• Review babysit_delta.py filter logic
• Review pr_queue_snapshot.py — resolve_self_logins() and priority chain
• Assess Codex P2 finding (discovery authors in self-login set)
• Review test coverage (test_babysit_delta.py, test_pr_queue_snapshot.py)
• Review docs / CHANGELOG / plugin.json

---

Core engine fix (babysit_delta.py)

The filter at babysit_delta.py:364-408 is correct:

• self_logins built once before both comprehensions — no per-item reconstruction.
• casefold() mirrors detect_foreign_activity's own convention at babysit_delta.py:205 exactly.
• str(item.get("author") or "").casefold() is correct: collect_feedback normalizes author to a plain string login (babysit_feedback.py:306), and test_self_login_item_is_still_classified_human_blocking asserts item["author"] == "solo" which locks that invariant.
• Scoping to dispatch deltas only is correct — self-authored items still land in feedback["human_blocking"] so human_stop["required"] and the merge gate stay intact. IDs accumulate in prev_human_ids normally, preventing the same ID from re-firing hypothetically.
• Empty self_logins is a no-op (no regression for unconfigured callers).

resolve_self_logins (pr_queue_snapshot.py:78-93)

The core intent — resolving @me independently of the discovery authors list so autopilot runs still populate self_logins — is correct and closes the reported vector. The non-mutation guard (resolved = list(authors)) is covered by test_input_author_list_is_not_mutated.

Codex P2 — discovery authors co-inhabit the self-login set

The Codex P2 finding at pr_queue_snapshot.py:138 is technically valid but describes a design limitation rather than a blocking regression.

resolve_self_logins(authors) returns the union of discovery authors and the authenticated login:

def resolve_self_logins(authors: list[str]) -> list[str]:
    self_login = gh.resolve_author("@me")   # "bot-login"
    resolved = list(authors)               # ["alice"]  ← discovery scope
    if self_login and self_login.casefold() not in {...}:
        resolved.append(self_login)
    return resolved                        # ["alice", "bot-login"]

build_config then sets self_logins = frozenset(["alice", "bot-login"]), so Alice's human feedback is excluded from the new-feedback dispatch delta. The test test_discovery_authors_are_unioned_with_the_self_login explicitly documents this behavior.

Three reasons this is not a blocker:

1. Intentional design. The plugin.json babysit_self_logins description reads "Extra GitHub posting identities (e.g. a project bot account) added to your gh api user login — the self set babysit-prs uses for discovery scope, readiness-gate classification rows, and the merge-gate self-exemption." The field is documented to serve double duty — discovery scope AND posting identity — so this PR is behaving as the existing contract specifies. The scenario Codex describes (authenticated identity ≠ discovery author) is outside the documented design target.

2. Human-stop safety is preserved. Even in the P2 scenario, feedback["human_blocking"] is unaffected — Alice's comment still sets human_stop["required"] = True and the merge gate halts. The only effect is that the dispatch arm doesn't re-fire on Alice's feedback, which is less wrong than the pre-fix infinite re-dispatch.

3. Pre-existing implicit behavior. Before this PR, self_logins = frozenset(resolved_authors) in the build_config fallback path already put discovery authors in self_logins for detect_foreign_activity and merge-gate exemption. This PR's filter reuses that established set.

Recommended follow-up: If the use case of watching non-self authors' PRs (--author alice where alice ≠ authenticated identity) is a real target, it warrants a dedicated posting_logins field in ClassifyConfig (separate from self_logins) and a corresponding config seam. That's a wider contract change than this bug fix. File alongside #497.

Codex P1 (single-PR mode) — already scoped out

The PR body explicitly tracks this as #497 with the fix direction. The --pr path takes the args.pr branch at build_snapshot:129 and doesn't reach the resolve_self_logins call, leaving config.self_logins empty there. The scope decision is correct — populating self_logins in --pr mode also activates detect_foreign_activity there, a wider blast-radius change. Not a blocker.

One minor note: test_unresolvable_self_login_leaves_discovery_authors_intact

test_pr_queue_snapshot.py:47-49 mocks resolve_author to return None. In production, resolve_author("@me") either returns a string login or raises RuntimeError (line 185-186 of babysit_gh.py); it only returns None for empty-string input (if not author). The guard if self_login and ... is still correct defensive code, but the test is exercising an unreachable production path. The test is harmless and the guard is right — worth a brief inline comment clarifying that this covers the defensive guard, not a real @me failure path.

Everything else looks good

• Version at 0.9.2 (correct; 0.9.1 slot was taken by the main-branch head-staleness fix). CHANGELOG entry is accurate and well-scoped.
• orchestration.md update at new_human_blocking_feedback precisely describes the exclusion and its safety guarantee.
• 6 new tests (2 non-blocking-arm tests added in 9a9c991, 4 blocking-arm from d4cb160). Coverage is complete across: suppression, case-insensitivity, preserved classification path (the key safety guarantee), non-self-login negative case, and both arms.
• Latest commit a64efe1 — removes an issue-tracker reference from a test comment per comment-hygiene CI. Clean.

Verdict: approve. The Codex P2 discovery-authors-in-self-logins observation is a design-level follow-up, not an engine defect, and should be tracked alongside #497. The core fix is correct, well-scoped, and fully tested.

[kyle-sexton]

Babysit worker (login kyle-sexton) — conflict-resolution round.

Conflict resolved. Merged origin/main into the branch (merge commit ddfd248). The only conflict was plugins/source-control/CHANGELOG.md — a version-slot collision: a sibling source-control PR landed ## [0.9.1] (review-trigger head-staleness F7/F8) on main first. Both sides had independently bumped plugin.json 0.9.0 → 0.9.1, so git auto-converged plugin.json silently to 0.9.1.

Version slot reassigned → 0.9.2. Main now owns 0.9.1; this PR's self-login-exclusion fix takes the next patch slot 0.9.2 (repo convention: fixes are patch bumps). Composed both CHANGELOG entries chronologically (0.9.2 self-login fix over 0.9.1 F7/F8) and corrected the silently-merged plugin.json to 0.9.2. No code conflict — the F7/F8 files (babysit_review_trigger.py, request_review.py) are disjoint from this branch's files (babysit_delta.py, pr_queue_snapshot.py). Full babysit test suite green (78 tests).

CI hygiene fix (a64efe1). The merge pulled in main's comment-hygiene gate, which flagged a tracker reference (issue #473) embedded in an inline # comment in test_babysit_delta.py. Removed it — the class docstring already anchors the issue context; assertions unchanged.

No new reviewer findings this round; prior threads remain addressed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a64efe1687

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".