Skip to content

Let HookToolset pin arguments the model must not choose - #73900

Merged
kaxil merged 1 commit into
commonai-object-storage-toolsetfrom
commonai-hook-pinned-arguments
Sep 30, 2026
Merged

kaxil merged 1 commit into
commonai-object-storage-toolsetfrom
commonai-hook-pinned-arguments

Conversation

@kaxil

@kaxil kaxil commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

HookToolset exposes the hook methods in allowed_methods, and until now the model chose every argument of those methods, including the ones that decide what it can reach, such as the bucket a storage hook reads. pinned_arguments fixes those values:

HookToolset(S3Hook(), allowed_methods=["list_keys", "read_key"], pinned_arguments={"bucket_name": "reports"})

A pinned argument is left out of the schema the model sees and passed to every call. If the model supplies it anyway, the call is refused with an error it can read, rather than silently using the model's value. Each call gets its own copy of a pinned value, so a hook method that mutates a list or dict cannot change what the next call receives.

Pinning fails closed. Every method in allowed_methods must name every pinned argument as a parameter it accepts by keyword, or the toolset raises ValueError when it is created. The alternative, skipping methods that don't take a pin, would leave the model free to choose that value on exactly the methods the Dag author didn't check, such as a copy_object whose destination bucket goes by another name.

pinned_arguments is experimental and listed on the stability page.

A scripted model that tries to set the pinned region gets a validation error back, and list_accounts runs with the pinned emea:

Pinned argument refused and pinned value used


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@kaxil
kaxil added this pull request to stack #73905 September 29, 2026 12:49
@kaxil kaxil closed this Sep 29, 2026
@kaxil kaxil reopened this Sep 29, 2026
@kaxil
kaxil marked this pull request as ready for review September 29, 2026 13:17
@kaxil
kaxil force-pushed the commonai-hook-pinned-arguments branch 2 times, most recently from cef8491 to b1245b7 Compare September 29, 2026 20:07
@kaxil
kaxil requested a review from vatsrahul1001 September 29, 2026 20:53
pinned_arguments fixes values such as the bucket a storage hook may
read. A pinned argument is left out of the schema the model sees, passed
to every allowed method, and refused if the model supplies it anyway.
Every allowed method has to take each pinned argument by name: one that
takes the same thing under another name, in a dict or through **kwargs
would let the model choose it after all, so the toolset refuses to be
built with it.
@kaxil
kaxil force-pushed the commonai-hook-pinned-arguments branch from b1245b7 to 42c514c Compare September 30, 2026 06:01
@kaxil
kaxil merged commit 35507e9 into main Sep 30, 2026
63 of 75 checks passed
@kaxil
kaxil deleted the commonai-hook-pinned-arguments branch September 30, 2026 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants