Skip to content

test: Add unit tests for BotManager Incus path - #60

Merged
sebyx07 merged 5 commits into
mainfrom
claudetm/feat/abstract-tool-execution
Feb 14, 2026
Merged

sebyx07 merged 5 commits into
mainfrom
claudetm/feat/abstract-tool-execution

Conversation

@sebyx07

@sebyx07 sebyx07 commented Feb 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add comprehensive unit tests for BotManager's Incus sandbox support:

  • _hasSandboxChanged() Incus tests: Test detection of changes in incusImage and incusProfile fields
  • Sandbox type migration tests: Test transitioning between docker and incus sandbox types
  • startBot() Incus test: Verify Incus config is correctly passed to ContainerPool
  • reloadBot() Incus tests: Test container recreation when Incus-specific fields change

Test Coverage

  • 4 new tests in _hasSandboxChanged() section for Incus-specific fields
  • 6 new tests in Incus sandbox support section covering:
    • Config passing to ContainerPool
    • Container recreation on incusImage change
    • Container recreation on incusProfile change
    • Sandbox type migration (docker → incus)
    • Sandbox type migration (incus → docker)
    • No-op when only non-sandbox config changes

Test Results

All 119 BotManager unit tests pass (10 new Incus-related tests added).

Part of PR Group

PR Group: Tool Layer Adaptation & Integration
Branch: claudetm/feat/abstract-tool-execution

This is the final task in the PR group that abstracts tool execution away from Docker specifics and adds Incus support throughout the codebase.

🤖 Generated with Claude Task Master

Greptile Overview

Greptile Summary

This PR completes the Incus sandbox support implementation by adding comprehensive unit tests for BotManager's Incus-related functionality. The PR includes 10 new tests covering Incus-specific sandbox change detection (incusImage and incusProfile fields), sandbox type migration between docker and incus, and proper configuration passing through the ContainerPool.

Key Changes:

  • Added 4 tests to _hasSandboxChanged() for Incus fields (incusImage, incusProfile) and type transitions
  • Added 6 tests for Incus support in startBot() and reloadBot() covering config passing, container recreation on Incus field changes, and sandbox type migrations
  • Updated ContainerPool tests to verify Incus backend creation and the new exec()/getBackend() methods
  • Refactored all tool tests (BashTool, FileTools, ToolExecutor) to use the new containerPool.exec() abstraction
  • Added destructuredArrayIgnorePattern to eslint config to support the [_botId, ...] pattern in tests
  • Added startup logging for available sandbox backends

The implementation correctly mirrors the Docker backend pattern by reusing a shared default IncusBackend instance for local bots (addressing the previous review comment). All 119 tests pass successfully.

Confidence Score: 5/5

  • This PR is safe to merge with no risk - it adds comprehensive test coverage without modifying core logic
  • All changes are test-focused with minimal production code modifications. The production changes (eslint config update, Incus field checks in _hasSandboxChanged(), incusHost option passing) are simple and well-tested. The previous concern about IncusBackend instantiation has been addressed in a prior commit. Tests are thorough, well-structured, and all 119 tests pass.
  • No files require special attention

Important Files Changed

Filename Overview
src/core/bot-manager.js Added Incus support: passes incusHost option to ContainerPool and checks incusImage/incusProfile fields in _hasSandboxChanged()
src/execution/container-pool.js Implemented Incus backend support with shared default instance, added exec() and getBackend() convenience methods
src/execution/tool-executor.js Refactored to use containerPool.exec() instead of directly accessing dockerManager.exec() for backend abstraction
src/tools/bash-tool.js Updated to use containerPool.exec() instead of direct dockerManager access
src/tools/file-tools.js Updated all file tools to use containerPool.exec() instead of direct dockerManager access
test/unit/core/BotManager.test.js Added 10 new tests for Incus support covering _hasSandboxChanged(), startBot(), reloadBot(), and sandbox type migration
test/unit/execution/ContainerPool.test.js Updated Incus test expectation and added comprehensive tests for new exec() and getBackend() methods

Sequence Diagram

sequenceDiagram
    participant BM as BotManager
    participant CP as ContainerPool
    participant IB as IncusBackend
    participant DB as DockerBackend
    participant TE as ToolExecutor
    participant BT as BashTool
    
    Note over BM,BT: Incus Support Integration
    
    BM->>CP: startBot(botId, config)<br/>{sandbox.type: 'incus', incusImage: 'ai-army-base'}
    CP->>CP: _getBackend(botId, config, {incusHost})
    alt No incusHost (local)
        CP->>IB: new IncusBackend() [shared instance]
    else With incusHost (remote)
        CP->>IB: new IncusBackend({socketPath: incusHost})
    end
    CP->>IB: createContainer(config)
    IB-->>CP: container
    CP-->>BM: container
    
    Note over TE,BT: Tool Execution Abstraction
    
    BT->>CP: exec(botId, command, options)
    CP->>CP: getContainer(botId)
    CP->>CP: _getBackendForBot(botId)
    alt Incus Backend
        CP->>IB: exec(container, command, options)
        IB-->>CP: {exitCode, stdout, stderr}
    else Docker Backend
        CP->>DB: exec(container, command, options)
        DB-->>CP: {exitCode, stdout, stderr}
    end
    CP-->>BT: result
    
    Note over BM: Sandbox Change Detection
    BM->>BM: _hasSandboxChanged(oldSandbox, newSandbox)
    Note right of BM: Checks incusImage<br/>and incusProfile
Loading

Last reviewed commit: 6e96f64

sebyx07 and others added 3 commits February 14, 2026 18:58
- Add ContainerPool.exec(botId, command, options) convenience method
  that gets the container, finds the correct backend, and delegates exec
- Add ContainerPool.getBackend(botId) public method for direct backend access
- Refactor bash-tool.js to use containerPool.exec() instead of
  getContainer() + dockerManager.exec() pattern
- Refactor file-tools.js (readFile, writeFile, glob, grep) similarly
- Refactor tool-executor.js (bash, readFile, writeFile) similarly
- Keep containerPool.dockerManager working for backward compatibility
- Tools no longer care which backend runs the command — they just
  call exec() and get { stdout, stderr, exitCode }
- Update all unit tests to use the new exec() abstraction in mocks

Co-Authored-By: Claude <noreply@anthropic.com>
…areness

- ContainerPool._getBackend() now instantiates IncusBackend instead of throwing
- ContainerPool._getBackend() accepts options object with dockerHost and incusHost
- BotManager.startBot() passes incusHost from worker assignment to ContainerPool
- BotManager._hasSandboxChanged() detects incusImage and incusProfile changes
- Orchestrator imports BACKEND_TYPES and logs available sandbox backends on startup
- Update ContainerPool test to expect IncusBackend creation instead of throw

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive unit tests covering Incus-specific functionality:
- Test `_hasSandboxChanged()` with Incus fields (incusImage, incusProfile)
- Test sandbox type changes between docker and incus
- Test `startBot()` passes Incus config to ContainerPool correctly
- Test `reloadBot()` recreates container when Incus fields change
- Test `reloadBot()` handles sandbox type migration (docker ↔ incus)

All 119 BotManager tests pass (6 new Incus-specific tests + 4 new
_hasSandboxChanged tests for Incus fields).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@sebyx07 sebyx07 added the claudetm Claude Task Master managed PRs label Feb 14, 2026
- Allows underscore-prefixed variables in array destructuring (e.g., _botId)
- Consistent with existing argsIgnorePattern and caughtErrorsIgnorePattern
- Fixes CI lint failure for unused destructured array variables in tests

Co-Authored-By: Claude <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/execution/container-pool.js
- Mirror the Docker backend pattern by lazily creating and reusing a
  shared default IncusBackend when no remote incusHost is specified
- Prevents creating redundant HTTP agents all pointing at the same
  Unix socket when multiple Incus bots use the local daemon
- Add _defaultIncusBackend property with lazy initialization

Addresses PR #60 review feedback from greptile-apps[bot]

Co-Authored-By: Claude <noreply@anthropic.com>
@sebyx07
sebyx07 merged commit cf07d8b into main Feb 14, 2026
7 checks passed
@sebyx07
sebyx07 deleted the claudetm/feat/abstract-tool-execution branch February 14, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claudetm Claude Task Master managed PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant