fix: persist packaged SEA binaries during extraction

[abhinavkr26104]

Summary

• copy packaged SEA resources into the persistent cache while the importlib.resources.as_file() context is active
• return the cached path instead of an extraction path that may already have been removed
• add a regression test that simulates temporary resource cleanup on context exit

Testing

• uv run --frozen pytest tests/test_sea_binary.py -q (5 passed)
• uv run --frozen ruff check .
• uv run --frozen pyright -p .
• uv run --frozen mypy --platform linux .
• full Python 3.9 and 3.14 suites run on Windows; only the pre-existing Windows failures addressed by Make byte fixtures checkout-stable and run tests on Windows CI #355 remained

Fixes #354

---

Summary by cubic

Persist packaged SEA binaries during extraction so resolve_binary_path returns a stable, cached path. Previously it returned a temporary extraction path that could be deleted when the as_file context exited; now it copies to a versioned cache during that context and returns the cached path. Fixes #354.

• Persist the resource to the versioned cache inside the extraction context and return the cached path.
• Keep env override and source checkout fallbacks unchanged.
• Add a regression test that simulates cleanup on context exit.

^{Written for commit 64f0711. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

Confidence score: 5/5

• In tests/test_sea_binary.py, the defaulting test now stubs _resource_binary_path to return resource_path, so it no longer exercises the real cache-write path where _resource_binary_path calls _copy_to_cache; this can let cache-regression bugs slip through undetected—restore coverage of the actual cache-copy behavior (or add a focused test for _copy_to_cache invocation).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/test_sea_binary.py">

<violation number="1" location="tests/test_sea_binary.py:50">
P3: The defaulting test no longer exercises the cache write. It mocks `_resource_binary_path` to return `resource_path` and removed the `_copy_to_cache` mock, but in production `_resource_binary_path` now copies to cache internally and returns the versioned cache path, so the test asserts `resolved == resource_path` against behavior it no longer models. The new context-test covers `_copy_to_cache` but only with an explicit `version="test"`, leaving the default (package-version → versioned cache dir) flow without a direct assertion. Update the mock to return a versioned cache path, or add an assertion that the resolved path equals `_cache_dir() / __version__ / filename` under real copy semantics.</violation>
</file>

Architecture diagram

sequenceDiagram
    participant Client as Client Code
    participant RB as resolve_binary_path()
    participant RBPath as _resource_binary_path()
    participant IR as importlib.resources
    participant Cache as _copy_to_cache()
    participant FS as File System
    
    Note over Client,FS: SEA Binary Resolution Flow
    
    Client->>RB: resolve_binary_path()
    RB->>RB: Get default filename
    RB->>RB: Determine version (env override or package version)
    
    RB->>RBPath: _resource_binary_path(filename, version)
    RBPath->>IR: files("stagehand")
    IR->>RBPath: Package resource root
    RBPath->>IR: as_file(candidate)
    
    alt ZIP-backed resource (wheel install)
        IR->>RBPath: Extract to temp location
        RBPath->>Cache: _copy_to_cache(src=temp, filename, version)
        Cache->>FS: Copy binary to cache dir
        Cache-->>RBPath: Cached path
        Note over RBPath: Temp extraction cleaned up on context exit
        RBPath-->>RB: Cached stable path
    else Directory resource (source tree)
        IR-->>RBPath: Direct path to file
        RBPath->>Cache: _copy_to_cache(src=direct, filename, version)
        Cache-->>RBPath: Cached path
        RBPath-->>RB: Cached stable path
    end
    
    RB-->>Client: Return cached binary path
    
    Note over Client,FS: Key behavior: binary is persisted<br/>to cache BEFORE context exits,<br/>ensuring a stable path survives<br/>temporary resource cleanup

Loading

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The defaulting test no longer exercises the cache write. It mocks _resource_binary_path to return resource_path and removed the _copy_to_cache mock, but in production _resource_binary_path now copies to cache internally and returns the versioned cache path, so the test asserts resolved == resource_path against behavior it no longer models. The new context-test covers _copy_to_cache but only with an explicit version="test", leaving the default (package-version → versioned cache dir) flow without a direct assertion. Update the mock to return a versioned cache path, or add an assertion that the resolved path equals _cache_dir() / __version__ / filename under real copy semantics.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_sea_binary.py, line 50:

<comment>The defaulting test no longer exercises the cache write. It mocks `_resource_binary_path` to return `resource_path` and removed the `_copy_to_cache` mock, but in production `_resource_binary_path` now copies to cache internally and returns the versioned cache path, so the test asserts `resolved == resource_path` against behavior it no longer models. The new context-test covers `_copy_to_cache` but only with an explicit `version="test"`, leaving the default (package-version → versioned cache dir) flow without a direct assertion. Update the mock to return a versioned cache path, or add an assertion that the resolved path equals `_cache_dir() / __version__ / filename` under real copy semantics.</comment>

<file context>
@@ -37,27 +39,55 @@ def test_resolve_binary_path_defaults_cache_version_to_package_version(
-    assert resolved == tmp_path / "cache" / sea_binary.default_binary_filename()
-    assert captured["src"] == resource_path
-    assert captured["filename"] == sea_binary.default_binary_filename()
+    assert resolved == resource_path
     assert captured["version"] == __version__
 
</file context>