fix: reject *args/**kwargs in @inject to prevent silent arg corruption#1
Merged
Conversation
Since 2.0.0, @Inject binds a task's arguments to its visible signature by name (bound.arguments), which makes injection parameter-order-insensitive but cannot faithfully forward *args/**kwargs: Signature.bind stores their values under the literal names "args"/"kwargs", so a task declaring *args/**kwargs alongside a FromDI parameter had its real payload silently misrouted into a keyword argument instead of raising. Reject such signatures at decoration time with a clear TypeError naming the offending parameter. A task with no FromDI parameter is returned unchanged and may still use *args/**kwargs. Documented in architecture/dependency-injection.md; notes at planning/releases/2.0.1.md. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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
1 participant
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.
The bug
Since 2.0.0,
@injectbinds a task's arguments to its visible signature by name (bound.arguments) — that's what makes injection parameter-order-insensitive. But a by-name call can't faithfully forward*args/**kwargs:inspect.Signature.bindstores their values under the literal names"args"/"kwargs", so a task declaring*args/**kwargsalongside aFromDIparameter had its real payload silently misrouted.Reproduced (pre-fix):
sample.delay(1, 2, foo="bar")ondef sample(svc: Annotated[Svc, FromDI(Svc)], *args, **kwargs)arrives inside the task asargs=(), kwargs={"args": (1, 2), "kwargs": {"foo": "bar"}}— silent corruption, no error.(Same latent bug was found and fixed in
modern-di-arqduring its build; this ports the guard back.)The fix
@injectnow raises a clearTypeErrorat decoration time when a task declares aVAR_POSITIONAL/VAR_KEYWORDparameter together with aFromDIparameter, naming the offending parameter. A task with noFromDIparameter is returned unchanged and may use*args/**kwargsfreely. TheDITaskpath inherits the guard (fails at class instantiation).TDD: two failing tests first (
test_inject_rejects_var_{positional,keyword}_with_fromdi), then the guard. Bug reproduced and fix verified by an independent reviewer. Documented inarchitecture/dependency-injection.md; notes atplanning/releases/2.0.1.md.100% coverage; ruff/ty/eof-fixer clean.
🤖 Generated with Claude Code