Skip to content

Add PR-state completion sweep#287

Merged
kjgbot merged 4 commits into
mainfrom
factory-sdk-pr-state-completion-sb-impl3
Jun 13, 2026
Merged

Add PR-state completion sweep#287
kjgbot merged 4 commits into
mainfrom
factory-sdk-pr-state-completion-sb-impl3

Conversation

@kjgbot

@kjgbot kjgbot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a periodic PR-state completion sweep for in-flight factory issues in live mode and runLoop
  • use the branch-aware PR resolver to trigger existing #completeIssue, preserving mergePolicy and close-never-merge behavior
  • coalesce concurrent completion triggers and skip draft/wrong PRs

Validation

  • npm run typecheck:node
  • npx vitest run packages/factory-sdk/src/orchestrator/factory.test.ts --testNamePattern "PR-state sweep|live mode runs the PR-state"
  • npx vitest run packages/factory-sdk
  • rm -rf /Users/khaliqgant/Projects/AgentWorkforce/pear/node_modules/.cache/pear-factory-sdk

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@kjgbot kjgbot added the no-agent-relay-review Disable agent-relay automated PR review/fixes label Jun 13, 2026
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kjgbot, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 58 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d59cfde-0c48-41bb-b021-b97e78089e31

📥 Commits

Reviewing files that changed from the base of the PR and between 1f478a0 and b4d4c49.

📒 Files selected for processing (9)
  • packages/factory-sdk/src/cli/fleet.ts
  • packages/factory-sdk/src/github/index.ts
  • packages/factory-sdk/src/github/merge-gate.ts
  • packages/factory-sdk/src/github/probe-closer.test.ts
  • packages/factory-sdk/src/github/probe-closer.ts
  • packages/factory-sdk/src/index.ts
  • packages/factory-sdk/src/orchestrator/factory.test.ts
  • packages/factory-sdk/src/orchestrator/factory.ts
  • packages/factory-sdk/src/types.ts
📝 Walkthrough

Walkthrough

FactoryLoop gains a periodic PR-state completion sweep that finds non-draft completion PRs for in-flight issues, closes/releases them idempotently (per-issue dedupe), respects merge policies/merge-gate readiness, and is covered by end-to-end tests including timer and concurrency scenarios.

Changes

PR Completion Sweep with Draft-Aware Concurrency

Layer / File(s) Summary
Completion sweep types, constants, and helpers
packages/factory-sdk/src/orchestrator/factory.ts
ResolvedIssuePr type carries optional draft flag; scheduling/batching constants control sweep timing/batch size; booleanValue helper parses untyped boolean probe fields.
Completion sweep runtime state
packages/factory-sdk/src/orchestrator/factory.ts
FactoryLoop adds timer reference, active flag, and per-issue #completionInFlight set; schedules immediate sweep during live startup and clears timer/state on stop.
PR resolution plumbing for draft status
packages/factory-sdk/src/orchestrator/factory.ts
resolveIssuePrFromMount and readProbePrCandidate parse and propagate optional draft status from probe candidates into completion resolution.
Completion sweep execution and lifecycle
packages/factory-sdk/src/orchestrator/factory.ts
#scheduleCompletionSweep and #sweepPrStateCompletions iterate eligible in-flight batch records, resolve completion PRs, skip missing/draft/non-scope results, validate batch-slot ownership before completing, yield between batches, refresh live heartbeat, and run at runLoop start.
Issue completion concurrency deduplication
packages/factory-sdk/src/orchestrator/factory.ts
#completeIssue uses #completionInFlight keyed by issue to early-return on concurrent work, and removes the key in a finally block.
Test infrastructure and completion-sweep coverage
packages/factory-sdk/src/orchestrator/factory.test.ts
Adds prFile() helper and tests covering wedged issue completion, live/backfill timer invocations, concurrency/idempotency, mergePolicy: 'never' release behavior, conditional merge when merge-gate is ready, and skipping unrelated or draft PRs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hopped through PRs both draft and bright,
Closed the wedged ones softly in the night,
Freed batch slots, counted each done,
Kept merges patient till green and fun,
A tidy loop — now everything’s light.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add PR-state completion sweep' is concise and directly describes the main feature being added, clearly summarizing the primary change in the changeset.
Description check ✅ Passed The description provides relevant context about the PR-state completion sweep feature, its purpose, implementation approach, and validation steps performed, all of which relate directly to the changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch factory-sdk-pr-state-completion-sb-impl3

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

❤️ Share

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

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #287 in AgentWorkforce/pear.
The review harness exited with code 1.
No review was posted; this needs operator attention.

1 similar comment
@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #287 in AgentWorkforce/pear.
The review harness exited with code 1.
No review was posted; this needs operator attention.

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

pr-reviewer could not complete review for #287 in AgentWorkforce/pear.
The review harness exited with code 1.
No review was posted; this needs operator attention.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/factory-sdk/src/orchestrator/factory.test.ts (1)

3271-3307: 💤 Low value

Consider verifying the specific counter for the wrong-PR case.

The test correctly verifies that issue 251 (draft PR) increments completionSweepDraftPr. However, issue 250's PR has a low match score ("Unrelated cleanup" title, "docs-cleanup" head_ref), so the PR resolver likely returns undefined, which should increment completionSweepMissingPr (as shown in context snippet 2). Adding a check for this counter would make the test more precise about why completion was skipped for each issue.

✨ Optional enhancement to verify missing-PR counter
   expect(factory.status().inFlight.map((issue) => issue.key)).toEqual(['AR-250', 'AR-251'])
   expect(factory.status().counters.completionSweepCompleted).toBeUndefined()
+  expect(factory.status().counters.completionSweepMissingPr).toBe(1)
   expect(factory.status().counters.completionSweepDraftPr).toBe(1)
 })
🤖 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 `@packages/factory-sdk/src/orchestrator/factory.test.ts` around lines 3271 -
3307, The test case "PR-state sweep does not complete on a wrong PR or draft PR"
should also assert that the wrong/unmatched PR increments the missing-PR
counter: add an expectation that
factory.status().counters.completionSweepMissingPr is 1 (in addition to the
existing check for completionSweepDraftPr === 1) so the test verifies both the
draft-PR and the missing/unmatched-PR branches are counted correctly.
🤖 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.

Nitpick comments:
In `@packages/factory-sdk/src/orchestrator/factory.test.ts`:
- Around line 3271-3307: The test case "PR-state sweep does not complete on a
wrong PR or draft PR" should also assert that the wrong/unmatched PR increments
the missing-PR counter: add an expectation that
factory.status().counters.completionSweepMissingPr is 1 (in addition to the
existing check for completionSweepDraftPr === 1) so the test verifies both the
draft-PR and the missing/unmatched-PR branches are counted correctly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: deee9434-9a5d-40a8-93ea-3087c306ad54

📥 Commits

Reviewing files that changed from the base of the PR and between c652420 and b1d21e8.

📒 Files selected for processing (2)
  • packages/factory-sdk/src/orchestrator/factory.test.ts
  • packages/factory-sdk/src/orchestrator/factory.ts

@kjgbot kjgbot force-pushed the factory-sdk-pr-state-completion-sb-impl3 branch from b1d21e8 to cff639c Compare June 13, 2026 20:41
@agent-relay-code

Copy link
Copy Markdown
Contributor

Reviewed PR #287 and made one scoped fix: added the missing assertion for the wrong-PR branch counter in factory.test.ts.

Addressed comments

  • gemini-code-assist[bot] quota warning: no actionable code finding; skipped.
  • coderabbitai[bot] walkthrough/tool note: ESLint install had failed in its environment; I validated locally with npm ci and npm run lint, no code change needed.
  • agent-relay-code[bot] harness exited code 1 comment 4699705650: no specific code finding; local PR review and verification completed.
  • agent-relay-code[bot] harness exited code 1 comment 4699713428: duplicate of prior harness failure; no code change needed.
  • agent-relay-code[bot] review-only/no changes comment 4699714128: no actionable code finding; local change was applied after validating current checkout.
  • coderabbitai[bot] nitpick on missing completionSweepMissingPr assertion: fixed in factory.test.ts.

Advisory Notes

None.

Verification

Passed:

  • npx vitest run packages/factory-sdk/src/orchestrator/factory.test.ts
  • npm run verify:mcp-resources-drift && npm run lint && npm run typecheck:web && npm run typecheck:node && npm test && npx vitest run && npm run build
  • npx playwright install --with-deps chromium && npm run build:web && npx playwright test --config playwright.fidelity.config.ts && npx playwright test --config playwright.redraw.config.ts
  • final npm run verify:mcp-resources-drift

PR metadata reports mergeable, but I am not printing READY because remote CI for the post-edit commit is not verifiable until the harness applies the local file edit.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #287 in AgentWorkforce/pear.
The review harness exited with code 1.
No review was posted; this needs operator attention.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Implemented two scoped fixes:

  • Scheduled the PR completion sweep for default backfill-and-subscribe startup too, not just live mode: factory.ts
  • Added regression coverage for that default start path: factory.test.ts
  • Added CodeRabbit’s requested missing-PR counter assertion: factory.test.ts

Addressed comments

  • gemini-code-assist[bot] quota warning: no code finding was raised; no change needed.
  • coderabbitai[bot] rate-limit/walkthrough comment: no actionable code finding in that comment; no change needed.
  • agent-relay-code[bot] harness failure comment 4699705650: no specific code finding was posted; validated current checkout through local checks below.
  • agent-relay-code[bot] harness failure comment 4699713428: duplicate harness failure notice with no specific code finding; validated current checkout through local checks below.
  • agent-relay-code[bot] push-outcome/harness failure comment 4699714128: no specific code finding was posted; validated current checkout through local checks below.
  • coderabbitai[bot] review nit about verifying completionSweepMissingPr: fixed in factory.test.ts.

Advisory Notes

None.

Local validation

Passed:

  • npm ci
  • npx vitest run packages/factory-sdk/src/orchestrator/factory.test.ts
  • npm run verify:mcp-resources-drift
  • npm run lint with existing warnings only
  • npm run typecheck:web
  • npm run typecheck:node
  • npm test
  • npx vitest run
  • npm run build
  • npm run build:web
  • npx playwright install --with-deps chromium
  • npx playwright test --config playwright.fidelity.config.ts
  • npx playwright test --config playwright.redraw.config.ts

I did not run the macOS dist:mac packaging job because this runner is Linux, and I did not verify GitHub mergeability from here.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Reviewed PR #287 against .workforce/pr.diff, changed files, current checkout, and PR comments. I did not make additional file edits in this run; the current checkout already contains the scoped fixes previously needed.

Addressed comments

  • gemini-code-assist[bot]: quota warning only; no actionable code finding.
  • coderabbitai[bot] walkthrough/tool note: ESLint install failure in CodeRabbit environment; no code finding. Local lint completed with existing warnings only.
  • coderabbitai[bot] nitpick about asserting completionSweepMissingPr: already fixed in factory.test.ts.
  • agent-relay-code[bot] harness failure comments 4699705650, 4699713428, 4699714128, and 4699758001: no specific code finding; validated current checkout locally.
  • agent-relay-code[bot] summary comments 4699750032 and 4699769166: confirmed the referenced fixes exist at factory.ts, factory.test.ts, and factory.test.ts.

Advisory Notes

None.

Local validation

Passed:

  • npx vitest run packages/factory-sdk/src/orchestrator/factory.test.ts
  • npm run verify:mcp-resources-drift && npm run lint && npm run typecheck:web && npm run typecheck:node && npm test && npx vitest run && npm run build
  • npx playwright install --with-deps chromium && npm run build:web && npx playwright test --config playwright.fidelity.config.ts && npx playwright test --config playwright.redraw.config.ts && npm run verify:mcp-resources-drift
  • Post-resource-kill smoke: npm run verify:mcp-resources-drift and the focused PR-state test

Caveat: repeated npm ci attempts were killed by this sandbox with exit 137 after installing for a while. The installed tree was usable for the validation above, but I am not claiming the install step itself completed cleanly here. GitHub metadata reports the PR mergeable, but remote required checks were not fully visible through the connector, so I am not printing READY.

@kjgbot kjgbot merged commit 96e6324 into main Jun 13, 2026
5 checks passed
@kjgbot kjgbot deleted the factory-sdk-pr-state-completion-sb-impl3 branch June 13, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-agent-relay-review Disable agent-relay automated PR review/fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant