Skip to content

sandbox: Make updated resource path configurable #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
p1-0tr merged 1 commit into from
Sep 23, 2025
Merged

sandbox: Make updated resource path configurable #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/inference/backends/llamacpp/llamacpp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func (l *llamaCpp) Run(ctx context.Context, socket, model string, mode inference
command.Stdout = serverLogStream
command.Stderr = out
},
l.updatedServerStoragePath,
filepath.Join(binPath, "com.docker.llama-server"),
args...,
)
Expand Down
9 changes: 6 additions & 3 deletions pkg/sandbox/sandbox_darwin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"os/user"
"path/filepath"
"strings"
)

Expand Down Expand Up @@ -78,8 +79,8 @@ const ConfigurationLlamaCpp = `(version 1)
(subpath "/usr")
(subpath "/System")
(regex #"Docker\.app/Contents/Resources/model-runner")
(subpath "[HOMEDIR]/.docker/bin/inference")
(subpath "[HOMEDIR]/.docker/bin/lib"))
(subpath "[UPDATEDBINPATH]")
(subpath "[UPDATEDLIBPATH]"))
(allow file-write*
(literal "/dev/null")
(subpath "/private/var")
Expand Down Expand Up @@ -116,7 +117,7 @@ func (s *sandbox) Close() error {
// configuration, for which a pre-defined value should be used. The modifier
// function allows for an optional callback (which may be nil) to configure the
// command before it is started.
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), name string, arg ...string) (Sandbox, error) {
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
// Look up the user's home directory.
currentUser, err := user.Current()
if err != nil {
Expand All @@ -133,6 +134,8 @@ func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd),
// text/template if this gets any more complex.
profile := strings.ReplaceAll(configuration, "[HOMEDIR]", currentUser.HomeDir)
profile = strings.ReplaceAll(profile, "[WORKDIR]", currentDirectory)
profile = strings.ReplaceAll(profile, "[UPDATEDBINPATH]", updatedBinPath)
profile = strings.ReplaceAll(profile, "[UPDATEDLIBPATH]", filepath.Join(filepath.Dir(updatedBinPath), "lib"))

// Create a subcontext we can use to regulate the process lifetime.
ctx, cancel := context.WithCancel(ctx)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sandbox/sandbox_other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s *sandbox) Close() error {
// configuration, for which a pre-defined value should be used. The modifier
// function allows for an optional callback (which may be nil) to configure the
// command before it is started.
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), name string, arg ...string) (Sandbox, error) {
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
Copy link
Contributor

@sourcery-ai sourcery-ai bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: updatedBinPath parameter is unused in this implementation.

If updatedBinPath is included for cross-platform consistency, please document this in the code. Otherwise, consider removing it to avoid confusion.

Suggested change
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
/*
updatedBinPath is included for cross-platform consistency with other platform-specific implementations.
It may not be used in this implementation, but is retained to keep the function signature uniform.
*/
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {

// Create a subcontext we can use to regulate the process lifetime.
ctx, cancel := context.WithCancel(ctx)

Expand Down
4 changes: 2 additions & 2 deletions pkg/sandbox/sandbox_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ func TestSandbox(t *testing.T) {
var sandbox Sandbox
var err error
if runtime.GOOS == "windows" {
sandbox, err = Create(t.Context(), ConfigurationLlamaCpp, nil, "go", "version")
sandbox, err = Create(t.Context(), ConfigurationLlamaCpp, nil, "/some/path/bin", "go", "version")
Copy link
Contributor

@sourcery-ai sourcery-ai bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Test does not verify the effect of configurable resource paths.

Please add assertions to confirm that the sandbox configuration uses the provided path and its derived lib path as intended.

Suggested implementation:

	if err != nil {
		t.Fatal("unable to create sandboxed process:", err)
	}

	// Assert that the sandbox uses the provided bin path
	if sandbox.BinPath != "/some/path/bin" {
		t.Errorf("expected BinPath to be '/some/path/bin', got '%s'", sandbox.BinPath)
	}

	// Assert that the sandbox uses the derived lib path
	expectedLibPath := "/some/path/bin/lib"
	if sandbox.LibPath != expectedLibPath {
		t.Errorf("expected LibPath to be '%s', got '%s'", expectedLibPath, sandbox.LibPath)
	}

If the Sandbox struct does not have BinPath or LibPath fields, you will need to adjust the field names to match your implementation. If the derived lib path logic is different, update expectedLibPath accordingly.

} else {
sandbox, err = Create(t.Context(), ConfigurationLlamaCpp, nil, "date")
sandbox, err = Create(t.Context(), ConfigurationLlamaCpp, nil, "/some/path/bin", "date")
}
if err != nil {
t.Fatal("unable to create sandboxed process:", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sandbox/sandbox_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (s *sandbox) Close() error {
// configuration, for which a pre-defined value should be used. The modifier
// function allows for an optional callback (which may be nil) to configure the
// command before it is started.
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), name string, arg ...string) (Sandbox, error) {
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
Copy link
Contributor

@sourcery-ai sourcery-ai bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: updatedBinPath parameter is unused in this implementation.

If updatedBinPath is included only for interface consistency, please document this or refactor to prevent confusion.

Suggested implementation:

/*
Create initializes a new Sandbox instance.

configuration: Pre-defined configuration value.
modifier: Optional callback to configure the command before it is started (may be nil).
updatedBinPath: Included for interface consistency with other platforms; currently unused in this implementation.
name: Name of the command to execute.
arg: Arguments to pass to the command.
*/
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
func Create(ctx context.Context, configuration string, modifier func(*exec.Cmd), updatedBinPath, name string, arg ...string) (Sandbox, error) {
	// updatedBinPath is intentionally unused in this implementation.
	// Parse the configuration and configure limits.
	limits := []winjob.Limit{winjob.WithKillOnJobClose()}
	tokens := limitTokenMatcher.FindAllString(configuration, -1)

// Parse the configuration and configure limits.
limits := []winjob.Limit{winjob.WithKillOnJobClose()}
tokens := limitTokenMatcher.FindAllString(configuration, -1)
Expand Down
Loading