From 9c73b6631e73f82f32acb4da8d76ab8878efefe8 Mon Sep 17 00:00:00 2001 From: Iftach Yakar Date: Sat, 14 Mar 2026 19:29:33 +0200 Subject: [PATCH 1/2] fix: improve previewUrl resolution error messages and add tests When app.previewUrl is set to an env var name that isn't available in the workflow job, the action now fails with a descriptive message naming the missing variable, rather than the generic "No base URL available". Also export resolveBaseUrl and add unit tests covering all resolution paths. Co-Authored-By: Claude Sonnet 4.6 --- packages/action/src/index.ts | 35 ++++++++++---- tests/unit/resolve-base-url.test.ts | 73 +++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 tests/unit/resolve-base-url.test.ts diff --git a/packages/action/src/index.ts b/packages/action/src/index.ts index b902cf4..130eb3c 100644 --- a/packages/action/src/index.ts +++ b/packages/action/src/index.ts @@ -154,13 +154,12 @@ async function run(): Promise { return; } - const baseUrl = resolveBaseUrl(config, previewUrlInput); - if (!baseUrl) { - core.setFailed( - 'No base URL available. Set app.previewUrl or app.startCommand + app.readyWhen in config.' - ); + const baseUrlResult = resolveBaseUrl(config, previewUrlInput); + if (!baseUrlResult.url) { + core.setFailed(baseUrlResult.error!); return; } + const baseUrl = baseUrlResult.url; if (config.setup) { core.info(`Running setup: ${config.setup}`); @@ -225,17 +224,33 @@ async function run(): Promise { } } -function resolveBaseUrl(config: GitGlimpseConfig, previewUrlOverride?: string): string | null { +export function resolveBaseUrl( + config: GitGlimpseConfig, + previewUrlOverride?: string +): { url: string; error?: never } | { url?: never; error: string } { const previewUrl = previewUrlOverride ?? config.app.previewUrl; if (previewUrl) { - const resolved = process.env[previewUrl] ?? previewUrl; - return resolved.startsWith('http') ? resolved : null; + const resolved = process.env[previewUrl]; + if (resolved === undefined) { + // previewUrl is a literal URL string, not an env var name + if (previewUrl.startsWith('http')) return { url: previewUrl }; + return { + error: `app.previewUrl is set to "${previewUrl}" but it doesn't look like a URL and no env var with that name was found. ` + + `Set it to a full URL (e.g. "https://my-preview.vercel.app") or an env var name that is available in this workflow job.`, + }; + } + if (!resolved.startsWith('http')) { + return { + error: `Env var "${previewUrl}" was found but its value "${resolved}" is not a valid URL. Expected a value starting with "http".`, + }; + } + return { url: resolved }; } if (config.app.readyWhen?.url) { const u = new URL(config.app.readyWhen.url); - return u.origin; + return { url: u.origin }; } - return 'http://localhost:3000'; + return { url: 'http://localhost:3000' }; } async function startApp( diff --git a/tests/unit/resolve-base-url.test.ts b/tests/unit/resolve-base-url.test.ts new file mode 100644 index 0000000..16d4de2 --- /dev/null +++ b/tests/unit/resolve-base-url.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { resolveBaseUrl } from '../../packages/action/src/index.js'; +import type { GitGlimpseConfig } from '../../packages/core/src/config/schema.js'; + +function makeConfig(app: GitGlimpseConfig['app']): GitGlimpseConfig { + return { + app, + llm: { provider: 'anthropic' }, + recording: { format: 'gif', maxDuration: 30, viewport: { width: 1280, height: 720 } }, + } as unknown as GitGlimpseConfig; +} + +describe('resolveBaseUrl', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('returns URL directly when previewUrl is a literal http URL', () => { + const result = resolveBaseUrl(makeConfig({ previewUrl: 'https://my-preview.vercel.app' })); + expect(result.url).toBe('https://my-preview.vercel.app'); + expect(result.error).toBeUndefined(); + }); + + it('resolves previewUrl as env var name when the env var is set', () => { + process.env['VERCEL_PREVIEW_URL'] = 'https://my-preview.vercel.app'; + const result = resolveBaseUrl(makeConfig({ previewUrl: 'VERCEL_PREVIEW_URL' })); + expect(result.url).toBe('https://my-preview.vercel.app'); + expect(result.error).toBeUndefined(); + }); + + it('returns a descriptive error when env var name is set but env var is missing', () => { + delete process.env['VERCEL_PREVIEW_URL']; + const result = resolveBaseUrl(makeConfig({ previewUrl: 'VERCEL_PREVIEW_URL' })); + expect(result.url).toBeUndefined(); + expect(result.error).toMatch(/VERCEL_PREVIEW_URL/); + expect(result.error).toMatch(/env var/); + }); + + it('returns a descriptive error when env var is set but value is not a URL', () => { + process.env['PREVIEW_URL'] = 'not-a-url'; + const result = resolveBaseUrl(makeConfig({ previewUrl: 'PREVIEW_URL' })); + expect(result.url).toBeUndefined(); + expect(result.error).toMatch(/PREVIEW_URL/); + expect(result.error).toMatch(/not a valid URL/); + }); + + it('falls back to localhost when no previewUrl and no readyWhen', () => { + const result = resolveBaseUrl(makeConfig({ startCommand: 'npm run dev' })); + expect(result.url).toBe('http://localhost:3000'); + }); + + it('uses readyWhen.url origin as base when set', () => { + const result = resolveBaseUrl( + makeConfig({ startCommand: 'npm run dev', readyWhen: { url: 'http://localhost:4000/health' } }) + ); + expect(result.url).toBe('http://localhost:4000'); + }); + + it('previewUrlOverride takes precedence over config', () => { + process.env['OVERRIDE_URL'] = 'https://override.example.com'; + const result = resolveBaseUrl( + makeConfig({ previewUrl: 'SOME_OTHER_VAR' }), + 'OVERRIDE_URL' + ); + expect(result.url).toBe('https://override.example.com'); + }); +}); From 666c42d50b8f543c3df0ebbcb5a1c225f211f41b Mon Sep 17 00:00:00 2001 From: Iftach Yakar Date: Sat, 14 Mar 2026 19:38:47 +0200 Subject: [PATCH 2/2] fix: extract resolveBaseUrl to own file to avoid @git-glimpse/core build dep in tests Importing from packages/action/src/index.ts in unit tests pulled in @git-glimpse/core by package name, which requires a prior build step that CI doesn't run for unit tests. Moving resolveBaseUrl to its own file with a direct source-path type import fixes the resolution error. Co-Authored-By: Claude Sonnet 4.6 --- packages/action/src/index.ts | 29 +---------------------- packages/action/src/resolve-base-url.ts | 31 +++++++++++++++++++++++++ tests/unit/resolve-base-url.test.ts | 2 +- 3 files changed, 33 insertions(+), 29 deletions(-) create mode 100644 packages/action/src/resolve-base-url.ts diff --git a/packages/action/src/index.ts b/packages/action/src/index.ts index 130eb3c..ae1abe1 100644 --- a/packages/action/src/index.ts +++ b/packages/action/src/index.ts @@ -13,6 +13,7 @@ import { DEFAULT_TRIGGER, type GitGlimpseConfig, } from '@git-glimpse/core'; +import { resolveBaseUrl } from './resolve-base-url.js'; function streamCommand(cmd: string, args: string[]): Promise { return new Promise((resolve, reject) => { @@ -224,34 +225,6 @@ async function run(): Promise { } } -export function resolveBaseUrl( - config: GitGlimpseConfig, - previewUrlOverride?: string -): { url: string; error?: never } | { url?: never; error: string } { - const previewUrl = previewUrlOverride ?? config.app.previewUrl; - if (previewUrl) { - const resolved = process.env[previewUrl]; - if (resolved === undefined) { - // previewUrl is a literal URL string, not an env var name - if (previewUrl.startsWith('http')) return { url: previewUrl }; - return { - error: `app.previewUrl is set to "${previewUrl}" but it doesn't look like a URL and no env var with that name was found. ` + - `Set it to a full URL (e.g. "https://my-preview.vercel.app") or an env var name that is available in this workflow job.`, - }; - } - if (!resolved.startsWith('http')) { - return { - error: `Env var "${previewUrl}" was found but its value "${resolved}" is not a valid URL. Expected a value starting with "http".`, - }; - } - return { url: resolved }; - } - if (config.app.readyWhen?.url) { - const u = new URL(config.app.readyWhen.url); - return { url: u.origin }; - } - return { url: 'http://localhost:3000' }; -} async function startApp( startCommand: string, diff --git a/packages/action/src/resolve-base-url.ts b/packages/action/src/resolve-base-url.ts new file mode 100644 index 0000000..0161080 --- /dev/null +++ b/packages/action/src/resolve-base-url.ts @@ -0,0 +1,31 @@ +import type { GitGlimpseConfig } from '../../core/src/config/schema.js'; + +export function resolveBaseUrl( + config: GitGlimpseConfig, + previewUrlOverride?: string +): { url: string; error?: never } | { url?: never; error: string } { + const previewUrl = previewUrlOverride ?? config.app.previewUrl; + if (previewUrl) { + const resolved = process.env[previewUrl]; + if (resolved === undefined) { + // previewUrl is a literal URL string, not an env var name + if (previewUrl.startsWith('http')) return { url: previewUrl }; + return { + error: + `app.previewUrl is set to "${previewUrl}" but it doesn't look like a URL and no env var with that name was found. ` + + `Set it to a full URL (e.g. "https://my-preview.vercel.app") or an env var name that is available in this workflow job.`, + }; + } + if (!resolved.startsWith('http')) { + return { + error: `Env var "${previewUrl}" was found but its value "${resolved}" is not a valid URL. Expected a value starting with "http".`, + }; + } + return { url: resolved }; + } + if (config.app.readyWhen?.url) { + const u = new URL(config.app.readyWhen.url); + return { url: u.origin }; + } + return { url: 'http://localhost:3000' }; +} diff --git a/tests/unit/resolve-base-url.test.ts b/tests/unit/resolve-base-url.test.ts index 16d4de2..32690d6 100644 --- a/tests/unit/resolve-base-url.test.ts +++ b/tests/unit/resolve-base-url.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { resolveBaseUrl } from '../../packages/action/src/index.js'; +import { resolveBaseUrl } from '../../packages/action/src/resolve-base-url.js'; import type { GitGlimpseConfig } from '../../packages/core/src/config/schema.js'; function makeConfig(app: GitGlimpseConfig['app']): GitGlimpseConfig {