Declarative API-key tester and migration

Task 6013 · Done · Coder · created · updated

## Original requirement (verbatim, epic body) > Add to Credential types: > > - Apify > - Zappier > - Make > - N8n - just check that it has a default option to connect to their main cloud > - Gumloop > - IFTTT > - Replit > - Lovable > - Heroku > > Make sure you reuse code, e.g. generic Oauth or something else generic. Improve the base connection. Each connector's code should be as slim as possible. This child covers asks A4 ("N8n - just check that it has a default option to connect to their main cloud") and A10–A12 ("reuse code… Improve the base connection… as slim as possible") for EXISTING types. It also builds the generic capability that the new API-key vendors in later children use. It adds NO new credential type. ## Settled requirements this child builds (PRD, verbatim) > **R-B. n8n default cloud option (A4)** — A new n8n credential's Base URL is pre-filled with `https://YOUR-NAME.app.n8n.cloud`; editing it to the real instance (cloud or self-hosted) works exactly as today. > > **R-C.1 Declarative API-key connector**: a type whose test is "one GET with the key in one header" is fully described by data in its registry entry (header name, value prefix, test path, optional Accept / extra headers, optional identity JSON path for the success sentence). The generic machinery performs the test; no per-type tester function exists for it. > > **R-C.2 Existing types migrated**: every existing tester that is exactly that shape … moves onto R-C.1 and its function is deleted. Their outcomes (status, message text) are byte-identical to today. > > **R-C.5** The existing structural guards (testable ⇔ tester, untestable ⇔ reason, connect ⇔ fields) keep holding — extended to declarative testers. ## Approach (Architecture §2.1, §2.5 n8n row, §2.6 — pinned contract, build to these exact names) 1. In `apps/api/app/credentials/registry.py`, add the frozen dataclass `KeyTestSpec`. It is pure data with no HTTP import, and has exactly these fields and defaults: - `path: str` - `key_field: str` - `header_name: str = "Authorization"` - `value_prefix: str = ""` - `accept: Optional[str] = "application/json"` (None = not shape-gated) - `extra_headers: tuple[tuple[str,str],...] = ()` - `field_headers: tuple[tuple[str,str],...] = ()` — pairs of (header name, config key); a header is sent only when its field is non-blank - `identity: tuple[tuple[str|int,...],...] = ()` - `identity_phrase: str = "as"` - `list_field: Optional[str] = None` - `blank_key_reason: Optional[str] = None` — for an optional key_field: a blank key returns `unsupported` with this sentence and sends no request - `reject_names_vendor: bool = False` Add `key_test: Optional[KeyTestSpec] = None` to `CredentialTypeSpec`. 2. In `apps/api/app/credentials/testers.py`, add `_declared_tester(slug) -> Tester`. It applies these verdict rules: 1. Call `_prepare(slug, config)`. A failure returns as today (`_field_problem` first). 2. If `blank_key_reason` is set and the key is blank, return `TestResult(ok=False, status="unsupported", message=blank_key_reason)` and send no request. 3. Call `_api_key_request(spec, dest, deadline, path=…, key=…, header_name=…, value_prefix=…, accept=…, extra_headers=dict(extra_headers) | {h: str(config[k]) for (h,k) in field_headers if not blank}, rejected_message=(f"{spec.label} rejected the {key field label} - check it." if reject_names_vendor else None))`, through the existing `_checked_request` `rejected_message` seam. 4. On 2xx: - `accept is None` → `_success(f"Connected to {spec.label} - {key field label} accepted.")` - `list_field` set → `_gated_body`; a non-list at `list_field` → `_unexpected_body`; otherwise `_credentials_accepted(spec)` - otherwise → `_named(spec, dest, outcome, *identity, phrase=identity_phrase)` 3. Rename the hand-written map to `_BESPOKE_TESTERS`. Build `TESTERS = {**_BESPOKE_TESTERS, **{s: _declared_tester(s) for s, t in CREDENTIAL_TYPES.items() if t.key_test}}`. Its name and shape do not change for consumers (`git_probe.py:24`, the route, tests). 4. Move the import guards into `_check_tester_registry(types, bespoke)`. It runs at import over the real registry, tests can call it on a synthetic dict, and it raises `RuntimeError`, not `assert`. - G1: `key_test` and a bespoke tester are mutually exclusive. - G2: testable ⇔ (bespoke or `key_test`). - G3: `path` starts with `/` and has no whitespace; `key_field` names a declared `secret` field; each `field_headers` key names a declared field. - G4: header names pass `_header_name_clause`; `value_prefix` passes `_value_prefix_clause`. - G5: `blank_key_reason` is set ⇔ the key_field FieldSpec is optional. - G6: `list_field` and `identity` are not both set. Keep the existing untestable ⇔ reason and connect ⇔ fields guards. 5. Migrate exactly these 8 types (Architecture §2.6) and DELETE their functions: - `n8n`: `KT(path="/api/v1/workflows?limit=1", key_field="api_key", header_name="X-N8N-API-KEY", accept=None)` (was `testers.py:792`) - `clockify`: `KT(path="/api/v1/user", key_field="api_key", header_name="X-Api-Key", identity=(("name",),("email",)))` (`:935`) - `hubspot`: `KT(path="/account-info/v3/details", key_field="access_token", value_prefix="Bearer", identity=(("portalId",),), identity_phrase="- portal")` (`:950`) - `gitlab`: `KT(path="/api/v4/user", key_field="token", header_name="PRIVATE-TOKEN", identity=(("username",),))` (`:1154`) - `anthropic_api_key`: `KT(path="/v1/models?limit=1", key_field="api_key", header_name="x-api-key", extra_headers=(("anthropic-version","2023-06-01"),), list_field="data")` (`:1446`) - `deepseek_api_key`: `KT(path="/models", key_field="api_key", value_prefix="Bearer", list_field="data")` (`:1456`) - `openai_api_key`: `KT(path="/v1/models", key_field="api_key", value_prefix="Bearer", list_field="data")` (`:1462`) - `gemini_api_key`: `KT(path="/v1beta/models", key_field="api_key", header_name="x-goog-api-key", list_field="models")` (`:1468`) Then: - Delete `_model_list` and `_ANTHROPIC_VERSION`. The version literal moves into the registry entry with its comment. - Delete `_api_key_call` if no bespoke tester still uses it. - Update the `_api_key_request` docstring to name the declarative path. - If a type's real behaviour is NOT exactly the single-GET shape the spec can express byte-identically, leave it bespoke and say so in the PR body. Never change its message to make it fit. 6. n8n pre-fill: `FieldSpec("base_url", "Base URL", "url", True, default="https://YOUR-NAME.app.n8n.cloud")`. The placeholder stays None, and the label "N8N" and the slug do not change. `default` pre-fills only a NEW row (`registry.py:28-30`, served by `FieldSpecOut.default`). 7. Narrow `apps/api/tests/test_oauth2_connect_types.py::_TESTABLE_CONNECTABLE` to `spec.oauth_connect is not None and slug in testers._BESPOKE_TESTERS`. A later declarative connect type (Heroku) then stays out of the arms that assert `_refresh_exchange` semantics. The non-vacuous assert at `:188` keeps holding through `google`. 8. Tests: - AC10 guard tests that call `_check_tester_registry` on synthetic dicts: empty path, relative path, testable with neither tester nor spec, key_test and bespoke both set, blank_key_reason on a required key. - A `_declared_tester` unit test on a synthetic spec that exercises `field_headers` (sent only when non-blank), `blank_key_reason` (`unsupported`, no request) and `reject_names_vendor` (401 → "{label} rejected the {key label} - check it."). This proves here the capabilities that later children rely on. 9. Existing registry-wide tests this child MUST update (found at the critic gate; `./dc check full` breaks without them): - `apps/api/tests/test_credentials.py:~2022` `_EXPECTED_TYPES`: change n8n's `base_url` `"default": None` to `"https://YOUR-NAME.app.n8n.cloud"`. This is the AC7 unit oracle, and editing this structural list is allowed. - `tools/tests/test_vendor_name_census.py:~339` `PINNED_TEXTS`: migrating Gemini removes every `gemini` string from `testers.py`, so delete that file's entry. Otherwise `test_every_allowlist_entry_still_points_at_a_file_that_needs_it` fails. - Keep the guard wording: the `RuntimeError` messages from `_check_tester_registry` keep the word "disagree" and name the offending slug. `test_the_structural_guard_actually_fires_at_import` (`test_credentials.py:~6083`) and the reason-drift test (`:~6035`) must pass unmodified. ## Acceptance criteria (PRD §6, verbatim — the ones this child owns) > AC7 — n8n: a NEW n8n row's Base URL is pre-filled `https://YOUR-NAME.app.n8n.cloud`; existing n8n rows are unchanged; Test against a real instance still works. > > AC8 — Declarative connector: no new type in R-A that is a single-GET key check has a tester function; migrated existing types have no tester function left; their test outcomes and messages are unchanged. (This child owns the migrated half.) > > AC10 — Guards still enforce consistency: a declarative type with a missing test path, or a testable type with neither tester nor declarative spec, fails at import. Quality bar (beyond the literal AC): the change must DELETE code, not add a parallel path. It FAILS if any of the 8 functions survives, if a `TESTERS` consumer had to change, or if a migrated message drifts by even one character. The n8n pre-fill must read as an obvious template for the operator to replace (`YOUR-NAME` does), not as a value that looks real. Credentials & reach: (Architecture §7, verbatim rows this child's oracles need) - Dev UI: AC7 (and AC11) — Credential `jaah/debug/main/operator-login` (operator login, works on dev) against `https://app.dev.jaah.ai` — **verified**: the roster fetch succeeded, and the dev console answered 200 from h2. - AC3/AC7 real green for migrated n8n — `gmind/system/n8n` — **verified**: fetch succeeded, and its instance host answered 200 from h2. - AC3 real green for migrated Clockify — `gmind/finance/clockify` — **verified**: fetch succeeded; `api.clockify.me` answered (401 to an unauthenticated probe). - AC3 real green for migrated Anthropic — `jaah/system/anthropic_api_key` — **verified**: fetch succeeded; `api.anthropic.com` answered (401 unauthenticated). - AC8/AC9/AC10 — worktree only — **verified**: nothing beyond the worktree. Out of scope: (PRD §7 + Architecture fences, verbatim where quoted) - "Do NOT change existing types' slugs, labels, fields, or test messages (except n8n's Base URL default, R-B). Existing saved rows of any type must open, test and decrypt exactly as before — do not change any `default_base_url` of an existing type (it is baked into sealed-secret AADs)." - "Do NOT add a per-type web component or touch the credential form beyond what generic rendering already does; no new field kind." — no file under `apps/web` changes. - "Do NOT add a DB migration." — nothing under `apps/api/migrations`. - "Do NOT refactor multi-call/bespoke testers (Jira, Xero, GitHub, Slack, AWS, Stripe, Google, Bitbucket, Azure DevOps)" — Headscale, GitHub App and generic_api_key stay bespoke too. - "Do NOT touch the generic types' semantics (`oauth2_client`, `generic_login`, `generic_api_key`) beyond moving shared code underneath them." - "Do NOT rename `N8N` → `n8n` label (not asked)." - Do NOT add any of the 8 new vendor types (apify, gumloop, heroku, ifttt, lovable, make, replit, zapier); sibling children own them. Do NOT add `require_one_of` or `allowed_hosts`; a sibling child owns them. - Do NOT edit any expected test-OUTCOME message (status or message sentence) in `apps/api/tests/test_credentials.py` (n8n `:3283`/`:5112`, Clockify/HubSpot `:4551-4559`, model lists `:4724`). They must pass unmodified (risk K1). Updating the structural `_EXPECTED_TYPES` n8n default (Approach 9) is allowed and required. Done means: - R-C.1/R-C.2 byte-identical — `./dc check full` is green, and the existing n8n/Clockify/HubSpot/GitLab/model-list message assertions in `apps/api/tests/test_credentials.py` are UNMODIFIED: `git diff origin/develop -- apps/api/tests/test_credentials.py` shows no changed expected-message line. - AC8 (migrated half) — `grep -n "def test_" apps/api/app/credentials/testers.py` shows none of n8n, clockify, hubspot, gitlab, anthropic_api_key, deepseek_api_key, openai_api_key, gemini_api_key. `_model_list` and `_ANTHROPIC_VERSION` are gone. - AC10 — a unit test calls `_check_tester_registry` on a synthetic spec with an empty or relative `path`, and on a testable type with neither tester nor spec, and asserts `RuntimeError` for each. It passes in `./dc check full`. - Declarative capabilities — unit tests on a synthetic spec prove `field_headers` (header sent only when the field is non-blank), `blank_key_reason` (`unsupported`, zero requests) and `reject_names_vendor` (401 → vendor-naming sentence). - AC7 — after this PR's Jaah Deploy succeeds on dev: - Credentials → New → N8N shows Base URL pre-filled with `https://YOUR-NAME.app.n8n.cloud`. - An existing n8n row still shows its own URL. - Test on the `gmind/system/n8n` row (or a row built from that credential) goes green with "Connected to N8N - API key accepted." - AC3 regression on migrated types — on dev, Test on a Clockify row (`gmind/finance/clockify`) and on an Anthropic API key row (`jaah/system/anthropic_api_key`) each goes green with the same sentence as before the change. - AC9 — the PR diff touches nothing under `apps/web` or `apps/api/migrations`.

Outcome

Declarative KeyTestSpec tester; 8 types migrated with their tester functions deleted and messages unchanged; n8n Base URL pre-filled. PR #5236 merged, gate and dev deploy green, dev UI tests green. Deferred: 4 credential-form UX fixes (in report).

Pull request

Artifacts

Activity

  • Draft
  • Ready
  • Runner started
  • Runner ended (done)