cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.0k Pull Requests 115 Actions Packages Projects Releases Wiki Activity

fix(resource-registry): use full inheritance chain in contextual binding type compatibility #3336

Merged
freemo merged 1 commits from bugfix/binding-resolution-inheritance-chain into master 2026-04-05 21:07:11 +00:00
Conversation 5 Commits 1 Files Changed 3
+134 -19
freemo commented 2026-04-05 10:06:49 +00:00
Owner
Copy Link
Copy Source

Summary

Fixes a bug in binding_resolution_service.py where _is_type_compatible performed only a flat one-level parent check against spec.parent_types, causing contextual resource binding to incorrectly reject valid bindings when the actual resource type was related to the required type through a multi-level inheritance chain (e.g., grandchild → child → parent).

Changes

  • src/cleveragents/application/services/binding_resolution_service.py:

    • Replaced the flat required_type in spec.parent_types check in _is_type_compatible with a delegation to self._registry.is_subtype_of(actual_type, required_type), which internally calls resolve_inheritance_chain() from cleveragents.resource.inheritance to walk the full ADR-042 inheritance chain to any depth.
    • Removed the now-unused ResourceTypeSpec import that was only needed to access parent_types directly.
    • _check_type_compatibility is implicitly fixed as it delegates to _is_type_compatible — no changes required there.

  • features/consolidated_binding_resolution.feature:

    • Added 4 new BDD scenarios covering the previously broken and adjacent cases:
      1. Two-level inheritance chain compatibility (child → parent).
      2. Three-level inheritance chain compatibility (grandchild → child → parent).
      3. Rejection of an unrelated type that shares no ancestry with the required type.
      4. Static binding resolved correctly via the full inheritance chain.

  • features/steps/binding_resolution_steps.py:

    • Extended the mock registry to expose is_subtype_of(), backed by the real is_subtype_of() function from cleveragents.resource.inheritance operating over an _inherits dict — ensuring tests exercise the actual inheritance logic rather than a stub.
    • Added new step definitions: the resource type X inherits from nothing and the resource type X inherits from Y to allow feature files to declaratively construct inheritance hierarchies.

Design Decisions

  • Delegate to registry.is_subtype_of() rather than re-implementing chain traversal: ResourceRegistryService already exposes is_subtype_of() as a public method. Using it keeps the API boundary clean and avoids duplicating or leaking knowledge of _load_type_registry() internals into the binding resolution layer.
  • Real inheritance logic in test mocks: The mock registry in the Behave step definitions uses the actual is_subtype_of() function from cleveragents.resource.inheritance (not a hand-rolled stub), so the new scenarios genuinely validate the inheritance traversal behaviour end-to-end at the unit/BDD level.
  • No changes to resolve_inheritance_chain() or is_subtype_of(): The root cause was entirely in the call site — the underlying chain-resolution logic in cleveragents.resource.inheritance was already correct per ADR-042. The fix is therefore minimal and surgical.

Testing

  • Unit tests (Behave): ✅ 20 scenarios passed (16 original + 4 new), 0 failed
  • lint: ✅ all checks passed
  • typecheck: ✅ 0 errors, 0 warnings

Related Issues

Closes #2929

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

## Summary Fixes a bug in `binding_resolution_service.py` where `_is_type_compatible` performed only a flat one-level parent check against `spec.parent_types`, causing contextual resource binding to incorrectly reject valid bindings when the actual resource type was related to the required type through a multi-level inheritance chain (e.g., grandchild → child → parent). ## Changes - **`src/cleveragents/application/services/binding_resolution_service.py`**: - Replaced the flat `required_type in spec.parent_types` check in `_is_type_compatible` with a delegation to `self._registry.is_subtype_of(actual_type, required_type)`, which internally calls `resolve_inheritance_chain()` from `cleveragents.resource.inheritance` to walk the full ADR-042 inheritance chain to any depth. - Removed the now-unused `ResourceTypeSpec` import that was only needed to access `parent_types` directly. - `_check_type_compatibility` is implicitly fixed as it delegates to `_is_type_compatible` — no changes required there. - **`features/consolidated_binding_resolution.feature`**: - Added 4 new BDD scenarios covering the previously broken and adjacent cases: 1. Two-level inheritance chain compatibility (child → parent). 2. Three-level inheritance chain compatibility (grandchild → child → parent). 3. Rejection of an unrelated type that shares no ancestry with the required type. 4. Static binding resolved correctly via the full inheritance chain. - **`features/steps/binding_resolution_steps.py`**: - Extended the mock registry to expose `is_subtype_of()`, backed by the real `is_subtype_of()` function from `cleveragents.resource.inheritance` operating over an `_inherits` dict — ensuring tests exercise the actual inheritance logic rather than a stub. - Added new step definitions: `the resource type X inherits from nothing` and `the resource type X inherits from Y` to allow feature files to declaratively construct inheritance hierarchies. ## Design Decisions - **Delegate to `registry.is_subtype_of()` rather than re-implementing chain traversal**: `ResourceRegistryService` already exposes `is_subtype_of()` as a public method. Using it keeps the API boundary clean and avoids duplicating or leaking knowledge of `_load_type_registry()` internals into the binding resolution layer. - **Real inheritance logic in test mocks**: The mock registry in the Behave step definitions uses the actual `is_subtype_of()` function from `cleveragents.resource.inheritance` (not a hand-rolled stub), so the new scenarios genuinely validate the inheritance traversal behaviour end-to-end at the unit/BDD level. - **No changes to `resolve_inheritance_chain()` or `is_subtype_of()`**: The root cause was entirely in the call site — the underlying chain-resolution logic in `cleveragents.resource.inheritance` was already correct per ADR-042. The fix is therefore minimal and surgical. ## Testing - Unit tests (Behave): ✅ 20 scenarios passed (16 original + 4 new), 0 failed - lint: ✅ all checks passed - typecheck: ✅ 0 errors, 0 warnings ## Related Issues Closes #2929 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
freemo commented 2026-04-05 10:19:21 +00:00
Author
Owner
Copy Link
Copy Source

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3336-1775373400]

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

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3336-1775373400] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo commented 2026-04-05 16:23:11 +00:00
Author
Owner
Copy Link
Copy Source

🔍 Code Review — REQUEST CHANGES

Reviewed PR #3336 with focus on api-consistency, naming-conventions, and code-patterns.

This is a well-scoped, surgical bugfix that correctly addresses the ADR-042 inheritance chain traversal issue. The core change — delegating _is_type_compatible to registry.is_subtype_of() instead of performing a flat parent_types membership check — is the right approach. The test coverage is thorough with 4 new BDD scenarios covering 2-level chains, 3-level chains, unrelated-type rejection, and static binding via inheritance.

However, there is one required change related to API contract consistency, and two PR metadata issues.

Required Changes

  1. [API-CONSISTENCY] Registry protocol contract is stale in docstrings

    • Location: src/cleveragents/application/services/binding_resolution_service.py
    • Class docstring (around line 50) states:

      The registry is injected as a protocol-compatible object exposing show_resource and show_type methods

    • __init__ docstring (around line 62) states:

      resource_registry: Object with show_resource(name_or_id) and show_type(name) methods.

    • Issue: The service now also requires is_subtype_of(type_name, ancestor_name) on the registry. Furthermore, show_type() is no longer called anywhere in this service (the only call site was in the old _is_type_compatible), so it is no longer a required method from this service's perspective.
    • Required: Update both docstrings to list is_subtype_of(type_name, ancestor_name) as a required method. Consider whether show_type should be removed from the documented contract (it may still be needed by other callers of the same registry, but this service no longer uses it).
    • Why: This is a first-class API consistency issue. Any consumer implementing a compatible registry (or writing a test mock) will follow these docstrings to know what methods to provide. The current docstrings are now misleading — they omit a required method and include one that is no longer used by this service.

  2. [METADATA] PR is missing milestone

    • Issue #2929 is assigned to milestone v3.2.0. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as its linked issue.
    • Required: Assign milestone v3.2.0 to this PR.

  3. [METADATA] PR is missing Type/ label

    • Per CONTRIBUTING.md, every PR must have exactly one Type/ label. This is a bugfix, so it should have Type/Bug.
    • Required: Add the Type/Bug label.

Deep Dive: API Consistency

Given special attention to API consistency across the change:

  • ✅ Method signature consistency: is_subtype_of(type_name, ancestor_name) on ResourceRegistryService matches the parameter order used in the call site (self._registry.is_subtype_of(actual_type, required_type)) — actual_type maps to type_name and required_type maps to ancestor_name. This is correct.
  • ✅ Mock fidelity: The test mock's is_subtype_of_mock correctly delegates to the real is_subtype_of() from cleveragents.resource.inheritance, ensuring tests exercise actual inheritance logic rather than a hand-rolled stub. Excellent practice.
  • ✅ Behavioral equivalence: The old code's actual_type == required_type fast-path and NotFoundError → False fallback are both preserved by the underlying is_subtype_of() implementation in inheritance.py (equality check at line 238, exception catch at lines 243-248).
  • ❌ Documented protocol contract: As noted above, the docstrings are now stale.

Deep Dive: Naming Conventions

  • ✅ Method names _is_type_compatible and _check_type_compatibility follow the project's underscore-prefixed private method convention
  • ✅ New step function names (step_type_inherits_nothing, step_type_inherits_from) follow the step_ prefix convention
  • ✅ The is_subtype_of_mock name clearly indicates it's a mock implementation
  • ✅ Feature scenario names are descriptive and follow the existing naming pattern

Deep Dive: Code Patterns

  • ✅ Delegation over duplication: The fix correctly delegates to the registry's existing is_subtype_of() rather than reimplementing chain traversal — respects Service Layer / Repository boundary
  • ✅ Single Responsibility: _is_type_compatible is now a thin wrapper, keeping binding resolution focused on binding logic
  • ✅ Fail-fast preserved: Error propagation paths are maintained
  • ✅ Clean import removal: The now-unused ResourceTypeSpec import was properly removed

Other Checks

  • ✅ Commit message: Follows Conventional Changelog format with ISSUES CLOSED: #2929 footer
  • ✅ Single atomic commit: One commit covering implementation + tests
  • ✅ PR description: Thorough, explains what/why/how
  • ✅ Closes #2929: Present in PR body
  • ✅ No # type: ignore: None found
  • ✅ File sizes: All files well under 500 lines
  • ✅ Test quality: Scenarios test meaningful behavior with clear Given/When/Then structure

Decision: REQUEST CHANGES 🔄

The code logic is correct and well-tested. The only blocking issue is the stale API contract documentation in the service docstrings, plus the missing PR metadata (milestone and label).

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

## 🔍 Code Review — REQUEST CHANGES Reviewed PR #3336 with focus on **api-consistency**, **naming-conventions**, and **code-patterns**. This is a well-scoped, surgical bugfix that correctly addresses the ADR-042 inheritance chain traversal issue. The core change — delegating `_is_type_compatible` to `registry.is_subtype_of()` instead of performing a flat `parent_types` membership check — is the right approach. The test coverage is thorough with 4 new BDD scenarios covering 2-level chains, 3-level chains, unrelated-type rejection, and static binding via inheritance. However, there is one required change related to API contract consistency, and two PR metadata issues. ### Required Changes 1. **[API-CONSISTENCY] Registry protocol contract is stale in docstrings** - Location: `src/cleveragents/application/services/binding_resolution_service.py` - **Class docstring** (around line 50) states: > *The registry is injected as a protocol-compatible object exposing `show_resource` and `show_type` methods* - **`__init__` docstring** (around line 62) states: > *resource_registry: Object with `show_resource(name_or_id)` and `show_type(name)` methods.* - **Issue**: The service now also requires `is_subtype_of(type_name, ancestor_name)` on the registry. Furthermore, `show_type()` is no longer called anywhere in this service (the only call site was in the old `_is_type_compatible`), so it is no longer a required method from this service's perspective. - **Required**: Update both docstrings to list `is_subtype_of(type_name, ancestor_name)` as a required method. Consider whether `show_type` should be removed from the documented contract (it may still be needed by other callers of the same registry, but this service no longer uses it). - **Why**: This is a first-class API consistency issue. Any consumer implementing a compatible registry (or writing a test mock) will follow these docstrings to know what methods to provide. The current docstrings are now misleading — they omit a required method and include one that is no longer used by this service. 2. **[METADATA] PR is missing milestone** - Issue #2929 is assigned to milestone **v3.2.0**. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as its linked issue. - **Required**: Assign milestone v3.2.0 to this PR. 3. **[METADATA] PR is missing `Type/` label** - Per CONTRIBUTING.md, every PR must have exactly one `Type/` label. This is a bugfix, so it should have `Type/Bug`. - **Required**: Add the `Type/Bug` label. ### Deep Dive: API Consistency Given special attention to API consistency across the change: - ✅ **Method signature consistency**: `is_subtype_of(type_name, ancestor_name)` on `ResourceRegistryService` matches the parameter order used in the call site (`self._registry.is_subtype_of(actual_type, required_type)`) — `actual_type` maps to `type_name` and `required_type` maps to `ancestor_name`. This is correct. - ✅ **Mock fidelity**: The test mock's `is_subtype_of_mock` correctly delegates to the real `is_subtype_of()` from `cleveragents.resource.inheritance`, ensuring tests exercise actual inheritance logic rather than a hand-rolled stub. Excellent practice. - ✅ **Behavioral equivalence**: The old code's `actual_type == required_type` fast-path and `NotFoundError → False` fallback are both preserved by the underlying `is_subtype_of()` implementation in `inheritance.py` (equality check at line 238, exception catch at lines 243-248). - ❌ **Documented protocol contract**: As noted above, the docstrings are now stale. ### Deep Dive: Naming Conventions - ✅ Method names `_is_type_compatible` and `_check_type_compatibility` follow the project's underscore-prefixed private method convention - ✅ New step function names (`step_type_inherits_nothing`, `step_type_inherits_from`) follow the `step_` prefix convention - ✅ The `is_subtype_of_mock` name clearly indicates it's a mock implementation - ✅ Feature scenario names are descriptive and follow the existing naming pattern ### Deep Dive: Code Patterns - ✅ **Delegation over duplication**: The fix correctly delegates to the registry's existing `is_subtype_of()` rather than reimplementing chain traversal — respects Service Layer / Repository boundary - ✅ **Single Responsibility**: `_is_type_compatible` is now a thin wrapper, keeping binding resolution focused on binding logic - ✅ **Fail-fast preserved**: Error propagation paths are maintained - ✅ **Clean import removal**: The now-unused `ResourceTypeSpec` import was properly removed ### Other Checks - ✅ **Commit message**: Follows Conventional Changelog format with `ISSUES CLOSED: #2929` footer - ✅ **Single atomic commit**: One commit covering implementation + tests - ✅ **PR description**: Thorough, explains what/why/how - ✅ **`Closes #2929`**: Present in PR body - ✅ **No `# type: ignore`**: None found - ✅ **File sizes**: All files well under 500 lines - ✅ **Test quality**: Scenarios test meaningful behavior with clear Given/When/Then structure **Decision: REQUEST CHANGES** 🔄 The code logic is correct and well-tested. The only blocking issue is the stale API contract documentation in the service docstrings, plus the missing PR metadata (milestone and label). --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo reviewed 2026-04-05 17:05:08 +00:00
freemo left a comment
Author
Owner
Copy Link
Copy Source

🔍 Code Review — COMMENT (Test-Focused)

Reviewed PR #3336 with focus on test-coverage-quality, test-scenario-completeness, and test-maintainability.

This is a well-scoped bugfix with a clean implementation — delegating _is_type_compatible to registry.is_subtype_of() is the right approach. The previous reviewer already flagged the stale docstrings and missing PR metadata (milestone + Type/ label), which I concur with. My review focuses specifically on the test dimension.

Deep Dive: Test Coverage Quality

✅ Real inheritance logic in mocks: The is_subtype_of_mock in binding_resolution_steps.py delegates to the real is_subtype_of() from cleveragents.resource.inheritance, building a type_registry dict from the mock's _inherits map. This is excellent practice — the tests exercise the actual chain-walking algorithm rather than a hand-rolled stub, giving genuine confidence in the fix.

✅ Meaningful scenario assertions: Each new scenario verifies concrete outcomes (resource_id, binding mode, deferred status, or ValidationError text), not just "it didn't crash". The three-level chain scenario (local/special-dev-container → devcontainer-instance → container-instance) directly reproduces the bug described in issue #2929.

✅ Negative testing included: The "rejects unrelated type even with inheritance present" scenario ensures the fix didn't accidentally make type checking too permissive — fs-directory correctly fails to match container-instance even when inheritance infrastructure is active.

✅ Existing scenarios still exercise the new code path: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario (which uses fs-directory with parent_types including git-checkout) continues to work because step_type_parent was updated to also populate _inherits, ensuring is_subtype_of() can walk the chain.

Deep Dive: Test Scenario Completeness

Issue #2929 acceptance criteria check:

  • ✅ _is_type_compatible uses is_subtype_of() — verified by all new scenarios
  • ✅ Tool requiring container-instance can bind to devcontainer-instance — two-level scenario
  • ✅ Tool requiring git-checkout cannot bind to unrelated fs-directory — unrelated-type scenario
  • ✅ Static binding uses corrected type compatibility — static binding inheritance scenario
  • ⚠️ Parameter binding via inheritance chain — NOT tested
  1. [TEST-GAP] Missing parameter binding inheritance scenario
    • Location: features/consolidated_binding_resolution.feature
    • Issue: The acceptance criteria in issue #2929 explicitly states: "Static and parameter binding also use the corrected type compatibility check." There is a new scenario for static binding via inheritance (@type_compat @inheritance @static_binding), but no corresponding scenario for parameter binding via inheritance.
    • The code path is covered because _resolve_parameter calls _check_type_compatibility → _is_type_compatible, which is the fixed method. However, having an explicit BDD scenario would:
      (a) Document this as a tested requirement
      (b) Guard against future regressions if parameter binding is refactored
      (c) Satisfy the acceptance criteria checklist completely
    • Suggested: Add a scenario like: 
      @type_compat @inheritance @parameter_binding
Scenario: Parameter binding resolves via two-level inheritance chain
  Given a mock resource registry
  And a binding resolution service using the registry
  Given a named resource "01HGZ6FE0AQDYTR4BX00000103" called "local/my-devcontainer" of type "devcontainer-instance" in the registry
  And the resource type "container-instance" inherits from nothing
  And the resource type "devcontainer-instance" inherits from "container-instance"
  And a tool "local/runner" with a parameter slot "env" of type "container-instance"
  And a project "local/proj" with no linked resources
  When the bindings are resolved with invocation params:
    | slot | ref                      |
    | env  | local/my-devcontainer    |
  Then the binding for slot "env" should have mode "parameter"
  And the binding for slot "env" should not be deferred

Deep Dive: Test Maintainability

  1. [TEST-MAINTAINABILITY] Stale scenario title and comments for NotFoundError case

    • Location: features/consolidated_binding_resolution.feature, scenario "Type compatibility returns False when show_type raises NotFoundError"
    • Issue: This scenario's title and section comment (# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)) reference show_type, but the service no longer calls show_type in _is_type_compatible. The method now delegates entirely to is_subtype_of(). The scenario still passes because is_subtype_of() internally handles unknown types, but the title is now misleading.
    • Impact: A future developer reading this scenario will think show_type is being tested, when in reality it's is_subtype_of's internal error handling that's being exercised. This creates confusion during maintenance.
    • Suggested: Update the scenario title to something like "Type compatibility returns False for unknown type" and update the section comment to reference is_subtype_of instead of show_type.

  2. [TEST-MAINTAINABILITY] Mock registry complexity growth

    • Location: features/steps/binding_resolution_steps.py, step_mock_registry
    • Observation (non-blocking): The mock registry now manages three parallel data structures (_resources, _types, _inherits). The step_type_parent step must update both _types and _inherits to stay consistent. This dual-update requirement is a potential source of subtle test bugs if a future contributor updates one but not the other.
    • The current implementation handles this correctly (both step_type_parent and the new step_type_inherits_from properly maintain both structures), but consider adding a brief comment in step_mock_registry explaining the relationship between _types (used by show_type) and _inherits (used by is_subtype_of).

  3. [TEST-QUALITY] Single-inheritance assumption in _inherits map

    • Location: features/steps/binding_resolution_steps.py, step_type_parent and step_type_inherits_from
    • Observation (non-blocking): The _inherits dict maps each type to a single parent (str | None), while parent_types supports multiple parents. The step_type_parent step uses parents[0] as the direct ancestor. This is fine for current scenarios but would silently produce incorrect results if a future test needs multi-parent inheritance. Worth a brief comment noting this limitation.

Other Checks (Standard Criteria)

  • ✅ Commit message: Follows Conventional Changelog format with ISSUES CLOSED: #2929 footer
  • ✅ Single atomic commit: One commit covering implementation + tests
  • ✅ PR description: Thorough, explains what/why/how with design decisions
  • ✅ Closes #2929: Present in PR body
  • ✅ No # type: ignore: None found
  • ✅ File sizes: binding_resolution_service.py ~330 lines, binding_resolution_steps.py ~430 lines — both well under 500
  • ✅ Imports at top of file: All imports are at the top (the from cleveragents.resource.inheritance import is_subtype_of inside step_mock_registry is acceptable as it's a test-time lazy import within a step function)
  • ✅ BDD test format: All new tests use proper Gherkin Given/When/Then structure
  • ✅ Tags: New scenarios properly tagged with @type_compat @inheritance plus specific tags
  • ❌ Stale docstrings: Already flagged by previous reviewer — class and __init__ docstrings still reference show_resource and show_type but not is_subtype_of
  • ❌ Missing milestone: Already flagged — should be v3.2.0
  • ❌ Missing Type/ label on PR: Already flagged — should have Type/Bug

Summary

Aspect Status
Test coverage quality ✅ Excellent — real inheritance logic in mocks, meaningful assertions
Test scenario completeness ⚠️ Good but missing parameter binding inheritance scenario
Test maintainability ⚠️ Stale scenario title for NotFoundError case; minor complexity concerns
Code correctness ✅ Fix is correct and minimal
Spec alignment ✅ Matches ADR-042 polymorphism guarantee
CONTRIBUTING.md compliance ❌ Missing milestone and Type/ label (already flagged)

Decision: COMMENT 💬

The code logic is correct and the test quality is high. The missing parameter binding inheritance scenario (item 1) is the most actionable finding from a test-completeness perspective. The stale scenario title (item 2) is a maintainability concern worth addressing. Items 3 and 4 are non-blocking observations.

Automated by CleverAgents Bot
Reviewer: Code Quality | Agent: ca-pr-self-reviewer

## 🔍 Code Review — COMMENT (Test-Focused) Reviewed PR #3336 with focus on **test-coverage-quality**, **test-scenario-completeness**, and **test-maintainability**. This is a well-scoped bugfix with a clean implementation — delegating `_is_type_compatible` to `registry.is_subtype_of()` is the right approach. The previous reviewer already flagged the stale docstrings and missing PR metadata (milestone + `Type/` label), which I concur with. My review focuses specifically on the test dimension. --- ### Deep Dive: Test Coverage Quality ✅ **Real inheritance logic in mocks**: The `is_subtype_of_mock` in `binding_resolution_steps.py` delegates to the *real* `is_subtype_of()` from `cleveragents.resource.inheritance`, building a `type_registry` dict from the mock's `_inherits` map. This is excellent practice — the tests exercise the actual chain-walking algorithm rather than a hand-rolled stub, giving genuine confidence in the fix. ✅ **Meaningful scenario assertions**: Each new scenario verifies concrete outcomes (resource_id, binding mode, deferred status, or ValidationError text), not just "it didn't crash". The three-level chain scenario (`local/special-dev-container → devcontainer-instance → container-instance`) directly reproduces the bug described in issue #2929. ✅ **Negative testing included**: The "rejects unrelated type even with inheritance present" scenario ensures the fix didn't accidentally make type checking too permissive — `fs-directory` correctly fails to match `container-instance` even when inheritance infrastructure is active. ✅ **Existing scenarios still exercise the new code path**: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario (which uses `fs-directory` with `parent_types` including `git-checkout`) continues to work because `step_type_parent` was updated to also populate `_inherits`, ensuring `is_subtype_of()` can walk the chain. ### Deep Dive: Test Scenario Completeness **Issue #2929 acceptance criteria check:** - ✅ `_is_type_compatible` uses `is_subtype_of()` — verified by all new scenarios - ✅ Tool requiring `container-instance` can bind to `devcontainer-instance` — two-level scenario - ✅ Tool requiring `git-checkout` cannot bind to unrelated `fs-directory` — unrelated-type scenario - ✅ Static binding uses corrected type compatibility — static binding inheritance scenario - ⚠️ **Parameter binding via inheritance chain — NOT tested** 1. **[TEST-GAP] Missing parameter binding inheritance scenario** - Location: `features/consolidated_binding_resolution.feature` - Issue: The acceptance criteria in issue #2929 explicitly states: *"Static and parameter binding also use the corrected type compatibility check."* There is a new scenario for static binding via inheritance (`@type_compat @inheritance @static_binding`), but no corresponding scenario for parameter binding via inheritance. - The code path *is* covered because `_resolve_parameter` calls `_check_type_compatibility` → `_is_type_compatible`, which is the fixed method. However, having an explicit BDD scenario would: (a) Document this as a tested requirement (b) Guard against future regressions if parameter binding is refactored (c) Satisfy the acceptance criteria checklist completely - **Suggested**: Add a scenario like: ```gherkin @type_compat @inheritance @parameter_binding Scenario: Parameter binding resolves via two-level inheritance chain Given a mock resource registry And a binding resolution service using the registry Given a named resource "01HGZ6FE0AQDYTR4BX00000103" called "local/my-devcontainer" of type "devcontainer-instance" in the registry And the resource type "container-instance" inherits from nothing And the resource type "devcontainer-instance" inherits from "container-instance" And a tool "local/runner" with a parameter slot "env" of type "container-instance" And a project "local/proj" with no linked resources When the bindings are resolved with invocation params: | slot | ref | | env | local/my-devcontainer | Then the binding for slot "env" should have mode "parameter" And the binding for slot "env" should not be deferred ``` ### Deep Dive: Test Maintainability 2. **[TEST-MAINTAINABILITY] Stale scenario title and comments for NotFoundError case** - Location: `features/consolidated_binding_resolution.feature`, scenario "Type compatibility returns False when show_type raises NotFoundError" - Issue: This scenario's title and section comment (`# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)`) reference `show_type`, but the service no longer calls `show_type` in `_is_type_compatible`. The method now delegates entirely to `is_subtype_of()`. The scenario still passes because `is_subtype_of()` internally handles unknown types, but the title is now misleading. - **Impact**: A future developer reading this scenario will think `show_type` is being tested, when in reality it's `is_subtype_of`'s internal error handling that's being exercised. This creates confusion during maintenance. - **Suggested**: Update the scenario title to something like "Type compatibility returns False for unknown type" and update the section comment to reference `is_subtype_of` instead of `show_type`. 3. **[TEST-MAINTAINABILITY] Mock registry complexity growth** - Location: `features/steps/binding_resolution_steps.py`, `step_mock_registry` - Observation (non-blocking): The mock registry now manages three parallel data structures (`_resources`, `_types`, `_inherits`). The `step_type_parent` step must update both `_types` and `_inherits` to stay consistent. This dual-update requirement is a potential source of subtle test bugs if a future contributor updates one but not the other. - The current implementation handles this correctly (both `step_type_parent` and the new `step_type_inherits_from` properly maintain both structures), but consider adding a brief comment in `step_mock_registry` explaining the relationship between `_types` (used by `show_type`) and `_inherits` (used by `is_subtype_of`). 4. **[TEST-QUALITY] Single-inheritance assumption in `_inherits` map** - Location: `features/steps/binding_resolution_steps.py`, `step_type_parent` and `step_type_inherits_from` - Observation (non-blocking): The `_inherits` dict maps each type to a single parent (`str | None`), while `parent_types` supports multiple parents. The `step_type_parent` step uses `parents[0]` as the direct ancestor. This is fine for current scenarios but would silently produce incorrect results if a future test needs multi-parent inheritance. Worth a brief comment noting this limitation. ### Other Checks (Standard Criteria) - ✅ **Commit message**: Follows Conventional Changelog format with `ISSUES CLOSED: #2929` footer - ✅ **Single atomic commit**: One commit covering implementation + tests - ✅ **PR description**: Thorough, explains what/why/how with design decisions - ✅ **`Closes #2929`**: Present in PR body - ✅ **No `# type: ignore`**: None found - ✅ **File sizes**: `binding_resolution_service.py` ~330 lines, `binding_resolution_steps.py` ~430 lines — both well under 500 - ✅ **Imports at top of file**: All imports are at the top (the `from cleveragents.resource.inheritance import is_subtype_of` inside `step_mock_registry` is acceptable as it's a test-time lazy import within a step function) - ✅ **BDD test format**: All new tests use proper Gherkin Given/When/Then structure - ✅ **Tags**: New scenarios properly tagged with `@type_compat @inheritance` plus specific tags - ❌ **Stale docstrings**: Already flagged by previous reviewer — class and `__init__` docstrings still reference `show_resource` and `show_type` but not `is_subtype_of` - ❌ **Missing milestone**: Already flagged — should be v3.2.0 - ❌ **Missing `Type/` label on PR**: Already flagged — should have `Type/Bug` ### Summary | Aspect | Status | |--------|--------| | Test coverage quality | ✅ Excellent — real inheritance logic in mocks, meaningful assertions | | Test scenario completeness | ⚠️ Good but missing parameter binding inheritance scenario | | Test maintainability | ⚠️ Stale scenario title for NotFoundError case; minor complexity concerns | | Code correctness | ✅ Fix is correct and minimal | | Spec alignment | ✅ Matches ADR-042 polymorphism guarantee | | CONTRIBUTING.md compliance | ❌ Missing milestone and Type/ label (already flagged) | **Decision: COMMENT** 💬 The code logic is correct and the test quality is high. The missing parameter binding inheritance scenario (item 1) is the most actionable finding from a test-completeness perspective. The stale scenario title (item 2) is a maintainability concern worth addressing. Items 3 and 4 are non-blocking observations. --- **Automated by CleverAgents Bot** Reviewer: Code Quality | Agent: ca-pr-self-reviewer
freemo force-pushed bugfix/binding-resolution-inheritance-chain from 04c18b86b8 to 3224f2136e 2026-04-05 17:55:58 +00:00 Compare
freemo added this to the v3.2.0 milestone 2026-04-05 17:56:05 +00:00
freemo commented 2026-04-05 17:56:32 +00:00
Author
Owner
Copy Link
Copy Source

Addressed all review feedback from both reviews. Here's a summary of what was implemented:

Changes Made

Required (REQUEST CHANGES review)

  1. [API-CONSISTENCY] Updated registry protocol contract in docstrings (binding_resolution_service.py):

    • Class docstring: replaced show_resource and show_type with show_resource and is_subtype_of as the required protocol methods
    • __init__ docstring: replaced show_type(name) with is_subtype_of(type_name, ancestor_name) in the Args description
    • show_type is no longer documented as a required method since this service no longer calls it

  2. [METADATA] Milestone v3.2.0 assigned ✅

  3. [METADATA] Type/Bug label added ✅

Addressed (COMMENT review)

  1. [TEST-GAP] Added parameter binding inheritance scenario (features/consolidated_binding_resolution.feature):

    • New scenario @type_compat @inheritance @parameter_binding: "Parameter binding resolves via two-level inheritance chain"
    • Uses local/my-devcontainer (type devcontainer-instance) with a parameter slot requiring container-instance
    • Exercises _resolve_parameter → _check_type_compatibility → _is_type_compatible → is_subtype_of() path
    • Completes the acceptance criteria coverage: contextual ✅, static ✅, parameter ✅

  2. [TEST-MAINTAINABILITY] Updated stale scenario title and comment (features/consolidated_binding_resolution.feature):

    • Section comment: # _is_type_compatible -- show_type raises NotFoundError (lines 369-370) → # _is_type_compatible -- is_subtype_of returns False for unknown type
    • Scenario title: Type compatibility returns False when show_type raises NotFoundError → Type compatibility returns False for unknown type

Items 3 and 4 from the COMMENT review (mock registry complexity and single-inheritance assumption) were non-blocking observations and are acknowledged but not changed, as the current implementation correctly handles all tested scenarios.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Addressed all review feedback from both reviews. Here's a summary of what was implemented: ## Changes Made ### Required (REQUEST CHANGES review) 1. **[API-CONSISTENCY] Updated registry protocol contract in docstrings** (`binding_resolution_service.py`): - Class docstring: replaced `show_resource` and `show_type` with `show_resource` and `is_subtype_of` as the required protocol methods - `__init__` docstring: replaced `show_type(name)` with `is_subtype_of(type_name, ancestor_name)` in the Args description - `show_type` is no longer documented as a required method since this service no longer calls it 2. **[METADATA] Milestone v3.2.0 assigned** ✅ 3. **[METADATA] `Type/Bug` label added** ✅ ### Addressed (COMMENT review) 4. **[TEST-GAP] Added parameter binding inheritance scenario** (`features/consolidated_binding_resolution.feature`): - New scenario `@type_compat @inheritance @parameter_binding`: "Parameter binding resolves via two-level inheritance chain" - Uses `local/my-devcontainer` (type `devcontainer-instance`) with a parameter slot requiring `container-instance` - Exercises `_resolve_parameter` → `_check_type_compatibility` → `_is_type_compatible` → `is_subtype_of()` path - Completes the acceptance criteria coverage: contextual ✅, static ✅, parameter ✅ 5. **[TEST-MAINTAINABILITY] Updated stale scenario title and comment** (`features/consolidated_binding_resolution.feature`): - Section comment: `# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)` → `# _is_type_compatible -- is_subtype_of returns False for unknown type` - Scenario title: `Type compatibility returns False when show_type raises NotFoundError` → `Type compatibility returns False for unknown type` Items 3 and 4 from the COMMENT review (mock registry complexity and single-inheritance assumption) were non-blocking observations and are acknowledged but not changed, as the current implementation correctly handles all tested scenarios. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
freemo reviewed 2026-04-05 19:38:40 +00:00
freemo left a comment
Author
Owner
Copy Link
Copy Source

🔍 Code Review — APPROVED (Performance-Focused)

Reviewed PR #3336 with focus on performance-implications, resource-usage, and scalability.

This is a well-scoped, correct bugfix that addresses a real ADR-042 compliance gap. The previous review cycle's feedback (stale docstrings, missing milestone/label, missing parameter binding scenario, stale scenario titles) has been fully addressed. The implementation correctly delegates type compatibility checking to registry.is_subtype_of() rather than reimplementing chain traversal.

Deep Dive: Performance Implications

Given special attention to the performance characteristics of the change:

Old code path (_is_type_compatible before this fix):

spec = self._registry.show_type(actual_type)  # 1 DB query, 1 row
return required_type in spec.parent_types      # O(n) list membership

New code path (_is_type_compatible after this fix):

return self._registry.is_subtype_of(actual_type, required_type)

Which in ResourceRegistryService.is_subtype_of() does:

registry = self._load_type_registry()  # 1 DB query, ALL rows
return is_subtype_of(type_name, ancestor_name, registry)  # O(depth) walk, max 5

Analysis:

  • The old code fetched one row per type check (show_type → single filter_by query)
  • The new code fetches all rows per type check (_load_type_registry → query(ResourceTypeModel).all())
  • In _find_matching_resources(), _is_type_compatible is called once per linked resource. For a project with N linked resources and a tool with M slots, this means M×N full-table scans instead of M×N single-row queries.

Verdict: Non-blocking. This is acceptable for the following reasons:

  1. Correctness over performance: The old code was wrong — it only checked immediate parents, violating ADR-042's polymorphism guarantee. A correct-but-slower implementation is strictly better than a fast-but-broken one.

  2. Pre-existing API design: ResourceRegistryService.is_subtype_of() and _load_type_registry() already existed with this exact behavior. This PR correctly uses the existing public API rather than reimplementing chain traversal or reaching into private internals. The performance characteristic is a property of the registry service, not of this change.

  3. Bounded chain depth: resolve_inheritance_chain() is bounded by MAX_CHAIN_DEPTH = 5 and uses cycle detection, so the chain walk itself is O(1) in practice.

  4. Typical scale: Projects typically have a small number of linked resources (single digits), and tools typically have 1-3 resource slots. The M×N product is small in practice.

  5. Future optimization path: If this becomes a bottleneck, the proper fix is adding caching to _load_type_registry() (e.g., a short-lived TTL cache or per-request memoization), which is orthogonal to this bugfix and should be tracked as a separate optimization issue.

⚠️ Recommendation: Consider filing a follow-up issue to add caching to ResourceRegistryService._load_type_registry(). This would benefit not just binding resolution but all callers of is_subtype_of(), resolve_type_inheritance_chain(), and remove_type().

Deep Dive: Resource Usage

  • ✅ No resource leaks: The _load_type_registry() method properly uses try/finally to close sessions
  • ✅ No unbounded allocations: The type registry dict is bounded by the number of registered types (finite, typically < 50)
  • ✅ No connection pool exhaustion risk: Each is_subtype_of call opens and closes a session within the same call frame — no long-held connections
  • ✅ Memory: The in-memory registry dict created per call is small (just {name: {inherits: ..., built_in: ...}} per type) and is garbage-collected after each call

Deep Dive: Scalability

  • ✅ Chain depth bounded: MAX_CHAIN_DEPTH = 5 prevents runaway traversal
  • ✅ Cycle detection: resolve_inheritance_chain() uses a visited set to detect and reject circular inheritance
  • ✅ Error handling preserved: is_subtype_of() catches ResourceTypeParentNotFoundError, ResourceTypeCircularInheritanceError, and ResourceTypeInheritanceDepthError, returning False for any broken chain — this is safe and graceful
  • ⚠️ Linear scan in contextual binding: _find_matching_resources() iterates all linked resources linearly. This is fine for typical project sizes but would not scale to projects with hundreds of linked resources. (This is pre-existing behavior, not introduced by this PR.)

Standard Criteria Checks

Criterion Status Notes
Commit message format ✅ fix(resource-registry): ... with ISSUES CLOSED: #2929
PR metadata ✅ Closes #2929, milestone v3.2.0, Type/Bug label
Docstrings updated ✅ Class and __init__ docstrings now list show_resource and is_subtype_of
No # type: ignore ✅ None found
File sizes ✅ binding_resolution_service.py ~330 lines, binding_resolution_steps.py ~430 lines
Imports at top ✅ The from cleveragents.resource.inheritance import is_subtype_of inside step_mock_registry is acceptable as a test-time lazy import
Spec alignment ✅ Correctly implements ADR-042 §Tool Binding Polymorphism
Test quality ✅ 5 new scenarios covering 2-level, 3-level, unrelated-type, static, and parameter binding via inheritance
Test uses real logic ✅ Mock's is_subtype_of_mock delegates to real is_subtype_of() from cleveragents.resource.inheritance
Error handling ✅ Fail-fast preserved; _is_type_compatible is now a thin delegation
Single atomic commit ✅ One commit covering implementation + tests + review feedback
Previous review feedback ✅ All items from both previous reviews addressed

Code Correctness

  • ✅ Parameter order: self._registry.is_subtype_of(actual_type, required_type) correctly maps to is_subtype_of(type_name, ancestor_name) — actual_type is the concrete type being checked, required_type is the ancestor
  • ✅ Equality fast-path preserved: is_subtype_of() in inheritance.py starts with if type_name == ancestor_name: return True
  • ✅ Unknown type handling preserved: is_subtype_of() catches all chain errors and returns False, matching the old behavior where NotFoundError from show_type() returned False
  • ✅ No behavioral regression for existing scenarios: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario continues to work because step_type_parent was updated to also populate _inherits

Decision: APPROVED ✅

The fix is correct, minimal, well-tested, and properly addresses the ADR-042 compliance gap. The performance observation about _load_type_registry() being called per type check is non-blocking and should be addressed as a separate optimization if it becomes a bottleneck.

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

## 🔍 Code Review — APPROVED (Performance-Focused) Reviewed PR #3336 with focus on **performance-implications**, **resource-usage**, and **scalability**. This is a well-scoped, correct bugfix that addresses a real ADR-042 compliance gap. The previous review cycle's feedback (stale docstrings, missing milestone/label, missing parameter binding scenario, stale scenario titles) has been fully addressed. The implementation correctly delegates type compatibility checking to `registry.is_subtype_of()` rather than reimplementing chain traversal. --- ### Deep Dive: Performance Implications Given special attention to the performance characteristics of the change: **Old code path** (`_is_type_compatible` before this fix): ```python spec = self._registry.show_type(actual_type) # 1 DB query, 1 row return required_type in spec.parent_types # O(n) list membership ``` **New code path** (`_is_type_compatible` after this fix): ```python return self._registry.is_subtype_of(actual_type, required_type) ``` Which in `ResourceRegistryService.is_subtype_of()` does: ```python registry = self._load_type_registry() # 1 DB query, ALL rows return is_subtype_of(type_name, ancestor_name, registry) # O(depth) walk, max 5 ``` **Analysis:** - The old code fetched **one row** per type check (`show_type` → single `filter_by` query) - The new code fetches **all rows** per type check (`_load_type_registry` → `query(ResourceTypeModel).all()`) - In `_find_matching_resources()`, `_is_type_compatible` is called once per linked resource. For a project with N linked resources and a tool with M slots, this means M×N full-table scans instead of M×N single-row queries. **Verdict: Non-blocking.** This is acceptable for the following reasons: 1. **Correctness over performance**: The old code was *wrong* — it only checked immediate parents, violating ADR-042's polymorphism guarantee. A correct-but-slower implementation is strictly better than a fast-but-broken one. 2. **Pre-existing API design**: `ResourceRegistryService.is_subtype_of()` and `_load_type_registry()` already existed with this exact behavior. This PR correctly uses the existing public API rather than reimplementing chain traversal or reaching into private internals. The performance characteristic is a property of the registry service, not of this change. 3. **Bounded chain depth**: `resolve_inheritance_chain()` is bounded by `MAX_CHAIN_DEPTH = 5` and uses cycle detection, so the chain walk itself is O(1) in practice. 4. **Typical scale**: Projects typically have a small number of linked resources (single digits), and tools typically have 1-3 resource slots. The M×N product is small in practice. 5. **Future optimization path**: If this becomes a bottleneck, the proper fix is adding caching to `_load_type_registry()` (e.g., a short-lived TTL cache or per-request memoization), which is orthogonal to this bugfix and should be tracked as a separate optimization issue. ⚠️ **Recommendation**: Consider filing a follow-up issue to add caching to `ResourceRegistryService._load_type_registry()`. This would benefit not just binding resolution but all callers of `is_subtype_of()`, `resolve_type_inheritance_chain()`, and `remove_type()`. ### Deep Dive: Resource Usage - ✅ **No resource leaks**: The `_load_type_registry()` method properly uses try/finally to close sessions - ✅ **No unbounded allocations**: The type registry dict is bounded by the number of registered types (finite, typically < 50) - ✅ **No connection pool exhaustion risk**: Each `is_subtype_of` call opens and closes a session within the same call frame — no long-held connections - ✅ **Memory**: The in-memory registry dict created per call is small (just `{name: {inherits: ..., built_in: ...}}` per type) and is garbage-collected after each call ### Deep Dive: Scalability - ✅ **Chain depth bounded**: MAX_CHAIN_DEPTH = 5 prevents runaway traversal - ✅ **Cycle detection**: `resolve_inheritance_chain()` uses a `visited` set to detect and reject circular inheritance - ✅ **Error handling preserved**: `is_subtype_of()` catches `ResourceTypeParentNotFoundError`, `ResourceTypeCircularInheritanceError`, and `ResourceTypeInheritanceDepthError`, returning `False` for any broken chain — this is safe and graceful - ⚠️ **Linear scan in contextual binding**: `_find_matching_resources()` iterates all linked resources linearly. This is fine for typical project sizes but would not scale to projects with hundreds of linked resources. (This is pre-existing behavior, not introduced by this PR.) ### Standard Criteria Checks | Criterion | Status | Notes | |-----------|--------|-------| | Commit message format | ✅ | `fix(resource-registry): ...` with `ISSUES CLOSED: #2929` | | PR metadata | ✅ | `Closes #2929`, milestone v3.2.0, `Type/Bug` label | | Docstrings updated | ✅ | Class and `__init__` docstrings now list `show_resource` and `is_subtype_of` | | No `# type: ignore` | ✅ | None found | | File sizes | ✅ | `binding_resolution_service.py` ~330 lines, `binding_resolution_steps.py` ~430 lines | | Imports at top | ✅ | The `from cleveragents.resource.inheritance import is_subtype_of` inside `step_mock_registry` is acceptable as a test-time lazy import | | Spec alignment | ✅ | Correctly implements ADR-042 §Tool Binding Polymorphism | | Test quality | ✅ | 5 new scenarios covering 2-level, 3-level, unrelated-type, static, and parameter binding via inheritance | | Test uses real logic | ✅ | Mock's `is_subtype_of_mock` delegates to real `is_subtype_of()` from `cleveragents.resource.inheritance` | | Error handling | ✅ | Fail-fast preserved; `_is_type_compatible` is now a thin delegation | | Single atomic commit | ✅ | One commit covering implementation + tests + review feedback | | Previous review feedback | ✅ | All items from both previous reviews addressed | ### Code Correctness - ✅ **Parameter order**: `self._registry.is_subtype_of(actual_type, required_type)` correctly maps to `is_subtype_of(type_name, ancestor_name)` — actual_type is the concrete type being checked, required_type is the ancestor - ✅ **Equality fast-path preserved**: `is_subtype_of()` in `inheritance.py` starts with `if type_name == ancestor_name: return True` - ✅ **Unknown type handling preserved**: `is_subtype_of()` catches all chain errors and returns `False`, matching the old behavior where `NotFoundError` from `show_type()` returned `False` - ✅ **No behavioral regression for existing scenarios**: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario continues to work because `step_type_parent` was updated to also populate `_inherits` **Decision: APPROVED** ✅ The fix is correct, minimal, well-tested, and properly addresses the ADR-042 compliance gap. The performance observation about `_load_type_registry()` being called per type check is non-blocking and should be addressed as a separate optimization if it becomes a bottleneck. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo merged commit 096cd79fbf into master 2026-04-05 21:07:11 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
freemo
Labels
Clear labels
auto/needs-reevaluation
Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.
 controller-managed
Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.
 overdue
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 Label
Milestone
No items
No Milestone v3.2.0
Projects
Clear projects
No project
Assignees
Clear 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
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Blocks
#2929 UAT: bug(resource-registry): contextual resource binding type compatibility check does not use full inheritance chain
cleveragents/cleveragents-core
Reference: cleveragents/cleveragents-core#3336
Delete Branch "bugfix/binding-resolution-inheritance-chain"

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?