Skip to content

fix(proxy): put the mitmproxy cache warm on the shared retry window - #912

Open
tend-agent wants to merge 1 commit into
fix/issue-909from
fix/issue-911
Open

fix(proxy): put the mitmproxy cache warm on the shared retry window#912
tend-agent wants to merge 1 commit into
fix/issue-909from
fix/issue-911

Conversation

@tend-agent

Copy link
Copy Markdown
Collaborator

Problem

lib/retry.sh covers three pre-agent third-party reaches. There is a fourth: proxy/setup-sandbox.sh warms the uvx cache before backgrounding the proxy, and on a cache miss that resolves and downloads mitmproxy from PyPI. The script runs under set -euo pipefail, and its step — Set up credential-isolation sandbox — sits three steps ahead of Run Claude, so a PyPI blip there loses the whole run having done none of the work the trigger asked for.

The miss is scheduled rather than rare: the actions/cache key is version-scoped (tend-mitmproxy-${{ runner.os }}-${{ inputs.mitmproxy_version }}), so every mitmproxy_version bump misses on each workflow's first run — which is exactly when a lost run is most expensive, since it reddens the weekly bump PR that carries the security-relevant pin.

Solution

Source the shared lib from setup-sandbox.sh and wrap the warm in retry_install. The source sits up top, next to the env checks, so a wrong path fails before the user creation and the workspace chown rather than mid-setup.

Stacked on #910 — the lib it sources lands there.

The two questions #911 left open

Is 60s enough for a cold mitmproxy resolve? Yes, with room to spare. The ~20s resolve figure in claude/action.yaml was undated and is now wrong. Measured on a GitHub-hosted runner (this triage job), 2026-08-09, with uv 0.12.3 and the pinned mitmproxy==12.2.3:

elapsed
cold UV_CACHE_DIR (43 packages, ~100MB downloaded) 2.28 / 2.26 / 2.27 / 2.24 s
warmed cache 0.516 / 0.514 / 0.515 s

So no third TIMEOUT parameter is needed — the case #909 raised and #910 declined stays declined. This PR also corrects the stale figure in the action comment, keeping the cache's real justification (not re-pulling ~100MB from PyPI on every run) rather than the seconds.

Does the backgrounded mitmdump need its own retry? No — retrying the warm already covers it. The launch shares UV_CACHE_DIR, and a cache warmed by this line serves the same --from invocation with no network at all:

$ UV_CACHE_DIR=$D uvx --from 'mitmproxy==12.2.3' mitmdump --version   # warm
$ UV_CACHE_DIR=$D uvx --offline --from 'mitmproxy==12.2.3' mitmdump --version
Mitmproxy: 12.2.3

Once the warm succeeds, PyPI is off the launch path, so the nohup-plus-readiness-wait shape never has to be retried. That's recorded in the comment at the warm.

Testing

Two tests in generator/tests/test_shared_steps.py, both failing before the change:

  • test_setup_sandbox_resolves_the_shared_retry_window runs the script's own . "${ACTION_PATH}/…" line (matched by content, not position) with ACTION_PATH set the way the action sets it, and asserts retry_install becomes a defined function. A path that doesn't resolve wouldn't degrade to an unretried warm — under set -euo pipefail it loses every run, not just the ones that hit a blip.
  • test_setup_sandbox_warms_the_mitmproxy_cache_through_the_window asserts the warm is an argument to retry_install rather than a bare command.

The second is a call-site assertion rather than the behavioural suite the three installers get. setup-sandbox.sh creates the sandbox user, chowns the workspace and installs a CA before reaching the warm, so faking a path to that line would assert more about the fakes than about the script; what the window itself does is already covered by the installer suites. Flagging the asymmetry rather than hiding it.

pre-commit run (shellcheck, ruff, actionlint) and the full 393-test generator suite pass.


Closes #911 — automated triage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant