fix: issue triage -- #155 #156 #154 #25, CI hardening, E2E improvements

Issue fixes:
- #155: /api/browse/playlists/parse now handles YouTube/YouTube Music URLs
- #156: stop passing album MBID to verifyArtistName (was calling MB /artist/{id}
  with an album MBID, always 404d); fix spotify trackCount stale value
- #154: remove hardcoded port-3030 detection from getApiBaseUrl -- now returns
  relative URLs by default so any host:port mapping works
- #25 (partial): fix spotify playlist trackCount to use tracks.length instead of
  stale playlist.tracks.total after pagination

Dead code / quality:
- Remove unused rootFolderPath param from processDownload + call sites
- Remove unused req params in route handlers (prefix _req)
- Remove dead push condition from integration.yml job gate
- Remove dead baseUrl constructor param and private field from ApiService
- Fix LibraryTabs hover effect: remove inline style={{ opacity: 0.1 }} that
  overrode Tailwind group-hover; change to group-hover:opacity-10
- Fix mobile tab centering in LibraryTabs (add justify-center)

CI security:
- Mask TEST_PASS before writing to GITHUB_ENV in all three workflow files
- Add missing concurrency block to nightly.yml
- Add username validation + remove credential echo in create-e2e-user.sh
- Fix global.setup.ts error message to mention .env.test

E2E:
- Fix vibe test race condition: replace Promise.race + transient text with
  stable trackCount.or(noData) assertion
- Fix security test flakiness: toBe(beforeCount) -> not.toBeGreaterThan for
  playlist count check (parallel tests can delete playlists concurrently)
- Fix global.setup.ts error message to reference .env.test file

Vibe map:
- Increase cluster label size (13->15 / 10->12 px) and opacity (50->70 / 35->50)
  for slightly better readability
This commit is contained in:
Your Name
2026-03-17 10:04:03 -05:00
parent 632d7d5030
commit fc8071b7aa
13 changed files with 67 additions and 65 deletions
+2 -1
View File
@@ -13,7 +13,7 @@ concurrency:
jobs: jobs:
e2e: e2e:
name: E2E Tests name: E2E Tests
if: github.event_name == 'push' || github.event.label.name == 'run-e2e' if: github.event_name == 'workflow_dispatch' || github.event.label.name == 'run-e2e'
runs-on: ubuntu-latest runs-on: ubuntu-latest
timeout-minutes: 60 timeout-minutes: 60
@@ -48,6 +48,7 @@ jobs:
run: | run: |
TEST_USER="kima_e2e" TEST_USER="kima_e2e"
TEST_PASS="$(openssl rand -hex 20)" TEST_PASS="$(openssl rand -hex 20)"
echo "::add-mask::${TEST_PASS}"
echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV" echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV"
echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV" echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV"
KIMA_CONTAINER=kima-e2e \ KIMA_CONTAINER=kima-e2e \
+5
View File
@@ -5,6 +5,10 @@ on:
- cron: "0 3 * * *" - cron: "0 3 * * *"
workflow_dispatch: workflow_dispatch:
concurrency:
group: nightly
cancel-in-progress: true
jobs: jobs:
full-e2e: full-e2e:
name: Full E2E Suite name: Full E2E Suite
@@ -44,6 +48,7 @@ jobs:
# Generate random credentials for this run -- no hardcoded passwords in source. # Generate random credentials for this run -- no hardcoded passwords in source.
TEST_USER="kima_e2e" TEST_USER="kima_e2e"
TEST_PASS="$(openssl rand -hex 20)" TEST_PASS="$(openssl rand -hex 20)"
echo "::add-mask::${TEST_PASS}"
echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV" echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV"
echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV" echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV"
KIMA_CONTAINER=kima-nightly \ KIMA_CONTAINER=kima-nightly \
+1
View File
@@ -65,6 +65,7 @@ jobs:
run: | run: |
TEST_USER="kima_e2e" TEST_USER="kima_e2e"
TEST_PASS="$(openssl rand -hex 20)" TEST_PASS="$(openssl rand -hex 20)"
echo "::add-mask::${TEST_PASS}"
echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV" echo "KIMA_TEST_USERNAME=${TEST_USER}" >> "$GITHUB_ENV"
echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV" echo "KIMA_TEST_PASSWORD=${TEST_PASS}" >> "$GITHUB_ENV"
KIMA_CONTAINER=kima-security \ KIMA_CONTAINER=kima-security \
+29 -6
View File
@@ -137,7 +137,7 @@ router.get("/playlists/:id", async (req, res) => {
* GET /api/browse/radios * GET /api/browse/radios
* Get all radio stations (mood/theme based mixes) * Get all radio stations (mood/theme based mixes)
*/ */
router.get("/radios", async (req, res) => { router.get("/radios", async (_req, res) => {
try { try {
logger.debug("[Browse] Fetching radio stations..."); logger.debug("[Browse] Fetching radio stations...");
const radios = await deezerService.getRadioStations(); const radios = await deezerService.getRadioStations();
@@ -156,7 +156,7 @@ router.get("/radios", async (req, res) => {
* GET /api/browse/radios/by-genre * GET /api/browse/radios/by-genre
* Get radio stations organized by genre * Get radio stations organized by genre
*/ */
router.get("/radios/by-genre", async (req, res) => { router.get("/radios/by-genre", async (_req, res) => {
try { try {
logger.debug("[Browse] Fetching radios by genre..."); logger.debug("[Browse] Fetching radios by genre...");
const genresWithRadios = await deezerService.getRadiosByGenre(); const genresWithRadios = await deezerService.getRadiosByGenre();
@@ -207,7 +207,7 @@ router.get("/radios/:id", async (req, res) => {
* GET /api/browse/genres * GET /api/browse/genres
* Get all available genres * Get all available genres
*/ */
router.get("/genres", async (req, res) => { router.get("/genres", async (_req, res) => {
try { try {
logger.debug("[Browse] Fetching genres..."); logger.debug("[Browse] Fetching genres...");
const genres = await deezerService.getGenres(); const genres = await deezerService.getGenres();
@@ -311,8 +311,31 @@ router.post("/playlists/parse", async (req, res) => {
}); });
} }
return res.status(400).json({ // Try YouTube / YouTube Music -- extract list= query param as playlist ID
error: "Invalid or unsupported URL. Please provide a Spotify or Deezer playlist URL." try {
const parsed = new URL(url);
const hostname = parsed.hostname.toLowerCase();
const isYouTube =
hostname.includes("youtube.com") ||
hostname.includes("youtu.be") ||
hostname.includes("music.youtube.com");
if (isYouTube) {
const listId = parsed.searchParams.get("list");
if (listId) {
return res.json({
source: "youtube",
type: "playlist",
id: listId,
url,
});
}
}
} catch {
// Invalid URL -- fall through to error
}
return res.status(400).json({
error: "Invalid or unsupported URL. Please provide a Spotify, Deezer, or YouTube playlist URL."
}); });
} catch (error) { } catch (error) {
safeError(res, "Parse URL", error); safeError(res, "Parse URL", error);
@@ -324,7 +347,7 @@ router.post("/playlists/parse", async (req, res) => {
* Get a combined view of featured content (playlists, genres) * Get a combined view of featured content (playlists, genres)
* Note: Radio stations are now internal (library-based), not from Deezer * Note: Radio stations are now internal (library-based), not from Deezer
*/ */
router.get("/all", async (req, res) => { router.get("/all", async (_req, res) => {
try { try {
logger.debug("[Browse] Fetching browse content (playlists + genres)..."); logger.debug("[Browse] Fetching browse content (playlists + genres)...");
+2 -5
View File
@@ -158,7 +158,7 @@ router.post("/", async (req, res) => {
// Single album download - verify artist name before proceeding // Single album download - verify artist name before proceeding
let verifiedArtistName = artistName; let verifiedArtistName = artistName;
if (type === "album" && artistName) { if (type === "album" && artistName) {
const verification = await verifyArtistName(artistName, mbid); const verification = await verifyArtistName(artistName);
if (verification.wasCorrected) { if (verification.wasCorrected) {
logger.debug( logger.debug(
`[DOWNLOAD] Artist name verified: "${artistName}" → "${verification.verifiedName}" (source: ${verification.source})` `[DOWNLOAD] Artist name verified: "${artistName}" → "${verification.verifiedName}" (source: ${verification.source})`
@@ -241,7 +241,6 @@ router.post("/", async (req, res) => {
type, type,
mbid, mbid,
subject, subject,
rootFolderPath,
verifiedArtistName, verifiedArtistName,
albumTitle albumTitle
).catch((error) => { ).catch((error) => {
@@ -486,7 +485,6 @@ async function processArtistDownload(
"album", "album",
albumMbid, albumMbid,
albumSubject, albumSubject,
rootFolderPath,
artistName, artistName,
albumTitle albumTitle
).catch((error) => { ).catch((error) => {
@@ -508,7 +506,6 @@ async function processDownload(
type: string, type: string,
mbid: string, mbid: string,
subject: string, subject: string,
rootFolderPath: string,
artistName?: string, artistName?: string,
albumTitle?: string albumTitle?: string
) { ) {
@@ -576,7 +573,7 @@ router.delete("/clear-all", async (req, res) => {
}); });
// POST /downloads/clear-lidarr-queue - Clear stuck/failed items from Lidarr's queue // POST /downloads/clear-lidarr-queue - Clear stuck/failed items from Lidarr's queue
router.post("/clear-lidarr-queue", async (req, res) => { router.post("/clear-lidarr-queue", async (_req, res) => {
try { try {
const result = await simpleDownloadManager.clearLidarrQueue(); const result = await simpleDownloadManager.clearLidarrQueue();
res.json({ res.json({
+1 -1
View File
@@ -761,7 +761,7 @@ class SpotifyService {
description: playlist.description, description: playlist.description,
owner: playlist.owner?.display_name || "Unknown", owner: playlist.owner?.display_name || "Unknown",
imageUrl: playlist.images?.[0]?.url || null, imageUrl: playlist.images?.[0]?.url || null,
trackCount: playlist.tracks?.total || tracks.length, trackCount: tracks.length,
tracks, tracks,
isPublic: playlist.public ?? true, isPublic: playlist.public ?? true,
}; };
@@ -20,7 +20,7 @@ export function LibraryTabs({ activeTab, onTabChange }: LibraryTabsProps) {
<div className="absolute -inset-x-4 -inset-y-2 bg-[#0a0a0a]/60 backdrop-blur-xl rounded-2xl border border-white/5" /> <div className="absolute -inset-x-4 -inset-y-2 bg-[#0a0a0a]/60 backdrop-blur-xl rounded-2xl border border-white/5" />
{/* Tab buttons */} {/* Tab buttons */}
<div className="relative flex gap-2 p-2"> <div className="relative flex justify-center gap-2 p-2">
{tabs.map((tab, index) => { {tabs.map((tab, index) => {
const isActive = activeTab === tab.id; const isActive = activeTab === tab.id;
const Icon = tab.icon; const Icon = tab.icon;
@@ -62,10 +62,9 @@ export function LibraryTabs({ activeTab, onTabChange }: LibraryTabsProps) {
{!isActive && ( {!isActive && (
<div <div
className={cn( className={cn(
"absolute inset-0 opacity-0 group-hover:opacity-100 transition-opacity duration-300 bg-gradient-to-r", "absolute inset-0 opacity-0 group-hover:opacity-10 transition-opacity duration-300 bg-gradient-to-r",
tab.gradient tab.gradient
)} )}
style={{ opacity: 0.1 }}
/> />
)} )}
</button> </button>
+2 -2
View File
@@ -227,8 +227,8 @@ export function VibeMap({
data: labels, data: labels,
getPosition: (d) => [d.x, d.y], getPosition: (d) => [d.x, d.y],
getText: (d) => d.label, getText: (d) => d.label,
getSize: labelZoom < 7 ? 13 : 10, getSize: labelZoom < 7 ? 15 : 12,
getColor: [255, 255, 255, labelZoom < 7 ? 50 : 35], getColor: [255, 255, 255, labelZoom < 7 ? 70 : 50],
fontFamily: "Montserrat, system-ui, sans-serif", fontFamily: "Montserrat, system-ui, sans-serif",
fontWeight: 500, fontWeight: 500,
getTextAnchor: "middle" as const, getTextAnchor: "middle" as const,
+6 -28
View File
@@ -68,40 +68,22 @@ export const getApiBaseUrl = () => {
return process.env.BACKEND_URL || "http://127.0.0.1:3006"; return process.env.BACKEND_URL || "http://127.0.0.1:3006";
} }
// Explicit env var takes precedence // Explicit env var: used for reverse proxies where the API is on a different origin
if (process.env.NEXT_PUBLIC_API_URL) { if (process.env.NEXT_PUBLIC_API_URL) {
return process.env.NEXT_PUBLIC_API_URL; return process.env.NEXT_PUBLIC_API_URL;
} }
// Docker all-in-one mode: Use relative URLs (Next.js rewrites will proxy) // Default: relative URLs so Next.js rewrites proxy to the backend.
// This is detected by checking if we're on the same port as the frontend // Works for any host port mapping (e.g. -p 9000:3030) because the browser
const frontendPort = // origin matches whatever port the user mapped -- no hardcoded port needed.
window.location.port || return "";
(window.location.protocol === "https:" ? "443" : "80");
if (
frontendPort === "3030" ||
frontendPort === "443" ||
frontendPort === "80"
) {
// Use relative paths - Next.js rewrites will proxy to backend
return "";
}
// Development mode: Backend on separate port
const currentHost = window.location.hostname;
const apiPort = "3006";
return `${window.location.protocol}//${currentHost}:${apiPort}`;
}; };
class ApiClient { class ApiClient {
private baseUrl: string;
private token: string | null = null; private token: string | null = null;
private tokenInitialized: boolean = false; private tokenInitialized: boolean = false;
constructor(baseUrl?: string) { constructor() {
// Don't set baseUrl in constructor - determine it dynamically on each request
this.baseUrl = baseUrl || "";
// Try to load token synchronously // Try to load token synchronously
if (typeof window !== "undefined") { if (typeof window !== "undefined") {
this.token = localStorage.getItem(AUTH_TOKEN_KEY); this.token = localStorage.getItem(AUTH_TOKEN_KEY);
@@ -172,11 +154,7 @@ class ApiClient {
} }
} }
// Get the base URL dynamically to support switching between localhost and IP
private getBaseUrl(): string { private getBaseUrl(): string {
if (this.baseUrl) {
return this.baseUrl;
}
return getApiBaseUrl(); return getApiBaseUrl();
} }
+1 -1
View File
@@ -19,7 +19,7 @@ async function globalSetup(): Promise<void> {
if (!username || !password) { if (!username || !password) {
throw new Error( throw new Error(
"E2E test user credentials not set.\n" + "E2E test user credentials not set.\n" +
"Set KIMA_TEST_USERNAME and KIMA_TEST_PASSWORD before running E2E tests.\n" + "Set KIMA_TEST_USERNAME and KIMA_TEST_PASSWORD in .env.test or export them before running E2E tests.\n" +
"To create a test user, run: bash scripts/create-e2e-user.sh" "To create a test user, run: bash scripts/create-e2e-user.sh"
); );
} }
+3 -2
View File
@@ -295,13 +295,14 @@ test.describe("Security", () => {
}); });
expect([400, 422]).toContain(createRes.status()); expect([400, 422]).toContain(createRes.status());
// Count must be unchanged // Count must not have increased -- parallel tests may delete playlists
// concurrently, so we only assert the invalid request didn't create anything.
const afterRes = await page.request.get("/api/playlists", { const afterRes = await page.request.get("/api/playlists", {
headers: { Authorization: `Bearer ${token}` }, headers: { Authorization: `Bearer ${token}` },
}); });
const after = await afterRes.json() as unknown[]; const after = await afterRes.json() as unknown[];
const afterCount = Array.isArray(after) ? after.length : 0; const afterCount = Array.isArray(after) ? after.length : 0;
expect(afterCount).toBe(beforeCount); expect(afterCount).not.toBeGreaterThan(beforeCount);
}); });
}); });
}); });
+7 -12
View File
@@ -56,21 +56,16 @@ test.describe("Vibe", () => {
// ---- Page load ---------------------------------------------------------- // ---- Page load ----------------------------------------------------------
test("vibe page renders canvas or no-data state", async ({ page }) => { test("vibe page renders map or no-data state", async ({ page }) => {
await page.goto("/vibe", { waitUntil: "domcontentloaded" }); await page.goto("/vibe", { waitUntil: "domcontentloaded" });
// Either a canvas (map rendered) or the no-data placeholder must appear // Wait for loading to settle: either the track count appears (map loaded with data)
const canvas = page.locator("canvas"); // or the no-data placeholder appears (library has no vibe embeddings yet).
const noData = page.locator("text=/No tracks with vibe|Computing music map/i"); // "Computing music map" is a transient loading state -- do not assert on it.
const trackCount = page.locator("text=/ tracks$/");
const noData = page.locator("text=/No tracks with vibe/i");
await Promise.race([ await expect(trackCount.or(noData)).toBeVisible({ timeout: 35_000 });
canvas.waitFor({ timeout: 35_000 }),
noData.waitFor({ timeout: 35_000 }),
]);
const hasCanvas = (await canvas.count()) > 0;
const hasNoData = (await noData.count()) > 0;
expect(hasCanvas || hasNoData).toBe(true);
}); });
test("toolbar buttons are present when map loads", async ({ page }) => { test("toolbar buttons are present when map loads", async ({ page }) => {
+6 -4
View File
@@ -17,6 +17,12 @@ CONTAINER="${KIMA_CONTAINER:-kima-test}"
TEST_USER="${KIMA_TEST_USERNAME:-kima_e2e}" TEST_USER="${KIMA_TEST_USERNAME:-kima_e2e}"
TEST_PASS="${KIMA_TEST_PASSWORD:-$(openssl rand -hex 20)}" TEST_PASS="${KIMA_TEST_PASSWORD:-$(openssl rand -hex 20)}"
# Validate username to prevent SQL injection via the heredoc
if [[ ! "${TEST_USER}" =~ ^[a-zA-Z0-9_]{3,32}$ ]]; then
echo "[e2e setup] ERROR: KIMA_TEST_USERNAME must be 3-32 alphanumeric/underscore characters" >&2
exit 1
fi
echo "[e2e setup] Creating test user '${TEST_USER}' in container '${CONTAINER}'..." echo "[e2e setup] Creating test user '${TEST_USER}' in container '${CONTAINER}'..."
# Generate bcrypt hash inside the container where bcrypt is installed. # Generate bcrypt hash inside the container where bcrypt is installed.
@@ -48,7 +54,3 @@ ENDSQL
' '
echo "[e2e setup] Test user '${TEST_USER}' ready." echo "[e2e setup] Test user '${TEST_USER}' ready."
echo ""
echo "Set these env vars before running Playwright:"
echo " export KIMA_TEST_USERNAME=${TEST_USER}"
echo " export KIMA_TEST_PASSWORD=${TEST_PASS}"