fix(source-zendesk-chat): name Zendesk Chat and the access token in 401 errors - #83336
fix(source-zendesk-chat): name Zendesk Chat and the access token in 401 errors#83336devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
Co-Authored-By: bot_apk <apk@cognition.ai>
|
Note Autopilot progressive rollouts are not enabled for the following modified connector(s):
This is a courtesy heads-up only — it does not block merge or fail any check. |
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
|
↪️ Triggering Reason: Draft |
|
🧪 Fix Validation Evidence🟢 Outcome: Fix ProvenReproduced the exact 401 this PR targets by running 🚦 Next Steps
📋 Connector & PR DetailsConnector: 📝 Evidence PlanHypothesisMessage-only change. The added 401 Proving criteria
Disproving criteriaOld generic text persists; or the 401 is retried/ignored rather than failing; or any stream loses records, changes schema, or newly errors versus control. Testing strategy decisionRegression tests + fault injection. No live customer connection pinning, and no candidate connections were selected — a healthy connection cannot produce a 401, so pinning could not have exercised criterion 1 and would have added customer risk for zero signal. Criterion 2 is exactly what comparison regression tests measure; this connector is a source with GSM integration-test credentials and is not on the forced-OAuth-write-back list (static Cases attempted
✅ Pre-flight Checks
Design intent: not a concern. The generic 401 text was a CDK default rather than a deliberate 📊 Detailed Evidence LogCase 1 — 401 fault injection (real Zendesk 401, invalid token,
|
| Command | Result |
|---|---|
| SPEC | ✅ no difference |
| CHECK | ✅ no difference (SUCCEEDED both sides) |
| DISCOVER | ✅ no difference |
| READ | ✅ no difference |
READ totals identical on both sides: 745 RECORD, 16 STATE, 36 TRACE, 75 LOG. Per-stream counts identical, all >0, so the shared error handler was exercised on every stream rather than only the check probe:
accounts 1 · agent_timeline 419 · agents 3 · bans 124 · chats 127 · departments 42 · goals 5 · roles 3 · routing_settings 1 · shortcuts 4 · skills 4 · triggers 12
Note on the pre-release publish
The first pre-release publish failed on a transient GitHub API 429 (https://github.com/airbytehq/airbyte/actions/runs/30905568068), unrelated to this PR; re-triggered as https://github.com/airbytehq/airbyte/actions/runs/30906111582. Neither piece of evidence above depended on that image.
No customer connections were involved in this validation, so there are no connection or workspace identifiers to redact.
|
|
|
↪️ Triggering Reason: |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review Report🟢 Review Action: APPROVED
All other 11 gates passed.
|
| Gate | Status | Enforced? | Details |
|---|---|---|---|
| PR Hygiene | PASS | Yes | Description is substantive (~3.9k chars of real content, not template); docs/integrations/sources/zendesk-chat.md gains a 1.3.22 changelog row; zero human review comments (all 12 PR comments are from bots). |
| Code Hygiene | PASS | WARNING | Source (manifest.yaml) changed and a test file changed in the same PR (unit_tests/mock_server/test_routing_settings.py). |
| Test Coverage | PASS | Yes | Behavioral change (title starts fix(...)); new test content added: def test_401_error_has_connector_specific_message with a new assert on the AirbyteConnectionStatus message. |
| Code Security | PASS | Yes | No changed file matches the auth/credential/token path patterns; diff hunks in manifest.yaml contain none of authenticator, access_token, client_secret, api_key, etc. (the new prose string "access token" is not the access_token keyword), and the metadata.yaml hunk is only dockerImageTag: 1.3.21 → 1.3.22. |
| Per-Record Performance | PASS | WARNING | No record-processing code touched; the manifest change is confined to error_handler wiring, which runs on error responses only. |
| Breaking Dependencies | PASS | WARNING | No pyproject.toml, poetry.lock, or gradle dependency changes; base image unchanged. |
| Backwards Compatibility | PASS | Yes (enforced) | No spec file, no schemas/*.json, no stream add/remove/rename, no primary-key/cursor field or value change, no type/format change. metadata.yaml change is a patch dockerImageTag bump only, so no releases.breakingChanges entry is required. |
| Forwards Compatibility | PASS | Warning (elevates Risk Level) | No state/cursor/checkpoint/partition-router/paginator/transformation keyword appears in any added or removed line (occurrences of record_selector in the file are context lines outside the hunks' +/- content). No partition-key shape change. |
| Behavioral Changes | FAIL | Warning (elevates Risk Level) | Operational-risk keywords present in diff hunks of manifest.yaml: error_handler, response_filters, HttpResponseFilter, http_codes, backoff_strategies, Retry-After. Mitigations on record: the removed inline handlers and the new shared definitions.error_handler are identical for 404 IGNORE and Retry-After backoff; the only semantic addition is a 401 → FAIL/config_error filter with a connector-specific message; and /ai-prove-fix comparison regression tests vs. control 1.3.21 were identical on SPEC/CHECK/DISCOVER/READ with matching record counts across all 12 streams. |
| Out-of-Scope Changes | PASS | Skip | All 4 changed files are in scope: 3 under airbyte-integrations/connectors/source-zendesk-chat/** and 1 under docs/. |
| CI Checks | PASS | Yes | Core checks on this SHA are green, including the re-run triggered when the PR was marked ready: Test source-zendesk-chat Connector, Lint source-zendesk-chat Connector, Build and Verify Artifacts (source-zendesk-chat), Format Check, Check Changelog Updated, Connector CI Checks Summary. Connector test results: 46 tests, 42 ✅ / 4 💤 / 0 ❌. Pre-release and rollout check-runs are excluded from this gate by design. |
| Live / E2E Tests | PASS | Yes | Validation is required (bug fix + API error-handling change). /ai-prove-fix posted 🟢 Fix Proven on this HEAD (comment 5178445154): a real Zendesk 401 reproduced the old CDK message on the master manifest and the new Zendesk-specific message on this PR's, with status staying FAILED/config_error, plus clean comparison regression tests against control 1.3.21. This gate maps that conclusion directly. |
Changed-line evidence for the keyword gates — added/removed lines were extracted from the PR diff (context lines excluded, comment lines excluded) before matching:
- Security keyword matches: none
- Forwards-compatibility keyword matches: none
- Behavioral-change keyword matches:
error_handler,response_filters,HttpResponseFilter,http_codes,backoff_strategies,Retry-After(all withinmanifest.yaml)
Spec comparison: not applicable — no spec*.json/spec*.yaml file and no spec section in manifest.yaml was modified.
📚 Evidence Consulted
Evidence
- Changed files: 4
airbyte-integrations/connectors/source-zendesk-chat/manifest.yaml(+26 / −96)airbyte-integrations/connectors/source-zendesk-chat/metadata.yaml(+1 / −1)airbyte-integrations/connectors/source-zendesk-chat/unit_tests/mock_server/test_routing_settings.py(+21 / −0)docs/integrations/sources/zendesk-chat.md(+1 / −0)
- CI checks: all core checks
successona05ae82; no failed, cancelled, or timed-out check-runs at time of review - PR labels:
connectors/source/zendesk-chat,hyd-prove,hyd-review - PR description: present and substantive
- Human reviews / review comments: none (0 reviews, 0 inline comments)
- Existing bot reviews for this SHA: none
/ai-prove-fixconclusion: 🟢 Fix Proven (fix(source-zendesk-chat): name Zendesk Chat and the access token in 401 errors #83336 (comment))
❓ How to Respond
Resolving a Failing Gate
Behavioral Changes is a warning-level gate: it surfaces here and raises the Risk Level, but it does not block approval and requires no remediation from you. The keyword match is expected for a PR that intentionally edits error-handling configuration.
If you believe a gate verdict is a false positive or cannot be remediated as written, request review from a human maintainer and explain the situation in a PR comment — a human reviewer can approve or merge over a bot verdict. Written explanations in the PR description do not change a gate verdict.
|
↪️ Triggering Reason: |
Auto-merge evaluation: FAILPreconditions (all must pass)
Change scope (at least one must pass)❌ No matching change scope detected. |
|
🙋 Escalated to #human-in-the-loop per Hands-Free AI Triage Project triage next step. Reason: |
What
When Zendesk rejects the credentials,
source-zendesk-chat'schecksurfaced the CDK's generic default 401 text:It names no vendor and no credential, and "Please ensure you are authenticated correctly." is exactly the vague remediation the error-message guidelines prohibit. Message-only fix; it now reads:
Related to https://github.com/airbytehq/oncall/issues/13227:
Requested by the
/ai-fixrun on that oncall issue.How
All 12 stream requesters in
manifest.yamlcarried an identical inlineDefaultErrorHandler(404IGNORE+Retry-Afterbackoff). Instead of copying a new filter into 12 places, the handler is now a singledefinitions.error_handlerthat each requester$refs, with a 401 filter added:404
IGNOREand theRetry-Afterbackoff are unchanged for every stream.Declarative-First Evaluation
Declarative only: the built-in
HttpResponseFiltersupports botherror_messageandfailure_type, so no custom Python component was needed. Prior art:source-pinterest/manifest.yaml.Test Coverage
New mock-server test
test_401_error_has_connector_specific_messagemocks a 401 on the check-probe endpoint and asserts the resultingAirbyteConnectionStatusmessage. Confirmed it fails against the parent commit's manifest (yielding the old CDK string) and passes here; full connector suite: 39 passed.Additionally validated by the
/ai-prove-fixrun on this PR: a real 401 from Zendesk (invalid access token, shipped base imagesource-declarative-manifest:7.23.8) reproduced the old message on themastermanifest and the new one here, withstatusstayingFAILED/config_error; comparison regression tests against control1.3.21were identical on SPEC/CHECK/DISCOVER/READ with matching record counts across all 12 streams. Evidence is in the prove-fix comment below.Known limitation (deliberately out of scope)
The
Stream routing_settings is not available:prefix still leaks the internal check-probe stream name. That prefix comes fromCheckStream._check_stream_availabilityinairbyte-python-cdk, and the generic 401 text it wrapped comes from that repo'sdefault_error_mapping.py— both are cross-cutting changes affecting every declarative connector and are better proposed separately. OAuth refresh-token support (the deeper cause of these 401s, since the connector uses a staticBearerAuthenticator) is tracked in https://github.com/airbytehq/oncall/issues/11261 with draft PR #73316; it is a breaking change, so this PR stays message-only.Review guide
airbyte-integrations/connectors/source-zendesk-chat/manifest.yamlairbyte-integrations/connectors/source-zendesk-chat/unit_tests/mock_server/test_routing_settings.pyUser Impact
Users whose Zendesk Chat access token is invalid, expired, or under-scoped now see a message naming the vendor and the credential instead of generic "Unauthorized" text. No schema, spec, state, or behavior change — non-breaking patch bump 1.3.21 → 1.3.22.
Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/8aa24cc2791a4bba882b5f8ec284dc19