From 9b038ca97490026f3502f65b0fdbacf51461df90 Mon Sep 17 00:00:00 2001
From: flint <flint@tps.dev>
Date: Wed, 14 Oct 2026 00:46:49 +0000
Subject: [PATCH 1/5] fix(cli): the review-trigger loop reads the agent's own
 catch-up feed

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.
---
 .../fixed-631-review-trigger-own-feed.md      |   1 +
 packages/cli/src/utils/codex-runtime.ts       |  15 +-
 packages/cli/src/utils/flair-task-loop.ts     |  49 +++---
 packages/cli/src/utils/pr-review-trigger.ts   |  26 +++
 packages/cli/test/review-trigger-loop.test.ts | 159 ++++++++++++++++++
 5 files changed, 218 insertions(+), 32 deletions(-)
 create mode 100644 .changelog/unreleased/fixed-631-review-trigger-own-feed.md
 create mode 100644 packages/cli/test/review-trigger-loop.test.ts

diff --git a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
new file mode 100644
index 00000000..6520eee0
--- /dev/null
+++ b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
@@ -0,0 +1 @@
+- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** 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. 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.
diff --git a/packages/cli/src/utils/codex-runtime.ts b/packages/cli/src/utils/codex-runtime.ts
index f956c234..fb7117d6 100644
--- a/packages/cli/src/utils/codex-runtime.ts
+++ b/packages/cli/src/utils/codex-runtime.ts
@@ -26,7 +26,7 @@ import {
 } from "./agent-lifecycle.js";
 import { FlairClient, type PresenceActivity } from "./flair-client.js";
 import { startTaskLoop } from "./flair-task-loop.js";
-import { handlePrOpened } from "./pr-review-trigger.js";
+import { startReviewTriggerLoop } from "./pr-review-trigger.js";
 import { completeRuntimeMail, pollRuntimeMail, type RuntimeMailConfig, runtimeBootPreflight, sendRuntimeMail } from "./runtime-mail.js";
 import { externalDispatchRefusal } from "./mail-tier.js";
 import type { Envelope } from "@tpsdev-ai/agent";
@@ -853,16 +853,11 @@ export async function runCodexRuntime(config: CodexRuntimeConfig): Promise<void>
     }
   });
 
-  // Review trigger — listens for pr.opened and auto-requests K&S review
+  // 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).
   if (config.reviewTrigger) {
-    const triggerConfig = {
-      reviewers: config.reviewTrigger.reviewers,
-      agentId,
-      repo: config.reviewTrigger.repo,
-    };
-    startTaskLoop(flair, `${agentId}-review-trigger`, async (event) => {
-      await handlePrOpened(event, triggerConfig);
-    }, { kinds: ["pr.opened"] });
+    startReviewTriggerLoop(flair, agentId, config.reviewTrigger);
     console.log(`[${agentId}] Review trigger active → ${config.reviewTrigger.reviewers.join(", ")}`);
   }
 
diff --git a/packages/cli/src/utils/flair-task-loop.ts b/packages/cli/src/utils/flair-task-loop.ts
index ab5b5bc5..ea4f230a 100644
--- a/packages/cli/src/utils/flair-task-loop.ts
+++ b/packages/cli/src/utils/flair-task-loop.ts
@@ -2,13 +2,15 @@
  * flair-task-loop.ts — Polls OrgEventCatchup for task.assigned events
  * targeting this agent. Runtime-agnostic: works without OpenClaw.
  *
- * CURSOR. The loop reads the catch-up feed through `FlairClient.getEventsSince`
- * (the validated reader in org-event-read.ts, cli#619), always sending its
- * position as Flair's `after` cursor, so Flair's own watermark for the agent
- * never chooses where reading starts. The position is persisted in
- * `~/.tps/cursors/<agentId>-task-loop.json` as `{ position, updatedAt }` and
- * replaced atomically (a temporary file in the same directory, fsynced, then
- * renamed over it). With no usable cursor file, the loop sets the position to
+ * CURSOR. The loop reads `agentId`'s catch-up feed through
+ * `FlairClient.getEventsSince` (the validated reader in org-event-read.ts,
+ * cli#619), always sending its position as Flair's `after` cursor, so Flair's
+ * own watermark for the agent never chooses where reading starts. The position
+ * is persisted in `~/.tps/cursors/<loopId>-task-loop.json` as
+ * `{ position, updatedAt }` and replaced atomically (a temporary file in the
+ * same directory, fsynced, then renamed over it). `loopId` names the cursor and
+ * is `agentId` unless the caller names another, so two loops reading one
+ * agent's feed keep separate positions. With no usable cursor file, the loop sets the position to
  * FIRST_BOOT_LOOKBACK_MS before now, once, and does not read the feed until
  * that position is saved: a failed save is logged by name and retried with
  * backoff, with the same position, and `stop()` ends the retrying. A legacy
@@ -95,9 +97,9 @@ function cursorDir(): string {
   return join(homeDir(), ".tps", "cursors");
 }
 
-/** The task loop's cursor file for `agentId`. */
-export function taskLoopCursorPath(agentId: string): string {
-  return join(cursorDir(), `${agentId}-task-loop.json`);
+/** The loop's cursor file for `loopId`. */
+export function taskLoopCursorPath(loopId: string): string {
+  return join(cursorDir(), `${loopId}-task-loop.json`);
 }
 
 /**
@@ -125,10 +127,10 @@ export function cursorFilePosition(data: unknown): string | undefined {
 }
 
 /** The persisted feed position (Flair's `after` cursor), or undefined when there is none to resume from. */
-function loadCursor(agentId: string): string | undefined {
-  const path = taskLoopCursorPath(agentId);
+function loadCursor(loopId: string): string | undefined {
+  const path = taskLoopCursorPath(loopId);
   if (!existsSync(path)) {
-    slog(`[${agentId}] No task loop cursor at ${path}; ${LOOKBACK_NOTE}`);
+    slog(`[${loopId}] No task loop cursor at ${path}; ${LOOKBACK_NOTE}`);
     return undefined;
   }
   let position: string | undefined;
@@ -139,7 +141,7 @@ function loadCursor(agentId: string): string | undefined {
     problem = e instanceof SyntaxError ? "it is not JSON" : `it cannot be read (${(e as NodeJS.ErrnoException).code ?? (e as Error).name})`;
   }
   if (position === undefined) {
-    swarn(`[${agentId}] Task loop cursor at ${path} is unusable (${problem}); treating it as no cursor, so ${LOOKBACK_NOTE}`);
+    swarn(`[${loopId}] Task loop cursor at ${path} is unusable (${problem}); treating it as no cursor, so ${LOOKBACK_NOTE}`);
   }
   return position;
 }
@@ -149,8 +151,8 @@ function loadCursor(agentId: string): string | undefined {
  * it, rename it over the cursor. Returns undefined on success, or the failure's
  * name (its code, such as EACCES, or its class) for the caller to log.
  */
-function saveCursor(agentId: string, position: string): string | undefined {
-  const path = taskLoopCursorPath(agentId);
+function saveCursor(loopId: string, position: string): string | undefined {
+  const path = taskLoopCursorPath(loopId);
   const tmp = `${path}.${process.pid}.${Date.now()}.tmp`;
   try {
     mkdirSync(cursorDir(), { recursive: true });
@@ -190,11 +192,14 @@ export function startTaskLoop(
   flair: FlairClient,
   agentId: string,
   handler: TaskHandler,
-  opts: { pollIntervalMs?: number; kinds?: string[] } = {},
+  opts: { pollIntervalMs?: number; kinds?: string[]; loopId?: string } = {},
 ): { stop: () => Promise<void> } {
   const basePollMs = opts.pollIntervalMs ?? DEFAULT_POLL_MS;
   const kinds = new Set(opts.kinds ?? ["task.assigned"]);
-  const loaded = loadCursor(agentId);
+  // The cursor is named for the loop, not the feed: two loops reading one
+  // agent's feed (`agentId`) keep separate positions.
+  const loopId = opts.loopId ?? agentId;
+  const loaded = loadCursor(loopId);
   // A new initial position is computed once, here; saving it is retried with
   // this same value until it succeeds, and the feed is not read before then.
   let position: string = loaded ?? initialPosition();
@@ -216,11 +221,11 @@ export function startTaskLoop(
   /** Save the initial position, retrying with backoff; false when stopped first. */
   async function saveInitialPosition(): Promise<boolean> {
     for (let attempt = 1; running; attempt++) {
-      const failure = saveCursor(agentId, position);
+      const failure = saveCursor(loopId, position);
       if (failure === undefined) return true;
       const delay = Math.min(basePollMs * 2 ** (attempt - 1), MAX_BACKOFF_MS);
       swarn(
-        `[${agentId}] Task loop cannot save its initial position to ${taskLoopCursorPath(agentId)} (${failure}); ` +
+        `[${loopId}] Task loop cannot save its initial position to ${taskLoopCursorPath(loopId)} (${failure}); ` +
           `the feed is not read until it is saved; retrying in ${delay} ms`,
       );
       await sleep(delay);
@@ -290,8 +295,8 @@ export function startTaskLoop(
             // A failed save is logged and the loop goes on from the position in
             // memory. The saved position is then behind it, so a restart replays
             // events rather than skipping them.
-            const failure = saveCursor(agentId, position);
-            if (failure !== undefined) swarn(`[${agentId}] Failed to persist cursor to ${taskLoopCursorPath(agentId)} (${failure})`);
+            const failure = saveCursor(loopId, position);
+            if (failure !== undefined) swarn(`[${loopId}] Failed to persist cursor to ${taskLoopCursorPath(loopId)} (${failure})`);
           }
 
           // Cap seen set
diff --git a/packages/cli/src/utils/pr-review-trigger.ts b/packages/cli/src/utils/pr-review-trigger.ts
index 54012a27..2b4d13b7 100644
--- a/packages/cli/src/utils/pr-review-trigger.ts
+++ b/packages/cli/src/utils/pr-review-trigger.ts
@@ -10,6 +10,8 @@
 
 import { spawnSync } from "node:child_process";
 import snooplogg from "snooplogg";
+import type { FlairClient } from "./flair-client.js";
+import { startTaskLoop } from "./flair-task-loop.js";
 const { log: slog, warn: swarn, error: serror } = snooplogg("tps:github");
 
 
@@ -85,3 +87,27 @@ export async function handlePrOpened(
 
   requestReviews(prNumber, config);
 }
+
+/**
+ * Start the review-trigger loop: poll the agent's OWN catch-up feed for
+ * `pr.opened` events and request reviews. 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). 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.
+ */
+export function startReviewTriggerLoop(
+  flair: FlairClient,
+  agentId: string,
+  config: { reviewers: string[]; repo: string },
+  opts: { pollIntervalMs?: number } = {},
+): { stop: () => Promise<void> } {
+  const triggerConfig: ReviewTriggerConfig = { reviewers: config.reviewers, agentId, repo: config.repo };
+  return startTaskLoop(
+    flair,
+    agentId,
+    async (event) => { await handlePrOpened(event, triggerConfig); },
+    { kinds: ["pr.opened"], loopId: `${agentId}-review-trigger`, pollIntervalMs: opts.pollIntervalMs },
+  );
+}
diff --git a/packages/cli/test/review-trigger-loop.test.ts b/packages/cli/test/review-trigger-loop.test.ts
new file mode 100644
index 00000000..1183c552
--- /dev/null
+++ b/packages/cli/test/review-trigger-loop.test.ts
@@ -0,0 +1,159 @@
+/**
+ * cli#631 — the review-trigger loop reads only feeds the agent may read: it
+ * reads the agent's OWN catch-up feed (`agentId`), the feed Flair serves an
+ * event addressed to that agent to. 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.
+ *
+ * The catch-up route is `orgEventCatchupRoute`, the shared signed stub modelled
+ * on Flair's coded catch-up contract; it refuses a non-admin read of another
+ * participant's feed with 403, so a loop that reads a foreign feed gets nothing.
+ * The loop runs through the real FlairClient, signer and envelope reader; only
+ * the HTTP endpoint is faked.
+ */
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { mkdtempSync, readFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { Writable } from "node:stream";
+import { SnoopLogg } from "snooplogg";
+import { createFlairClient } from "../src/utils/flair-client.js";
+import { startReviewTriggerLoop } from "../src/utils/pr-review-trigger.js";
+import {
+  type CatchupFeed,
+  type CatchupRoute,
+  installStubFlairFetch,
+  orgEventCatchupRoute,
+  type StubFlair,
+  writeKeyFile,
+} from "./helpers/stub-flair.js";
+
+const AGENT = "agent-a";
+const AUTHOR = "agent-b";
+const SEED = Buffer.alloc(32, 0x31);
+const REPO = "example/repo";
+
+let stub: StubFlair | undefined;
+let home: string;
+let keyPath: string;
+let warnings = "";
+let output: Writable;
+let logger: SnoopLogg;
+const savedHome = process.env.HOME;
+
+beforeEach(() => {
+  home = mkdtempSync(join(tmpdir(), "tps-review-trigger-"));
+  process.env.HOME = home;
+  keyPath = writeKeyFile(join(home, ".tps", "identity"), AGENT, SEED);
+  warnings = "";
+  output = new Writable({ write(chunk, _encoding, done) { warnings += chunk.toString(); done(); } });
+  logger = new SnoopLogg().enable("tps:flair").snoop().pipe(output, { colors: false });
+});
+
+afterEach(() => {
+  logger.unsnoop().unpipe(output);
+  stub?.stop();
+  stub = undefined;
+  if (savedHome === undefined) delete process.env.HOME;
+  else process.env.HOME = savedHome;
+  rmSync(home, { recursive: true, force: true });
+});
+
+/** The review-trigger loop's cursor file under the test HOME (its own, not the task loop's). */
+const cursorFile = () => join(home, ".tps", "cursors", `${AGENT}-review-trigger-task-loop.json`);
+
+/** The cursor position the loop persisted, or undefined when it wrote none. */
+function cursorPosition(): string | undefined {
+  try {
+    return JSON.parse(readFileSync(cursorFile(), "utf-8")).position;
+  } catch {
+    return undefined;
+  }
+}
+
+const client = () => createFlairClient(AGENT, stub!.url, keyPath);
+
+/** A pr.opened event `handlePrOpened` can request reviews for. */
+const prOpened = (id: string, minutesAgo: number, targetIds: string[] = [AGENT]) => ({
+  id, kind: "pr.opened", authorId: AUTHOR, summary: `PR ${id}`,
+  detail: `https://github.com/${REPO}/pull/1`,
+  createdAt: new Date(Date.now() - minutesAgo * 60_000).toISOString(), targetIds,
+});
+
+/** The feed position Flair gives an event: `<createdAt>\t<id>`. */
+const positionOf = (e: { createdAt: string; id: string }) => `${e.createdAt}\t${e.id}`;
+
+/** Serve `feed` through the verifying stub, recording every participant a catch-up request names. */
+function serve(feed: CatchupFeed): { route: CatchupRoute; asked: string[] } {
+  stub?.stop();
+  const route = orgEventCatchupRoute(feed);
+  const asked: string[] = [];
+  stub = installStubFlairFetch({ [AGENT]: SEED }, (req, url, caller) => {
+    if (!url.pathname.startsWith("/OrgEventCatchup/")) return undefined;
+    asked.push(decodeURIComponent(url.pathname.slice("/OrgEventCatchup/".length)));
+    return route(req, url, caller);
+  });
+  return { route, asked };
+}
+
+/** Serve `answer` for every catch-up request, recording the `after` query. */
+function serveAnswer(answer: (req: Request, url: URL) => Response): CatchupRoute {
+  stub?.stop();
+  const route = ((req: Request, url: URL) => {
+    route.afters.push(url.searchParams.get("after") ?? "");
+    return answer(req, url);
+  }) as CatchupRoute;
+  route.afters = [];
+  route.served = [];
+  stub = installStubFlairFetch({ [AGENT]: SEED }, (req, url) => url.pathname.startsWith("/OrgEventCatchup/") ? route(req, url, "") : undefined);
+  return route;
+}
+
+/**
+ * Poll `cond` until it holds or `timeoutMs` elapses; return whether it held. The
+ * default stays under bun's 5 s test timeout, so a failing test still stops its
+ * loop in `finally`.
+ */
+async function waitFor(cond: () => boolean, timeoutMs = 3_000): Promise<boolean> {
+  const deadline = performance.now() + timeoutMs;
+  while (performance.now() < deadline) {
+    if (cond()) return true;
+    await Bun.sleep(10);
+  }
+  return cond();
+}
+
+const LOOP_OPTS = { pollIntervalMs: 30 };
+
+describe("cli#631 — the review-trigger loop reads only feeds the agent may read", () => {
+  test("it reads the agent's own feed, so a pr.opened trigger addressed to the agent is handled", async () => {
+    const e1 = prOpened("e1", 5);
+    const { route, asked } = serve({ events: [e1] });
+    const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: [], repo: REPO }, LOOP_OPTS);
+    try {
+      // The cursor advances only after the handler succeeds; a refused read advances nothing.
+      expect(await waitFor(() => cursorPosition() === positionOf(e1))).toBe(true);
+    } finally {
+      await loop.stop();
+    }
+    // Every request named the agent's own feed. A foreign feed (…-review-trigger) would 403 and deliver nothing.
+    expect(asked.length).toBeGreaterThan(0);
+    expect(new Set(asked)).toEqual(new Set([AGENT]));
+    expect(route.served).toEqual(["e1"]);
+    expect(warnings).not.toContain("unavailable");
+  });
+
+  test("a refused read is reported unavailable, not treated as an empty feed", async () => {
+    const route = serveAnswer(() => Response.json({ error: "forbidden" }, { status: 403 }));
+    const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: [], repo: REPO }, LOOP_OPTS);
+    try {
+      expect(await waitFor(() => route.afters.length >= 1)).toBe(true);
+      expect(await waitFor(() => warnings.includes("catch-up read unavailable"))).toBe(true);
+    } finally {
+      await loop.stop();
+    }
+    expect(warnings).toContain("(HTTP 403)");
+    // The initial position was saved; the refused read advanced the cursor no further.
+    expect(cursorPosition()).toBe(route.afters[0]);
+  });
+});

From 7f825f19ff615b8cea86391a9e6347d59eae7033 Mon Sep 17 00:00:00 2001
From: flint <flint@tps.dev>
Date: Thu, 15 Oct 2026 16:39:28 +0000
Subject: [PATCH 2/5] test(cli): the review-trigger loop's own-feed case
 asserts the review request

The handled case configured no reviewers, so it passed with the review
request removed. It now configures reviewers and asserts the gh-as command
and payload for a targeted pr.opened event, and the claims that the loop
requests a review end to end are narrowed to what this change guarantees
(when the loop is started with a review-trigger config it reads only the
agent's own feed).
---
 .../fixed-631-review-trigger-own-feed.md      |  2 +-
 packages/cli/src/utils/codex-runtime.ts       |  6 +++---
 packages/cli/src/utils/pr-review-trigger.ts   | 18 +++++++++---------
 packages/cli/test/review-trigger-loop.test.ts | 19 +++++++++++++++++--
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
index 6520eee0..551e4b86 100644
--- a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
+++ b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
@@ -1 +1 @@
-- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** 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. 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.
+- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** It read a feed named for the trigger (`<agentId>-review-trigger`) while signing as the agent, which Flair refuses with 403. 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.
diff --git a/packages/cli/src/utils/codex-runtime.ts b/packages/cli/src/utils/codex-runtime.ts
index fb7117d6..aa3e4a20 100644
--- a/packages/cli/src/utils/codex-runtime.ts
+++ b/packages/cli/src/utils/codex-runtime.ts
@@ -853,9 +853,9 @@ export async function runCodexRuntime(config: CodexRuntimeConfig): Promise<void>
     }
   });
 
-  // 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).
+  // Review trigger — starts the review-trigger loop on the agent's own catch-up
+  // feed (cli#631: Flair refuses a read of another participant's feed, so the
+  // loop received no events while it read one).
   if (config.reviewTrigger) {
     startReviewTriggerLoop(flair, agentId, config.reviewTrigger);
     console.log(`[${agentId}] Review trigger active → ${config.reviewTrigger.reviewers.join(", ")}`);
diff --git a/packages/cli/src/utils/pr-review-trigger.ts b/packages/cli/src/utils/pr-review-trigger.ts
index 2b4d13b7..dcd89e93 100644
--- a/packages/cli/src/utils/pr-review-trigger.ts
+++ b/packages/cli/src/utils/pr-review-trigger.ts
@@ -76,6 +76,7 @@ export function reRequestReviewer(
 export async function handlePrOpened(
   event: { detail?: string; summary: string; refId?: string },
   config: ReviewTriggerConfig,
+  deps: ReviewRequestDeps = {},
 ): Promise<void> {
   const detail = event.detail ?? "";
   const prNumber = extractPrNumber(detail);
@@ -85,29 +86,28 @@ export async function handlePrOpened(
     return;
   }
 
-  requestReviews(prNumber, config);
+  requestReviews(prNumber, config, deps);
 }
 
 /**
  * Start the review-trigger loop: poll the agent's OWN catch-up feed for
- * `pr.opened` events and request reviews. 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). 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.
+ * `pr.opened` events. 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).
+ * 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.
  */
 export function startReviewTriggerLoop(
   flair: FlairClient,
   agentId: string,
   config: { reviewers: string[]; repo: string },
-  opts: { pollIntervalMs?: number } = {},
+  opts: { pollIntervalMs?: number; deps?: ReviewRequestDeps } = {},
 ): { stop: () => Promise<void> } {
   const triggerConfig: ReviewTriggerConfig = { reviewers: config.reviewers, agentId, repo: config.repo };
   return startTaskLoop(
     flair,
     agentId,
-    async (event) => { await handlePrOpened(event, triggerConfig); },
+    async (event) => { await handlePrOpened(event, triggerConfig, opts.deps); },
     { kinds: ["pr.opened"], loopId: `${agentId}-review-trigger`, pollIntervalMs: opts.pollIntervalMs },
   );
 }
diff --git a/packages/cli/test/review-trigger-loop.test.ts b/packages/cli/test/review-trigger-loop.test.ts
index 1183c552..c8aa5c8d 100644
--- a/packages/cli/test/review-trigger-loop.test.ts
+++ b/packages/cli/test/review-trigger-loop.test.ts
@@ -126,16 +126,31 @@ async function waitFor(cond: () => boolean, timeoutMs = 3_000): Promise<boolean>
 const LOOP_OPTS = { pollIntervalMs: 30 };
 
 describe("cli#631 — the review-trigger loop reads only feeds the agent may read", () => {
-  test("it reads the agent's own feed, so a pr.opened trigger addressed to the agent is handled", async () => {
+  test("it reads the agent's own feed and requests the configured reviewers for a trigger addressed to the agent", async () => {
     const e1 = prOpened("e1", 5);
     const { route, asked } = serve({ events: [e1] });
-    const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: [], repo: REPO }, LOOP_OPTS);
+    const ghCalls: Array<{ cmd: string; args: string[]; input?: string }> = [];
+    const deps = {
+      spawnSyncImpl: ((cmd: string, args: string[], opts?: { input?: string }) => {
+        ghCalls.push({ cmd, args, input: opts?.input });
+        return { status: 0, stdout: "", stderr: "" };
+      }) as any,
+    };
+    const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: ["reviewer-1"], repo: REPO }, { ...LOOP_OPTS, deps });
     try {
       // The cursor advances only after the handler succeeds; a refused read advances nothing.
       expect(await waitFor(() => cursorPosition() === positionOf(e1))).toBe(true);
+      expect(ghCalls).toHaveLength(1);
     } finally {
       await loop.stop();
     }
+    // The handler issued the review request the review-trigger loop exists for: gh-as, as the agent,
+    // on the PR in the event's /pull/<n> URL, with the configured reviewers as the payload.
+    expect(ghCalls[0]?.cmd).toBe("gh-as");
+    expect(ghCalls[0]?.args).toEqual([
+      AGENT, "api", "--method", "POST", `repos/${REPO}/pulls/1/requested_reviewers`, "--input", "-",
+    ]);
+    expect(ghCalls[0]?.input).toBe(JSON.stringify({ reviewers: ["reviewer-1"] }));
     // Every request named the agent's own feed. A foreign feed (…-review-trigger) would 403 and deliver nothing.
     expect(asked.length).toBeGreaterThan(0);
     expect(new Set(asked)).toEqual(new Set([AGENT]));

From ee633af84caf7fe3d468da7bfc7e4ee149b748a5 Mon Sep 17 00:00:00 2001
From: anvil <anvil@tpsdev.ai>
Date: Thu, 15 Oct 2026 19:06:49 +0000
Subject: [PATCH 3/5] fix(cli): refuse a loopId that is not a single safe
 filename component

startTaskLoop names the cursor file <loopId>-task-loop.json inside
~/.tps/cursors, so an unsafe loopId could resolve a cursor outside that
directory. It now requires a single filename component (1-64 chars of
A-Za-z0-9._- , not "." or "..") and refuses any other with
InvalidLoopIdError before reading or writing a cursor.
---
 packages/cli/src/utils/flair-task-loop.ts     | 30 ++++++++++++++++++-
 packages/cli/test/flair-task-loop.test.ts     | 30 +++++++++++++++++--
 packages/cli/test/review-trigger-loop.test.ts |  7 +++--
 3 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/packages/cli/src/utils/flair-task-loop.ts b/packages/cli/src/utils/flair-task-loop.ts
index ea4f230a..42762e1c 100644
--- a/packages/cli/src/utils/flair-task-loop.ts
+++ b/packages/cli/src/utils/flair-task-loop.ts
@@ -10,7 +10,9 @@
  * `{ position, updatedAt }` and replaced atomically (a temporary file in the
  * same directory, fsynced, then renamed over it). `loopId` names the cursor and
  * is `agentId` unless the caller names another, so two loops reading one
- * agent's feed keep separate positions. With no usable cursor file, the loop sets the position to
+ * agent's feed keep separate positions. It must be a single safe filename
+ * component (see `LOOP_ID_PATTERN`); `startTaskLoop` refuses any other with
+ * `InvalidLoopIdError` before reading or writing a cursor. With no usable cursor file, the loop sets the position to
  * FIRST_BOOT_LOOKBACK_MS before now, once, and does not read the feed until
  * that position is saved: a failed save is logged by name and retried with
  * backoff, with the same position, and `stop()` ends the retrying. A legacy
@@ -97,6 +99,28 @@ function cursorDir(): string {
   return join(homeDir(), ".tps", "cursors");
 }
 
+/**
+ * The allowed set for `loopId`: a single filename component of 1..64 characters
+ * from `A-Z a-z 0-9 . _ -`, excluding the two dot components `.` and `..`. It
+ * names the cursor file `<loopId>-task-loop.json` inside the cursors directory,
+ * so a path separator, a NUL, an empty or over-long string, or `.`/`..` is
+ * refused rather than allowed to resolve a cursor outside that directory.
+ */
+const LOOP_ID_PATTERN = /^[A-Za-z0-9._-]{1,64}$/;
+
+/** Whether `loopId` is a single safe filename component (see `LOOP_ID_PATTERN`). */
+export function isSafeLoopId(loopId: string): boolean {
+  return LOOP_ID_PATTERN.test(loopId) && loopId !== "." && loopId !== "..";
+}
+
+/** Thrown by `startTaskLoop` for a `loopId` that is not a single safe filename component. */
+export class InvalidLoopIdError extends Error {
+  constructor(loopId: string) {
+    super(`Invalid loopId ${JSON.stringify(loopId)}: it must be a single filename component matching ${LOOP_ID_PATTERN} and not "." or ".."`);
+    this.name = "InvalidLoopIdError";
+  }
+}
+
 /** The loop's cursor file for `loopId`. */
 export function taskLoopCursorPath(loopId: string): string {
   return join(cursorDir(), `${loopId}-task-loop.json`);
@@ -199,6 +223,10 @@ export function startTaskLoop(
   // The cursor is named for the loop, not the feed: two loops reading one
   // agent's feed (`agentId`) keep separate positions.
   const loopId = opts.loopId ?? agentId;
+  // Refuse an unsafe loopId before any cursor is read or written: it names the
+  // cursor file, so one with a path separator or a traversal segment could
+  // resolve a cursor outside the cursors directory.
+  if (!isSafeLoopId(loopId)) throw new InvalidLoopIdError(loopId);
   const loaded = loadCursor(loopId);
   // A new initial position is computed once, here; saving it is retried with
   // this same value until it succeeds, and the feed is not read before then.
diff --git a/packages/cli/test/flair-task-loop.test.ts b/packages/cli/test/flair-task-loop.test.ts
index 7daae2bc..2378764a 100644
--- a/packages/cli/test/flair-task-loop.test.ts
+++ b/packages/cli/test/flair-task-loop.test.ts
@@ -10,13 +10,13 @@
  * the signing and parsing seams are exercised; only the HTTP endpoint is faked.
  */
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { Writable } from "node:stream";
 import { SnoopLogg } from "snooplogg";
-import { createFlairClient } from "../src/utils/flair-client.js";
-import { MAX_POSITION_SKEW_MS, startTaskLoop } from "../src/utils/flair-task-loop.js";
+import { createFlairClient, type FlairClient } from "../src/utils/flair-client.js";
+import { InvalidLoopIdError, MAX_POSITION_SKEW_MS, startTaskLoop } from "../src/utils/flair-task-loop.js";
 import { createPatchShared } from "./helpers/patch-shared.js";
 import {
   type CatchupFeed,
@@ -709,3 +709,27 @@ describe("cli#624 — the task loop's cursor file", () => {
     expect(readdirSync(join(home, ".tps", "cursors"))).toEqual([`${AGENT}-task-loop.json`]);
   });
 });
+
+describe("cli#631 — loopId is a single safe filename component", () => {
+  test("refuses a traversal, separator, dot or over-long loopId before reading or writing a cursor", () => {
+    // A client the loop could not use even if it got past the check: the refusal must come first.
+    const untouched = {} as FlairClient;
+    for (const bad of ["../../outside", "a/b", "..", ".", "a\\b", "x".repeat(65), "a\0b"]) {
+      expect(() => startTaskLoop(untouched, AGENT, async () => {}, { loopId: bad })).toThrow(InvalidLoopIdError);
+    }
+    // Nothing was read or written: the cursors directory does not exist, and the
+    // path a traversal loopId would reach is untouched.
+    expect(existsSync(join(home, ".tps", "cursors"))).toBe(false);
+    expect(existsSync(join(home, "outside-task-loop.json"))).toBe(false);
+  });
+
+  test("accepts a valid loopId and names its cursor file with it", async () => {
+    serveFeed({ events: [] });
+    const loop = startTaskLoop(client(), AGENT, async () => {}, { loopId: "agent-a-review-trigger", pollIntervalMs: 60_000 });
+    try {
+      expect(await waitFor(() => existsSync(join(home, ".tps", "cursors", "agent-a-review-trigger-task-loop.json")))).toBe(true);
+    } finally {
+      await loop.stop();
+    }
+  });
+});
diff --git a/packages/cli/test/review-trigger-loop.test.ts b/packages/cli/test/review-trigger-loop.test.ts
index c8aa5c8d..aa307d3a 100644
--- a/packages/cli/test/review-trigger-loop.test.ts
+++ b/packages/cli/test/review-trigger-loop.test.ts
@@ -8,8 +8,9 @@
  * The catch-up route is `orgEventCatchupRoute`, the shared signed stub modelled
  * on Flair's coded catch-up contract; it refuses a non-admin read of another
  * participant's feed with 403, so a loop that reads a foreign feed gets nothing.
- * The loop runs through the real FlairClient, signer and envelope reader; only
- * the HTTP endpoint is faked.
+ * The loop runs through the real FlairClient, signer and envelope reader; the
+ * HTTP catch-up endpoint is faked, and the case that drives the review request
+ * fakes the `gh-as` process call too (`spawnSyncImpl`).
  */
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import { mkdtempSync, readFileSync, rmSync } from "node:fs";
@@ -126,7 +127,7 @@ async function waitFor(cond: () => boolean, timeoutMs = 3_000): Promise<boolean>
 const LOOP_OPTS = { pollIntervalMs: 30 };
 
 describe("cli#631 — the review-trigger loop reads only feeds the agent may read", () => {
-  test("it reads the agent's own feed and requests the configured reviewers for a trigger addressed to the agent", async () => {
+  test("with the HTTP catch-up endpoint faked and the gh-as process call faked (spawnSyncImpl), it reads the agent's own feed and requests the configured reviewers for a trigger addressed to the agent", async () => {
     const e1 = prOpened("e1", 5);
     const { route, asked } = serve({ events: [e1] });
     const ghCalls: Array<{ cmd: string; args: string[]; input?: string }> = [];

From 1a034317119608cdd01db14fa94710189b6ef319 Mon Sep 17 00:00:00 2001
From: flint <flint@tps.dev>
Date: Thu, 15 Oct 2026 16:09:49 -0700
Subject: [PATCH 4/5] round: address the review verdict (Codex fix round)

Round 4 (Codex, Anvil busy): derived loop IDs allowed through 79 chars (64-char agent ID + -review-trigger), boundary-tested; cursor-separation comments conditioned on distinct loop IDs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../fixed-631-review-trigger-own-feed.md      |  2 +-
 packages/cli/src/utils/codex-runtime.ts       |  4 +-
 packages/cli/src/utils/flair-task-loop.ts     | 14 +++----
 packages/cli/src/utils/pr-review-trigger.ts   |  8 +---
 packages/cli/test/flair-task-loop.test.ts     | 20 +++++-----
 packages/cli/test/review-trigger-loop.test.ts | 37 ++++++++++++-------
 6 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
index 551e4b86..cd0cdcb2 100644
--- a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
+++ b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
@@ -1 +1 @@
-- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** It read a feed named for the trigger (`<agentId>-review-trigger`) while signing as the agent, which Flair refuses with 403. 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.
+- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** It keeps a distinct review cursor and accepts loop IDs through 79 characters, including a 64-character agent ID plus `-review-trigger`.
diff --git a/packages/cli/src/utils/codex-runtime.ts b/packages/cli/src/utils/codex-runtime.ts
index aa3e4a20..6265a171 100644
--- a/packages/cli/src/utils/codex-runtime.ts
+++ b/packages/cli/src/utils/codex-runtime.ts
@@ -853,9 +853,7 @@ export async function runCodexRuntime(config: CodexRuntimeConfig): Promise<void>
     }
   });
 
-  // Review trigger — starts the review-trigger loop on the agent's own catch-up
-  // feed (cli#631: Flair refuses a read of another participant's feed, so the
-  // loop received no events while it read one).
+  // Review trigger — polls the agent's own feed with a distinct review cursor.
   if (config.reviewTrigger) {
     startReviewTriggerLoop(flair, agentId, config.reviewTrigger);
     console.log(`[${agentId}] Review trigger active → ${config.reviewTrigger.reviewers.join(", ")}`);
diff --git a/packages/cli/src/utils/flair-task-loop.ts b/packages/cli/src/utils/flair-task-loop.ts
index 42762e1c..35ecc6f2 100644
--- a/packages/cli/src/utils/flair-task-loop.ts
+++ b/packages/cli/src/utils/flair-task-loop.ts
@@ -8,10 +8,9 @@
  * own watermark for the agent never chooses where reading starts. The position
  * is persisted in `~/.tps/cursors/<loopId>-task-loop.json` as
  * `{ position, updatedAt }` and replaced atomically (a temporary file in the
- * same directory, fsynced, then renamed over it). `loopId` names the cursor and
- * is `agentId` unless the caller names another, so two loops reading one
- * agent's feed keep separate positions. It must be a single safe filename
- * component (see `LOOP_ID_PATTERN`); `startTaskLoop` refuses any other with
+ * same directory, fsynced, then renamed over it). Distinct `loopId` values keep
+ * separate positions; callers that omit it share the `agentId` cursor. It must
+ * be a single safe filename component (see `LOOP_ID_PATTERN`); `startTaskLoop` refuses any other with
  * `InvalidLoopIdError` before reading or writing a cursor. With no usable cursor file, the loop sets the position to
  * FIRST_BOOT_LOOKBACK_MS before now, once, and does not read the feed until
  * that position is saved: a failed save is logged by name and retried with
@@ -100,13 +99,13 @@ function cursorDir(): string {
 }
 
 /**
- * The allowed set for `loopId`: a single filename component of 1..64 characters
+ * The allowed set for `loopId`: a single filename component of 1..79 characters
  * from `A-Z a-z 0-9 . _ -`, excluding the two dot components `.` and `..`. It
  * names the cursor file `<loopId>-task-loop.json` inside the cursors directory,
  * so a path separator, a NUL, an empty or over-long string, or `.`/`..` is
  * refused rather than allowed to resolve a cursor outside that directory.
  */
-const LOOP_ID_PATTERN = /^[A-Za-z0-9._-]{1,64}$/;
+const LOOP_ID_PATTERN = /^[A-Za-z0-9._-]{1,79}$/;
 
 /** Whether `loopId` is a single safe filename component (see `LOOP_ID_PATTERN`). */
 export function isSafeLoopId(loopId: string): boolean {
@@ -220,8 +219,7 @@ export function startTaskLoop(
 ): { stop: () => Promise<void> } {
   const basePollMs = opts.pollIntervalMs ?? DEFAULT_POLL_MS;
   const kinds = new Set(opts.kinds ?? ["task.assigned"]);
-  // The cursor is named for the loop, not the feed: two loops reading one
-  // agent's feed (`agentId`) keep separate positions.
+  // Distinct loop IDs keep separate positions; omitted IDs share the agentId cursor.
   const loopId = opts.loopId ?? agentId;
   // Refuse an unsafe loopId before any cursor is read or written: it names the
   // cursor file, so one with a path separator or a traversal segment could
diff --git a/packages/cli/src/utils/pr-review-trigger.ts b/packages/cli/src/utils/pr-review-trigger.ts
index dcd89e93..0934968c 100644
--- a/packages/cli/src/utils/pr-review-trigger.ts
+++ b/packages/cli/src/utils/pr-review-trigger.ts
@@ -90,12 +90,8 @@ export async function handlePrOpened(
 }
 
 /**
- * Start the review-trigger loop: poll the agent's OWN catch-up feed for
- * `pr.opened` events. 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).
- * 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.
+ * Poll the agent's own feed for `pr.opened`, with a distinct review cursor.
+ * The loop ID allows the CLI's 64-character agent ID plus `-review-trigger`.
  */
 export function startReviewTriggerLoop(
   flair: FlairClient,
diff --git a/packages/cli/test/flair-task-loop.test.ts b/packages/cli/test/flair-task-loop.test.ts
index 2378764a..a122ee37 100644
--- a/packages/cli/test/flair-task-loop.test.ts
+++ b/packages/cli/test/flair-task-loop.test.ts
@@ -10,7 +10,7 @@
  * the signing and parsing seams are exercised; only the HTTP endpoint is faked.
  */
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { Writable } from "node:stream";
@@ -507,16 +507,16 @@ describe("cli#624 — the initial position is set once and always sent", () => {
 });
 
 describe("cli#624 — the initial position is saved before the first read", () => {
-  // A real write failure: the cursor directory is made unwritable, then restored.
+  // A regular file blocks creation of the cursor directory, including for root.
   const cursors = () => join(home, ".tps", "cursors");
   const lockCursors = () => {
-    mkdirSync(cursors(), { recursive: true });
-    chmodSync(cursors(), 0o500);
+    mkdirSync(join(home, ".tps"), { recursive: true });
+    writeFileSync(cursors(), "blocked");
   };
-  const unlockCursors = () => chmodSync(cursors(), 0o700);
+  const unlockCursors = () => rmSync(cursors());
   const saveFailures = () => warnings.split("cannot save its initial position").length - 1;
 
-  test.skipIf(process.getuid?.() === 0)(
+  test(
     "while the initial position cannot be saved nothing is read; after recovery the saved position is the one fixed at start, and a restart retries the first task",
     async () => {
       const startNow = Date.now();
@@ -535,9 +535,9 @@ describe("cli#624 — the initial position is saved before the first read", () =
         }, { pollIntervalMs: 20 });
         try {
           expect(await waitFor(() => saveFailures() >= 3)).toBe(true);
-          expect(warnings).toContain(`cannot save its initial position to ${cursorFile()} (EACCES)`);
+          expect(warnings).toContain(`cannot save its initial position to ${cursorFile()} (EEXIST)`);
           expect(catchup.afters).toEqual([]);
-          // Half an hour passes before the directory is writable again: an initial
+          // Half an hour passes before the directory can be created: an initial
           // position recomputed now would start after e1.
           offset = 30 * 60_000;
           unlockCursors();
@@ -567,7 +567,7 @@ describe("cli#624 — the initial position is saved before the first read", () =
     },
   );
 
-  test.skipIf(process.getuid?.() === 0)("stop ends the retrying of the initial save promptly, and nothing is read", async () => {
+  test("stop ends the retrying of the initial save promptly, and nothing is read", async () => {
     const catchup = serveFeed({ events: [assigned("e1", 5)] });
     lockCursors();
     try {
@@ -714,7 +714,7 @@ describe("cli#631 — loopId is a single safe filename component", () => {
   test("refuses a traversal, separator, dot or over-long loopId before reading or writing a cursor", () => {
     // A client the loop could not use even if it got past the check: the refusal must come first.
     const untouched = {} as FlairClient;
-    for (const bad of ["../../outside", "a/b", "..", ".", "a\\b", "x".repeat(65), "a\0b"]) {
+    for (const bad of ["../../outside", "a/b", "..", ".", "a\\b", "x".repeat(80), "a\0b"]) {
       expect(() => startTaskLoop(untouched, AGENT, async () => {}, { loopId: bad })).toThrow(InvalidLoopIdError);
     }
     // Nothing was read or written: the cursors directory does not exist, and the
diff --git a/packages/cli/test/review-trigger-loop.test.ts b/packages/cli/test/review-trigger-loop.test.ts
index aa307d3a..4e8033a8 100644
--- a/packages/cli/test/review-trigger-loop.test.ts
+++ b/packages/cli/test/review-trigger-loop.test.ts
@@ -13,12 +13,13 @@
  * fakes the `gh-as` process call too (`spawnSyncImpl`).
  */
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { mkdtempSync, readFileSync, rmSync } from "node:fs";
+import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { Writable } from "node:stream";
 import { SnoopLogg } from "snooplogg";
 import { createFlairClient } from "../src/utils/flair-client.js";
+import { InvalidLoopIdError } from "../src/utils/flair-task-loop.js";
 import { startReviewTriggerLoop } from "../src/utils/pr-review-trigger.js";
 import {
   type CatchupFeed,
@@ -61,18 +62,18 @@ afterEach(() => {
 });
 
 /** The review-trigger loop's cursor file under the test HOME (its own, not the task loop's). */
-const cursorFile = () => join(home, ".tps", "cursors", `${AGENT}-review-trigger-task-loop.json`);
+const cursorFile = (agentId = AGENT) => join(home, ".tps", "cursors", `${agentId}-review-trigger-task-loop.json`);
 
 /** The cursor position the loop persisted, or undefined when it wrote none. */
-function cursorPosition(): string | undefined {
+function cursorPosition(agentId = AGENT): string | undefined {
   try {
-    return JSON.parse(readFileSync(cursorFile(), "utf-8")).position;
+    return JSON.parse(readFileSync(cursorFile(agentId), "utf-8")).position;
   } catch {
     return undefined;
   }
 }
 
-const client = () => createFlairClient(AGENT, stub!.url, keyPath);
+const client = (agentId = AGENT) => createFlairClient(agentId, stub!.url, keyPath);
 
 /** A pr.opened event `handlePrOpened` can request reviews for. */
 const prOpened = (id: string, minutesAgo: number, targetIds: string[] = [AGENT]) => ({
@@ -85,11 +86,11 @@ const prOpened = (id: string, minutesAgo: number, targetIds: string[] = [AGENT])
 const positionOf = (e: { createdAt: string; id: string }) => `${e.createdAt}\t${e.id}`;
 
 /** Serve `feed` through the verifying stub, recording every participant a catch-up request names. */
-function serve(feed: CatchupFeed): { route: CatchupRoute; asked: string[] } {
+function serve(feed: CatchupFeed, agentId = AGENT): { route: CatchupRoute; asked: string[] } {
   stub?.stop();
   const route = orgEventCatchupRoute(feed);
   const asked: string[] = [];
-  stub = installStubFlairFetch({ [AGENT]: SEED }, (req, url, caller) => {
+  stub = installStubFlairFetch({ [agentId]: SEED }, (req, url, caller) => {
     if (!url.pathname.startsWith("/OrgEventCatchup/")) return undefined;
     asked.push(decodeURIComponent(url.pathname.slice("/OrgEventCatchup/".length)));
     return route(req, url, caller);
@@ -127,9 +128,10 @@ async function waitFor(cond: () => boolean, timeoutMs = 3_000): Promise<boolean>
 const LOOP_OPTS = { pollIntervalMs: 30 };
 
 describe("cli#631 — the review-trigger loop reads only feeds the agent may read", () => {
-  test("with the HTTP catch-up endpoint faked and the gh-as process call faked (spawnSyncImpl), it reads the agent's own feed and requests the configured reviewers for a trigger addressed to the agent", async () => {
-    const e1 = prOpened("e1", 5);
-    const { route, asked } = serve({ events: [e1] });
+  test.each([AGENT, "a".repeat(64)])("agent %s: with the HTTP catch-up endpoint faked and the gh-as process call faked (spawnSyncImpl), it reads the agent's own feed and requests the configured reviewers for a trigger addressed to the agent", async (agentId) => {
+    keyPath = writeKeyFile(join(home, ".tps", "identity"), agentId, SEED);
+    const e1 = prOpened("e1", 5, [agentId]);
+    const { route, asked } = serve({ events: [e1] }, agentId);
     const ghCalls: Array<{ cmd: string; args: string[]; input?: string }> = [];
     const deps = {
       spawnSyncImpl: ((cmd: string, args: string[], opts?: { input?: string }) => {
@@ -137,10 +139,10 @@ describe("cli#631 — the review-trigger loop reads only feeds the agent may rea
         return { status: 0, stdout: "", stderr: "" };
       }) as any,
     };
-    const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: ["reviewer-1"], repo: REPO }, { ...LOOP_OPTS, deps });
+    const loop = startReviewTriggerLoop(client(agentId), agentId, { reviewers: ["reviewer-1"], repo: REPO }, { ...LOOP_OPTS, deps });
     try {
       // The cursor advances only after the handler succeeds; a refused read advances nothing.
-      expect(await waitFor(() => cursorPosition() === positionOf(e1))).toBe(true);
+      expect(await waitFor(() => cursorPosition(agentId) === positionOf(e1))).toBe(true);
       expect(ghCalls).toHaveLength(1);
     } finally {
       await loop.stop();
@@ -149,16 +151,23 @@ describe("cli#631 — the review-trigger loop reads only feeds the agent may rea
     // on the PR in the event's /pull/<n> URL, with the configured reviewers as the payload.
     expect(ghCalls[0]?.cmd).toBe("gh-as");
     expect(ghCalls[0]?.args).toEqual([
-      AGENT, "api", "--method", "POST", `repos/${REPO}/pulls/1/requested_reviewers`, "--input", "-",
+      agentId, "api", "--method", "POST", `repos/${REPO}/pulls/1/requested_reviewers`, "--input", "-",
     ]);
     expect(ghCalls[0]?.input).toBe(JSON.stringify({ reviewers: ["reviewer-1"] }));
     // Every request named the agent's own feed. A foreign feed (…-review-trigger) would 403 and deliver nothing.
     expect(asked.length).toBeGreaterThan(0);
-    expect(new Set(asked)).toEqual(new Set([AGENT]));
+    expect(new Set(asked)).toEqual(new Set([agentId]));
     expect(route.served).toEqual(["e1"]);
     expect(warnings).not.toContain("unavailable");
   });
 
+  test("refuses the 80-character review loop ID derived from a 65-character agent ID before cursor access", () => {
+    serve({ events: [] });
+    expect(() => startReviewTriggerLoop(client(), "a".repeat(65), { reviewers: [], repo: REPO }))
+      .toThrow(InvalidLoopIdError);
+    expect(existsSync(join(home, ".tps", "cursors"))).toBe(false);
+  });
+
   test("a refused read is reported unavailable, not treated as an empty feed", async () => {
     const route = serveAnswer(() => Response.json({ error: "forbidden" }, { status: 403 }));
     const loop = startReviewTriggerLoop(client(), AGENT, { reviewers: [], repo: REPO }, LOOP_OPTS);

From 1f6eea40d6af2c67309387617897fa8007710bb2 Mon Sep 17 00:00:00 2001
From: flint <flint@tps.dev>
Date: Thu, 15 Oct 2026 16:17:14 -0700
Subject: [PATCH 5/5] round: address the review verdict (Codex fix round)

Text (Codex): cursor separation stated as the two loops' distinct filenames; the 403 qualified to non-admin agents

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../unreleased/fixed-631-review-trigger-own-feed.md      | 2 +-
 packages/cli/src/utils/codex-runtime.ts                  | 2 +-
 packages/cli/src/utils/flair-task-loop.ts                | 6 +++---
 packages/cli/src/utils/pr-review-trigger.ts              | 2 +-
 packages/cli/test/review-trigger-loop.test.ts            | 9 ++++-----
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
index cd0cdcb2..953910f4 100644
--- a/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
+++ b/.changelog/unreleased/fixed-631-review-trigger-own-feed.md
@@ -1 +1 @@
-- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** It keeps a distinct review cursor and accepts loop IDs through 79 characters, including a 64-character agent ID plus `-review-trigger`.
+- **The codex runtime's review-trigger loop reads the agent's own catch-up feed (Closes #631).** It uses a different cursor filename from the task loop and accepts loop IDs through 79 characters, including a 64-character agent ID plus `-review-trigger`.
diff --git a/packages/cli/src/utils/codex-runtime.ts b/packages/cli/src/utils/codex-runtime.ts
index 6265a171..ce2d42f2 100644
--- a/packages/cli/src/utils/codex-runtime.ts
+++ b/packages/cli/src/utils/codex-runtime.ts
@@ -853,7 +853,7 @@ export async function runCodexRuntime(config: CodexRuntimeConfig): Promise<void>
     }
   });
 
-  // Review trigger — polls the agent's own feed with a distinct review cursor.
+  // Review trigger — polls the agent's own feed using a different cursor filename from the task loop.
   if (config.reviewTrigger) {
     startReviewTriggerLoop(flair, agentId, config.reviewTrigger);
     console.log(`[${agentId}] Review trigger active → ${config.reviewTrigger.reviewers.join(", ")}`);
diff --git a/packages/cli/src/utils/flair-task-loop.ts b/packages/cli/src/utils/flair-task-loop.ts
index 35ecc6f2..aa00febf 100644
--- a/packages/cli/src/utils/flair-task-loop.ts
+++ b/packages/cli/src/utils/flair-task-loop.ts
@@ -8,8 +8,8 @@
  * own watermark for the agent never chooses where reading starts. The position
  * is persisted in `~/.tps/cursors/<loopId>-task-loop.json` as
  * `{ position, updatedAt }` and replaced atomically (a temporary file in the
- * same directory, fsynced, then renamed over it). Distinct `loopId` values keep
- * separate positions; callers that omit it share the `agentId` cursor. It must
+ * same directory, fsynced, then renamed over it). The task and review loops use
+ * different cursor filenames; callers that omit `loopId` use the `agentId` cursor. It must
  * be a single safe filename component (see `LOOP_ID_PATTERN`); `startTaskLoop` refuses any other with
  * `InvalidLoopIdError` before reading or writing a cursor. With no usable cursor file, the loop sets the position to
  * FIRST_BOOT_LOOKBACK_MS before now, once, and does not read the feed until
@@ -219,7 +219,7 @@ export function startTaskLoop(
 ): { stop: () => Promise<void> } {
   const basePollMs = opts.pollIntervalMs ?? DEFAULT_POLL_MS;
   const kinds = new Set(opts.kinds ?? ["task.assigned"]);
-  // Distinct loop IDs keep separate positions; omitted IDs share the agentId cursor.
+  // Omitted loop IDs use the agentId cursor.
   const loopId = opts.loopId ?? agentId;
   // Refuse an unsafe loopId before any cursor is read or written: it names the
   // cursor file, so one with a path separator or a traversal segment could
diff --git a/packages/cli/src/utils/pr-review-trigger.ts b/packages/cli/src/utils/pr-review-trigger.ts
index 0934968c..4552b647 100644
--- a/packages/cli/src/utils/pr-review-trigger.ts
+++ b/packages/cli/src/utils/pr-review-trigger.ts
@@ -90,7 +90,7 @@ export async function handlePrOpened(
 }
 
 /**
- * Poll the agent's own feed for `pr.opened`, with a distinct review cursor.
+ * Poll the agent's own feed for `pr.opened`, using a different cursor filename from the task loop.
  * The loop ID allows the CLI's 64-character agent ID plus `-review-trigger`.
  */
 export function startReviewTriggerLoop(
diff --git a/packages/cli/test/review-trigger-loop.test.ts b/packages/cli/test/review-trigger-loop.test.ts
index 4e8033a8..eceeec36 100644
--- a/packages/cli/test/review-trigger-loop.test.ts
+++ b/packages/cli/test/review-trigger-loop.test.ts
@@ -1,13 +1,12 @@
 /**
  * cli#631 — the review-trigger loop reads only feeds the agent may read: it
  * reads the agent's OWN catch-up feed (`agentId`), the feed Flair serves an
- * event addressed to that agent to. 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.
+ * event addressed to that agent to. Flair refuses a non-admin agent's read of
+ * `<agentId>-review-trigger` with 403.
  *
  * The catch-up route is `orgEventCatchupRoute`, the shared signed stub modelled
  * on Flair's coded catch-up contract; it refuses a non-admin read of another
- * participant's feed with 403, so a loop that reads a foreign feed gets nothing.
+ * participant's feed with 403, so a non-admin agent gets nothing from a foreign feed.
  * The loop runs through the real FlairClient, signer and envelope reader; the
  * HTTP catch-up endpoint is faked, and the case that drives the review request
  * fakes the `gh-as` process call too (`spawnSyncImpl`).
@@ -154,7 +153,7 @@ describe("cli#631 — the review-trigger loop reads only feeds the agent may rea
       agentId, "api", "--method", "POST", `repos/${REPO}/pulls/1/requested_reviewers`, "--input", "-",
     ]);
     expect(ghCalls[0]?.input).toBe(JSON.stringify({ reviewers: ["reviewer-1"] }));
-    // Every request named the agent's own feed. A foreign feed (…-review-trigger) would 403 and deliver nothing.
+    // Every request named the agent's own feed. A non-admin agent's foreign-feed read would get 403.
     expect(asked.length).toBeGreaterThan(0);
     expect(new Set(asked)).toEqual(new Set([agentId]));
     expect(route.served).toEqual(["e1"]);