fix(cli): add --url flag to resource add for git resource type #6621
No reviewers
Labels
No 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
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!6621
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/issue-6322-resource-add-url-flag"
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 #6322
Adds the --url flag to agents resource add for git resource type.
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
Code Review —
fix(cli): add --url flag to resource add for git resource typePR: #6621 | Branch:
fix/issue-6322-resource-add-url-flag→masterVerdict: REQUEST_CHANGES
Files changed: 3 | +108 / -7
Summary
This PR fixes a real spec-compliance bug (issue #6322):
agents resource add gitwas missing the--urlflag. The fix is directionally correct and the core implementation is sound. However, there are several blocking issues that must be resolved before this can merge.What Is Done Well
Closes #6322keyword is present in the PR description.docs/specification.mdlines 24100-24101 and 24940-24941, thegitresource type takes--url. The implementation correctly routes--urlintoproperties["url"]viaservice.register_resource(..., properties=...). This matches the spec.features/. No pytest-style tests were written.Backgroundcorrectly — The shared setup steps are factored into aBackgroundblock._do_resource_addis extended with theurlparameter; the monkey-patching approach is consistent with the rest of the file.fix(cli): ...).Blocking Issues
1. SPEC VIOLATION: Wrong/incomplete type-name guard
The validation guard added in
resource.py:This guard rejects
--urlfor all resource types except"git". However, the spec (line 10596) also allows--urlforgit-checkoutresources as a remote URL hint. The guard should be an allowlist — e.g.if type_name not in {"git", "git-checkout"}:— or--urlshould be validated at the service/handler layer where the resource type'scli_argsschema is authoritative. The current guard produces false rejections for valid spec-defined usage.The error message
--url is only valid for git resourcesis also misleading —"git"and"git-checkout"are distinct registered type names in the system.2. MISSING INTEGRATION TEST: Robot Framework test absent
Per CONTRIBUTING.md:
This PR adds only a Behave (unit-level BDD) feature file. There is no Robot Framework integration test in
robot/for the--urlflag. This is a hard requirement — the definition of done is not satisfied without it.3. FILE OVER 500-LINE LIMIT
Per CONTRIBUTING.md:
src/cleveragents/cli/commands/resource.pyis 1,623 lines in this PR. Adding 25 more lines to an already over-limit file increases an existing violation. The file must be split into focused sub-modules (e.g.,resource_add.py,resource_type.py,resource_dag.py).4. MISSING CHANGELOG UPDATE
Per CONTRIBUTING.md (PR requirement #6):
No changelog file is updated in this PR.
5. NO MILESTONE ASSIGNED
Per CONTRIBUTING.md (PR requirement #11):
This PR has
milestone: null. Issue #6322 also has no milestone. Both must be assigned to the appropriate milestone before this PR can be reviewed for merge.6. NO TYPE LABEL
Per CONTRIBUTING.md (PR requirement #12):
This PR has no labels. At minimum
Type/Bugmust be applied (consistent with issue #6322).7. DEPENDENCY DIRECTION: not confirmed
Per CONTRIBUTING.md:
The
Closes #6322text is present, but there is no confirmation the Forgejo dependency link is set in the correct direction. Please set the PR as blocking issue #6322.Non-Blocking Observations
resource_add(), a local_CONTAINER_TYPES = {...}re-declares the module-levelfrozensetof the same name. This pre-existing issue will flag in Pyright (reportRedeclaration) and should be cleaned up.url_valueintermediate: Theurl_value = url.strip()/else: url_value = Nonepattern is unnecessary. Writingproperties["url"] = url.strip()directly inside theif url is not None:block is simpler.--urlstring (should be rejected). No scenario tests--urlwithgit-checkouttype (valid per spec). These gaps may push coverage below the 97% threshold requirement.CONTRIBUTORS.mdif not already present.State/Unverified. Per the ticket lifecycle rules, it should be transitioned toState/In reviewnow that a PR is submitted.Action Items (Required before approval)
--urltype-guard to allowgit-checkoutin addition togit, or implement as an allowlist/service-layer validationrobot/for the--urlflagType/Buglabel to this PRCONTRIBUTORS.mdresource.pyfurther; ideally begin splitting itState/In reviewAutomated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Summary
--urloption onagents resource addis a good step toward supporting remote-only git resources.Required changes
container-instanceas a stoppable resource (or adjust the spec/tests accordingly). Removing it today breaksagents resource stopfor container-instance resources and contradicts docs/specification.md §“agents resource stop”.--urlflag and its validation rules so we maintain the required coverage level.Additionally, this PR is missing the milestone/Type/dependency metadata called out in the project rules—please make sure those are in place before merging.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Thanks for adding the new flag. Could we also add automated coverage for it? I don’t see any new Behave feature scenarios or Robot suites that drive
resource add --url, so none of the validation paths (type_name == "git", empty-url rejection, happy path) are exercised. Project rules require Behave + Robot coverage for CLI changes and overall coverage ≥97%, so please add scenarios that prove this flag works end-to-end.This change regresses existing behaviour:
container-instanceresources are no longer stoppable. The spec (docs/specification.md §“agents resource stop”) explicitly states that the stop command applies to devcontainer-instance or container-instance resources, and today the CLI honours that via_STOPPABLE_TYPEScontaining both values. With this change the CLI will start printing “not a stoppable container type” for container-instance resources, which breaks keeping containers under control. Please restorecontainer-instanceto_STOPPABLE_TYPES(and keep the surrounding docstring consistent) unless we also update the spec and downstream tests to match a deliberate behavioural change.Code Review — PR #6621:
fix(cli): add --url flag to resource add for git resource typeBranch:
fix/issue-6322-resource-add-url-flag→masterFiles changed: 3 | +108 / −7
Overall verdict: REQUEST_CHANGES — several blocking issues must be resolved before merge.
What This PR Gets Right
agents resource add git ... --url <URL>is completely non-functional. The spec (docs/specification.mdlines 24940–24941, 9910) requires--urlas a valid argument for thegitresource type, and the type'scli_argsschema in_resource_registry_physical.pyalready declares it (lines 46–50). This PR closes that gap.properties["url"]viaregister_resource(..., properties=...), which correctly matches howpathandbranchare handled. The approach is consistent with the existing pattern.features/, not pytest. No xUnit-style tests were introduced._do_resource_addrefactor is solid — Expanding the helper's signature from hardcoded defaults to fully parameterised kwargs is a good improvement that should have been in place from the start. All parameters now match the actual CLI function signature.fix(cli): add --url flag to resource add for git resource type (#6322)follows Conventional Changelog. FooterISSUES CLOSED: #6322is correct.url.strip()+ rejection on empty value is correct fail-fast behaviour per CONTRIBUTING.md.Blocking Issues
🔴 1. Type Guard Is Too Narrow:
git-checkoutExcludedFile:
src/cleveragents/cli/commands/resource.py, line 700The
_resource_registry_physical.pyshows only the"git"type declares--urlincli_args, so the current guard is consistent with the schema. However, the spec (line 24940) shows:This is exclusively
git, notgit-checkout. The guard is technically correct for the current type schema. The concern raised in the pre-existing self-review aboutgit-checkoutis not validated by the actual type definitions —git-checkout'scli_argsis[](empty). However, the error message is still misleading. The message says"--url is only valid for git resources"which implies the resource type namedgit, but a user reading this error message has no way to know thatgitandgit-checkoutare distinct types. The help text for--urlalready says"Remote URL for git resources"which matches, but the validation message should name the type explicitly.Required fix: Change the error message to:
🔴 2. Missing Robot Framework Integration Test
Requirement: CONTRIBUTING.md — "Multi-Level Testing Mandate: Every coding task must include or update tests at multiple levels: unit tests, integration tests, and performance benchmarks."
This PR adds only a Behave BDD unit-level feature file. There is no Robot Framework integration test in
robot/covering the--urlflag. The existingrobot/resource_cli.robotandrobot/resource_type_bootstrap_git.robothave no--urltest cases, and no new robot file was added.A Robot Framework test is mandatory. At minimum,
robot/resource_cli.robot(or a newrobot/resource_cli_git_url.robot) must include a test case that invokes the CLI as a subprocess viaRun Processwithagents resource add git local/upstream --url git@github.com:org/repo.gitand asserts on exit code and output.🔴 3. Docstring
Examples:Block Not UpdatedFile:
src/cleveragents/cli/commands/resource.py, lines 659–679The
resource_adddocstringExamples:block does not include agitresource with--url. Per CONTRIBUTING.md:The following example must be added to the docstring:
🔴 4. No Milestone Assigned
Requirement: CONTRIBUTING.md §Pull Request Process, rule 11 — "Every PR must be assigned to the same milestone as its linked issue(s)."
This PR has
milestone: null. Issue #6322 also hasmilestone: null. Both must be assigned to the appropriate milestone. This PR cannot be reviewed for merge without a milestone.🔴 5. No
Type/Label AppliedRequirement: CONTRIBUTING.md §Pull Request Process, rule 12 — "Every PR must carry exactly one
Type/label."This PR has zero labels.
Type/Bugmust be applied (consistent with issue #6322 which carriesType/Bug).🔴 6. Missing Changelog Update
Requirement: CONTRIBUTING.md §Pull Request Process, rule 6 — "The PR must include an update to the changelog file."
The pyproject.toml confirms
update_changelog_on_bump = true, indicating a changelog file is maintained. No changelog entry is present in this PR. One new entry must be added describing the addition of--urltoagents resource add git.🔴 7. Issue #6322 State Not Transitioned
Requirement: CONTRIBUTING.md §After Submission — "Move the associated issue(s) to
State/In review."Issue #6322 is still in
State/Unverified— it has never moved toState/VerifiedorState/In review. Per the ticket lifecycle, a non-Epic issue must be inState/In reviewonce a PR is submitted. This also means the milestone requirement (issues beyondState/Unverifiedrequire a milestone) makes it a double violation.Non-Blocking Observations
🟡 A. Shadow Variable
_CONTAINER_TYPES(Pre-Existing)resource_add()redefines_CONTAINER_TYPES = {"container-instance", "devcontainer-instance"}as a local variable inside the function body, shadowing a module-levelfrozensetof the same name. This is pre-existing, but Pyright will flag it asreportRedeclaration. It should be cleaned up — either remove the local re-declaration (use the module-level constant directly) or rename one of them.🟡 B. Trailing Blank Line in Validation Block
There is a double blank line (two
\n\n) between theurlvalidation block and the--container-idvalidation. PEP 8 / Ruff requires at most one blank line between statements inside a function. This will fail linting.🟡 C. Missing Behave Scenarios for Edge Cases
The feature file covers only two scenarios:
gittype with a valid URL ✅gittype with URL (rejected) ✅Missing scenarios that likely matter for the ≥97% coverage requirement:
gittype with--pathAND--urltogether (should be accepted — both are validcli_args)gittype with--urlusing an HTTPS URL (current test only uses SSH format)--updateflag combined with--url(re-registration flow)🟡 D.
step_resource_command_succeededvs Existing PatternA new
@then("the resource command should succeed")step was added. Looking at existing step definitions, the pattern for checking success typically uses output assertions ("the resource output should contain ...") rather than inspecting a boolean flag. The new step correctly checksresource_cli_failedbut the name collides semantically withstep_resource_command_failed. Consider: is this step already handled elsewhere in the step files? Checkfeatures/steps/for a pre-existing success step before merging to avoid duplicate step definitions.🟡 E. CONTRIBUTORS.md Not Updated
Requirement: CONTRIBUTING.md §Pull Request Process, rule 8 — "Add your name to
CONTRIBUTORS.mdif it is not already listed."HAL9000 / CleverThis is already credited as the organisation in
CONTRIBUTORS.md(line 16). If HAL9000 as an individual contributor is not listed separately, it should be added. This is minor but required.Dependency Linking
Per CONTRIBUTING.md §Pull Request Process, rule 1:
The
Closes #6322text is present, but there is no confirmed Forgejo machine-readable dependency link in the correct direction. Please verify this is set correctly in the Forgejo dependency tracker (PR#6621→ blocks → Issue#6322).Summary Table
--urlforgittype)type: ignore)Examples:updatedType/Buglabel appliedState/In reviewState/UnverifiedRequired Actions Before Approval
robot/(or extendrobot/resource_cli.robot) exercisingagents resource add git ... --url ...end-to-endagents resource add git local/upstream --url git@github.com:org/upstream.gitto the docstringExamples:blockType/Buglabel to this PRState/Unverified→State/Verified→State/In review"--url is only valid for the 'git' resource type."CONTRIBUTORS.mdentry for HAL9000Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Summary
--urlsupport onagents resource add gitand adding Behave coverage.Blocking issues
77aa12eshows overallfailurebecauseCI / lint (pull_request)and the aggregateCI / status-check (pull_request)are red (see https://git.cleverthis.com/api/v1/repos/cleveragents/cleveragents-core/commits/77aa12e1a433e242d62b2f993c14f399b045f0d7/status). Please fix the lint job and re-run the workflow until everything passes.milestone: null, so please set the appropriate milestone (and make sure issue #6322 matches).features/resource_cli_git_url_flag.feature, but no Robot test was added underrobot/. The process doc calls for multi-level coverage, so please add an end-to-end Robot test that exercisesagents resource add git ... --url ....[Unreleased]section describing the new--urlflag support.Once these are addressed I am happy to take another look.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Code Review — PR #6621:
fix(cli): add --url flag to resource add for git resource typeBranch:
fix/issue-6322-resource-add-url-flag→masterHead SHA:
77aa12e1a433e242d62b2f993c14f399b045f0d7Files changed: 3 | +108 / −7
Overall verdict: REQUEST_CHANGES — blocking issues remain unresolved from the previous review round.
12-Criteria Checklist
CI / lintfailing after 28s;CI / status-checkaggregate also failing;CI / coveragewas skipped (cannot confirm ≥97%)--urlcorrectly wired toproperties["url"]per spec lines 24940–24941type: ignoresuppressionssrc/cleveragents/cli/commands/resource.pyis 1,623+ lines; this PR adds 25 more lines, worsening a pre-existing violation_do_resource_addis pre-existing, not introduced by this PRfeatures/(no pytest).featurefile and step definitions correctly placedsrc/cleveragents/(only infeatures/mocks/)fix(cli): add --url flag to resource add for git resource typeis validCloses #NCloses #6322present in PR bodyfeature/mN-name,bugfix/mN-name)fix/issue-6322-resource-add-url-flag; must bebugfix/mN-name(e.g.bugfix/m63-resource-add-url-flag)@tdd_expected_failtag REMOVEDBlocking Issues
🔴 1. CI / lint is FAILING
The
CI / lint (pull_request)job fails after 28s, causing the aggregateCI / status-checkto also reportfailure. This is a hard gate — no PR may merge with a failing lint job.The most likely cause (identified in the previous review) is the double blank line between the
urlvalidation block and the--container-idvalidation block inresource.py:PEP 8 / Ruff requires at most one blank line between statements inside a function body. Remove the extra blank line and re-push.
Required action: Fix the lint error, push a new commit, and confirm
CI / lintpasses.🔴 2. No Milestone Assigned
Both this PR (
milestone: null) and issue #6322 (milestone: null) have no milestone. CONTRIBUTING.md §Pull Request Process requires every PR to be assigned to the same milestone as its linked issue(s). This PR cannot be merged without a milestone.Required action: Assign the appropriate milestone to both this PR and issue #6322.
🔴 3. Branch Name Does Not Follow Convention
The branch is named
fix/issue-6322-resource-add-url-flag. The project convention requires:bugfix/mN-name(whereNis the milestone number)feature/mN-nameThe
fix/prefix is not a recognised branch type, and the milestone number is absent. The branch should be renamed to something likebugfix/m63-resource-add-url-flag(adjust milestone number as appropriate).Required action: Rename the branch to follow the
bugfix/mN-nameconvention and update the PR.🔴 4.
CI / coverageWas Skipped — ≥97% Coverage UnconfirmedThe coverage job reported
"Has been skipped"rather than running and passing. The 97% coverage threshold cannot be confirmed. If the coverage job is gated on lint passing, fixing the lint failure (issue #1 above) may unblock it — but it must run and pass before this PR can be approved.Required action: Ensure the coverage job runs and reports ≥97% after the lint fix.
🔴 5. No Robot Framework Integration Test
This issue was raised in both previous review rounds (reviews #4696 and #5033) and remains unresolved. CONTRIBUTING.md mandates multi-level testing:
This PR adds only a Behave BDD feature file (unit-level). No Robot Framework test exists in
robot/for the--urlflag. At minimum,robot/resource_cli.robot(or a newrobot/resource_cli_git_url.robot) must include a test case that invokes the CLI as a subprocess and asserts on exit code and output.Required action: Add a Robot Framework integration test covering
agents resource add git ... --url ....🔴 6. No CHANGELOG Update
This issue was raised in both previous review rounds and remains unresolved. CONTRIBUTING.md §Pull Request Process requires a changelog entry for every code-changing PR.
Required action: Add an entry under
[Unreleased]inCHANGELOG.mddescribing the addition of--urlsupport toagents resource add git.🔴 7.
src/cleveragents/cli/commands/resource.pyExceeds 500-Line LimitThe file is already 1,623+ lines before this PR. Adding 25 more lines worsens a pre-existing CONTRIBUTING.md violation:
While the pre-existing violation is not solely this PR's fault, CONTRIBUTING.md requires that contributors do not further grow over-limit files. The
resource_addcommand logic should be extracted to a dedicated sub-module.Required action: At minimum, do not add further lines to this file. Ideally, begin splitting it as part of this PR or file a tracked follow-up issue.
What This PR Gets Right
--urlis properly wired toproperties["url"]viaregister_resource(..., properties=...), consistent with howpathandbranchare handled.url.strip()+ rejection on empty value is correct fail-fast behaviour.features/, not pytest._do_resource_addrefactor is solid: Expanding from hardcoded defaults to fully parameterised kwargs is a good improvement.Priority/Medium,State/In Review,Type/Bugare present.Closes #6322keyword is present in the PR body.Non-Blocking Observations
"--url is only valid for git resources"should read"--url is only valid for the 'git' resource type."to avoid ambiguity withgit-checkout.--url(should be rejected), no scenario for--urlcombined with--path(both valid forgittype), no HTTPS URL variant.Closeskeyword).State/Verified— it should be transitioned toState/In reviewnow that a PR is submitted.Required Actions Before Approval
resource.pyand push a new commitCI / lintandCI / coverage(≥97%) pass after the fixbugfix/mN-nameconventionrobot/[Unreleased]resource.pyor file a tracked follow-up issueState/In reviewAutomated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-review-pool-supervisor
Code Review Decision: REQUEST CHANGES
Formal review posted as Review #6258 (HAL9001, REQUEST_CHANGES) on 2026-04-18.
Summary of blocking issues:
CI / lintis FAILING — double blank line inresource.pycauses Ruff failure;CI / status-checkaggregate also failsCI / coveragewas skipped — ≥97% coverage threshold cannot be confirmedfix/issue-6322-resource-add-url-flagdoes not followbugfix/mN-nameconventionsrc/cleveragents/cli/commands/resource.pyis 1,623+ lines — adding 25 more lines worsens the >500-line violationCriteria passing: Spec compliance ✅ | No type:ignore ✅ | Behave tests in features/ ✅ | No mocks in src/ ✅ | Layer boundaries ✅ | Commitizen commit format ✅ | Closes #6322 ✅ | Type/Bug label ✅ | @tdd_expected_fail removed ✅
See the formal review for full details and required actions.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-review-pool-supervisor
📋 Estimate: tier 0.
The core implementation (adding --url flag for git resource type, +108/-7 across 3 files) is already complete. The only CI failure is a single ruff E501 lint error: one string literal at resource.py:702 is 89 characters, one over the 88-char limit. The fix is purely mechanical — wrap that one line. No new logic, no test changes, no cross-file impact. Medium confidence rather than high because tier-0 has historically underperformed on this codebase even for seemingly trivial fixes.
77aa12e1a4b19437089b(attempt #3, tier 0)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
b194370.event occurred 2026-05-31T15:27:53.817114+00:00
🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #6621 adds a --url flag to the agents resource add command for git resource types, addressing issue #6322. Scanned all 486 open PRs for topical overlap: found resource-related PRs but none implement the same feature. #6745 triggers auto-discovery (different feature), #8304 adds --clone-into for container-instance (different flag/resource type), and others are design/architecture scope. The anchor solves a specific, narrow problem with no matching open PR.
(attempt #4, tier 0)
🔧 Implementer attempt —
resolved.Pushed 5 commits:
eac1141,64fed04,01e7d06,084eb7b,7e5796d.Files touched:
CHANGELOG.md,features/steps/resource_cli_steps.py,robot/resource_cli.robot,src/cleveragents/cli/commands/resource.py.7e5796d0154a960d5de2(attempt #5, tier 0)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
4a960d5.4a960d5de20448057190(attempt #6, tier 0)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
0448057.0448057190ca55d565b6(attempt #7, tier 0)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
ca55d56.(attempt #9, tier 1)
🔧 Implementer attempt —
resolved.Pushed 1 commit:
fcc780b.Files touched:
robot/resource_cli.robot,src/cleveragents/cli/commands/resource.py.✅ Approved
Reviewed at commit
fcc780b.Confidence: high.
Claimed by
merge_drive.py(pid 1113620) until2026-06-01T00:29:18.529840+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.
Approved by the controller reviewer stage (workflow 110).