-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(background-agent): pass model on resume to preserve category config #846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(background-agent): pass model on resume to preserve category config #846
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
7169b14 to
50c6790
Compare
The resume method was not passing the stored model from the task, causing Sisyphus-Junior to revert to the default model when resumed. This fix adds the model to the prompt body in resume(), matching the existing behavior in launch(). Fixes code-yeongyu#826
50c6790 to
8402b55
Compare
I don't think not passing is intended tho. Assuming no config change after the init, this should work. Still need some manual testing Edit: btw I noticed the CI failing one test? Is it related? If yes maybe fix it |
the reason is say that is that this pr removes the comment |
Yea, but in resuming context, should it keep using prev model? Think about it, I think just keep the previous approach: Honor the configuration instead of override |
|
This looks like a good update to me. I traced this by talking with the codebase. I wanted to understand all the reasons for resuming a task and whether or not it makes sense to pass in the model. Reasons for Task Resuming:
Why This Change Makes Sense:Background tasks launched with specific models (from category configs like "visual-engineering" → "google/gemini-3-pro-preview") should resume with the same model they started with. Currently:
This inconsistency means resumed tasks could switch models unexpectedly. The proposed change adds Impact: Prevents model drift during task continuation, especially important for category-specific model requirements. 🎯 |
Summary
Changes
src/features/background-agent/manager.ts: Add model to prompt body in resume method, matching the launch method patternsrc/features/background-agent/manager.test.ts: Add tests for model persistence on resumeThe resume path was missing the model parameter that launch() correctly passes. When a task with a category-configured model was resumed, it would revert to the default model instead of using the stored override.
Testing
bun run typecheck bun test src/features/background-agent/Related Issues
Closes #826
Note
CI may fail due to a pre-existing flaky test issue on
devbranch. See #848 for details.