Reject misplaced empty `runtime_kwargs`

Treat `runtime_kwargs` as provided whenever it is not `None`.
Previously an empty dict bypassed placement validation and was
silently ignored when `an` or `portal` selected an existing runtime.

Reject both placement modes before actor startup for empty and
configured runtime kwargs while preserving empty-dict use when
`to_actor.run()` owns its private runtime.

Caught-during: review remediation
Found-via: `/code-review` P3 option-validation finding

Review: PR #481 (opencode)
https://github.com/goodboy/tractor/pull/481#pullrequestreview-4956692120

Prompt-IO: ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md

(this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`))
drop_ria_nursery
Gud Boi 2026-08-18 22:11:26 -04:00
parent b38efed753
commit 557065d8b5
4 changed files with 84 additions and 7 deletions

View File

@ -0,0 +1,34 @@
---
model: openai/gpt-5.6-sol
service: opencode
session: ses_3e4c90d3eafeqHEtRYSIHgHhpA
timestamp: 2026-08-19T02:07:57Z
git_ref: b38efed7
scope: code
substantive: true
raw_file: 20260819T020757Z_b38efed7_prompt_io.raw.md
---
## Prompt
Resolve the remaining P3 review finding before landing PR #481:
`runtime_kwargs={}` must not be silently accepted alongside either
`an=` or `portal=` merely because the dict is falsey. Keep this as its
own final review-remediation commit.
## Response summary
Treat any non-`None` `runtime_kwargs` value as provided when validating
placement. Cover both placement APIs with empty and configured dicts,
proving the error is raised locally before actor startup.
## Files changed
- `tractor/to_actor/_api.py` - validate option presence explicitly.
- `tests/test_to_actor.py` - cover four invalid option combinations.
## Human edits
No direct line edits. The human accepted the P3 finding, required it to
remain separate from the five P2 behavioral commits and prioritized it
before the final PR #484 integration rebase and PR #481 landing steps.

View File

@ -0,0 +1,25 @@
---
model: openai/gpt-5.6-sol
service: opencode
timestamp: 2026-08-19T02:07:57Z
git_ref: b38efed7
diff_cmd: git diff HEAD~1..HEAD
---
Fix the final PR #481 review finding: `runtime_kwargs` is mutually
exclusive with both caller placement options whenever it is provided,
including an empty dict.
> `git diff HEAD~1..HEAD -- tractor/to_actor/_api.py tests/test_to_actor.py`
Use an explicit `is not None` check rather than dict truthiness. Expand
the validation regression across `an=` and `portal=`, each with empty
and configured runtime kwargs, so every invalid combination fails
before actor runtime startup.
Verification:
- Trio/TCP: `23 passed`
- Trio/UDS: `23 passed`
- `mp_spawn`/TCP: `23 passed`
- Ruff and `git diff --check`: clean

View File

@ -511,12 +511,30 @@ def test_rejects_portal_and_an_combo():
)
def test_rejects_runtime_kwargs_with_placement():
@pytest.mark.parametrize(
'placement',
['an', 'portal'],
)
@pytest.mark.parametrize(
'runtime_kwargs',
[
{},
{'loglevel': 'cancel'},
],
ids=['empty', 'configured'],
)
def test_rejects_runtime_kwargs_with_placement(
placement: str,
runtime_kwargs: dict,
):
'''
`runtime_kwargs` only applies when the call opens
its own private actor-nursery; passing it alongside
a placement opt is an error, never silently
ignored.
ignored. In particular, an empty dict still means the
caller provided this mutually exclusive option; testing
both placement modes prevents truthiness checks from
accepting it before any actor runtime is started.
'''
with pytest.raises(ValueError):
@ -525,10 +543,10 @@ def test_rejects_runtime_kwargs_with_placement():
to_actor.run,
add_one,
1,
an=object(),
runtime_kwargs=dict(
loglevel='cancel',
),
**{
placement: object(),
'runtime_kwargs': runtime_kwargs,
},
)
)

View File

@ -295,7 +295,7 @@ async def run(
fn, args, kwargs = _normalize_call(fn, args)
if (
runtime_kwargs
runtime_kwargs is not None
and
(
an is not None