Skip to content

fix(cli): the codex runtime's review-trigger loop reads the agent's own feed - #642

Open
tps-anvil wants to merge 1 commit into
mainfrom
fix/631-review-trigger-own-feed
Open

tps-anvil wants to merge 1 commit into
mainfrom
fix/631-review-trigger-own-feed

Conversation

@tps-anvil

Copy link
Copy Markdown
Collaborator

Closes #631.

The codex runtime's review-trigger loop read the catch-up feed of <agentId>-review-trigger while signing as <agentId>. Flair refuses a non-admin read of another participant's feed with 403, so the loop reported the feed unavailable (since #629) and, before that, handled nothing. It now reads the agent's own feed (agentId), where Flair serves an event addressed to that agent.

What changes

  • pr-review-trigger.ts gains startReviewTriggerLoop, which starts the pr.opened loop on the agent's own feed; codex-runtime.ts calls it instead of starting the loop against the <agentId>-review-trigger feed.
  • flair-task-loop.ts takes an optional loopId naming the cursor file, defaulting to agentId. The review-trigger loop's cursor stays at <agentId>-review-trigger-task-loop.json (the file it used before), so the two loops reading one agent's feed keep separate positions.

The pr.opened publisher (the runtime's autoCommit path) emits the event with no targetIds, i.e. a broadcast, and the agent's own feed serves broadcast events, so the loop receives it. No publisher change was made; if the trigger must be delivered by targeting, that publisher is the one that would set targetIds.

Evidence

Measured on 9b038ca (main 5f19a02).

  • New review-trigger-loop.test.ts (2 cases): against the shared signed catch-up stub, the loop reads only the agent's own feed (every catch-up request names agent-a; a foreign feed is refused with 403), a pr.opened event addressed to the agent is handled (the loop's cursor advances to the event's position), and a refused read is reported unavailable (Task loop catch-up read unavailable (HTTP 403)) rather than read as empty.
  • Mutation: with startReviewTriggerLoop reading <agentId>-review-trigger again, the "reads its own feed / handles a targeted trigger" case fails at the cursor assertion; restored, both cases pass.
  • Touched tests (10 files, 135 cases): review-trigger-loop, flair-task-loop, pr-review-trigger, home-per-call, office-status, agent-logs, codex-presence-444, auto-commit, mail-trust-ceiling, discord-adapter — 135 pass / 0 fail.
  • Full cli suite (HOME-isolated launcher, run from packages/cli): head 3333 pass / 7 skip / 0 fail (3340 tests, 213 files), main 3331 pass / 7 skip / 0 fail (3338 tests, 212 files); the +2 tests are the new file's cases.
  • bun run build (tsc) clean; bun run lint:ci clean (469 warnings, 84 infos, unchanged); node scripts/changelog-fragments.mjs check clean.

Closes #631.

The codex runtime's review-trigger loop read the catch-up feed of
`<agentId>-review-trigger` while signing as `<agentId>`, which Flair refuses
with 403, so it never handled its pr.opened events. It now reads the agent's
own feed; its cursor stays under `<agentId>-review-trigger-task-loop.json`,
separate from the task loop's, so the two loops reading one agent's feed keep
separate positions.
@tps-anvil
tps-anvil requested a review from a team as a code owner October 14, 2026 00:56
@tps-anvil

Copy link
Copy Markdown
Collaborator Author

cli#631 sweep — added/changed sentences vs the code

Scope: every sentence git diff origin/main...HEAD adds or changes in prose,
comments, test titles, the changelog fragment and the PR body. Three claim types:
(a) outcomes/status codes per branch; (b) scope words (only/never/every/all/none/always);
(c) coverage claims (does the named test assert it, live or unit).

flair-task-loop.ts — docstring / comments

  • "The loop reads agentId's catch-up feed through FlairClient.getEventsSince…" (a) TRUE — poll() calls flair.getEventsSince(agentId, …); getEventsSince is the validated reader. (b) no scope word.
  • "…always sending its position as Flair's after cursor, so Flair's own watermark for the agent never chooses where reading starts." (a) unchanged from main (pre-existing); (b) "always"/"never" are the The task loop and FlairClient.getEventsSince read Flair's catch-up envelope #624 guarantee, unchanged here. KEEP.
  • "The position is persisted in ~/.tps/cursors/<loopId>-task-loop.json as { position, updatedAt }…" (a) TRUE — taskLoopCursorPath(loopId) = <loopId>-task-loop.json; written by saveCursor. Only the <agentId>→<loopId> token changed.
  • NEW "loopId names the cursor and is agentId unless the caller names another, so two loops reading one agent's feed keep separate positions." (a) TRUE — const loopId = opts.loopId ?? agentId; loadCursor/saveCursor/taskLoopCursorPath take loopId. (b) "two loops…keep separate positions" holds because distinct loopId → distinct file. KEEP.
  • "The loop's cursor file for loopId." (a) TRUE (param rename only).
  • "The cursor is named for the loop, not the feed: two loops reading one agent's feed (agentId) keep separate positions." (a)(b) same as above. KEEP.

pr-review-trigger.ts — new docstring

  • "Start the review-trigger loop: poll the agent's OWN catch-up feed for pr.opened events and request reviews." (a) TRUE — startTaskLoop(flair, agentId, …, {kinds:["pr.opened"]}); handler is handlePrOpened → requestReviews. (b) "OWN" is the point of the fix. KEEP.
  • "The feed read is agentId's, so Flair serves it to the agent that signs the read (a read of another participant's feed is refused)." (a) TRUE — FlairClient signs as its agentId; Flair's ownership rule (and orgEventCatchupRoute) refuse caller≠participant for non-admins. KEEP.
  • "Its cursor under ~/.tps/cursors/<agentId>-review-trigger-task-loop.json is separate from the task loop's, so the two loops reading one agent's feed keep separate positions." (a) TRUE — loopId: \${agentId}-review-trigger`; task loop uses agentId`. KEEP.

codex-runtime.ts — comment

  • "Review trigger — listens for pr.opened on the agent's own feed and auto-requests K&S review (cli#631: reading another participant's feed was refused by Flair, so the loop never received its events)." (a) TRUE — the call is now startReviewTriggerLoop(flair, agentId, …), reading agentId's feed. (b) "never received its events" TRUE on main (403 → unavailable → handles nothing). KEEP.

test/review-trigger-loop.test.ts — header / titles / comments

  • Header "…reads only feeds the agent may read: it reads the agent's OWN catch-up feed (agentId)… Before this change the loop read the feed of <agentId>-review-trigger while signing as agentId, which Flair refuses with 403, so it never received its events." (a) TRUE — asserts below. (b) "only" — the loop reads one feed, agentId's. KEEP.
  • "The catch-up route is orgEventCatchupRoute… it refuses a non-admin read of another participant's feed with 403, so a loop that reads a foreign feed gets nothing." (a) TRUE — stub caller !== participant → 403. KEEP. (c) the mutation shows exactly this.
  • describe "the review-trigger loop reads only feeds the agent may read" (b) "only" — one feed. KEEP (matches the issue).
  • test 1 title "it reads the agent's own feed, so a pr.opened trigger addressed to the agent is handled" (c) asserted: new Set(asked) == {agent-a}; route.served == ["e1"]; cursor advanced to the event's position. KEEP.
  • comment "The cursor advances only after the handler succeeds; a refused read advances nothing." (a) TRUE — The task loop and FlairClient.getEventsSince read Flair's catch-up envelope #624 walk: seen.add(event.id) after the handler; a failed handler breaks; an unavailable read persists nothing. KEEP.
  • comment "Every request named the agent's own feed. A foreign feed (…-review-trigger) would 403 and deliver nothing." (b) "Every" — asserted asked.length > 0 and set == {agent-a}. KEEP.
  • test 2 title "a refused read is reported unavailable, not treated as an empty feed" (c) asserted: warn "catch-up read unavailable" + "(HTTP 403)". KEEP.
  • comment "The initial position was saved; the refused read advanced the cursor no further." (c) asserted: cursorPosition() === the first after. KEEP.

.changelog/unreleased/fixed-631-review-trigger-own-feed.md

  • "…reads the agent's own catch-up feed (Closes codex runtime review-trigger loop reads only feeds the agent may read #631)." (a) TRUE.
  • "It read a feed named for the trigger (<agentId>-review-trigger) while signing as the agent, which Flair refuses with 403, so it handled no pr.opened events." (a) TRUE on origin/main (the pre-fix code, the last release); 403 → the loop handled nothing. KEEP.
  • "It now reads the agent's own feed, where Flair serves an event addressed to that agent, and keeps its cursor under <agentId>-review-trigger-task-loop.json, separate from the task loop's, so the two loops reading one feed track their positions independently." (a) TRUE. KEEP.

Guarantee-word grep

git diff -U0 origin/main...HEAD -- '*.md' .changelog docs | grep '^+' | grep -inwE 'only|never|every|always|exactly|guarantee|cannot|all|none|any'
→ hits are the pre-existing #624 guarantee in flair-task-loop.ts (unchanged lines are not +), plus the new "only/never/every" above, each verified KEEP. Same grep over added comment lines and test titles: all KEEP.

Not changed / findings

  • The pr.opened publisher (codex-runtime.ts autoCommit path) emits the event with no targetIds (a broadcast). Flair serves broadcast events in the agent's own feed, so the loop receives it; no publisher change was made. If addressed (targeted) delivery is later required, that publisher is the one that would set targetIds.
  • startTaskLoop still acts on a matching kind when the event is a broadcast (targets.length === 0) or targets agentId. That filter is The task loop and FlairClient.getEventsSince read Flair's catch-up envelope #624's and was not changed here.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 571df2ab-b4da-4970-ac29-742796b311b8
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

codex runtime review-trigger loop reads only feeds the agent may read

2 participants