Repository navigation
Conversation
https://miniupnp.tuxfamily.org does not answer any more, and the dash-depends-sources fallback bucket has no copy of miniupnpc-2.2.7.tar.gz, so `make -C depends download` fails and with it every CI job, since all build jobs wait for the "Cache depends sources" job. Point the package at the author's other mirror, http://miniupnp.free.fr, which serves the tarball with the expected hash. This is what Bitcoin Core did in bitcoin#30151 for the same outage but it had been reverted later by bitcoin core by mistake and never refixed, instead bitcoin core dropped miniupnpc altogether (bitcoin#31130), so there is no later upstream fix to backport.
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. WalkthroughThe miniupnpc download base URL changes from Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk:
|
|
⛔ Final review complete — 1 blocking finding(s) (commit abb199a) · triage: low |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The dependency mirror change is technically correct and preserves the existing filename and SHA-256 verification; the available depends and source-build checks are successful, with remaining source tests still in progress. However, the PR title uses the disallowed build scope, causing the mandatory title-validation check to fail, and the dependency documentation still points users to the unavailable mirror.
🔴 1 blocking | 🟡 1 suggestion(s)
2 finding(s) not shown inline (the lines are not part of this PR's diff)
🔴 Blocking: PR title scope build is not allowed by the mandatory title validator
.github/workflows/semantic-pull-request.yml:43-55
The PR title is fix(build): fetch miniupnpc sources from miniupnp.free.fr, but this workflow allows build only as a type (line 34), not as a scope. The configured semantic-pull-request action therefore rejects the title, as confirmed by the failed Validate PR title check and its unknown-scope annotation. Retitle the PR without the scope, for example build: fetch miniupnpc sources from miniupnp.free.fr, or update the workflow's allowed scopes before merging.
source: glm-5.3-flash (phase1-reviewer: general)
🟡 Suggestion: Update the documented MiniUPnPc mirror
doc/dependencies.md:43
The build configuration now fetches MiniUPnPc from http://miniupnp.free.fr/files/, while the dependency table still directs users to https://miniupnp.tuxfamily.org/, the unavailable mirror this PR is fixing. Keeping the documentation on the dead host leaves the documented dependency source inconsistent with the working build configuration.
| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](http://miniupnp.free.fr/) | [2.2.7](https://github.com/bitcoin/bitcoin/pull/29707) | 2.1 | No |
source: glm-5.3-flash (phase1-reviewer: general); gpt-6.1-sol (phase2-reviewer: general, dash-core-commit-history)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — This is a small, self-contained build dependency URL change in depends/packages/miniupnpc.mk whose correctness can be readily confirmed by fetching the tarball and verifying its expected hash. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 98% left, weekly 91% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions are optional: fix them forward in a later PR rather than pushing another round just for them.
- Address every current finding in one push: self-review the full diff first, and don't push piecemeal.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `.github/workflows/semantic-pull-request.yml`:
- [BLOCKING] .github/workflows/semantic-pull-request.yml:43-55: PR title scope `build` is not allowed by the mandatory title validator
The PR title is `fix(build): fetch miniupnpc sources from miniupnp.free.fr`, but this workflow allows `build` only as a type (line 34), not as a scope. The configured semantic-pull-request action therefore rejects the title, as confirmed by the failed `Validate PR title` check and its unknown-scope annotation. Retitle the PR without the scope, for example `build: fetch miniupnpc sources from miniupnp.free.fr`, or update the workflow's allowed scopes before merging.
In `doc/dependencies.md`:
- [SUGGESTION] doc/dependencies.md:43: Update the documented MiniUPnPc mirror
The build configuration now fetches MiniUPnPc from `http://miniupnp.free.fr/files/`, while the dependency table still directs users to `https://miniupnp.tuxfamily.org/`, the unavailable mirror this PR is fixing. Keeping the documentation on the dead host leaves the documented dependency source inconsistent with the working build configuration.
Issue being fixed or feature implemented
So apparently build with depends without cache can't succeed anymore.
What was done?
Point the package at the author's other mirror, http://miniupnp.free.fr, which serves the tarball with the expected hash. This is what Bitcoin Core did in bitcoin#30151 for the same outage but it had been reverted later by bitcoin core by mistake and never refixed, instead bitcoin core dropped miniupnpc altogether (bitcoin#31130), so there is no later upstream fix to backport.
How Has This Been Tested?
Can be tested by CI on #7939 when rebased on top of merged this PR
Breaking Changes
N/A
Checklist: