From 823fe65b437982c8bf09405c4bee21297fa37014 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Thu, 3 Sep 2026 22:12:54 +0000 Subject: [PATCH] refactor: one canonical YouTube URL rule in server/lib/youtubeUrl.js (#6014) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Music Video track import rejected `music.youtube.com`, `/shorts/`, `/live/`, and `/embed/` links with `400 YOUTUBE_URL_INVALID` even though yt-dlp handles them and both the brain ingest and the Google Takeout importer already accepted them. The cause was drift: the same "is this one YouTube video, and which one?" question was answered by four separate copies of the regexes — an older one in `trackYoutubeImport.js` (duplicated again in `routes/tracks.js`) that predated `music.`/shorts/live/embed support, a newer one in `youtubeIngest.js`, the id parser buried in `youtubeImport.js`, and a fifth mirror on the client. Extract the rule to `server/lib/youtubeUrl.js` (`YOUTUBE_VIDEO_URL_RE`, `youtubeVideoIdFromUrl` / `youtubeVideoId`, `isYoutubeVideoUrl`, `assertYoutubeVideoUrl`, `YOUTUBE_URL_INVALID_MESSAGE`) and point every server caller at it. The existing service-level names (`YOUTUBE_INGEST_URL_RE`, `assertYoutubeIngestUrl`, `YOUTUBE_URL_RE`, `assertYoutubeUrl`) stay as aliases, so nothing downstream had to change. This also removes the layering violation the duplication forced: the id parser lived in a service, so `server/lib/youtubeUrl.mirror.test.js` had to import from `server/services/` — libraries reaching into services. Track import now accepts every URL shape the other two pipelines do, and the rejection message names all of them. Claude-Session: https://claude.ai/code/session_01VjkWVTfzKyRuAv3HEsspwN --- client/src/lib/youtubeUrl.js | 6 +- server/lib/README.md | 1 + server/lib/index.js | 1 + server/lib/youtubeUrl.js | 75 ++++++++++++++++++++++ server/lib/youtubeUrl.mirror.test.js | 14 ++-- server/routes/tracks.js | 5 +- server/routes/tracks.test.js | 14 +++- server/services/trackYoutubeImport.js | 28 ++++---- server/services/trackYoutubeImport.test.js | 29 +++++++++ server/services/youtubeImport.js | 20 ++---- server/services/youtubeIngest.js | 31 +++------ server/services/youtubeSync.js | 2 +- 12 files changed, 154 insertions(+), 72 deletions(-) create mode 100644 server/lib/youtubeUrl.js diff --git a/client/src/lib/youtubeUrl.js b/client/src/lib/youtubeUrl.js index 2323ef0202..f29d83430e 100644 --- a/client/src/lib/youtubeUrl.js +++ b/client/src/lib/youtubeUrl.js @@ -1,6 +1,6 @@ /** - * Single-video YouTube URL detection — MIRROR of `YOUTUBE_INGEST_URL_RE` in - * `server/services/youtubeIngest.js` (authoritative there). + * Single-video YouTube URL detection — MIRROR of `YOUTUBE_VIDEO_URL_RE` in + * `server/lib/youtubeUrl.js` (authoritative there). * * The Quick Capture box swaps its whole submit path (brain capture → YouTube * ingest) based on this predicate, and reveals the ingest options panel from it, @@ -15,7 +15,7 @@ const SINGLE_VIDEO_RE = /^https?:\/\/(www\.|m\.|music\.)?(youtube\.com\/(watch\?[^\s#]*\bv=[\w-]{6,}|shorts\/[\w-]{6,}|live\/[\w-]{6,}|embed\/[\w-]{6,})|youtu\.be\/[\w-]{6,})/i; -/** The video id in a YouTube URL, or null. Mirrors `youtubeVideoIdFromUrl` server-side. */ +/** The video id in a YouTube URL, or null. Mirrors `youtubeVideoIdFromUrl` in `server/lib/youtubeUrl.js`. */ export function youtubeVideoId(url) { if (!url) return null; const s = String(url).trim(); diff --git a/server/lib/README.md b/server/lib/README.md index a1256d9834..0d1d478eb3 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -305,6 +305,7 @@ pm` default, `NPM_CONFIG_PREFIX`, nvm/Volta) installed `codex` successfully and | `primaryCheckoutGuard.js` | `capturePrimaryCheckoutState(path)` / `detectPrimaryCheckoutDrift(baseline, {agentBranch})` + `PRIMARY_CHECKOUT_MUTATED_REASON`/`_CATEGORY` — the branch-jack detector (#3680): stamps the PRIMARY checkout's branch + HEAD onto a worktree agent's metadata at spawn (`agentLifecycle.js`) and re-reads it in the shared finalize path (`agentFinalization.js`), so a worktree-isolated agent that commits to the primary is recorded as a FAILURE naming the drifted branch, the commit count, and the `git reset --hard` recovery — instead of a silent "completed". Detect-and-report only: the reset discards commits, so it stays a human decision. Non-throwing (runs outside the request lifecycle); an unreadable checkout reports no drift rather than inventing one. | | `pythonSetup.js` | Python venv / runner setup helpers. | | `vttTranscript.js` | WebVTT/SRT → readable prose. `vttToPlainText(vtt)` (paragraph-joined transcript), `vttToLines(vtt)` (cleaned caption lines), `cleanCaptionLine(line)`. Collapses YouTube auto-captions' rolling repetition and strips inline ``/timestamp markup. Used by the brain YouTube ingest. | +| `youtubeUrl.js` | Canonical YouTube single-video URL rule (#6014). `YOUTUBE_VIDEO_URL_RE` (accepts `watch`/`shorts`/`live`/`embed` plus the `www.`/`m.`/`music.` hosts, rejects playlists, channels, and `/@handle` feeds), `youtubeVideoIdFromUrl(url)` (alias `youtubeVideoId`), `isYoutubeVideoUrl(url)`, `assertYoutubeVideoUrl(url)` (returns the id, else throws 400 `YOUTUBE_URL_INVALID`), and the shared `YOUTUBE_URL_INVALID_MESSAGE`. Single source for the brain ingest, the Takeout importer, the history scrape, and the Music Video track import; mirrored in `client/src/lib/youtubeUrl.js` and pinned by `youtubeUrl.mirror.test.js`. | | `ytdlp.js` | `findYtDlp()` — cached discovery of the `yt-dlp` binary on PATH, mirrors `findFfmpeg()` in `ffmpeg.js`. Used by the track YouTube-import job. | ## Networking diff --git a/server/lib/index.js b/server/lib/index.js index 317fe4071f..99344b553b 100644 --- a/server/lib/index.js +++ b/server/lib/index.js @@ -290,6 +290,7 @@ export * from './processEnv.js'; export * from './primaryCheckoutGuard.js'; export * from './pythonSetup.js'; export * from './vttTranscript.js'; +export * from './youtubeUrl.js'; export * from './ytdlp.js'; // === Networking === diff --git a/server/lib/youtubeUrl.js b/server/lib/youtubeUrl.js new file mode 100644 index 0000000000..23b0d26f03 --- /dev/null +++ b/server/lib/youtubeUrl.js @@ -0,0 +1,75 @@ +/** + * Canonical YouTube single-video URL rule (#6014) — the ONE place the server + * decides "is this a single YouTube video, and which one?". + * + * Before this module the same question was answered four different ways: the + * Takeout importer owned the id parser (so `server/lib/` tests had to import + * from `server/services/`, inverting the dependency rule), the brain ingest + * declared its own accept-regex, the Music Video track import declared an OLDER + * regex that predated `music.youtube.com` / shorts / live / embed support, and + * the client mirrored all of it a fifth time. The drift was user-visible: + * pasting a YouTube Music or Shorts link into track import returned + * `400 YOUTUBE_URL_INVALID` even though yt-dlp handles it and the other two + * pipelines accepted it. + * + * `client/src/lib/youtubeUrl.js` is the browser mirror of this rule (Quick + * Capture swaps its whole submit path on it); `youtubeUrl.mirror.test.js` + * asserts the two agree on behavior, so port any change there verbatim. + * + * Deliberately narrow: playlists, channels, and `/@handle` pages are NOT + * matched — a paste that would have yt-dlp pull 300 videos must fail fast + * rather than silently start a batch download. + */ +import { ServerError } from './errorHandler.js'; + +/** + * Accepts every URL shape that carries exactly one video id, across the + * `www.` / `m.` / `music.` hosts. Playlists, channels, and feeds are rejected. + */ +export const YOUTUBE_VIDEO_URL_RE = + /^https?:\/\/(www\.|m\.|music\.)?(youtube\.com\/(watch\?[^\s#]*\bv=[\w-]{6,}|shorts\/[\w-]{6,}|live\/[\w-]{6,}|embed\/[\w-]{6,})|youtu\.be\/[\w-]{6,})/i; + +/** + * The video id in a YouTube URL, or null. Handles every shape YouTube emits: + * watch?v=ID · youtu.be/ID · /shorts/ID · /embed/ID · /live/ID · /v/ID + * (`/live/` is the permalink a finished livestream keeps — Takeout watch + * records carry it, so it must resolve rather than be dropped as unrecognized.) + * + * The charset is bounded so a garbage query string can't smuggle a giant "id" + * into a dedupe key. Intentionally looser than `YOUTUBE_VIDEO_URL_RE`: it also + * answers for URL fragments the Takeout importer meets, so validation callers + * must test the regex too (see `isYoutubeVideoUrl`). + */ +export function youtubeVideoIdFromUrl(url) { + if (!url) return null; + const s = String(url).trim(); + const vParam = /[?&]v=([A-Za-z0-9_-]{6,20})/.exec(s); + if (vParam) return vParam[1]; + const pathId = /(?:youtu\.be\/|\/shorts\/|\/embed\/|\/live\/|\/v\/)([A-Za-z0-9_-]{6,20})/.exec(s); + return pathId ? pathId[1] : null; +} + +/** Alias matching the client mirror's naming, so both layers read alike. */ +export const youtubeVideoId = youtubeVideoIdFromUrl; + +/** True when `url` is a single-video YouTube URL the server will accept. */ +export function isYoutubeVideoUrl(url) { + return typeof url === 'string' && YOUTUBE_VIDEO_URL_RE.test(url.trim()) && !!youtubeVideoIdFromUrl(url); +} + +/** Shared rejection copy, so the Zod schemas and the services name one rule. */ +export const YOUTUBE_URL_INVALID_MESSAGE = + 'Expected a single-video YouTube URL (watch, shorts, live, embed, music.youtube.com, or youtu.be) — playlists and channels are not supported'; + +/** + * Validate a URL and hand back the video id it carries — the id has to be + * parsed to validate at all, so returning it keeps the caller from parsing the + * same URL a second time (and from disagreeing about the answer). + */ +export function assertYoutubeVideoUrl(url) { + const videoId = isYoutubeVideoUrl(url) ? youtubeVideoIdFromUrl(url) : null; + if (!videoId) { + throw new ServerError(YOUTUBE_URL_INVALID_MESSAGE, { status: 400, code: 'YOUTUBE_URL_INVALID' }); + } + return videoId; +} diff --git a/server/lib/youtubeUrl.mirror.test.js b/server/lib/youtubeUrl.mirror.test.js index e184505a78..f46067a808 100644 --- a/server/lib/youtubeUrl.mirror.test.js +++ b/server/lib/youtubeUrl.mirror.test.js @@ -1,21 +1,20 @@ /** * Mirror parity test for the YouTube single-video URL rule. * - * Authoritative: `YOUTUBE_INGEST_URL_RE` + `youtubeVideoIdFromUrl` (server). + * Authoritative: `isYoutubeVideoUrl` + `youtubeVideoIdFromUrl` (`server/lib/youtubeUrl.js`). * Mirror: `isYoutubeVideoUrl` + `youtubeVideoId` (client). * * Quick Capture swaps its ENTIRE submit path on the client predicate — a looser * client offers ingest options for a URL the server will reject with a 400, and * a tighter one silently files a real video as a plain link. Unlike the * bareUrl mirror (which compares declaration source text), this compares - * BEHAVIOR: the two live in differently-shaped modules — the server's id parser - * is shared with the Takeout importer — so a text diff would fail on structure + * BEHAVIOR: the two live in differently-shaped modules — the client copy also + * carries the ingest-options table — so a text diff would fail on structure * rather than on drift. */ import { describe, it, expect } from 'vitest'; -import { YOUTUBE_INGEST_URL_RE } from '../services/youtubeIngest.js'; -import { youtubeVideoIdFromUrl } from '../services/youtubeImport.js'; +import { isYoutubeVideoUrl as serverAccepts, youtubeVideoIdFromUrl } from './youtubeUrl.js'; import { isYoutubeVideoUrl, youtubeVideoId } from '../../client/src/lib/youtubeUrl.js'; const CASES = [ @@ -39,11 +38,6 @@ const CASES = [ '', ]; -// The server's own answer, assembled from its two halves exactly the way -// assertYoutubeIngestUrl does. -const serverAccepts = (url) => - typeof url === 'string' && YOUTUBE_INGEST_URL_RE.test(url) && !!youtubeVideoIdFromUrl(url); - describe('youtubeUrl server↔client mirror parity', () => { it.each(CASES)('agrees on whether %j is a single-video YouTube URL', (url) => { expect(isYoutubeVideoUrl(url)).toBe(serverAccepts(url)); diff --git a/server/routes/tracks.js b/server/routes/tracks.js index 19fb316933..810d4d1d63 100644 --- a/server/routes/tracks.js +++ b/server/routes/tracks.js @@ -39,8 +39,9 @@ import { isSupportedMusicUpload, assertSafeMusicFilename, MUSIC_UPLOAD_MAX_BYTES, } from '../services/pipeline/musicLibrary.js'; import { - startYoutubeImport, attachImportSseClient, cancelYoutubeImport, YOUTUBE_URL_RE, + startYoutubeImport, attachImportSseClient, cancelYoutubeImport, } from '../services/trackYoutubeImport.js'; +import { YOUTUBE_VIDEO_URL_RE, YOUTUBE_URL_INVALID_MESSAGE } from '../lib/youtubeUrl.js'; import { generateChiptuneScore, renderChiptuneTrack, publishChiptuneTrack } from '../services/chiptune.js'; const router = Router(); @@ -102,7 +103,7 @@ const attachSchema = z.object({ }); const youtubeImportSchema = z.object({ - url: z.string().trim().regex(YOUTUBE_URL_RE, 'Not a recognized YouTube URL (expected youtube.com/watch, youtu.be, or m.youtube.com)'), + url: z.string().trim().regex(YOUTUBE_VIDEO_URL_RE, YOUTUBE_URL_INVALID_MESSAGE), }); // Reuse the pipeline audio stage's multipart upload contract (50MB, audio MIME). diff --git a/server/routes/tracks.test.js b/server/routes/tracks.test.js index a23ec8eda3..e9b8c92172 100644 --- a/server/routes/tracks.test.js +++ b/server/routes/tracks.test.js @@ -53,8 +53,9 @@ vi.mock('../services/albums/index.js', () => ({ getAlbum: vi.fn(async () => null), updateAlbum: vi.fn(async (id, patch) => ({ id, ...patch })), })); +// The URL rule is NOT mocked: it lives in `lib/youtubeUrl.js` and the route +// imports it from there, so these cases exercise the real accept/reject regex. vi.mock('../services/trackYoutubeImport.js', () => ({ - YOUTUBE_URL_RE: /^https?:\/\/(www\.|m\.)?(youtube\.com\/watch\?[^\s#]*\bv=[\w-]{6,}|youtu\.be\/[\w-]{6,})/i, startYoutubeImport: vi.fn(async () => ({ jobId: 'job-1' })), attachImportSseClient: vi.fn(() => true), cancelYoutubeImport: vi.fn(() => true), @@ -130,6 +131,17 @@ describe('tracks routes', () => { expect(r.status).toBe(202); }); + it.each([ + 'https://music.youtube.com/watch?v=dQw4w9WgXcQ', + 'https://www.youtube.com/shorts/dQw4w9WgXcQ', + 'https://www.youtube.com/live/dQw4w9WgXcQ', + 'https://www.youtube.com/embed/dQw4w9WgXcQ', + ])('POST /import/youtube accepts %s (#6014 — the drifted regex rejected these)', async (url) => { + const r = await request(app).post('/api/tracks/import/youtube').send({ url }); + expect(r.status).toBe(202); + expect(ytImport.startYoutubeImport).toHaveBeenCalledWith(url); + }); + it('POST /import/youtube rejects a non-YouTube URL (never reaches the service)', async () => { const r = await request(app).post('/api/tracks/import/youtube').send({ url: 'https://vimeo.com/12345' }); expect(r.status).toBe(400); diff --git a/server/services/trackYoutubeImport.js b/server/services/trackYoutubeImport.js index bd1e8ffda1..221bef4ffd 100644 --- a/server/services/trackYoutubeImport.js +++ b/server/services/trackYoutubeImport.js @@ -13,7 +13,7 @@ import { randomUUID } from 'crypto'; import { join } from 'path'; -import { ServerError } from '../lib/errorHandler.js'; +import { assertYoutubeVideoUrl, YOUTUBE_VIDEO_URL_RE } from '../lib/youtubeUrl.js'; import { shortId, PATHS } from '../lib/fileUtils.js'; import { probeVideoDuration } from '../lib/ffmpeg.js'; import { broadcastSse, attachSseClient as attachSse, closeJobAfterDelay } from '../lib/sseUtils.js'; @@ -22,20 +22,16 @@ import { importUploadedTrack, MUSIC_UPLOAD_MAX_BYTES } from './pipeline/musicLib import { createTrack, DURATION_MAX_SEC } from './tracks/index.js'; import { resolveYtDlpBinaries, downloadAudioToTempMp3, cleanupYtDlpTemp } from './ytdlpAudioImport.js'; -// youtube.com/watch, youtu.be, and m.youtube.com only (issue #1945 scope: -// "start narrow" — other video hosts are explicitly out of scope). This also -// constrains what a shelled-out yt-dlp will touch, even though args are -// passed as an array (no shell interpolation). -export const YOUTUBE_URL_RE = /^https?:\/\/(www\.|m\.)?(youtube\.com\/watch\?[^\s#]*\bv=[\w-]{6,}|youtu\.be\/[\w-]{6,})/i; - -export function assertYoutubeUrl(url) { - if (typeof url !== 'string' || !YOUTUBE_URL_RE.test(url)) { - throw new ServerError( - 'Not a recognized YouTube URL (expected youtube.com/watch, youtu.be, or m.youtube.com)', - { status: 400, code: 'YOUTUBE_URL_INVALID' }, - ); - } -} +// YouTube-only by design (issue #1945 scope: "start narrow" — other video hosts +// are explicitly out of scope). This also constrains what a shelled-out yt-dlp +// will touch, even though args are passed as an array (no shell interpolation). +// The rule itself is canonical in `lib/youtubeUrl.js` (#6014) — importing it is +// what taught this path to accept music.youtube.com, shorts, live, and embed +// links, which the brain ingest and the Takeout importer already handled. +export { + YOUTUBE_VIDEO_URL_RE as YOUTUBE_URL_RE, + assertYoutubeVideoUrl as assertYoutubeUrl, +}; // jobId -> { clients, lastPayload, process, canceled } const importJobs = new Map(); @@ -75,7 +71,7 @@ export function cancelYoutubeImport(jobId) { * or `{ type: 'canceled' }`. */ export async function startYoutubeImport(url) { - assertYoutubeUrl(url); + assertYoutubeVideoUrl(url); const { ytDlp, ffmpeg } = await resolveYtDlpBinaries(); const jobId = randomUUID(); diff --git a/server/services/trackYoutubeImport.test.js b/server/services/trackYoutubeImport.test.js index 4eb6fa898d..ad8544faa6 100644 --- a/server/services/trackYoutubeImport.test.js +++ b/server/services/trackYoutubeImport.test.js @@ -74,6 +74,35 @@ describe('YOUTUBE_URL_RE / assertYoutubeUrl', () => { expect(YOUTUBE_URL_RE.test('https://www.youtube.com/watch?list=PL123&v=dQw4w9WgXcQ')).toBe(true); }); + // #6014: this path used to declare its OWN, older regex that predated + // music.youtube.com / shorts / live / embed support, so a Music Video track + // import of a YouTube Music or Shorts link 400'd while the brain ingest and + // the Takeout importer both accepted it. + it.each([ + 'https://music.youtube.com/watch?v=dQw4w9WgXcQ', + 'https://www.youtube.com/shorts/dQw4w9WgXcQ', + 'https://youtube.com/shorts/dQw4w9WgXcQ', + 'https://www.youtube.com/live/dQw4w9WgXcQ', + 'https://www.youtube.com/embed/dQw4w9WgXcQ', + ])('accepts %s', (url) => { + expect(YOUTUBE_URL_RE.test(url)).toBe(true); + expect(() => assertYoutubeUrl(url)).not.toThrow(); + }); + + it('returns the video id so the caller need not re-parse the URL', () => { + expect(assertYoutubeUrl('https://music.youtube.com/watch?v=dQw4w9WgXcQ')).toBe('dQw4w9WgXcQ'); + expect(assertYoutubeUrl('https://youtu.be/dQw4w9WgXcQ')).toBe('dQw4w9WgXcQ'); + }); + + it.each([ + 'https://www.youtube.com/playlist?list=PLabcdefghij', + 'https://www.youtube.com/@somechannel', + 'https://www.youtube.com/feed/history', + ])('rejects %s — a batch paste must not start a 300-video download', (url) => { + expect(YOUTUBE_URL_RE.test(url)).toBe(false); + expect(() => assertYoutubeUrl(url)).toThrow(/single-video YouTube URL/); + }); + it('rejects a non-YouTube host', () => { expect(YOUTUBE_URL_RE.test('https://vimeo.com/12345')).toBe(false); expect(() => assertYoutubeUrl('https://vimeo.com/12345')).toThrow(/YouTube/); diff --git a/server/services/youtubeImport.js b/server/services/youtubeImport.js index 13059b27c6..236b052380 100644 --- a/server/services/youtubeImport.js +++ b/server/services/youtubeImport.js @@ -30,6 +30,7 @@ */ import { readFile } from 'fs/promises'; import { collectZipEntries, isZipUpload } from '../lib/zipStream.js'; +import { youtubeVideoIdFromUrl } from '../lib/youtubeUrl.js'; import { shortSummary, recordEvents, localDayKey } from './humanActivity.js'; import { getUserTimezone } from './userTimezone.js'; @@ -39,22 +40,9 @@ import { getUserTimezone } from './userTimezone.js'; // extraction and dedupe-key construction. // --------------------------------------------------------------------------- -// Extract the 11-char YouTube video id from any of the URL shapes YouTube emits: -// watch?v=ID · youtu.be/ID · /shorts/ID · /embed/ID · /v/ID · music.youtube.com -// Returns the id or null. The trailing charset is bounded so a garbage query -// string can't smuggle a giant "id" into the dedupe key. -export function youtubeVideoIdFromUrl(url) { - if (!url) return null; - const s = String(url).trim(); - const vParam = /[?&]v=([A-Za-z0-9_-]{6,20})/.exec(s); - if (vParam) return vParam[1]; - // `/live/` is the permalink shape a finished livestream keeps — Takeout watch - // records for streams carry it, and the brain ingest accepts it, so it must - // resolve to an id here rather than being silently dropped as unrecognized. - const pathId = /(?:youtu\.be\/|\/shorts\/|\/embed\/|\/live\/|\/v\/)([A-Za-z0-9_-]{6,20})/.exec(s); - if (pathId) return pathId[1]; - return null; -} +// Video-id extraction is the canonical rule in `lib/youtubeUrl.js` — re-exported +// here so the importer's long-standing public surface keeps working (#6014). +export { youtubeVideoIdFromUrl }; // Resolve a Takeout watch timestamp to a UTC ISO string, or null if unparseable. // Takeout's `time` is ISO-8601 with a `Z` (or an explicit offset), so a plain diff --git a/server/services/youtubeIngest.js b/server/services/youtubeIngest.js index 1a98f6026f..280af94a6d 100644 --- a/server/services/youtubeIngest.js +++ b/server/services/youtubeIngest.js @@ -52,7 +52,7 @@ import { vttToPlainText } from '../lib/vttTranscript.js'; import { createMutex } from '../lib/asyncMutex.js'; import { downloadAudioToTempMp3 } from './ytdlpAudioImport.js'; import { downloadVideoIntoLibrary } from './videoDownload.js'; -import { youtubeVideoIdFromUrl } from './youtubeImport.js'; +import { assertYoutubeVideoUrl, YOUTUBE_VIDEO_URL_RE } from '../lib/youtubeUrl.js'; import * as obsidian from './obsidian.js'; import * as brainStorage from './brainStorage.js'; import { createLinkFromUrl } from './brain.js'; @@ -64,27 +64,12 @@ import { recordEvents } from './humanActivity.js'; // cos.js listens on to spawn, so queueing behaves identically either way. import { addTask } from './cosTaskStore.js'; -// Accepts the URL shapes that carry a single video id. Playlists, channels, and -// `/@handle` pages are rejected up front so a paste that would have yt-dlp pull -// 300 videos fails with a clear message instead of running for an hour. -export const YOUTUBE_INGEST_URL_RE = - /^https?:\/\/(www\.|m\.|music\.)?(youtube\.com\/(watch\?[^\s#]*\bv=[\w-]{6,}|shorts\/[\w-]{6,}|live\/[\w-]{6,}|embed\/[\w-]{6,})|youtu\.be\/[\w-]{6,})/i; - -/** - * Validate an ingest URL and hand back the video id it carries — the id has to - * be parsed to validate at all, so returning it keeps the caller from parsing - * the same URL a second time (and from disagreeing about the answer). - */ -export function assertYoutubeIngestUrl(url) { - const videoId = typeof url === 'string' && YOUTUBE_INGEST_URL_RE.test(url) ? youtubeVideoIdFromUrl(url) : null; - if (!videoId) { - throw new ServerError( - 'Expected a single-video YouTube URL (watch, shorts, live, or youtu.be) — playlists and channels are not supported', - { status: 400, code: 'YOUTUBE_URL_INVALID' }, - ); - } - return videoId; -} +// The accept rule and its validator are canonical in `lib/youtubeUrl.js` (#6014); +// these aliases keep the ingest's established names for existing importers. +export { + YOUTUBE_VIDEO_URL_RE as YOUTUBE_INGEST_URL_RE, + assertYoutubeVideoUrl as assertYoutubeIngestUrl, +}; // Bound resource use, same reasoning as the audio/video importers: a livestream // archive or a 12-hour upload would otherwise download unbounded. Generous — @@ -570,7 +555,7 @@ export async function startYoutubeIngest({ tags = [], priority, } = {}) { - const videoId = assertYoutubeIngestUrl(url); + const videoId = assertYoutubeVideoUrl(url); if (!captureTranscript && !downloadVideo && !ingestAudio) { throw new ServerError('Pick at least one of: transcript, video, audio', { status: 400, diff --git a/server/services/youtubeSync.js b/server/services/youtubeSync.js index 05bc2c2f8e..ec056d5a75 100644 --- a/server/services/youtubeSync.js +++ b/server/services/youtubeSync.js @@ -35,7 +35,7 @@ import { getUserTimezone } from './userTimezone.js'; import { getSettings } from './settings.js'; import { findOrOpenPage, listCdpPages, isAuthPage, evaluateOnPage } from './browserService.js'; import { shortSummary } from './humanActivity.js'; -import { youtubeVideoIdFromUrl } from './youtubeImport.js'; +import { youtubeVideoIdFromUrl } from '../lib/youtubeUrl.js'; // --------------------------------------------------------------------------- // Constants