fix: validate volume mount source paths before spawning detached process

[stakeswky]

Summary

Fixes #2485

When running thv run --volume <source>:<target> with a non-existent source path, the command previously appeared to succeed but the workload would silently enter an error state. The actual error was only visible in the proxy log file.

Changes

pkg/runner/config_builder.go — Added pre-flight validation in processVolumeMounts():

• Checks that source paths exist on the host filesystem using os.Stat() (follows symlinks)
• Resolves relative paths to absolute before checking
• Skips validation for Kubernetes operator context (paths are container-relative)
• Skips validation for resource:// URIs
• Returns a clear, actionable error message: volume mount source path does not exist: /path

pkg/runner/config_builder_test.go — Updated existing tests and added coverage:

• Existing volume mount tests now use t.TempDir() for real source paths instead of hardcoded /host paths
• Added test case for non-existent source path (expects error)

Approach

Implemented Option 1 from the issue (validate in processVolumeMounts()) because:

1. It fails at the earliest possible point (during config building)
2. It prevents invalid configs from being created
3. It's context-aware (skips for K8s operator)
4. It aligns with the existing validation pattern in config_builder.go

Before / After

Before:

$ thv run --volume /nonexistent/path:/app playwright
MCP server is running in the background (PID: 12345)
# Silently fails — error only in log file

After:

$ thv run --volume /nonexistent/path:/app playwright
Error: volume mount source path does not exist: /nonexistent/path

[JAORMX]

Thanks for the contribution! The overall approach is solid and correctly follows Option 1 from the issue. The operator/CLI context split is handled well.

Two must-fix items before this can merge:

1. Incorrect resource URI check: strings.HasPrefix(source, "resource://") will not match actual resource URIs like volume://mydata. MountDeclaration already provides IsResourceURI() which handles all URI schemes correctly. Use !mount.IsResourceURI() instead.

2. Misleading error on non-ENOENT failures: If os.Stat fails for a reason other than "not exists" (e.g., permission denied), the error still says "does not exist". Use os.IsNotExist(err) to distinguish the two cases, as the issue itself suggested.

Additionally, a couple of should-fix items around error message clarity (showing resolved absolute paths for relative inputs) and missing test coverage for the operator context and resource URI skip paths. See inline comments for details.

[JAORMX]

Must-fix: The resource:// prefix check is incorrect. MountDeclaration.Parse() returns source as scheme + "://" + resource — where scheme is the actual URI scheme (e.g., volume, tmpfs), not the literal string resource. So a mount like volume://mydata:/app returns source = "volume://mydata", which does not start with resource:// and would incorrectly be stat'd on the filesystem.

MountDeclaration already has an IsResourceURI() method that handles this correctly. Suggested fix:

Suggested change

	// Validate source path exists on the host filesystem (CLI context only).
	if b.buildContext != BuildContextOperator && !mount.IsResourceURI() {

This also makes the code self-documenting — it reads as "skip if this is a resource URI" rather than checking a raw string prefix.

[JAORMX]

Must-fix: All os.Stat errors are reported as "does not exist", but the path could exist and be inaccessible (e.g., permission denied). This would send users debugging in the wrong direction.

The issue itself suggested the correct pattern. Distinguish os.IsNotExist from other errors:

Suggested change

	return fmt.Errorf("failed to resolve volume mount source path %q: %w", source, err)
	}
	}
	if _, err := os.Stat(absSource); err != nil {
	if os.IsNotExist(err) {
	return fmt.Errorf("volume mount source path does not exist: %s", absSource)
	}
	return fmt.Errorf("failed to access volume mount source path %s: %w", absSource, err)
	}

Note: this also switches to absSource in the "does not exist" message (see next comment).

[JAORMX]

Should-fix: When a relative path is used (e.g., ./data:/app), the error shows the original relative path (./data) rather than the resolved absolute path. If the user's working directory isn't what they expect, this makes debugging harder.

Consider showing absSource instead of source in the error message so the user can see exactly which path was checked.

[JAORMX]

Should-fix: A couple of test cases are missing that would strengthen confidence in the guard conditions:

1. Operator context with non-existent path: A test using NewOperatorRunConfigBuilder with WithVolumes([]string{"/nonexistent/path:/container"}) that asserts no error. This proves the BuildContextOperator skip actually works. Without this, a regression that removes the operator guard would go undetected.

2. Resource URI skips validation: A test with a resource URI volume (e.g., volume://mydata:/app) that asserts no error. This would have caught the resource:// prefix bug noted in the other comment.