cleveragents/cleveractors-core

Fork

You've already forked cleveractors-core

Code Issues 21 Pull requests 4 Projects Releases Packages Wiki Activity Actions 373

test(registry): add failing regression for prefix-vs-type-name resolve path (#131) #133

Merged

CoreRasurae merged 1 commit from tdd/m1-registry-resolve-full-type-names into master

2026-08-11 09:34:04 +00:00

Conversation 3 Commits 1 Files changed 2 +48

CoreRasurae commented

2026-08-10 17:57:06 +00:00

Member

Copy link

Summary

Companion TDD issue-capture test for #131 ("Registry reference resolution builds /agt/… instead of /agent/…, failing with HTTP 404 on compliant registries"). Adds a Behave regression scenario proving the bug: _RegistryReferenceStrategy in reference_resolver.py maps package_type="agent" through _TEMPLATE_TYPE_TO_PACKAGE_TYPE to "agt" before calling RegistryClient.resolve_package, so PackageContentResolver.resolve(ref, package_type="agent") (the path AgentReferenceResolver uses, ADR-2037) requests GET /agt/{ns}/{name} instead of the spec-compliant GET /agent/{ns}/{name} (Package Registry Standard §8.2.2). pkg_agt_ is only the Package ID prefix (§3.2) — a separate concept from the URL route segment.

New scenario in features/registry_http_client.feature, steps added to the existing shared features/steps/registry_http_client_steps.py.
Tagged @tdd_issue @tdd_issue_131 @tdd_expected_fail.
Drives PackageContentResolver.resolve() against a mocked httpx transport and asserts the requested path is /agent/acme/assistant, not /agt/acme/assistant.
Confirmed locally that removing @tdd_expected_fail makes the assertion genuinely fail with AssertionError (bug reproduced); with the tag present, TddExpectedFailPolicy inverts the result so CI passes.

Closes #132

Test plan

nox -s lint — green
nox -s format -- --check — green
nox -s typecheck — green
nox -s security_scan — green
nox -s dead_code — green
nox -s unit_tests — 3064/3064 scenarios green
nox -s coverage_report — 96.7% (>= repo gate 96.5%)
nox -s integration_tests — 357/357 Robot tests green
nox -s benchmark_regression skipped — test-only change, no ASV-relevant code touched
Confirmed assertion fails via AssertionError (not ValueError/RuntimeError) when @tdd_expected_fail is removed locally

## Summary Companion TDD issue-capture test for #131 ("Registry reference resolution builds /agt/… instead of /agent/…, failing with HTTP 404 on compliant registries"). Adds a Behave regression scenario proving the bug: `_RegistryReferenceStrategy` in `reference_resolver.py` maps `package_type="agent"` through `_TEMPLATE_TYPE_TO_PACKAGE_TYPE` to `"agt"` before calling `RegistryClient.resolve_package`, so `PackageContentResolver.resolve(ref, package_type="agent")` (the path `AgentReferenceResolver` uses, ADR-2037) requests `GET /agt/{ns}/{name}` instead of the spec-compliant `GET /agent/{ns}/{name}` (Package Registry Standard §8.2.2). `pkg_agt_` is only the Package ID prefix (§3.2) — a separate concept from the URL route segment. - New scenario in `features/registry_http_client.feature`, steps added to the existing shared `features/steps/registry_http_client_steps.py`. - Tagged `@tdd_issue @tdd_issue_131 @tdd_expected_fail`. - Drives `PackageContentResolver.resolve()` against a mocked httpx transport and asserts the requested path is `/agent/acme/assistant`, not `/agt/acme/assistant`. - Confirmed locally that removing `@tdd_expected_fail` makes the assertion genuinely fail with `AssertionError` (bug reproduced); with the tag present, `TddExpectedFailPolicy` inverts the result so CI passes. Closes #132 ## Test plan - [x] `nox -s lint` — green - [x] `nox -s format -- --check` — green - [x] `nox -s typecheck` — green - [x] `nox -s security_scan` — green - [x] `nox -s dead_code` — green - [x] `nox -s unit_tests` — 3064/3064 scenarios green - [x] `nox -s coverage_report` — 96.7% (>= repo gate 96.5%) - [x] `nox -s integration_tests` — 357/357 Robot tests green - [x] `nox -s benchmark_regression` skipped — test-only change, no ASV-relevant code touched - [x] Confirmed assertion fails via `AssertionError` (not `ValueError`/`RuntimeError`) when `@tdd_expected_fail` is removed locally

CoreRasurae added 1 commit

2026-08-10 17:57:11 +00:00

test(registry): add failing regression for prefix-vs-type-name resolve path (#131 )

Some checks failed

CI / lint (pull_request) Successful in 1m29s

Details

CI / typecheck (pull_request) Successful in 1m58s

Details

CI / security (pull_request) Successful in 1m49s

Details

CI / quality (pull_request) Successful in 2m12s

Details

CI / build (pull_request) Successful in 2m18s

Details

CI / integration_tests (pull_request) Successful in 3m41s

Details

CI / unit_tests (pull_request) Successful in 6m32s

Details

CI / coverage (pull_request) Failing after 13m0s

Details

CI / benchmark (pull_request) Failing after 19m42s

Details

CI / status-check (pull_request) Failing after 6s

Details

CI / lint (push) Failing after 1m13s

Details

CI / typecheck (push) Failing after 1m42s

Details

CI / security (push) Failing after 1m34s

Details

CI / quality (push) Failing after 1m42s

Details

CI / unit_tests (push) Failing after 1m42s

Details

CI / coverage (push) Has been skipped

Details

CI / integration_tests (push) Failing after 1m47s

Details

CI / benchmark (push) Failing after 2m31s

Details

CI / build (push) Failing after 2m36s

Details

CI / status-check (push) Failing after 13s

Details

e8d41cc200

_RegistryReferenceStrategy in reference_resolver.py maps a package_type
through _TEMPLATE_TYPE_TO_PACKAGE_TYPE (e.g. "agent" -> "agt") before
calling RegistryClient.resolve_package, so a registry reference resolved
via PackageContentResolver.resolve(ref, package_type="agent") (as used by
AgentReferenceResolver, ADR-2037) requests GET /agt/{ns}/{name} instead of
GET /agent/{ns}/{name}. Per the Package Registry Standard §8.2.2, the
{package_type} URL segment is the full type name, not the pkg_agt_-style
Package ID prefix defined in §3.2 — a compliant registry answers 404 for
/agt/....

Adds a new scenario to features/registry_http_client.feature (extending
the existing shared registry_http_client_steps.py rather than a new step
file) that drives PackageContentResolver.resolve() for an "agent" registry
reference against a mocked httpx transport and asserts the requested path
is "/agent/acme/assistant", not "/agt/acme/assistant". Confirmed locally
that the assertion fails with AssertionError when @tdd_expected_fail is
removed, proving the bug is genuinely reproduced; with the tag present,
TddExpectedFailPolicy inverts the failure so nox -s unit_tests stays green.

ISSUES CLOSED: #132

CoreRasurae added this to the v2.1.0 milestone

2026-08-10 17:58:10 +00:00

CoreRasurae added the

Type

Testing

label

2026-08-10 17:58:17 +00:00

CoreRasurae added a new dependency

2026-08-10 17:58:48 +00:00

#132 TDD: Registry reference resolution builds /agt/… instead of /agent/…, failing with HTTP 404 on compliant registries

CoreRasurae referenced this pull request

2026-08-10 20:58:57 +00:00

fix(registry): use full package type names in registry resolve URL #134

hurui200320 approved these changes

2026-08-10 23:49:14 +00:00

hurui200320 left a comment

Copy link

PR Review: !133 (Ticket #132)

Verdict: Approve

This PR correctly implements the TDD issue-capture test required by the bug-fix workflow for #131. The new Behave scenario is properly tagged (@tdd_issue @tdd_issue_131 @tdd_expected_fail), drives the real PackageContentResolver.resolve() code path, and asserts the spec-compliant request path /agent/acme/assistant. The failure mode is a plain AssertionError, so TddExpectedFailPolicy will invert it as expected and keep CI green while the bug remains unfixed. The step definitions are clean, well-placed in the existing registry HTTP client step file, and follow the project’s BDD conventions.

I checked the PR conversation and found no existing review comments or author responses that would change this assessment.

Critical Issues

None

Major Issues

None

Minor Issues

Limited type coverage in the capture test
features/registry_http_client.feature:241
The scenario only reproduces the bug for package_type="agent". The root cause in _RegistryReferenceStrategy applies the _TEMPLATE_TYPE_TO_PACKAGE_TYPE mapping for all template/system types (graph, skill, template, etc.). The TDD issue (#132) only asks for an agent example, so this is acceptable for the capture PR, but the follow-up bugfix PR for #131 should add regression coverage for the other affected types as listed in #131’s acceptance criteria.
Coverage threshold documentation inconsistency
noxfile.py:27 / CONTRIBUTING.md:1240
The PR test plan reports 96.7% coverage against the noxfile.py threshold of 96.5%. Note that CONTRIBUTING.md states the project-specific gate is 97%. This is a pre-existing inconsistency, not introduced by this PR, and CI will use the noxfile.py value, so it does not block approval. It would be worth aligning the two documents so future TDD issues don’t carry contradictory acceptance criteria.

Nits

features/registry_http_client.feature:242-243 uses two consecutive When steps (When the mock server returns… / When I resolve…). The mock setup would read slightly better as a Given, but this matches the style already used throughout the file, so it is fine as-is.

Summary

A focused, well-scoped TDD capture PR. The test genuinely reproduces the /agt/ vs /agent/ URL bug, is correctly tagged for the expected-fail inversion workflow, and does not introduce any regressions or code-quality issues. The follow-up fix in #131 should remove @tdd_expected_fail and expand type coverage.

## PR Review: !133 (Ticket #132) ### Verdict: Approve This PR correctly implements the TDD issue-capture test required by the bug-fix workflow for #131. The new Behave scenario is properly tagged (`@tdd_issue @tdd_issue_131 @tdd_expected_fail`), drives the real `PackageContentResolver.resolve()` code path, and asserts the spec-compliant request path `/agent/acme/assistant`. The failure mode is a plain `AssertionError`, so `TddExpectedFailPolicy` will invert it as expected and keep CI green while the bug remains unfixed. The step definitions are clean, well-placed in the existing registry HTTP client step file, and follow the project’s BDD conventions. I checked the PR conversation and found no existing review comments or author responses that would change this assessment. ### Critical Issues None ### Major Issues None ### Minor Issues 1. **Limited type coverage in the capture test** `features/registry_http_client.feature:241` The scenario only reproduces the bug for `package_type="agent"`. The root cause in `_RegistryReferenceStrategy` applies the `_TEMPLATE_TYPE_TO_PACKAGE_TYPE` mapping for *all* template/system types (`graph`, `skill`, `template`, etc.). The TDD issue (#132) only asks for an agent example, so this is acceptable for the capture PR, but the follow-up bugfix PR for #131 should add regression coverage for the other affected types as listed in #131’s acceptance criteria. 2. **Coverage threshold documentation inconsistency** `noxfile.py:27` / `CONTRIBUTING.md:1240` The PR test plan reports 96.7% coverage against the `noxfile.py` threshold of 96.5%. Note that `CONTRIBUTING.md` states the project-specific gate is 97%. This is a pre-existing inconsistency, not introduced by this PR, and CI will use the `noxfile.py` value, so it does not block approval. It would be worth aligning the two documents so future TDD issues don’t carry contradictory acceptance criteria. ### Nits - `features/registry_http_client.feature:242-243` uses two consecutive `When` steps (`When the mock server returns…` / `When I resolve…`). The mock setup would read slightly better as a `Given`, but this matches the style already used throughout the file, so it is fine as-is. ### Summary A focused, well-scoped TDD capture PR. The test genuinely reproduces the `/agt/` vs `/agent/` URL bug, is correctly tagged for the expected-fail inversion workflow, and does not introduce any regressions or code-quality issues. The follow-up fix in #131 should remove `@tdd_expected_fail` and expand type coverage.

CoreRasurae commented

2026-08-11 08:27:23 +00:00

Author

Member

Copy link

Minor issues number 2, is not relevant, we have a noxfile threshold at 96.5% while having a 97% in the specification, but the 97% is the rounded value, which is also obtainable from a 96.5% real coverage.

CoreRasurae commented

2026-08-11 09:32:37 +00:00

Author

Member

Copy link

@hurui200320 Thanks for the review. I checked each Minor Issue against issue #132's own scope, the linked bug #131, docs/index.md/ADR-2037, and the existing coverage artifact before deciding what to change. Summary: no code changes applied — both Minor Issues, on inspection, don't call for a fix in this PR.

1. Limited type coverage in the capture test — not applied.
You already noted this is acceptable for the capture PR since #132 only asks for an agent example, and pointed to "the follow-up bugfix PR for #131" for the other types. I kept it that way rather than expanding scope here: #132's acceptance criteria explicitly scope the TDD scenario to package_type="agent" only, and #131's own acceptance criteria already list the skill/graph regression coverage as deliverables of the bugfix PR (where @tdd_expected_fail also gets removed). Adding that coverage in this PR would mean this TDD issue-capture commit does two things (agent capture + broader regression coverage) instead of one, which conflicts with the single-commit/single-responsibility scoping this issue was written with. So this is intentionally left for #131's PR, not overlooked.

2. Coverage threshold documentation inconsistency (noxfile.py:27 vs CONTRIBUTING.md:1240) — not applied, and I don't think it's actually an inconsistency.
I pulled the coverage artifact already produced for this PR (build/coverage.json) to check the numbers directly: raw percent_covered is 96.6978%, but Slipcover's own percent_covered_display field for that same run reports "97". So COVERAGE_THRESHOLD = 96.5 in noxfile.py isn't a stray/incorrect number relative to the documented 97% gate — it's calibrated to Slipcover's own rounding: any run that raw-scores >= 96.5% is a run Slipcover itself would display as 97%. CONTRIBUTING.md's "97% enforced merge gate" and noxfile.py's 96.5 floor describe the same effective gate, just at different precision (rounded-display vs. raw-with-buffer). This matches @CoreRasurae's comment on your review. Given that, and since noxfile.py's threshold is a project-wide CI gate (not something scoped to issue #132), I didn't touch it — changing the literal value would be a separate, unrelated decision outside this issue's atomic scope, and the underlying premise (a genuine mismatch) doesn't hold up once you look at Slipcover's own rounding behavior.

Nit (consecutive When/When steps) — no change, per your own note that it matches existing file style.

Happy to open a follow-up issue for either point if you'd still like the docstring/threshold values made more self-explanatory going forward — let me know and I'll file it rather than fold it into this TDD-capture commit.

@hurui200320 Thanks for the review. I checked each Minor Issue against issue #132's own scope, the linked bug #131, `docs/index.md`/ADR-2037, and the existing coverage artifact before deciding what to change. Summary: **no code changes applied** — both Minor Issues, on inspection, don't call for a fix in this PR. **1. Limited type coverage in the capture test — not applied.** You already noted this is acceptable for the capture PR since #132 only asks for an `agent` example, and pointed to "the follow-up bugfix PR for #131" for the other types. I kept it that way rather than expanding scope here: #132's acceptance criteria explicitly scope the TDD scenario to `package_type="agent"` only, and #131's own acceptance criteria already list the `skill`/`graph` regression coverage as deliverables of the *bugfix* PR (where `@tdd_expected_fail` also gets removed). Adding that coverage in this PR would mean this TDD issue-capture commit does two things (agent capture + broader regression coverage) instead of one, which conflicts with the single-commit/single-responsibility scoping this issue was written with. So this is intentionally left for #131's PR, not overlooked. **2. Coverage threshold documentation inconsistency (`noxfile.py:27` vs `CONTRIBUTING.md:1240`) — not applied, and I don't think it's actually an inconsistency.** I pulled the coverage artifact already produced for this PR (`build/coverage.json`) to check the numbers directly: raw `percent_covered` is `96.6978%`, but Slipcover's own `percent_covered_display` field for that same run reports `"97"`. So `COVERAGE_THRESHOLD = 96.5` in `noxfile.py` isn't a stray/incorrect number relative to the documented 97% gate — it's calibrated to Slipcover's own rounding: any run that raw-scores >= 96.5% is a run Slipcover itself would display as 97%. CONTRIBUTING.md's "97% enforced merge gate" and `noxfile.py`'s 96.5 floor describe the same effective gate, just at different precision (rounded-display vs. raw-with-buffer). This matches @CoreRasurae's comment on your review. Given that, and since `noxfile.py`'s threshold is a project-wide CI gate (not something scoped to issue #132), I didn't touch it — changing the literal value would be a separate, unrelated decision outside this issue's atomic scope, and the underlying premise (a genuine mismatch) doesn't hold up once you look at Slipcover's own rounding behavior. **Nit (consecutive `When`/`When` steps) — no change, per your own note that it matches existing file style.** Happy to open a follow-up issue for either point if you'd still like the docstring/threshold values made more self-explanatory going forward — let me know and I'll file it rather than fold it into this TDD-capture commit.

CoreRasurae merged commit e8d41cc200 into master

2026-08-11 09:34:04 +00:00

CoreRasurae deleted branch tdd/m1-registry-resolve-full-type-names

2026-08-11 09:34:16 +00:00

Reviewers

No reviewers

hurui200320

Labels

Clear labels

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