fix: restore the pinned policy invariants path fallback - #24
Merged
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_785f6b1d-428f-47a7-8ead-fb8c7e7a4e80) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe policy consumer now retrieves invariant captures from current and legacy commit-pinned paths. It falls back after HTTP 404 responses and propagates other capture errors. Tests cover URL selection, fallback, and error handling. ChangesPolicy invariant fallback
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
6 tasks
loadinglucian
added a commit
that referenced
this pull request
Aug 2, 2026
## Summary PR #24 restored the invariants path fallback after it turned out to be load-bearing, but left a docstring promising its removal once a commit touching `support-policy.json` landed after the rename. That removal condition is wrong, so this replaces it. `support-policy.json` carries `policyInvariantsDigest`, and `consumer.py` rejects a capture where the captured policy does not match the captured invariants. The two documents only agree inside the tree php-bin reviewed them in, which is exactly why the capture pins to the commit that last changed the policy: at that commit php-bin's own validation had just checked one against the other. That makes the pin correct rather than under-specified, and rules out the obvious alternatives: - Pinning to whichever of the two documents changed last would pair new invariants with a policy still carrying the previous digest, and the capture would fail on every invariants-only change. - Advancing the pin with a fresh `support-policy.json` commit would need a real edit, but every field in that file is a semantic claim and its digest already matches. There is no honest change to make, and an empty commit is not one. So the pinned commit can be arbitrarily old, and the invariants path is whatever php-bin's layout was at the time. The two-URL list is permanent compatibility with historical layouts rather than a pending cleanup, and the next path move should extend it. The docstring now says that and explains why the alternatives do not work, so the reasoning sits with the code. Docstring and one test comment only. No executable change. ## Verification - [x] `scripts/test.sh` - [x] Unsupported-target behavior remains explicit - [x] Release selection and checksum behavior are covered by tests - [x] The `php-bin` release contract remains compatible `scripts/test.sh` passes locally at 9 tests plus the plugin contract, and `Plugin contract` is green on this PR. The remaining three items are unaffected by construction: the diff changes a docstring and a comment, so no target check, release selection, checksum path, or contract surface is touched. `git diff` on this branch is confined to the docstring body in `fetch_first_url` and two comment lines in `test/test_autorelease.py`. ## Security - [x] Downloads still require an exact SHA-256 entry - [x] No credentials, generated fixtures, or install directories are committed Both hold for the same reason: there is no executable change. The checksum requirement lives in the install path, which this PR does not touch, and the diff adds no files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #23 removed the policy invariants path fallback on the understanding that it was dead code. It was not, and the consumer workflow failed on main straight after the merge:
Policy captures do not read php-bin main.
POLICY_COMMIT_SELECTOR_URLselects the most recent commit that touchedsupport-policy.json, and the capture pins to that exact commit so the policy and its invariants always come from one reviewed state. That commit is still2a9a100cfrom 2026-07-27, which predates the maintenance to autorelease rename, so the invariants it publishes are atmaintenance/policy-invariants.json. The rename never touchedsupport-policy.json, so the selector never moved forward.This restores
fetch_first_urland the two pinned invariants URLs. Behavior is identical to the state before #23. The only substantive change is the docstring, which now records the real condition for removing the fallback: a commit that touchessupport-policy.jsonlanding after the rename, rather than php-bin main merging its rename, which already happened.The two operator control fallbacks that #23 also removed stay removed. Both read at php-bin main's tip, so they were genuinely unreachable.
Summary by CodeRabbit