Skip to content

chore: sync with upstream API changes 2026-06-12#29

Merged
anneschuth merged 3 commits into
mainfrom
api-sync/2026-06-12-105742
Jun 19, 2026
Merged

chore: sync with upstream API changes 2026-06-12#29
anneschuth merged 3 commits into
mainfrom
api-sync/2026-06-12-105742

Conversation

@anneschuth

Copy link
Copy Markdown
Member

API Sync - 2026-06-12

Upstream API changes detected and auto-implemented by Claude.

API diff

New Endpoints: 2


POST /api/v2/admin/orphans/confirm
GET /api/v2/admin/orphans/report

Deleted Endpoints: None


Modified Endpoints: None


Coverage

Upstream API: 46 current endpoints
Covered by CLI: 36
Not covered: 10
Deprecated v1: 10 (skipped, CLI uses v2)
Non-API/infra: 0 (skipped)

Uncovered endpoints:
GET /api/federation/health
Federation Health [federation]
GET /api/federation/peers
List Peers [federation]
POST /api/projects/{project_name}/registries/by-credentials
Add Registry By Credentials [v1 (deprecated)]
POST /api/projects/{project_name}/registries/by-secret
Add Registry By Secret [v1 (deprecated)]
POST /api/tasks
Create Task [tasks]
POST /api/v1/projects/{project_name}/images/push
Push Image [images]
POST /api/v2/admin/cleanup/trigger
Trigger Cleanup [admin]
POST /api/v2/admin/orphans/confirm
Confirm Orphans [admin]
GET /api/v2/admin/orphans/report
Orphan Sweep Report [admin]
POST /api/v2/admin/reconciliation/trigger
Trigger Reconciliation [admin]


Review carefully before merging. All changes should be additive only.

@anneschuth anneschuth added api-sync Automated API sync changes automated Created by automated workflow labels Jun 12, 2026
claude[bot]
claude Bot previously requested changes Jun 12, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues found, both around test coverage. Backwards compatibility is clean — all changes are additive.

🟠 Missing client tests: get_orphan_report and confirm_orphans are not tested in test_client.py. Every other admin client method has a dedicated test (see test_list_admin_marked_* and test_delete_admin_mark_*). These should cover the HTTP path, payload shape, and basic response handling.

🟠 Missing CLI integration tests: test_cli.py has tests for admin delete dry-run and help output, but orphan-report and orphan-confirm have no CLI-level tests. Missing cases: orphan-report basic invocation, orphan-confirm --dry-run showing the endpoint, orphan-confirm with no --item (should exit 1), and orphan-confirm with an invalid item format.

🟡 keycloak_client realm not validated client-side (see inline): The API docs say realm is required for keycloak_client, but the CLI will silently send a payload without it and let the server return an error. A client-side check would give a clearer error message.

No backwards-incompatible changes. Design principles are followed: read-only orphan-report has no --yes/--dry-run, mutating orphan-confirm has both plus @handle_api_errors, dry-run is checked before confirm_action.

@anneschuth anneschuth force-pushed the api-sync/2026-06-12-105742 branch from e4bbe87 to 2e148ad Compare June 19, 2026 11:57
@claude claude Bot dismissed their stale review June 19, 2026 11:58

Superseded by new review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three findings from the previous review have been addressed — client tests, CLI integration tests, and the keycloak_client realm validation are all now present.

Three minor issues remain (see inline comments):

🟡 Test docstring (test_client.py): says "v1 GET" — should be "v2 GET".

🟡 Empty name not rejected (admin.py): --item "postgresql_database:" yields name="" and passes client-side validation. Add if not name: after the split.

🟡 Empty realm not rejected (admin.py): --item "keycloak_client:name:" sets entry["realm"] = "", so "realm" not in entry is False and the guard is bypassed. Change to if not entry.get("realm"):.

No backwards-incompatible changes. Design principles are followed throughout: orphan-report is correctly read-only (no --yes/--dry-run), orphan-confirm has both plus @handle_api_errors, dry-run is checked before confirm_action, and _request (not _async_request) is correctly used since both endpoints return 200, not 202.

@anneschuth

Copy link
Copy Markdown
Member Author

Review-opmerkingen verwerkt:

  • Client-tests toegevoegd voor get_orphan_report en confirm_orphans (HTTP-pad, payload-vorm, response) in test_client.py.
  • CLI-tests toegevoegd voor orphan-report en orphan-confirm in test_cli.py: basis-invocatie, --dry-run toont het endpoint, ontbrekend --item geeft exit 1, en ongeldig item-formaat wordt afgewezen.
  • keycloak_client valideert nu client-side: een ontbrekende realm geeft een duidelijke fout voordat er een API-call gaat. Ook het item-type wordt nu client-side gecontroleerd tegen de toegestane lijst.

Branch is gerebased op main. Alle tests groen, lint en format schoon.

github-actions Bot and others added 3 commits June 19, 2026 14:11
Implements two new upstream endpoints:
- GET /api/v2/admin/orphans/report → `zad admin orphan-report`
- POST /api/v2/admin/orphans/confirm → `zad admin orphan-confirm`

The report command is read-only and shows the classified orphan sweep.
The confirm command marks selected items for grace-period deletion via
repeatable --item TYPE:NAME (or TYPE:NAME:REALM for keycloak_client).
- client-tests voor get_orphan_report en confirm_orphans
- CLI-tests voor orphan-report/orphan-confirm (dry-run, ontbrekend item,
  ongeldig formaat, onbekend type, keycloak_client zonder realm)
- valideer item-type client-side en eis een realm voor keycloak_client
@anneschuth anneschuth force-pushed the api-sync/2026-06-12-105742 branch from 2e148ad to b8b4ec4 Compare June 19, 2026 12:11

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor findings; no critical or significant issues.

What's correct:

  • Both commands are purely additive — backwards-compat baseline updated correctly
  • orphan-confirm follows all mutating-command conventions: @handle_api_errors, --dry-run before confirm_action, --yes/-y, render_success
  • Client methods use _request (sync), which is correct: the OpenAPI spec shows 200 responses, not 202, so _async_request would be wrong here
  • No project requirement on admin commands — correct, these are cluster-level ops
  • Input validation is client-side before any API call — good defensive design
  • ORPHAN_TYPES / ORPHAN_TYPES_REQUIRING_REALM constants make the validation easy to extend

result = client.delete_admin_mark(mark_id)
formatter.render(result)
formatter.render_success(f"Deletion mark '{mark_id}' removed.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor — non-standard verb vocabulary

CLAUDE.md defines a binding verb table: list, create, add, delete, describe, status, refresh, update-image, check, assign. The sub-command names here use report and confirm, neither of which appears in that table.

orphan-report is the closer call — orphan-status would map to "show current state" semantics, though an active sweep is arguably more than a status read. orphan-confirm has no clean equivalent in the vocabulary.

This may be an intentional exception for admin-specific semantics with no better fit, but it deviates from the stated binding rules. If there is a deliberate exception policy for admin commands, it would be worth documenting in CLAUDE.md.

Comment thread tests/test_cli.py
assert "orphan-report" in out
assert "orphan-confirm" in out


Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor — no happy-path CLI test for orphan-report

orphan-report is read-only with no --dry-run flag, so the only meaningful CLI-level test path is a real execution with a mocked HTTP response. The current tests only verify help-text content (--dry-run and --yes are absent), which does not exercise the render path.

The client-level test (test_get_orphan_report_returns_json) covers the API call, but a subprocess test that mocks GET /v2/admin/orphans/report and checks formatter.render() output would catch regressions in the command plumbing. Compare test_confirm_orphans_sends_items_payload — a parallel test using respx on the CLI level would be the right companion here.

@anneschuth anneschuth merged commit 2393eab into main Jun 19, 2026
11 checks passed
@anneschuth anneschuth deleted the api-sync/2026-06-12-105742 branch June 19, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-sync Automated API sync changes automated Created by automated workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant