Skip to content

fix(youtube): honor account auth for list reads#667

Merged
steipete merged 1 commit into
mainfrom
fix/youtube-oauth-list-account
May 30, 2026
Merged

fix(youtube): honor account auth for list reads#667
steipete merged 1 commit into
mainfrom
fix/youtube-oauth-list-account

Conversation

@steipete
Copy link
Copy Markdown
Collaborator

Summary

  • honor configured YouTube OAuth accounts for youtube videos list and youtube comments list
  • keep the existing API-key fallback when no account selector is supplied
  • route comments list through the Google-required youtube.force-ssl scope and surface a reauth hint when the stored token lacks it

Closes #664.

Tests

  • go test ./internal/cmd -run 'TestYouTube'
  • go test ./internal/cmd ./internal/googleapi -run 'TestYouTube'
  • make ci
  • autoreview: /Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode local --parallel-tests "make ci" clean

Live proof

  • clawdbot@gmail.com, with GOG_YOUTUBE_API_KEY=definitely-invalid: yt videos list --id dQw4w9WgXcQ --max 1 returned one video via OAuth.
  • clawdbot@gmail.com, comments path: reached OAuth and now returns the new youtube.force-ssl reauth hint because the stored token lacks that scope.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

Codex review: needs maintainer review before merge. Reviewed May 30, 2026, 3:43 PM ET / 19:43 UTC.

Summary
The branch routes youtube videos list and youtube comments list through account-aware OAuth helpers when an account selector is present, adds a comments youtube.force-ssl client and reauth hint, and adds focused command tests plus a changelog note.

Reproducibility: yes. Source inspection on current main shows both affected list commands call the API-key service unconditionally, so an account selector cannot be honored on those paths.

Review metrics: 2 noteworthy metrics.

  • Diff size: 5 files changed, +203/-5. The patch is small but crosses command routing, Google API auth construction, tests, and release notes.
  • Auth-routed commands: 2 command paths changed. Both changed user-facing commands now choose between API-key and OAuth clients based on account selection.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Maintainer should explicitly accept the account-selector precedence before merge.

Risk before merge

  • [P1] With --account, GOG_ACCOUNT, or a direct access token present, videos list and comments list will prefer OAuth over the API-key path; existing environments that relied on a global account setting plus an API key may now need YouTube OAuth setup or, for comments, a youtube.force-ssl reauth.

Maintainer options:

  1. Accept Account-Selector Precedence (recommended)
    Maintainers can land the PR if they agree that --account, GOG_ACCOUNT, and direct access tokens intentionally opt these reads into OAuth even when an API key exists.
  2. Add Fallback Regression Proof
    Before merge, the branch could add or confirm tests showing videos list and comments list still use the API key when no account selector is present.
  3. Narrow The Selector Surface
    If GOG_ACCOUNT switching is too broad for public reads, revise the helper to limit OAuth selection to the accepted selector set and document that choice.

Next step before merge

  • [P2] No repair lane is needed; maintainer review should decide whether to accept the auth-routing precedence and then merge or request a narrower selector behavior.

Security
Cleared: No supply-chain, dependency, workflow, secret-handling, or credential-leak concern was found; the OAuth scope change is tracked as functional merge risk.

Review details

Best possible solution:

Land a focused fix that uses OAuth only when an account selector is present, preserves no-selector API-key reads, and gives users a clear comments reauth path when the stored token lacks the required scope.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows both affected list commands call the API-key service unconditionally, so an account selector cannot be honored on those paths.

Is this the best way to solve the issue?

Yes. The PR uses existing account-selection helpers and adds a narrow comments-scope client; the remaining question is maintainer acceptance of the OAuth-over-API-key precedence when an account selector is present.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against cf9c5150c0a2.

Label changes

Label changes:

  • add P2: This is a normal-priority auth-provider bug fix limited to YouTube list reads.
  • add merge-risk: 🚨 compatibility: Existing API-key workflows with a global account selector can route differently after merge and may require OAuth setup.
  • add merge-risk: 🚨 auth-provider: The diff changes YouTube client selection and adds a comments-specific OAuth scope path.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this collaborator PR; the body still includes live OAuth proof for videos and the comments reauth hint.

Label justifications:

  • P2: This is a normal-priority auth-provider bug fix limited to YouTube list reads.
  • merge-risk: 🚨 compatibility: Existing API-key workflows with a global account selector can route differently after merge and may require OAuth setup.
  • merge-risk: 🚨 auth-provider: The diff changes YouTube client selection and adds a comments-specific OAuth scope path.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this collaborator PR; the body still includes live OAuth proof for videos and the comments reauth hint.
Evidence reviewed

What I checked:

  • Repository policy applied: AGENTS.md was read fully; its PR review-mode guidance required inspecting the PR without switching branches or changing code. (AGENTS.md:33, cf9c5150c0a2)
  • Current main reproduces source problem: On current main, YouTubeVideosListCmd.Run constructs the service with getYouTubeServiceWithAPIKey(ctx) unconditionally, so --account cannot affect video list reads. (internal/cmd/youtube.go:136, cf9c5150c0a2)
  • Current main comments path is also API-key-only: On current main, YouTubeCommentsListCmd.Run also calls getYouTubeServiceWithAPIKey(ctx) unconditionally before listing comment threads. (internal/cmd/youtube.go:292, cf9c5150c0a2)
  • PR implementation changes service routing: The PR commit changes videos to getYouTubeReadService, comments to getYouTubeCommentsService, and adds helpers that choose OAuth when an account selector or direct token is present. (internal/cmd/youtube.go:139, 3f9e7339e1c2)
  • PR adds scoped comments client: The PR adds NewYouTubeCommentsForAccount, which requests the comments-specific OAuth scope through the existing account-scoped client path. (internal/googleapi/youtube.go:68, 3f9e7339e1c2)
  • Regression tests cover OAuth selection: The PR adds tests for videos list --account, comments list --account, and videos list --account auto using OAuth services instead of the API-key service. (internal/cmd/youtube_test.go:108, 3f9e7339e1c2)

Likely related people:

  • steipete: Current main blame shows the YouTube command implementation in release commit b25a3c029b37d318b9b3db73ca6ed50d703e2de6 by Peter Steinberger, and this PR's commit updates the same command and Google API files. (role: current YouTube command author and recent area contributor; confidence: high; commits: b25a3c029b37, 3f9e7339e1c2; files: internal/cmd/youtube.go, internal/cmd/youtube_services.go, internal/googleapi/youtube.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels May 30, 2026
@steipete steipete merged commit 5e4daad into main May 30, 2026
9 checks passed
@steipete steipete deleted the fix/youtube-oauth-list-account branch May 30, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

youtube videos/comments list are API-key-only, don't honor --account (OAuth)

1 participant