Skip to content

Warn on suspicious Dag and task IDs at TS SDK build time - #70993

Merged
jason810496 merged 2 commits into
apache:mainfrom
Andrushika:ts-sdk-suspicious-id-warnings
Aug 20, 2026
Merged

Warn on suspicious Dag and task IDs at TS SDK build time#70993
jason810496 merged 2 commits into
apache:mainfrom
Andrushika:ts-sdk-suspicious-id-warnings

Conversation

@Andrushika

Copy link
Copy Markdown
Contributor

Why

ts-sdk has validated Dag and task IDs by throwing at registration since #69400. The review in #69965 settled on best-effort build time warnings for all SDKs instead, so a stale client-side rule cannot block packing or task runs.

What

airflow-ts-pack now warns on invalid IDs (over 250 chars, bad characters, ..) during build time, and still packs.
The task registry keeps only the non-empty check. ID length is counted in code points now, since .length counts astral characters twice.

related: #69400, #69965, #69937


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code (Fable 5) following the guidelines

@Andrushika

Copy link
Copy Markdown
ContributorAuthor

Hi @jason810496 could you please take a look if you have time? Thanks a lot!

@potiukpotiuk added the ready for maintainer review Set after triaging when all criteria pass. label Aug 13, 2026

@jason810496jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure! I prefer to wait until #71144 get merged first, then rebase this one on top of it, thanks.

@jason810496jason810496 added this to the TS SDK 1.0 Beta milestone Aug 17, 2026
The throwing check from apache#69400, moved to manifest build time by
apache#71144, still fails packing outright when the client-side ID rules
drift from the server. Follow the direction settled in apache#69965: warn
at build time, let the server validate authoritatively.
@Andrushika
Andrushikaforce-pushed the ts-sdk-suspicious-id-warnings branch from 2977fc2 to 897a0c1CompareAugust 19, 2026 10:45
@Andrushika

Copy link
Copy Markdown
ContributorAuthor

Rebased, thanks!

@jason810496jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM overall, thanks.

Comment threadts-sdk/src/coordinator/manifest.ts
Comment threadts-sdk/src/cli/pack.ts Outdated
A numeric dagId becomes a string object key in the manifest while the
registry Map keeps the number, so the handler can never match at
runtime and the warning path cannot see it. Empty task ids violate the
bundle metadata schema's minLength. Both are client-side type and
schema defects rather than server rules that could drift, so they fail
the pack instead of warning.
@jason810496
jason810496 merged commit 259fe2d into apache:mainAug 20, 2026
89 checks passed
@Andrushika
Andrushika deleted the ts-sdk-suspicious-id-warnings branch August 20, 2026 14:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ts-sdkready for maintainer reviewSet after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Andrushika@jason810496@potiuk