chore(upstream): bump yt-dlp pin + rescan strings#3
Conversation
github-actions
Bot
force-pushed
the
chore/upstream-scan
branch
from
880bbb6 to
4ae7d38
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1).github/workflows/**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughWorkflow now resolves the latest yt-dlp tag to its underlying commit SHA; the scan script treats the known-strings file as a curated snapshot and writes expanded metadata (schemaVersion, ytDlpVersion, ytDlpCommit, note) while reusing prior strings when present. Changesyt-dlp versioning and string snapshot management
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/upstream-scan.yml:
- Around line 34-38: The script currently assigns commit from tag_ref using jq
-r which yields the literal "null" with exit code 0 if .object.sha/.object.type
is missing; update the dereference logic around the tag_ref and commit
assignments (the tag_ref and commit variables and the two curl/jq calls) to fail
fast by: use jq -e (or test the jq output) so missing fields cause a non-zero
status, validate that the extracted commit is non-empty and not the string
"null" after each jq call, and if validation fails log an error and exit
non‑zero instead of continuing to write a bad commit into
data/yt-dlp-version.json; apply the same check after the tag object dereference
branch that fetches the annotated tag object.
In `@scripts/scan-yt-dlp-source.mjs`:
- Around line 151-163: prior.strings is currently returned wholesale which
prevents merging new rawHits and removing deleted fragments; change the logic
that builds strings so it merges rawHits with prior metadata by computing the
same id = sha1(`${hit.fragment}::${hit.call}`) for each raw hit, build a map of
raw hits, then for each raw id produce an entry that takes curated fields (e.g.,
notes, kind) from prior.strings if present while updating source/call/fragment
from the raw hit (new hits should get kind: null), and drop any prior entries
whose id is not present in rawHits; keep the final strings array sorted by id
(preserving the existing [...byHash.values()].sort(...)) and replace the current
early-return that just uses prior.strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60d3237f-e671-4f4c-92ff-adf5a6f48acb
📒 Files selected for processing (2)
.github/workflows/upstream-scan.ymlscripts/scan-yt-dlp-source.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/**
⚙️ CodeRabbit configuration file
Review GitHub Actions for least-privilege permissions, unsafe secret exposure, untrusted PR execution, shell quoting, release/tag correctness, npm trusted publishing assumptions, artifact handling, and commands that can fail silently.
Files:
.github/workflows/upstream-scan.yml
scripts/**
⚙️ CodeRabbit configuration file
Review Node scripts for unsafe command construction, stale temporary directories, path traversal, network/release API assumptions, missing error handling, platform-specific behavior, and reproducibility in CI.
Files:
scripts/scan-yt-dlp-source.mjs
🔇 Additional comments (1)
scripts/scan-yt-dlp-source.mjs (1)
44-45: LGTM!Also applies to: 168-174
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/upstream-scan.yml:
- Around line 40-46: The tag dereference logic currently only checks for
empty/null SHAs (variables tag_type and commit) but must also verify the Git
object type is a commit; update the logic that processes the tag ref target (the
response used to set tag_type/commit) to extract and assert the returned
object.type is "commit" (error out if not), and likewise after fetching
tag_object for annotated tags ensure jq checks '.object.type' == "commit" before
accepting '.object.sha'; reference the variables/objects tag_type, tag_object,
and commit and use jq to validate object.type explicitly so non-commit targets
are rejected with a clear error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd7d843e-903d-4b80-8b2e-107361762b54
📒 Files selected for processing (1)
.github/workflows/upstream-scan.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
⚙️ CodeRabbit configuration file
Review GitHub Actions for least-privilege permissions, unsafe secret exposure, untrusted PR execution, shell quoting, release/tag correctness, npm trusted publishing assumptions, artifact handling, and commands that can fail silently.
Files:
.github/workflows/upstream-scan.yml
🔇 Additional comments (1)
.github/workflows/upstream-scan.yml (1)
35-39: LGTM!
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/upstream-scan.yml (1)
59-62:⚠️ Potential issue | 🟠 MajorEnsure
pull_requestCI runs automatically for the PR created byupstream-scan.yml
upstream-scan.ymlopens the PR withpeter-evans/create-pull-request@v8usingtoken: ${{ secrets.GITHUB_TOKEN }}(lines 59-62). PRs created/updated with the defaultGITHUB_TOKENputpull_requestworkflow runs into an approval-required/manual-approval state rather than running automatically—sotest.yml’spull_requestchecks may not execute until someone approves, undermining “Tests must pass before merge” unless you rely on additional branch protection/manual gating.Use a GitHub App token or PAT (instead of
GITHUB_TOKEN) forcreate-pull-request, or run the tests insideupstream-scan.ymlbefore opening the PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/upstream-scan.yml around lines 59 - 62, The PR created by peter-evans/create-pull-request@v8 in upstream-scan.yml currently uses token: ${{ secrets.GITHUB_TOKEN }}, which causes pull_request workflows (test.yml) to require manual approval; replace the token with a non-GITHUB_TOKEN credential (e.g., token: ${{ secrets.PERSONAL_ACCESS_TOKEN }} or a GitHub App token stored in secrets) so the created PR triggers pull_request checks automatically, or alternatively modify upstream-scan.yml to run the required tests inline before calling the create-pull-request step; update the token input for the create-pull-request action (and ensure the new secret exists) or add the test steps prior to invoking create-pull-request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/upstream-scan.yml:
- Around line 59-62: The PR created by peter-evans/create-pull-request@v8 in
upstream-scan.yml currently uses token: ${{ secrets.GITHUB_TOKEN }}, which
causes pull_request workflows (test.yml) to require manual approval; replace the
token with a non-GITHUB_TOKEN credential (e.g., token: ${{
secrets.PERSONAL_ACCESS_TOKEN }} or a GitHub App token stored in secrets) so the
created PR triggers pull_request checks automatically, or alternatively modify
upstream-scan.yml to run the required tests inline before calling the
create-pull-request step; update the token input for the create-pull-request
action (and ensure the new secret exists) or add the test steps prior to
invoking create-pull-request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f6da5c8-7773-4283-b8a8-b842d7d8ab91
📒 Files selected for processing (1)
.github/workflows/upstream-scan.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
⚙️ CodeRabbit configuration file
Review GitHub Actions for least-privilege permissions, unsafe secret exposure, untrusted PR execution, shell quoting, release/tag correctness, npm trusted publishing assumptions, artifact handling, and commands that can fail silently.
Files:
.github/workflows/upstream-scan.yml
🔇 Additional comments (2)
.github/workflows/upstream-scan.yml (2)
40-50: LGTM!
34-36: ⚡ Quick winClarify the REST API “array response” concern: it’s not
/git/ref/*, it’s/git/matching-refs/*GitHub’s REST docs specify that
GET /git/ref/{ref}returns a single ref only when{ref}is an exact match (otherwise it returns 404); prefix/starts-with “array of matches” behavior is forGET /git/matching-refs/{ref}, not forgit/ref. So changing togit/ref/tags/${latest}doesn’t directly address the claimed “prefix tag → array → jq fails” scenario—either${latest}isn’t the exact ref name you think it is, or the failure is due to a different unexpected response/404 path.Also, the operational note about
create-pull-request+secrets.GITHUB_TOKENnot triggering downstream workflow runs should be validated against the repo’s actual trigger configuration (e.g.,pull_requestvsworkflow_run, and any token/permission conditions).
|
CodeRabbit follow-up:
|
1 participant
Automated weekly scan picked up changes from the current
data/yt-dlp-version.jsonpin.Review:
data/known-yt-dlp-strings.jsondiff — new entries land withkind: null. Assign a kind (or leave null intentionally + document why innotes).src/patterns.tsand add a fixture undertests/fixtures/yt-dlp-stderr/<kind>/.upstream-coverage.test.tsenforces non-null kinds round-trip correctly.Changes
Workflow (.github/workflows/upstream-scan.yml)
scripts/scan-yt-dlp-source.mjs
Public API / package behavior
Internal classifier / regex / snapshot / metadata changes
Risk areas
Tests / checks to run before merge