source-control:babysit-prs: bulk --resolve --include-human can sweep a thread deliberately deferred pending a human ruling

[kyle-sexton]

Lane: babysit, observed on #660, session 2026-07-20.

Triggering example: #660 had a security-relevant P1 thread the author deliberately left unresolved pending a maintainer ruling (tracked in #649, needs-human). Kyle's own reply on that thread is a principled deferral ("held pending ruling"), not a fix. The worker needed --include-human to resolve OTHER, genuinely-addressed threads (since Kyle's own classification replies flip botOnly to false on those). When it dry-ran a bulk --resolve --include-human across the PR, the tool flagged the deferred security thread as would-resolve too — isOutdated:false didn't stop it, since outdated-ness only tracks whether the underlying diff line moved, not whether a human deliberately parked the thread.

The worker avoided this by using explicit --thread-id per thread instead of a bulk sweep — but that's a manual discipline the worker had to notice and apply correctly; nothing in the tool itself prevents a less careful pass from silently resolving a live, deliberately-deferred security concern.

Expected: --include-human bulk resolution should have some signal that distinguishes "human replied with a fix/classification confirming this is addressed" from "human replied explicitly deferring/parking this thread" — e.g. don't bulk-resolve a thread whose most recent human reply contains defer language ("deferred", "pending ruling", "held pending", "needs-human"), or require per-thread --thread-id confirmation whenever the most recent comment is human-authored, rather than allowing a blanket bulk pass once any human reply exists on the PR.

Category: bug (safety gap) — no incident occurred (the worker used the safe per-thread path), but the bulk path is a real footgun for a less careful pass.

[kyle-sexton]

This was generated by AI during triage.

Agent Brief

Type: Bug (safety gap)
Summary: Machine-enforce the existing "pin each --include-human resolve per-thread" discipline so the guarded resolve wrapper rejects a bare bulk --include-human --resolve (no --thread-id), eliminating the blanket sweep that can silently resolve a deliberately-deferred human thread.

Framing (why this is gap-closure, not a design choice)

The skill's safety reference already mandates that a bare --include-human resolve must never be surfaced, and that each vetted thread be pinned individually with a single --thread-id (plus its comment-count and last-updated pins). The tool does not enforce this: it only enforces the equivalent rule for the --autonomous --resolve path, which already refuses to run without a pinned single --thread-id. This bug is therefore make the machine enforce the policy the tool's own docs already require for the --include-human path — the direct precedent is the --autonomous --resolve per-thread requirement, and the same "agent discipline vs machine-enforced" split tracked separately for the push-displacement (isOutdated) bypass.

The issue offered two directions: (a) detect defer/parking prose in the most recent human reply and exclude such threads, or (b) require per-thread --thread-id confirmation instead of a blanket bulk pass. Direction (b) is the root-cause fix for this component, and (a) is explicitly rejected, because the guarded resolver's own contract is deterministic guards only — every judgment that needs reasoning stays with the evaluating agent. A prose "is this a deferral" heuristic is exactly the reasoning-based guard this module refuses to host; it would also carry a false-negative security risk (an unusually-phrased deferral silently swept). Direction (b) is fully deterministic and changes no documented workflow — the per-thread pinned form is already the form the docs instruct.

Current behavior:

• The guarded resolve wrapper accepts --resolve --include-human with no --thread-id. This re-fetches the live thread list and resolves every eligible thread (all bot, AI-review, and human threads on the PR) in one blanket pass.
• The only "already-addressed" signal available to the bulk path is GitHub's isOutdated (the anchored diff line moved). isOutdated does not track human intent, so a thread a human deliberately parked ("held pending ruling", not a fix) is indistinguishable from a genuinely-addressed one and is swept by the same blanket pass.
• A careful operator avoids this only by manually choosing the per-thread --thread-id path — pure agent discipline. Nothing in the tool prevents a less careful bulk pass from silently resolving a live, deliberately-deferred security concern.
• By contrast, the --autonomous --resolve path already refuses to run without a single pinned --thread-id, turning that path into a per-thread vetted loop.

Desired behavior:

• --resolve --include-human without a single --thread-id is a usage error (fail closed, refuse to act), with a message that states why (a bulk human-thread resolve re-evaluates every eligible thread at execution time and could sweep a thread deliberately parked pending a human ruling) and directs the caller to the per-thread pinned form. This mirrors the existing --autonomous --resolve refusal.
• --resolve --include-human --thread-id <id> with the required pins (comment-count and last-updated) continues to work exactly as today — resolving one operator-vetted human/AI-review/bot thread. The fix removes only the blanket form, not the ability to resolve human threads.
• List/dry-run mode is unaffected: with --include-human but no --resolve, the wrapper still enumerates every thread and reports would-resolve across human threads, so an operator retains full visibility of what the per-thread pass would target.
• The refusal fires on the flag combination alone — it does not attempt to read thread content or classify the human reply's intent.

Key interfaces / behavior:

• The wrapper's argument-validation stage — the same stage that today refuses --autonomous --resolve without a --thread-id — gains a parallel refusal for --include-human --resolve without a --thread-id. Exit with the existing usage-error exit code (the code already used for the --autonomous bulk refusal and other usage errors), emitting the wrapper's single JSON object carrying an error field.
• The per-thread action classifier and the TOCTOU pin logic (comment-count and last-updated pins) are unchanged — this fix is purely at the flag-combination gate; once a --thread-id is present the existing pinned per-thread path runs as-is.
• No new configuration, no new content signal, no new thread projection field.

Documentation (in scope — the tool will reject commands the docs currently show):

• The skill's main instructions and safety/autopilot reference currently show a bare bulk --resolve --include-human (the "resolve each addressed thread via --resolve --include-human" instruction and the autopilot thread-widening passages). After this change those exact commands are refused. Every such occurrence must be updated to the pinned per-thread form (--resolve --include-human --thread-id <id> --expected-comment-count <n> --expected-last-updated <ts>), matching the form the safety reference already prescribes elsewhere.
• The safety reference's existing "never surface a bare --autonomous or --include-human resolve" discipline line must be updated to note that the --include-human half is now machine-enforced, not merely a discipline expectation (parallel to how the --autonomous half is described).

Acceptance criteria:

• Invoking the resolve wrapper with --resolve --include-human and no --thread-id exits with the wrapper's usage-error exit code and emits JSON with an error field explaining the refusal; no thread is resolved.
• Invoking with --resolve --include-human --thread-id <id> plus the required comment-count and last-updated pins resolves that single thread exactly as before (verified against the existing per-thread resolve behavior/tests).
• Invoking with --include-human and no --resolve (list/dry-run) still enumerates and reports would-resolve for human threads — visibility is preserved.
• A regression test in the wrapper's guard/resolve test suite asserts the bare-bulk --include-human --resolve refusal, alongside the existing --autonomous --resolve bulk-refusal test.
• The skill's full test/lint suite (the plugin's Python test + ruff configuration under the skill's scripts) passes with the change.
• No documentation in the skill shows a bare --resolve --include-human without a --thread-id; every occurrence uses the pinned per-thread form, and the safety reference notes the --include-human bulk refusal is machine-enforced.

Out of scope:

• Content-based deferral detection. Do not add prose/keyword scanning ("deferred", "pending ruling", "held pending", "needs-human") to the resolver. That is a reasoning-based guard the wrapper's contract deliberately excludes; distinguishing a parked thread from an addressed one remains the evaluating agent's judgment. Note: this fix removes the blanket-sweep footgun; it does not by itself distinguish a parked thread from an addressed one when an operator explicitly targets it per---thread-id — that residual protection stays with per-thread operator judgment. A deterministic, explicit machine-readable "hold" marker convention (a token a human puts in the parking comment that the resolver treats as a hard refuse) is a plausible future defense-in-depth enhancement, deliberately deferred here — not part of this change.
• The --autonomous --resolve path and its existing per-thread requirement — unchanged.
• The push-displacement (isOutdated) bypass tracked separately — unchanged.
• --allow-unpinned-thread under --include-human. Current behavior (the interactive-only unpinned override remains available for a single --thread-id resolve, and is not gated the way --autonomous gates it) is preserved as-is by this change; whether autopilot's unattended --include-human runs should additionally refuse --allow-unpinned-thread the way --autonomous does is a separate hardening question and is not decided here.

Confidence: HIGH — recommend status: ready.