fix: route Slack dispatch replies to babysitters - #287
Conversation
|
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. |
|
Caution Review failedAn error occurred during the review process. Please try again later. 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: 21b31ab3d3
ℹ️ 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".
![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 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
7d8af75 to
3f047fe
Compare
There was a problem hiding this comment.
2 issues found across 9 files
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/orchestrator/factory.ts">
<violation number="1" location="src/orchestrator/factory.ts:13306">
P1: When a work unit reopens while a terminal-thread reply is still routing, this call removes the terminal fence without waiting for that route. The old reply can then be durably queued for the new dispatch; stop and drain in-flight Slack routes before clearing the terminal fence and creating the new conversation.</violation>
<violation number="2" location="src/orchestrator/factory.ts:14274">
P2: If Slack writeback is unavailable during startup, this undelivered-reply receipt aborts `#rearmSlackReplyWatchers` before it re-arms the remaining threads. Handle this terminal watch's receipt failure as retryable state maintenance and continue rehydrating other watchers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // A reopened work unit needs a fresh dispatch notification and a fresh | ||
| // conversation. Do not let the old grace-period watcher (or its expiry | ||
| // timer) capture and later tear down the new dispatch. | ||
| await this.#stopSlackWatcher(record.issue) |
There was a problem hiding this comment.
P1: When a work unit reopens while a terminal-thread reply is still routing, this call removes the terminal fence without waiting for that route. The old reply can then be durably queued for the new dispatch; stop and drain in-flight Slack routes before clearing the terminal fence and creating the new conversation.
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 13306:
<comment>When a work unit reopens while a terminal-thread reply is still routing, this call removes the terminal fence without waiting for that route. The old reply can then be durably queued for the new dispatch; stop and drain in-flight Slack routes before clearing the terminal fence and creating the new conversation.</comment>
<file context>
@@ -13289,6 +13297,14 @@ export class FactoryLoop implements Factory {
+ // A reopened work unit needs a fresh dispatch notification and a fresh
+ // conversation. Do not let the old grace-period watcher (or its expiry
+ // timer) capture and later tear down the new dispatch.
+ await this.#stopSlackWatcher(record.issue)
+ }
const existingThread = await this.#persistedSlackThread(key)
</file context>
| this.#terminalSlackWatchIssues.add(key) | ||
| const conversationId = slackConversationId(watch.threadId) | ||
| await this.#slackConversationTurns.cancel(conversationId) | ||
| await this.#surfaceUndeliveredSlackConversation(watch.threadId) |
There was a problem hiding this comment.
P2: If Slack writeback is unavailable during startup, this undelivered-reply receipt aborts #rearmSlackReplyWatchers before it re-arms the remaining threads. Handle this terminal watch's receipt failure as retryable state maintenance and continue rehydrating other watchers.
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 14274:
<comment>If Slack writeback is unavailable during startup, this undelivered-reply receipt aborts `#rearmSlackReplyWatchers` before it re-arms the remaining threads. Handle this terminal watch's receipt failure as retryable state maintenance and continue rehydrating other watchers.</comment>
<file context>
@@ -14214,6 +14255,33 @@ export class FactoryLoop implements Factory {
+ this.#terminalSlackWatchIssues.add(key)
+ const conversationId = slackConversationId(watch.threadId)
+ await this.#slackConversationTurns.cancel(conversationId)
+ await this.#surfaceUndeliveredSlackConversation(watch.threadId)
+ await this.#state.clearConversationSession(this.#workspaceId, conversationId)
+ await this.#rearmSlackWatcher(watchRecord, watch.threadId, {
</file context>
1 participant
Fixes #273
Fixes #227
Summary
fleet.resumecontext instead of Relay DM injection, and post a visibly acknowledged receipt only after durable enqueue succeedsDiscriminating coverage
The new triage-restart, receipt-retry, and missing-owner long-reply tests were applied test-only to
eaaa076; all failed there as expected before the implementation.Verification
npm run buildnpm run featuremap:checkgit diff --checkThe repository-requested Veto MCP diff review was unavailable in this worker lane, so the checks above and a manual diff review are the fallback review record.