Remove Mergeable check and fix Devin status polling#2072
Conversation
- Remove bot-ci.ts which created the 'Mergeable: bot_ci' check - Remove related tests and fixtures for bot_ci handler - Fix Devin status polling to reconcile stuck checks on startup - Now fetches all sessions (not just running) on startup - Updates checks for finished/expired sessions immediately - Only tracks working/blocked sessions for ongoing polling - Update vitest config to pass with no tests Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR removes the bot-ci feature handler and associated infrastructure while refactoring the Devin poller to conditionally handle sessions by their status (Working, Blocked, Finished, Expired) with corresponding check updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/bot/src/devin/poller.ts (1)
208-246: Consider extracting status handling to reduce duplication.The status-based branching logic here (Finished → success, Expired → cancelled, other → neutral) is repeated in
checkPRStatus(lines 339-375 and 415-443). This duplication increases maintenance burden and risks divergence.Consider extracting a helper method:
private getCheckOutputForStatus( status: DevinSessionStatus | undefined, sessionId: string ): { status: "in_progress" | "completed"; conclusion?: "success" | "cancelled" | "neutral"; output: { title: string; summary: string } } | null { switch (status) { case DevinSessionStatus.Working: return { status: "in_progress", output: { title: "Devin is working", summary: `Devin session ${sessionId} is currently working on this PR.` } }; case DevinSessionStatus.Blocked: return { status: "in_progress", output: { title: "Devin is blocked", summary: `Devin session ${sessionId} is blocked and waiting for input.` } }; case DevinSessionStatus.Finished: return { status: "completed", conclusion: "success", output: { title: "Devin finished", summary: `Devin session ${sessionId} has completed.` } }; case DevinSessionStatus.Expired: return { status: "completed", conclusion: "cancelled", output: { title: "Devin session expired", summary: `Devin session ${sessionId} has expired.` } }; default: return status ? { status: "completed", conclusion: "neutral", output: { title: "Devin session ended", summary: `Devin session ${sessionId} ended with status: ${status}` } } : null; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/bot/src/devin/poller.ts(2 hunks)apps/bot/src/features/bot-ci.ts(0 hunks)apps/bot/src/features/index.ts(0 hunks)apps/bot/src/index.ts(0 hunks)apps/bot/test/fixtures/check_run.completed.failure.json(0 hunks)apps/bot/test/fixtures/check_run.completed.success.json(0 hunks)apps/bot/test/fixtures/check_run.created.json(0 hunks)apps/bot/test/index.test.ts(0 hunks)apps/bot/vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (7)
- apps/bot/src/features/index.ts
- apps/bot/test/index.test.ts
- apps/bot/src/index.ts
- apps/bot/src/features/bot-ci.ts
- apps/bot/test/fixtures/check_run.completed.success.json
- apps/bot/test/fixtures/check_run.created.json
- apps/bot/test/fixtures/check_run.completed.failure.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/bot/src/devin/poller.tsapps/bot/vitest.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/bot/src/devin/poller.tsapps/bot/vitest.config.ts
**/*.config.{ts,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Agent configuration should be centralized and externalized from implementation logic
Files:
apps/bot/vitest.config.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:24.348Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/desktop-e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:24.348Z
Learning: Applies to apps/desktop-e2e/**/*.{test,spec}.{js,ts,tsx} : Refer to `scripts/setup-desktop-e2e.sh` for end-to-end test environment setup if setup is missing
Applied to files:
apps/bot/vitest.config.ts
📚 Learning: 2025-11-24T16:32:13.593Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: packages/nango/.cursor/rules/nango.mdc:0-0
Timestamp: 2025-11-24T16:32:13.593Z
Learning: Applies to packages/nango/**/*.ts : Integration must pass `nango dryrun` validation testing successfully
Applied to files:
apps/bot/vitest.config.ts
🧬 Code graph analysis (1)
apps/bot/src/devin/poller.ts (1)
apps/bot/src/devin/list.ts (1)
listDevinSessions(28-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: Devin
- GitHub Check: fmt
🔇 Additional comments (3)
apps/bot/vitest.config.ts (1)
7-7: CI will pass even if no tests are discoveredAdding
passWithNoTests: truealigns with the bot test suite removal, but note that CI will silently succeed in the future if tests are accidentally not discovered for this package. Confirm whether this is intentional forapps/bot's long-term testing strategy.apps/bot/src/devin/poller.ts (2)
171-171: LGTM!Removing the status filter allows discovering sessions in any state, enabling the new logic to properly handle Finished/Expired sessions by updating their check status immediately during discovery.
198-212: Clean refactoring of the discovery flow.The pattern of constructing
trackedPRupfront and then conditionally tracking or updating based on status is clear and maintainable. Only active sessions (Working/Blocked) are tracked for ongoing polling.
Summary
This PR makes two changes to the bot:
Removes the "Mergeable: bot_ci" check feature - The entire
bot-ci.tshandler that created a mirror check called "Mergeable: bot_ci" has been removed, along with its tests and fixtures.Fixes stuck "Devin is working" checks - Modified
discoverExistingDevinPRs()to reconcile checks for sessions that finished while the bot was down. Previously, the startup discovery only looked forstatus: "running"sessions, so if a Devin session finished during a bot restart, the GitHub check would stay stuck as "in_progress" forever. Now it fetches all recent sessions and immediately updates checks for finished/expired sessions.Review & Testing Checklist for Human
listDevinSessions({ status: "running" })tolistDevinSessions({})could return significantly more sessions. Verify this doesn't cause rate limiting or performance issues.Notes
The test file was removed entirely because all tests were for the removed bot_ci feature. Added
passWithNoTests: trueto vitest config so CI doesn't fail.Link to Devin run: https://app.devin.ai/sessions/cd2064dd2b5943d0ac9947bec873a4c1
Requested by: yujonglee (@yujonglee)