Repository navigation
Conversation
…und the draft cache, ease far-off polling From the review bot's passes over #15403: - useScheduledSend.schedule returned quietly on a non-200 answer. The API client throws on error statuses, so only another 2xx got there, but the composer had already cleared the message, so it was lost without a word. It fails like any other error now, and the message goes back. - useChatDraft's in-memory copy kept every draft for the tab's life and was read before session storage; it now holds only drafts storage refused. - useAtResetJobs read the list every minute however far off the turn was; a turn more than an hour away is read every 15 minutes. Fixes SECRT-2959. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
ntindle
requested review from
0ubbe and
Abhi1992002
and removed request for
a team
October 16, 2026 09:23
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
github-actions
Bot
added
platform/frontend
Member
Author
|
/review |
|
🤖 Review of |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
WalkthroughThe changes update chat draft storage fallback behavior, adjust scheduled-job polling intervals, and treat scheduling responses other than status 200 as failures. Tests cover the draft fallback, polling interval, and retained composer draft. ChangesChat draft storage
Scheduled-send behavior
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the three main changes: handling failed send-at-reset scheduling, limiting draft caching, and reducing polling for distant jobs. Description check The description directly explains the motivation, implementation, tests, and validation for all changes in the pull request. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
github-actions
Bot
added
cla: signed
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #15591 +/- ##
==========================================
- Coverage 86.87% 86.86% -0.01%
==========================================
Files 4947 4947
Lines 378328 378329 +1
Branches 33657 33659 +2
==========================================
- Hits 328656 328650 -6
- Misses 44196 44204 +8
+ Partials 5476 5475 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Open
14 tasks done
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Verdict: Approved — No findings require changes before approval; this review counts as an approval.
Three well-scoped copilot bug fixes (fail a non-200 schedule answer, bound the in-memory draft cache, ease far-off at-reset polling) each traced correctly through callers and helpers and each backed by a regression test that fails on dev. No blockers, no security/auth/data-exposure impact, no copy-policy issues.
Risk level: low | Human review: not required | Duration: 297s | Reviewed: 1eec668c
Findings: 🔴 0 blockers | 🟠 0 should fix | 🟡 2 nice to have | 🔵 0 nits
Optional advice — does not block approval
🟡 Nice to Have
- 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/components/SendAtReset/__tests__/useScheduledSend.test.tsx:193Optionally assert the failure toast copy in the non-200 schedule test — The test confirms the draft is repopulated on a non-200 answer but does not assert the recovery toast copy from describeSendFailure; a regression dropping the toast would pass here.
Suggestion: Optionally spy on toastSendFailure and assert the 'Your message is back in the composer' message. Non-blocking — the toast path is covered elsewhere. - 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/components/SendAtReset/useAtResetJobs.ts:55Consider tiering the polling ceiling beyond a day — A day-out turn still polls ~96x/day under the fixed 15-min ceiling. Advisory only; the change is already a large reduction over the prior per-minute cadence.
Suggestion: If it ever matters, scale the ceiling with untilDue for far-future turns.
GitHub CI on reviewed head: success
Validation and specialist details
Specialist Reports
| Specialist | Status | Summary |
|---|---|---|
| security | ✅ PASS | Well-scoped client-side copilot fix (fail a non-200 schedule, bound the draft cache, ease far-off polling) with no security, auth, or data-exposure impact and no concrete defects. |
| architect | ✅ PASS | Tight, well-scoped fix of three Send-at-reset issues (silent dead branch, stale draft cache, far-off polling cadence) with matching regression tests; no concrete defects found. |
| performance | ✅ PASS | A clean set of three copilot fixes that reduce polling load for far-off scheduled sends, bound a previously unbounded in-memory draft cache, and harden an unreachable error branch — all net-positive for performance and memory with no blocking defects. |
| testing | ✅ PASS | Three focused copilot fixes each backed by a genuine regression test that fails on dev; assertions are meaningful and no defects found. |
| quality | ✅ PASS | A clean, well-tested three-part fix (fail the dead reset branch, bound the draft cache, ease far-off polling) with no concrete defects found. |
| product | ✅ PASS | Three well-scoped copilot fixes (fail-on-non-200 schedule, bounded draft cache, eased far-off polling) all trace correctly through their callers and are covered by tests that fail on dev; no defects or copy issues found. |
| discussion | ✅ PASS | No failing required checks on head 1eec668 (all pending, mergeable) and no open human-reviewer requests in the discussion snapshot. |
| ui-reviewer (local) | ✅ PASS | Three correctly-implemented, well-tested copilot bug fixes (fail-on-non-200 schedule, bounded draft cache, eased far-off polling) with no reachable defect and no human-signoff path touched. |
| ui-reviewer (hosted) | ✅ PASS | Three correct, well-tested copilot hook fixes (fail-on-non-200 schedule, bounded draft cache, eased far-off polling); backend/client contract confirms 200 is the only success status, and all 19 targeted tests pass. |
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SECRT-2959 (part of SECRT-2958, the usage-limits stack's follow-ups).
Why / What / How
Why: three things the review bot flagged on #15403, all in Send at reset:
useScheduledSend.schedulereturned quietly when the schedule call answered anything but 200. The API client throws on error statuses, so this only ran on another 2xx, which the endpoint never sends; but if it ever ran, the composer had already cleared the message, so it was gone without a word.useChatDraft's in-memorywrittenmap kept every draft the tab ever saw and was read before session storage, so it could also shadow storage that had been cleared since.useAtResetJobsread the at-reset list every minute for as long as a send waited, days if need be.What:
How: the dead branch throws an
Errorwith no message, whichdescribeSendFailureturns into "Your message is back in the composer. Try again." Nothing else changes. SECRT-2969 asks how CI could catch this kind of dead branch across the frontend (there are 238status === 200checks on generated responses).Changes 🏗️
SendAtReset/useScheduledSend.ts: a non-200 answer to the schedule call fails instead of returning.ChatInput/useChatDraft.ts:writtenholds only drafts storage refused.SendAtReset/useAtResetJobs.ts: 15-minute reads for a turn more than an hour away.useScheduledSend.test.tsx,chatDraft.test.tsx,useAtResetJobs.test.tsx; each new test fails ondev's code.Agents and large language models used
Checklist 📋
For code changes:
scheduleQuietlyputs the message back in the draft (fails ondev: the message is lost)dev)dev: read every minute); the existing due-time test still passesTZ=UTCtsc --noEmitcleanFor configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes🤖 Generated with Claude Code