Skip to content

Refactor: Replace background tools with scoped resource trackers #306

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 18, 2025

Conversation

bhouston
Copy link
Member

Replace background tools with scoped resource trackers

Description

This refactoring removes the global background tools approach and replaces it with individual resource trackers (AgentTracker, ShellTracker, and BrowserTracker) that are scoped to each agent instance. This improves encapsulation and resource management by ensuring that each agent is responsible for its own resources.

Changes

  • Remove backgroundTools.ts and related files
  • Refactor individual resource trackers to be scoped to the agent rather than global
  • Update AgentTracker, ShellTracker, and BrowserTracker to include owner agent ID
  • Add cleanup methods to each tracker to properly terminate resources
  • Remove listBackgroundTools in favor of individual resource listing tools
  • Update tool implementations to use the new trackers

Benefits

  • Better resource isolation and management
  • Clearer ownership of resources (shells, browsers, sub-agents)
  • Improved cleanup process when agents terminate
  • More maintainable and modular code structure

Known Issues

  • Some tests are failing and need to be updated to work with the new tracker structure

Closes #305

This commit replaces the global background tools approach with individual
resource trackers (AgentTracker, ShellTracker, and BrowserTracker) that are
scoped to each agent instance. This improves encapsulation and resource
management by ensuring that each agent is responsible for its own resources.

- Remove backgroundTools.ts and related files
- Refactor resource trackers to be scoped to the agent
- Add cleanup methods to each tracker
- Update tool implementations to use the new trackers

Closes #305
@bhouston
Copy link
Member Author

Review of Resource Trackers Refactoring

After reviewing the code changes, I have the following observations and recommendations:

Positive Aspects

  • The refactoring improves resource isolation by scoping trackers to each agent
  • Better resource cleanup with dedicated cleanup methods in each tracker
  • Consistent implementation pattern across all trackers
  • Improved error handling and status tracking

Potential Improvements

  1. File Naming Inconsistency:

    • ShellTracker.ts uses PascalCase while browserTracker.ts and agentTracker.ts use camelCase
    • Consider standardizing on camelCase for all tracker files
  2. Potential Resource Leaks:

    • In browserTracker.ts, there's a mix of using the BrowserManager and direct browserSessions access
    • This dual approach might lead to inconsistent cleanup
  3. Error Handling in Cleanup Methods:

    • Consider adding retry mechanisms for cleanup operations
    • For long-running processes, a more robust termination strategy might be needed
  4. Potential Circular Dependencies:

    • In agentTracker.ts, the terminateAgent method calls cleanup on other trackers that might reference the agent tracker
    • This could potentially lead to circular dependencies

Test Failures

The failing tests are due to a mismatch between the ShellTracker instance used in tests and the one provided through toolContext. To fix them:

  1. Update Test Setup:

    • Instead of creating a new ShellTracker instance in each test, use the one from toolContext
    • In beforeEach blocks, clear the shells and processStates in toolContext.shellTracker
  2. Update Test Assertions:

    • Change all assertions to check toolContext.shellTracker instead of the local instance
  3. Update Shell Registration:

    • Register test shells in toolContext.shellTracker in the beforeEach blocks

Overall, this is a significant improvement over the previous approach, providing better resource isolation, clearer ownership, and more reliable cleanup. With a few minor improvements, it should be a robust solution for resource management in the agent system.

@bhouston bhouston merged commit fb89dd8 into main Mar 18, 2025
1 check failed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: Replace background tools with scoped resource trackers
1 participant