Skip to content

feat(kt-config): validate GraphTypePlugin version contract at registration#253

Merged
charlie83Gs merged 2 commits intomainfrom
feat/graph-type-version-contract-validation
Apr 20, 2026
Merged

feat(kt-config): validate GraphTypePlugin version contract at registration#253
charlie83Gs merged 2 commits intomainfrom
feat/graph-type-version-contract-validation

Conversation

@charlie83Gs
Copy link
Copy Markdown
Contributor

Summary

Catches broken version/migration contracts at register_graph_type before they can reach the Phase 7 migration planner. Four checks on the declared contract:

  • current_version must be an integer >= 1 (versions are 1-indexed; the default plugin ships at v1).
  • Every declared migration must be single-step: to_version == from_version + 1. Multi-step hops would leave gaps the planner can't walk.
  • from_version must be >= 1 — negative/zero source versions never correspond to a real plugin state.
  • Every hop from v1 to current_version - 1 must have at least one migration registered. A plugin shipping at v3 with only a 2 → 3 migration would strand v1 graphs.

Dup check runs first so re-registering the same graph_type_id stays a silent no-op (matches the existing idempotency contract, keeps test/dev-tooling stubs from tripping the migration contract when they re-register over a working plugin).

One drive-by fix: services/api/tests/test_graph_types_router.py::_FakeGraphType advertises current_version=3 with no migrations — silenced on main but trips the new validator. Added two inert _MigrationStub rows so it registers cleanly.

Test plan

  • libs/kt-config: 203 pass (12 new validation tests)
  • services/api: 2/2 graph-types-router tests pass after stub fix
  • Full just test-unit across all 24 packages: green
  • pre-commit + pre-push lefthook green

🤖 Generated with Claude Code

charlie83Gs and others added 2 commits April 20, 2026 11:42
…ation

Catches broken version/migration contracts before they reach the
Phase 7 migration planner:

- current_version must be an integer >= 1 (versions are 1-indexed;
  default plugin ships at v1).
- Every declared migration must be single-step: to_version ==
  from_version + 1. Multi-step hops would leave gaps the planner
  can't walk.
- from_version must be >= 1 — negative/zero source versions never
  correspond to a real plugin state.
- Every hop from v1 to current_version - 1 must have at least one
  migration registered. A plugin shipping at v3 with only a 2 -> 3
  migration would strand v1 graphs.

Dup check still runs first so re-registering over an existing id
stays a silent no-op (matches the existing idempotency contract;
also keeps test/dev-tooling stubs from tripping the migration
contract on the replacement).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PluginRegistry.register_graph_type now validates that every hop from
v1 to current_version has at least one migration declared. The
API router test's _FakeGraphType returns current_version=3 without
any migrations — silenced today but trips the validator introduced
in the same change.

Add two no-op _MigrationStub rows covering the v1 -> v2 and v2 -> v3
hops so the plugin registers cleanly. The router doesn't run
migrations; stubs are inert.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@charlie83Gs charlie83Gs merged commit edc4761 into main Apr 20, 2026
27 checks passed
@charlie83Gs charlie83Gs deleted the feat/graph-type-version-contract-validation branch April 20, 2026 17:53
@github-actions
Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

charlie83Gs added a commit that referenced this pull request Apr 20, 2026
…elta tiebreak

- Status filter inverted from ``status == 'active'`` to
  ``status.not_in(('deleted', 'provisioning', 'error'))`` — matches
  the docstring's exclusion list and makes future statuses
  (``archived`` / ``suspended`` / etc.) opt-out rather than opt-in.
  Reviewer flagged the drift risk correctly; the inversion moves
  the question to "do we skip this?" at review time.
- Test uses ``timedelta(microseconds=1)`` instead of mutating via
  ``replace()`` — same 1/1M flake guard as PR #258.
- Comment on ``_V2Plugin.migrations()`` + ``_M`` stub explaining the
  stub is load-bearing for ``register_graph_type``'s hop-coverage
  validator (PR #253), not dead code. Discovery tests don't call
  the planner, but they DO call ``register_graph_type``, which is
  what needs the migration row.
- New test ``test_unknown_status_is_auto_included`` pins the
  inverted-filter contract so a future refactor that silently
  narrows back to ``== 'active'`` trips here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant