[codex] harden dependency pin guard - #292
Conversation
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dc0f4d915
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| path.write_text(updated, encoding="utf-8") | ||
|
|
||
| for repo, matches in sorted(qsl_refs.items()): | ||
| refs = sorted({ref for _, _, ref in matches}) |
There was a problem hiding this comment.
When both dependency files use the same mutable ref such as @main for a QuantStrategyLab repo, refs has length 1, so this branch treats it as consistent and returns success. Because QSL_REF_RE accepts non-SHA refs, the now-blocking CI guard can pass with unpinned internal dependencies, letting Docker/pip resolve moving code at install time instead of enforcing the SHA pins this guard is meant to protect. Please validate each matched ref is a 40-character commit SHA before accepting the single-ref case.
Useful? React with 👍 / 👎.
| for line_no, line in enumerate(content.splitlines(), start=1): | ||
| for repo, ref in QSL_REF_RE.findall(line): | ||
| qsl_refs.setdefault(repo, []).append((path, line_no, ref)) |
There was a problem hiding this comment.
When a requirements or constraints file keeps a commented rollback/example line containing an old QuantStrategyLab GitHub URL, this loop still records it in qsl_refs; with the guard now blocking in CI, an ignored comment can fail every PR even though pip would not install it. Please skip comment-only lines before applying QSL_REF_RE so only active dependency declarations participate in the consistency check.
Useful? React with 👍 / 👎.
|
|
||
| QPK_PIN_URL = "https://raw.githubusercontent.com/QuantStrategyLab/QuantPlatformKit/main/QPK_PIN" | ||
| QPK_REF_RE = re.compile(r"QuantPlatformKit\.git@([a-f0-9]{40})") | ||
| QSL_REF_RE = re.compile(r"github\.com/QuantStrategyLab/([A-Za-z0-9_.-]+)\.git@([A-Za-z0-9._/-]+)") |
There was a problem hiding this comment.
pip's VCS documentation lists supported direct Git forms such as MyProject @ git+https://git.example.com/MyProject and allows a git ref on that URL, but this regex only matches URLs with .git@. If either requirements or constraints uses the valid suffixless GitHub form, the guard records only the other side (or neither side) and can pass despite ref drift, leaving the Docker pip install -r requirements.txt -c constraints.txt conflict this check is meant to prevent. Please make the .git suffix optional when matching QuantStrategyLab GitHub dependencies.
Useful? React with 👍 / 👎.
1 participant
Summary
constraints*.txtin the scanned dependency filescontinue-on-errorWhy
A deployment can fail when
requirements.txtandconstraints.txtcarry different git refs for the same internal package. This hardens CI so that drift is caught before Docker/Cloud Run deploy.Validation
python3 scripts/check_qpk_pin_consistency.pypython3 -m pytest -q tests/test_dependency_pin_guard.pyactionlint .github/workflows/ci.ymlgit diff --check