From 20e89334c09d47c8b58c5ee31af18784cd6a788a Mon Sep 17 00:00:00 2001 From: goodboy Date: Tue, 18 Aug 2026 22:11:26 -0400 Subject: [PATCH] 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`)) --- .../20260819T020757Z_b38efed7_prompt_io.md | 34 +++++++++++++++++++ ...20260819T020757Z_b38efed7_prompt_io.raw.md | 25 ++++++++++++++ tests/test_to_actor.py | 30 ++++++++++++---- tractor/to_actor/_api.py | 2 +- 4 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md create mode 100644 ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.raw.md diff --git a/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md b/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md new file mode 100644 index 00000000..f9472ca3 --- /dev/null +++ b/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.md @@ -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. diff --git a/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.raw.md b/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.raw.md new file mode 100644 index 00000000..9ee17f63 --- /dev/null +++ b/ai/prompt-io/opencode/20260819T020757Z_b38efed7_prompt_io.raw.md @@ -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 diff --git a/tests/test_to_actor.py b/tests/test_to_actor.py index abee1c3b..a163c2b6 100644 --- a/tests/test_to_actor.py +++ b/tests/test_to_actor.py @@ -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, + }, ) ) diff --git a/tractor/to_actor/_api.py b/tractor/to_actor/_api.py index 49c8b91c..d26a163b 100644 --- a/tractor/to_actor/_api.py +++ b/tractor/to_actor/_api.py @@ -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