feat(integrations): migrate Groundcover, GitLab, Sentry, PostHog and Vercel onto the shared setup flow#4194
Conversation
…Vercel onto the shared setup flow Second batch of #4168, and the first that needed no new spec machinery — everything these five required landed with #4191. Two more wizard configurators were dropping credentials on the floor: _configure_vercel called sync_env_values({}) so neither the token nor the team id reached any tier but the store, and _configure_sentry wrote the URL, org and project slugs but never SENTRY_AUTH_TOKEN to the keyring. That is five such bugs across two batches; hand-written per-vendor persistence was wrong more often than it was right. Both are fixed by removing the persistence code rather than correcting it. GitLab was worse: _setup_gitlab prompted for a URL and a token, called upsert_integration, and stopped — no check for a blank token and no probe at all, so a typo produced a stored integration that failed on first use. Three of the five name their env vars differently from the credential they carry (auth_token as GITLAB_ACCESS_TOKEN, organization_slug as SENTRY_ORG_SLUG, base_url as SENTRY_URL), which is the mistake the round-trip test exists to catch; it now covers all nine migrated integrations. The readers — _catalog_impl, integrations/gitlab and integrations/posthog/config — import the same constants the specs write, so the two sides cannot drift. config/constants/sentry.py and posthog.py already existed but hold OpenSRE's *own* telemetry config: the DSN it reports its crashes to and the write-only key for its analytics project. Those are unrelated to what a user supplies to query their own Sentry or PostHog, so both docstrings now say so and the integration names sit under a separate section. Deletes the four wizard validators, whose files contained nothing else. Three were literally build_X_config + validate_X_config, which is what register_validation_verifier already wires up; Vercel's duplicated client.probe_access(). Vercel's probe failure path turned out to be untested, so tests/integrations/vercel/test_client.py gains that coverage rather than losing it with the validator.
|
@greptile review |
Greptile code reviewThis 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: 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 SummaryThis PR migrates five integrations — Groundcover, GitLab, Sentry, PostHog, and Vercel — onto the shared
Confidence Score: 5/5Safe to merge — the change is a clean structural migration with no new logic, and the two credential-persistence bugs it fixes are clearly demonstrated by the extended round-trip test suite. Every changed surface is a direct mechanical substitution: string literals replaced with constants, hand-written persistence loops replaced with apply_setup, and thin shim files deleted. The round-trip tests now cover all nine migrated integrations, and the new Vercel client tests close the one gap left when the validator shims were removed. No branching logic was added, no auth boundary was moved, and the env-var routing decisions (keyring vs .env) follow the existing is_sensitive_env_key convention without any exceptions. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User: opensre integrations setup service] --> B[CLI handler: _setup_service]
B --> C[_run_spec_setup / configure_from_spec]
C --> D[IntegrationSetupSpec]
D --> E{apply_setup}
E --> F[_collect_credentials
validates required fields]
F -->|missing required field| G[Exit with error
nothing persisted]
F -->|all fields present| H[_verify
calls spec.verify]
H -->|status != passed| I[Retry prompt
nothing persisted]
H -->|passed| J[_persist_env
keyring for _TOKEN/_KEY
.env for everything else]
J --> K[upsert_integration
integration store]
K --> L[Setup complete]
subgraph New setup.py specs
M[gitlab/setup.py GITLAB_SETUP]
N[groundcover/setup.py GROUNDCOVER_SETUP]
O[sentry/setup.py SENTRY_SETUP]
P[posthog/setup.py POSTHOG_SETUP]
Q[vercel/setup.py VERCEL_SETUP]
end
D -.->|one of| M
D -.->|one of| N
D -.->|one of| O
D -.->|one of| P
D -.->|one of| Q
Reviews (3): Last reviewed commit: "style(config): annotate the new env-name..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR migrates five more integrations — Groundcover, GitLab, Sentry, PostHog, and Vercel — onto the
Confidence Score: 5/5Safe to merge. All five integrations now follow the same write path; the bug fixes are structurally sound and fully covered by the extended round-trip and wizard-flow test suites. Every stated bug fix is verified end-to-end: the env round-trip test confirms each credential reaches the correct tier, and the wizard flow tests confirm no credentials are saved when verification fails. The constants files are internally consistent with the batch-1 pattern. The deleted validator wrappers had no logic that is not now covered by register_validation_verifier / register_probe_verifier. No behavioral regressions were identified. No files require special attention. The only things worth a second read are the intentional credential/env-var name mismatches (auth_token → GITLAB_ACCESS_TOKEN, base_url → SENTRY_URL) — these are documented in both the constants modules and the PR description and are verified by the round-trip suite. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI/Wizard
participant configure_from_spec
participant apply_setup
participant Verifier
participant Keyring
participant DotEnv
participant Store
User->>CLI/Wizard: opensre integrations setup service
CLI/Wizard->>configure_from_spec: "configure_from_spec(SETUP_SPEC, title=...)"
configure_from_spec->>configure_from_spec: prompt for each field
configure_from_spec->>apply_setup: apply_setup(spec, values)
apply_setup->>apply_setup: _collect_credentials() validate required fields
alt missing required field
apply_setup-->>configure_from_spec: "SetupOutcome(ok=False)"
configure_from_spec->>User: re-prompt
end
apply_setup->>Verifier: spec.verify(setup, credentials)
Verifier-->>apply_setup: status passed or failed
alt verification failed
apply_setup-->>configure_from_spec: "SetupOutcome(ok=False)"
configure_from_spec->>User: re-prompt
end
apply_setup->>Keyring: sync_env_secret(TOKEN_ENV, value)
apply_setup->>DotEnv: sync_env_values non-secret envs
apply_setup->>Store: upsert_integration(service, credentials)
apply_setup-->>configure_from_spec: "SetupOutcome(ok=True, env_path=...)"
configure_from_spec-->>User: display title and env path
Reviews (2): Last reviewed commit: "feat(integrations): migrate Groundcover,..." | Re-trigger Greptile |
config/constants/sentry.py and posthog.py annotate every constant they hold, including DEFAULT_SENTRY_BASE_URL added in the same change; the new *_ENV names did not, which reads as though they are variables. The standalone per-vendor env modules (telegram, datadog, gitlab, …) use no Final and stay as they are — they are consistent with each other and with the telegram precedent. Only the two files that mix styles change.
|
@greptile review |
|
💜 One more reason the project grows. Thanks @muddlebee — your contribution just landed!
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
Keep both migration batches' constants, CLI setup cases, and env round-trip coverage. Drop wizard health validators deleted on either side (gitlab/sentry/posthog/vercel and dagster/pagerduty/incident_io).
1 participant
Part of #4168. Follows #4191 (Datadog, Honeycomb, Coralogix).
Describe the changes you have made in this PR -
Second migration batch onto the shared setup flow: Groundcover, GitLab, Sentry, PostHog, Vercel. No new spec machinery — everything these needed already landed in #4191, which is the point of batching this way.
Two more wizard configurators were silently dropping credentials
_configure_vercelsync_env_values({})— neitherVERCEL_API_TOKENnorVERCEL_TEAM_IDreached any tier but the store_configure_sentrySENTRY_URL,SENTRY_ORG_SLUGandSENTRY_PROJECT_SLUG, but neverSENTRY_AUTH_TOKENto the keyringThat is five such bugs across two batches (Datadog, Honeycomb, Coralogix, Sentry, Vercel). The pattern is consistent enough to be worth stating plainly: hand-written per-vendor persistence was wrong more often than it was right. Both are fixed structurally rather than patched — the configurators no longer contain persistence code at all.
GitLab had no validation and no verification
_setup_gitlabprompted for a URL and a token and calledupsert_integration— no_dieon a blank token, no probe. A typo'd or empty token produced a stored integration that failed on first use. It now verifies before it persists, like every other spec-backed integration.Three of the five have env var names that do not match the credential
auth_token(GitLab)GITLAB_ACCESS_TOKENorganization_slug(Sentry)SENTRY_ORG_SLUGbase_url(Sentry)SENTRY_URLThis is exactly the class of mistake
tests/integrations/test_setup_spec_env_round_trip.pyexists to catch, and it now covers all nine migrated integrations. I mutation-tested it again on the worst offender — pointing GitLab's spec at the plausible-but-wrongGITLAB_AUTH_TOKENfails the suite:The readers are single-sourced against the same constants the specs write (
integrations/_catalog_impl.py,integrations/gitlab/__init__.py,integrations/posthog/config.py), so the two sides cannot drift for these vendors.Sentry and PostHog constants needed care
config/constants/sentry.pyandposthog.pyalready existed, but they hold OpenSRE's own telemetry config — the DSN it reports its own crashes to, and the write-only key for its own analytics project. Those are unrelated to the credentials a user supplies to query their Sentry or PostHog. Both module docstrings now say so explicitly, and the integration names sit under a separate marked section.Deduplication
validate_gitlab_integration,validate_sentry_integration,validate_posthog_integrationandvalidate_vercel_integrationare deleted — all four files contained nothing else. The first three were literallybuild_X_config+validate_X_config, which is exactly whatregister_validation_verifieralready wires up; the Vercel one duplicatedclient.probe_access().Before deleting the Vercel tests I checked what they actually covered and found
probe_access's failure path untested, sotests/integrations/vercel/test_client.pygains two tests (API error reported, blank token reportsmissingnotfailed) rather than losing that coverage.Demo/Screenshot for feature changes and bug fixes -
GitLab verify-before-persist, sandboxed
HOME, default base URL accepted, bad token:Before this change GitLab did not verify at all — that token would have been stored.
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Same problem as #4191 — three surfaces configured integrations and disagreed about where credentials land, so whether an integration blocked
make deploydepended on which command you used. This batch moves five more onto the one flow.I picked these five because they are all "N fields plus a registered verifier" with no branching prompts, which meant the batch needed no new spec features and stays reviewable. Grafana is still deferred: its wizard path has a
_confirmboolean (verify_ssl) and aca_bundlefield that is only asked when that boolean is true, andSetupFieldmodels neither a boolean nor a conditional field. That is a design decision I would rather make against Grafana and the databases together than smuggle into a batch PR.Key pieces:
integrations/<vendor>/setup.py× 5 — declarative field lists; no logic.config/constants/<vendor>.py— env names, imported by both the writer (spec) and the reader (load_env_integrationsand friends), which is what makes the two sides unable to drift.configure_from_spec.Edge cases covered by the extended parametrized suites: blank required field fails on that field rather than after every prompt; defaulted fields (
GITLAB_BASE_URL,SENTRY_URL,POSTHOG_BASE_URL,GROUNDCOVER_MCP_URL,GROUNDCOVER_TIMEZONE) accept a bare enter; optional fields (VERCEL_TEAM_ID,SENTRY_PROJECT_SLUG, Groundcover's tenant/backend) may be left blank and clear every tier when blanked; failed verification persists nothing anywhere.One deliberate non-change: Groundcover's key resolves from
GROUNDCOVER_API_KEYorGROUNDCOVER_MCP_TOKEN. Setup writes the primary name only, so there is one place to look; the fallback still resolves for anyone who already set it.Checklist before requesting a review