test(ce-babysit-pr): state and pin the review-gate acquisition rules - #1632
test(ce-babysit-pr): state and pin the review-gate acquisition rules#1632khsaurabh wants to merge 3 commits into
Conversation
Issue EveryInc#1618: the settle gate's four-surface lookup (reactions, top-level comments, check runs, reviews) is agent-prose; the eval cells cover the reasoning from prepared data but nothing pinned the acquisition rules. settle.md now states them explicitly - identity correlation across REST/GraphQL bot logins, head attribution for PR-scoped vs commit-scoped surfaces, pagination, and probe failure as inconclusive - and a new bun contract suite pins all four. Option (b) from the issue; no live-fixture eval cell (determinism).
![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: df9e98ac46
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| **Acquisition rules for that lookup (mechanical, not judgment):** | ||
|
|
||
| - **Identity correlation.** The same reviewer appears with different logins across APIs - REST returns `cursor[bot]` / `chatgpt-codex-connector[bot]`, GraphQL returns `cursor` / `chatgpt-codex-connector`. Normalize before comparing: strip a trailing `[bot]` and lowercase. Never correlate a reviewer across surfaces on the raw login string. |
There was a problem hiding this comment.
When a GitHub App slug and a human login share the same base name, unconditional [bot] stripping collapses distinct actors into one key. A terminal check or review from one actor can then be mistaken for output satisfying the other's announcement, clearing the gate and potentially allowing stack-land to merge while the actual reviewer is still running. Preserve the actor kind during normalization and correlate only proven Bot/App identities; treat ambiguous cross-kind matches as inconclusive.
AGENTS.md reference: AGENTS.md:L136-L138
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 413165d. Normalization now keeps the actor kind attached to the normalized name: a [bot] identity correlates only across app/bot surfaces, a human login sharing the base name is a different actor, and a cross-kind match is inconclusive - never evidence the announced review finished.
| expect(SETTLE_BODY).toMatch(/strip a trailing `\[bot\]`/) | ||
| expect(SETTLE_BODY).toMatch(/[Nn]ever correlate a reviewer across surfaces on the raw login/) |
There was a problem hiding this comment.
This test pins suffix stripping but never asserts the second required operation, lowercasing, so deleting and lowercase from settle.md leaves the entire suite green. That permits case-varying API identities to stop correlating without the mechanical guard detecting the regression; add a direct assertion for the lowercase requirement as the smallest falsifiable unit.
AGENTS.md reference: AGENTS.md:L216-L219
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 413165d. The test now asserts strip a trailing \[bot]` and lowercase` as one pinned phrase, so deleting the lowercase half fails the suite.
|
The |
Codex review on EveryInc#1632: unconditional [bot] stripping collapses an app slug and a human login that share a base name into one key, so one actor's terminal output could clear the gate for the other's announced review. Pin lowercasing as its own assertion (deleting it left the suite green) and require cross-kind matches to be treated as inconclusive.
2 participants
Summary
Issue #1618:
ce-babysit-pr's settle gate rests on a four-surface lookup (PR-body reactions, top-level comments, check runs on the head, reviews) that the agent performs fromsettle.mdprose. The five eval cells cover the reasoning from prepared data; nothing pinned the acquisition rules, and two of them were not stated at all.Takes the issue's option (b): state the rules, pin them deterministically. No live-fixture eval cell (option a) - it trades determinism and needs a stable subject; easy to add later if you want realism coverage too.
settle.mdnow states, next to the review-still-coming gate:cursor[bot], GraphQL returnscursor; normalize (strip trailing[bot], lowercase) before comparing a reviewer across surfaces.Validation
tests/skills/ce-babysit-pr-settle-acquisition.test.ts- 4 contract tests pinning each rule insettle.md.bun test tests/skills/ce-babysit-pr-settle-acquisition.test.ts tests/ce-babysit-pr-snapshot.test.ts tests/ce-babysit-pr-contract.test.ts- 220 pass, 0 fail.Closes #1618
Security Disclosure
No security-relevant changes.
Agent Disclosure