fix(source-control): exclude self-logins from automation human-stop gate

[kyle-sexton]

Fixes #902

Self-authored classification replies no longer block branch refresh or review triggers. Merge/triage still consult human_stop.required; automation gates use new human_stop.external_required.

Test plan

• python3 -m pytest plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py -k self_login (7 passed)

Related

N/A

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

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

---

Security Review

• Gather context (diff, changed files)
• Read changed files in full
• Analyze logic/authz/injection/secret-handling risks
• Post findings

Scope: git diff origin/main...HEAD — 7 files, all in plugins/source-control (plugin manifest bump, changelog, and the babysit-prs human-stop gate logic in babysit_delta.py, babysit_feedback.py, refresh_pr_branch.py, request_review.py, plus a test). No GitHub Actions workflow files, secrets, or shell/subprocess call sites are touched by this diff, so those categories (unpinned actions, pull_request_target, permission widening, command/shell injection) don't apply here — this is pure Python control-flow logic gating automated branch-refresh and review-trigger actions on a "human stop" signal.

Core mechanism reviewed: human_stop.external_required is a new field that excludes feedback authored by configured self_logins (the automation's own posting identity) from the automation gate, while human_stop.required (unfiltered) still gates merge/triage. The exclusion in babysit_feedback.py#L258-L283 and babysit_delta.py#L467-L482 is an exact, case-folded string match against an operator-supplied self_logins set — it is not derived from anything in PR-authored content (comment bodies, branch names, etc.), so an external commenter cannot forge membership in that set to get their own blocking feedback silently excluded. I did not find a way for untrusted PR content to widen self_logins or otherwise spoof the author field GitHub reports for a comment/review.

No CRITICAL or IMPORTANT security issues found in this diff.

---

SUGGESTION / Confidence: High (not a vulnerability — fail-closed, informational)

The self_logins exclusion isn't actually wired into the two "live re-check" call sites that decide whether to perform the mutation:

• refresh_pr_branch.py#L67 — fetch_current_human_stop(repo, number, current)
• request_review.py#L137 — fetch_current_human_stop(repo, number, current)

Neither passes self_logins=, so per babysit_feedback.py#L288-L303 normalized_self defaults to an empty set, so external_required computed at this call site always equals required (no self-authored items get excluded). Since run_locked in both scripts checks the snapshot's human_stop (correctly filtered via classify_pr, which does wire config.self_logins) before calling validate_current_candidate, but validate_current_candidate then re-derives human_stop live without the exclusion, the live re-check will still raise "human review stop is active" whenever the only blocking feedback is self-authored — i.e. exactly the scenario issue #902 and this PR's description say should no longer block branch refresh / review triggers. This doesn't weaken any gate (it's strictly more restrictive than intended, not less), so there's no security bypass — but it means the fix likely doesn't take effect at runtime for these two paths. The test plan only covers classify_pr (test_babysit_delta.py -k self_login); there's no test exercising refresh_pr_branch.py/request_review.py's live re-check with a self-authored-only blocker. Worth a look, though it's a correctness gap rather than a security one. Fix this →

---

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80f5b8ab36

ℹ️ 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 1m 59s —— View job

---

Security Review

• Gather context (diff via gh pr diff 2469, changed files)
• Read changed files in full (babysit_delta.py, babysit_feedback.py, refresh_pr_branch.py, request_review.py, plus manifest/changelog/test)
• Analyze logic/authz/injection/secret-handling risks
• Post findings

Scope: gh pr diff 2469 against HEAD 1d74f3b — 7 files, all in plugins/source-control (plugin manifest bump, changelog, and the babysit-prs human-stop gate logic). No GitHub Actions workflow files, secrets, or shell/subprocess call sites are touched, so unpinned-action, pull_request_target, permission-widening, and command/shell-injection categories don't apply — this is pure Python control-flow logic gating automated branch-refresh and review-trigger actions on a "human stop" signal.

Core mechanism reviewed: human_stop.external_required excludes feedback authored by operator-configured self_logins from the automation gate, while human_stop.required (unfiltered) still gates merge/triage. The match in babysit_classify.py#L176-L178 is an exact, case-folded string comparison against an operator-supplied set, derived from GitHub's own author.login field — not from anything in PR-authored content — so an external commenter cannot spoof membership in self_logins to get their own blocking feedback silently excluded. No authorization bypass found in this mechanism.

No CRITICAL or IMPORTANT security issues found in this diff.

---

SUGGESTION / Confidence: High (not a vulnerability — fail-closed, informational)

The self_logins exclusion still isn't wired into the two live re-check call sites that gate the actual mutation, so the fix in this PR doesn't take effect for the paths issue #902 names:

• refresh_pr_branch.py#L67 — human_stop_blocks_automation(fetch_current_human_stop(repo, number, current))
• request_review.py#L137-L138 — same pattern

Neither passes self_logins= into fetch_current_human_stop, so per babysit_feedback.py#L288-L302 it defaults to None → normalized_self = frozenset() in human_stop_state, so external_required always equals required at these two call sites, and the new human_stop_blocks_automation helper prefers external_required when present — so it still resolves to the unfiltered value there. The run_locked snapshot check in both scripts reads human_stop from state correctly filtered (via classify_pr, which does wire config.self_logins — babysit_delta.py#L467-L482), but validate_current_candidate's live re-check re-derives human_stop without the exclusion and will still raise "human review stop is active" whenever the only blocking feedback is self-authored — exactly the scenario this PR intends to unblock.

This is strictly more restrictive than intended (fail-closed), not a security bypass, so it's not exploitable. The added test (test_babysit_delta.py -k self_login) only covers classify_pr; there's no test exercising the live re-check path in refresh_pr_branch.py/request_review.py. This was also flagged by Codex's automated review on this same commit and remains present. Fix this →

---

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.

[claude]

Claude finished @kyle-sexton's task in 1m 4s —— View job

---

Security Review

• Gather context (git diff origin/main...HEAD, changed files)
• Read changed files in full
• Analyze logic/authz/injection/secret-handling risks
• Post findings

Scope: git diff origin/main...HEAD at HEAD a9b3d4b — 7 files, all in plugins/source-control (plugin manifest bump, changelog, and the babysit-prs human-stop gate logic). No GitHub Actions workflow files, secrets, or shell/subprocess call sites are touched, so unpinned-action, pull_request_target, permission-widening, and command/shell-injection categories don't apply — this is pure Python control-flow logic gating automated branch-refresh and review-trigger actions on a "human stop" signal.

Core mechanism reviewed: human_stop.external_required excludes feedback authored by operator-configured self_logins from the automation gate, while human_stop.required (unfiltered) still gates merge/triage. The match against self_logins (case-folded, exact string comparison against GitHub's author.login) is not derived from anything in PR-authored content, so an external commenter cannot spoof membership in self_logins to get their own blocking feedback silently excluded. No authorization bypass found in this mechanism itself.

No CRITICAL or IMPORTANT security issues found in this diff.

---

SUGGESTION / Confidence: High (not a vulnerability — fail-closed, informational)

This commit adds the human_stop_blocks_automation() helper and wires it into both live re-check call sites, but the underlying data it reads is still unfiltered at those sites, so the fix does not take effect for the two paths issue #902 names:

• refresh_pr_branch.py#L67 — human_stop_blocks_automation(fetch_current_human_stop(repo, number, current))
• request_review.py#L137-L138 — same pattern

Neither passes self_logins= into fetch_current_human_stop. Per babysit_feedback.py#L290-L303, the omitted kwarg defaults to None → normalized_self = frozenset() in human_stop_state, so external_blocking equals human_blocking unfiltered, and external_required always equals required at these two call sites. Since human_stop_blocks_automation prefers external_required when present, it still resolves to the unfiltered value there — the live re-check will keep raising "human review stop is active" whenever the only blocking feedback is self-authored, exactly the scenario this PR intends to unblock.

Neither script even has a self_logins/FeedbackConfig value in scope to pass at that call site: refresh_pr_branch.py's validate_current_candidate (L56-81) takes no config parameter at all, and request_review.py's ReviewTriggerConfig (used by validate_current_candidate at L80-144) has no self_logins field — only babysit_delta.py's DeltaConfig carries it.

This is strictly more restrictive than intended (fail-closed), not a security bypass, so it is not exploitable. The added test (test_babysit_delta.py -k self_login, diff) only asserts classify_pr's output and still doesn't exercise the live re-check path in either script. This exact gap was flagged on the two prior commits in this PR (by this review lane and by Codex) and remains unaddressed as of a9b3d4b. Fix this →

---

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.

[claude]

Claude finished @cursor[bot]'s task in 2m 51s —— View job

---

Security Review

• Gather context (gh pr diff 2469, changed files)
• Read changed files in full at HEAD c65169a
• Analyze logic/authz/injection/secret-handling risks
• Post findings

Scope: gh pr diff 2469 at HEAD c65169a — 7 files, all in plugins/source-control (plugin manifest bump, changelog, and the babysit-prs human-stop gate logic). No GitHub Actions workflow files, secrets, or shell/subprocess call sites are touched, so unpinned-action, pull_request_target, permission-widening, and command/shell-injection categories don't apply — this is pure Python control-flow logic gating automated branch-refresh and review-trigger actions on a "human stop" signal. This head's only new commit versus the prior-reviewed a9b3d4b is a version-bump/changelog chore (plugin.json, CHANGELOG.md only) — the babysit-prs scripts are unchanged from a9b3d4b.

Core mechanism reviewed: human_stop.external_required excludes feedback authored by operator-configured self_logins from the automation gate, while human_stop.required (unfiltered) still gates merge/triage. The match against self_logins (babysit_delta.py#L467-L476) is an exact, case-folded string comparison against GitHub's own author.login field, not against anything in PR-authored content, so an external commenter cannot spoof membership in self_logins to get their own blocking feedback silently excluded. No authorization bypass found in this mechanism.

No CRITICAL or IMPORTANT security issues found in this diff.

---

SUGGESTION / Confidence: High (not a vulnerability — fail-closed, informational) — posted inline on both call sites:

The self_logins exclusion still isn't wired into the two live re-check call sites that gate the actual mutation, so the fix does not take effect for the two paths issue #902 names:

• refresh_pr_branch.py#L67 — human_stop_blocks_automation(fetch_current_human_stop(repo, number, current))
• request_review.py#L137-L138 — same pattern

Neither passes self_logins= into fetch_current_human_stop. Per babysit_feedback.py#L290-L303 the omitted kwarg defaults to None → normalized_self = frozenset(), so external_required always equals required at these two call sites, and human_stop_blocks_automation still raises "human review stop is active" whenever the only blocking feedback is self-authored — exactly the scenario this PR intends to unblock. Neither script has self_logins in scope to pass at that call site: refresh_pr_branch.py's validate_current_candidate takes no config parameter at all, and request_review.py's ReviewTriggerConfig has no self_logins field — only babysit_delta.py's DeltaConfig carries one.

This is strictly more restrictive than intended (fail-closed), not a security bypass, so it's not exploitable. The added test (test_babysit_delta.py -k self_login) only covers classify_pr; there's still no test exercising the live re-check path in either script.

This exact gap was flagged by this lane and by Codex on all three prior commits in this PR (80f5b8ab, 1d74f3b, a9b3d4b) and remains unaddressed at this head — c65169a only bumped the plugin version and changelog. Fix this →

---

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.
· Branch

[github-actions]

Last security-reviewed head: feb26b8b75f5354d153566e04d03355b09502fa4. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @cursor[bot]'s task in 2m 50s —— View job

---

Code Review

• Gather context (gh pr diff 2469, CLAUDE.md, REVIEW.md, security-lane check)
• Determine skip-gate applicability
• Review changed files for correctness/maintainability
• Post findings

Scope: gh pr diff 2469 at HEAD c65169a — 7 files in plugins/source-control (manifest bump, changelog, babysit-prs human-stop gate logic, test). This repo carries .github/workflows/claude-security-review.yml, so per REVIEW.md this lane owns correctness/design/tests/docs and leaves vulnerabilities/authz/injection to the security lane — findings below are both correctness, not security.

Two inline findings posted:

1. 🔴 plugins/source-control/CHANGELOG.md#L6-L29 — unresolved git merge-conflict markers committed to the file. Literal <<<<<<< HEAD / ======= / >>>>>>> a9b3d4b5 (...) markers landed via the latest commit ("chore: bump plugin versions for changelog parity") and are still present at HEAD. The [0.53.10]/[0.53.11]/[0.53.13] entries are garbled as a result. Suggested clean resolution included inline.

2. 🔴 refresh_pr_branch.py:67 / request_review.py:137 — self_logins still isn't threaded into the live human-stop re-check. Both call fetch_current_human_stop(repo, number, current) without self_logins=, so it defaults to an empty set and external_required collapses to unfiltered required at these two call sites — the live re-check still raises "human review stop is active" when the only blocking feedback is self-authored, i.e. exactly the branch-refresh/review-trigger scenario issue #902 and this PR's description say should no longer block. Neither function has a self_logins/config value in scope to pass through yet (ReviewTriggerConfig has no such field). This is fail-closed, not a security bypass, which is why it's reported here rather than the security lane — but it means the fix likely doesn't take effect at runtime for these two paths. This exact gap was already flagged three times on prior commits of this PR (Codex, and the security-review lane on 80f5b8a/1d74f3b/a9b3d4b) and remains unresolved.

The added test (test_babysit_delta.py -k self_login) only asserts classify_pr's output (author-claimed "7 passed" — not independently re-run here since Bash execution wasn't available in this session) and still doesn't exercise the live re-check path in either script.

No other correctness, design, or test-coverage issues found in this diff.
· branch cursor/refresh-self-login-human-stop-63e1

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 22s —— View job

---

Security Review

• Gather context (diff since last-reviewed head c65169a, changed files)
• Read changed files in full
• Analyze logic/authz/injection/secret-handling risks
• Post findings

Scope: HEAD is now 2bee057, a merge of main into this branch. The merge brought in unrelated changes to other plugins (autonomy, guardrails, rate-limit-guard, work-items, etc. — none touching plugins/source-control). Verified git diff c65169a...2bee057 -- plugins/source-control is empty and git diff origin/main...HEAD --stat still shows only the same 7 files as before (manifest bump, changelog, babysit-prs human-stop gate logic + test). No new security-relevant content since the last review at c65169a.

No CRITICAL or IMPORTANT security issues found in this diff. (Unchanged conclusion from the prior review at this head's content: the self_logins exclusion mechanism itself — case-folded exact match against GitHub's author.login, not derived from PR-authored content — has no authorization bypass.)

---

Still open (informational, not a security finding, fail-closed): self_logins is still not threaded into the live human-stop re-check in refresh_pr_branch.py#L67 and request_review.py#L137, so the live re-check still blocks branch refresh/review triggers on self-authored-only feedback (the exact scenario #902 targets). This is fail-closed (more restrictive, not less), so it's a correctness gap rather than a security bypass — already flagged repeatedly by this lane, the code-review lane, and Codex on prior commits (80f5b8a, 1d74f3b, a9b3d4b, c65169a) and remains unresolved. Also still open: unresolved git merge-conflict markers (<<<<<<< HEAD / ======= / >>>>>>>) in plugins/source-control/CHANGELOG.md — confirmed still present at this head, unchanged since flagged by the code-review lane. Both are correctness/hygiene issues outside this lane's security scope; see the code-review lane's comments for details.

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.
· Branch

[claude]

Claude finished @kyle-sexton's task in 1m 16s —— View job

---

Code Review

• Gather context (gh pr diff 2469, changed files, prior review history)
• Check skip gate — HEAD 2bee057 unchanged in review scope
• Review changed files for correctness/maintainability
• Post findings

Skip gate triggered: git diff c65169a..2bee057 -- plugins/source-control is empty — the only new commit (2bee057, a merge of main into this branch) does not touch any file in plugins/source-control. This lane already posted a successful review against this exact content at HEAD c65169a (comment at 2026-08-12T19:24:42Z, with two inline findings). That review still fully applies, so no new findings are posted here per the skip gate ("this head already has a successful review from this lane that still applies").

For reference, both prior findings remain unresolved at this HEAD:

1. CHANGELOG.md#L6-L29 — unresolved git merge-conflict markers (<<<<<<< HEAD / ======= / >>>>>>> a9b3d4b5 (...)) still committed in the file.
2. refresh_pr_branch.py#L67 / request_review.py#L137 — self_logins still isn't threaded into the live human-stop re-check (fetch_current_human_stop(repo, number, current) is called without self_logins=), so external_required collapses to unfiltered required there and the live re-check still blocks branch refresh / review triggers on self-authored-only feedback — the exact scenario issue source-control:babysit-prs: refresh_pr_branch.py's active-human-review-stop check false-fires on self-authored replies (#473-class, freshness gate) #902 and this PR intend to fix. refresh_pr_branch.py's validate_current_candidate has no config in scope, and request_review.py's ReviewTriggerConfig has no self_logins field.

Both were already flagged multiple times (Codex, and this lane's security/code-review reviews on 80f5b8a, 1d74f3b, a9b3d4b, c65169a) and remain unresolved.
· Branch

[claude]

Claude finished @cursor[bot]'s task in 1m 10s —— View job

---

Security Review

• Gather context (git diff since last-reviewed head, changed files)
• Check skip gate
• Post findings

Skip gate triggered — this head already has a successful security review that still applies.

HEAD is now feb26b8. The only new commit since the last-reviewed head (2bee057) is fa762fb ("fix(hook-utils): write emit_telemetry data to temp file for large payloads"), merged into this branch. Verified:

• git diff 2bee0575f321b5992316e5a870d0b3adad9e7e60..feb26b8b75f5354d153566e04d03355b09502fa4 -- plugins/source-control/skills/babysit-prs/scripts/{babysit_delta,babysit_feedback,refresh_pr_branch,request_review}.py plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py is empty — none of the babysit-prs logic changed.
• git diff origin/main...HEAD --stat still shows the same 7 files as the PR description (manifest bump, changelog, babysit-prs human-stop gate logic, test) — matching this PR's stated scope.
• The only content change touching plugins/source-control since 2bee057 is to CHANGELOG.md (the unresolved merge-conflict markers previously flagged by the code-review lane are now resolved) and hooks/hook-utils.sh (picked up from main via the merge, unrelated to this PR's diff against origin/main).

Neither of those touches security-relevant logic, so this lane's prior conclusion at 2bee057/c65169a still applies: the self_logins exclusion mechanism itself (case-folded exact match against GitHub's author.login, not derived from PR-authored content) has no authorization bypass, and no CRITICAL or IMPORTANT security issues were found in this diff.

Still open (informational, not a security finding, fail-closed): self_logins is still not threaded into the live human-stop re-check in refresh_pr_branch.py#L67 and request_review.py#L137 — both still call fetch_current_human_stop(repo, number, current) without self_logins=, so the live re-check still blocks branch refresh/review triggers on self-authored-only feedback (the exact scenario #902 targets). This is fail-closed (more restrictive, not less), so it remains a correctness gap rather than a security bypass — already flagged repeatedly by this lane, the code-review lane, and Codex on prior commits and remains unresolved at this head. See the code-review lane's comments for details; not re-reported here as a security finding.

No exploitable authorization gaps, injection surfaces, or credential/token handling issues were found in this PR's diff.
· Branch

[claude]

Claude finished @cursor[bot]'s task in 2m 38s —— View job

---

Code Review

• Gather context (gh pr diff 2469, prior review history for this lane)
• Check skip gate against current HEAD feb26b8b
• Diff babysit-prs scripts against the last commit this lane reviewed (c65169a)
• Post status update

Skip gate (partial): git diff c65169a..feb26b8 -- plugins/source-control/skills/babysit-prs is empty — the babysit-prs script/test content this lane reviews is byte-identical to what was already reviewed and inline-commented at c65169a (2026-08-12T19:27Z). The only PR-scoped change at this head vs c65169a is plugin.json's version bump (0.53.16 → 0.53.17, picked up from unrelated merge-forwards) and CHANGELOG.md.

Resolved since the last review: the CHANGELOG.md merge-conflict markers flagged at c65169a are gone. The two merge-forwards from main (2bee057 → feb26b8) brought in a clean upstream changelog and this PR's [0.53.17] entry now stacks on top with no <<<<<<</=======/>>>>>>> markers — confirmed by reading the file at this head: CHANGELOG.md. No further action needed here.

Still open, unresolved: self-logins still aren't threaded into the live human-stop re-check:

• refresh_pr_branch.py#L67 — human_stop_blocks_automation(fetch_current_human_stop(repo, number, current))
• request_review.py#L137-138 — same pattern

fetch_current_human_stop is called without self_logins= at both call sites, so it defaults to an empty set, external_required collapses to unfiltered required, and human_stop_blocks_automation still raises "human review stop is active" when the only blocking feedback is self-authored — exactly the scenario issue #902 and this PR intend to unblock. refresh_pr_branch.py's validate_current_candidate has no config in scope at all to source self_logins from, and request_review.py's ReviewTriggerConfig has no self_logins field.

Since the script content is byte-identical to c65169a, I'm not re-posting duplicate inline comments for this — the existing ones from this lane and from Codex, anchored to these same lines, remain valid and unresolved. This gap has now been flagged across four prior heads (80f5b8a, 1d74f3b, a9b3d4b, c65169a) by Codex and this lane's security/code-review passes, and survived two merge-forwards plus a version-bump commit without being addressed.

No other correctness, design, or test-coverage issues found in the delta since the last review.
· Branch