Skip to content

fix(exec): honor explicit null node selection - #1248

Merged
penso merged 2 commits into
moltis-org:mainfrom
mikemikimike:codex/fix-node-null-1246
Aug 31, 2026
Merged

fix(exec): honor explicit null node selection#1248
penso merged 2 commits into
moltis-org:mainfrom
mikemikimike:codex/fix-node-null-1246

Conversation

@mikemikimike

Copy link
Copy Markdown

Summary

  • honor an explicit node: null as a request to use the local execution path
  • preserve configured and provider-selected defaults when node is omitted
  • add a regression test with a connected node provider and a configured default

Root cause

ExecTool converted the optional node value to a string and then fell back to the configured or provider default whenever that conversion returned None. This made an explicit JSON null indistinguishable from an omitted field, so a tool call intended to return to the local sandbox continued to execute on the selected SSH node.

Validation

  • targeted Rust formatting check for crates/tools/src/exec.rs
  • git diff --check
  • local test execution was blocked by the pinned nightly toolchain's incomplete installation and an uncached Git submodule dependency; the repository's remote CI is required for compilation and test verification

Closes #1246.

Signed-off-by: mikemikimike <13286568797@163.com>
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR now distinguishes an explicitly null node from an omitted value, routing explicit null requests through local execution while preserving configured and provider defaults when omitted.

  • Updates the published exec-tool schema to accept both string and null node values.
  • Bypasses remote node selection when the caller explicitly supplies node: null.
  • Adds regression coverage for local execution and the nullable schema.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/tools/src/exec.rs Correctly exposes null in the node schema and treats explicit null as a local-execution override without changing omitted-node behavior.
crates/tools/src/exec/tests.rs Adds focused regression tests covering explicit-null local routing and the corresponding schema contract.

Reviews (2): Last reviewed commit: "fix(exec): allow null node in tool schem..." | Re-trigger Greptile

Comment thread crates/tools/src/exec.rs
@mikemikimike

Copy link
Copy Markdown
Author

fixed-in-7bdc19e

@penso

penso commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@greptileai review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: can't run on sandbox after a node is added

2 participants