fix(preflight): reject empty connection_qualified_name before extraction [CONNECT-1738] - #3869
Open
SwarnadeepChatterjee wants to merge 2 commits into
Open
SwarnadeepChatterjee wants to merge 2 commits into
SwarnadeepChatterjee wants to merge 2 commits into
Conversation
…ion [CONNECT-1738] The setup wizard can submit a Connection snapshot whose attributes map has no qualifiedName and schedule the workflow anyway. Nothing in the frontend, heracles, or the SDK preflight gate rejects it. The run then fails deep in transform_data after minutes of wasted extraction work. Add a platform-level check to the preflight gate activity that validates the connection qualified name from the extraction snapshot before credential resolution. The check: - Inspects both connection.attributes.qualifiedName and the top-level connection_qualified_name field - Skips when no connection data exists (non-connection workflows) - Returns NOT_READY with a typed InvalidInputError when the connection is present but the qualified name is empty - Respects the gate's enforce/soft mode (blocks in hard, reports in soft)
SwarnadeepChatterjee
requested review from
Aryamanz29,
OnkarVO7,
atlan-ci,
cmgrote and
vaibhavatlan
as code owners
September 20, 2026 09:50
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 81.7%) Detailed Coverage ReportThis message was truncated. Download full message |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
…havioral tests Re-apply the _check_connection_qualified_name helper and gate insertion that were lost during a stash-pop merge conflict. The check now runs inside the heartbeat try-block (after beats is initialised) so _emit_outcome can access the liveness guard. Replace the import-only red tests with proper behavioral gate-activity tests that exercise build_preflight_gate_activity end-to-end: - hard mode raises PreflightFailed on the broken snapshot - soft mode returns NOT_READY without raising - healthy snapshots proceed normally - non-connection snapshots skip the check - handler is never invoked when CQN is empty (short-circuit) Ref: CONNECT-1738
cmgrote
requested changes
Sep 21, 2026
cmgrote
left a comment
cmgrote
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for chasing this down to a concrete seam — the failure mode is real and the "reject before the 12 extraction activities" instinct is the right one. My concern is with where the rejection lives, and I think the gate placement is load-bearing for three defects rather than just a style preference.
The invariant already belongs to the contract
ConnectionAttributes already states this rule in its own docstring (application_sdk/contracts/types.py:504):
qualified_nameandnameare identity: a Connection without them is not addressable, so an explicitnullis rejected rather than coerced.
The model enforces that for an explicit null and not for an absent key — absent falls through to qualified_name: str = "". CONNECT-1738 is precisely the absent case. So this isn't a missing check; it's a hole in an invariant the contract already claims to own, and the PR patches the symptom two layers downstream of it.
Doing it on ConnectionRef buys four things the gate can't:
| contract | gate check | |
|---|---|---|
| coverage | every reader — publish node, incremental helpers, e2e harness, preflight_persist |
only workflows with the gate injected |
| policy | invariant, always | behind App.preflight_gate_mode, default "soft" |
| aliasing | alias_generator=to_camel + populate_by_name already resolve camel/snake |
hand-rolled .get("qualifiedName") or .get("qualified_name") |
| readers | one | two, which already disagree (#3 below) |
The honest counter-argument: a model_validator that raises during workflow-input deserialization becomes a workflow-task failure that retries forever with an opaque pydantic error — worse than today's five-minute failure. So I'd split it: the contract owns the predicate, an activity owns the verdict. Add something like ConnectionRef.is_addressable (or a classifier returning "absent / addressable / populated-but-unidentifiable"), and have the gate call that instead of re-deriving it from a raw dict. One definition, still a clean terminal failure.
Separately, if we want the submit path to reject too: /workflows/v1/start validates via a bare model_validate (handler/service.py:1273) rather than _validate_request, so a contract-level raise there answers 500 instead of a 422 naming the field. Small fix, worth pairing with this.
Three defects I reproduced on the branch
1. The soft default means this doesn't fix the reported failure
App.preflight_gate_mode defaults to "soft" (app/base.py:820), and every block site in preflight_gate.py is behind if enforce (2224, 2279, 2444). There's no unconditional-block path. Unless the app has opted into hard mode, the run still executes all 12 extraction activities — the original symptom is unchanged.
test_soft_mode_returns_not_ready_for_empty_cqn asserts exactly this behaviour and counts it as a pass, which is what hides it.
Worth deciding explicitly: is "the payload is malformed" a source-readiness opinion the app gets to soften? I'd argue no, and that this outcome should block irrespective of mode — which is another reason it doesn't sit naturally inside the gate's verdict taxonomy.
2. "Skips when no connection data exists" doesn't hold — it blocks every generated app
ExtractionInput.connection is Field(default_factory=ConnectionRef), and a default ConnectionRef dumps to {'typeName': 'Connection', 'attributes': {}} — never None. So if top_level_cqn is None and connection is None is dead code for every SDK-generated app:
from application_sdk.execution._temporal.preflight_gate import _check_connection_qualified_name
from application_sdk.templates.contracts.sql_metadata import ExtractionInput
snapshot = ExtractionInput().model_dump(mode="json")
print(snapshot["connection"])
# {'typeName': 'Connection', 'attributes': {}}
print(_check_connection_qualified_name(snapshot))
# name='connection_qualified_name' passed=False message='Connection snapshot is
# missing qualifiedName. ...' ← expected None (no connection was ever selected)Every toolkit example subclasses that ExtractionInput, including minimal, which declares no connection widget at all. Soft mode turns that into a would_block row on every healthy run of every connectionless app; hard mode blocks them.
3. It's a second reader of a value that already has a canonical one
preflight_persist.connection_qualified_name() (preflight_persist.py:241) already reads this, through ConnectionRef, and additionally handles the connection-qualified-name kebab key and the single-element-list form. Same activity, same snapshot, opposite answers:
from application_sdk.execution._temporal.preflight_gate import _check_connection_qualified_name as new
from application_sdk.execution._temporal.preflight_persist import connection_qualified_name as canonical
cases = {
"list-of-one": {"connection_qualified_name": ["default/x/1700000000"]},
"kebab key": {"connection-qualified-name": "default/x/1700000000",
"connection": {"attributes": {}}},
}
for label, snap in cases.items():
verdict = "BLOCK" if new(snap) is not None else "pass"
print(f"{label:12} canonical={canonical(snap)!r:28} new_check={verdict}")
# list-of-one canonical='default/x/1700000000' new_check=BLOCK
# kebab key canonical='default/x/1700000000' new_check=BLOCKThe same activity would persist a row carrying the qualified name alongside a verdict saying it is empty.
Common root cause for 2 and 3: every fixture in the new test file is a hand-written dict, so nothing exercises the shape a real ExtractionInput.model_dump(mode="json") actually produces. Building the fixtures from a real input would have caught both.
Suggested shape
- Put the predicate on
ConnectionRef/ConnectionAttributes— "populated but carries no identity". - Have the gate read the qualified name via
preflight_persist.connection_qualified_name()and classify with that predicate; drop_check_connection_qualified_nameentirely. - Decide whether a malformed-payload outcome blocks regardless of
preflight_gate_mode, and say so in the code either way. - Rebuild the fixtures from real
ExtractionInput(...).model_dump(mode="json")payloads, and add a case for an app with no connection widget.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
qualifiedNameand schedules the workflow anyway (CONNECT-1738)NOT_READYwhen the connection is present but the qualified name is emptywould_blockin soft modeContext
A prior production incident traced repeated nightly failures to a setup wizard submitting a
Connectionsnapshot carrying onlydefaultCredentialGuidand noqualifiedName. The existingConnectionQualifiedNameEmptyErrorchecks inapplication_sdk/common/incremental/helpers.pyonly fire duringtransform_data— after 12 extraction activities and ~5 minutes of wasted work. Neither the AE-leveluser_publish_preflightnor the app-level{app}:preflightvalidated the connection shape.What changed
application_sdk/execution/_temporal/preflight_gate.py_check_connection_qualified_name()helper + call in gate activity before credential resolutiontests/unit/execution/test_preflight_gate_connection_qn.pyTest plan