Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions client/src/lib/youtubeUrl.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions server/lib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<c>`/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
Expand Down
1 change: 1 addition & 0 deletions server/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ===
Expand Down
75 changes: 75 additions & 0 deletions server/lib/youtubeUrl.js
Original file line number Diff line number Diff line change
@@ -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;
}
14 changes: 4 additions & 10 deletions server/lib/youtubeUrl.mirror.test.js
Original file line number Diff line number Diff line change
@@ -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 = [
Expand All @@ -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));
Expand Down
5 changes: 3 additions & 2 deletions server/routes/tracks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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).
Expand Down
14 changes: 13 additions & 1 deletion server/routes/tracks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 12 additions & 16 deletions server/services/trackYoutubeImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
29 changes: 29 additions & 0 deletions server/services/trackYoutubeImport.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
Expand Down
20 changes: 4 additions & 16 deletions server/services/youtubeImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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
Expand Down
31 changes: 8 additions & 23 deletions server/services/youtubeIngest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 —
Expand Down Expand Up @@ -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,
Expand Down
Loading