Skip to content

fix: treat None-valued conditions as false in InterpolatedBoolean - #1152

Merged
Patrick Nilan (pnilan) merged 1 commit into
mainfrom
devin/1789406745-interpolated-boolean-none
Sep 14, 2026
Merged

Patrick Nilan (pnilan) merged 1 commit into
mainfrom
devin/1789406745-interpolated-boolean-none

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

👉 TL;DR: Declarative condition expressions that evaluate to "nothing" (a missing config key, an explicit null, or an and/or chain that short-circuits to nothing) were being treated as true. They now evaluate to false, matching how Jinja's own {% if %} and Python truthiness behave.

Specifically, None is added to InterpolatedBoolean.FALSE_VALUES so InterpolatedBoolean.eval returns False when JinjaInterpolation.eval yields None.

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/17241:

Related to https://github.com/airbytehq/airbyte-internal-issues/issues/17212 (source-linear OAuth regression caused by this behavior; connector-side workaround in airbytehq/airbyte#85789).

Changes

  • airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py: add None to FALSE_VALUES.
  • unit_tests/sources/declarative/interpolation/test_interpolated_boolean.py: add cases for {{ none }}, .get() on a missing key, a key present with null, and/or chains that short-circuit to None (all False), and an or chain that falls through to a truthy value (True). The five False cases fail on main and pass with this change.

Why

JinjaInterpolation.eval renders the template to a string and ast.literal_evals it, so {{ config.get('api_key') }} renders "None" and comes back as Python None. None was not in FALSE_VALUES and None == False is False, so eval fell through to return True. Reproduced on main @ 95439d6:

{{ none }}                                        {}                 -> True
{{ config.get('api_key') }}                       {}                 -> True
{{ config.api_key }}                              {'api_key': None}  -> True
{{ config.get('a') and config.get('b') }}         {'a': 1}           -> True

This bit source-linear: a ConfigAddFields condition {{ config.get('api_key') }} fired for OAuth configs and wrote an empty credentials.api_key, breaking OAuth token persistence. The connector-side workaround (config.get('api_key', False)) still misfires when api_key is explicitly null; this change closes that gap at the CDK level.

Behavior change / blast radius

This is a behavior change for any manifest condition that currently relies on a None-valued expression being truthy. From the issue author's scan of 534 manifests in airbytehq/airbyte:

  • Two expressions change behavior when a key is missing:
    • source-airtable bases stream RecordFilter {{ record.get("permissionLevel") }} — bases without permissionLevel would now be filtered out.
    • source-zendesk-support stop_condition: {{ response.get("end_of_stream") }} — pagination continues (rather than stops) when end_of_stream is absent but a next cursor exists.
  • ~37 expressions in 18 connectors change only when a key is present with an explicit null.

Monorepo connectors pin their CDK version, so those two should be reviewed when they next bump the CDK. Custom Builder connectors receive minor/patch CDK releases automatically via DeclarativeSourceUpdater, so this note is the user-facing changelog signal.

Versioning decision

I've titled this fix: (patch) rather than fix!: (major). Reasoning: the change aligns InterpolatedBoolean with the documented Jinja/Python semantics that manifest authors already expect, the previous behavior was a bug that produced incorrect connector behavior (17212), and no public interface changes. If maintainers would rather ship this as a major release given the Builder auto-update path, retitling to fix!: is the only change needed.

  • Reviewer: confirm fix: (patch) vs fix!: (major) for this behavior change.

Test plan

  • poetry run pytest unit_tests/sources/declarative/interpolation/ -q — 196 passed.
  • poetry run pytest unit_tests/sources/declarative/ -q — full declarative suite (in progress locally; will follow up with results on this PR).
  • poetry run ruff check . and poetry run ruff format --check . — clean.
  • Not tested: live syncs of source-airtable / source-zendesk-support on this branch.

Requested via the /ai-fix playbook from the triage session for issue 17241 (issue author: Patrick Nilan (@pnilan)).

Link to Devin session: https://app.devin.ai/sessions/5369826e03544650b0c3edd431cef88e
Open in Devin Desktop: https://app.devin.ai/desktop/session/5369826e03544650b0c3edd431cef88e?variant=devin

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1789406745-interpolated-boolean-none#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1789406745-interpolated-boolean-none

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

A moderate correctness issue remains because string values rendered as None may be incorrectly treated as false.

Pull request overview

Updates declarative boolean interpolation so None conditions evaluate as false, with regression coverage.

Changes:

  • Adds None to InterpolatedBoolean.FALSE_VALUES.
  • Tests missing, explicit-null, short-circuit, and truthy fallback cases.
File summaries
File Summary
unit_tests/sources/declarative/interpolation/test_interpolated_boolean.py Adds regression tests for null and short-circuit conditions.
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py Treats interpolated None values as false.
Review details

Suppressed comments (1)

airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py:22

  • Adding the Python None sentinel here also classifies any rendered string exactly equal to None as false: JinjaInterpolation._literal_eval parses the output of {{ 'None' }} (and a config value 'None') into None before this membership check. That expression/value is truthy in both Jinja and Python, so this introduces a false negative beyond the intended null case. Preserve the original Jinja value/type, or otherwise distinguish a real null from a string rendering, and add a regression test.
    None,
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pnilan
Patrick Nilan (pnilan) marked this pull request as ready for review September 14, 2026 17:36
@pnilan
Patrick Nilan (pnilan) requested a review from a team as a code owner September 14, 2026 17:36
@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 388 tests  +6   4 376 ✅ +6   9m 19s ⏱️ -32s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 42f64b6. ± Comparison against base commit 95439d6.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 391 tests  +6   4 379 ✅ +6   14m 13s ⏱️ + 3m 16s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 42f64b6. ± Comparison against base commit 95439d6.

@pnilan
Patrick Nilan (pnilan) merged commit 96a7c0a into main Sep 14, 2026
33 of 34 checks passed
@pnilan
Patrick Nilan (pnilan) deleted the devin/1789406745-interpolated-boolean-none branch September 14, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants