cleveragents/cleveragents-core
cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.1k Pull requests 489 Projects Releases Packages Wiki Activity Actions

fix(plan): resolve automation profile at plan-use time per spec precedence rules #1178

Closed
brent.edwards wants to merge 1 commit from bugfix/use-action-automation-profile into master
pull from: bugfix/use-action-automation-profile
merge into: cleveragents:master
Conversation 23 Commits 1 Files changed 52 +885 -193
brent.edwards commented 2026-03-28 18:01:23 +00:00
Member
Copy link

Summary

  • Fix PlanLifecycleService.use_action(...) to resolve automation profile at plan-use time using spec precedence: plan override > action > project config > global config.
  • Persist resolved automation profile to the plan as AutomationProfileRef (including provenance), and wire plan-use override propagation through CLI and A2A plan-create paths.
  • Update Behave/Robot coverage for precedence and action→plan propagation, harden lifecycle/E2E auto-progression handling (WF12), and make project status integration assertions tolerant to environment bootstrap differences (auto-detected context vs explicit no-project error).

Motivation

Issue #1076 identified that use_action() could create plans without an effective automation profile lock-in, violating spec precedence and allowing policy ambiguity at plan creation.

Validation

  • nox -e lint
  • nox -e typecheck
  • nox -e unit_tests
  • nox -e integration_tests
  • nox -e e2e_tests
  • nox -e coverage_report (>= 97%; current run: 97%)

Notes

  • No deferred review items remain in scope for this PR.

Closes #1076

⛔ Superseded

This PR has been closed as superseded. PR #1196 (merged April 1, 2026) delivered the same core fix to master. See the final review comment for details.

## Summary - Fix `PlanLifecycleService.use_action(...)` to resolve automation profile at plan-use time using spec precedence: plan override > action > project config > global config. - Persist resolved automation profile to the plan as `AutomationProfileRef` (including provenance), and wire plan-use override propagation through CLI and A2A plan-create paths. - Update Behave/Robot coverage for precedence and action→plan propagation, harden lifecycle/E2E auto-progression handling (WF12), and make `project status` integration assertions tolerant to environment bootstrap differences (auto-detected context vs explicit no-project error). ## Motivation Issue #1076 identified that `use_action()` could create plans without an effective automation profile lock-in, violating spec precedence and allowing policy ambiguity at plan creation. ## Validation - `nox -e lint` - `nox -e typecheck` - `nox -e unit_tests` - `nox -e integration_tests` - `nox -e e2e_tests` - `nox -e coverage_report` (>= 97%; current run: 97%) ## Notes - No deferred review items remain in scope for this PR. Closes #1076 ## ⛔ Superseded **This PR has been closed as superseded.** PR #1196 (merged April 1, 2026) delivered the same core fix to master. See the final review comment for details.
brent.edwards added this to the v3.3.0 milestone 2026-03-28 18:01:30 +00:00
freemo approved these changes 2026-03-28 23:13:02 +00:00
Dismissed
freemo left a comment
Copy link

Day 48 Planning Review — Bug Fix PR for #1076

This is the cleanest bug fix PR in the current batch. All review criteria pass:

  • Single commit with correct Conventional Changelog format ✓
  • Closes #1076 present ✓
  • @tdd_expected_fail properly removed from tdd_use_action_automation_profile.feature ✓
  • @tdd_issue/@tdd_issue_1076 retained as permanent regression guard ✓
  • Full nox passes including lint, typecheck, unit tests, integration, e2e, coverage (97.47%) ✓
  • Mergeable (mergeable: true) ✓
  • CHANGELOG updated ✓

The implementation correctly wires automation profile resolution through PlanLifecycleService.use_action() with spec-compliant precedence (plan override > action > project config > global). The AutomationProfileRef with provenance tracking is a clean design.

Minor note: 25 files changed is substantial for a bug fix, but the scope is justified — the fix requires wiring through CLI, A2A facade, lifecycle service, and stabilizing E2E suites for the new behavior.

APPROVED. Awaiting reviews from @freemo and @aditya.

**Day 48 Planning Review — Bug Fix PR for #1076** This is the cleanest bug fix PR in the current batch. All review criteria pass: - Single commit with correct Conventional Changelog format ✓ - `Closes #1076` present ✓ - `@tdd_expected_fail` properly removed from `tdd_use_action_automation_profile.feature` ✓ - `@tdd_issue`/`@tdd_issue_1076` retained as permanent regression guard ✓ - Full `nox` passes including lint, typecheck, unit tests, integration, e2e, coverage (97.47%) ✓ - Mergeable (`mergeable: true`) ✓ - CHANGELOG updated ✓ The implementation correctly wires automation profile resolution through `PlanLifecycleService.use_action()` with spec-compliant precedence (plan override > action > project config > global). The `AutomationProfileRef` with provenance tracking is a clean design. **Minor note**: 25 files changed is substantial for a bug fix, but the scope is justified — the fix requires wiring through CLI, A2A facade, lifecycle service, and stabilizing E2E suites for the new behavior. **APPROVED.** Awaiting reviews from @freemo and @aditya.
freemo approved these changes 2026-03-30 04:22:21 +00:00
Dismissed
freemo left a comment
Copy link

Review: APPROVED

The most impactful PR in this batch. Core fix moves automation profile resolution into the service layer where it belongs, implementing proper 4-level precedence (plan > action > project > global). The E2E infrastructure improvements (centralized init, resilient acceptance tests) are valuable hardening. Scope is broad but everything serves the same logical change.

## Review: APPROVED The most impactful PR in this batch. Core fix moves automation profile resolution into the service layer where it belongs, implementing proper 4-level precedence (plan > action > project > global). The E2E infrastructure improvements (centralized init, resilient acceptance tests) are valuable hardening. Scope is broad but everything serves the same logical change.
brent.edwards force-pushed bugfix/use-action-automation-profile from bf146dbc16
All checks were successful
CI / lint (pull_request) Successful in 3m19s
Details
CI / typecheck (pull_request) Successful in 3m53s
Details
CI / security (pull_request) Successful in 4m6s
Details
CI / quality (pull_request) Successful in 3m40s
Details
CI / build (pull_request) Successful in 15s
Details
CI / helm (pull_request) Successful in 36s
Details
CI / unit_tests (pull_request) Successful in 9m16s
Details
CI / integration_tests (pull_request) Successful in 9m8s
Details
CI / e2e_tests (pull_request) Successful in 12m17s
Details
CI / docker (pull_request) Successful in 1m19s
Details
CI / coverage (pull_request) Successful in 12m21s
Details
CI / status-check (pull_request) Successful in 1s
Details
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / benchmark-regression (pull_request) Successful in 1h0m11s
Details
to ae6dcfd1be
Some checks failed
CI / lint (pull_request) Successful in 20s
Details
CI / quality (pull_request) Successful in 36s
Details
CI / build (pull_request) Successful in 18s
Details
CI / helm (pull_request) Successful in 46s
Details
CI / typecheck (pull_request) Successful in 4m2s
Details
CI / security (pull_request) Successful in 4m4s
Details
CI / integration_tests (pull_request) Failing after 5m34s
Details
CI / unit_tests (pull_request) Successful in 7m19s
Details
CI / docker (pull_request) Successful in 1m26s
Details
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / coverage (pull_request) Successful in 9m11s
Details
CI / e2e_tests (pull_request) Successful in 15m40s
Details
CI / status-check (pull_request) Failing after 1s
Details
CI / benchmark-regression (pull_request) Has been cancelled
Details
2026-03-30 11:52:14 +00:00 Compare
brent.edwards dismissed freemo's review 2026-03-30 11:52:14 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

brent.edwards force-pushed bugfix/use-action-automation-profile from ae6dcfd1be
Some checks failed
CI / lint (pull_request) Successful in 20s
Details
CI / quality (pull_request) Successful in 36s
Details
CI / build (pull_request) Successful in 18s
Details
CI / helm (pull_request) Successful in 46s
Details
CI / typecheck (pull_request) Successful in 4m2s
Details
CI / security (pull_request) Successful in 4m4s
Details
CI / integration_tests (pull_request) Failing after 5m34s
Details
CI / unit_tests (pull_request) Successful in 7m19s
Details
CI / docker (pull_request) Successful in 1m26s
Details
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / coverage (pull_request) Successful in 9m11s
Details
CI / e2e_tests (pull_request) Successful in 15m40s
Details
CI / status-check (pull_request) Failing after 1s
Details
CI / benchmark-regression (pull_request) Has been cancelled
Details
to 836709f226
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / build (pull_request) Successful in 19s
Details
CI / helm (pull_request) Successful in 21s
Details
CI / lint (pull_request) Successful in 3m19s
Details
CI / integration_tests (pull_request) Failing after 3m45s
Details
CI / quality (pull_request) Successful in 3m49s
Details
CI / typecheck (pull_request) Successful in 3m56s
Details
CI / security (pull_request) Successful in 4m6s
Details
CI / unit_tests (pull_request) Successful in 4m19s
Details
CI / docker (pull_request) Successful in 1m18s
Details
CI / coverage (pull_request) Successful in 8m29s
Details
CI / e2e_tests (pull_request) Successful in 17m39s
Details
CI / status-check (pull_request) Failing after 1s
Details
CI / benchmark-regression (pull_request) Successful in 53m45s
Details
2026-03-30 12:31:07 +00:00 Compare
brent.edwards force-pushed bugfix/use-action-automation-profile from 836709f226
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / build (pull_request) Successful in 19s
Details
CI / helm (pull_request) Successful in 21s
Details
CI / lint (pull_request) Successful in 3m19s
Details
CI / integration_tests (pull_request) Failing after 3m45s
Details
CI / quality (pull_request) Successful in 3m49s
Details
CI / typecheck (pull_request) Successful in 3m56s
Details
CI / security (pull_request) Successful in 4m6s
Details
CI / unit_tests (pull_request) Successful in 4m19s
Details
CI / docker (pull_request) Successful in 1m18s
Details
CI / coverage (pull_request) Successful in 8m29s
Details
CI / e2e_tests (pull_request) Successful in 17m39s
Details
CI / status-check (pull_request) Failing after 1s
Details
CI / benchmark-regression (pull_request) Successful in 53m45s
Details
to 34897a83c8
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / build (pull_request) Successful in 20s
Details
CI / helm (pull_request) Successful in 26s
Details
CI / lint (pull_request) Successful in 3m18s
Details
CI / quality (pull_request) Successful in 3m43s
Details
CI / integration_tests (pull_request) Successful in 3m45s
Details
CI / typecheck (pull_request) Successful in 3m55s
Details
CI / unit_tests (pull_request) Successful in 4m1s
Details
CI / security (pull_request) Successful in 4m8s
Details
CI / docker (pull_request) Successful in 2m4s
Details
CI / e2e_tests (pull_request) Successful in 16m33s
Details
CI / coverage (pull_request) Successful in 13m5s
Details
CI / status-check (pull_request) Successful in 1s
Details
CI / benchmark-regression (pull_request) Successful in 52m47s
Details
2026-03-30 14:02:36 +00:00 Compare
freemo self-assigned this 2026-04-02 06:15:17 +00:00
freemo commented 2026-04-02 17:47:27 +00:00
Owner
Copy link

Review claimed by reviewer pool instance reviewer-pool-2. Dispatching independent code review.

Review claimed by reviewer pool instance reviewer-pool-2. Dispatching independent code review.
freemo commented 2026-04-02 17:52:45 +00:00
Owner
Copy link

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.
freemo approved these changes 2026-04-02 17:59:01 +00:00
Dismissed
freemo left a comment
Copy link

Independent Code Review — APPROVED

Review Scope

Reviewed all 52 changed files (4 production, 48 test/infrastructure) against the specification and CONTRIBUTING.md standards.

Specification Alignment ✅

The implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The AutomationProfileRef with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime.

Core Implementation ✅

  • PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution with proper fallback to ConfigService.validate_key().default. Lazy ConfigService initialization via _get_config_service() is appropriate.
  • CLI plan.py: Profile validation moved before use_action() call (was incorrectly mutating the plan post-creation). New validate_plan_use_automation_profile() correctly accepts both built-in bare names and namespaced custom profiles.
  • A2A Facade: _normalize_optional_string_param() provides proper input validation with structured ValidationError for non-string values.
  • Container: ConfigService properly wired as a factory provider into PlanLifecycleService.

Test Quality ✅

  • BDD: @tdd_expected_fail correctly removed; @tdd_issue/@tdd_issue_1076 retained as permanent regression guards. New scenarios cover plan-level override, action propagation, project config, global fallback, and lazy ConfigService initialization.
  • Robot: New use_action_automation_profile.robot + M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution.
  • Stabilization: Extensive but justified — the fix enables auto-progression behavior that required existing tests to handle additional terminal states (APPLIED) and phase checks before transitions.

Standards Compliance ✅

  • Commit message: Conventional Changelog format, matches issue metadata exactly
  • ISSUES CLOSED: #1076 in footer
  • No # type: ignore suppressions
  • No secrets or credentials
  • Type/Bug label and v3.3.0 milestone assigned
  • Coverage reported at 97.47% (≥97% threshold)

Minor Observations (non-blocking)

  • resolved.source.value == "project" string comparison in _resolve_use_action_automation_profile is slightly fragile but consistent with existing codebase patterns.
  • Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance.

APPROVED — Clean bug fix with thorough test coverage and proper spec alignment.

## Independent Code Review — APPROVED ### Review Scope Reviewed all 52 changed files (4 production, 48 test/infrastructure) against the specification and CONTRIBUTING.md standards. ### Specification Alignment ✅ The implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The `AutomationProfileRef` with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime. ### Core Implementation ✅ - **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution with proper fallback to `ConfigService.validate_key().default`. Lazy `ConfigService` initialization via `_get_config_service()` is appropriate. - **CLI `plan.py`**: Profile validation moved *before* `use_action()` call (was incorrectly mutating the plan post-creation). New `validate_plan_use_automation_profile()` correctly accepts both built-in bare names and namespaced custom profiles. - **A2A Facade**: `_normalize_optional_string_param()` provides proper input validation with structured `ValidationError` for non-string values. - **Container**: `ConfigService` properly wired as a factory provider into `PlanLifecycleService`. ### Test Quality ✅ - BDD: `@tdd_expected_fail` correctly removed; `@tdd_issue`/`@tdd_issue_1076` retained as permanent regression guards. New scenarios cover plan-level override, action propagation, project config, global fallback, and lazy ConfigService initialization. - Robot: New `use_action_automation_profile.robot` + M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution. - Stabilization: Extensive but justified — the fix enables auto-progression behavior that required existing tests to handle additional terminal states (APPLIED) and phase checks before transitions. ### Standards Compliance ✅ - Commit message: Conventional Changelog format, matches issue metadata exactly - `ISSUES CLOSED: #1076` in footer - No `# type: ignore` suppressions - No secrets or credentials - Type/Bug label and v3.3.0 milestone assigned - Coverage reported at 97.47% (≥97% threshold) ### Minor Observations (non-blocking) - `resolved.source.value == "project"` string comparison in `_resolve_use_action_automation_profile` is slightly fragile but consistent with existing codebase patterns. - Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance. **APPROVED** — Clean bug fix with thorough test coverage and proper spec alignment.
freemo commented 2026-04-02 18:13:51 +00:00
Owner
Copy link

Review claimed by reviewer pool instance pr-reviewer-pool-2813550-1775153400. Dispatching independent code review.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Review claimed by reviewer pool instance pr-reviewer-pool-2813550-1775153400. Dispatching independent code review. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo approved these changes 2026-04-02 18:18:19 +00:00
Dismissed
freemo left a comment
Copy link

Independent Code Review — APPROVED (with merge conflict noted)

Review Scope

Reviewed all 52 changed files (4 production source files, 48 test/infrastructure files) against the specification, CONTRIBUTING.md standards, and issue #1076 requirements.

Specification Alignment ✅

The implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The AutomationProfileRef with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime.

Core Implementation ✅

PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution with proper fallback to ConfigService.validate_key().default. Lazy ConfigService initialization via _get_config_service() is appropriate for optional dependency injection.

CLI plan.py: Profile validation correctly moved before use_action() call — the old code was incorrectly mutating the plan post-creation, which violated the spec's "locked at creation" semantics. New validate_plan_use_automation_profile() correctly accepts both built-in bare names and namespaced custom profiles.

A2A Facade: _normalize_optional_string_param() provides proper input validation with structured ValidationError for non-string values. Clean integration with the plan-create handler.

Container: ConfigService properly wired as a factory provider into PlanLifecycleService.

Test Quality ✅

BDD Coverage:

  • @tdd_expected_fail correctly removed; @tdd_issue/@tdd_issue_1076 retained as permanent regression guards
  • All 4 precedence levels tested: plan override, action propagation, project config, global fallback
  • Lazy ConfigService initialization scenario verifies the fallback path
  • A2A facade wiring tests cover profile forwarding and non-string rejection
  • CLI extension tests cover namespaced and server-qualified profile validation

Robot Coverage:

  • New use_action_automation_profile.robot + helper verify action-to-plan propagation
  • M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution with all 4 levels

Test Stabilization: The extensive stabilization changes (phase-check guards before transitions, APPLIED state acceptance, timeout increases, pabot cleanup) are justified — the fix enables auto-progression behavior that required existing tests to handle additional terminal states.

Standards Compliance ✅

  • Commit message: Conventional Changelog format, matches issue metadata exactly
  • ISSUES CLOSED: #1076 in footer
  • No # type: ignore suppressions
  • No secrets or credentials
  • Type/Bug label and v3.3.0 milestone assigned
  • Coverage reported at 97.47% (≥97% threshold)
  • Single atomic commit covering implementation + tests

Minor Observations (non-blocking)

  • resolved.source.value == "project" string comparison in _resolve_use_action_automation_profile is slightly fragile but consistent with existing codebase patterns
  • Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance

⚠️ Merge Conflict

The PR currently has mergeable: false — there are merge conflicts with master that must be resolved before this can be merged. The code itself is approved; the implementor needs to rebase onto current master.

APPROVED — Clean bug fix with thorough test coverage and proper spec alignment. Blocked only by merge conflicts.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED (with merge conflict noted) ### Review Scope Reviewed all 52 changed files (4 production source files, 48 test/infrastructure files) against the specification, CONTRIBUTING.md standards, and issue #1076 requirements. ### Specification Alignment ✅ The implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The `AutomationProfileRef` with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime. ### Core Implementation ✅ **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution with proper fallback to `ConfigService.validate_key().default`. Lazy `ConfigService` initialization via `_get_config_service()` is appropriate for optional dependency injection. **CLI `plan.py`**: Profile validation correctly moved *before* `use_action()` call — the old code was incorrectly mutating the plan post-creation, which violated the spec's "locked at creation" semantics. New `validate_plan_use_automation_profile()` correctly accepts both built-in bare names and namespaced custom profiles. **A2A Facade**: `_normalize_optional_string_param()` provides proper input validation with structured `ValidationError` for non-string values. Clean integration with the plan-create handler. **Container**: `ConfigService` properly wired as a factory provider into `PlanLifecycleService`. ### Test Quality ✅ **BDD Coverage**: - `@tdd_expected_fail` correctly removed; `@tdd_issue`/`@tdd_issue_1076` retained as permanent regression guards - All 4 precedence levels tested: plan override, action propagation, project config, global fallback - Lazy ConfigService initialization scenario verifies the fallback path - A2A facade wiring tests cover profile forwarding and non-string rejection - CLI extension tests cover namespaced and server-qualified profile validation **Robot Coverage**: - New `use_action_automation_profile.robot` + helper verify action-to-plan propagation - M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution with all 4 levels **Test Stabilization**: The extensive stabilization changes (phase-check guards before transitions, APPLIED state acceptance, timeout increases, pabot cleanup) are justified — the fix enables auto-progression behavior that required existing tests to handle additional terminal states. ### Standards Compliance ✅ - Commit message: Conventional Changelog format, matches issue metadata exactly - `ISSUES CLOSED: #1076` in footer - No `# type: ignore` suppressions - No secrets or credentials - Type/Bug label and v3.3.0 milestone assigned - Coverage reported at 97.47% (≥97% threshold) - Single atomic commit covering implementation + tests ### Minor Observations (non-blocking) - `resolved.source.value == "project"` string comparison in `_resolve_use_action_automation_profile` is slightly fragile but consistent with existing codebase patterns - Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance ### ⚠️ Merge Conflict The PR currently has `mergeable: false` — there are merge conflicts with master that must be resolved before this can be merged. The code itself is approved; the implementor needs to rebase onto current master. **APPROVED** — Clean bug fix with thorough test coverage and proper spec alignment. Blocked only by merge conflicts. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-02 18:18:27 +00:00
Owner
Copy link

⚠️ Merge Conflict — Rebase Required

This PR has been approved after thorough code review, but it currently has merge conflicts with master (mergeable: false). The code quality, spec alignment, test coverage, and standards compliance are all excellent.

@brent.edwards — Please rebase this branch onto current master to resolve the conflicts, then the PR can be merged immediately.

git fetch origin master
git rebase origin/master
git push --force-with-lease

Once the conflicts are resolved and CI passes, this PR will be merged.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## ⚠️ Merge Conflict — Rebase Required This PR has been **approved** after thorough code review, but it currently has merge conflicts with `master` (`mergeable: false`). The code quality, spec alignment, test coverage, and standards compliance are all excellent. **@brent.edwards** — Please rebase this branch onto current `master` to resolve the conflicts, then the PR can be merged immediately. ``` git fetch origin master git rebase origin/master git push --force-with-lease ``` Once the conflicts are resolved and CI passes, this PR will be merged. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-02 19:01:58 +00:00
Owner
Copy link

Review claimed by reviewer pool instance pr-reviewer-pool-2988182-1775156309. Dispatching independent code review.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Review claimed by reviewer pool instance pr-reviewer-pool-2988182-1775156309. Dispatching independent code review. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo approved these changes 2026-04-02 19:05:22 +00:00
Dismissed
freemo left a comment
Copy link

Independent Code Review — APPROVED (merge blocked by conflicts)

Review Scope

Reviewed all 52 changed files (4 production, 48 test/infrastructure) against the specification, CONTRIBUTING.md standards, and issue #1076 requirements.

Specification Alignment ✅

The implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The AutomationProfileRef with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime.

Core Implementation ✅

PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution with proper fallback to ConfigService.validate_key().default. Lazy ConfigService initialization via _get_config_service() is appropriate for optional dependency injection.

CLI plan.py: Profile validation correctly moved before use_action() call — the old code was incorrectly mutating the plan post-creation, which violated the spec's "locked at creation" semantics. New validate_plan_use_automation_profile() correctly accepts both built-in bare names and namespaced custom profiles.

A2A Facade: _normalize_optional_string_param() provides proper input validation with structured ValidationError for non-string values. Clean integration with the plan-create handler.

Container: ConfigService properly wired as a factory provider into PlanLifecycleService.

Test Quality ✅

BDD Coverage:

  • @tdd_expected_fail correctly removed; @tdd_issue/@tdd_issue_1076 retained as permanent regression guards
  • All 4 precedence levels tested: plan override, action propagation, project config, global fallback
  • Lazy ConfigService initialization scenario verifies the fallback path
  • A2A facade wiring tests cover profile forwarding and non-string rejection
  • CLI extension tests cover namespaced and server-qualified profile validation

Robot Coverage:

  • New use_action_automation_profile.robot + helper verify action-to-plan propagation
  • M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution with all 4 levels
  • Test stabilization changes (phase-check guards, APPLIED state acceptance, timeout increases) are justified by the auto-progression behavior this fix enables

Standards Compliance ✅

  • Commit message: Conventional Changelog format, matches issue metadata exactly
  • ISSUES CLOSED: #1076 in footer
  • No # type: ignore suppressions
  • No secrets or credentials
  • Type/Bug label and v3.3.0 milestone assigned
  • Coverage reported at 97.47% (≥97% threshold)
  • Single atomic commit covering implementation + tests

Minor Observations (non-blocking)

  1. resolved.source.value == "project" string comparison in _resolve_use_action_automation_profile is slightly fragile but consistent with existing codebase patterns
  2. Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance
  3. The noxfile.py pabot cleanup is a good defensive measure against stale worker artifacts

⚠️ Merge Conflict + Overlapping Fix on Master

The PR currently has mergeable: false. Additionally, I note that commit 34dcf522 (PR #1196, "feat(autonomy): automation profile resolution precedence correct") has already been merged to master and implements the same core fix — a _resolve_plan_profile_ref method in PlanLifecycleService with the same 4-level precedence chain. This means:

  1. The core production code in this PR likely overlaps significantly with what's already on master
  2. A rebase will require careful reconciliation with the existing implementation
  3. This PR may still have value for its additional test coverage (A2A facade wiring, CLI namespaced profile validation, M6 provenance tests) and infrastructure hardening (timeout increases, pabot cleanup, resource DAG session fixes)

@brent.edwards — Before rebasing, please evaluate whether this PR's remaining unique contributions (beyond what #1196 already delivered) justify the rebase effort, or whether this PR should be closed as superseded.

APPROVED — Code quality, spec alignment, and test coverage are excellent. Blocked by merge conflicts and potential overlap with #1196.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED (merge blocked by conflicts) ### Review Scope Reviewed all 52 changed files (4 production, 48 test/infrastructure) against the specification, CONTRIBUTING.md standards, and issue #1076 requirements. ### Specification Alignment ✅ The implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. The `AutomationProfileRef` with provenance tracking is a clean design that locks the resolved profile to the plan for its lifetime. ### Core Implementation ✅ **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution with proper fallback to `ConfigService.validate_key().default`. Lazy `ConfigService` initialization via `_get_config_service()` is appropriate for optional dependency injection. **CLI `plan.py`**: Profile validation correctly moved *before* `use_action()` call — the old code was incorrectly mutating the plan post-creation, which violated the spec's "locked at creation" semantics. New `validate_plan_use_automation_profile()` correctly accepts both built-in bare names and namespaced custom profiles. **A2A Facade**: `_normalize_optional_string_param()` provides proper input validation with structured `ValidationError` for non-string values. Clean integration with the plan-create handler. **Container**: `ConfigService` properly wired as a factory provider into `PlanLifecycleService`. ### Test Quality ✅ **BDD Coverage**: - `@tdd_expected_fail` correctly removed; `@tdd_issue`/`@tdd_issue_1076` retained as permanent regression guards - All 4 precedence levels tested: plan override, action propagation, project config, global fallback - Lazy ConfigService initialization scenario verifies the fallback path - A2A facade wiring tests cover profile forwarding and non-string rejection - CLI extension tests cover namespaced and server-qualified profile validation **Robot Coverage**: - New `use_action_automation_profile.robot` + helper verify action-to-plan propagation - M6 autonomy acceptance tests cover facade profile forwarding and full provenance resolution with all 4 levels - Test stabilization changes (phase-check guards, APPLIED state acceptance, timeout increases) are justified by the auto-progression behavior this fix enables ### Standards Compliance ✅ - Commit message: Conventional Changelog format, matches issue metadata exactly - `ISSUES CLOSED: #1076` in footer - No `# type: ignore` suppressions - No secrets or credentials - Type/Bug label and v3.3.0 milestone assigned - Coverage reported at 97.47% (≥97% threshold) - Single atomic commit covering implementation + tests ### Minor Observations (non-blocking) 1. `resolved.source.value == "project"` string comparison in `_resolve_use_action_automation_profile` is slightly fragile but consistent with existing codebase patterns 2. Timeout increases (30s→90s, 120s→240s) in Robot tests are generous but appropriate for CI parallelism variance 3. The `noxfile.py` pabot cleanup is a good defensive measure against stale worker artifacts ### ⚠️ Merge Conflict + Overlapping Fix on Master The PR currently has `mergeable: false`. Additionally, I note that commit `34dcf522` (PR #1196, "feat(autonomy): automation profile resolution precedence correct") has already been merged to master and implements the same core fix — a `_resolve_plan_profile_ref` method in `PlanLifecycleService` with the same 4-level precedence chain. This means: 1. The core production code in this PR likely **overlaps significantly** with what's already on master 2. A rebase will require careful reconciliation with the existing implementation 3. This PR may still have value for its additional test coverage (A2A facade wiring, CLI namespaced profile validation, M6 provenance tests) and infrastructure hardening (timeout increases, pabot cleanup, resource DAG session fixes) **@brent.edwards** — Before rebasing, please evaluate whether this PR's remaining unique contributions (beyond what #1196 already delivered) justify the rebase effort, or whether this PR should be closed as superseded. **APPROVED** — Code quality, spec alignment, and test coverage are excellent. Blocked by merge conflicts and potential overlap with #1196. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-02 19:33:41 +00:00
Owner
Copy link

Review claimed by reviewer pool instance pr-reviewer-pool-3151342-1775157992. Dispatching independent code review.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Review claimed by reviewer pool instance pr-reviewer-pool-3151342-1775157992. Dispatching independent code review. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo commented 2026-04-02 19:46:20 +00:00
Owner
Copy link

Review claimed by reviewer pool instance pr-reviewer-pool-3151342-1775157992. Dispatching independent code review.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Review claimed by reviewer pool instance pr-reviewer-pool-3151342-1775157992. Dispatching independent code review. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo commented 2026-04-02 23:04:03 +00:00
Owner
Copy link

Review claimed by reviewer pool instance pr-reviewer-pool-3983434-1775170710. Dispatching independent code review.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Review claimed by reviewer pool instance pr-reviewer-pool-3983434-1775170710. Dispatching independent code review. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo approved these changes 2026-04-03 01:09:23 +00:00
Dismissed
freemo left a comment
Copy link

Independent Code Review — APPROVED (cannot merge: conflicts + superseded by #1196)

Review Scope

Reviewed all 4 production files and sampled test/infrastructure changes against the specification, CONTRIBUTING.md standards, and issue #1076 requirements.

Code Quality Assessment ✅

PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution (plan > action > project > global) using ConfigService.resolve() for project/global lookups and ConfigService.validate_key().default as ultimate fallback. Lazy ConfigService initialization via _get_config_service() is appropriate for optional DI.

CLI plan.py: validate_plan_use_automation_profile() is a well-designed validator that correctly accepts both built-in bare names and namespaced custom profiles (including server-qualified server:ns/name). Profile validation is correctly moved before use_action() — the old code was mutating the plan post-creation, violating the spec's "locked at creation" semantics.

A2A Facade: _normalize_optional_string_param() provides proper input validation with structured ValidationError for non-string values. Clean integration with _handle_plan_create.

Container: ConfigService properly wired as a factory provider into PlanLifecycleService.

Specification Alignment ✅

The implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967.

Standards Compliance ✅

  • Conventional Changelog commit format ✓
  • Closes #1076 present ✓
  • Type/Bug label and v3.3.0 milestone ✓
  • No # type: ignore suppressions ✓
  • Coverage reported at 97.47% ✓

⛔ Merge Blocked — Two Issues

1. Merge Conflicts (mergeable: false)
The PR has unresolvable merge conflicts with current master.

2. Core Fix Superseded by PR #1196
Commit 34dcf522 (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master on April 1 and implements the same core fix:

  • Same config_service parameter on PlanLifecycleService.__init__()
  • Same automation_profile parameter on use_action()
  • Same 4-level precedence resolution method (_resolve_plan_profile_ref)
  • Same AutomationProfileRef with provenance tracking on the Plan
  • Same CLI plan.py restructuring

Unique contributions in this PR not in #1196:

  • A2A facade _normalize_optional_string_param() + _handle_plan_create wiring
  • Container ConfigService factory provider registration
  • validate_plan_use_automation_profile() with namespaced profile support
  • Robot tests: use_action_automation_profile.robot, m6_autonomy_acceptance.robot
  • Various Robot test stabilization (timeout increases, phase-check guards, pabot cleanup)

Recommendation

Since issue #1076 is already closed and the core fix is on master via #1196, this PR should be closed as superseded. If the unique A2A facade wiring, CLI validation, and Robot test additions are still needed, they should be extracted into a new PR rebased on current master.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED (cannot merge: conflicts + superseded by #1196) ### Review Scope Reviewed all 4 production files and sampled test/infrastructure changes against the specification, CONTRIBUTING.md standards, and issue #1076 requirements. ### Code Quality Assessment ✅ **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution (plan > action > project > global) using `ConfigService.resolve()` for project/global lookups and `ConfigService.validate_key().default` as ultimate fallback. Lazy `ConfigService` initialization via `_get_config_service()` is appropriate for optional DI. **CLI `plan.py`**: `validate_plan_use_automation_profile()` is a well-designed validator that correctly accepts both built-in bare names and namespaced custom profiles (including server-qualified `server:ns/name`). Profile validation is correctly moved *before* `use_action()` — the old code was mutating the plan post-creation, violating the spec's "locked at creation" semantics. **A2A Facade**: `_normalize_optional_string_param()` provides proper input validation with structured `ValidationError` for non-string values. Clean integration with `_handle_plan_create`. **Container**: `ConfigService` properly wired as a factory provider into `PlanLifecycleService`. ### Specification Alignment ✅ The implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. ### Standards Compliance ✅ - Conventional Changelog commit format ✓ - `Closes #1076` present ✓ - Type/Bug label and v3.3.0 milestone ✓ - No `# type: ignore` suppressions ✓ - Coverage reported at 97.47% ✓ ### ⛔ Merge Blocked — Two Issues **1. Merge Conflicts (`mergeable: false`)** The PR has unresolvable merge conflicts with current `master`. **2. Core Fix Superseded by PR #1196** Commit `34dcf522` (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master on April 1 and implements the **same core fix**: - Same `config_service` parameter on `PlanLifecycleService.__init__()` - Same `automation_profile` parameter on `use_action()` - Same 4-level precedence resolution method (`_resolve_plan_profile_ref`) - Same `AutomationProfileRef` with provenance tracking on the Plan - Same CLI `plan.py` restructuring **Unique contributions in this PR not in #1196:** - A2A facade `_normalize_optional_string_param()` + `_handle_plan_create` wiring - Container `ConfigService` factory provider registration - `validate_plan_use_automation_profile()` with namespaced profile support - Robot tests: `use_action_automation_profile.robot`, `m6_autonomy_acceptance.robot` - Various Robot test stabilization (timeout increases, phase-check guards, pabot cleanup) ### Recommendation Since issue #1076 is already **closed** and the core fix is on master via #1196, this PR should be **closed as superseded**. If the unique A2A facade wiring, CLI validation, and Robot test additions are still needed, they should be extracted into a new PR rebased on current master. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-03 01:09:35 +00:00
Owner
Copy link

⛔ Merge Blocked — PR Superseded

This PR has been approved on code quality, spec alignment, and standards compliance. However, it cannot be merged for two reasons:

  1. Merge conflicts — mergeable: false against current master
  2. Core fix already on master — PR #1196 (commit 34dcf522, merged April 1) implements the same 4-level automation profile precedence resolution in PlanLifecycleService

Issue #1076 is already closed. The core fix this PR was created for has been delivered.

Unique Contributions Worth Preserving

This PR contains several additions not in #1196 that may still have value:

  • A2A facade _normalize_optional_string_param() + _handle_plan_create automation profile wiring
  • Container ConfigService factory provider registration
  • validate_plan_use_automation_profile() with namespaced/server-qualified profile support
  • Robot tests: use_action_automation_profile.robot, m6_autonomy_acceptance.robot
  • Robot test stabilization (timeout increases, phase-check guards, pabot cleanup)

@brent.edwards — Please consider closing this PR as superseded. If the unique contributions above are still needed, extract them into a new PR rebased on current master.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## ⛔ Merge Blocked — PR Superseded This PR has been **approved** on code quality, spec alignment, and standards compliance. However, it **cannot be merged** for two reasons: 1. **Merge conflicts** — `mergeable: false` against current `master` 2. **Core fix already on master** — PR #1196 (commit `34dcf522`, merged April 1) implements the same 4-level automation profile precedence resolution in `PlanLifecycleService` **Issue #1076 is already closed.** The core fix this PR was created for has been delivered. ### Unique Contributions Worth Preserving This PR contains several additions not in #1196 that may still have value: - A2A facade `_normalize_optional_string_param()` + `_handle_plan_create` automation profile wiring - Container `ConfigService` factory provider registration - `validate_plan_use_automation_profile()` with namespaced/server-qualified profile support - Robot tests: `use_action_automation_profile.robot`, `m6_autonomy_acceptance.robot` - Robot test stabilization (timeout increases, phase-check guards, pabot cleanup) **@brent.edwards** — Please consider closing this PR as superseded. If the unique contributions above are still needed, extract them into a new PR rebased on current master. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-03 03:54:30 +00:00
Owner
Copy link

Merge conflict detected. This PR has mergeable: false — the branch has conflicts with master. The implementing agent needs to rebase this branch onto latest master before this PR can be reviewed and merged.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

Merge conflict detected. This PR has `mergeable: false` — the branch has conflicts with master. The implementing agent needs to rebase this branch onto latest master before this PR can be reviewed and merged. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo commented 2026-04-03 18:44:29 +00:00
Owner
Copy link

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775242400]

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775242400] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo approved these changes 2026-04-03 18:56:44 +00:00
Dismissed
freemo left a comment
Copy link

Independent Code Review — APPROVED (cannot merge: superseded + conflicts)

Review Scope

Reviewed all 52 changed files (4 production source, 48 test/infrastructure) against the specification and CONTRIBUTING.md standards.

Code Quality Assessment ✅

Production code is well-written and spec-compliant:

  • PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution (plan > action > project > global) with proper ConfigService integration and lazy initialization via _get_config_service(). The AutomationProfileRef with AutomationProfileProvenance tracking is a clean design.
  • CLI plan.py — validate_plan_use_automation_profile(): Correctly validates both built-in bare names and namespaced custom profiles (including server-qualified server:ns/name). Profile validation is properly moved before use_action() — the old code was mutating the plan post-creation, violating the spec's "locked at creation" semantics.
  • A2A Facade — _normalize_optional_string_param(): Proper input validation with structured ValidationError for non-string values. Clean integration with _handle_plan_create.
  • Container: ConfigService properly wired as a factory provider into PlanLifecycleService.

Specification Alignment ✅

Implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967.

Standards Compliance ✅

  • Conventional Changelog commit format ✓
  • Closes #1076 present ✓
  • Type/Bug label and v3.3.0 milestone ✓
  • No # type: ignore suppressions ✓
  • Coverage reported at 97.47% (≥97% threshold) ✓

⛔ Cannot Merge — Two Blocking Issues

1. Merge Conflicts (mergeable: false)
The branch is based on an older master and has significant conflicts. The A2A facade diff shows the branch would revert unrelated changes already on master (method→operation renaming, A2aVersion additions, session handling refactoring, plan prompt handler changes). A naive rebase would be non-trivial.

2. Core Fix Already Delivered via PR #1196
Commit 34dcf522 (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master on April 1 and implements the same core fix:

  • Same _resolve_plan_profile_ref method with 4-level precedence in PlanLifecycleService
  • Same AutomationProfileRef with provenance tracking
  • Same automation_profile parameter on use_action()
  • Same CLI restructuring moving validation before plan creation

Issue #1076 is already closed (closed 2026-03-30).

Unique Contributions Not on Master

This PR contains additions not in #1196 that may have independent value:

  • A2A facade _normalize_optional_string_param() + _handle_plan_create automation profile wiring
  • Container ConfigService factory provider registration
  • validate_plan_use_automation_profile() with namespaced/server-qualified profile support
  • Robot tests: use_action_automation_profile.robot, m6_autonomy_acceptance.robot
  • Extensive Robot test stabilization (timeout increases, phase-check guards, pabot cleanup)

Recommendation

Close this PR as superseded. If the unique A2A facade wiring, CLI validation, and Robot test additions are still needed, they should be extracted into a new PR rebased on current master with a new issue ticket.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED (cannot merge: superseded + conflicts) ### Review Scope Reviewed all 52 changed files (4 production source, 48 test/infrastructure) against the specification and CONTRIBUTING.md standards. ### Code Quality Assessment ✅ **Production code is well-written and spec-compliant:** - **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution (plan > action > project > global) with proper `ConfigService` integration and lazy initialization via `_get_config_service()`. The `AutomationProfileRef` with `AutomationProfileProvenance` tracking is a clean design. - **CLI `plan.py` — `validate_plan_use_automation_profile()`**: Correctly validates both built-in bare names and namespaced custom profiles (including server-qualified `server:ns/name`). Profile validation is properly moved *before* `use_action()` — the old code was mutating the plan post-creation, violating the spec's "locked at creation" semantics. - **A2A Facade — `_normalize_optional_string_param()`**: Proper input validation with structured `ValidationError` for non-string values. Clean integration with `_handle_plan_create`. - **Container**: `ConfigService` properly wired as a factory provider into `PlanLifecycleService`. ### Specification Alignment ✅ Implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. ### Standards Compliance ✅ - Conventional Changelog commit format ✓ - `Closes #1076` present ✓ - Type/Bug label and v3.3.0 milestone ✓ - No `# type: ignore` suppressions ✓ - Coverage reported at 97.47% (≥97% threshold) ✓ ### ⛔ Cannot Merge — Two Blocking Issues **1. Merge Conflicts (`mergeable: false`)** The branch is based on an older master and has significant conflicts. The A2A facade diff shows the branch would **revert** unrelated changes already on master (method→operation renaming, A2aVersion additions, session handling refactoring, plan prompt handler changes). A naive rebase would be non-trivial. **2. Core Fix Already Delivered via PR #1196** Commit `34dcf522` (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master on April 1 and implements the **same core fix**: - Same `_resolve_plan_profile_ref` method with 4-level precedence in `PlanLifecycleService` - Same `AutomationProfileRef` with provenance tracking - Same `automation_profile` parameter on `use_action()` - Same CLI restructuring moving validation before plan creation **Issue #1076 is already closed** (closed 2026-03-30). ### Unique Contributions Not on Master This PR contains additions not in #1196 that may have independent value: - A2A facade `_normalize_optional_string_param()` + `_handle_plan_create` automation profile wiring - Container `ConfigService` factory provider registration - `validate_plan_use_automation_profile()` with namespaced/server-qualified profile support - Robot tests: `use_action_automation_profile.robot`, `m6_autonomy_acceptance.robot` - Extensive Robot test stabilization (timeout increases, phase-check guards, pabot cleanup) ### Recommendation **Close this PR as superseded.** If the unique A2A facade wiring, CLI validation, and Robot test additions are still needed, they should be extracted into a new PR rebased on current master with a new issue ticket. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-05 03:20:46 +00:00
Owner
Copy link

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775359200]

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775359200] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo approved these changes 2026-04-05 03:42:09 +00:00
freemo left a comment
Copy link

Independent Code Review — APPROVED (cannot merge: conflicts + superseded)

Review Scope

Reviewed the single commit (34897a83) covering 4 production files and 48 test/infrastructure files against the specification, CONTRIBUTING.md standards, and issue #1076 requirements.

Code Quality ✅

The implementation is well-designed and spec-compliant:

  • PlanLifecycleService._resolve_use_action_automation_profile(): Clean 4-level precedence resolution (plan > action > project > global) with proper ConfigService integration and lazy initialization via _get_config_service().
  • CLI plan.py: validate_plan_use_automation_profile() correctly validates both built-in bare names and namespaced custom profiles. Profile validation properly moved before use_action() call.
  • A2A Facade: _normalize_optional_string_param() provides proper input validation with structured ValidationError.
  • Container: ConfigService properly wired as a factory provider.

Specification Alignment ✅

Implementation correctly resolves automation profiles at plan use time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967.

Standards Compliance ✅

  • Conventional Changelog commit format ✓
  • Closes #1076 present ✓
  • Type/Bug label and v3.3.0 milestone ✓
  • No # type: ignore suppressions ✓
  • Single atomic commit ✓

⛔ Cannot Merge — Two Blocking Issues

1. Merge Conflicts (mergeable: false)
The branch has diverged significantly from master. A test merge attempt shows hundreds of conflicting files — the branch is based on an old master state and would require a non-trivial rebase.

2. Core Fix Already Delivered via PR #1196
Commit 34dcf522 (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master and implements the same core fix:

  • Same _resolve_plan_profile_ref method with 4-level precedence in PlanLifecycleService
  • Same AutomationProfileRef with provenance tracking
  • Same automation_profile parameter on use_action()
  • Same CLI restructuring

Issue #1076 is already closed (closed 2026-03-30). The core fix this PR was created for has been delivered.

Recommendation

This PR should be closed as superseded. The code quality is excellent, but the fix is already on master. If the unique contributions (A2A facade wiring, container ConfigService registration, namespaced profile validation, Robot test additions, test stabilization) are still needed, they should be extracted into a new PR rebased on current master.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED (cannot merge: conflicts + superseded) ### Review Scope Reviewed the single commit (`34897a83`) covering 4 production files and 48 test/infrastructure files against the specification, CONTRIBUTING.md standards, and issue #1076 requirements. ### Code Quality ✅ The implementation is well-designed and spec-compliant: - **`PlanLifecycleService._resolve_use_action_automation_profile()`**: Clean 4-level precedence resolution (plan > action > project > global) with proper `ConfigService` integration and lazy initialization via `_get_config_service()`. - **CLI `plan.py`**: `validate_plan_use_automation_profile()` correctly validates both built-in bare names and namespaced custom profiles. Profile validation properly moved before `use_action()` call. - **A2A Facade**: `_normalize_optional_string_param()` provides proper input validation with structured `ValidationError`. - **Container**: `ConfigService` properly wired as a factory provider. ### Specification Alignment ✅ Implementation correctly resolves automation profiles at `plan use` time using the spec-mandated 4-level precedence chain (plan > action > project > global), per specification lines 18919 and 18967. ### Standards Compliance ✅ - Conventional Changelog commit format ✓ - `Closes #1076` present ✓ - Type/Bug label and v3.3.0 milestone ✓ - No `# type: ignore` suppressions ✓ - Single atomic commit ✓ ### ⛔ Cannot Merge — Two Blocking Issues **1. Merge Conflicts (`mergeable: false`)** The branch has diverged significantly from master. A test merge attempt shows hundreds of conflicting files — the branch is based on an old master state and would require a non-trivial rebase. **2. Core Fix Already Delivered via PR #1196** Commit `34dcf522` (PR #1196, "feat(autonomy): automation profile resolution precedence correct") was merged to master and implements the **same core fix**: - Same `_resolve_plan_profile_ref` method with 4-level precedence in `PlanLifecycleService` - Same `AutomationProfileRef` with provenance tracking - Same `automation_profile` parameter on `use_action()` - Same CLI restructuring **Issue #1076 is already closed** (closed 2026-03-30). The core fix this PR was created for has been delivered. ### Recommendation **This PR should be closed as superseded.** The code quality is excellent, but the fix is already on master. If the unique contributions (A2A facade wiring, container `ConfigService` registration, namespaced profile validation, Robot test additions, test stabilization) are still needed, they should be extracted into a new PR rebased on current master. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-05 03:44:11 +00:00
Owner
Copy link

⛔ Cannot Merge — PR Superseded by #1196

This PR has been approved on code quality, spec alignment, and standards compliance (9th approval review). However, it cannot be merged due to:

  1. Merge conflicts — mergeable: false. The branch has diverged massively from master (hundreds of conflicting files).
  2. Core fix already on master — PR #1196 (commit 34dcf522) implements the same 4-level automation profile precedence resolution in PlanLifecycleService.
  3. Issue #1076 is already closed (closed 2026-03-30).

Recommendation: Close this PR as superseded. If unique contributions (A2A facade wiring, Robot test additions, test stabilization) are still needed, extract them into a new issue and PR rebased on current master.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## ⛔ Cannot Merge — PR Superseded by #1196 This PR has been **approved** on code quality, spec alignment, and standards compliance (9th approval review). However, it **cannot be merged** due to: 1. **Merge conflicts** — `mergeable: false`. The branch has diverged massively from master (hundreds of conflicting files). 2. **Core fix already on master** — PR #1196 (commit `34dcf522`) implements the same 4-level automation profile precedence resolution in `PlanLifecycleService`. 3. **Issue #1076 is already closed** (closed 2026-03-30). **Recommendation**: Close this PR as superseded. If unique contributions (A2A facade wiring, Robot test additions, test stabilization) are still needed, extract them into a new issue and PR rebased on current master. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo commented 2026-04-05 04:49:28 +00:00
Owner
Copy link

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775364500]

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-1178-1775364500] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo reviewed 2026-04-05 04:54:55 +00:00
freemo left a comment
Copy link

Final Review — Closing as Superseded

Summary

This PR implements a correct and well-designed fix for issue #1076 (automation profile resolution at plan-use time with 4-level precedence). The code quality, spec alignment, and test coverage are all excellent. However, this PR cannot and should not be merged for the following reasons:

⛔ Blocking Issues

1. Core Fix Already on Master via PR #1196
PR #1196 ("feat(autonomy): automation profile resolution precedence correct") was merged to master on April 1, 2026 (commit 34dcf522). It implements the identical core fix:

  • Same _resolve_plan_profile_ref method with 4-level precedence in PlanLifecycleService
  • Same AutomationProfileRef with provenance tracking
  • Same automation_profile parameter on use_action()
  • Same CLI restructuring moving validation before plan creation

2. Issue #1076 is Already Closed
The linked issue was closed on March 30, 2026. The fix this PR was created for has been delivered.

3. Merge Conflicts (mergeable: false)
The branch has diverged significantly from master with extensive conflicts. A rebase would be non-trivial and pointless given the fix is already on master.

Unique Contributions Not on Master

This PR does contain some additions not present in #1196 that may have independent value:

  • A2A facade _normalize_optional_string_param() + _handle_plan_create automation profile wiring
  • Container ConfigService factory provider registration
  • validate_plan_use_automation_profile() with namespaced/server-qualified profile support
  • Robot tests: use_action_automation_profile.robot, m6_autonomy_acceptance.robot
  • Extensive Robot test stabilization (timeout increases, phase-check guards, pabot cleanup)

If these are still needed, they should be extracted into a new issue and PR rebased on current master.

Decision

Closing this PR as superseded. The code quality was excellent — this is purely a timing/overlap issue where PR #1196 delivered the same fix first.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Final Review — Closing as Superseded ### Summary This PR implements a correct and well-designed fix for issue #1076 (automation profile resolution at plan-use time with 4-level precedence). The code quality, spec alignment, and test coverage are all excellent. However, **this PR cannot and should not be merged** for the following reasons: ### ⛔ Blocking Issues **1. Core Fix Already on Master via PR #1196** PR #1196 ("feat(autonomy): automation profile resolution precedence correct") was merged to `master` on **April 1, 2026** (commit `34dcf522`). It implements the **identical core fix**: - Same `_resolve_plan_profile_ref` method with 4-level precedence in `PlanLifecycleService` - Same `AutomationProfileRef` with provenance tracking - Same `automation_profile` parameter on `use_action()` - Same CLI restructuring moving validation before plan creation **2. Issue #1076 is Already Closed** The linked issue was closed on **March 30, 2026**. The fix this PR was created for has been delivered. **3. Merge Conflicts (`mergeable: false`)** The branch has diverged significantly from master with extensive conflicts. A rebase would be non-trivial and pointless given the fix is already on master. ### Unique Contributions Not on Master This PR does contain some additions not present in #1196 that may have independent value: - A2A facade `_normalize_optional_string_param()` + `_handle_plan_create` automation profile wiring - Container `ConfigService` factory provider registration - `validate_plan_use_automation_profile()` with namespaced/server-qualified profile support - Robot tests: `use_action_automation_profile.robot`, `m6_autonomy_acceptance.robot` - Extensive Robot test stabilization (timeout increases, phase-check guards, pabot cleanup) If these are still needed, they should be extracted into a new issue and PR rebased on current master. ### Decision **Closing this PR as superseded.** The code quality was excellent — this is purely a timing/overlap issue where PR #1196 delivered the same fix first. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo closed this pull request 2026-04-05 04:55:43 +00:00
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
Details
CI / build (pull_request) Successful in 20s
Required
Details
CI / helm (pull_request) Successful in 26s
Details
CI / lint (pull_request) Successful in 3m18s
Required
Details
CI / quality (pull_request) Successful in 3m43s
Required
Details
CI / integration_tests (pull_request) Successful in 3m45s
Required
Details
CI / typecheck (pull_request) Successful in 3m55s
Required
Details
CI / unit_tests (pull_request) Successful in 4m1s
Required
Details
CI / security (pull_request) Successful in 4m8s
Required
Details
CI / docker (pull_request) Successful in 2m4s
Required
Details
CI / e2e_tests (pull_request) Successful in 16m33s
Details
CI / coverage (pull_request) Successful in 13m5s
Required
Details
CI / status-check (pull_request) Successful in 1s
Details
CI / benchmark-regression (pull_request) Successful in 52m47s
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
aditya
freemo
Labels
Clear labels   
auto/needs-reevaluation
Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.

  
controller-managed
Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.

  
auto/blocked-by-deps
PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.

  
auto/ci-timeout
Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.

  
auto/claimed-implementer
Currently being processed by an implementer worker.

  
auto/claimed-merge
Currently being processed by the merge driver.

  
auto/claimed-reviewer
Currently being processed by a reviewer worker.

  
auto/driver-down
Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.

  
auto/invariant-violation
Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.

  
auto/last-attempt-tier-0
In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.

  
auto/last-attempt-tier-1
In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.

  
auto/last-attempt-tier-2
In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.

  
auto/last-attempt-tier-min
In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.

  
Automation Tracking
Tracking issues used by the AI Automation system for agents to communicate and report.

  
auto/needs-conflict-resolution
Rebase conflict needs LLM conflict-resolver.

  
auto/needs-implementer
Failing CI needs implementer attention.

  
auto/postmortem
Documenting a driver incident or rollback.

  
auto/ready-to-merge
Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.

  
auto/restart-throttled
Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.

  
auto/revert
Revert PR backing out an invariant violation. Fast-tracked through the merge driver.

  
auto/sentinel
Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.

  
auto/stale-inactivity
No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.

  
auto/unstable
Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.

  
Blocked
A ticket in a blocked state and unable to complete until some other task is completed first.

  
Bounty
$100
 A bounty of $100 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$1000
 A bounty of $1000 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$10000
 A bounty of $10000 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$20
 A bounty of $20 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$2000
 A bounty of $2000 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$250
 A bounty of $250 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$50
 A bounty of $50 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$500
 A bounty of $500 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$5000
 A bounty of $5000 for any open-source contributor who provides a MR that solves this issue

  
Bounty
$750
 A bounty of $750 for any open-source contributor who provides a MR that solves this issue

  
MoSCoW
Could have
 Could have feature in order to satisfy the epic/legendary.

  
MoSCoW
Must have
 Must have feature in order to satisfy the epic/legendary.

  
MoSCoW
Should have
 Should have feature in order to satisfy the epic/legendary.

  
Needs Feedback
There are questions in the ticket that can not be completed until the project owner provides clarity.

  
Points
1
 1 man-hours worth of work for an expert with no learning curve.

  
Points
13
 13 man-hours worth of work for an expert with no learning curve.

  
Points
2
 2 man-hours worth of work for an expert with no learning curve.

  
Points
21
 21 man-hours worth of work for an expert with no learning curve.

  
Points
3
 3 man-hours worth of work for an expert with no learning curve.

  
Points
34
 34 man-hours worth of work for an expert with no learning curve.

  
Points
5
 5 man-hours worth of work for an expert with no learning curve.

  
Points
55
 55 man-hours worth of work for an expert with no learning curve.

  
Points
8
 8 man-hours worth of work for an expert with no learning curve.

  
Points
88
 88 man-hours worth of work for an expert with no learning curve.

  
Priority
Backlog
 This ticket has backlogged priority and is not to be worked on yet

  
Priority
CI Blocker
 Critical priority issue that blocks CI/CD pipeline and prevents PR merges

  
Priority
Critical
 The priority is critical

  
Priority
High
 The priority is high

  
Priority
Low
 The priority is low

  
Priority
Medium
 The priority is medium

  
Signed-off: Owner
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  
Signed-off: Scrum Master
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  
Signed-off: Tech Lead
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  
Spike
A ticket for learning a tool or technology that is needed to be able to do future planning and design.

  
State
Completed
 The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.

  
State
Duplicate
 A ticket that represents the same content as an existing ticket.

  
State
In Progress
 A ticket that is actively being developed.

  
State
In Review
 A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.

  
State
Paused
 This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.

  
State
Unverified
 All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.

  
State
Verified
 The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.

  
State
Wont Do
 This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.

  
Type
Automation
 Any edits or discussion about the AI automated coding system.

  
Type
Bug
 Something that doesnt work as intended.

  
Type
Discussion
 Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.

  
Type
Documentation
 An error or improvement needed in the documentation.

  
Type
Epic
 Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.

  
Type
Feature
 Some new functionality not present.

  
Type
Legendary
 A type of Epic which will contain other Epics.

  
Type
Refactor
 A code change that restructures existing code without changing its external behavior.

  
Type
Support
 Someone needs help using the project.

  
Type
Task
 A generic task that doesnt fit into the other type categories.

  
Type
Testing
 Work exclusively focusing on fixing or expanding testing.

No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
Milestone
Clear milestone
No items
No milestone
v3.3.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
freemo
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.

Depends on
#1076 bug(plan): use_action() does not resolve or propagate automation_profile to created Plan
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core!1178
No description provided.