fix(skills): honor the standing babysit opt-out and cut the watch's cost - #1609
Merged
Conversation
…s it `auto_babysit: false` was named as the standing opt-out in the babysit handoff but never read there. The only config read lived in the Step 4 reference, scoped to the teaching-gate keys, so the Step 5 gate depended on remembering an earlier step. Compaction destroys exactly that, which is why the reporter saw Codex babysit PRs they had opted out of. The resolution now happens at the gate, and an opted-out handoff is stated as a successful terminal — without that, the gate's own "stop and report it blocked" language turned a deliberate choice into a failure. A per-run `babysit:` token now decides the run whatever the config says, which also covers `continuous`/`checkpoint` under a standing opt-out. `lfg`'s in-pipeline babysit is bounded and stays out of scope; that is now stated at the step and in the configuration reference rather than reading as an oversight. The contract test asserted the literal string `auto_babysit: false`, which prose that never read anything satisfied. It now pins the read mechanism and the successful-terminal framing. Fixes #1601
Three sources of waste in a watch that can run for hours. Every non-empty top-level PR body is a feedback candidate, and the resolver decides — so a coverage bot's comment woke the agent and bought a full `ce-resolve-pr-feedback` dispatch to conclude "status noise", and split the ordinary push -> bots comment -> CI finishes sequence into two agent rounds. A comment-only wake now waits for the candidate set to stop changing, so a burst costs one dispatch and real work claims the tick and handles the comments with it. No candidate leaves the attention set; the deterministic layer still judges nothing, which is what #1467 fixed. A missing or unparseable clock wakes rather than holding, so a candidate the resolver has never seen cannot be stranded for the run. The per-tick snapshot the agent reads was pretty-printed: 6,473 bytes against 4,453 compact on a 6-thread / 14-check / 5-comment PR, so 31% of every tick was whitespace nothing reads. The persisted state file stays indented — it costs no agent tokens and a human debugging a stuck watch reads it. The 45KB watch-loop reference ended with an "Edge cases summary" that duplicated the section above it and pointed at the file it was already in. Removed, and the two mid-tick pointers now re-read only if the session has not already loaded it.
Captures the #1601 root cause as a reusable rule, plus the eval lesson that produced the evidence: the standard full-skill cell passed in both arms on both hosts and did not reproduce the bug at all, because a model handed the whole skill finds the key mentioned anywhere and goes reading. Only a fixture that withheld the rest of the skill discriminated. To eval a rule whose failure mode is context loss, the fixture must reproduce the context loss. Adds Attention set and Feedback candidate to CONCEPTS.md — the candidate vs. actionable distinction is what the deterministic/judgment split in the watch loop is built on.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea9dad4a0
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A standing
auto_babysit: falsenow actually stops the babysit handoff, and a watch that does run spends far fewer agent tokens. The setting already existed and was documented — it was simply never read at the gate that consumes it.The reported symptom was Codex babysitting PRs the user had opted out of, "because of drifts and compactions." That is precisely the mechanism: the handoff block named
auto_babysitas the opt-out, but the only config read lived in the Step 4 reference, scoped to the teaching-gate keys. The Step 5 gate depended on remembering an earlier step, and compaction destroys exactly that.Why not default babysit off
The issue proposes making babysit opt-in. This keeps it default-on and fixes the opt-out instead, because the reporter's own configuration shows the intended path was already the one they took — it just did nothing. Flipping the default would trade a broken setting for a worse one for everyone who wants the watch.
lfg's in-pipeline babysit is deliberately still exempt: it is bounded at 3 fix rounds and is how that pipeline reaches its "CI decided" result, so a standing opt-out from the open-ended watch should not silently truncate it. That scope is now stated at the step and in the configuration reference rather than reading as an oversight.One shape change worth reviewing: an opted-out handoff is now a successful terminal, not a blocked one. Without that, the gate's own "stop and report it blocked" language swallowed the opt-out — pre-fix Claude reported the PR "currently unmonitored," turning a deliberate choice into a failure state.
What was burning tokens
The watcher itself was already free — it is a Python poll that spends no agent tokens and only wakes on real change. The waste was around it.
Every non-empty top-level PR body is a feedback candidate the resolver must classify, by design. So a coverage bot's comment woke the agent and bought a full
ce-resolve-pr-feedbackdispatch to conclude "status noise", and split the ordinary push → bots comment → CI finishes sequence into two agent rounds. A comment-only wake now waits for the candidate set to settle, so a burst costs one dispatch and real work claims the tick and handles the comments with it. Nothing is dropped, only deferred — and an unknown clock wakes rather than holds, so a comment the resolver has never seen cannot be stranded.The per-tick snapshot the agent reads was pretty-printed, measured on a 6-thread / 14-check / 5-comment PR:
The persisted state file stays indented — it costs no agent tokens and a human debugging a stuck watch reads it.
The 45KB watch-loop reference also ended with an "Edge cases summary" that duplicated the section above it and pointed at the file it was already in; the two mid-tick pointers now re-read only if the session has not already loaded it.
How this was verified
The standard full-skill eval cell passed in both arms on both hosts — it did not reproduce the bug at all. A model handed the whole skill finds the key mentioned anywhere and goes reading, which masks the defect entirely.
So the eval withholds context instead: hand the agent only the Step 5 reference, as a compacted run would see it, against a repo with an active
auto_babysit: false.auto_babysit: falseis the standing repo config."auto_babysit: false"Full suite 3674 pass / 0 fail;
release:validateandplugin:validateclean.The contract test previously asserted the literal string
auto_babysit: falseappeared in the reference — which prose that never read anything satisfied. It now pins the read mechanism and the successful-terminal framing.New concepts
Withholding context to eval a context-loss defect
The usual way to eval a skill change is to inject the whole skill into a fresh agent and grade what it does. That silently cannot test a class of defect: one whose failure mode is missing context. A capable model given the full skill will compensate for prose that never tells it to do something, because it can see the surrounding text that implies it — so the buggy version passes.
The fixture here inverts that. It hands the agent only the one reference that owns the decision, which is a cheap stand-in for what survives a compaction, and grades the same declared decision pre- and post-change. That is what separated the arms when the full-skill cell could not.
When not to use it: only when the rule under test is supposed to fire from a reference the agent loads at a step. A rule that lives in the always-loaded body is not testable this way, because withholding the body tests a state that never happens.
Fixes #1601
Security Disclosure
Changes to the bundled
pr-snapshotscript are limited to JSON output encoding, wake-timing logic, and one new numeric CLI flag — no change to path handling, input parsing, subprocess execution, credentials, or permissions, and no dependency changes. Two test fixtures add.compound-engineering/config.yamlfiles undertests/skill-eval-cell/fixtures/; they contain no secrets and are not on the config-resolution path, which reads only the repo root. Residual risk: the new coalescing window defers a comment-only wake, so a human comment on an otherwise-idle PR is picked up on a later tick rather than the first — bounded, and it fails toward waking whenever the timing state is unknown.Agent Disclosure
Claude Code · claude-opus-5[1m]