Rectify: run_python Type-Erasure Boundary — Annotation-Aware Coercion#1972
Merged
Trecek merged 5 commits intoMay 6, 2026
Merged
Conversation
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
Trecek
force-pushed
the
annotate-pr-diff-has-100-failure-rate-int-vs-str-type-mismat/1965
branch
from
021061a to
c35cd0c
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Coerce primitive scalar types at the _import_and_call dispatch boundary, closing the type-safety gap between direct MCP tools (Pydantic-validated) and run_python callables (raw dict). When LLM JSON passes an int for a str-annotated param, _coerce_scalar now converts int→str, str→int, str→float, int→float as needed, with structlog warnings for auditability. Add str() guards at three subprocess call sites as defense-in-depth: - smoke_utils.annotate_pr_diff: pr_number - recipe._cmd_rpc.wait_for_direct_merge: pr_number - recipe._cmd_rpc.force_push_and_wait_mergeability: review_pr_number Fix implementation.yaml note that primed LLM to pass integer PR numbers. Add parametrized test class TestImportAndCallTypeCoercion (8 cases) and callable-level tests for annotate_pr_diff, wait_for_direct_merge, and force_push_and_wait_mergeability with int inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…esolution ARCH-003 requires all except-Exception blocks to include a logger call. The get_type_hints fallback now logs a warning with exc_info. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rd in _coerce_scalar - Remove redundant `import typing` inside _coerce_scalar (already at module level) - Remove misleading `a is not None` clause from __args__ filters (type objects, not singletons) - Add `isinstance(val, bool)` early-return guard to prevent bool→str coercion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nconvertible_value_not_coerced Test asserts success=False — name now accurately reflects that coercion is skipped and the callable fails, not that the value "passes through". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Trecek
force-pushed
the
annotate-pr-diff-has-100-failure-rate-int-vs-str-type-mismat/1965
branch
from
d1944b1 to
af1c482
Compare
Trecek
deleted the
annotate-pr-diff-has-100-failure-rate-int-vs-str-type-mismat/1965
branch
Trecek
added a commit
that referenced
this pull request
May 8, 2026
…#1972) ## Summary The `run_python` MCP tool dispatches callables through `_import_and_call` using a `dict[str, object]` parameter — a type-erasure boundary. Direct MCP tools get Pydantic lax-mode validation via FastMCP (which coerces `str→int` automatically), but `run_python` callables receive raw unvalidated values. The dispatcher already calls `inspect.signature(func)` but only uses it for `None→default` coercion (PR #1602), never reading `param.annotation`. This creates an asymmetry where `str`-typed callable parameters receiving JSON integers crash at subprocess boundaries, f-string operations, or Path construction. The architectural fix: extend `_import_and_call`'s existing coercion loop to read `param.annotation` and coerce primitive scalar types, closing the type-safety gap between the two dispatch paths. Defense-in-depth: add `str()` guards at each subprocess call site. Structural tests: a parametrized test matrix that exercises every callable × every param type combination through `_import_and_call`. Closes #1965 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260505-183247-696032/.autoskillit/temp/rectify/rectify_run_python_type_coercion_2026-05-05_183800.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
The
run_pythonMCP tool dispatches callables through_import_and_callusing adict[str, object]parameter — a type-erasure boundary. Direct MCP tools get Pydantic lax-mode validation via FastMCP (which coercesstr→intautomatically), butrun_pythoncallables receive raw unvalidated values. The dispatcher already callsinspect.signature(func)but only uses it forNone→defaultcoercion (PR #1602), never readingparam.annotation. This creates an asymmetry wherestr-typed callable parameters receiving JSON integers crash at subprocess boundaries, f-string operations, or Path construction.The architectural fix: extend
_import_and_call's existing coercion loop to readparam.annotationand coerce primitive scalar types, closing the type-safety gap between the two dispatch paths. Defense-in-depth: addstr()guards at each subprocess call site. Structural tests: a parametrized test matrix that exercises every callable × every param type combination through_import_and_call.Closes #1965
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260505-183247-696032/.autoskillit/temp/rectify/rectify_run_python_type_coercion_2026-05-05_183800.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
Token Efficiency