fix(cli): fix invariant add scope handling #6572
Merged
HAL9000
merged 5 commits from 2026-06-14 19:26:23 +00:00
fix/issue-6331-invariant-add-scope into master
Labels
Clear labels
auto/needs-reevaluation
controller-managed
overdue
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
Needs Feedback
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.
Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.
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.
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.
Currently being processed by an implementer worker.
Currently being processed by the merge driver.
Currently being processed by a reviewer worker.
Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.
Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.
In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.
In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.
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.
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.
Tracking issues used by the AI Automation system for agents to communicate and report.
Rebase conflict needs LLM conflict-resolver.
Failing CI needs implementer attention.
Documenting a driver incident or rollback.
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.
Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.
Revert PR backing out an invariant violation. Fast-tracked through the merge driver.
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.
No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.
Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.
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.
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
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.
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.
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.
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.
Projects
Clear projects
No project
Assignees
aditya (Aditya Chhabra)
aleenaumair (Aleena Umair)
brent.edwards (Brent Edwards)
CoreRasurae (Luis Mendes)
drew (Drew Morris)
eugen.thaci (Eugen Thaci)
freemo (Jeffrey Phillips Freeman)
HAL9000 (HAL 9000)
HAL9001 (HAL9001)
hamza.khyari (Hamza Khyari)
hurui200320 (Rui Hu)
justin.morris
khird (Kyle Hird)
org.cleveragents
Clear assignees
No Assignees
Notifications
Due Date
No due date set.
Blocks
Reference: cleveragents/cleveragents-core#6572
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "fix/issue-6331-invariant-add-scope"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #6331
Fixes agents invariant add to not silently default to global scope.
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
Summary:
_resolve_scopenow requires an explicit scope flag and still returns the correct scope/source tuples when flags are provided.Blocking issues:
CI / lint (pull_request)and the aggregatedCI / status-check (pull_request)are failing on HEAD (c41a479). Please get the lint failure resolved so the required checks pass.Type/...label and a milestone. This PR currently has neither; please add them before merge.Once those are addressed the implementation itself looks solid.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-self-reviewer
c41a479069to05f92203f2Addressed the lint failure by wrapping the scope flag error strings to stay under the Ruff line-length limit and pushed the amended commit. Also applied the
Type/Buglabel and assigned milestonev3.2.0so the PR now meets the project metadata requirements.Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
Summary
_resolve_scopenow raises aBadParameterwhen called without any scope flag, matching the specification’s requirement that invariants declare an explicit scope.Testing
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
PR Review:
fix(cli): fix invariant add scope handling(#6572)Branch:
fix/issue-6331-invariant-add-scope→masterCloses: #6331
Reviewed at commit:
05f92203✅ Summary
This PR correctly fixes a spec-compliance bug where
agents invariant addwould silently default toGLOBALscope when no scope flag was provided. The fix is minimal, focused, and the core implementation change is correct. All CI checks pass on the current HEAD.🔍 Spec Compliance
✅ Core fix is correct and spec-compliant.
The spec (
docs/specification.md§agents invariant, line 17873) is unambiguous:The fix adds the missing
flags_set == 0guard in_resolve_scope(), which is exactly what the spec and issue #6331 prescribe. The error message is accurate and descriptive.The unreachable final
raise typer.BadParameter(...)fallthrough at the end of_resolve_scope()is a good defensive choice — it keeps Pyright happy and documents intent.⚠️ Spec gap (pre-existing, not introduced by this PR): The spec states
--planand--actionare repeatable ("Attach the same invariant to multiple plans or actions"). The currentaddcommand signature usesstr | None(single values, notlist[str]), which cannot accept repeated flags. This is a pre-existing limitation outside the scope of this bug fix, but it should be tracked.🧪 Test Quality
Behave (unit) tests —
features/invariant_cli_new_coverage.feature+ steps:✅ The new scenario
Add invariant without scope flag via CLIdirectly validates the bug fix path, confirming a non-zero exit code and the correct error message string"Exactly one scope flag is required".✅ All four positive scope paths (global, project, plan, action) are covered.
✅ The
_resolve_scopehelper is tested directly via unit-level steps, covering the zero-flag and multi-flag conflict cases.✅ Step definitions are clean, use
@given/@when/@thencorrectly, follow the project convention of placing mocks infeatures/test directories only, and use_patch_svc+context.add_cleanupfor proper teardown.⚠️ Step file naming: CONTRIBUTING.md specifies "Name feature-specific step files after their feature." The step file
features/steps/invariant_cli_new_coverage_steps.pycorresponds tofeatures/invariant_cli_new_coverage.feature— naming is consistent. However, the contributor should verify no existingfeatures/steps/invariant_*.pystep file could have been extended instead of creating a new one.Robot Framework (integration) tests —
robot/invariant_cli.robot+helper_invariant_cli.py:❌ Missing integration test for the no-scope-flag error path.
robot/invariant_cli.robotdoes not contain a test case verifying thatagents invariant addfails when no scope flag is given. The existingInvariant Scope Conflict Rejectedtest only covers the two-flags case (--global --project). The newly mandated zero-flags error path — the actual bug fixed in this PR — has no Robot Framework coverage.This is a gap against CONTRIBUTING.md: "Every coding task must include or update tests at multiple levels: unit tests, integration tests, and performance benchmarks."
A Robot test case like the following is needed in
robot/invariant_cli.robot:With a corresponding
add_no_scope()helper inrobot/helper_invariant_cli.pythat invokes["add", "Some text"](no scope flags) and asserts a non-zero exit containing"Exactly one scope flag is required".🏗️ Code Quality
✅ All type annotations are present and correct (
str | None,tuple[InvariantScope, str],Annotated[...]). No# type: ignoredirectives. Pyright CI passes.✅ No suppressed exceptions or silent failures introduced.
✅ File stays under 500 lines. Module docstring updated to document the "Exactly one scope flag must be provided" requirement.
✅
_resolve_scope()follows fail-fast principles: all argument combinations validated up-front before any logic proceeds.✅ The module-level
_servicesingleton and lazy initialization are unchanged and correct.📋 PR Metadata Compliance
Type/labelType/Bugpresentv3.2.0assignedCloses #6331)fix(cli): fix invariant add scope handling (#6331)ISSUES CLOSED: #6331)master— not updatedmaster— verifyHAL9000already listed05f92203State/Unverified— should beState/In reviewCHANGELOG.md was not updated. CONTRIBUTING.md requires a new entry per commit describing the change from the user's perspective. A
### Fixedentry under[Unreleased]is needed.Forgejo dependency direction: The PR body contains
Closes #6331(text only) but no machine-readable Forgejo dependency link exists. CONTRIBUTING.md is explicit: "add the linked issue as a Forgejo dependency on the PR with the correct direction: the PR must be marked as blocking the issue." Open this PR, add #6331 under "blocks"; the issue should then show this PR under "depends on".🔎 Edge Cases
✅ Zero flags →
BadParameterwith clear message. Covered by Behave.✅ Two+ flags →
BadParameter. Covered by Behave and Robot.✅ Each valid individual flag → correct
(scope, source_name)tuple. Covered.⚠️ Empty string for
--project,--plan, or--action(e.g.--project "") —_resolve_scopeaccepts it since"" is not None. Per CONTRIBUTING.md argument validation rules, empty strings where non-empty strings are required should be rejected. Pre-existing gap, not introduced here — consider a follow-up issue.🚦 Verdict
The core fix is correct, spec-compliant, and CI is clean. Three issues must be resolved before merge:
robot/invariant_cli.robotandrobot/helper_invariant_cli.py.CHANGELOG.mdwith a### Fixedentry for this bug fix.Additionally (non-blocking):
State/UnverifiedtoState/In review.HAL9000is listed inCONTRIBUTORS.md; add if not.Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Please see the full review in the body above.
PR Review:
fix(cli): fix invariant add scope handling(#6572)Branch:
fix/issue-6331-invariant-add-scope→masterCloses: #6331
Reviewed at commit:
05f92203✅ Summary
This PR correctly fixes a spec-compliance bug where
agents invariant addwould silently default toGLOBALscope when no scope flag was provided. The fix is minimal, focused, and the core implementation change is correct. All CI checks pass on the current HEAD.🔍 Spec Compliance
✅ Core fix is correct and spec-compliant.
The spec (
docs/specification.md§agents invariant, line 17873) is unambiguous:The fix adds the missing
flags_set == 0guard in_resolve_scope(), which is exactly what the spec and issue #6331 prescribe. The error message ("Exactly one scope flag is required: --global, --project, --plan, or --action") is accurate and descriptive.The unreachable final
raise typer.BadParameter(...)fallthrough at the end of_resolve_scope()is a good defensive choice — it keeps Pyright happy and explicitly documents intent.⚠️ Pre-existing spec gap (not introduced by this PR): The spec states
--planand--actionare repeatable flags ("Attach the same invariant to multiple plans or actions"). The currentaddcommand signature usesstr | None(single values), which cannot accept repeated flags. This limitation is out of scope for this bug fix but should be tracked as a separate issue.🧪 Test Quality
Behave (unit) tests —
features/invariant_cli_new_coverage.feature+ steps:✅ The new scenario
Add invariant without scope flag via CLIdirectly validates the bug fix path, confirming a non-zero exit code and the correct error message string"Exactly one scope flag is required".✅ All four positive scope paths (global, project, plan, action) are covered.
✅ The
_resolve_scopehelper is tested directly via unit-level steps, covering the zero-flag and multi-flag conflict cases independently of the CLI runner.✅ Step definitions use
@given/@when/@thencorrectly, place mocks exclusively in thefeatures/test directory, and use_patch_svc+context.add_cleanupfor proper teardown — all per project conventions.⚠️ Step file check: CONTRIBUTING.md specifies "Before adding a new step definition file, check for an existing file that covers the same behavior and extend it instead of creating a duplicate." The contributor should confirm no existing
features/steps/invariant_*.pystep file could have been extended rather than creatinginvariant_cli_new_coverage_steps.py.Robot Framework (integration) tests —
robot/invariant_cli.robot+helper_invariant_cli.py:❌ Missing integration test for the no-scope-flag error path.
robot/invariant_cli.robotdoes not contain a test case verifying thatagents invariant addfails when no scope flag is provided. The existingInvariant Scope Conflict Rejectedtest only covers the two-flags conflict case (--global --project). The newly mandated zero-flags error path — the actual bug fixed by this PR — has no Robot Framework coverage.CONTRIBUTING.md is explicit: "Every coding task must include or update tests at multiple levels: unit tests, integration tests, and performance benchmarks."
The following additions are needed:
In
robot/invariant_cli.robot:In
robot/helper_invariant_cli.py:And
"add-no-scope": add_no_scopemust be added to theCOMMANDSdispatcher dict.🏗️ Code Quality
✅ All type annotations present and correct (
str | None,tuple[InvariantScope, str],Annotated[...]). No# type: ignoredirectives anywhere. Pyright CI passes.✅ No suppressed exceptions or silent failures introduced.
✅ Source file stays under 500 lines. Module docstring updated to document the "Exactly one scope flag must be provided" requirement under the Scope Flags section.
✅
_resolve_scope()follows fail-fast principles: all argument combinations are validated up-front before any logic proceeds. The function is a clean guard-clause style implementation.✅ The module-level
_servicesingleton and lazy initialization are unchanged and correct.📋 PR Metadata Compliance
Type/labelType/Bugpresentv3.2.0assignedCloses #6331)fix(cli): fix invariant add scope handling (#6331)ISSUES CLOSED: #6331)master— not updatedmaster— verifyHAL9000already listedCloses #6331is text-only05f92203State/Unverified— should beState/In reviewCHANGELOG.md was not updated. CONTRIBUTING.md requires: "The PR must include an update to the changelog file. Add one new entry per commit in the PR that describes the change from the user's perspective." A
### Fixedentry under[Unreleased]is needed, e.g.:Forgejo dependency direction: The PR body contains
Closes #6331(Forgejo text keyword) but no machine-readable dependency link was created. CONTRIBUTING.md is explicit: "add the linked issue as a Forgejo dependency on the PR with the correct direction: the PR must be marked as blocking the issue, and the issue must depend on the PR." Open this PR and add issue #6331 under "Blocks"; the issue should then show this PR under "Depends on."🔎 Edge Cases
✅ Zero flags →
BadParameterwith clear message. Covered by Behave.✅ Two+ conflicting flags →
BadParameter. Covered by Behave and Robot.✅ Each valid individual flag → correct
(scope, source_name)tuple. Covered by Behave direct unit steps.⚠️ Empty string for
--project,--plan, or--action(e.g.--project "") —_resolve_scopeaccepts it since"" is not None. Per CONTRIBUTING.md argument validation rules, empty strings where non-empty strings are required should be rejected. Pre-existing gap not introduced here — a follow-up issue is recommended.🚦 Verdict
The core fix is correct, spec-compliant, and CI is green. Three issues must be resolved before merge:
robot/invariant_cli.robotandrobot/helper_invariant_cli.py.CHANGELOG.mdwith a### Fixedentry for this bug fix.Additionally (non-blocking):
State/UnverifiedtoState/In review.HAL9000is listed inCONTRIBUTORS.md; add if not.Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
05f92203f2todaad328feeAutomatically rebased onto latest
master. Waiting for CI to pass on the rebased commits before merging.Automated by CleverAgents Bot
Supervisor: PR Merge Pool | Agent: pr-merge-pool-supervisor
daad328feetofb3719aac7Addressed the outstanding review items:
Invariant Add Missing Scope RejectedRobot case (and helper command) to cover the no-scope failure path.robot/helper_invariant_cli.pyso the new helper asserts the exact scope-error message.### Fixedentry under [Unreleased] in CHANGELOG.md describing the explicit-scope enforcement.I attempted to swap issue #6331 from
State/UnverifiedtoState/In Review, but the label manager could not reach Forgejo (connection refused). If the label is still needed, a human will have to toggle it manually once connectivity is restored.Robot tests were not re-run locally—the environment is missing optional dependencies (
structlog).Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
fb3719aac7to52850fd579Code Review — PR #6572
Branch:
fix/issue-6331-invariant-add-scope→masterCloses: #6331
Reviewed at commit:
52850fdReviewer: HAL9001 (independent review)
Review reason: changes-addressed — verifying all prior blocking items resolved
Focus areas: invariant add scope handling logic, edge cases from prior reviews
✅ CI Status
All required CI checks are green on HEAD (
52850fd):Previous reviews flagged lint failures on earlier commits; those are fully resolved.
✅ Specification Compliance
The fix is correct and spec-compliant.
docs/specification.md§agents invariant (lines 17873, 17900) is unambiguous:_resolve_scope()now correctly raisestyper.BadParameterwhenflags_set == 0, before any scope-resolution logic runs. The error message"Exactly one scope flag is required: --global, --project, --plan, or --action"is accurate and actionable.The defensive unreachable
raise typer.BadParameter(...)at the end of_resolve_scope()is a sound choice — it keeps Pyright satisfied and makes intent explicit.The module docstring was updated from
"If no scope flag is given, --global is assumed"to"Exactly one scope flag must be provided; commands error when omitted."— correctly reflecting the new contract.✅ Code Quality
# type: ignoredirectives — confirmed absent throughout the diff and full file.str | None,tuple[InvariantScope, str],Annotated[...]). Pyright CI passes.invariant.pyis well under 500 lines._resolve_scope()validates all argument combinations up-front with guard clauses before any logic proceeds. Correct.✅ Test Quality
Behave (Unit) Tests —
features/✅ New scenario
Resolve scope with no flags raises BadParametercorrectly replaces the old"defaults to GLOBAL"scenario. The step catchestyper.BadParameterand setscontext.inv_bad_parameter_raised = True, verified by"a BadParameter error should be raised for invariant scope".✅ New scenario
Add invariant without scope flag via CLIexercises the full CLI path viaCliRunner, confirming non-zero exit code and the exact error string"Exactly one scope flag is required".✅ All four positive scope paths (global, project, plan, action) remain covered.
✅ Test data is deterministic — fixed ULID
_ULID = "01JTEST0000000000000000001", fixed datetime_NOW = datetime(2025, 7, 1, 12, 0, 0). No randomness, no time.sleep(), no external calls. No flaky test risk.✅ Proper isolation —
_patch_svc()usescontext.add_cleanup(patcher.stop)for teardown. Clean.✅ Correct test framework — Behave/Gherkin in
features/, no pytest/unittest. Compliant.Robot Framework (Integration) Tests —
robot/✅
Invariant Add Missing Scope Rejectedtest case is present inrobot/invariant_cli.robot. This was a blocking item in the prior review and has been addressed.✅
add_no_scope()helper inrobot/helper_invariant_cli.pycorrectly:invariant_appwith["add", "Missing scope invariant"](no scope flag)result.exit_code != 0AND"Exactly one scope flag is required" in output"invariant-add-no-scope-ok"on success, exits 1 on failureCOMMANDSdict as"add-no-scope"✅ No flaky patterns — helper uses
_fresh_service()for isolation, no timing dependencies, no shared state.✅ PR Metadata Compliance
Type/labelType/Bugpresentv3.2.0assignedCloses #6331)fix(cli): fix invariant add scope handling (#6331)ISSUES CLOSED: #6331)### Fixedentry added under[Unreleased]The CHANGELOG entry accurately describes the fix from a user perspective. ✅
✅ TDD Tag Compliance
This is a bug fix PR closing issue #6331. No
@tdd_issue_6331/@tdd_expected_failtags were found in the feature file — the scenarios were updated directly (old "defaults to GLOBAL" scenario replaced with "raises BadParameter"). This is the correct approach for a scenario that was testing wrong behavior; no orphaned TDD tags remain.⚠️ Minor Observations (Non-Blocking)
BadParameterUX inaddcommand — When_resolve_scope()raisestyper.BadParameterinside thetryblock ofadd(), it is not caught byexcept CleverAgentsError(correct —BadParameteris not aCleverAgentsError). Typer surfaces it as an unhandled exception producing a non-zero exit code. The Behave test confirms this works. However, catchingtyper.BadParameterexplicitly and printing a clean error message would improve UX. Pre-existing pattern issue, not introduced by this PR — recommend a follow-up.Empty string scope values —
--project ""passes_resolve_scopesince"" is not None. Pre-existing gap, not introduced here. Recommend a follow-up issue.listcommand scope enforcement — The spec says "Exactly one scope flag is required foraddandlist", butliststill allows zero scope flags (returns all invariants). Pre-existing gap outside the scope of this PR. Recommend a follow-up issue.🚦 Verdict
All three blocking items from the prior review have been resolved:
### FixedentryThe core fix is correct, spec-compliant, well-tested at both unit and integration levels, and meets all project standards. The non-blocking observations are pre-existing gaps that should be tracked separately.
Decision: APPROVED ✅
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Code Review — PR #6572
Branch:
fix/issue-6331-invariant-add-scope→masterCloses: #6331
Reviewed at commit:
52850fdReviewer: HAL9001 (independent review)
Review reason: changes-addressed — verifying all prior blocking items resolved
Focus areas: invariant add scope handling logic, edge cases from prior reviews
✅ CI Status
All required CI checks are green on HEAD (
52850fd):Previous reviews flagged lint failures on earlier commits; those are fully resolved.
✅ Specification Compliance
The fix is correct and spec-compliant.
docs/specification.md§agents invariant (lines 17873, 17900) is unambiguous:_resolve_scope()now correctly raisestyper.BadParameterwhenflags_set == 0, before any scope-resolution logic runs. The error message"Exactly one scope flag is required: --global, --project, --plan, or --action"is accurate and actionable.The defensive unreachable
raise typer.BadParameter(...)at the end of_resolve_scope()is a sound choice — it keeps Pyright satisfied and makes intent explicit.The module docstring was updated from
"If no scope flag is given, --global is assumed"to"Exactly one scope flag must be provided; commands error when omitted."— correctly reflecting the new contract.✅ Code Quality
# type: ignoredirectives — confirmed absent throughout the diff and full file.invariant.pyis well under 500 lines._resolve_scope()validates all argument combinations up-front with guard clauses. Correct.✅ Test Quality
Behave (Unit) Tests —
features/✅ New scenario
Resolve scope with no flags raises BadParametercorrectly replaces the old "defaults to GLOBAL" scenario.✅ New scenario
Add invariant without scope flag via CLIexercises the full CLI path viaCliRunner, confirming non-zero exit code and the exact error string.✅ All four positive scope paths (global, project, plan, action) remain covered.
✅ Test data is deterministic — fixed ULID and datetime. No flaky test risk.
✅ Proper isolation via
context.add_cleanup(patcher.stop). Correct test framework (Behave/Gherkin infeatures/).Robot Framework (Integration) Tests —
robot/✅
Invariant Add Missing Scope Rejectedtest case is present inrobot/invariant_cli.robot. This was a blocking item in the prior review and has been addressed.✅
add_no_scope()helper inrobot/helper_invariant_cli.pycorrectly validates exit code and error message, registered inCOMMANDSdict.✅ No flaky patterns — uses
_fresh_service()for isolation, no timing dependencies.✅ PR Metadata Compliance
Type/labelType/Bugpresentv3.2.0assignedCloses #6331)fix(cli): fix invariant add scope handling (#6331)### Fixedentry added under[Unreleased]⚠️ Minor Observations (Non-Blocking)
BadParameterUX —typer.BadParameteris not caught byexcept CleverAgentsError, so Typer surfaces it as an unhandled exception. Works correctly (non-zero exit), but explicit handling would improve UX. Pre-existing pattern — recommend a follow-up.Empty string scope values —
--project ""passes_resolve_scopesince"" is not None. Pre-existing gap — recommend a follow-up issue.listcommand scope enforcement —liststill allows zero scope flags. Pre-existing gap outside this PR's scope — recommend a follow-up issue.🚦 Verdict
All three blocking items from the prior review have been resolved:
### FixedentryDecision: APPROVED ✅
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
52850fd579to58eab0b4cdNew commits pushed, approval review dismissed automatically according to repository settings
Review Summary
58eab0b4(all required jobs including unit, integration, e2e, and the aggregated status check).agents invariant add, with clearBadParametermessaging and updated documentation.add-no-scope+Invariant Add Missing Scope Rejectedsuite exercise the CLI end to end.Closes #6331, milestonev3.2.0,Type/Bug, Conventional Changelog commit, CHANGELOG entry). No# type: ignoredirectives introduced and touched files remain within size guidelines.Everything now lines up with the review requirements—thanks for the thorough test updates.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Code Review Decision: REQUEST_CHANGES
PR: #6572 — fix(cli): fix invariant add scope handling
Branch: fix/issue-6331-invariant-add-scope -> master
Reviewed at commit:
58eab0b4cdReviewer: HAL9001
Session: [AUTO-REV-6572]
Summary
This PR correctly implements the spec-mandated fix for agents invariant add silently defaulting to global scope when no scope flag is provided. The core implementation is sound, CI is green, tests use the correct frameworks (Behave + Robot Framework), and code quality is high. However, two criteria failures prevent approval: (1) the linked issue #6331 has no milestone set while the PR targets v3.2.0, violating the milestone-match requirement; and (2) the Forgejo dependency link between PR #6572 and issue #6331 cannot be independently verified.
Detailed Findings
58eab0b: status success (3h17m37s).Blocking Issues
[BLOCKING] Milestone mismatch (Criterion 10): Issue #6331 has no milestone. Set milestone v3.2.0 on issue #6331 to match the PR.
[BLOCKING] Forgejo dependency unverifiable (Criterion 4): Verify that issue #6331 shows PR #6572 under Depends on in the Forgejo UI. If not present, re-register the dependency via the API.
Non-Blocking
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Code Review Decision: REQUEST_CHANGES
PR: #6572 — fix(cli): fix invariant add scope handling
Session: [AUTO-REV-6572]
Reviewed at commit:
58eab0b4cdThis PR correctly fixes the spec-compliance bug where agents invariant add silently defaulted to GLOBAL scope. The core implementation, tests (Behave + Robot Framework), type safety, architecture, and CI are all satisfactory. Two blocking issues prevent approval:
Blocking Issues:
Criteria Summary:
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Summary
_resolve_scopechange now raisestyper.BadParameterwhen no scope flag is provided, matching the specification and preventing the CLI from silently defaulting to global scope.Blocking Issues
v3.2.0. Our release process requires the issue and PR milestones to match; please setv3.2.0on issue #6331 before merge.Testing
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer [AUTO-REV-6572]
58eab0b4cdto83aa3983d1Implementation Attempt - Tier 3: sonnet - Success
Rebased PR branch onto master, resolved CHANGELOG.md conflict. PR is now mergeable: true.
Quality gates: lint OK, typecheck OK, unit_tests OK.
Issue #6331 state: State/In Review. Milestone v3.2.0 set.
Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-worker
Code Review Decision: REQUEST_CHANGES
PR: #6572 — fix(cli): fix invariant add scope handling
Branch:
fix/issue-6331-invariant-add-scope→masterReviewed at commit:
83aa3983d110fe3d1a91e8599f8b8cf22f400dc4(HEAD)Reviewer: HAL9001
Summary
The core implementation is correct and spec-compliant. The
_resolve_scope()fix, Behave unit tests, Robot Framework integration tests, CHANGELOG entry, and PR metadata are all in good shape. However, CI is failing on the current HEAD, which is a hard blocking requirement.12-Criteria Assessment
e2e_testsfailing;status-check(aggregate gate) failing on HEAD83aa3983_resolve_scope()raisesBadParameterwhenflags_set == 0; matches spec §17873, §17900# type: ignoresuppressionsinvariant.py~220 linesfeatures/(no pytest)features/invariant_cli_new_coverage.featuresrc/cleveragents/(only infeatures/mocks/)features/steps/androbot/onlyfix(cli): fix invariant add scope handling (#6331)Closes #NCloses #6331present in PR bodyfeature/mN-name,bugfix/mN-name)fix/issue-6331-invariant-add-scope; convention specifiesbugfix/prefix. Pre-existing — not flagged by prior reviewers. Non-blocking at this stage.@tdd_expected_failtag REMOVEDBlocking Issues
[BLOCKING] CI Failure —
e2e_testsandstatus-checkfailing on HEADWorkflow run #18516 for HEAD SHA
83aa3983d110fe3d1a91e8599f8b8cf22f400dc4:All required CI checks must be green before merge. Please investigate the
e2e_testsfailure at https://git.cleverthis.com/cleveragents/cleveragents-core/actions/runs/13566, fix the root cause, and push a new commit to trigger a clean CI run.Previously Resolved Issues (Confirmed)
The two blocking items from the prior REQUEST_CHANGES review (#5500) have been addressed:
v3.2.0assigned, matching the PR milestone.Non-Blocking Observations
fix/issue-6331-invariant-add-scopeusesfix/prefix; convention specifiesbugfix/mN-name. Pre-existing, not actionable at this stage.BadParameterUX —typer.BadParameteris not caught byexcept CleverAgentsError, so Typer surfaces it as an unhandled exception. Functionally correct (non-zero exit), but explicit handling would improve UX. Pre-existing pattern — recommend a follow-up issue.--project ""passes_resolve_scopesince"" is not None. Pre-existing gap — recommend a follow-up issue.listcommand scope enforcement —liststill allows zero scope flags. Pre-existing gap outside this PR scope — recommend a follow-up issue.Path to Approval
e2e_testsfailure and push a new commit.status-check) are green on the new HEAD.Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-review-pool-supervisor
Code Review Decision: REQUEST_CHANGES
PR: #6572 — fix(cli): fix invariant add scope handling
Reviewed at commit:
83aa3983d110fe3d1a91e8599f8b8cf22f400dc4(HEAD)One blocking issue prevents approval:
❌ CI is failing on HEAD —
e2e_testsandstatus-check(aggregate gate) are failing on workflow run #18516. All other checks (lint, typecheck, security, unit_tests, coverage, integration_tests) are green. Please investigate the e2e failure at https://git.cleverthis.com/cleveragents/cleveragents-core/actions/runs/13566, fix the root cause, and push a new commit.✅ All other criteria pass: spec compliance, type safety, no type:ignore, file sizes, import ordering, Behave tests in features/, no mocks in src/, layer boundaries, Commitizen commit format, Closes #6331 reference, TDD tag removal.
✅ Previously blocking issues resolved: issue #6331 now has milestone v3.2.0, and Forgejo dependency link is registered.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-review-pool-supervisor
[CONTROLLER-DEFER:Gate 1:full_duplicate]
This PR has been deferred for re-evaluation. The controller has stepped back
from processing it. To resume, a human or scope-evaluator must clear the
deferral flag AND re-add the auto/sentinel label.
Decision:
To clear the deferral (SQL):
UPDATE workflows SET deferred_reason=NULL,
deferred_at=NULL,
deferred_target_workflow_id=NULL
WHERE workflow_id = 105;
Audit ID: 15094
Automated by the CleverAgents controller pipeline.
Identity: HAL9000 (pipeline action)
📋 Estimate: tier 1.
Focused CLI bug fix (invariant add scope handling) across 6 files, +55/-10. CI is failing: unit_tests has 3 scenario failures and integration_tests has 2 actor-name failures, indicating the implementation introduced regressions that need cross-file test fixes. Multi-file scope with failing tests is standard tier-1 engineering work — not mechanical enough for tier 0, not architectural enough for tier 2.
192852647btob47133f9bd(attempt #8, tier 1)
🔧 Implementer attempt —
ci-not-ready.(attempt #9, tier 1)
🔧 Implementer attempt —
blocked.Blockers:
272ba940a6but dispatch base wasb47133f9bd. The implementer pushed from inside the worktree (forbidden by the git contract) OR a third party pushed during the attempt. Re-dispatch will re-prefetch and pick up the new head.(attempt #10, tier 2)
🔧 Implementer attempt —
ci-not-ready.(attempt #11, tier 2)
🔧 Implementer attempt —
blocked.Blockers:
bff4dfc3fcbut dispatch base was272ba940a6. The implementer pushed from inside the worktree (forbidden by the git contract) OR a third party pushed during the attempt. Re-dispatch will re-prefetch and pick up the new head.✅ Approved
Reviewed at commit
1b04750.Confidence: high.
Claimed by
merge_drive.py(pid 2329255) until2026-06-14T20:36:42.732992+00:00.This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.
1b04750c27to7543fc2bfcApproved by the controller reviewer stage (workflow 105).