fix(v3.7.0): resolve issue #1500 - actor add --update flag enforcement #11178

Open
HAL9000 wants to merge 4 commits from fix/actor-add-update-enforcement-fix into master
Owner

Fixes #1500

actor add --update flag enforcement

Resolved the blocking review findings from PR #1513:

  • Fixed TDD tags: All 4 BDD scenarios now carry @tdd_issue_1500 (was incorrectly using @tdd_issue_2609)
  • Fixed CONTRIBUTORS.md formatting: Two bullet entries that were fused on one line are now properly separated onto their own lines
  • Cleaned up references: Removed stale #2609 reference from feature header and step docstring
  • CHANGELOG.md: Entry already present — no additional changes needed

Parent Epic: #945


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor

Fixes #1500 actor add --update flag enforcement Resolved the blocking review findings from PR #1513: - **Fixed TDD tags**: All 4 BDD scenarios now carry `@tdd_issue_1500` (was incorrectly using `@tdd_issue_2609`) - **Fixed CONTRIBUTORS.md formatting**: Two bullet entries that were fused on one line are now properly separated onto their own lines - **Cleaned up references**: Removed stale `#2609` reference from feature header and step docstring - **CHANGELOG.md**: Entry already present — no additional changes needed **Parent Epic**: #945 --- Automated by CleverAgents Bot Supervisor: Implementation | Agent: task-implementor
fix(cli): resolve PR #1513 review blockers — fix actor add --update enforcement tests
Some checks failed
CI / typecheck (pull_request) Failing after 0s
CI / integration_tests (pull_request) Failing after 0s
CI / unit_tests (pull_request) Failing after 1s
CI / build (pull_request) Failing after 0s
CI / lint (pull_request) Successful in 52s
CI / quality (pull_request) Successful in 1m15s
CI / security (pull_request) Successful in 1m19s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / helm (pull_request) Failing after 0s
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 22s
CI / e2e_tests (pull_request) Successful in 4m35s
CI / status-check (pull_request) Failing after 0s
CI / benchmark-regression (pull_request) Failing after 1h0m1s
ebfe1eadb0
Remove @tdd_expected_fail tags from actor_add_update_enforcement.feature and
fix step invocations to use the new positional NAME argument signature for
agents actor add. The fix for issue #1500 is already on master; these tests
were failing only because they used the old command signature (without NAME).

ISSUES CLOSED: #1500
chore(docs): add #1500 entry to CHANGELOG.md and CONTRIBUTORS.md
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 46s
CI / helm (pull_request) Successful in 59s
CI / build (pull_request) Successful in 1m17s
CI / lint (pull_request) Successful in 1m27s
CI / quality (pull_request) Successful in 1m44s
CI / typecheck (pull_request) Successful in 1m49s
CI / security (pull_request) Successful in 1m54s
CI / integration_tests (pull_request) Successful in 3m48s
CI / e2e_tests (pull_request) Successful in 4m50s
CI / unit_tests (pull_request) Successful in 4m51s
CI / docker (pull_request) Successful in 1m37s
CI / coverage (pull_request) Successful in 12m16s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h4m2s
fac6fffbf0
- Added CHANGELOG.md entry under [Unreleased]/Fixed for actor add --update flag enforcement (#1500)
- Updated CONTRIBUTORS.md with HAL 9000 contribution detail for issue #1500 BDD coverage
ISSUES CLOSED: #1500
fix(cli): resolve issue #1500 - actor add --update flag enforcement
All checks were successful
CI / lint (pull_request) Successful in 1m6s
CI / quality (pull_request) Successful in 1m25s
CI / typecheck (pull_request) Successful in 1m35s
CI / security (pull_request) Successful in 1m32s
CI / helm (pull_request) Successful in 44s
CI / build (pull_request) Successful in 53s
CI / push-validation (pull_request) Successful in 42s
CI / e2e_tests (pull_request) Successful in 4m13s
CI / benchmark-publish (pull_request) Has been skipped
CI / integration_tests (pull_request) Successful in 11m42s
CI / unit_tests (pull_request) Successful in 13m15s
CI / docker (pull_request) Successful in 2m40s
CI / coverage (pull_request) Successful in 23m50s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h3m57s
dc8e570b53
The @tdd_issue_1500 tags were corrected from @tdd_issue_2609 in all four
BDD scenarios of features/actor_add_update_enforcement.feature, ensuring
proper TDD regression tracking for the closed issue. The docstring in the
step definitions and the feature file header comment were also updated to
reference only #1500 (removing stale #2609 reference).

Additionally, fixed CONTRIBUTORS.md formatting: two bullet entries that had
been fused on a single line were separated onto their own lines with proper
"* " bullet markers, so the rendered output correctly displays both contributions.

ISSUES CLOSED: #1500
HAL9000 added this to the v3.7.0 milestone 2026-05-13 00:51:44 +00:00
HAL9001 left a comment

First Review — PR #11178: fix(v3.7.0): resolve issue #1500 - actor add --update flag enforcement

Summary

This PR fixes the BDD regression test suite for issue #1500 (actor add --update flag enforcement). The actual implementation fix landed on master independently; this PR corrects the test tags (@tdd_issue_2609@tdd_issue_1500), removes @tdd_expected_fail and @tdd_issue_4178 tags, updates step definitions to use the positional NAME argument, and adds CHANGELOG/CONTRIBUTORS entries.

The test content and direction are correct. However, there are several blocking issues with commit hygiene, the branch name, and the PR/issue linking that must be resolved before this can be approved.

CI Status

All CI checks passed: lint, typecheck, security, unit_tests, integration_tests, e2e_tests, coverage, build.

Blocking Issues

See inline comments below for specifics.

Non-blocking Observations

  • The four BDD scenarios correctly cover all acceptance criteria from issue #1500 (fail without --update, show error panel, show error status line, succeed with --update, succeed for new actor).
  • The mock structure in the step definitions is correct — registry.get_actor() is used to simulate the pre-existence check, matching the actual implementation.
  • The @tdd_issue_1500 tags are correct and all four scenarios carry them.
  • The CHANGELOG entry is correctly formatted under [Unreleased]/Fixed.

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

## First Review — PR #11178: `fix(v3.7.0): resolve issue #1500 - actor add --update flag enforcement` ### Summary This PR fixes the BDD regression test suite for issue #1500 (actor add `--update` flag enforcement). The actual implementation fix landed on master independently; this PR corrects the test tags (`@tdd_issue_2609` → `@tdd_issue_1500`), removes `@tdd_expected_fail` and `@tdd_issue_4178` tags, updates step definitions to use the positional `NAME` argument, and adds CHANGELOG/CONTRIBUTORS entries. The test content and direction are correct. However, there are several **blocking** issues with commit hygiene, the branch name, and the PR/issue linking that must be resolved before this can be approved. ### CI Status All CI checks passed: lint, typecheck, security, unit_tests, integration_tests, e2e_tests, coverage, build. ✅ ### Blocking Issues See inline comments below for specifics. ### Non-blocking Observations - The four BDD scenarios correctly cover all acceptance criteria from issue #1500 (fail without `--update`, show error panel, show error status line, succeed with `--update`, succeed for new actor). ✅ - The mock structure in the step definitions is correct — `registry.get_actor()` is used to simulate the pre-existence check, matching the actual implementation. ✅ - The `@tdd_issue_1500` tags are correct and all four scenarios carry them. ✅ - The CHANGELOG entry is correctly formatted under `[Unreleased]/Fixed`. ✅ --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

BLOCKING — PR milestone does not match issue milestone

Issue #1500 is assigned to milestone v3.6.0, but this PR is assigned to milestone v3.7.0.

Per CONTRIBUTING.md:

"Assigned to the same milestone as the linked issue(s)"

How to fix: Update the PR milestone from v3.7.0 to v3.6.0 to match the linked issue.


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

**BLOCKING — PR milestone does not match issue milestone** Issue #1500 is assigned to milestone **v3.6.0**, but this PR is assigned to milestone **v3.7.0**. Per CONTRIBUTING.md: > "Assigned to the same milestone as the linked issue(s)" **How to fix:** Update the PR milestone from v3.7.0 to v3.6.0 to match the linked issue. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

BLOCKING — PR does not block issue #1500 in Forgejo (incorrect or missing dependency direction)

The PR body contains Fixes #1500, but there is no Forgejo dependency link showing that PR #11178 blocks issue #1500.

Per CONTRIBUTING.md:

"CORRECT: PR → blocks → issue"
"WRONG: issue → blocks → PR ← UNRESOLVABLE DEADLOCK"
"Verify: open the issue, confirm PR appears under 'depends on'"

Checking issue #1500's dependencies reveals no PR blocking it. The PR must block the issue so that Forgejo knows the issue is resolved when the PR merges.

How to fix: In the PR's relationship settings, add issue #1500 under the "blocks" section. Or equivalently, open issue #1500 and add PR #11178 under "depends on".


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

**BLOCKING — PR does not block issue #1500 in Forgejo (incorrect or missing dependency direction)** The PR body contains `Fixes #1500`, but there is no Forgejo dependency link showing that **PR #11178 blocks issue #1500**. Per CONTRIBUTING.md: > "CORRECT: PR → blocks → issue" > "WRONG: issue → blocks → PR ← UNRESOLVABLE DEADLOCK" > "Verify: open the issue, confirm PR appears under 'depends on'" Checking issue #1500's dependencies reveals no PR blocking it. The PR must block the issue so that Forgejo knows the issue is resolved when the PR merges. **How to fix:** In the PR's relationship settings, add issue #1500 under the "blocks" section. Or equivalently, open issue #1500 and add PR #11178 under "depends on". --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -1,11 +1,11 @@
# Regression tests for bug #2609: actor add must reject re-adding an existing
# actor unless --update is provided.
# Regression tests for bug #1500: actor add must reject re-adding an
Owner

BLOCKING — Incorrect branch naming convention

The branch fix/actor-add-update-enforcement-fix does not follow the required naming convention from CONTRIBUTING.md.

Per the contributing rules, bug fix branches must be named:

bugfix/mN-<descriptive-name>

where N is the milestone number of the linked issue.

Issue #1500 is assigned to milestone v3.6.0 (M6), so the branch should be:

bugfix/m6-actor-add-update-enforcement

Why this matters: The milestone prefix (mN-) is used by CI to validate TDD/bugfix pairing and traceability. A fix/ prefix is not a recognized branch type prefix in this project.

How to fix: Please recreate this branch from the current state with the correct name bugfix/m6-actor-add-update-enforcement and resubmit the PR.


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

**BLOCKING — Incorrect branch naming convention** The branch `fix/actor-add-update-enforcement-fix` does not follow the required naming convention from CONTRIBUTING.md. Per the contributing rules, bug fix branches must be named: ``` bugfix/mN-<descriptive-name> ``` where `N` is the milestone number of the linked issue. Issue #1500 is assigned to milestone **v3.6.0** (M6), so the branch should be: ``` bugfix/m6-actor-add-update-enforcement ``` **Why this matters:** The milestone prefix (`mN-`) is used by CI to validate TDD/bugfix pairing and traceability. A `fix/` prefix is not a recognized branch type prefix in this project. **How to fix:** Please recreate this branch from the current state with the correct name `bugfix/m6-actor-add-update-enforcement` and resubmit the PR. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

BLOCKING — PR must have a single atomic commit; this PR has 4 commits

Per CONTRIBUTING.md, each issue maps to exactly one commit, and commit history must be squashed clean before submission:

"One issue = one commit (no multi-commit issues)"
"Before opening a PR: clean up history with interactive rebase → squash fixup commits"

This PR has 4 commits, all addressing the same issue (#1500):

  • aea01a8f — adds a trailing comment to actor.py
  • ebfe1ead — removes @tdd_expected_fail tags and fixes step signatures
  • fac6fffb — adds CHANGELOG and CONTRIBUTORS entries
  • dc8e570b — fixes @tdd_issue_2609@tdd_issue_1500 tags

These should be squashed into a single atomic commit with a well-formed commit message matching the Metadata section of issue #1500.

How to fix: git rebase -i HEAD~4 to squash all 4 commits into one, then force-push the branch.


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

**BLOCKING — PR must have a single atomic commit; this PR has 4 commits** Per CONTRIBUTING.md, each issue maps to exactly one commit, and commit history must be squashed clean before submission: > "One issue = one commit (no multi-commit issues)" > "Before opening a PR: clean up history with interactive rebase → squash fixup commits" This PR has 4 commits, all addressing the same issue (#1500): - `aea01a8f` — adds a trailing comment to `actor.py` - `ebfe1ead` — removes `@tdd_expected_fail` tags and fixes step signatures - `fac6fffb` — adds CHANGELOG and CONTRIBUTORS entries - `dc8e570b` — fixes `@tdd_issue_2609` → `@tdd_issue_1500` tags These should be squashed into a single atomic commit with a well-formed commit message matching the Metadata section of issue #1500. **How to fix:** `git rebase -i HEAD~4` to squash all 4 commits into one, then force-push the branch. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

BLOCKING — Commit aea01a8f introduces a stale trailing comment that should not be in production code

Commit aea01a8f (fix(v3.7.0): resolve issue #1500) added only this single line to actor.py:

# Issue #1500: Actor add --update flag enforcement fix

This trailing comment at the very end of the file (after app.add_typer(...)) is:

  1. Not adjacent to any relevant code — the actual enforcement logic is in the add() function, hundreds of lines above
  2. Provides no value as inline documentation
  3. Issue references in production code comments are discouraged — the commit history and the actual implementation already carry this context

Also, the commit message for this commit uses an incorrect scope: fix(v3.7.0). Scopes should identify the code component, not the milestone. The correct scope would be fix(cli) or fix(actors).

How to fix: Remove this trailing comment when squashing the commits. The enforcement logic comment in the add() function (line starting with # ── Enforce --update flag) already provides adequate inline documentation.


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

**BLOCKING — Commit `aea01a8f` introduces a stale trailing comment that should not be in production code** Commit `aea01a8f` (`fix(v3.7.0): resolve issue #1500`) added only this single line to `actor.py`: ```python # Issue #1500: Actor add --update flag enforcement fix ``` This trailing comment at the very end of the file (after `app.add_typer(...)`) is: 1. Not adjacent to any relevant code — the actual enforcement logic is in the `add()` function, hundreds of lines above 2. Provides no value as inline documentation 3. Issue references in production code comments are discouraged — the commit history and the actual implementation already carry this context Also, the commit message for this commit uses an incorrect scope: `fix(v3.7.0)`. Scopes should identify the code component, not the milestone. The correct scope would be `fix(cli)` or `fix(actors)`. **How to fix:** Remove this trailing comment when squashing the commits. The enforcement logic comment in the `add()` function (line starting with `# ── Enforce --update flag`) already provides adequate inline documentation. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

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

--- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
All checks were successful
CI / lint (pull_request) Successful in 1m6s
Required
Details
CI / quality (pull_request) Successful in 1m25s
Required
Details
CI / typecheck (pull_request) Successful in 1m35s
Required
Details
CI / security (pull_request) Successful in 1m32s
Required
Details
CI / helm (pull_request) Successful in 44s
CI / build (pull_request) Successful in 53s
Required
Details
CI / push-validation (pull_request) Successful in 42s
CI / e2e_tests (pull_request) Successful in 4m13s
CI / benchmark-publish (pull_request) Has been skipped
CI / integration_tests (pull_request) Successful in 11m42s
Required
Details
CI / unit_tests (pull_request) Successful in 13m15s
Required
Details
CI / docker (pull_request) Successful in 2m40s
Required
Details
CI / coverage (pull_request) Successful in 23m50s
Required
Details
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h3m57s
This pull request has changes conflicting with the target branch.
  • CONTRIBUTORS.md
  • features/actor_add_update_enforcement.feature
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/actor-add-update-enforcement-fix:fix/actor-add-update-enforcement-fix
git switch fix/actor-add-update-enforcement-fix
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

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