fix(source-control): refuse autonomous resolve on own PRs

[kyle-sexton]

Summary

• Add fetch_pull_request_author and fail closed when --autonomous or --independent-resolver targets a PR whose author is in --self-logins
• Interactive modes are unchanged

Closes #1287

Test plan

• python3 -m pytest plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_resolve_thread.py (109 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 1m 25s —— View job

---

Security Review — PR #2451

Todo list

• Gather context (PR diff, changed files)
• Review babysit_gh.py changes
• Review babysit_resolve_thread.py changes
• Review test file changes (context only)
• Post final security review findings

Result: No security issues found

Reviewed the two implementation files (babysit_gh.py, babysit_resolve_thread.py) that make up this fix. This is a self-contained, defense-in-depth change — no .github/workflows files or CI config are touched.

Things I specifically checked and found sound:

• New fetch_pull_request_author (babysit_gh.py#L358) invokes gh through the existing _run_gh/run_command seam, which passes an argv list to subprocess.run with allowed_executables=("gh",) and no shell — no command-injection surface. repo/number are validated upstream by parse_repo_number (owner/repo regex allowlist) before reaching this call.
• Trust boundary for pr_author: the value comes straight from gh pr view --json author, i.e. GitHub's API — it can't be spoofed via PR/comment body content the way, say, a github.event.* template value could be.
• Fail-safe defaults: is_self_login / normalize_self_logins only match a non-empty, explicitly-configured self login (babysit_classify.py#L162-L178); a null/deleted author resolves to "" and correctly never matches, so the guard degrades to "does nothing" rather than to a false match — consistent with how --self-logins already behaves elsewhere in this file.
• No override flag: unlike --allow-unpinned-thread, this refusal has no autonomous escape hatch, matching the PR description's intent ("mirrors the merge gate's human-only --allow-unprotected pattern") — an autonomous/independent-resolver run can't self-authorize past it.
• Ordering: the check runs after owner-scope validation and after self_logins resolution, consistent with the file's existing ordering comments, and it exits 2 on --resolve (mutating) vs 0 on a read-only analysis call — no path was found where the refusal is silently skipped for a matching self-authored PR in autonomous/independent-resolver mode.

Test additions (test_babysit_resolve_thread.py) directly cover own-PR refusal, the --resolve exit-code-2 path, and that non-self authors proceed normally — nothing there weakens an assertion or disables a check.

No CRITICAL, IMPORTANT, or SUGGESTION findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecc4dcce26

ℹ️ 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 4m 31s —— View job

---

Claude's review

Todo list

• Gather context (read CLAUDE.md, REVIEW.md, diff, changed files)
• Understand the request (code review of the fix)
• Review babysit_gh.py changes
• Review babysit_resolve_thread.py changes
• Review test coverage changes
• Post findings as inline comments and summary

Summary

The fix itself is sound: fetch_pull_request_author is a minimal, correctly-guarded REST call (fail-closed via is_json_object/RuntimeError on any unexpected shape), and the new own-PR check in main() sits in the right place in the guard sequence — after the cheap argument-shape checks and owner-scope refusal, after --self-logins/@me resolution, but before the (more expensive, paginated) thread fetch. The --self-logins-empty-by-default behavior (guard is dormant unless the caller passes --self-logins) matches every other self-identity guard in this file, and SKILL.md confirms the orchestration always passes --self-logins @me,<extra> for this entry point, so the guard is live in the documented usage pattern. Exit-code choice (0 for list mode, 2 for --resolve mode) is consistent with the file's existing taxonomy (pre-flight refusals main()-level use 2; the resolve-outcome-specific "nothing resolved" case uses 10).

Two things worth a look before merge:

1. reference/guard-contract.md / scripts/tests/guard_contract.py weren't updated for the new guard. guard-contract.md's own header states it's generated from guard_contract.py and that its rows "are executed as assertions by scripts/tests/test_guards.py" — the canonical, CI-checked inventory of guard behavior that a "host permission classifier, an orchestration prompt, or a downstream consumer" is told it may rely on. The resolve.* rows there (resolve.allowlist-absent, resolve.autonomous-bulk-refused, etc.) don't include the new own-PR refusal, and test_guards.py's completeness checks (e.g. test_every_mutating_entry_point_is_backed) only validate that existing rows are self-consistent — they won't fail just because a new guard was added to the script but never registered in the contract. So this is a silent gap: CI won't catch it, and a consumer reading the generated doc today has no way to know this refusal exists. Worth adding a resolve.* row for it (and regenerating the doc) in this PR or as an immediate follow-up.
2. Test coverage only exercises --autonomous, not --independent-resolver — see inline comment on test_babysit_resolve_thread.py:829. The two modes share the exact same guard branch (if args.autonomous or args.independent_resolver:) so risk is low, but _run_independent's helper hardcodes a non-self PR author, so no test actually proves the refusal fires (or exits with the right code) under --independent-resolver, despite the PR description claiming both modes are covered.

Neither is a correctness bug in the shipped behavior; both are maintainability/coverage gaps worth closing given this skill's emphasis on the guard contract as an enforced, citable source of truth.

Test plan (author-claimed, not independently run in this review — sandbox denied running pytest): python3 -m pytest .../test_babysit_resolve_thread.py reportedly 109 passed.

[github-actions]

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