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

Fix cleanup_stale to preserve branches with commits beyond HEAD (#11121) #11132

Closed
HAL9000 wants to merge 3 commits from fix/cleanup-stale-preserve-commits into master
pull from: fix/cleanup-stale-preserve-commits
merge into: cleveragents:master
Conversation 5 Commits 3 Files changed 5 +173 -3
HAL9000 commented 2026-05-11 22:48:08 +00:00
Owner
Copy link

Summary

This PR fixes issue #11121 where cleanup_stale was destroying git worktree branches that contained commits beyond HEAD, causing plan apply to fail in finding artifacts on re-invocation.

Problem

When cleanup_stale was re-invoked during a plan execution, it would destroy branches containing LLM-produced changes from previous plan executions. This prevented plan apply from discovering committed LLM output, resulting in zero artifacts being found.

Solution

Modified src/cleveragents/infrastructure/sandbox/git_worktree.py to add a commits-beyond-HEAD check in the cleanup_stale method:

  • Checks if a branch has commits beyond HEAD (git log --oneline HEAD...branch) before destroying it
  • Preserves branches containing LLM-produced changes from previous plan executions
  • Only cleans up branches with no committed work (truly stale branches)

Changes

  • src/cleveragents/infrastructure/sandbox/git_worktree.py - Core fix: commits-beyond-HEAD check added to cleanup_stale, docstring updated
  • CHANGELOG.md - Added entry under [Unreleased]/### Fixed
  • CONTRIBUTORS.md - Added HAL 9000 contribution note
  • features/sandbox_reexecute_with_changes.feature - New BDD test (3 scenarios)
  • features/steps/sandbox_reexecute_with_changes_steps.py - New step definitions

PR Compliance Checklist

  • CHANGELOG.md entry under [Unreleased] section
  • CONTRIBUTORS.md updated with contribution note
  • Commit footer includes "ISSUES CLOSED: #11121"
  • BDD/Behave tests added for the changed behaviour
  • CI passes — CI run in progress, awaiting results on commit af2e1767
  • Epic reference — see Cross References below

Cross References

  • Parent Issue: #11121 — cleanup_stale destroys git worktree branch on re-invoked execute, causing plan apply to find zero artifacts
  • TDD Issue Capture: #11120 (companion test for the bug)
  • Related PR: #11127 (WIP: guard against cleanup_stale destroying execute output)

Closes #11121

## Summary This PR fixes issue #11121 where `cleanup_stale` was destroying git worktree branches that contained commits beyond HEAD, causing plan apply to fail in finding artifacts on re-invocation. ## Problem When `cleanup_stale` was re-invoked during a plan execution, it would destroy branches containing LLM-produced changes from previous plan executions. This prevented plan apply from discovering committed LLM output, resulting in zero artifacts being found. ## Solution Modified `src/cleveragents/infrastructure/sandbox/git_worktree.py` to add a commits-beyond-HEAD check in the `cleanup_stale` method: - Checks if a branch has commits beyond HEAD (`git log --oneline HEAD...branch`) before destroying it - Preserves branches containing LLM-produced changes from previous plan executions - Only cleans up branches with no committed work (truly stale branches) ## Changes - **src/cleveragents/infrastructure/sandbox/git_worktree.py** - Core fix: commits-beyond-HEAD check added to cleanup_stale, docstring updated - **CHANGELOG.md** - Added entry under [Unreleased]/### Fixed - **CONTRIBUTORS.md** - Added HAL 9000 contribution note - **features/sandbox_reexecute_with_changes.feature** - New BDD test (3 scenarios) - **features/steps/sandbox_reexecute_with_changes_steps.py** - New step definitions ## PR Compliance Checklist - [x] CHANGELOG.md entry under [Unreleased] section - [x] CONTRIBUTORS.md updated with contribution note - [x] Commit footer includes "ISSUES CLOSED: #11121" - [x] BDD/Behave tests added for the changed behaviour - [ ] CI passes — CI run in progress, awaiting results on commit af2e1767 - [x] Epic reference — see Cross References below ## Cross References - Parent Issue: #11121 — `cleanup_stale destroys git worktree branch on re-invoked execute, causing plan apply to find zero artifacts` - TDD Issue Capture: #11120 (companion test for the bug) - Related PR: #11127 (WIP: guard against cleanup_stale destroying execute output) Closes #11121
Fix cleanup_stale to preserve branches with commits beyond HEAD (#11121)
Some checks failed
CI / push-validation (pull_request) Successful in 48s
Details
CI / helm (pull_request) Successful in 53s
Details
CI / build (pull_request) Successful in 1m8s
Details
CI / lint (pull_request) Failing after 1m31s
Details
CI / quality (pull_request) Successful in 1m29s
Details
CI / typecheck (pull_request) Successful in 1m39s
Details
CI / security (pull_request) Successful in 2m7s
Details
CI / integration_tests (pull_request) Successful in 4m33s
Details
CI / unit_tests (pull_request) Failing after 4m59s
Details
CI / coverage (pull_request) Has been skipped
Details
CI / docker (pull_request) Has been skipped
Details
CI / status-check (pull_request) Failing after 4s
Details
eb29e3bea3 
The cleanup_stale method in GitWorktreeSandbox now checks whether a branch
has any commits beyond HEAD before destroying it. Previously, ALL branches
matching cleveragents/plan-{plan_id} were removed regardless of whether they
contained committed LLM output from a previous plan execute. This caused
plan apply to find zero artifacts on re-invoked execute.

The fix uses "git log --oneline HEAD...branch" to detect extra commits.
If present, the branch is preserved (not stale). Only branches with no
committed work are cleaned up as before.

ISSUES CLOSED: #11121
HAL9001 requested changes 2026-05-12 00:23:23 +00:00
Dismissed
HAL9001 left a comment
Copy link

Review: PR #11132 — Fix cleanup_stale to preserve branches with commits beyond HEAD

Thank you for the fix. The core logic in cleanup_stale is conceptually sound — adding a commits-beyond-HEAD guard before destroying a branch is the right direction. However, this PR has several blocking issues that must be resolved before it can be approved.

CI Status — BLOCKING

Two required CI gates are failing:

  • ❌ CI / lint — failing after 1m31s
  • ❌ CI / unit_tests — failing after 4m59s
  • ❌ CI / status-check — failing (aggregate of above)

Per project policy, all required CI gates (lint, typecheck, security, unit_tests, coverage) must pass before a PR can be approved. Please fix the lint and unit test failures and push a new commit. The coverage check was skipped (blocked by unit_tests failing) — ensure it passes once unit_tests is green.

BLOCKER 1 — Wrong Branch Name

Issue #11121 ## Metadata specifies:

Branch: bugfix/m3-cleanup-stale-destroys-execute-output

This PR was pushed from fix/cleanup-stale-preserve-commits. This violates two rules:

  1. Branch naming convention: Bug fix branches must use the bugfix/mN- prefix (not fix/). See CONTRIBUTING.md: "Bug fix (the implementation of the fix) → bugfix/mN-<descriptive-name>".
  2. Metadata traceability: The branch name must match the Branch field in the issue Metadata exactly — verbatim.

Action required: The PR must be re-submitted from bugfix/m3-cleanup-stale-destroys-execute-output.

BLOCKER 2 — Commit Message First Line Does Not Match Issue Metadata

Issue #11121 ## Metadata prescribes:

Commit Message: fix(plan): guard cleanup_stale against execute/complete plans awaiting apply

The actual commit message first line is:

Fix cleanup_stale to preserve branches with commits beyond HEAD (#11121)

This is not Conventional Changelog format and does not match the prescribed message. Per CONTRIBUTING.md: "If the issue has a Metadata section with a Commit Message field → use that text EXACTLY as the first line — verbatim, copy-paste. Do NOT paraphrase, reword, or reformat it."

Action required: Amend/rebase to change the commit first line to exactly: fix(plan): guard cleanup_stale against execute/complete plans awaiting apply

BLOCKER 3 — TDD Regression Test from Issue #11120 Not Addressed

Issue #11121 acceptance criteria explicitly states:

"The TDD test from issue #11120 passes with the @tdd_expected_fail tag removed"

And the Subtasks list:

"Remove the @tdd_expected_fail tag from the test introduced in #11120 (leave @tdd_issue and @tdd_issue_11120 tags in place as permanent regression guards)"

This PR introduces a new feature file (sandbox_reexecute_with_changes.feature) instead of removing @tdd_expected_fail from the existing TDD test introduced by issue #11120. The TDD workflow requires: (a) the TDD issue-capture test must be the regression guard, and (b) the fix must make that test pass by removing the @tdd_expected_fail tag. The TDD issue (#11120) is listed as a dependency of the bug issue (#11121).

Action required:

  1. Find the existing test file from issue #11120 (likely in features/) and remove @tdd_expected_fail from its scenario(s), leaving @tdd_issue and @tdd_issue_11120 tags in place.
  2. Verify that scenario now passes with this fix in place.
  3. The new sandbox_reexecute_with_changes.feature may supplement the above but cannot substitute for it.

BLOCKER 4 — Missing Milestone on PR

Issue #11121 is assigned to milestone v3.2.0. The PR has no milestone assigned.

Per CONTRIBUTING.md PR checklist item 12: "Assigned to the same milestone as the linked issue(s)".

Action required: Assign milestone v3.2.0 to this PR.

BLOCKER 5 — Missing Type Label on PR

The PR only has State/In Review. It is missing a Type/ label. Since the linked issue is Type/Bug, the PR should carry Type/Bug.

Per CONTRIBUTING.md: "Exactly one Type/ label: Type/Bug, Type/Feature, or Type/Task".

Action required: Add Type/Bug label to this PR.

BLOCKER 6 — Potential Bug in Range Operator in cleanup_stale

In git_worktree.py, the commits-beyond-HEAD check uses three-dot syntax:

["log", "--oneline", f"HEAD...{branch_name}"]

The three-dot (...) symmetric difference includes commits reachable from either ref but not both. In a repository where master has diverged from the branch (e.g. after rebases or merges), this could yield commits reachable from HEAD but not from the branch — producing false positives (reporting extra commits when there are none ahead of HEAD, just commits that HEAD is ahead of the branch). The correct operator to detect "commits in branch that are not reachable from HEAD" is the two-dot range:

["log", "--oneline", f"HEAD..{branch_name}"]

This counts only commits reachable from branch_name but not from HEAD — exactly what is needed.

Action required: Change HEAD...{branch_name} to HEAD..{branch_name} in cleanup_stale.

Non-blocking Observations

  1. New BDD test tags: sandbox_reexecute_with_changes.feature scenarios guard against regression of bug #11121. Consider adding @tdd_issue and @tdd_issue_11121 tags to these scenarios as permanent regression guards, consistent with the project TDD tagging convention.

  2. Unnecessary git checkout calls in step definitions: The step_commit_on_worktree_branch step calls _git(["checkout", context.swc_branch], cwd=context.swc_repo) and then _git(["checkout", "main"], cwd=context.swc_repo). These checkout operations on the main repo are unnecessary — the commit itself correctly targets context.swc_wt_dir. The checkout calls could cause unexpected side effects in parallel test runs. Consider removing them.

Summary

Category Status
CI (lint + unit_tests) FAILING — must fix
Branch name WRONG — must be bugfix/m3-cleanup-stale-destroys-execute-output
Commit message WRONG — must match issue Metadata verbatim
TDD #11120 test NOT ADDRESSED — @tdd_expected_fail not removed
Milestone MISSING — set to v3.2.0
Type label MISSING — add Type/Bug
Range operator bug HEAD...branch must be HEAD..branch
Core logic (cleanup_stale guard) Conceptually correct
CHANGELOG updated Yes
CONTRIBUTORS.md updated Yes
ISSUES CLOSED footer Yes
Type safety (no # type: ignore) Pass
Security Pass

Please fix all 6 blocking issues and push a new commit. Once CI is green and the above items are resolved, this can be re-reviewed.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

## Review: PR #11132 — Fix cleanup_stale to preserve branches with commits beyond HEAD Thank you for the fix. The core logic in `cleanup_stale` is conceptually sound — adding a commits-beyond-HEAD guard before destroying a branch is the right direction. However, this PR has **several blocking issues** that must be resolved before it can be approved. ### CI Status — BLOCKING Two required CI gates are **failing**: - ❌ `CI / lint` — failing after 1m31s - ❌ `CI / unit_tests` — failing after 4m59s - ❌ `CI / status-check` — failing (aggregate of above) Per project policy, all required CI gates (`lint`, `typecheck`, `security`, `unit_tests`, `coverage`) must pass before a PR can be approved. Please fix the lint and unit test failures and push a new commit. The `coverage` check was skipped (blocked by `unit_tests` failing) — ensure it passes once `unit_tests` is green. --- ### BLOCKER 1 — Wrong Branch Name Issue #11121 `## Metadata` specifies: ``` Branch: bugfix/m3-cleanup-stale-destroys-execute-output ``` This PR was pushed from `fix/cleanup-stale-preserve-commits`. This violates two rules: 1. **Branch naming convention**: Bug fix branches must use the `bugfix/mN-` prefix (not `fix/`). See CONTRIBUTING.md: *"Bug fix (the implementation of the fix) → `bugfix/mN-<descriptive-name>`"*. 2. **Metadata traceability**: The branch name must match the `Branch` field in the issue Metadata exactly — verbatim. **Action required**: The PR must be re-submitted from `bugfix/m3-cleanup-stale-destroys-execute-output`. --- ### BLOCKER 2 — Commit Message First Line Does Not Match Issue Metadata Issue #11121 `## Metadata` prescribes: ``` Commit Message: fix(plan): guard cleanup_stale against execute/complete plans awaiting apply ``` The actual commit message first line is: ``` Fix cleanup_stale to preserve branches with commits beyond HEAD (#11121) ``` This is not Conventional Changelog format and does not match the prescribed message. Per CONTRIBUTING.md: *"If the issue has a Metadata section with a Commit Message field → use that text EXACTLY as the first line — verbatim, copy-paste. Do NOT paraphrase, reword, or reformat it."* **Action required**: Amend/rebase to change the commit first line to exactly: `fix(plan): guard cleanup_stale against execute/complete plans awaiting apply` --- ### BLOCKER 3 — TDD Regression Test from Issue #11120 Not Addressed Issue #11121 acceptance criteria explicitly states: > *"The TDD test from issue #11120 passes with the `@tdd_expected_fail` tag removed"* And the Subtasks list: > *"Remove the `@tdd_expected_fail` tag from the test introduced in #11120 (leave `@tdd_issue` and `@tdd_issue_11120` tags in place as permanent regression guards)"* This PR introduces a **new** feature file (`sandbox_reexecute_with_changes.feature`) instead of removing `@tdd_expected_fail` from the existing TDD test introduced by issue #11120. The TDD workflow requires: (a) the TDD issue-capture test must be the regression guard, and (b) the fix must make that test pass by removing the `@tdd_expected_fail` tag. The TDD issue (#11120) is listed as a dependency of the bug issue (#11121). **Action required**: 1. Find the existing test file from issue #11120 (likely in `features/`) and remove `@tdd_expected_fail` from its scenario(s), leaving `@tdd_issue` and `@tdd_issue_11120` tags in place. 2. Verify that scenario now passes with this fix in place. 3. The new `sandbox_reexecute_with_changes.feature` may supplement the above but cannot substitute for it. --- ### BLOCKER 4 — Missing Milestone on PR Issue #11121 is assigned to milestone `v3.2.0`. The PR has no milestone assigned. Per CONTRIBUTING.md PR checklist item 12: *"Assigned to the same milestone as the linked issue(s)"*. **Action required**: Assign milestone `v3.2.0` to this PR. --- ### BLOCKER 5 — Missing Type Label on PR The PR only has `State/In Review`. It is missing a `Type/` label. Since the linked issue is `Type/Bug`, the PR should carry `Type/Bug`. Per CONTRIBUTING.md: *"Exactly one Type/ label: Type/Bug, Type/Feature, or Type/Task"*. **Action required**: Add `Type/Bug` label to this PR. --- ### BLOCKER 6 — Potential Bug in Range Operator in `cleanup_stale` In `git_worktree.py`, the commits-beyond-HEAD check uses three-dot syntax: ```python ["log", "--oneline", f"HEAD...{branch_name}"] ``` The three-dot (`...`) symmetric difference includes commits reachable from **either** ref but not both. In a repository where `master` has diverged from the branch (e.g. after rebases or merges), this could yield commits reachable from `HEAD` but not from the branch — producing false positives (reporting extra commits when there are none ahead of HEAD, just commits that HEAD is ahead of the branch). The correct operator to detect *"commits in branch that are not reachable from HEAD"* is the two-dot range: ```python ["log", "--oneline", f"HEAD..{branch_name}"] ``` This counts only commits reachable from `branch_name` but not from `HEAD` — exactly what is needed. **Action required**: Change `HEAD...{branch_name}` to `HEAD..{branch_name}` in `cleanup_stale`. --- ### Non-blocking Observations 1. **New BDD test tags**: `sandbox_reexecute_with_changes.feature` scenarios guard against regression of bug #11121. Consider adding `@tdd_issue` and `@tdd_issue_11121` tags to these scenarios as permanent regression guards, consistent with the project TDD tagging convention. 2. **Unnecessary `git checkout` calls in step definitions**: The `step_commit_on_worktree_branch` step calls `_git(["checkout", context.swc_branch], cwd=context.swc_repo)` and then `_git(["checkout", "main"], cwd=context.swc_repo)`. These `checkout` operations on the main repo are unnecessary — the commit itself correctly targets `context.swc_wt_dir`. The checkout calls could cause unexpected side effects in parallel test runs. Consider removing them. --- ### Summary | Category | Status | |---|---| | CI (lint + unit_tests) | FAILING — must fix | | Branch name | WRONG — must be `bugfix/m3-cleanup-stale-destroys-execute-output` | | Commit message | WRONG — must match issue Metadata verbatim | | TDD #11120 test | NOT ADDRESSED — `@tdd_expected_fail` not removed | | Milestone | MISSING — set to v3.2.0 | | Type label | MISSING — add Type/Bug | | Range operator bug | `HEAD...branch` must be `HEAD..branch` | | Core logic (cleanup_stale guard) | Conceptually correct | | CHANGELOG updated | Yes | | CONTRIBUTORS.md updated | Yes | | ISSUES CLOSED footer | Yes | | Type safety (no # type: ignore) | Pass | | Security | Pass | Please fix all 6 blocking issues and push a new commit. Once CI is green and the above items are resolved, this can be re-reviewed. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
features/sandbox_reexecute_with_changes.feature
@ -0,0 +1,22 @@
@sandbox-reexecute-with-changes
HAL9001 commented 2026-05-12 00:23:23 +00:00
Owner
Copy link

Non-blocking suggestion — Add @tdd_issue regression tags

These scenarios guard against regression of bug #11121. Per the project TDD convention, scenarios that serve as permanent regression guards for a specific bug should be tagged with @tdd_issue and @tdd_issue_<N> so they are recognized as permanent regression guards:

@sandbox-reexecute-with-changes
@tdd_issue @tdd_issue_11121
Feature: Preserve worktree branches with commits beyond HEAD during cleanup (#11121)

This is a suggestion, not a blocker.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Non-blocking suggestion — Add `@tdd_issue` regression tags** These scenarios guard against regression of bug #11121. Per the project TDD convention, scenarios that serve as permanent regression guards for a specific bug should be tagged with `@tdd_issue` and `@tdd_issue_<N>` so they are recognized as permanent regression guards: ```gherkin @sandbox-reexecute-with-changes @tdd_issue @tdd_issue_11121 Feature: Preserve worktree branches with commits beyond HEAD during cleanup (#11121) ``` This is a suggestion, not a blocker. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
src/cleveragents/infrastructure/sandbox/git_worktree.py
HAL9001 commented 2026-05-12 00:23:23 +00:00
Owner
Copy link

BLOCKER — Wrong range operator: use HEAD.. not HEAD...

The three-dot symmetric difference (HEAD...branch) includes commits reachable from either ref but not both. In a repository where master has commits that the sandbox branch does not have (diverged history after rebase or merge), this will include those extra master commits in extra_commits, producing a false positive — the branch will be incorrectly preserved as "not stale" even when it contains no work ahead of HEAD.

The semantically correct check is the two-dot range HEAD..branch, which lists only commits reachable from branch but NOT from HEAD — exactly the set of commits that represent work produced during plan execute:

# CORRECT — lists only commits ahead of HEAD in the sandbox branch
["log", "--oneline", f"HEAD..{branch_name}"]

Please change f"HEAD...{branch_name}" to f"HEAD..{branch_name}" here.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKER — Wrong range operator: use `HEAD..` not `HEAD...`** The three-dot symmetric difference (`HEAD...branch`) includes commits reachable from *either* ref but not both. In a repository where `master` has commits that the sandbox branch does not have (diverged history after rebase or merge), this will include those extra `master` commits in `extra_commits`, producing a false positive — the branch will be incorrectly preserved as "not stale" even when it contains no work ahead of HEAD. The semantically correct check is the two-dot range `HEAD..branch`, which lists only commits reachable from `branch` but NOT from `HEAD` — exactly the set of commits that represent work produced during plan execute: ```python # CORRECT — lists only commits ahead of HEAD in the sandbox branch ["log", "--oneline", f"HEAD..{branch_name}"] ``` Please change `f"HEAD...{branch_name}"` to `f"HEAD..{branch_name}"` here. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
HAL9001 commented 2026-05-12 00:23:26 +00:00
Owner
Copy link

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

--- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
HAL9000 added this to the v3.2.0 milestone 2026-05-12 01:45:40 +00:00
fix(ruff): remove unused import os and format git call in sandbox_reexecute_with_changes_steps.py
Some checks failed
CI / push-validation (pull_request) Successful in 40s
Details
CI / helm (pull_request) Successful in 50s
Details
CI / build (pull_request) Successful in 57s
Details
CI / lint (pull_request) Successful in 1m10s
Details
CI / quality (pull_request) Successful in 1m18s
Details
CI / typecheck (pull_request) Successful in 1m25s
Details
CI / security (pull_request) Successful in 1m46s
Details
CI / integration_tests (pull_request) Successful in 4m22s
Details
CI / unit_tests (pull_request) Failing after 6m22s
Details
CI / coverage (pull_request) Has been skipped
Details
CI / docker (pull_request) Has been skipped
Details
CI / status-check (pull_request) Failing after 3s
Details
af2e176732 
This fixes the ruff lint failure (F401: imported but unused ) that was
causing the CI lint job to fail on PR #11132.
chore(ci): re-trigger workflow for af2e1767 lint fix
Some checks failed
CI / helm (pull_request) Successful in 47s
Details
CI / push-validation (pull_request) Successful in 39s
Details
CI / build (pull_request) Successful in 1m3s
Details
CI / quality (pull_request) Successful in 1m13s
Details
CI / lint (pull_request) Successful in 1m22s
Details
CI / typecheck (pull_request) Successful in 1m40s
Details
CI / security (pull_request) Successful in 2m4s
Details
CI / unit_tests (pull_request) Failing after 4m32s
Details
CI / integration_tests (pull_request) Successful in 4m30s
Details
CI / coverage (pull_request) Has been skipped
Details
CI / docker (pull_request) Has been skipped
Details
CI / status-check (pull_request) Failing after 3s
Details
285ea1f4ed
hurui200320 commented 2026-05-12 03:08:19 +00:00
Member
Copy link

close as it dup with #11127

close as it dup with #11127
hurui200320 closed this pull request 2026-05-12 03:08:19 +00:00
HAL9001 requested changes 2026-05-12 09:36:26 +00:00
HAL9001 left a comment
Copy link

Re-Review — PR #11132

Anchored to commit c1c07628. Prior review was at eb29e3be.

Prior Feedback — Resolution Status

✅ RESOLVED — Missing milestone (BLOCKER 4)

Milestone v3.2.0 is now assigned.

✅ RESOLVED — Missing Type/Bug label (BLOCKER 5)

Type/Bug label is now applied. Labels are correct: MoSCoW/Must have, Priority/Critical, State/In Review, Type/Bug.

❌ STILL OPEN — Wrong branch name (BLOCKER 1)

Branch remains fix/cleanup-stale-preserve-commits. This violates the bugfix/m3- naming convention and the Metadata field in issue #11121 which specifies bugfix/m3-cleanup-stale-destroys-execute-output. Cannot be changed post-creation — a new PR from the correctly-named branch is required.

❌ STILL OPEN — Commit message mismatch (BLOCKER 2)

The fix commit message first line is:

fix: cleanup_stale preserves branches with extra commits beyond HEAD (#11121)

Issue #11121 Metadata prescribes:

fix(plan): guard cleanup_stale against execute/complete plans awaiting apply

Per CONTRIBUTING.md: the first line must match the Metadata field verbatim. Additionally, appending (#11121) to the commit message first line violates the Conventional Changelog format.

❌ STILL OPEN — Range operator HEAD... (BLOCKER 6)

git_worktree.py still uses HEAD...{branch_name} (three-dot symmetric difference) at lines 200 and 301. The correct operator is HEAD..{branch_name} (two-dot) to detect commits reachable from the branch but not from HEAD. Please fix both occurrences.

❌ PENDING — TDD #11120 @tdd_expected_fail not removed (BLOCKER 3)

PR #11123 (the TDD issue-capture test for #11121) has not yet been merged to master. Until it is merged, @tdd_expected_fail cannot be removed here. This blocker depends on #11123 merging first. Once #11123 is merged to master, rebase this branch and remove @tdd_expected_fail from the TDD scenarios in features/tdd_cleanup_stale_destroys_execute_output.feature.

New Blocking Issues

🔴 NEW BLOCKER — CI failing: lint, unit_tests, integration_tests

Required CI gates are still failing on head c1c07628:

  • ❌ lint — failing
  • ❌ unit_tests — failing
  • ❌ integration_tests — failing
  • ❌ status-check — failing

All required gates must pass before approval. Run nox -s lint && nox -s unit_tests && nox -s integration_tests locally and fix all failures.

🔴 NEW BLOCKER — Multiple unrelated commits added (atomicity violation)

Since the prior review, the following commits were added to this PR that are entirely unrelated to the cleanup_stale fix:

  1. Fix validation bypass for code longer than 10 characters... — an unrelated validation bypass fix
  2. fix(cli): add spec-required --data-dir, --config-path, and -v global options to main_callback — an unrelated CLI fix
  3. feat(ci): implement TDD bug tag quality gate for bug fix PRs — a new CI quality gate feature

Per CONTRIBUTING.md: one issue = one commit (atomic). These unrelated changes must be removed from this PR branch. Each belongs in its own separate PR for its own issue.

The PR should contain ONLY:

  1. The cleanup_stale guard implementation in git_worktree.py
  2. BDD tests for the cleanup_stale guard
  3. CHANGELOG and CONTRIBUTORS entries

Summary

Issue Status
CI failing (lint, unit_tests, integration_tests) ❌ NEW BLOCKER
Multiple unrelated commits in PR ❌ NEW BLOCKER
Branch name (fix/ instead of bugfix/m3-) ❌ OPEN
Commit message first line mismatch ❌ OPEN
Range operator HEAD... should be HEAD.. ❌ OPEN
TDD #11120 @tdd_expected_fail removal ❌ PENDING (merge #11123 first)
Milestone v3.2.0 ✅ RESOLVED
Type/Bug label ✅ RESOLVED

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

## Re-Review — PR #11132 Anchored to commit `c1c07628`. Prior review was at `eb29e3be`. --- ## Prior Feedback — Resolution Status ### ✅ RESOLVED — Missing milestone (BLOCKER 4) Milestone `v3.2.0` is now assigned. ### ✅ RESOLVED — Missing Type/Bug label (BLOCKER 5) `Type/Bug` label is now applied. Labels are correct: `MoSCoW/Must have`, `Priority/Critical`, `State/In Review`, `Type/Bug`. ### ❌ STILL OPEN — Wrong branch name (BLOCKER 1) Branch remains `fix/cleanup-stale-preserve-commits`. This violates the `bugfix/m3-` naming convention and the Metadata field in issue #11121 which specifies `bugfix/m3-cleanup-stale-destroys-execute-output`. Cannot be changed post-creation — a new PR from the correctly-named branch is required. ### ❌ STILL OPEN — Commit message mismatch (BLOCKER 2) The fix commit message first line is: ``` fix: cleanup_stale preserves branches with extra commits beyond HEAD (#11121) ``` Issue #11121 Metadata prescribes: ``` fix(plan): guard cleanup_stale against execute/complete plans awaiting apply ``` Per CONTRIBUTING.md: the first line must match the Metadata field verbatim. Additionally, appending `(#11121)` to the commit message first line violates the Conventional Changelog format. ### ❌ STILL OPEN — Range operator `HEAD...` (BLOCKER 6) `git_worktree.py` still uses `HEAD...{branch_name}` (three-dot symmetric difference) at lines 200 and 301. The correct operator is `HEAD..{branch_name}` (two-dot) to detect commits reachable from the branch but not from HEAD. Please fix both occurrences. ### ❌ PENDING — TDD #11120 `@tdd_expected_fail` not removed (BLOCKER 3) PR #11123 (the TDD issue-capture test for #11121) has not yet been merged to master. Until it is merged, `@tdd_expected_fail` cannot be removed here. This blocker depends on #11123 merging first. Once #11123 is merged to master, rebase this branch and remove `@tdd_expected_fail` from the TDD scenarios in `features/tdd_cleanup_stale_destroys_execute_output.feature`. --- ## New Blocking Issues ### 🔴 NEW BLOCKER — CI failing: lint, unit_tests, integration_tests Required CI gates are still failing on head `c1c07628`: - ❌ `lint` — failing - ❌ `unit_tests` — failing - ❌ `integration_tests` — failing - ❌ `status-check` — failing All required gates must pass before approval. Run `nox -s lint && nox -s unit_tests && nox -s integration_tests` locally and fix all failures. ### 🔴 NEW BLOCKER — Multiple unrelated commits added (atomicity violation) Since the prior review, the following commits were added to this PR that are **entirely unrelated** to the `cleanup_stale` fix: 1. `Fix validation bypass for code longer than 10 characters...` — an unrelated validation bypass fix 2. `fix(cli): add spec-required --data-dir, --config-path, and -v global options to main_callback` — an unrelated CLI fix 3. `feat(ci): implement TDD bug tag quality gate for bug fix PRs` — a new CI quality gate feature Per CONTRIBUTING.md: one issue = one commit (atomic). These unrelated changes must be removed from this PR branch. Each belongs in its own separate PR for its own issue. The PR should contain ONLY: 1. The `cleanup_stale` guard implementation in `git_worktree.py` 2. BDD tests for the `cleanup_stale` guard 3. CHANGELOG and CONTRIBUTORS entries --- ## Summary | Issue | Status | |---|---| | CI failing (lint, unit_tests, integration_tests) | ❌ NEW BLOCKER | | Multiple unrelated commits in PR | ❌ NEW BLOCKER | | Branch name (`fix/` instead of `bugfix/m3-`) | ❌ OPEN | | Commit message first line mismatch | ❌ OPEN | | Range operator `HEAD...` should be `HEAD..` | ❌ OPEN | | TDD #11120 `@tdd_expected_fail` removal | ❌ PENDING (merge #11123 first) | | Milestone v3.2.0 | ✅ RESOLVED | | Type/Bug label | ✅ RESOLVED | --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
HAL9001 commented 2026-05-12 09:36:31 +00:00
Owner
Copy link

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

--- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Some checks failed
CI / helm (pull_request) Successful in 47s
Details
CI / push-validation (pull_request) Successful in 39s
Details
CI / build (pull_request) Successful in 1m3s
Required
Details
CI / quality (pull_request) Successful in 1m13s
Required
Details
CI / lint (pull_request) Successful in 1m22s
Required
Details
CI / typecheck (pull_request) Successful in 1m40s
Required
Details
CI / security (pull_request) Successful in 2m4s
Required
Details
CI / unit_tests (pull_request) Failing after 4m32s
Required
Details
CI / integration_tests (pull_request) Successful in 4m30s
Required
Details
CI / coverage (pull_request) Has been skipped
Required
Details
CI / docker (pull_request) Has been skipped
Required
Details
CI / status-check (pull_request) Failing after 3s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
HAL9001
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.2.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!11132
No description provided.