feat(ai-engineer): split the sleep counter into foreground blocks, deferred watchers and unattributed Codex#2282
Conversation
…ferred watchers and unattributed Codex The raw sleep total could not answer the question it was used to answer. It scored a contract-COMPLIANT backgrounded watcher identically to a foreground busy-wait, and the improver's own runs arm several watchers per tick — so the metric polluted itself with the noise of the component reading it. Classification is structural (`run_in_background` on the tool call), so prose cannot fake it; the key is omitted when false, so absence reads as foreground. Codex is a STATED GAP, not a zero: 767 live exec_command calls carried `yield_time_ms` (an output-read timeout) and zero carried any background flag, so that runtime exposes no surface to classify. Its sleeps are counted but not attributed. One jq expression with a class filter rather than a second extractor — a parallel copy is how detector parity broke six times on the redactor — plus a sum check that warns when the classes stop summing to the total. This SHARPENS the measurement and removes nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codex review for measurement correctness: whether the class filter and the total can drift apart, whether the |
![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: f75ed95fad
ℹ️ 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".
…ally, exclude never-ran calls Three review findings, all valid: 1. The labels claimed a compliance verdict the data does not support. run_in_background is LAUNCH MODE, not intent: the contract permits a foreground bare sleep as a local timer for a process the agent started, and a backgrounded sleep can still be a redundant poll. Labels are now neutral and the note says plainly that a foreground count is a candidate, not a violation, and a background count is not an exoneration. 2. Four independent scans of a LIVE corpus (the sibling writes transcripts while we read) could observe four different states and fire the drift warning spuriously. Each file is now snapshotted once and every class comes from that copy; the total is the SUM of the classes, so drift is impossible by construction rather than merely detected. The drift warning is dropped with it — over a derived sum it would be a vacuous guard. 3. A hook-blocked or permission-denied call never ran, so counting it as a foreground block reports waiting that never happened. Note on (3): keying the exclusion on is_error was too broad and the suite caught it. A TIMED-OUT sleep also carries is_error:true, but it ran — and it is the most expensive block in the corpus, so suppressing it would have hidden the worst case while claiming to measure it. The exclusion matches the two never-ran shapes explicitly, with a guard pinning the timeout case. 111/111 tests; the two new guards RED-proven by ablation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All three findings fixed in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5edf384647
ℹ️ 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".
…regex, halve the parse passes Four review findings, all valid: P1 — the snapshot directory holds an UNREDACTED transcript copy and extracted command text, either of which can carry inline credentials, and it was not in the signal traps. A scheduler SIGTERM could leave that material in /tmp. It is now registered with both traps BEFORE it is populated, and deregistered when removed. The never-ran shapes were a SECOND hand-maintained copy of a list the safety detector already had, so a permission-DENIED sleep still counted as a foreground launch while a hook-blocked one did not. Both now read one NEVER_RAN_RE constant — two copies of a shape list is exactly how detector parity broke six times on the redactor. Denial ids are resolved once per file instead of once per class, cutting six full jq passes per transcript to two. The report note claimed every errored result was excluded, which contradicted both the code and the timeout regression test. Narrowed to the never-ran cases. On the signal-cleanup test: an end-to-end kill test was attempted twice and was VACUOUS both times (it passed with SNAP deliberately removed from the traps). Replaced with a deterministic structural assertion, with the residual gap stated in the test rather than hidden behind a green tick. 114/114; the new guards RED-proven by ablation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All four findings fixed in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2b612f662
ℹ️ 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".
…e tagged pass Round 3 raised four findings and three clustered on the snapshot added in round 2 — which is the signal that the snapshot was the defect, not something to harden. Removing it closes all three at once and leaves the script smaller. The P1 was unfixable in place: `main` is the left side of `main | redact`, so it runs in a pipeline subshell and a trap set there never fires when the scheduler signals the top-level PID. The snapshot existed only to keep the class counts consistent — a single tagged pass gives that for free, writes no unredacted copy to disk, and needs no cleanup at all. Four transcript traversals become one. Each line of a command carries a control-character class tag, so multi-line commands keep their structure and untrusted command text cannot forge a tag and move itself between classes (guarded). Also: the never-ran match is now anchored to the harness denial envelope, so an executed command that fails while printing "approval denied for tool" in its own output is no longer treated as never-run and silently removed from every class; and the skill's guidance no longer calls a background launch "compliant", which contradicted the report's own warning and would have produced exactly the false verdict that warning exists to prevent. Classification now lives in one function, so there is no second classifier to drift from the first. 115/115; live runtime 21.5s -> 17.8s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All four fixed in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af7531e4ae
ℹ️ 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".
…enial envelope
A tool_result carries `content` either as a plain string or as the
array-of-text-blocks shape. Anchoring the never-ran match (previous commit)
without normalising meant `tostring` produced `[{"type":"text"...` for the array
form, so the pattern never reached the denial text and a call that never ran was
counted as an executed foreground launch.
Normalised the same way the safety detector already does. Every denial fixture
in the suite used string content, which is why anchoring looked correct — the
array shape is now covered explicitly, RED-proven by ablation.
115/115.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixed in @codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Readiness — self-promoting.
|
1 participant
Why
We have been trying to tell whether the agents busy-wait, and the number we were using could not answer that. It counted a legitimate background watcher exactly the same as a genuine block — and the improver's own runs create several watchers every tick, so the measurement was polluted by the thing reading it. Yesterday a falling total was read as progress when the per-session rate had actually gone up.
What
Splits the sleep count into three: real foreground blocks, compliant background watchers, and Codex's sleeps, which we honestly cannot attribute because that runtime exposes no background flag at all. Rates are now reported per session with the denominator stated.
Running it on live data immediately changed the picture: of 340 sleeps, only 101 are actual foreground blocks — and Codex, previously assumed near-zero, accounts for 195 of them, the largest share. The prior conclusion that this was a Claude-only problem does not survive the sharper measurement.
This only sharpens what we measure — nothing was narrowed or removed.
Fixes #2283