Repository navigation
ExecutionProvider: declare supported runtimes, execute synchronously - #104
Merged
Merged
Conversation
…nously ExecutionCapabilities and the unenforced network and filesystem flags go: a provider declares supportedRuntimes() and keeps its own timeout clamp. execute runs on the tool call's thread and must stop, throwing CancellationException with the interrupt status kept, when that thread is interrupted or the call's token fires. An interrupted JShell execute now closes its sandbox session.
…inert ReplSession.close is idempotent, so the interrupt path closes the session directly instead of running the callback through its compare-and-set; the flag now only keeps a kill that fires after the call open, which the one remaining comment says. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The handlers are held in an unmodifiable EnumMap, whose key set is the supported runtimes in Runtime declaration order, so the second field that copied them goes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
It compared Set.of(JSHELL) with itself, which supportsJShellOnly already pins; the section heading no longer names the deleted capabilities. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… CHANGELOG paragraphs ExecuteTool no longer waits on a provider future: a provider's Error now leaves through the tool worker like any tool's, which the row already covers. The two 3.0 paragraphs edited in place had lines past 100 columns. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The tool call's interrupt is now how a session stops a running execute, and execute untracks the child before it throws. Its SIGTERM left a child that ignores TERM running unbounded and out of reach of close() and the shutdown hook; before, the provider's own thread kept waiting and the timeout escalated to SIGKILL. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Guarantee 5 of the synchronous execute was covered only at the tool level; this drives it through a session, where the tool worker records the provider's Error and the result fails with the same instance. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…terrupt status Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
A key set refuses add whether or not its map is modifiable, so only a removal proves the set is unmodifiable. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The token's registration is removed a few lines later, and a SIGTERM the token sends to an already SIGKILLed child does nothing. Co-Authored-By: Claude Opus 5.5 <[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
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.
Spec:
v3-simplify-execution.ExecutionProviderdeclares its supported runtimes, andexecuteruns synchronously.What changes
ExecutionCapabilities(the record and its builder) is deleted.ExecutionProvider.supportedRuntimes()replacescapabilities().ExecuteArgumentsstill refuses any other runtime before it builds a request, with the same message.executecontract:executereturnsExecutionResultand runs on the tool call's own thread. Its javadoc states the contract:CancellationException, keeping the interrupt status;Future.get, neverjoin;RuntimeExceptiononly when the provider itself is broken.ExecuteTool.dispatchcalls the provider directly:CancellationException→Execute: cancelled;RuntimeException→Execute: provider failed: <message or simple name>;Errorpropagates as itself and ends the session through the tool worker.LocalProcessExecutionProvider:EnumMap, so they iterate inRuntimedeclaration order.maxTimeoutis a private field that only the clamp reads.withNetworkAllowed/withFilesystemWriteAllowedare removed.JShellExecutionProvider:SessionExecution.runruns the snippet on the caller's thread.CancellationException, on every exit path. That includes a sandbox that answers the interrupt with its own exception, which was Sail's round-1 finding.JvmSandboxConfig.Builder.withSandboxPolicy/SandboxPolicy.noEgress().NoopExecutionProvider: it supports no runtimes and returns its refusal directly.ExecuteToolas a waiting path.SessionEnding's javadoc and from the 3.0 CHANGELOG paragraph.package-infonow says requests run on the calling thread.Tests
session.test.ExecutionCall. It runsexecuteon a virtual thread of its own and records the interrupt status at the end. It is shared with repl through the session test-jar.ExecuteTool: every outcome mapping.TimedOut, andinflightCount()is 0.Errorfails the result as the same instance.interruptKillsAChildThatIgnoresSigterminterruptKillsTheChildsDescendantsinterruptWhilePermitWaitThrowsCancellationAndKeepsTheInterruptexecutesOnASharedTokenLeaveNoCallbackOnItanErrorTheProviderThrowsFailsTheResultAsItselfExecutionCapabilitiesTest;builderWithAllOptions;supportedRuntimesIsStable, which compared a constant with itself.>→>=clamp mutants, which behave identically;mvn -B clean verify javadoc:jar: BUILD SUCCESS, 7863 tests run, 0 failures, 0 errors, 62 skipped (assumption-gated and live-key tests).Recorded, not changed (theoretical)
DispatchedCall.cancelsettles the answer before it interrupts.Size
Net main-code lines (excluding tests and docs): −286 (+173 / −459): session −236, repl −50. The spec budgeted about −260.
🤖 Generated with Claude Code