Skip to content

chore(source-control): give babysit-loop headroom under the skill line cap - #1627

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/1620-babysit-loop-headroom
Jul 26, 2026
Merged

chore(source-control): give babysit-loop headroom under the skill line cap#1627
kyle-sexton merged 1 commit into
mainfrom
chore/1620-babysit-loop-headroom

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #1620. babysit-loop/SKILL.md goes from 499/500 to 459/500 — 41 lines of headroom, up from one. Nothing is deleted; one section is relocated to a skill-local spoke.

The survey the triage deferred

The triage explicitly did not survey babysit-loop's sections and asked whoever took it to confirm against the current file. Section sizes on main:

Section Lines Verdict
Cycle shape 111 No — the skill's core algorithm and its safety-critical rung partition; extracting guts the body
Escalation 69 Yes — 55 of those are the pre-escalation dispatch, detail for one narrow gated path
Telemetry and durable loop state 54 No — convention §4 requires each lane to inline the gh api upsert
Autonomy dimensions, tiers, knobs 51 No — the merge-authority answer; readers need it inline
Required argument and config resolution 48 No — invocation basics, and it already defers to reference/config-resolution.md
Rate-limit guard floor 36 No — convention §6, byte-identical across lanes
Gotchas 32 No — short, scannable, high-value where it is

The extracted section wins on three counts. It is reachable only when an invocation types both autopilot and --merge c3-this-run, so most readers never need it. It was already written as a pointer target ("the full dispatch contract … are owned by Escalation below"), so it was progressive-disclosure shaped before it was a spoke. And it is babysit-loop's own named exception, carrying no cross-lane symmetry constraint — which is exactly what disqualified the two obvious candidates.

On §6 and §4 — why the floor did not move

The triage floated the rate-limit floor as a candidate and flagged the §6 problem. Confirmed, and it rules the floor out for a single-lane change: §6 requires each lane body to inline the operable floor and exists "so the values stay byte-identical across lanes; fleet audits check conformance per consumer". Moving it for one lane breaks that symmetry, and moving it for all three plus rewording §6 is the triage's option 1 — a much larger change that should be its own decision.

The same reasoning independently rules out Telemetry, which the triage did not flag: §4 requires each lane to inline the gh api upsert, for the same runtime-reach reason.

What stayed in the body

Deliberately, every safety-relevant constraint is still named where a reader meets it:

  • the four blocker classes the dispatch never touches (operator-parked items, human blocking feedback, merge conflicts, C4/C5), each named inline;
  • that a landed resolution re-runs step 3's provenance, C4-diff and rung partition before any merge-capable invocation;
  • that an unresolved or uncertain blocker escalates exactly as it would without the exception;
  • that the deterministic gate is not weakened.

The spoke owns mechanics only: frontier-alias resolution and its escalate-rather-than-dispatch rule, the lease and independence requirements, each blocker class's rationale, and the code-change worker lifecycle. No safety floor moved out of the body.

Two smaller changes that follow from the move

The Autonomy section's second widening paragraph restated the dispatch description the new pointer now carries. Rather than leave a third copy to drift, it is reduced to the part only it says — the deterministic gate is not weakened.

prompts/loops/loop-lane-prompts.md cited (babysit-loop, Escalation) for the frontier-alias rule this change moves. Both renders now cite the spoke that owns it. (That file carries the 3b block twice; block-body edits must land in both.)

The honest residual

This does not fix the underlying problem, and I want to be clear about that rather than imply otherwise.

At 459 the file still sits well above the 200-line soft target, so check-skill.sh still warns. Closing that gap means either repeated extractions — each trading body cohesion for headroom, with diminishing candidates that are not convention-constrained or safety-critical — or accepting that a lane skill is legitimately a large document and the soft target is wrong for this class. That is a judgment call, it applies to more than this file, and it should be made once rather than re-litigated per PR. I did not force it here because forcing it is exactly what the issue warned against.

I also did not take option 3 (raise the cap). The cap did real work here: it surfaced a section that genuinely belonged in a spoke. Raising it would have deferred the same wall with nothing learned.

Related

Not closed by this PR:

Verification

Run locally, not assumed:

  • scripts/check-changed-skills.sh origin/mainCHECK-SKILL babysit-loop: PASS — 0 errors, 1 warning(s), SKILL.md 459/500 lines.
  • All four lane skills pass check-skill.sh: babysit-loop 459, babysit-prs 499, work-loop 330, attend-queue 192.
  • No orphan-spoke warning — check 15 requires SKILL.md to reference reference/, and it does.
  • markdownlint-cli2 clean on all three touched files.
  • Content preservation checked by word-stream diff of the extracted region against the spoke: the only differences are the heading restructure and two SKILL.md qualifiers added to cross-references that now point back at the hub.

🤖 Agent-authored (autonomous babysit lane). Generated with Claude Code

https://claude.ai/code/session_016E9qM8CanWf8KkFGcmg4jo

…e cap

Moves the explicit-`autopilot` pre-escalation dispatch mechanics into a
skill-local progressive-disclosure spoke, taking babysit-loop/SKILL.md from
499/500 to 459/500. Nothing is deleted; the section is relocated.

Why this section. It is deep mechanical detail for one narrow path that is
reachable only when an invocation types both `autopilot` and
`--merge c3-this-run`, so most readers never need it, and it was already
written as a pointer target ("owned by Escalation below"). It is
babysit-loop's own named exception, so unlike the rate-limit floor (convention
§6) and the telemetry upsert (§4) it carries no byte-identical-across-lanes
constraint -- moving it changes one lane without breaking lane symmetry. Those
two constrained sections, and Cycle shape's rung partition, were the other
large candidates and were left alone for that reason.

Every safety-relevant constraint stays named in the body where a reader meets
it: the four blocker classes the dispatch never touches, the re-partition
after a resolution lands, and that an unresolved or uncertain blocker
escalates unchanged. The spoke owns the mechanics -- frontier-alias resolution
and its escalate-rather-than-dispatch rule, the lease and independence
requirements, each blocker class's rationale, and the code-change worker
lifecycle. No safety floor moved out of the body.

The Autonomy section's second widening paragraph restated the dispatch
description that the new pointer now carries, so it is reduced to the part
only it says -- the deterministic gate is not weakened -- rather than left as
a third copy to drift.

loop-lane-prompts.md cited "(babysit-loop, Escalation)" for the frontier-alias
rule that this change moves; both renders now cite the spoke that owns it.

Closes #1620

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016E9qM8CanWf8KkFGcmg4jo
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context and understand the request
  • Read changed files
  • Verify content preservation and cross-references
  • Post review findings

Summary

This is a clean structural extraction. The spoke file and the updated hub body hold together correctly. No safety floors moved, and the mechanical verification in the PR description is accurate against the current files.

What I verified

Content preservation. Every safety-relevant statement the PR description commits to keeping in the body is there in SKILL.md:

  • All four blocker classes named inline in the Escalation section (operator-parked, human blocking feedback, merge conflicts, C4/C5) — ✓ lines 296–303
  • Re-partition on resolution (re-runs step 3's provenance, C4-diff and rung partition before any merge-capable invocation) — ✓ line 301
  • Unresolved or uncertain blocker escalates exactly as without exception — ✓ line 302
  • Gate not weakened — ✓ line 130

Spoke correctness. reference/pre-escalation-dispatch.md is scoped correctly on line 5–7: reachable only under the autopilot + --merge c3-this-run pair, with the SKILL.md Escalation section owning when the dispatch fires and its bounds, and this file owning how it runs. The bidirectional pointers are consistent.

SKILL.md qualifiers in the spoke. Lines 55 and 65 correctly say "SKILL.md Cycle shape, step 3" rather than bare "Cycle shape, step 3", which would be ambiguous with the hub no longer nearby.

loop-lane-prompts.md dual-render update. Both identical occurrences of (babysit-loop, Escalation) were updated to (babysit-loop, \reference/pre-escalation-dispatch.md`)— line 561 (template) and line 1392 (filled instance formelodic-software/claude-code-plugins`). Both are correct: the frontier-alias rule now lives in the spoke, not the Escalation section.

One minor note

reference/pre-escalation-dispatch.md line 48–49:

The one exception babysit-prs gained in this change is scoped to security/P1 escalation…

"In this change" is a changelog artifact — it will read oddly once the PR is merged and the context is gone. Something like "The one exception babysit-prs carries for security/P1 escalation…" would be timeless. Not a blocker; just something to note for a follow-up or squash edit before merge.

Overall

The extraction is correct. The section that moved was genuinely spoke-shaped (reachable only via one narrow path, already written as a pointer target), and the things that stayed in the body are the ones that a reader meets on the way through regardless of invocation mode. The PR description's table and rationale are accurate against the files as they stand. Nothing further to resolve.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #1627

  • Gather context and understand the PR
  • Read changed files (SKILL.md, pre-escalation-dispatch.md, loop-lane-prompts.md)
  • Analyze for security issues in the changed content
  • Post findings

No security issues found.

This PR is a prose refactor of agent instruction files — extracting the pre-escalation dispatch mechanics from SKILL.md into a new spoke file. The changed files contain no executable code, shell commands, workflow YAML, or credential handling. The review focused on whether the reorganization weakens any of the agent-behavior security invariants the original text established.

What was checked:

  • Human-blocking-feedback bypass — The constraint that a human CHANGES_REQUESTED review, explicit human blocking language, or an unresolved inline human thread is a stop-and-ask condition (never dispatched past) is still named inline in SKILL.md L299 and preserved with full rationale in the spoke. No bypass introduced.

  • Operator-parked item gating — The distinction between machine-escalated and operator-parked needs-human items, and the step-3 withholding rule, are both inline in SKILL.md and detailed in the spoke. The gate is intact.

  • Escalation-on-failure invariant — "An unresolved or uncertain blocker escalates exactly as it would without the exception" appears inline in the updated SKILL.md at L302–303. This is the key safety floor; it did not move to the spoke only.

  • C4/C5 exclusion — Named inline in SKILL.md and preserved in the spoke. The rung partition ceiling is unchanged.

  • Merge-conflict routing — Named inline; still routes to the dedicated merge-only worker with no-rebase constraint.

  • Lease and guarded-mutation requirements — These moved to the spoke as mechanics detail (how the dispatch runs, not when to escalate). The spoke is an explicit pointer target: SKILL.md's Escalation section ends with a named pointer to the spoke for "the full contract — frontier-tier resolution and its escalate-rather-than-dispatch rule, the lease and independence requirements." Any agent implementing this path is directed there before running it.

  • Frontier-alias fallback — The rule "a run that cannot establish which alias currently satisfies frontier escalates rather than dispatching" moved from inline SKILL.md to the spoke only. This is the one behavioral detail not summarized inline. It's a capability concern (an agent that misses it might dispatch at a non-frontier tier), not a security bypass (the escalation-on-resolution-failure invariant is still inline). Confidence that this is exploitable is low — the spoke is an explicit mandatory read for any implementation of this path.

  • loop-lane-prompts.md citation update — A reference string change from (babysit-loop, Escalation) to (babysit-loop, \reference/pre-escalation-dispatch.md`)`. No behavioral change.

  • Trust boundaries, injection surfaces, permission widening — None. No workflow files touched, no permissions: blocks, no script injection surface, no credential handling.

@kyle-sexton
kyle-sexton merged commit a85b222 into main Jul 26, 2026
31 checks passed
@kyle-sexton
kyle-sexton deleted the chore/1620-babysit-loop-headroom branch July 26, 2026 22:54
kyle-sexton added a commit that referenced this pull request Jul 29, 2026
…is in flight (#1634)

🤖 Agent-authored (adjudication lane), implementing the code finding this
lane filed as #1629.

## Summary

The merge gate reads GitHub's mergeability, and a review bot that
re-reviews on every push
contradicts that reading for the few minutes its next round takes. The
review does not exist yet,
so there is no unresolved thread to block on and `mergeStateStatus`
stays `CLEAN` throughout — a
gate looking only at that merges past findings which land seconds later.

Measured on #1594, where this actually happened. Codex latency, commit →
posted review, on all
five rounds:

| Head | Committed | Reviewed | Latency |
|---|---|---|---|
| `dddce901` | 18:43:30Z | 18:48:44Z | 5m14s |
| `81e5369d` | 19:56:53Z | 20:01:47Z | 4m54s |
| `372b9872` | 21:24:27Z | 21:28:29Z | 4m02s |
| `3ec45192` | 21:34:46Z | 21:39:04Z | 4m18s |
| `edd9cbfb` | 21:44:00Z | 21:49:06Z | 5m06s |

The merge fired 4m40s after the final commit — below the mean of every
prior sample on that same
PR — and round 5 posted 26 seconds later with two valid findings, one a
regression #1594 itself
introduced (#1613). Round 5 also arrived *after* `head_ref_deleted`, so
no push-side discipline
could have prevented it. Only declining to merge inside the window can.

Two things made the gate structurally unable to see this, both verified
in code:

- `babysit_merge.py` never received `babysit_review_bot_logins` — only
`pr_queue_snapshot.py` and
`request_review.py` did — so it had no notion of a configured reviewer
to wait for.
- The existing reviewer-awareness module could not have covered this
reviewer anyway.
`ReviewTriggerConfig.configured` requires all three of `trigger_phrase`,
`reviewer_logins`, and
`gate_context`, and `review_gate_state()` derives every signal by
matching `gate_context` against
commit-status/check-run entries. Codex publishes a *review*, not a
status context, so there is no
  value that key could take.

## What changed

`babysit_review_bot_logins` paired with the new
`babysit_review_settle_minutes` adds a merge-gate
policy blocker while a configured reviewer still owes the **live head**
a review and that head is
younger than the window.

- **A review of the live head clears it before the clock is read.** The
already-reviewed case adds
no latency and makes no extra request. Evidence is a submitted review
*or* an inline review
comment carrying the head's commit id — `review-trigger.md`'s existing
`has_current_head_review`
  test, reused rather than restated.
- **The window bounds the wait**, so a reviewer that never engages
cannot wedge a PR. It is a
  latency budget, not a review requirement.
- **An unestablishable head age holds** rather than merging on an
unverifiable clock; self-clearing
  on the next run.
- **Both keys or neither** — either alone is a usage error (exit 2),
never a silently inert flag,
and a window converting to under a second is refused rather than
truncated to an inert zero. The
gate defaults no duration of its own, because how long a reviewer takes
is a property of the
  reviewer, not of this gate.

Unconfigured, the gate is byte-for-byte its prior self and issues no
request it did not issue
before. The reviewer corpus is now fetched once per run and shared with
the autopilot merge tier
rather than fetched twice.

### Which clock the age is measured on

This is the part review moved most — it changed in all three rounds — so
it is worth stating
plainly. Head age comes from the **most recent CI start for the live
head**, read from the **raw**
`statusCheckRollup` the gate already fetches — no extra request. Three
properties, each one a
review finding:

- **Server-generated, not client-written.** The first implementation
used the commit's committer
date, which the pushing client controls. A commit pushed long after it
was written — local
batching, an offline delay, replaying an existing commit — read as
already-settled, so the hold
did not fire on exactly the push that triggered a fresh review. A CI
start is minted after the
push, so it can only make a head look *more* recent, which errs toward
holding.
- **Raw rollup, not the classified one.** `classify_checks` keeps only
the newest run per check
  identity, which is lossy in a way this must not depend on.
- **Newest, not oldest — and this direction reversed under review.**
Round 2 pushed toward oldest,
to stop a re-run re-arming the hold. Round 3 showed oldest is the
*unsafe* direction: check runs
live on the SHA, so a head returning to a previously-checked SHA
(force-push A → B → A) still
carries A's original runs, and reading the oldest calls a brand-new head
settled. A fail-closed
gate accepts the latency failure and refuses the safety one. The
accepted cost — a re-run
extending the wait by one window — rarely applies, because a re-run does
not move the head, so an
  already-reviewed head clears the hold before the clock is read at all.

**One residual, stated rather than closed.** If a force-push back to a
checked SHA mints new check
runs, the hold fires correctly; if GitHub reuses the existing results
and mints none, that head
reads as settled. Which it does is unverified, and no queryable "this
SHA became the head at T"
record covers ordinary pushes and force-pushes alike. `safety.md` scopes
the hold as strong for
ordinary pushes and best-effort across a head reverting to an
already-tested SHA.

The committer date survives only as a fallback for a rollup with no
timestamp. It is the weaker
clock and errs the wrong way, so `safety.md` names it as a known weak
spot for repositories with no
checks on their PRs, and a test asserts that behavior deliberately
rather than leaving it implicit.

### The cost, priced explicitly

The hold costs up to one settle window of latency on any merge whose
head the configured reviewer
has not yet reviewed — including *every* merge while that reviewer is
down. That is the trade: a
bounded wait in exchange for not merging past a review already on its
way. It is opt-in per repo,
and unset it costs nothing.

### A limitation shipped knowingly, not silently

Reviewer evidence requires the author to be `Bot`-typed, inherited from
the shared current-head
test. A configured reviewer GitHub reports as a `User` — the class
`--extra-bot-logins` exists for
— never clears the hold early, so every merge waits the full window.
Fail-closed but permanently
slower. Pre-existing in the shared module, and the reviewer motivating
#1629 is genuinely
`Bot`-typed, so it is filed as **#1642** with probe evidence and named
at the hold's `safety.md`
section rather than deferred quietly.

## Scope note: the line cap

`babysit-prs/SKILL.md` was at **499 of a hard 500** — the same wall
#1620 described, which #1627
relieved for `babysit-loop` only. Documenting a new config key needs one
line, and the cap fails at
`>=`, so this was a hard blocker rather than a nicety. The autopilot
tier's per-PR steps,
exclusions, draft handling, and widened scopes move **verbatim** to
`skills/babysit-prs/reference/autopilot.md`, leaving a pointer; the body
goes 499 → 471. Nothing is
deleted, and the tier's operative commands stay where they already
lived, in `reference/safety.md`.
Two `test_skill_contract.py` assertions that guarded the moved prose now
read it at the spoke, and
a new one asserts the body does not re-inline it.

## Test plan

`bash plugins/source-control/skills/babysit-prs/scripts/engine.test.sh`
— **473 tests, OK**; ruff
clean; all 11 guarded-wrapper behavior checks pass.

`CHECK_SKILL_SKILLS_ROOT=… bash
plugins/skill-quality/scripts/check-skill.sh babysit-prs` —
**PASS, 0 errors** (SKILL.md 471/500, markdownlint clean, script test
passed). The one warning is
the pre-existing soft-target-200 advisory.

`python3 scripts/check-manifest-duplicate-keys.py` — no duplicate keys
across 62 manifests.
`markdownlint-cli2` on every changed/added markdown file — 0 errors.
`plugin.json` parses.

30 tests in `tests/test_babysit_merge_review_settle.py` cover:

- **Unconfigured is inert** — asserted against *recorded call counts*,
not just the verdict: no
reviews fetch, no review-comments fetch, no commit read, `reviewSettle:
{enabled: false}`.
- The #1594 shape — young head, reviewer's review on the *previous* head
— blocks.
- A review of another head, and another bot's review of this head, do
not clear the hold.
- Current-head review merges immediately **and reads no clock** (the
no-added-latency path).
- An inline review comment on this head also clears it.
- The server clock beats a stale committer date; **a head returning to a
previously-checked SHA
still holds**; a re-run extends the wait (the accepted cost) while **a
re-run on an
already-reviewed head never reaches the clock** (why that cost is
acceptable); newest wins across
distinct identities; a `StatusContext` contributes its `createdAt`; the
committer-date fallback
  and its documented weak spot.
- Head older than the window stops waiting.
- Commit-read failure and an unparsable date both hold.
- Settle and tier share exactly one reviews fetch, **and the tier still
reaches its verdict on the
  injected corpus** rather than only being called once.
- Paired configuration is fail-closed: each flag alone, an empty login
set, `0`/`-5`/`abc`/`nan`/
`inf`/`-inf`, and sub-second windows are all exit 2; a valid pair
reaches `evaluate` as 600
  seconds.
- Window boundary is exclusive; missing head adds no second blocker;
naive timestamps read as UTC.

## Review

Three findings across two Codex rounds, plus an independent
fresh-context audit. Every finding was
a defect **this PR introduced**, so every one is fixed here rather than
tracked — the
never-defer-a-self-introduced-regression rule from #1614. The one
finding that was *not* this PR's
to fix is #1642, filed with evidence.

**The two escalated P1 threads were resolved by an independent
context**, sharing no history with
the one that authored the PR — the separation `safety.md`'s security/P1
escalation rule requires,
and the reason the authoring context left them open rather than
self-resolving.

Both named one scenario reached by two doors: force-push A → B → A
restores a head whose FIRST
occurrence's review still matches by commit id, because GitHub keeps a
review against the SHA
rather than against the head position. `has_current_head_review` matched
on the SHA alone, so that
stale review satisfied the current-head short-circuit and the gate
merged before any clock was
read. Flipping the clock direction did not touch that path.

`has_current_head_review` now takes an optional `not_before` bound. Only
the settle hold passes
one — the review-trigger completion rule keeps its SHA-only semantics —
and the bound is the newest
CI start on the live head, the same server-observed signal the age is
already measured on, read
from the rollup already in hand. A review predating the bound, or
carrying no parseable timestamp,
no longer clears the hold; evidence records now carry `submittedAt` /
`created_at` so there is
something to compare. A check start cannot separate a restored head from
a re-run on the standing
head, so a re-run minted after the review now re-arms the hold for up to
one window instead of
short-circuiting past it: bounded latency accepted to refuse the safety
failure, priced in
`safety.md` and the CHANGELOG rather than left implicit.

A head with no check starts has no bound, so the earlier review still
clears the hold — precisely
the residual `safety.md` already scoped, now pinned by a test so it is a
decision on record. The
head-appearance clock the reviewer proposed was deliberately **not**
added: only the recency
comparison was needed to close the reachable half, and a second partial
clock plus another API
call in a privileged gate is scope this PR should not take on.

The P2 on `safety.md`'s rendering rule is fixed too: a settle window
configured without reviewer
logins is now refused at the orchestrator, the one case the CLI's
both-or-neither usage error
cannot catch (the instruction omitted both flags, so the lone flag never
reached the CLI).

## Related

Closes #1629. Came out of the #1614 adjudication; #1630 is the separate
governance question that
adjudication surfaced and is deliberately not addressed here. #1642 is
the deferred finding.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 29, 2026
…d rounds (#1633)

🤖 Authored by the autonomous babysit lane (fable-autopilot).

Closes #1614. Implements the adjudicated decision in
#1614 (comment)
(3-0: two independent verifiers plus orchestrator synthesis).

## Summary

The issue proposed three things. One is rejected, one is adopted in a
narrower and stricter form than proposed, and one is settled.

**Item 1 — bound review rounds at 2: rejected.** A ceiling already
exists (`babysit_advisory_fix_round_cap`, default 100, counted per PR in
the durable feedback ledger), and both its homes forbid the proposed
shape in words — "do not stop after a small, arbitrary number of rounds
while real, still-fixable advisory findings remain." A cap of 2
repurposes a backstop against its own rationale, and it was announced
and broken on the very next PR (#1615, round 3, correctly, because the
finding was a real hole in the previous round's fix). Nothing here
lowers the ceiling.

What ships instead is a signal rather than a counter: a third class in
the non-convergence taxonomy. **(c) is a self-inflicted finding** — new
and distinct, but against text this lane's own prior fix on this PR
introduced. Provenance decides the class, never severity. A (c) finding
is fixed like any other in-scope defect and is never deferrable, but it
is counted: a second consecutive all-(c) round means incremental
patching is injecting defects about as fast as it removes them. The
response is a change of **method** — rewrite the contested section whole
in one commit, or report it for a human decision — never a licence to
ship a known defect.

**Item 2 — sanction resolve-by-tracking: adopted, narrowed, and
reclassified.** The issue called this "a documentation change to lane
policy." It is not. `babysit_resolve_thread.py:267-270` already fails
closed under `--autonomous` — "so the worker cannot resolve a
still-current finding and self-satisfy the merge gate" — and `safety.md`
already lists "Resolve any thread over a live, unaddressed finding" as
never-do. A valid finding judged out of scope is, by construction, live
and unaddressed, so item 2 as written would have removed that guard.
**The guard is untouched.**

What ships is stricter than today's behavior, not looser:

- **D4.6 (new)** — a `VALID (defer)` counts as a disposition only when
durable: the tracker item is filed **before** the D5 reply, carries the
finding's own evidence, and is re-queried to confirm the cited id
resolves. A deferral whose only record is thread prose is a dropped
finding and the thread stays open. Today nothing requires the artifact
at all.
- **Never defer a finding this change introduced** — `VALID (defer)` is
for a defect already on the base branch, outside this change's files, or
owned by another contract. Provenance decides, never severity: a
self-introduced regression wearing a low-severity badge is still a
regression the change is shipping.
- **D7.5 becomes author- *and* classification-conditional** —
eligibility now turns on a recorded disposition (`VALID (fix now)` +
pushed fix, grounded `VALID (defer)`, or `INCORRECT` +
counter-evidence), not authorship alone. `UNCERTAIN` escalates and is
never resolved. Authorship conditions still apply in full on top.
- **Independent authorization** — a defer-resolution on a PR the same
session intends to merge routes through the pre-escalation dispatch, so
it is adjudicated by a context that is not trying to merge.

**Item 3 — ruleset vs. a per-commit reviewer: settled.** No ruleset
change. `required_review_thread_resolution` is a plain boolean, so the
head-/outdated-/author-scoping the issue asked for is not expressible;
and with `required_approving_review_count: 0` it is the only review gate
the org has, so dropping it is rejected. Forbidding `--auto` was already
shipped policy — only the reason was missing, and that sentence now sits
where the prohibition lives. The one live lever, turning off the
reviewer's automatic re-review and driving it through the already-built
but dormant review-trigger module, is reserved to the maintainer: the
toggle is external, repo-wide, and unverifiable from this repository.

### Placement

Both lane `SKILL.md` files sat at 499 against the 500 hard cap when this
work started, so the substantive text was routed to
`plugins/source-control/reference/review-discipline.md` (no cap)
regardless of #1620. #1627 has since landed and given `babysit-loop`
headroom, which is what makes the single pointer in its Escalation
section affordable — a lane raising a cap-policy question through that
contract previously had no reason to open `safety.md` first, which is
precisely how #1614 came to be filed against the rule that forbids it.
`babysit-loop/SKILL.md` goes 459 → 465 of 500.

## Test plan

All gates run locally against `origin/main`, all green:

- `scripts/check-changed-skills.sh` — 3 skills checked (`babysit-loop`,
`babysit-prs`, `pull-request`), 0 failed
- `scripts/check-changelog-parity.sh --check` and `--check-bump
origin/main`
- `scripts/validate-plugins.sh` — all plugin manifests and the catalog
- `scripts/check-skill-portability.sh origin/main` — no unexcused
coupling tokens
- `scripts/check-contract-slice-prune.sh --check` and `--check-diff
origin/main`
- `scripts/check-cross-plugin-source-drift.sh --check`,
`scripts/check-plugin-manifest-presence.sh`,
`scripts/check-orphaned-fixtures.sh --check`,
`scripts/check-skill-leaf-names.sh --check`
- `markdownlint-cli2` on all 7 changed markdown files — 0 issues
- `typos --config _typos.toml` — clean
- `babysit-prs` Python suite — 442 tests, OK

Behavioral verification is this PR itself: its own review threads are
worked under the policy it ships (D4.6 grounded deferrals, the (c)
provenance rule, D7.5 classification-conditional resolution).

## Related

- Closes #1614
- Decision comment:
#1614 (comment)
- #1615, #1594, #1613 — the runs the decision is grounded in
- #1627 — the headroom that makes the `babysit-loop` pointer affordable

---

## Resume state (session ended for audit — a cold agent can pick this up
here)

**Status: complete and green, held for human thread resolution. Not
blocked by missing work.**

Head `b87c2747`. All CI green. Six review findings across four Codex
rounds, every one fixed and verified. **Four threads deliberately
unresolved.**

### Why the lane did not merge

Not a failure to finish — two rules this PR itself ships forbid it:

1. **Three open threads carry P1 badges.** The guarded resolver returns
`"action": "skipped-severity-marked"` under `--autonomous` ("unattended:
never a security or P1 thread"). A flag combination would technically
lift it (`--resolve --include-human` without `--autonomous`); the lane
did not use it, because that widening is sanctioned for human
participants in an addressed thread, not for shedding the severity
guard, and `safety.md` says a blocked action "is escalated, never routed
around".
2. **This session authored every fix and would be the merging context.**
The authorization rule shipped here says a resolution that unblocks a
merge must be adjudicated by a context that is not trying to merge.
Self-clearing would make this PR the first violation of its own policy,
in the commit that introduces it.

### To finish (human, ~5 minutes)

1. Verify the fixes: `d782f3ff`, `f685eb57`, `8649f7b4`, `5057ba09`,
`b87c2747`.
2. Resolve the four open threads.
3. **Squash-merge synchronously.** `--auto` is prohibited here, and this
PR ships the sentence explaining why.

### Nothing else is outstanding

No TODOs, no partial edits, no unpushed commits, working tree clean.
Every gate was run locally before each push: `check-changed-skills.sh`
(3 skills), `check-changelog-parity.sh --check-bump`,
`validate-plugins.sh`, `check-skill-portability.sh`,
`check-contract-slice-prune.sh`, `check-cross-plugin-source-drift.sh`,
markdownlint, typos, and the 442-test `babysit-prs` Python suite.

### Deliberately out of scope, filed separately

- **#1641** — worker tier has no route to retire a disproved *current*
bot thread. Predates this PR; closing it must not weaken the
`--autonomous` `isOutdated` guard.
- **Independent-resolver mode for `babysit_resolve_thread.py`** —
required by the routing rule `b87c2747` adds: a current, addressed bot
thread now goes to the independent resolution dispatch, which needs a
wrapper mode to resolve through. Being filed by the `pr-1594` lane; link
it here when it lands. Deliberately not scoped into this docs PR.
- **Codex "Automatic reviews" toggle** — maintainer-only, external
console. This PR is direct evidence for turning it off: four automatic
review rounds on a documentation PR, each triggered by the previous
round's own fix.

### One correction to the record

An earlier status comment on this PR claimed CI was green before the
checks had finished. They then failed on `editorconfig`,
`plugin-schema`, and `workflow-schema` with `curl: (35) Connection
reset` and `fetch failed` — transient network faults fetching the
checker binary and remote JSON schemas, on a commit that added two lines
to one markdown file and touched no manifest. Re-running the failed jobs
cleared all three; no code change was made or warranted. The claim was
premature rather than wrong, and it is superseded by the later status
comment.

---

# PARKED — head `d8690555`, awaiting human/dispatched-lane thread
resolution

*This section supersedes every earlier resume/status note in this body
and in the comments.*

The autonomous lane is released. All work is pushed, CI is green, the
working tree is clean. **Nothing is stranded on local disk.** What
remains cannot be done by this session's shape — see "Why parked".

## Per-thread state

| # | Finding | Sev | Fix | Independent verdict | Action needed |
|---|---|---|---|---|---|
| T1 | Resolve all newly addressed bot threads (`monitor.md`) | P2 |
`d782f3ff` | — | ✅ resolved |
| T2 | Make deferred-resolution dispatch reachable | P2 | `d782f3ff` | —
| ✅ resolved |
| T3 | Require dispositions for **every** finding before resolving | P1
| `f685eb57` | **VERIFIED complete**
([reply](#1633 (comment)))
| **resolve** |
| T4 | Provenance across file boundaries | P1 | `f685eb57` | **VERIFIED
complete**
([reply](#1633 (comment)))
| **resolve** |
| T5 | Current `INCORRECT` unresolvable in worker mode | P2 | `f685eb57`
| — | ✅ resolved |
| T6 | Apply D4.6 before making deferrals resolvable | P1 | `8649f7b4` |
**VERIFIED complete**
([reply](#1633 (comment)))
| **resolve** |
| T7 | Gate full-mode deferral resolution independently | P2 |
`5057ba09` + **`d8690555`** | INCOMPLETE at `5057ba09`
([reply](#1633 (comment)))
— the `:272` gap it named is fixed in `d8690555` | **re-verify at head,
then resolve** |
| T8 | Fall back when no independent resolver exists | P2 | `09681aa1` |
— | **resolve** |
| T9 | Run D4.6 on subagent-returned deferrals | P2 | **none — open** |
— | **fix or defer (see below)** |
| T10 | Persist the self-inflicted round classification | P2 | **none —
filed as #1660** | — | **decide: fix here or ship on #1660** |

## What the operator needs to do

1. **Re-verify T7 at head `d8690555`.** The independent resolver marked
it INCOMPLETE at `5057ba09` because `pull-request/SKILL.md:272` still
restated the full eligibility enumeration (including grounded deferral)
with no carve-out, on the merge-capable path — and because this lane's
"no remaining gaps" claim was false. `d8690555` cuts that enumeration so
the gotcha **points** at D7.5 instead of restating it (pointer-not-copy,
per the resolver's recommended shape), and the corrected matrix reports
zero gaps.
2. **Resolve T3, T4, T6, T7, T8.** All fixed and, where reviewed,
independently verified.
3. **Decide T9 and T10** — both are open round-6 findings, both valid,
both class (c). Neither is fixed.
4. **Squash-merge synchronously.** `--auto` is prohibited here, and this
PR ships the sentence explaining why.

## Why parked rather than merged

Three reasons, each independently sufficient:

- **Severity guard.** Three threads carry P1 badges;
`babysit_resolve_thread.py::classify` returns `skipped-severity-marked`
for any severity-marked thread under `--autonomous` — *"unattended:
never a security or P1 thread."* A flag combination would technically
lift it; this lane did not use it, because that widening is sanctioned
for human participants in an addressed thread, not for shedding the
severity guard, and `safety.md` says a blocked action is escalated,
never routed around.
- **Self-adjudication.** This session authored every fix and would be
the merging context — exactly what the authorization rule shipping in
this PR forbids.
- **Reachability.** Thread resolution here is reachable only through the
babysit-loop typed-argument dispatch contract, which this session's
shape cannot synthesize.

## Open findings not fixed here, deliberately

- **T9** — D4.6 is absent from the subagent-return contract
(`review-discipline.md` §2, lines 134–138), so a ≥3-finding comment can
post a D5 deferral without the provenance check or the tracker item.
This is the round-3 defect on a **sixth** surface. Worth knowing: **the
corrected coverage matrix did not catch it** — the matrix keys on the
*eligibility* clause and never asks whether a surface enumerating the
**D-sequence** includes D4.6. That widens the acceptance criteria on
#1659.
- **T10 → #1660** — the (c) tripwire this PR ships is
**unreconstructable after context rollover**. The test needs the
previous round's classification; `record_advisory_round` persists only
`{"recorded_at": ...}`. A fresh worker cannot evaluate it, which is the
normal case for the loop this rule is written for. Filed separately
because it needs a ledger-schema change, not a docs edit.

## Filed rather than folded in

- **#1659** — mechanize the surface/clause coverage matrix as a CI gate.
The adopted recommendation; carries all rounds as evidence.
- **#1660** — the (c) tripwire reconstructability defect above.
- **#1641** — worker tier has no route to retire a disproved *current*
bot thread; orchestrator-side retirement endorsed as leading candidate,
final call open for operator triage.

## Commits

`d782f3ff` · `f685eb57` · `8649f7b4` · `5057ba09` · `b87c2747` ·
`09681aa1` · `d8690555`

## What this PR demonstrated

Seven review rounds, ten findings, **every one class (c)** — a defect in
text this PR itself authored. The tripwire fired at round 2 on the PR
introducing it, and the escalating responses are the actual result: a
clause patch failed; a deliberate whole-section rewrite failed (round 3
found the surface it missed); a hand-run mechanical sweep converged but
then produced a **false** all-clear (round 7's resolver caught `:272`);
and the corrected sweep still missed T9, because it checked one clause
family and not step-sequence completeness.

The invariant across all of it: **a contract distributed over eight-plus
surfaces regenerates this defect class on every edit, and no amount of
care substitutes for a mechanical check that covers every clause.** That
is the argument for #1659, and it is worth more than the policy text
this PR set out to write.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(source-control): babysit-loop/SKILL.md sits 1 line under the 500-line hard cap, blocking any net-positive edit

1 participant