Skip to content

fix(smoke): publish cassettes 0644 so the nightly refresh can commit them - #1118

Closed
aminsamir45 wants to merge 1 commit into
mainfrom
fix/smoke-cassette-key-determinism
Closed

fix(smoke): publish cassettes 0644 so the nightly refresh can commit them#1118
aminsamir45 wants to merge 1 commit into
mainfrom
fix/smoke-cassette-key-determinism

Conversation

@aminsamir45

@aminsamir45 aminsamir45 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

The nightly cassette refresh has failed every night since it was added — every scheduled run back to 2026-07-19 is red. save() published cassettes with CreateTemp's 0600, owned by the container's root, into a directory bind-mounted from the repo. The runner's git then could not read what the recording had just written:

error: open("smoke/mitmproxy/cassettes/0feb08ea….json"): Permission denied
fatal: updating files failed          → exit 128

The step dies before gh pr create, so no refresh PR has ever opened. That's why the committed cassettes are still the original 2026-07-23 recording and a month of provider drift has gone undetected. This chmods 0644 before the atomic rename.

Relationship to #1117

This PR originally also fixed the cassette-key determinism bug that was turning every replay-only PR run red (cassette: cache miss: POST /v1/responses on TestOpenAIResponsesAPI, caused by #1101 injecting a per-run prompt_cache_key). #1117 landed that fix ~10 minutes earlier, so I've rebased onto it and dropped my duplicate — this is now only the remaining half.

Worth stating explicitly, since the two bugs look redundant but aren't: #1117 stops the bleeding on PR runs, and this is the reason that bug could never have healed on its own. A stale-cassette problem is supposed to be caught and auto-corrected by the nightly refresh; with the refresh silently dead, the next drift would have gone unnoticed the same way.

Verification

  • New test fails against pre-fix code with the real symptom (got 600 want 644) and passes after.
  • Full smoke suite green on this branch against the committed cassettes: ok workweave/router/smoke, all six tests including TestOpenAIResponsesAPI.
  • gofmt, go vet ./smoke/..., and go test ./smoke/mitmproxy/ clean.

The mode can't be verified end-to-end from a PR run (it needs a record-mode run with a key), so the test pins it directly. Follow-up worth watching: confirm the next nightly actually opens its refresh PR — and expect that PR to carry a month of accumulated drift, so review it rather than rubber-stamping.

🤖 Generated with Weave Router

…y refresh

Every replay-only smoke run has failed since #1101 merged, on PRs that
change nothing about the request path. Two independent bugs.

Cassette keys were not run-stable. #1101 began injecting prompt_cache_key
into the upstream body as a session-affinity hint. It is derived from the
API key id, and run.sh seeds a brand-new router key on every run, so the
key hashed differently each time — a guaranteed
`cassette: cache miss: POST /v1/responses` on TestOpenAIResponsesAPI. The
observed per-branch keys were the symptom: same scenario, different hash
each run. requestKey now canonicalizes the body, stripping the per-run
router-derived fields listed in volatileBodyFields.

The nightly cassette refresh has been red for a month and would not have
healed this on its own. save() published cassettes via CreateTemp's 0600,
owned by the container's root, into a repo bind mount — so the runner's
git could not read what it had just recorded and died with
"Permission denied" (exit 128) before opening a refresh PR. Cassettes are
now chmod 0644 before the atomic rename.

Tests cover both halves of the key contract — one scenario hashes the same
across runs, genuinely different requests still do not collide — and pin
the published file mode. Verified end to end: the full suite reproduces the
CI failure on the unfixed store and passes with it.

Co-Authored-By: Weave Router <router@workweave.ai>
@aminsamir45 aminsamir45 changed the title fix(smoke): make cassette keys stable across runs; unbreak the nightly refresh fix(smoke): publish cassettes 0644 so the nightly refresh can commit them Aug 28, 2026
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for the posted P1 finding in the PR review.
  • T-Rex prepared a focused legacy cassette compatibility test source to support the proof.
  • T-Rex generated a focused replay-only compatibility test output to validate the proof.
  • T-Rex captured a PR request-key regression suite output to cross-check the proof against related changes.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix(smoke): make cassette keys stable ac..." | Re-trigger Greptile

Comment thread smoke/mitmproxy/store.go
h.Write([]byte(path))
h.Write([]byte{0})
h.Write(body)
h.Write(canonicalizeBody(body))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Legacy cassette keys are unreachable

Canonicalizing prompt_cache_key changes the hash—and therefore filename—for every existing JSON cassette recorded with that field. Replay-only lookup uses only this new key, so a cassette stored under its prior raw-body hash now produces a cache miss until it is rerecorded. Rename or migrate affected cassettes in this change, or fall back to the legacy raw-body key after a canonical-key miss until the migration is complete.

Artifacts

Focused legacy cassette compatibility test source

Focused replay-only compatibility test output

  • This executed test output shows distinct old and new hashes and the observed replay-only cache miss despite the old-key cassette being readable.

PR request-key regression suite output

  • This executed regression suite shows the PR’s new-key stability and cassette-mode tests pass while not covering legacy filename compatibility.

View artifacts

T-Rex Ran code and verified through T-Rex

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