Merged
Conversation
yingjiehe-xyz
force-pushed
the
yingjiehe/agent
branch
from
ce7b91b to
c465505
Compare
tlongwell-block
approved these changes
Sep 29, 2025
DOsinga
reviewed
Sep 29, 2025
yingjiehe-xyz
force-pushed
the
yingjiehe/agent
branch
from
6c8c491 to
4ef0142
Compare
DOsinga
approved these changes
Sep 29, 2025
| impl AgentManager { | ||
| pub async fn new(max_sessions: Option<usize>) -> Result<Self> { | ||
| /// Reset the global singleton - ONLY for testing | ||
| pub fn reset_for_test() { |
Collaborator
There was a problem hiding this comment.
you do want to annotate this with test only though
Contributor
Author
There was a problem hiding this comment.
I don't think it work here, we can use #[cfg(test)] if the tests are agent class are in the same package, otherwise, tests cannot find the reset_for_test
see error:
no function or associated item named `reset_for_test` found for struct `AgentManager` in the current scope
| #[serial] | ||
| async fn test_session_isolation() { | ||
| let manager = AgentManager::new(None).await.unwrap(); | ||
| AgentManager::reset_for_test(); |
Collaborator
There was a problem hiding this comment.
why do we need to reset these? I'd imagine you only want to reset for testing for situations where you actually want to test that a new singleton does somethign different
Contributor
Author
There was a problem hiding this comment.
I think it is better to reset every time to keep the tests isolated from each other
Collaborator
There was a problem hiding this comment.
yeah, no. you should isolate these tests from the rest of the system, but if you write tests for a singleton and then reset the singleton all the time, what are you testing?
Contributor
Author
There was a problem hiding this comment.
Do you mean that we want to cover the singleton access crossing multiple test cases? Unit tests should only cover each simple functionalities of the methods, we have a test case to test the multiple access as the same time, and others for the main logic, to make all these tests isolated, we need such reset.
zanesq
added a commit
that referenced
this pull request
Sep 30, 2025
…-unification * 'main' of github.com:block/goose: (24 commits) feat(cli): add `path` & `limit` to `session list` command (#4878) Allow better concurrent access (#4896) fix: Windows prompt cursor positioning issue with ANSI escape sequences (#4464) Fix: LiteLLM API key field not showing in UI configuration (#4105) fix: path is duplicated on tool calls causing them to fail (#4658) (#4859) add new prompt to get all available tutorials (#4802) Add filtering for agentVisible: false messages on streaming providers (#4847) alexhancock/mcp-crate-cleanup (#4885) docs: rename sub-recipe to subrecipe (#4886) docs: new multi-model section with autopilot topic (#4864) make agent manager singleton (#4880) Cli web auth token (#4456) fix(token_counter): fix panic with GitHub Copilot (#4632) Revert "Internal MCP Crate Cleanup (#4800)" (#4883) remove 2 redundant comments and one that lies (#4866) Internal MCP Crate Cleanup (#4800) Fix #4612: Return non-zero exit code when CLI session ends with error (#4621) Dead code cleanup (#4873) fix: restoring test data and correcting name (#4875) Add .goosehints file to enforce lowercase branding in documentation (#4870) ...
wpfleger96
added a commit
to wpfleger96/goose
that referenced
this pull request
Oct 1, 2025
* main: (206 commits) Tiny: fix github casing (block#4903) remove anyOf from create_task tool (block#4897) chore(deps): bump tracing-subscriber from 0.3.19 to 0.3.20 (block#4442) fix optional recipe schema zod validation (block#4900) Added CMD+T keyboard shortcut that takes you to the Home tab (block#4541) feat(cli): add `path` & `limit` to `session list` command (block#4878) Allow better concurrent access (block#4896) fix: Windows prompt cursor positioning issue with ANSI escape sequences (block#4464) Fix: LiteLLM API key field not showing in UI configuration (block#4105) fix: path is duplicated on tool calls causing them to fail (block#4658) (block#4859) add new prompt to get all available tutorials (block#4802) Add filtering for agentVisible: false messages on streaming providers (block#4847) alexhancock/mcp-crate-cleanup (block#4885) docs: rename sub-recipe to subrecipe (block#4886) docs: new multi-model section with autopilot topic (block#4864) make agent manager singleton (block#4880) Cli web auth token (block#4456) fix(token_counter): fix panic with GitHub Copilot (block#4632) Revert "Internal MCP Crate Cleanup (block#4800)" (block#4883) remove 2 redundant comments and one that lies (block#4866) ...
HikaruEgashira
pushed a commit
to HikaruEgashira/goose
that referenced
this pull request
Oct 3, 2025
Signed-off-by: HikaruEgashira <[email protected]>
This was referenced Oct 8, 2025
Closed
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
3 participants
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.
Agent manager should be created every time and can be shared in different classes