bug(cli): automation_profile._get_service() bypasses DI container #990

Closed
opened 2026-03-16 23:52:46 +00:00 by brent.edwards · 9 comments
Member

Background

Found during review of PR #803. This is a pre-existing bug, not introduced by that PR.

Description

The automation_profile CLI module's _get_service() function manually constructs create_engine / sessionmaker / AutomationProfileRepository directly instead of resolving through the DI container. This is the only CLI command that manually constructs infrastructure — every other command uses the container.

This means:

  • Database connection settings may diverge from the container's configuration
  • Any container-level middleware (logging, metrics, error handling) is bypassed
  • The repository instance doesn't benefit from container lifecycle management

Expected Behavior

_get_service() should resolve AutomationProfileService (or its dependencies) through the DI container, consistent with all other CLI commands.

Acceptance Criteria

  • _get_service() uses the DI container to resolve dependencies
  • Manual create_engine / sessionmaker construction is removed
  • Behavior is consistent with other CLI command patterns
  • Unit test verifies the service is obtained from the container
  • nox passes with coverage >= 97%

References

  • Discovered in: PR #803 review (finding P1-13)
  • File: src/cleveragents/cli/automation_profile.py (_get_service function)
## Background Found during review of PR #803. This is a pre-existing bug, not introduced by that PR. ## Description The `automation_profile` CLI module's `_get_service()` function manually constructs `create_engine` / `sessionmaker` / `AutomationProfileRepository` directly instead of resolving through the DI container. This is the only CLI command that manually constructs infrastructure — every other command uses the container. This means: - Database connection settings may diverge from the container's configuration - Any container-level middleware (logging, metrics, error handling) is bypassed - The repository instance doesn't benefit from container lifecycle management ## Expected Behavior `_get_service()` should resolve `AutomationProfileService` (or its dependencies) through the DI container, consistent with all other CLI commands. ## Acceptance Criteria - [x] `_get_service()` uses the DI container to resolve dependencies - [x] Manual `create_engine` / `sessionmaker` construction is removed - [x] Behavior is consistent with other CLI command patterns - [x] Unit test verifies the service is obtained from the container - [x] `nox` passes with coverage >= 97% ## References - Discovered in: PR #803 review (finding P1-13) - File: `src/cleveragents/cli/automation_profile.py` (_get_service function)
freemo added this to the v3.3.0 milestone 2026-03-17 18:17:35 +00:00
Owner

TDD workflow initiated for this bug:

  • Created TDD issue #1031 to write a tagged test that captures this DI bypass.
  • Dependency: this issue is blocked by #1031.
  • TDD assigned to @hamza.khyari (infrastructure expertise).
  • Once #1031 merges, @freemo creates branch bugfix/m4-automation-profile-di-bypass.

Triage summary: Assigned to v3.3.0 (M4). Priority/Critical — only CLI command bypassing the DI container.


PM triage — Day 37

**TDD workflow initiated for this bug:** - Created TDD issue #1031 to write a tagged test that captures this DI bypass. - Dependency: this issue is blocked by #1031. - TDD assigned to @hamza.khyari (infrastructure expertise). - Once #1031 merges, @freemo creates branch `bugfix/m4-automation-profile-di-bypass`. **Triage summary:** Assigned to v3.3.0 (M4). Priority/Critical — only CLI command bypassing the DI container. --- *PM triage — Day 37*
Owner

Assigned to @CoreRasurae for bug fix based on developer expertise (DI container — Luis has deep DI expertise). This bug and its TDD counterpart (#1031) are top priority per project policy — bugs always take precedence over feature work.

Assigned to @CoreRasurae for bug fix based on developer expertise (DI container — Luis has deep DI expertise). This bug and its TDD counterpart (#1031) are top priority per project policy — bugs always take precedence over feature work.
Owner

Planning Agent — Discussion Review

Assignment change noted: Day 37 triage assigned the bug fix to @freemo with a specific branch name (bugfix/m4-automation-profile-di-bypass). The latest comment reassigns to @CoreRasurae citing "DI container — Luis has deep DI expertise."

Both are valid assignees. However, the original triage specifically noted that this is "the only CLI command bypassing the DI container," which suggests @freemo's knowledge of the CLI command layer is more relevant than general DI expertise. The fix likely involves changing how automation_profile._get_service() resolves its dependency — from direct instantiation to container lookup.

Ruling: Keeping @CoreRasurae's assignment since it was the most recent decision and Luis does have strong DI expertise. However, @freemo should be available for consultation on the automation_profile command-level context.

TDD issue #1031 is correctly identified as the blocker. 3-point estimate is appropriate.

No disputes on scope or priority.

## Planning Agent — Discussion Review **Assignment change noted:** Day 37 triage assigned the bug fix to @freemo with a specific branch name (`bugfix/m4-automation-profile-di-bypass`). The latest comment reassigns to @CoreRasurae citing "DI container — Luis has deep DI expertise." Both are valid assignees. However, the original triage specifically noted that this is "the only CLI command bypassing the DI container," which suggests @freemo's knowledge of the CLI command layer is more relevant than general DI expertise. The fix likely involves changing how `automation_profile._get_service()` resolves its dependency — from direct instantiation to container lookup. **Ruling:** Keeping @CoreRasurae's assignment since it was the most recent decision and Luis does have strong DI expertise. However, @freemo should be available for consultation on the `automation_profile` command-level context. TDD issue #1031 is correctly identified as the blocker. 3-point estimate is appropriate. No disputes on scope or priority.
freemo self-assigned this 2026-03-23 03:29:58 +00:00
Owner

Action required: Create Forgejo dependency link.

This bug issue must have a Forgejo dependency link to TDD issue #1031 (bug is blocked by TDD issue). The Forgejo REST API does not support creating dependency links in Forgejo 14 — this must be done via the web UI:

  1. Open this issue in the browser
  2. In the sidebar, find "Dependencies" and click "Add dependency"
  3. Search for #1031 and add it as a dependency (this issue depends on #1031)

This ensures the correct TDD workflow ordering: #1031 (write the tagged test) must be completed before this issue (implement the fix) can begin.

**Action required: Create Forgejo dependency link.** This bug issue must have a Forgejo dependency link to TDD issue #1031 (bug is blocked by TDD issue). The Forgejo REST API does not support creating dependency links in Forgejo 14 — this must be done via the web UI: 1. Open this issue in the browser 2. In the sidebar, find "Dependencies" and click "Add dependency" 3. Search for #1031 and add it as a dependency (this issue depends on #1031) This ensures the correct TDD workflow ordering: #1031 (write the tagged test) must be completed before this issue (implement the fix) can begin.
brent.edwards added reference bugfix/m4-automation-profile-di-bypass 2026-03-28 05:20:27 +00:00
Author
Member

Starting implementation for #990 in isolated workspace /tmp/cleveragents-990 on branch bugfix/m4-automation-profile-di-bypass.

Preparation completed:

  • Verified issue is open.
  • Reassigned to brent.edwards per execution requirement.
  • Transitioned label from State/Verified to State/In Progress.
  • Set issue Ref to branch bugfix/m4-automation-profile-di-bypass.
  • Reviewed docs/specification.md, CONTRIBUTING.md, and docs/timeline.md for alignment.

Implementation plan:

  1. Add a BDD regression scenario asserting automation_profile._get_service() resolves service via DI container provider.
  2. Refactor CLI command helper to delegate to container-managed provider instead of manually constructing SQLAlchemy engine/session/repository.
  3. Add/adjust DI container wiring for AutomationProfileService construction so CLI uses consistent container lifecycle.
  4. Run nox quality gates and coverage, then update checklist + journal with results.
Starting implementation for #990 in isolated workspace `/tmp/cleveragents-990` on branch `bugfix/m4-automation-profile-di-bypass`. Preparation completed: - Verified issue is open. - Reassigned to `brent.edwards` per execution requirement. - Transitioned label from `State/Verified` to `State/In Progress`. - Set issue Ref to branch `bugfix/m4-automation-profile-di-bypass`. - Reviewed `docs/specification.md`, `CONTRIBUTING.md`, and `docs/timeline.md` for alignment. Implementation plan: 1. Add a BDD regression scenario asserting `automation_profile._get_service()` resolves service via DI container provider. 2. Refactor CLI command helper to delegate to container-managed provider instead of manually constructing SQLAlchemy engine/session/repository. 3. Add/adjust DI container wiring for `AutomationProfileService` construction so CLI uses consistent container lifecycle. 4. Run nox quality gates and coverage, then update checklist + journal with results.
Author
Member

Implementation journal update for #990:

What changed

  1. CLI DI wiring fix
  • Updated src/cleveragents/cli/commands/automation_profile.py (_get_service) to resolve service from DI container:
    • now obtains container via _get_container()
    • returns container.automation_profile_service()
  • Removed direct/manual wiring path from CLI helper (no local create_engine/sessionmaker/AutomationProfileRepository construction in this helper anymore).
  1. Container provider added
  • Updated src/cleveragents/application/container.py:
    • added _build_automation_profile_service(database_url)
    • added provider automation_profile_service
    • imports AutomationProfileService
  • This aligns automation profile command behavior with existing CLI patterns that resolve through container providers.
  1. TDD / behavior tests
  • Added issue-focused BDD test:
    • features/tdd_automation_profile_di_container.feature
    • features/steps/tdd_automation_profile_di_container_steps.py
  • Updated old coverage-boost tests that previously depended on manual CLI construction:
    • features/automation_profile_cli_coverage_boost_r2.feature
    • features/steps/automation_profile_cli_coverage_boost_r2_steps.py
  • New/updated tests assert container delegation behavior rather than direct infra construction.

Additional stability work discovered while running required gates

While running full quality gates, a pre-existing/flaky integration failure appeared in Robot.Resource Dag (Cycle Detection Rejects A To B To A):

  • symptom: ResourceTypeNotFoundError: Resource type 'robot/cycle-type' not found
  • root cause in robot test script setup was inconsistent session behavior for in-memory SQLite path.

To stabilize gate runs, updated robot/resource_dag.robot to use a single shared SQLAlchemy session per script scenario (lambda: session) for repository access, which removes nondeterministic cross-session visibility behavior in that test script.

Quality gates executed

  • nox -e lint
  • nox -e typecheck
  • nox -e unit_tests
  • nox -e integration_tests -- --processes 2
  • nox -e coverage_report
    • coverage summary from build/coverage.json: 97.4658% (>=97 threshold)
  • Full suite:
    • TEST_PROCESSES=2 nox (all default sessions, including integration/tests/docs/build/benchmark/coverage)

Notes:

  • nox -e e2e_tests was executed separately and currently has unrelated environment/workflow failures in WF suites (server/CI workflow setups). This session is not part of default nox execution path but was inspected during validation.

I will proceed with commit/PR metadata and then transition this issue to State/In Review.

Implementation journal update for #990: ## What changed 1) **CLI DI wiring fix** - Updated `src/cleveragents/cli/commands/automation_profile.py` (`_get_service`) to resolve service from DI container: - now obtains container via `_get_container()` - returns `container.automation_profile_service()` - Removed direct/manual wiring path from CLI helper (no local `create_engine/sessionmaker/AutomationProfileRepository` construction in this helper anymore). 2) **Container provider added** - Updated `src/cleveragents/application/container.py`: - added `_build_automation_profile_service(database_url)` - added provider `automation_profile_service` - imports `AutomationProfileService` - This aligns automation profile command behavior with existing CLI patterns that resolve through container providers. 3) **TDD / behavior tests** - Added issue-focused BDD test: - `features/tdd_automation_profile_di_container.feature` - `features/steps/tdd_automation_profile_di_container_steps.py` - Updated old coverage-boost tests that previously depended on manual CLI construction: - `features/automation_profile_cli_coverage_boost_r2.feature` - `features/steps/automation_profile_cli_coverage_boost_r2_steps.py` - New/updated tests assert container delegation behavior rather than direct infra construction. ## Additional stability work discovered while running required gates While running full quality gates, a pre-existing/flaky integration failure appeared in `Robot.Resource Dag` (`Cycle Detection Rejects A To B To A`): - symptom: `ResourceTypeNotFoundError: Resource type 'robot/cycle-type' not found` - root cause in robot test script setup was inconsistent session behavior for in-memory SQLite path. To stabilize gate runs, updated `robot/resource_dag.robot` to use a single shared SQLAlchemy session per script scenario (`lambda: session`) for repository access, which removes nondeterministic cross-session visibility behavior in that test script. ## Quality gates executed - `nox -e lint` ✅ - `nox -e typecheck` ✅ - `nox -e unit_tests` ✅ - `nox -e integration_tests -- --processes 2` ✅ - `nox -e coverage_report` ✅ - coverage summary from `build/coverage.json`: `97.4658%` (>=97 threshold) - Full suite: - `TEST_PROCESSES=2 nox` ✅ (all default sessions, including integration/tests/docs/build/benchmark/coverage) Notes: - `nox -e e2e_tests` was executed separately and currently has unrelated environment/workflow failures in WF suites (server/CI workflow setups). This session is not part of default `nox` execution path but was inspected during validation. I will proceed with commit/PR metadata and then transition this issue to `State/In Review`.
Author
Member

Final delivery update for #990:

  • Commit: cab36ef8f8536504861e8776d9be6f1f3cad9677
  • Branch: bugfix/m4-automation-profile-di-bypass
  • PR: #1181

Issue state transitioned to State/In Review.

PR metadata applied:

  • Milestone: v3.3.0
  • Label: Type/Bug
  • Dependency link added from PR -> issue #990

CI/checks status on PR head: success (all checks green).

Final delivery update for #990: - Commit: `cab36ef8f8536504861e8776d9be6f1f3cad9677` - Branch: `bugfix/m4-automation-profile-di-bypass` - PR: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1181 Issue state transitioned to `State/In Review`. PR metadata applied: - Milestone: `v3.3.0` - Label: `Type/Bug` - Dependency link added from PR -> issue #990 CI/checks status on PR head: **success** (all checks green).
Owner

PR #1181 reviewed, approved, and merged.

The fix correctly routes automation_profile._get_service() through the DI container instead of manually constructing SQLAlchemy engine/session/repository. The container now has a dedicated automation_profile_service provider following the same pattern as all other DB-backed services. Tests were updated to verify delegation behavior rather than implementation details, including an integration-style scenario using a real container with a temporary SQLite database.

PR #1181 reviewed, approved, and merged. The fix correctly routes `automation_profile._get_service()` through the DI container instead of manually constructing SQLAlchemy engine/session/repository. The container now has a dedicated `automation_profile_service` provider following the same pattern as all other DB-backed services. Tests were updated to verify delegation behavior rather than implementation details, including an integration-style scenario using a real container with a temporary SQLite database.
Owner

PR #1181 reviewed, approved, and merged. The _get_service() function in automation_profile.py now correctly resolves AutomationProfileService through the DI container, consistent with all other CLI commands.

PR #1181 reviewed, approved, and merged. The `_get_service()` function in `automation_profile.py` now correctly resolves `AutomationProfileService` through the DI container, consistent with all other CLI commands.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#990
No description provided.