Fix template-field logic timing in BashOperator

[bramhanandlingala]

Fixes the standard provider's BashOperator entry from the #70296 exemption-list burn-down.

bash_command is a template field, but __init__ classified it as inline-vs-script-file using the raw, un-rendered string — so a templated value could be misclassified before Jinja ever rendered it.

• Moved the classification into execute(), after templating occurs.
• __init__ no longer touches bash_command's content.
• Removed BashOperator from the exemption list.
• Added a test covering a templated bash_command resolving to a script file.

Related to #70296

Gen-AI disclosure: I used a generative AI tool to help identify the root
cause, write tests, and draft the PR description. I reviewed, tested, and
verified all changes locally before submitting.

Was generative AI tooling used to co-author this PR?

• Yes — Claude

Generated-by: Claude following the guidelines

[shahar1]

Hey, I've somehow missed it in #70315 that I've already merged - but it should be clarified:

You must disclose AI usage in the code changes within the PR body according to the guidelines.
I'd like to ask editing the PR body of both your merged PRs ( #68342,
#70315), as well as all of your open PRs to add AI attribution with the PR body - otherwise they will be blocked from merging.
Once this is addressed, I will remove the block. Please do not create new PRs that are gen-AI assisted without disclosure.

[bramhanandlingala]

Hi @shahar1

Thanks for flagging this, and apologies for the oversight — you're right, that
should have been disclosed from the start.

I've added the Gen-AI disclosure to the PR body of #68342 and #70315, as well
as all my currently open PRs. I'll make sure to include it going forward.

Let me know if the wording needs any adjustment, and thanks for catching this.

Concerns addressed

[shahar1]

Hi @shahar1

Thanks for flagging this, and apologies for the oversight — you're right, that should have been disclosed from the start.

I've added the Gen-AI disclosure to the PR body of #68342 and #70315, as well as all my currently open PRs. I'll make sure to include it going forward.

Let me know if the wording needs any adjustment, and thanks for catching this.

Almsot there, there's a failure in the static checks and one of the tests

[bramhanandlingala]

Hi @shahar1,@uranusjr

Thanks for flagging — pushed a fix for both. The _is_inline_cmd attribute was accidentally dropped from init in my last commit, causing the AttributeError; restored it and cleaned up the classification logic as per @uranusjr comment. The static check was just a missing trailing newline in the test file.

Re-requesting review, thanks for your patience!

[bramhanandlingala]

Hey @shahar1, I've made the change according to your suggestion — moved the classification into a render_template_fields override so it runs on the raw bash_command before templating replaces the script-file value with its rendered contents. Re-requesting review, thanks!

[shahar1]

Hey @shahar1, I've made the change according to your suggestion — moved the classification into a render_template_fields override so it runs on the raw bash_command before templating replaces the script-file value with its rendered contents. Re-requesting review, thanks!

Hey, apparently I've made a mistake - commented in the last thread with the details.

[bramhanandlingala]

Thanks for the clarification, @shahar1. No worries, and thanks for investigating it further. I've reverted the previous approach. Could you please take another look when you have a chance?

[shahar1]

Thanks for the clarification, @shahar1. No worries, and thanks for investigating it further. I've reverted the previous approach. Could you please take another look when you have a chance?

When you get to the state that the PR is ready for re-review, please let me know and let the CI/CD run until it ends (w/o merging from main). Thanks!

Concerns addressed

[bramhanandlingala]

All checks are passing now, @shahar1 — ready for another look whenever you get a chance. Thanks!

[shahar1]

Human Summary

We're making progress, well done for your dedication!

AI Summary

Found 2 blocking issues and 1 major: the current head classifies inline-vs-script in execute(), which runs after the Task SDK has replaced a script-file bash_command with the file's rendered contents — re-introducing the regression demonstrated empirically earlier in this thread (the revert landed back on the original classification point rather than the pre-substitution one described in the previous comment). The @task.bash classification line also still needs to be restored, and the new test passes on main unchanged. Details are in the inline comments; all findings were re-verified independently by code trace plus empirical runs on main before posting.

The premise of the PR is right — __init__-time classification is genuinely wrong for templated values rendering to .sh paths — the remaining work is only where the classification runs.

---

This review was drafted by an AI-assisted tool and confirmed by an Apache Airflow maintainer. After you've addressed the points above and pushed an update, an Apache Airflow maintainer — a real person — will take the next look at the PR. The findings cite the project's review criteria; if you think one of them is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Apache Airflow handles maintainer review: contributing-docs/05_pull_requests.rst.

---

Drafted-by: Claude Code (Fable 5) using Apache Magpie; reviewed by @shahar1 before posting

[shahar1]

Blocking — script-file commands now run inline

By the time execute() runs, a literal bash_command="script.sh" no longer holds the path — the Task SDK has already substituted the file's rendered contents (parse time via resolve_template_files(), templater.py#L91; render time via the template_ext file branch, templater.py#L256 — and the worker always renders before execute). Script contents virtually never end in .sh, so every real script-file command classifies as inline and runs via bash -c "<entire script>", defeating _run_rendered_script_file's documented purpose ("prevents 'Argument list too long' error" — Linux caps a single argv entry at ~128 KiB) and changing $0. This is the same failure mode as the comparison table earlier in this thread — this classification point is even later than the variant that table measured:

                  this branch                          main
_is_inline_cmd :  True                                 False
239 KB script  :  OSError [Errno 7] Arg list too long  OK
$0             :  /usr/bin/bash                        tmpXXXX.sh

Classification has to happen before the contents substitution — per the earlier comment: a resolve_template_files() override that records _is_inline_cmd from the raw value before calling super(), unguarded (the Any widening you added is exactly what makes that safe), with an is None fallback in execute() for values that only materialize at runtime.

Drafted by Claude Code (Fable 5) using Apache Magpie; reviewed by shahar1 before posting.

[shahar1]

Blocking — @task.bash returning a .sh filename now runs inline

The deleted classification line above this call needs to be restored (also called out in the earlier thread comment): at parse time the decorator's bash_command is the non-str SET_DURING_EXECUTION sentinel, so no parse-time hook can ever classify it — the deleted line was the only point where the raw filename returned by the callable was still observable. Without it, self.render_template_fields(context) substitutes the .sh file's rendered contents, and super().execute() then classifies script content as inline — same failure mode as in operators/bash.py (E2BIG on large scripts, $0 change).

Drafted by Claude Code (Fable 5) using Apache Magpie; reviewed by shahar1 before posting.

[shahar1]

Test does not fail without the fix

BashOperator._is_inline_command(bash_command="sample.sh") returns False on main as well (verified empirically), and the render path this test exercises is untouched by the diff — so this test passes with and without the change, and also would not catch the regressions flagged above. Per the testing standard in AGENTS.md: "every test must fail without the PR's change."

Assert on the strategy the operator actually picks instead — e.g. mock _run_inline_command / _run_rendered_script_file and assert which one runs (PR #70369 has a test in exactly that shape, worth borrowing).

Drafted by Claude Code (Fable 5) using Apache Magpie; reviewed by shahar1 before posting.

[shahar1]

_is_inline_cmd is now dead code

This attribute is assigned None here and never read anywhere on this branch — execute() calls the classmethod directly, and the decorator's write was removed. Either wire it into the pre-substitution classification described above (which uses it), or remove it. Knock-on: decorators/test_bash.py:107 asserts this vestigial attribute; under the pre-substitution fix it becomes False and needs the one-line update mentioned in the earlier thread comment.

Drafted by Claude Code (Fable 5) using Apache Magpie; reviewed by shahar1 before posting.

[potiuk]

Thanks — but this duplicates #70369, which makes the same change to the same four files and was opened the day before. That one is already approved, so I'm closing this in its favour. (It also has changes requested here that #70369 doesn't.)

I've left a fuller note on #70678 about checking for an existing PR before starting — same situation there, and on #70621 which I closed earlier today. No criticism of the work itself; it's just worth a quick search first so the effort goes somewhere it isn't already covered.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting