fix(factory): park completed PRs in human review#362
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
More reviews will be available in 9 minutes and 44 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 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 |
![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: b64adb1588
ℹ️ 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".
| this.#postMergeDoneAdvances.add(advanceKey) | ||
|
|
||
| try { | ||
| await this.#linear.setState(issue, this.#config.stateIds.done) |
There was a problem hiding this comment.
When config.dryRun is enabled and a merged PR event arrives for an existing Human Review/Agent Implementing issue that is not currently in #batch, this new post-merge path still calls setState and can also post Slack messages. Other external write paths, such as closed GitHub issue handling, explicitly skip writes under dry-run, so a dry-run live daemon can now mutate Linear just by observing a merged PR webhook.
Useful? React with 👍 / 👎.
| const humanReview = opts.targetState !== 'done' && | ||
| this.#config.terminalState === 'human-review' && | ||
| Boolean(this.#config.stateIds.humanReview) |
There was a problem hiding this comment.
With terminalState: 'human-review' and stateIds.humanReview configured, this default now sends every #completeIssue() caller that omits targetState to Human Review. That includes #handleGithubIssueChange when a mirrored GitHub issue closes while its Linear mirror is in-flight; the source issue is already closed, so the mirror should be completed to Done, not parked for human review with agents released as issue-human-review.
Useful? React with 👍 / 👎.
|
Addressed the blocking review item: Also kept the follow-up refinements for the non-blocking comments:
Verification rerun:
|
khaliqgant
force-pushed
the
ar-273-human-review-terminal-state
branch
from
9fa7856 to
5594a68
Compare
|
ℹ️ 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. Findings
I did not auto-edit either issue because both touch semantic completion/writeback behavior in safety-sensitive lifecycle code. Addressed comments
Verification Local commands passed: GitHub reports PR #362 is open, mergeable, not draft, and CI run |
|
Findings
Addressed comments
Advisory Notes
Verification Passed locally after a clean
I did not print |
|
Findings
Addressed Comments
Verification No files were edited. Passed locally: GitHub API reports PR #362 is open, non-draft, mergeable with |
Merge origin/main (#360 webhook handler, #362 park completed PRs). The ar-366 re-dispatch and ar-243 PR-state sweep tests came from main with pre-label-routing implementer names; under AR-274 label routing the implementer is repo-slug suffixed (ar-366-impl-pear / ar-243-impl-pear). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* AR-274 route implementers by repo labels * fix(factory): fail dispatch without repo labels * AR-274 rebind reviewer to label route * AR-274 require repo labels for dispatch (no default-route fallback) Remove defaultRouteAssignment/repoSlug so an unlabeled or unmapped issue fails dispatch loudly instead of silently routing to repos.default. Cap label implementers at MAX_LABEL_IMPLEMENTERS=4 and rebind the reviewer to the first label route via routeReviewerSpec(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(factory): green CI + dedupe invalid-label dispatch comments - fleet.test.ts: implementer name is now repo-slug suffixed (ar-77-impl-pear) under label routing. - Dedupe the invalid-label dispatch failure comment by reason+offending-labels signature so a stuck Ready issue (or the writeback's own change event) no longer re-posts the same notice every cycle; clear on successful dispatch. Addresses Codex review on PR #363. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(factory): suffix implementer names for merged-in main tests Merge origin/main (#360 webhook handler, #362 park completed PRs). The ar-366 re-dispatch and ar-243 PR-state sweep tests came from main with pre-label-routing implementer names; under AR-274 label routing the implementer is repo-slug suffixed (ar-366-impl-pear / ar-243-impl-pear). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
stateIds.humanReviewis omittedReview Notes
LINEAR_STATE_IDSis documented as the AR-team state catalog;FactoryConfigstill omitshumanReviewfrom the defaultstateIds, so non-AR operators must opt into their own review-state UUID beforeterminalState: "human-review"changes behavior.babysitter.enabledis false so merged PRs can advance review-state issues to Done. Legacy direct-to-Done configurations should be unaffected because the post-merge handler only advances issues still in Agent Implementing or Human Review.Tests
npx vitest run packages/factory-sdk/src/config/schema.test.ts packages/factory-sdk/src/orchestrator/factory.test.tsE2E Notes
Do NOT auto-merge. Merge policy: never.