Skip to content

fix(knowledge): isolate build-yt-dlp-args tests from host cookie env - #3657

Merged
kyle-sexton merged 1 commit into
mainfrom
cursor/fix-3447-ytdlp-test-env-df93
Sep 2, 2026
Merged

fix(knowledge): isolate build-yt-dlp-args tests from host cookie env#3657
kyle-sexton merged 1 commit into
mainfrom
cursor/fix-3447-ytdlp-test-env-df93

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closes #3447

Summary

Six buildYtDlpArgs cases omitted env, so resolveYtDlpAuthArgs fell through to process.env. A runner with VIDEO_DIGEST_YT_DLP_COOKIES_FILE (or the legacy YOUTUBE_ spelling) exported then failed the positional and not.toContain assertions.

Fix

Pass env: {} in those six cases, matching adapter-argv-conformance.test.js. Add a case that exports a cookies file on process.env and asserts isolated argv does not contain --cookies.

knowledge 0.13.33 -> 0.13.34.

Verification

scripts/affected-tests.sh --run selected the Node suite (this runner reports NOT RUN for that ecosystem). Ran npx vitest run acquisition/build-yt-dlp-args.test.js with VIDEO_DIGEST_YT_DLP_COOKIES_FILE=/tmp/should-not-appear.txt exported: 16/16 passed.

Related

N/A

Open in Web Open in Cursor 

Six cases omitted env, so resolveYtDlpAuthArgs fell through to process.env
and a runner with VIDEO_DIGEST_YT_DLP_COOKIES_FILE exported failed the
positional assertions. Pass env: {} to match the conformance suite.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 45s —— View job


Security review — PR #3657

  • Gather context (PR diff, changed files)
  • Run /review:security-review skill
  • Post findings

Skip gate reached: no security-relevant surface.

This PR only touches test code and metadata:

  • build-yt-dlp-args.test.js — adds env: {} to six existing test cases and one new case asserting that a host-exported VIDEO_DIGEST_YT_DLP_COOKIES_FILE does not leak into isolated buildYtDlpArgs output. The new test correctly saves/restores process.env in a try/finally block, so it doesn't leave polluted state for other tests.
  • plugin.json — version bump (0.13.33 → 0.13.34)
  • CHANGELOG.md — changelog entry

No production/auth logic (resolveYtDlpAuthArgs, buildYtDlpArgs) is modified — this is purely a test-isolation fix that makes the existing suite deterministic regardless of the host runner's environment. No injection, authz, credential-handling, or Actions-workflow changes are present.

No security issues found.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Last security-reviewed head: 15bd5be7040445e7a5f03ee3af7c1989d42ca32e. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@kyle-sexton
kyle-sexton marked this pull request as ready for review September 2, 2026 10:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T10:03:40.863703Z 15bd5be Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kyle-sexton
kyle-sexton merged commit 3052c4b into main Sep 2, 2026
69 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/fix-3447-ytdlp-test-env-df93 branch September 2, 2026 10:01
@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 7s —— View job


Security review — PR #3657 (re-run)

  • Gather context (PR diff, changed files)
  • Run /review:security-review skill
  • Post findings

Skip gate reached: this head already has a successful security review that still applies.

HEAD SHA 15bd5be7040445e7a5f03ee3af7c1989d42ca32e is identical to the SHA already reviewed in the prior pass on this PR — no commits have landed since. That review covered the full diff (test-only change adding env: {} isolation to six buildYtDlpArgs cases plus one new leak-guard case in build-yt-dlp-args.test.js, plus a version bump and changelog entry) and found no security-relevant surface: no production/auth logic touched, no injection, authz, credential-handling, or Actions-workflow changes.

Re-confirmed via gh pr diff 3657 against the same diff — nothing new to report.

No security issues found.

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 26s —— View job


Code review — PR #3657

  • Gather context (PR diff, changed files, state)
  • Check skip gate
  • Post findings

Skip gate reached: PR is merged (not open).

Per the /review:code-review skill's skip gate: "PR is closed or not open" → stop early and post nothing else. This PR (#3657) already merged (head 15bd5be7040445e7a5f03ee3af7c1989d42ca32e), so there's no open PR to attach a review to. A prior security-review lane run also already completed on this same head with no issues found.

No code-review findings are being posted since the review lane doesn't apply to a merged PR.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

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.

knowledge: build-yt-dlp-args tests omit env, so real cookie env vars inject argv and break assertions

2 participants