Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,20 @@

## Pull requests

- Every PR: CI green first, then wait for the automated reviewer
(`chatgpt-codex-connector`) to finish. It does not re-review on its own
after a rebase or force-push: after every push, post an `@codex review` PR
comment if no fresh review appears within a few minutes, and confirm its
summary comment cites the current head SHA before merging.
- Reviewer quota fallback: if the connector answers "usage limits reached", or
no review arrives within about ten minutes of two `@codex review` requests,
record the last-reviewed and unreviewed head SHAs in the PR body's "Review
status" section and merge on green CI. Re-request the review once credits
return; a thread it opens on an already-merged PR is answered in a
follow-up PR like any other.
- Address every review thread — fix it in the same PR or reply with a precise
reason — and reply on every thread. After each push, re-check for new
threads and repeat until there are none. Only then merge.
- Every PR gets a self-review before merge: spawn a local reviewer subagent
(`change-risk-reviewer` if available, else `generalPurpose`; prefer a
different model from the author's, e.g. `gpt-5.6-sol-high`) with the repo
path and the PR number or branch, asking for concrete merge risks only —
bugs, breaking changes, missed tests, doc or changeset gaps — against the
current diff vs `origin/main`. Fix or explicitly dismiss every finding in
the PR description under a "Self-review" section (reviewer model, findings,
disposition), run the reviewer once more after fixes, then merge on green
CI.
- Never solicit external reviews: no `@codex review` comments, no waiting for
the connector, no unreviewed-SHA bookkeeping. Review comments that arrive
on their own (Codex or human) are still addressed — fix in the same PR or
reply with a precise reason — and re-checked after each push until none
remain.
- PRs are squash-merged. Review threads left on an already-merged PR must
still be answered, in a follow-up PR.

Expand Down
14 changes: 11 additions & 3 deletions packages/agent-bundle/tests/mcp-probe-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,13 +709,18 @@ it('settle() fences in-flight probes, not only already-registered teardowns (#39
const connectReleased = new Promise<void>((resolvePromise) => {
releaseConnect = resolvePromise;
});
let connectReached!: () => void;
const connectStarted = new Promise<void>((resolvePromise) => {
connectReached = resolvePromise;
});
try {
const service = serviceFor(root, {
createClient: () => client({
connect: async () => {
// A probe that is still connecting when shutdown begins: its
// teardown is not registered yet, so a fence over teardowns alone
// would resolve immediately.
connectReached();
await connectReleased;
},
}),
Expand All @@ -732,13 +737,16 @@ it('settle() fences in-flight probes, not only already-registered teardowns (#39

const probe = service.probe({ host: 'claude', serverName: 'timeline' });
void probe.then(() => { events.push('report-returned'); });
// Let the probe reach its (blocked) connect before shutdown starts.
await new Promise((resolvePromise) => setTimeout(resolvePromise, 20));
// Wait for the probe to reach its (blocked) connect before shutdown
// starts: plugin data exists by then, and the teardown is not registered.
await connectStarted;
await expect(readFile(join(pluginData!, 'proof.txt'), 'utf8')).resolves.toBe('present');

let settled = false;
const settle = service.settle().then(() => { settled = true; });
await new Promise((resolvePromise) => setTimeout(resolvePromise, 50));
// A fence over registered teardowns alone would resolve within the
// current macrotask; draining one is enough to observe that regression.
await new Promise((resolvePromise) => setImmediate(resolvePromise));
expect(settled).toBe(false);

releaseConnect();
Expand Down
Loading