Widen babysitter intake to routed pull requests - #223
Conversation
📝 WalkthroughWalkthroughThe PR adds routed open-PR discovery, durable per-PR ownership, capacity-bounded babysitter admission, restart recovery, and pre-write opt-out checks. It also extends configuration, prompts, tests, documentation, verification procedures, and feature catalog counts. ChangesRouted PR babysitter
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FactoryLoop
participant Discovery
participant StateStore
participant Babysitter
FactoryLoop->>Discovery: Scan configured repositories
Discovery-->>FactoryLoop: Return eligible PR candidates
FactoryLoop->>StateStore: Claim candidate with lease
StateStore-->>FactoryLoop: Confirm ownership or defer
FactoryLoop->>Babysitter: Spawn standalone PR task
Babysitter->>StateStore: Complete or release claim on exit
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 714a7ba5bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/github/routed-pr-babysitter.test.ts (1)
33-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTighten the fixture and cover a rejecting
listTree.Two small points:
- Line 68 spreads
filesand then overwrites the same key, so the spread has no effect. Pass the single record directly to state the intent.- The fake
listTreenever rejects, so the discovery failure path stays untested. Add a case where one alias root rejects. That case also documents the expected behavior for the per-root failure handling raised insrc/github/routed-pr-babysitter.ts.🧪 Proposed change
const byIdentity = await discoverRoutedPullRequests( - mount({ ...files, '/github/repos/AgentWorkforce__pear/pulls/by-id/7.json': pull() }), + mount({ '/github/repos/AgentWorkforce__pear/pulls/by-id/7.json': pull() }), config({ excludePullRequests: ['AgentWorkforce/pear#7'] }), )Also applies to: 67-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/github/routed-pr-babysitter.test.ts` around lines 33 - 36, Update the fixture setup around mount and the affected test to pass the single file record directly instead of spreading and overwriting files. Extend the MountClient fake or add a focused test using listTree to reject for one alias root, and assert the discovery flow’s expected per-root failure handling in the routed PR babysitter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/github/routed-pr-babysitter.ts`:
- Around line 77-89: Update the root-scanning loop in the repository path around
roots and MountClient.listTree so each root is handled in its own try/catch.
Record the failure for a rejected root, then continue scanning the remaining
alias root instead of aborting the entire repository scan; preserve adding
successful results to paths and the existing failure reporting format.
In `@src/orchestrator/factory.ts`:
- Around line 1697-1705: Update `#sweepRoutedPrBabysitters` to handle failures
from `#runRoutedPrBabysitterSweep` at its own boundary by attaching a catch
handler that logs the error and resolves without rethrowing, while retaining the
existing finally cleanup and in-flight promise deduplication. Ensure callers
such as runOnce and the fire-and-forget invocation in `#handlePrChange` cannot
have routed sweep errors abort the primary loop or become unhandled rejections.
- Around line 5876-5889: Update the routed babysitter exit branch around
`#routedPrBabysitterAgents` and completeRoutedPrBabysitter so it only marks the
durable claim complete when isCompletionReason(reason) is true. For crash,
reconciled-missing, registration-lag, delivery failure, and other non-completion
reasons, preserve the interrupted session instead of calling
completeRoutedPrBabysitter, while retaining cleanup and metrics behavior
consistent with the issue-created counterpart.
- Around line 1679-1695: Wrap the body of `#restoreRoutedPrBabysitterClaims` in a
try/catch so failures from fleet.roster or listRoutedPrBabysitterClaims do not
propagate into `#start` and abort daemon startup. Log a warning with the existing
logger, matching the fail-closed error-handling pattern used by
`#adoptInFlightAgents`, while preserving the current claim-restoration behavior on
success.
- Around line 1785-1806: Track the failed spawn in the per-sweep stats used by
the trailing capacityDeferred calculation: add or increment a dedicated failure
count in the catch around `#spawnRoutedPrBabysitter`, and subtract that count from
the remaining-candidates calculation. Preserve the existing claim release,
cumulative routedPrBabysitterSpawnFailures increment, and warning log behavior.
In `@src/state/file-state-store.ts`:
- Around line 676-695: The routed PR claim mutation must prune completed claims
older than a bounded retention age before checking the existing claim. Apply
this cleanup inside the same mutation lock in src/state/file-state-store.ts
lines 676-695 and persist the pruned document, then apply the identical rule in
src/state/in-memory-state-store.ts lines 639-675 so both stores produce matching
outcomes for aged claims.
---
Nitpick comments:
In `@src/github/routed-pr-babysitter.test.ts`:
- Around line 33-36: Update the fixture setup around mount and the affected test
to pass the single file record directly instead of spreading and overwriting
files. Extend the MountClient fake or add a focused test using listTree to
reject for one alias root, and assert the discovery flow’s expected per-root
failure handling in the routed PR babysitter.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c244e4e9-3bdb-495f-ac7f-83567af69678
📒 Files selected for processing (18)
.agentworkforce/agents/factory-feature-guardian/agent.test.ts.agentworkforce/features/manifest.yaml.agentworkforce/features/verify/procedures.mdREADME.mdsrc/cli/fleet.tssrc/config/schema.test.tssrc/config/schema.tssrc/dispatch/templates.test.tssrc/dispatch/templates.tssrc/github/index.tssrc/github/routed-pr-babysitter.test.tssrc/github/routed-pr-babysitter.tssrc/orchestrator/factory.test.tssrc/orchestrator/factory.tssrc/ports/state.tssrc/state/file-state-store.tssrc/state/in-memory-state-store.tssrc/state/routed-pr-babysitter-claim.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 18 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
714a7ba to
8055748
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/state/in-memory-state-store.ts (1)
640-688: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the shared claim algorithm.
claimRoutedPrBabysitterhere and insrc/state/file-state-store.tslines 655-708 implement the same seven-step algorithm against two different containers (Mapand plain object). The two copies must stay identical, becausesrc/state/routed-pr-babysitter-claim.test.tsasserts equal outcomes for both stores. A shared pure helper that takes get/set/delete/entries accessors would remove the drift risk.This matches the existing dispatch-lifecycle duplication in the same files, so it is optional for this PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/state/in-memory-state-store.ts` around lines 640 - 688, Optionally extract the shared claim algorithm used by InMemoryStateStore.claimRoutedPrBabysitter and FileStateStore.claimRoutedPrBabysitter into one pure helper parameterized by get, set, delete, and entries accessors. Keep the seven-step behavior and outcomes identical for Map and plain-object storage, and update both store methods to delegate to the helper.src/state/routed-pr-babysitter-claim.test.ts (1)
197-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the temporary directories and add two negative cases.
Each
file statetest creates a directory undertmpdir()and never removes it. Add anafterEachorafterAllcleanup that removes the created roots.The suite also does not cover two owner guards that the implementations enforce:
releaseRoutedPrBabysitterClaimreturningtruefor the correct owner, andmarkRoutedPrBabysitterRunningreturningfalsefor a wrong owner. Add those cases so both stores stay aligned on ownership checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/state/routed-pr-babysitter-claim.test.ts` around lines 197 - 203, Update the routed PR babysitter claim tests around verifyClaims to track each file-state temporary root and remove it in afterEach or afterAll. Add coverage for releaseRoutedPrBabysitterClaim returning true for the owning caller and markRoutedPrBabysitterRunning returning false for a non-owner, ensuring both memory and file stores exercise these ownership guards.src/orchestrator/factory.test.ts (1)
16325-16372: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a bound on routed retries.
The test emits one
crashexit and asserts two spawns. It does not assert an upper bound on retries. If the sweep respawned on every crash without a limit, this test would still pass. A crash-looping PR would then consume a work-unit slot forever.Add a case that emits repeated
crashexits and asserts the spawn count stops increasing, or confirm that a retry limit exists insrc/orchestrator/factory.ts.#!/bin/bash # Description: Check for a retry bound on routed PR babysitter respawns. rg -n -C 6 'routedPrBabysitter' src/orchestrator/factory.ts | rg -n -C 6 'attempt|retry|max|limit'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/orchestrator/factory.test.ts` around lines 16325 - 16372, Bound routed PR babysitter respawns in the factory flow, using the retry/attempt tracking around routedPrBabysitter in factory.ts so repeated crash exits stop creating new spawns after the configured maximum. Update the test case to emit repeated crash exits and assert the spawn count no longer increases once the retry limit is reached, while preserving the existing successful-exit completion behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/state/file-state-store.ts`:
- Around line 729-747: Update adoptRoutedPrBabysitterClaim in
src/state/file-state-store.ts (lines 729-747) and
src/state/in-memory-state-store.ts (lines 705-719) to reject running claims with
a live lease unless the existing claim owner matches the adopting owner; permit
adoption only when claim.leaseUntilMs <= nowMs or ownership matches, preserving
the current mutation and persistence behavior.
---
Nitpick comments:
In `@src/orchestrator/factory.test.ts`:
- Around line 16325-16372: Bound routed PR babysitter respawns in the factory
flow, using the retry/attempt tracking around routedPrBabysitter in factory.ts
so repeated crash exits stop creating new spawns after the configured maximum.
Update the test case to emit repeated crash exits and assert the spawn count no
longer increases once the retry limit is reached, while preserving the existing
successful-exit completion behavior.
In `@src/state/in-memory-state-store.ts`:
- Around line 640-688: Optionally extract the shared claim algorithm used by
InMemoryStateStore.claimRoutedPrBabysitter and
FileStateStore.claimRoutedPrBabysitter into one pure helper parameterized by
get, set, delete, and entries accessors. Keep the seven-step behavior and
outcomes identical for Map and plain-object storage, and update both store
methods to delegate to the helper.
In `@src/state/routed-pr-babysitter-claim.test.ts`:
- Around line 197-203: Update the routed PR babysitter claim tests around
verifyClaims to track each file-state temporary root and remove it in afterEach
or afterAll. Add coverage for releaseRoutedPrBabysitterClaim returning true for
the owning caller and markRoutedPrBabysitterRunning returning false for a
non-owner, ensuring both memory and file stores exercise these ownership guards.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 120d73ac-384b-487b-96a3-864ac4a5f9be
📒 Files selected for processing (11)
src/cli/fleet.tssrc/config/schema.test.tssrc/config/schema.tssrc/github/routed-pr-babysitter.test.tssrc/github/routed-pr-babysitter.tssrc/orchestrator/factory.test.tssrc/orchestrator/factory.tssrc/ports/state.tssrc/state/file-state-store.tssrc/state/in-memory-state-store.tssrc/state/routed-pr-babysitter-claim.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/cli/fleet.ts
- src/config/schema.ts
- src/github/routed-pr-babysitter.ts
- src/orchestrator/factory.ts
There was a problem hiding this comment.
All reported issues were addressed across 18 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
5 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/state/in-memory-state-store.ts">
<violation number="1" location="src/state/in-memory-state-store.ts:705">
P1: An agent completing after its lease expires can mark stale work complete and suppress re-admission for 30 days; require a live lease when completing the claim.</violation>
</file>
<file name="src/orchestrator/factory.ts">
<violation number="1" location="src/orchestrator/factory.ts:6035">
P2: An abnormal issue-created babysitter exit leaves its durable claim running after its token is discarded, so a recovered worker's later successful exit cannot complete it. Release `issueClaim` on non-completion before deleting the local entry, matching routed-worker handling.</violation>
<violation number="2" location="src/orchestrator/factory.ts:10738">
P2: On the new `if (!markedRunning)` claim-lost guard in #ensureBabysitter, the code releases the agent and returns without clearing the `#babysitterSpawned`/`#babysitterPr` bookkeeping that was set up just before. Because a subsequent #ensureBabysitter for the same issue+PR short-circuits on `#babysitterSpawned.has(babysitterKey)` (awaiting the in-flight promise and reading the stale `#babysitterPr` entry), the babysitter for that PR is never re-spawned within this process — it stays wedged until a restart. The new fail-closed guard is meant to recover from a lost claim, so it should also roll back the pre-spawn markers (and release the claim if it still exists) so a later event can retry.</violation>
</file>
<file name="src/config/schema.ts">
<violation number="1" location="src/config/schema.ts:134">
P2: The `babysitter.excludePullRequests` regex was changed from `[A-Za-z0-9_.-]{1,100}` to `[A-Za-z0-9_.-]{0,99}` for the repository segment. Lowering the minimum to 0 means the validator now accepts an empty repository segment, e.g. `owner/#1`, which is not a real GitHub identity and can never match a scanned PR (so an operator typo silently does nothing), while simultaneously lowering the max to 99 so a legitimate 100-character repository name (GitHub's documented upper bound) is now rejected as 'expected owner/repo#number'. The added test only covers the 1-character repo `owner/r#1`, which already validated under the previous `{1,100}` range, so this regex edit does not appear necessary to achieve the stated goal. Consider restoring a 1..N range with a required first character (e.g. `[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})` to keep 1–100 while mirroring the owner segment's shape).</violation>
<violation number="2" location="src/config/schema.ts:340">
P3: The new `requireRoutedBabysitterRepos` resolution check only applies the org prefix in its final fallback (`org ? `${org}/${name}` : name`), but the runtime path in `routedPrRepos` (src/github/routed-pr-babysitter.ts) applies the org prefix whenever the resolved value lacks a slash, including when it came from an explicit `byLabel` entry. So a config where `byLabel` maps a name to a bare repo name while `repos.org` is set would be rejected by this schema check ('must resolve at least one owner/repository route') even though `routedPrRepos` would route it as `${org}/${name}` at runtime. Applying the same org-prefix logic to the byLabel/overrides results would keep the schema guard and the runtime scanner consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const claim = this.#workspace(workspaceId).routedPrBabysitterClaims.get(identity) | ||
| if ( | ||
| !claim || | ||
| claim.owner !== owner || |
There was a problem hiding this comment.
P1: An agent completing after its lease expires can mark stale work complete and suppress re-admission for 30 days; require a live lease when completing the claim.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/state/in-memory-state-store.ts, line 705:
<comment>An agent completing after its lease expires can mark stale work complete and suppress re-admission for 30 days; require a live lease when completing the claim.</comment>
<file context>
@@ -691,11 +695,18 @@ export class InMemoryStateStore implements StateStore {
- if (!claim || claim.owner !== owner || claim.status === 'complete') return false
+ if (
+ !claim ||
+ claim.owner !== owner ||
+ claim.claimId !== claimId ||
+ claim.status === 'complete' ||
</file context>
| const issueClaim = this.#issueBabysitterClaims.get(name) | ||
| if (isCompletionReason(reason) && exiting?.spec.role === 'babysitter' && issueClaim) { | ||
| await this.#state.completeRoutedPrBabysitter( | ||
| this.#workspaceId, | ||
| issueClaim.identity, | ||
| this.#dispatchLifecycleOwner, | ||
| issueClaim.claimId, | ||
| name, | ||
| this.#clock.now(), | ||
| ) | ||
| } | ||
| this.#issueBabysitterClaims.delete(name) |
There was a problem hiding this comment.
P2: An abnormal issue-created babysitter exit leaves its durable claim running after its token is discarded, so a recovered worker's later successful exit cannot complete it. Release issueClaim on non-completion before deleting the local entry, matching routed-worker handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/orchestrator/factory.ts, line 6035:
<comment>An abnormal issue-created babysitter exit leaves its durable claim running after its token is discarded, so a recovered worker's later successful exit cannot complete it. Release `issueClaim` on non-completion before deleting the local entry, matching routed-worker handling.</comment>
<file context>
@@ -6008,14 +6032,18 @@ export class FactoryLoop implements Factory {
const exiting = record.agents.get(name)
- if (isCompletionReason(reason) && exiting?.spec.role === 'babysitter' && exiting.spec.ownedPullRequest) {
+ const issueClaim = this.#issueBabysitterClaims.get(name)
+ if (isCompletionReason(reason) && exiting?.spec.role === 'babysitter' && issueClaim) {
await this.#state.completeRoutedPrBabysitter(
</file context>
| const issueClaim = this.#issueBabysitterClaims.get(name) | |
| if (isCompletionReason(reason) && exiting?.spec.role === 'babysitter' && issueClaim) { | |
| await this.#state.completeRoutedPrBabysitter( | |
| this.#workspaceId, | |
| issueClaim.identity, | |
| this.#dispatchLifecycleOwner, | |
| issueClaim.claimId, | |
| name, | |
| this.#clock.now(), | |
| ) | |
| } | |
| this.#issueBabysitterClaims.delete(name) | |
| const issueClaim = this.#issueBabysitterClaims.get(name) | |
| if (exiting?.spec.role === 'babysitter' && issueClaim) { | |
| if (isCompletionReason(reason)) { | |
| await this.#state.completeRoutedPrBabysitter( | |
| this.#workspaceId, | |
| issueClaim.identity, | |
| this.#dispatchLifecycleOwner, | |
| issueClaim.claimId, | |
| name, | |
| this.#clock.now(), | |
| ) | |
| } else { | |
| await this.#state.releaseRoutedPrBabysitterClaim( | |
| this.#workspaceId, | |
| issueClaim.identity, | |
| this.#dispatchLifecycleOwner, | |
| issueClaim.claimId, | |
| ) | |
| } | |
| } | |
| this.#issueBabysitterClaims.delete(name) |
| // before their first provider write. | ||
| excludeLabels: z.array(z.string().trim().min(1)).default(['factory:skip-babysitter']), | ||
| excludePullRequests: z.array(z.string().regex( | ||
| /^[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})\/[A-Za-z0-9_.-]{0,99}#[1-9]\d*$/u, |
There was a problem hiding this comment.
P2: The babysitter.excludePullRequests regex was changed from [A-Za-z0-9_.-]{1,100} to [A-Za-z0-9_.-]{0,99} for the repository segment. Lowering the minimum to 0 means the validator now accepts an empty repository segment, e.g. owner/#1, which is not a real GitHub identity and can never match a scanned PR (so an operator typo silently does nothing), while simultaneously lowering the max to 99 so a legitimate 100-character repository name (GitHub's documented upper bound) is now rejected as 'expected owner/repo#number'. The added test only covers the 1-character repo owner/r#1, which already validated under the previous {1,100} range, so this regex edit does not appear necessary to achieve the stated goal. Consider restoring a 1..N range with a required first character (e.g. [A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99}) to keep 1–100 while mirroring the owner segment's shape).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config/schema.ts, line 134:
<comment>The `babysitter.excludePullRequests` regex was changed from `[A-Za-z0-9_.-]{1,100}` to `[A-Za-z0-9_.-]{0,99}` for the repository segment. Lowering the minimum to 0 means the validator now accepts an empty repository segment, e.g. `owner/#1`, which is not a real GitHub identity and can never match a scanned PR (so an operator typo silently does nothing), while simultaneously lowering the max to 99 so a legitimate 100-character repository name (GitHub's documented upper bound) is now rejected as 'expected owner/repo#number'. The added test only covers the 1-character repo `owner/r#1`, which already validated under the previous `{1,100}` range, so this regex edit does not appear necessary to achieve the stated goal. Consider restoring a 1..N range with a required first character (e.g. `[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})` to keep 1–100 while mirroring the owner segment's shape).</comment>
<file context>
@@ -131,7 +131,7 @@ const babysitterSchema = z.object({
excludeLabels: z.array(z.string().trim().min(1)).default(['factory:skip-babysitter']),
excludePullRequests: z.array(z.string().regex(
- /^[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})\/[A-Za-z0-9_.-]{1,100}#[1-9]\d*$/u,
+ /^[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})\/[A-Za-z0-9_.-]{0,99}#[1-9]\d*$/u,
'expected owner/repo#number',
)).default([]),
</file context>
| /^[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})\/[A-Za-z0-9_.-]{0,99}#[1-9]\d*$/u, | |
| /^[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})\/[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,99})#[1-9]\d*$/u, |
| tracked.result?.name ?? trackedName, | ||
| this.#clock.now(), | ||
| ) | ||
| if (!markedRunning) { |
There was a problem hiding this comment.
P2: On the new if (!markedRunning) claim-lost guard in #ensureBabysitter, the code releases the agent and returns without clearing the #babysitterSpawned/#babysitterPr bookkeeping that was set up just before. Because a subsequent #ensureBabysitter for the same issue+PR short-circuits on #babysitterSpawned.has(babysitterKey) (awaiting the in-flight promise and reading the stale #babysitterPr entry), the babysitter for that PR is never re-spawned within this process — it stays wedged until a restart. The new fail-closed guard is meant to recover from a lost claim, so it should also roll back the pre-spawn markers (and release the claim if it still exists) so a later event can retry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/orchestrator/factory.ts, line 10738:
<comment>On the new `if (!markedRunning)` claim-lost guard in #ensureBabysitter, the code releases the agent and returns without clearing the `#babysitterSpawned`/`#babysitterPr` bookkeeping that was set up just before. Because a subsequent #ensureBabysitter for the same issue+PR short-circuits on `#babysitterSpawned.has(babysitterKey)` (awaiting the in-flight promise and reading the stale `#babysitterPr` entry), the babysitter for that PR is never re-spawned within this process — it stays wedged until a restart. The new fail-closed guard is meant to recover from a lost claim, so it should also roll back the pre-spawn markers (and release the claim if it still exists) so a later event can retry.</comment>
<file context>
@@ -10660,13 +10727,22 @@ export class FactoryLoop implements Factory {
tracked.result?.name ?? trackedName,
this.#clock.now(),
)
+ if (!markedRunning) {
+ await this.#fleet.release(tracked.result?.name ?? trackedName, 'pr-work-claim-lost')
+ return
</file context>
| } | ||
| if (cfg.babysitter.mode === 'routed-open-prs') { | ||
| const routedRepos = (cfg.repos.names ?? []).map((name) => | ||
| cfg.repos.byLabel[name] ?? cfg.repos.overrides[name] ?? |
There was a problem hiding this comment.
P3: The new requireRoutedBabysitterRepos resolution check only applies the org prefix in its final fallback (org ? ${org}/${name} : name), but the runtime path in routedPrRepos (src/github/routed-pr-babysitter.ts) applies the org prefix whenever the resolved value lacks a slash, including when it came from an explicit byLabel entry. So a config where byLabel maps a name to a bare repo name while repos.org is set would be rejected by this schema check ('must resolve at least one owner/repository route') even though routedPrRepos would route it as ${org}/${name} at runtime. Applying the same org-prefix logic to the byLabel/overrides results would keep the schema guard and the runtime scanner consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config/schema.ts, line 340:
<comment>The new `requireRoutedBabysitterRepos` resolution check only applies the org prefix in its final fallback (`org ? `${org}/${name}` : name`), but the runtime path in `routedPrRepos` (src/github/routed-pr-babysitter.ts) applies the org prefix whenever the resolved value lacks a slash, including when it came from an explicit `byLabel` entry. So a config where `byLabel` maps a name to a bare repo name while `repos.org` is set would be rejected by this schema check ('must resolve at least one owner/repository route') even though `routedPrRepos` would route it as `${org}/${name}` at runtime. Applying the same org-prefix logic to the byLabel/overrides results would keep the schema guard and the runtime scanner consistent.</comment>
<file context>
@@ -333,6 +333,20 @@ const requireRoutedBabysitterRepos = (
+ }
+ if (cfg.babysitter.mode === 'routed-open-prs') {
+ const routedRepos = (cfg.repos.names ?? []).map((name) =>
+ cfg.repos.byLabel[name] ?? cfg.repos.overrides[name] ??
+ (cfg.repos.org ? `${cfg.repos.org}/${name}` : name)
+ )
</file context>
1 participant
Closes #222.
What changed
babysitter.mode: routed-open-prsintake that scans only repositories named byrepos.names.workspace + owner/repo#PR; issue-created and routed intake share the same claim before spawn.factory:skip-babysitterlabel and configuredexcludePullRequestsidentities. Automatic prompts re-check labels before every provider write.batchSize, admits at most one new PR per sweep, renews active leases, and logs exact discovery/admission/defer/failure counts.notifyHumans: false.The volume controls are grounded in the 2026-08-07 GitHub API measurement recorded in #222: 302 open PRs, 248 ready for review, across 41 repositories.
Safety and sequencing
This PR is safe to land before #221 only because
factory-createdremains the default mode. Do not enablerouted-open-prsuntil #221's GitHub App identity path is deployed and validated; otherwise broad automated writes could still be attributed to the local user.No live sweep was run and no other PR was written to. The widened discovery path is independent of issue triage/dispatch and therefore does not assume the currently degraded dispatcher or Relayfile WebSocket path works.
Validation
All checks are reported by exit code:
npm run build— exit 0npm run featuremap:check— exit 0npm test -- --run— exit 0; 80 files, 1,443 tests passedgit diff --check— exit 0The tests cover routed-only scope, incomplete metadata fail-closed behavior, label/config opt-out before spawn, notification suppression, atomic contention across memory and file stores, capacity, revision-based re-admission, and issue/routed integration.
Summary by cubic
Adds a new
routed-open-prsbabysitter mode to adopt existing open PRs from routed repos, with durable PR-scoped claims shared with the issue path and generation-fenced leases to prevent stale transitions. Addresses Linear #222 while keeping the default issue-driven mode unchanged and notifications off unless explicitly enabled.New Features
babysitter.mode: routed-open-prsscans onlyrepos.names; admits open, non-draft, same-repo PRs, and uses the PR title/body as the spec when no issue lifecycle exists.owner/repo#number, with restart-safe leases, claimId-fenced generations, guarded adoption (no stealing live leases), and cross-path conflict suppression.batchSize, renews leases, starts at most one new routed PR per sweep, and logs detailed discovery/admission/deferral counters.babysitter.excludeLabels(defaultfactory:skip-babysitter) andbabysitter.excludePullRequests; routed agents re-check labels before their first and every later provider write;babysitter.notifyHumansdefaults to false; explicit CLI babysits keep notifications on; task template includes opt-out and notification flags.repos.namesto resolve at least oneowner/repo.Migration
factory-createdremains the default.babysitter.enabled: true,babysitter.mode: routed-open-prs, and ensurerepos.namesresolves routed repos (optionally setexcludePullRequests,excludeLabels,notifyHumans).dryRun: true) and do not enable routed intake until the GitHub App identity path from [factory] The babysitter writes to GitHub as the local gh user while Factory's own PRs are authored by the app #221 is deployed and validated.Written for commit fd3769d. Summary will update on new commits.