test(integrations): add registry self-consistency invariants by Davidson3556 · Pull Request #2773 · Tracer-Cloud/opensre

Davidson3556 · 2026-06-08T09:38:23Z

Describe the changes you have made in this PR -

Adds tests/integrations/test_registry_invariants.py with five static assertions over the integration registry and the CLI dispatch:

test_setup_orders_are_unique — no two services share a setup_order
test_verify_orders_are_unique — no two services share a verify_order
test_every_setup_service_has_a_cli_handler — every SUPPORTED_SETUP_SERVICES entry has a _HANDLERS entry in app/integrations/cli.py
test_every_cli_handler_is_registered_in_registry — reverse direction, no orphan handlers
test_every_verifier_has_a_verify_order — every spec with verifier=... has a verify_order so the positional CLI works

The two order tests are marked xfail(strict=False) because main currently has 9 pre-existing collisions (4 setup + 5 verify), which the renumber PR will clear. After that lands, the markers flip to strict (or are removed entirely) so the gate is permanent. The other three tests pass on main today.

No production code changes. tests/integrations/ is already routed by make test-scope, so this naturally gates any PR that touches app/integrations/.

Demo/Screenshot for feature changes and bug fixes -

Default CI run (xfail markers in effect, what this PR posts):

Screen.Recording.2026-06-08.at.02.35.35.mov

The collision payload names every offending service, so the renumber author has the full punch list inline.

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

No, I wrote all the code myself
Yes, I used AI assistance (continue below)

If you used AI assistance:

I have reviewed every single line of the AI-generated code
I can explain the purpose and logic of each function/component I added
I have tested edge cases and understand how the code handles them
I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

The registry in app/integrations/registry.py and the CLI handler dict in app/integrations/cli.py are two halves of the same contract. They have to stay in sync, but nothing enforces that. So when someone adds a new integration, they can pick a setup_order that collides with another service, or skip wiring a _HANDLERS entry, and the drift only surfaces when a user runs opensre integrations setup/verify <svc> and hits a Click rejection or non-deterministic ordering. That is how main ended up with 9 collisions today, including a jenkins/dagster collision that landed after muddlebee's audit in #opensre-integrations.

The tests are pure registry introspection. No integration is probed, no subprocess is spawned, no fixtures are loaded. Five invariants total, runs in under 2 seconds.

I considered putting the check in a standalone CI script (or a pre-commit hook) instead of a pytest file. Two reasons I went with tests: make test-scope already routes anything under tests/integrations/ to run on PRs that touch app/integrations/, so this naturally gates the right diffs without any new CI wiring; and the assertion messages name the offending services inline, which is friendlier than a script logging to stderr.

The two order-uniqueness tests use xfail(strict=False) rather than skip deliberately. skip would silently drop them. xfail(strict=False) keeps them collecting and visible in CI output, so when the renumber PR flips the registry clean, the tests become XPASS and the author flips the markers to strict (or removes them) in the same PR. That is the planned signal.

Helper: _duplicates(values) returns a {value: count} map of only the duplicated entries. Both order-uniqueness tests use it so the assertion output formats consistently across both axes.

Checklist before requesting a review

I have added proper PR title and linked to the issue
I have performed a self-review of my code
I can explain the purpose of every function, class, and logic block I added
I understand why my changes work and have tested them thoroughly
I have considered potential edge cases and how my code handles them
If it is a core feature, I have added thorough tests
My code follows the project's style guidelines and conventions

github-actions · 2026-06-08T09:38:33Z

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

greptile-apps · 2026-06-08T09:43:33Z

Greptile Summary

Adds tests/integrations/test_registry_invariants.py with five static invariants over the integration registry and CLI dispatch, and improves the assertion message in the existing test_every_setup_spec_has_handler test. No production code is changed.

Order-uniqueness tests are correctly gated with xfail(strict=False) to surface 9 pre-existing collisions without blocking CI.
Bidirectional handler/registry sync tests close the drift window where an orphan handler or a missing handler would silently affect _SETUP_SERVICES.
test_every_verifier_has_a_verify_order guards against verifiers added without an explicit verify_order.

Confidence Score: 5/5

Test-only change; no production code is touched and the new tests cannot break any existing behavior.

All five invariants are logically sound and the xfail strategy is appropriate. The only finding is a misleading assertion message with no runtime impact.

No files require special attention beyond the minor message fix in test_registry_invariants.py.

Important Files Changed

Filename	Overview
tests/integrations/test_registry_invariants.py	New file adding five registry self-consistency invariants; one assertion message inaccurately describes the consequence of a missing verify_order.
tests/integrations/test_registry.py	Improved assertion message in test_every_setup_spec_has_handler; no logic changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Registry["app/integrations/registry.py"]
        SPECS["INTEGRATION_SPECS"]
        SETUP["SUPPORTED_SETUP_SERVICES"]
        VERIFY["SUPPORTED_VERIFY_SERVICES"]
        SPECS --> SETUP
        SPECS --> VERIFY
    end
    subgraph CLI["app/integrations/cli.py"]
        HANDLERS["_HANDLERS dict"]
        SETUP_SERVICES["_SETUP_SERVICES (intersection)"]
        HANDLERS --> SETUP_SERVICES
        SETUP --> SETUP_SERVICES
    end
    subgraph Invariants["test_registry_invariants.py"]
        T1["test_setup_orders_are_unique (xfail)"]
        T2["test_verify_orders_are_unique (xfail)"]
        T3["test_every_cli_handler_is_registered_in_registry"]
        T4["test_every_verifier_has_a_verify_order"]
    end
    subgraph Existing["test_registry.py"]
        T5["test_every_setup_spec_has_handler"]
    end
    SPECS -->|setup_order uniqueness| T1
    SPECS -->|verify_order uniqueness| T2
    HANDLERS -->|orphan handler check| T3
    SETUP -->|orphan handler check| T3
    SPECS -->|verifier completeness| T4
    SETUP -->|forward direction| T5
    HANDLERS -->|forward direction| T5

_{Reviews (3): Last reviewed commit: "test(integrations): fold duplicate handl..." | Re-trigger Greptile}

Davidson3556 · 2026-06-08T10:05:43Z

@greptile review

kespineira (Tracer-Cloud#2773) flagged that test_every_setup_service_has_a_cli_handler asserts the same invariant as the existing test_every_setup_spec_has_handler added in Tracer-Cloud#2537. Both check that every SUPPORTED_SETUP_SERVICES entry has a matching _HANDLERS key. Fold the descriptive failure message (offending-service list) into the existing test in tests/integrations/test_registry.py and drop the duplicate from test_registry_invariants.py. Update the module docstring to point at the canonical forward-direction test. The reverse-direction (orphan handler) test in test_registry_invariants.py is retained — it covers an invariant the existing test does not check.

Davidson3556 · 2026-06-08T11:30:53Z

@greptile review

Add tests/integrations/test_registry_invariants.py with five static assertions over INTEGRATION_SPECS and the CLI _HANDLERS dict: - setup_order uniqueness across specs - verify_order uniqueness across specs - every setup-enabled service has a CLI handler (and vice versa) - every verifier has a verify_order (so positional CLI works) The two order tests are marked xfail(strict=False) because main has 9 collisions (jenkins+dagster being the latest, landed after the audit). The renumber PR flips both to strict / removes the markers. Refs Tracer-Cloud#2772

- _duplicates returns set[int]; count was never consumed by callers - Replace misleading "Click" wording in two assertion messages. app/integrations/cli.py uses a hand-rolled sys.argv parser plus questionary.select, not Click. New wording names the real failure modes: silent _SETUP_SERVICES exclusion and the cmd_setup "Usage: setup <service>" error.

kespineira (Tracer-Cloud#2773) flagged that test_every_setup_service_has_a_cli_handler asserts the same invariant as the existing test_every_setup_spec_has_handler added in Tracer-Cloud#2537. Both check that every SUPPORTED_SETUP_SERVICES entry has a matching _HANDLERS key. Fold the descriptive failure message (offending-service list) into the existing test in tests/integrations/test_registry.py and drop the duplicate from test_registry_invariants.py. Update the module docstring to point at the canonical forward-direction test. The reverse-direction (orphan handler) test in test_registry_invariants.py is retained — it covers an invariant the existing test does not check.

github-actions · 2026-06-10T05:07:26Z

🎯 Bullseye. @Davidson3556 opened a PR, kept the vibes clean, and got it merged. Absolute cinema. 🎬

👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

…ions (#2791) Pre-existing main carried 9 collisions surfaced by the registry contract tests (#2773): 4 setup_order ties and 5 verify_order ties. Renumber the later-added spec in each collision pair (preserving first-occurrence in source = original number) to a free slot at the high end of the range, with one fill-in at verify_order=6 (the only existing gap): setup_order tracer 12->25, telegram 19->26, whatsapp 19->27, mysql 20->28, dagster 24->29 verify_order victoria_logs 2->6, kafka 22->37, mysql 27->38, openclaw 28->39, dagster 36->40 The two xfail markers on test_setup_orders_are_unique and test_verify_orders_are_unique are now removed since both assertions pass on this state. Any future PR that reintroduces a collision will fail those tests strictly. Closes #2790

greptile-apps Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread tests/integrations/test_registry_invariants.py Outdated

Comment thread tests/integrations/test_registry_invariants.py

Comment thread tests/integrations/test_registry_invariants.py Outdated

Davidson3556 mentioned this pull request Jun 8, 2026

[BUG] benchmark dispatch test fails on fork PRs (missing ANTHROPIC_API_KEY) #2774

Closed

kespineira reviewed Jun 8, 2026

View reviewed changes

Comment thread tests/integrations/test_registry_invariants.py Outdated

Davidson3556 added 3 commits June 9, 2026 03:03

Davidson3556 force-pushed the test/registry-invariants branch from def7612 to 10c5f36 Compare June 9, 2026 10:11

Devesh36 merged commit b6016ef into Tracer-Cloud:main Jun 10, 2026
14 checks passed

This was referenced Jun 10, 2026

[IMPROVEMENT] Renumber INTEGRATION_SPECS to resolve 9 setup_order / verify_order collisions #2790

Closed

fix(integrations): renumber INTEGRATION_SPECS to resolve order collis… #2791

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(integrations): add registry self-consistency invariants#2773

test(integrations): add registry self-consistency invariants#2773
Devesh36 merged 3 commits into
Tracer-Cloud:mainfrom
Davidson3556:test/registry-invariants

Davidson3556 commented Jun 8, 2026

Uh oh!

github-actions Bot commented Jun 8, 2026

Uh oh!

greptile-apps Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Davidson3556 commented Jun 8, 2026

Uh oh!

Uh oh!

Davidson3556 commented Jun 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Davidson3556 commented Jun 8, 2026

Describe the changes you have made in this PR -

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Checklist before requesting a review

Uh oh!

github-actions Bot commented Jun 8, 2026

Greptile code review

Uh oh!

greptile-apps Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Davidson3556 commented Jun 8, 2026

Uh oh!

Uh oh!

Davidson3556 commented Jun 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Jun 8, 2026 •

edited

Loading