RT-164: gate-fork hook allows the worktree's own open run-gate by m4ttheweric · Pull Request #290 · m4ttstack/mattstack

m4ttheweric · 2026-09-16T01:06:05Z

Summary

Fixes the gate-fork hook to allow a worktree's own open run-gate through, instead of denying it as an improvised fork.

Harness routing plus a red repro of RT-162 finding 2
Hook now allows when this worktree's run already holds an open gate
Deny message now points at rt gate ask (live since the wave-2 restart) instead of stale guidance
Row-split regression test added, plus a fix for an inverted test name

Also: repo-purity caught identifying test-fixture strings (traced to the RT-162 dogfood examples); swapped for neutral acme placeholders in a separate fix commit before push.

Semantic change (deliberate)

The run-gate list call now passes --open: a parked run gate no longer allows this hook's form, only an open one does. This was accidental generosity in the original fix, not intent -- a parked gate is deliberately not-live, matching the parked-gate resume contract from wave 1. The owner resumes the pane first. This also fixes a real cliff: without --open, the list rides the daemon's default 500-row page (oldest-first), so the fix would have silently stopped seeing the live gate once the run: subject table passed 500 rows (~3 weeks out at the live rate). This supersedes RT-178, filed for the same cliff.

Review

CodeRabbit was rate-limited (org cap: 1 review/hour) with zero comments posted; the fallback model (fable) ran the substitute adversarial review of this security-relevant permission gate (POSIX sh, allow/deny for real tool calls). Shellcheck and dash -n both clean; JSON escaping in the deny path verified safe against quote/backslash injection; row-split anchoring verified safe against real daemon output; tests confirmed to drive the actual script (mutation-tested against both main and a stripped PR script). One blocking finding, fixed: the run-gate list's 500-row cliff described above, plus a test pinning that a parked gate denies (verified it fails against the original bug by reverting both the --open flag and the client-side status check together).

Left as reported, no fixes (shepherd's call): a daemon-side gap where ~27% of live run gates carry no origin.worktree (only gate:ask's session-owns-the-run path stamps it), ticketed separately; the match is per-worktree not per-caller, so a forgotten open run gate is a standing allow for any pane in that tree; one-sided symlink coverage, theoretical until the Hogwarts relocation; a pre-existing JSON-escaping gap (tab/newline in a subject or cwd) now duplicated into the new code, not introduced by this PR.

🤖 Generated with Claude Code

…gate

… name Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…e placeholders Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-09-16T01:06:13Z

Warning

Review limit reached

Next included review available in 15 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 82 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3a4d7c21-9165-4afb-b281-93452aec36d9

📥 Commits

Reviewing files that changed from the base of the PR and between 630bd13 and efe23df.

📒 Files selected for processing (2)

lib/__tests__/gate-fork-hook.test.ts
scripts/hooks/gate-fork.sh

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

…ed run gates no longer allow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- fix the codex provisional-session-id note: point at starting the daemon (rt daemon start), not just re-checking rt agent show -- the fallback path never captures a real id on its own - drop --yolo/--no-yolo's misleading `default: false` from the command tree; the real default comes from agent.<provider>.yolo, which can be true - widen the codex herdr session-id poll interval from 500ms to 2s so the 10-minute capture budget spawns ~300 herdr subprocesses instead of up to 1200 Not addressed: the gate-fork.sh pagination finding. That file is from an already-merged, unrelated PR (#290) that only appears in this PR's diff range because this branch predates it; not part of this change.

…ts (#291) * agent-argv: split into provider-dispatched module, add claude --yolo Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * agent-argv: add codex provider argv/pane-command builders Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * settings: replace flat agent.* rows with provider-scoped agent.claude.*/agent.codex.* Replace the four flat settings keys (agent.model, agent.effort, agent.account, agent.extraArgs) with ten provider-scoped rows (agent.provider, plus six claude rows, plus three codex rows). agent.provider defaults to "claude" to preserve existing behavior; the per-provider rows carry no defaults by design, omitting the launch flag when unset. Update the registry test to expect 65 total suite keys (was 59). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * state: add agents.yolo column and updateAgentSessionId codex mints its own session id and never accepts an externally-chosen one, so rt overwrites its own placeholder once the real one is captured. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * rt agent: add --provider and --yolo flags Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * daemon: resolve rt agent provider from payload/settings, dispatch argv per provider Resolves agent.provider (payload > setting > claude default), rejects --account for codex, and threads model/effort/extraArgs/yolo through provider-scoped agent.<provider>.* settings. rec.provider/rec.yolo are now set from resolved values instead of the prior hardcoded "claude". launch() dispatches through buildAgentArgv/buildAgentPaneCommand instead of calling the claude-only builders directly, and the inv literal now carries yolo through to the argv/pane-command builders so --yolo actually reaches the launched process. * agent: capture codex's real session id (headless --json stream, herdr agent get) Step 0 verified both guessed field names against real runs and both were wrong: - codex exec --json emits {"type":"thread.started","thread_id":"<uuid>"} (confirmed with codex-cli), not a top-level "session_id" on some line. extractSessionId now scans for thread_id instead. - herdr agent get reports the id at result.agent.agent_session.value (confirmed with a real herdr pane running codex), not result.agentSessionId / agentSessionId. It only appears once the pane's first prompt has been sent to codex; a freshly launched, unprompted pane reports agent_status without an agent_session key at all. Also: lib/state/index.ts's barrel did not re-export updateAgentSessionId from agents-store.ts even though the function exists there (needed to add it), and TextDecoderStream's WritableStream<BufferSource> vs ReadableStream<Uint8Array>'s bun-types disagreed just enough to fail `bunx tsc --noEmit`; added a narrow cast to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * agent: fix misleading codex herdr session-id warning message The warning fired whenever herdrAgentSessionId timed out named only one cause (herdr integration missing), but a herdr-surface codex launch with no prompt times out the same way -- agent_session never populates until codex completes a turn, integration or no. Reword to name both causes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * agent: whole-branch review fixes for the codex provider Critical/Important: - scope the codex herdr session-id poll to the runner the launch used, so --bg and herd-spawned workers stop polling the ambient herdr server - raise that poll's budget to 10 minutes (AGENT_WAIT_TIMEOUT_MS precedent); herdr only reports the id after the pane's first turn completes - skip session-id capture in the daemon-down CLI fallback via a new skipSessionCapture option; a detached poll never belongs in a short-lived process - document that a codex start's sessionId is provisional, and say so in rt agent start's non-JSON output - gate the chat-handle reservation and the gate-fork --settings file on provider === claude; codex reads neither - herd:spawn pins provider claude, so a global agent.provider = codex cannot silently degrade herd's chat/gate machinery Minor: - cancel, not just release, extractSessionId's tee branch on the early return - .catch on both detached capture chains - persist an explicit yolo false; add --no-yolo to opt out of the setting - provider-aware "requires a prompt" error - render provider and yolo in rt agent list/show - --provider/--yolo/--no-yolo in lib/command-tree-def.ts, stale agent.* hints repointed at the provider-scoped keys, generated reference regenerated - comment corrections: agent-argv path, yolo-on-resume, result-file format, extractSessionId's actual match rule, misleading codex argv test name Tests: the socket-scoped/injected runner paths, the settings-fallback resolution for provider/model/effort/extraArgs/yolo, the codex handle/hook skips, herd's pinned provider, and the fallback's suppressed poll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent tests: resolve the codex capture poll so it does not outlive the test An okRunner answers `agent get` with no agent_session, so the codex handle/hook test left a 500ms poll spinning for its full ten-minute budget after the test returned. codexRunner answers with a session id instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent: keep the RT-46 legacy-path guard green; use rtDir() in the new test The gate-fork docblock spelled the hook directory as a literal home-relative path, which lib/__tests__/rt-paths.test.ts scans for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent: address CodeRabbit findings on PR #291 - fix the codex provisional-session-id note: point at starting the daemon (rt daemon start), not just re-checking rt agent show -- the fallback path never captures a real id on its own - drop --yolo/--no-yolo's misleading `default: false` from the command tree; the real default comes from agent.<provider>.yolo, which can be true - widen the codex herdr session-id poll interval from 500ms to 2s so the 10-minute capture budget spawns ~300 herdr subprocesses instead of up to 1200 Not addressed: the gate-fork.sh pagination finding. That file is from an already-merged, unrelated PR (#290) that only appears in this PR's diff range because this branch predates it; not part of this change. --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>

* gate-fork hook: harness routing + red repro of RT-162 finding 2 * gate-fork hook: allow when this worktree's run already holds an open gate * gate-fork hook: deny message points at rt gate ask * gate-fork hook test: add row-split regression test, fix inverted test name Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * gate-fork hook test: swap identifying fixture strings for neutral acme placeholders Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * gate-fork hook: --open on the run: list fixes the 500-row cliff, parked run gates no longer allow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

…ts (#291) * agent-argv: split into provider-dispatched module, add claude --yolo Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * agent-argv: add codex provider argv/pane-command builders Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * settings: replace flat agent.* rows with provider-scoped agent.claude.*/agent.codex.* Replace the four flat settings keys (agent.model, agent.effort, agent.account, agent.extraArgs) with ten provider-scoped rows (agent.provider, plus six claude rows, plus three codex rows). agent.provider defaults to "claude" to preserve existing behavior; the per-provider rows carry no defaults by design, omitting the launch flag when unset. Update the registry test to expect 65 total suite keys (was 59). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * state: add agents.yolo column and updateAgentSessionId codex mints its own session id and never accepts an externally-chosen one, so rt overwrites its own placeholder once the real one is captured. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * rt agent: add --provider and --yolo flags Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * daemon: resolve rt agent provider from payload/settings, dispatch argv per provider Resolves agent.provider (payload > setting > claude default), rejects --account for codex, and threads model/effort/extraArgs/yolo through provider-scoped agent.<provider>.* settings. rec.provider/rec.yolo are now set from resolved values instead of the prior hardcoded "claude". launch() dispatches through buildAgentArgv/buildAgentPaneCommand instead of calling the claude-only builders directly, and the inv literal now carries yolo through to the argv/pane-command builders so --yolo actually reaches the launched process. * agent: capture codex's real session id (headless --json stream, herdr agent get) Step 0 verified both guessed field names against real runs and both were wrong: - codex exec --json emits {"type":"thread.started","thread_id":"<uuid>"} (confirmed with codex-cli), not a top-level "session_id" on some line. extractSessionId now scans for thread_id instead. - herdr agent get reports the id at result.agent.agent_session.value (confirmed with a real herdr pane running codex), not result.agentSessionId / agentSessionId. It only appears once the pane's first prompt has been sent to codex; a freshly launched, unprompted pane reports agent_status without an agent_session key at all. Also: lib/state/index.ts's barrel did not re-export updateAgentSessionId from agents-store.ts even though the function exists there (needed to add it), and TextDecoderStream's WritableStream<BufferSource> vs ReadableStream<Uint8Array>'s bun-types disagreed just enough to fail `bunx tsc --noEmit`; added a narrow cast to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * agent: fix misleading codex herdr session-id warning message The warning fired whenever herdrAgentSessionId timed out named only one cause (herdr integration missing), but a herdr-surface codex launch with no prompt times out the same way -- agent_session never populates until codex completes a turn, integration or no. Reword to name both causes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * agent: whole-branch review fixes for the codex provider Critical/Important: - scope the codex herdr session-id poll to the runner the launch used, so --bg and herd-spawned workers stop polling the ambient herdr server - raise that poll's budget to 10 minutes (AGENT_WAIT_TIMEOUT_MS precedent); herdr only reports the id after the pane's first turn completes - skip session-id capture in the daemon-down CLI fallback via a new skipSessionCapture option; a detached poll never belongs in a short-lived process - document that a codex start's sessionId is provisional, and say so in rt agent start's non-JSON output - gate the chat-handle reservation and the gate-fork --settings file on provider === claude; codex reads neither - herd:spawn pins provider claude, so a global agent.provider = codex cannot silently degrade herd's chat/gate machinery Minor: - cancel, not just release, extractSessionId's tee branch on the early return - .catch on both detached capture chains - persist an explicit yolo false; add --no-yolo to opt out of the setting - provider-aware "requires a prompt" error - render provider and yolo in rt agent list/show - --provider/--yolo/--no-yolo in lib/command-tree-def.ts, stale agent.* hints repointed at the provider-scoped keys, generated reference regenerated - comment corrections: agent-argv path, yolo-on-resume, result-file format, extractSessionId's actual match rule, misleading codex argv test name Tests: the socket-scoped/injected runner paths, the settings-fallback resolution for provider/model/effort/extraArgs/yolo, the codex handle/hook skips, herd's pinned provider, and the fallback's suppressed poll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent tests: resolve the codex capture poll so it does not outlive the test An okRunner answers `agent get` with no agent_session, so the codex handle/hook test left a 500ms poll spinning for its full ten-minute budget after the test returned. codexRunner answers with a session id instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent: keep the RT-46 legacy-path guard green; use rtDir() in the new test The gate-fork docblock spelled the hook directory as a literal home-relative path, which lib/__tests__/rt-paths.test.ts scans for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * agent: address CodeRabbit findings on PR #291 - fix the codex provisional-session-id note: point at starting the daemon (rt daemon start), not just re-checking rt agent show -- the fallback path never captures a real id on its own - drop --yolo/--no-yolo's misleading `default: false` from the command tree; the real default comes from agent.<provider>.yolo, which can be true - widen the codex herdr session-id poll interval from 500ms to 2s so the 10-minute capture budget spawns ~300 herdr subprocesses instead of up to 1200 Not addressed: the gate-fork.sh pagination finding. That file is from an already-merged, unrelated PR (#290) that only appears in this PR's diff range because this branch predates it; not part of this change. --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>

m4ttheweric and others added 5 commits September 15, 2026 18:47

gate-fork hook: harness routing + red repro of RT-162 finding 2

50c016f

gate-fork hook: allow when this worktree's run already holds an open …

83d224c

…gate

gate-fork hook: deny message points at rt gate ask

14df715

gate-fork hook test: add row-split regression test, fix inverted test…

41e8446

… name Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

gate-fork hook test: swap identifying fixture strings for neutral acm…

338de35

…e placeholders Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

gate-fork hook: --open on the run: list fixes the 500-row cliff, park…

efe23df

…ed run gates no longer allow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

m4ttheweric merged commit 7f27b71 into main Sep 16, 2026
4 checks passed

m4ttheweric deleted the rt-164 branch September 16, 2026 01:34

Navigation Menu

RT-164: gate-fork hook allows the worktree's own open run-gate - #290

RT-164: gate-fork hook allows the worktree's own open run-gate#290
m4ttheweric merged 6 commits into
mainfrom
rt-164

m4ttheweric commented Sep 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 16, 2026 •

edited

Loading

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

m4ttheweric commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Semantic change (deliberate)

Review

Uh oh!

coderabbitai Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

m4ttheweric commented Sep 16, 2026 •

edited

Loading

coderabbitai Bot commented Sep 16, 2026 •

edited

Loading