Harden perf-smoke seeder mount permissions

[Neil4561]

Summary

• write benchmark output inside the container and copy normalized artifacts back to the runner
• isolate writable JIT and Kit caches per run/task/backend while preserving reuse across samples
• use disposable source clones and clean all run-scoped directories to avoid stale ownership on long-lived runners

Test plan

• Run the complete perf-smoke unit suite (52 passed, 1 skipped)
• Run pre-commit hooks on both changed files
• Verify cache reuse, artifact extraction, source reuse, and cleanup with UID-1000 Docker containers

[greptile-apps]

Greptile Summary

This PR hardens the perf-smoke seeder's container interaction by writing benchmark output inside the container and copying it out via docker cp rather than bind-mounting the host artifact directory, which avoids UID-mismatch permission failures from UID-1000 Docker containers.

• Removes --rm from docker run and replaces it with an explicit docker rm -f after artifact extraction, so the container stays alive long enough for docker cp to read its /tmp/bench_out.
• Isolates Kit caches per run/task/backend (mirroring the existing JIT cache isolation) and registers both cache roots and disposable source-clone directories for atexit cleanup, preventing stale ownership on long-lived runners.
• Renames _jit_cache_path → _cache_bucket_path and _cleanup_jit_cache → _cleanup_run_dir to reflect the broader scope of both helpers, with corresponding test updates and two new tests for Kit cache isolation and the docker cp flow.

Confidence Score: 4/5

The PR is safe to merge; the core artifact-extraction logic is well-tested and the permission-isolation strategy is sound.

The artifact-extraction logic is well-tested and the permission-isolation strategy is sound. The only finding is that copy_result is assigned inside the try block but read in the return statement outside it — static analysers flag this, and a future try/except wrapper could expose a genuine NameError. Everything else — cache isolation, atexit cleanup, container lifecycle, docker cp semantics, and test coverage — looks correct.

tools/perf_smoke_test/seed_baselines.py around the try/finally block in _docker_run_benchmark (lines 476-489).

Important Files Changed

Filename	Overview
tools/perf_smoke_test/seed_baselines.py	Rewires artifact extraction from a bind-mount to docker cp + explicit docker rm -f; isolates Kit cache per run/task/backend; makes source-clone directories uniquely disposable. One static-analysis concern: copy_result is assigned inside try but referenced in the return outside it.
tools/perf_smoke_test/test/test_seed_ancestry.py	Adds tests for unique/disposable seed-source directories, Kit cache isolation, renamed cleanup helper, and the new docker cp artifact extraction flow; test coverage aligns well with the implementation changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Main
    participant Docker
    participant Container
    participant Host

    Main->>Docker: "docker rm -f container (pre-flight)"
    Main->>Docker: "docker run --name container (no --rm)"
    Note over Container: "umask 000, mkdir -p /tmp/bench_out, run benchmark"
    Docker-->>Main: "exit code"
    Main->>Docker: "docker cp container:/tmp/bench_out/. artifact_dir"
    Docker-->>Host: "copy files"
    Main->>Host: "chmod -R a+rwX artifact_dir"
    Main->>Docker: "docker rm -f container"
    Note over Main: "return composite exit code"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Main
    participant Docker
    participant Container
    participant Host

    Main->>Docker: "docker rm -f container (pre-flight)"
    Main->>Docker: "docker run --name container (no --rm)"
    Note over Container: "umask 000, mkdir -p /tmp/bench_out, run benchmark"
    Docker-->>Main: "exit code"
    Main->>Docker: "docker cp container:/tmp/bench_out/. artifact_dir"
    Docker-->>Host: "copy files"
    Main->>Host: "chmod -R a+rwX artifact_dir"
    Main->>Docker: "docker rm -f container"
    Note over Main: "return composite exit code"

Loading

_{Reviews (1): Last reviewed commit: "Harden perf-smoke seeder mount permissio..." | Re-trigger Greptile}

[greptile-apps]

copy_result referenced outside the try block it is assigned in

copy_result is assigned only inside the try block (line 478) but read on the return at line 489, which is outside the try/finally. At runtime this is safe: any OS-level exception from subprocess.run(["docker", "cp", ...]) propagates through finally and exits the function without reaching the return. However, static-analysis tools (mypy, ruff) flag this as a potentially-unbound reference, and a future maintainer who wraps the outer body in a try/except could trigger a genuine NameError. Initializing copy_result = subprocess.CompletedProcess([], returncode=1) (or similar sentinel) before the try block would make the intent explicit and satisfy static analysis without changing runtime behaviour.

[Neil4561]

@fatimaanes

[fatimaanes]

LGTM — solid fix for the L40S seeder output/cache permission blocker. Verified the docker cp + host chmod path is sound (CI image runs as USER isaaclab uid 1000 = runner uid) and per-bucket Kit cache isolation is the right call for the camera-task crash. Approving.