From d41cabccf56ed508454e35a6bcb9e1df0df76b64 Mon Sep 17 00:00:00 2001 From: ProPR Codex Bot Date: Thu, 3 Sep 2026 09:49:53 +0000 Subject: [PATCH 01/18] feat(previews): add repository visual preview settings --- packages/api/routes/configRepoValidation.ts | 95 ++++++++++++++++++- packages/api/routes/configRoutes.ts | 12 ++- packages/api/test/configRepoRoutes.test.ts | 47 ++++++++- .../api/test/configRepoValidation.test.ts | 43 +++++++++ packages/core/src/config/configManager.ts | 58 +++++++++++ .../core/test/visualPreviewConfig.test.ts | 51 ++++++++++ propr-ui/src/api/proprTypes.ts | 6 ++ .../src/components/RepositoryListContent.tsx | 3 + .../src/components/RepositoryListItem.tsx | 94 +++++++++++++++++- propr-ui/src/hooks/useRepositoryManagement.ts | 56 +++++++++-- propr-ui/src/pages/RepositoriesPage.tsx | 4 +- 11 files changed, 455 insertions(+), 14 deletions(-) create mode 100644 packages/core/test/visualPreviewConfig.test.ts diff --git a/packages/api/routes/configRepoValidation.ts b/packages/api/routes/configRepoValidation.ts index bb19561c2..c5c3a762d 100644 --- a/packages/api/routes/configRepoValidation.ts +++ b/packages/api/routes/configRepoValidation.ts @@ -1,7 +1,27 @@ import { randomUUID } from 'crypto'; -import type { RepoToMonitor } from '@propr/core'; +import type { RepoToMonitor, VisualPreviewSettings, VisualPreviewType } from '@propr/core'; import { normalizeOptionalBranchName } from './branchNameValidation.js'; +const MAX_VISUAL_PREVIEW_INSTRUCTIONS_LENGTH = 4000; + +function normalizeStoredVisualPreviewSettings(value: unknown): VisualPreviewSettings { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return { enabled: false, types: ['image'] }; + } + const candidate = value as Partial; + const types = Array.isArray(candidate.types) + ? [...new Set(candidate.types.filter((type): type is VisualPreviewType => type === 'image' || type === 'video'))] + : []; + const instructions = typeof candidate.instructions === 'string' && candidate.instructions.trim() + ? candidate.instructions.trim() + : undefined; + return { + enabled: candidate.enabled === true, + types: types.length > 0 ? types : ['image'], + ...(instructions ? { instructions } : {}) + }; +} + type ValidationResult = { ok: true; value: T } | { ok: false; error: string }; function success(value: T): ValidationResult { @@ -45,6 +65,13 @@ export function withDefaultRepoAutoFollowup(repo: RepoToMonitor): RepoToMonitor return { ...repo, autoFollowupOnFailedCi: repo.autoFollowupOnFailedCi === true }; } +export function withDefaultRepoOptions(repo: RepoToMonitor): RepoToMonitor { + return { + ...withDefaultRepoAutoFollowup(repo), + visualPreview: normalizeStoredVisualPreviewSettings(repo.visualPreview) + }; +} + export function preserveRepoAutoFollowup( previousRepos: RepoToMonitor[], normalizedRepos: RepoToMonitor[], @@ -58,6 +85,69 @@ export function preserveRepoAutoFollowup( }); } +export function preserveRepoVisualPreview( + previousRepos: RepoToMonitor[], + normalizedRepos: RepoToMonitor[], + incomingRepos: unknown[] +): RepoToMonitor[] { + const explicitByRepository = new Map(); + normalizedRepos.forEach((repo, index) => { + const incoming = incomingRepos[index] as Partial; + if (incoming.visualPreview !== undefined) { + explicitByRepository.set(repo.name.trim().toLowerCase(), normalizeStoredVisualPreviewSettings(repo.visualPreview)); + } + }); + + return normalizedRepos.map(repo => { + const repositoryKey = repo.name.trim().toLowerCase(); + const explicit = explicitByRepository.get(repositoryKey); + if (explicit) return { ...repo, visualPreview: explicit }; + + const previous = previousRepos.find(candidate => candidate.name.trim().toLowerCase() === repositoryKey); + return { ...repo, visualPreview: normalizeStoredVisualPreviewSettings(previous?.visualPreview) }; + }); +} + +function normalizeVisualPreviewTypes(value: unknown, repoName: string): ValidationResult { + if (!Array.isArray(value)) { + return failure(`Invalid visualPreview.types format for ${repoName}: must be an array`); + } + if (value.some(type => type !== 'image' && type !== 'video')) { + return failure(`Invalid visualPreview.types format for ${repoName}: supported values are image and video`); + } + return success([...new Set(value as VisualPreviewType[])]); +} + +function normalizeVisualPreview(value: unknown, repoName: string): ValidationResult { + if (value === undefined) return success({ enabled: false, types: ['image'] }); + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return failure(`Invalid visualPreview format for ${repoName}: must be an object`); + } + + const candidate = value as Partial; + if (typeof candidate.enabled !== 'boolean') { + return failure(`Invalid visualPreview.enabled format for ${repoName}: must be a boolean`); + } + const types = normalizeVisualPreviewTypes(candidate.types, repoName); + if (!types.ok) return types; + if (candidate.enabled && types.value.length === 0) { + return failure(`Invalid visualPreview.types format for ${repoName}: select at least one type when previews are enabled`); + } + if (candidate.instructions !== undefined && typeof candidate.instructions !== 'string') { + return failure(`Invalid visualPreview.instructions format for ${repoName}: must be a string`); + } + const instructions = candidate.instructions?.trim(); + if (instructions && instructions.length > MAX_VISUAL_PREVIEW_INSTRUCTIONS_LENGTH) { + return failure(`Invalid visualPreview.instructions format for ${repoName}: must be ${MAX_VISUAL_PREVIEW_INSTRUCTIONS_LENGTH} characters or fewer`); + } + + return success({ + enabled: candidate.enabled, + types: types.value.length > 0 ? types.value : ['image'], + ...(instructions ? { instructions } : {}) + }); +} + export function normalizeRepoConfig(repo: unknown): ValidationResult { const candidateResult = parseRepoObject(repo); if (!candidateResult.ok) return candidateResult; @@ -78,12 +168,15 @@ export function normalizeRepoConfig(repo: unknown): ValidationResult body.followup_ignore_keywords, validate: followup_ignore_keywords => parseNormalizedStringArrayResult(followup_ignore_keywords, 'followup_ignore_keywords'), save: followup_ignore_keywords => configStore.saveFollowupIgnoreKeywords(followup_ignore_keywords), subtype: 'followup_ignore_keywords_update', body: followup_ignore_keywords => ({ followup_ignore_keywords }), committedErrorMessage: 'Follow-up ignore keywords were saved, but publishing the config update notification failed. Persisted config may require a follow-up check.' }); const getRepos = createJsonGetHandler( - async () => (await configStore.loadMonitoredReposRaw()).map(withDefaultRepoAutoFollowup), + async () => (await configStore.loadMonitoredReposRaw()).map(withDefaultRepoOptions), repos_to_monitor => ({ repos_to_monitor }), 'Failed to load repository configuration', '/api/config/repos GET' @@ -214,7 +219,8 @@ export function createConfigRoutes(deps: ConfigRoutesDeps) { } const result = await withConfigLock(redisClient, 'config:repos:lock', async lock => { const previousRepos = await configStore.loadMonitoredReposRaw(); - const processedRepos = preserveRepoAutoFollowup(previousRepos, validatedRepos, repos_to_monitor); + const withPreservedAutoFollowup = preserveRepoAutoFollowup(previousRepos, validatedRepos, repos_to_monitor); + const processedRepos = preserveRepoVisualPreview(previousRepos, withPreservedAutoFollowup, repos_to_monitor); return saveThenPublishConfigUpdate({ save: async () => { await database.transaction(async trx => { diff --git a/packages/api/test/configRepoRoutes.test.ts b/packages/api/test/configRepoRoutes.test.ts index 96ca416e9..630381b6c 100644 --- a/packages/api/test/configRepoRoutes.test.ts +++ b/packages/api/test/configRepoRoutes.test.ts @@ -43,7 +43,8 @@ test('GET repository config returns false for legacy entries with a missing opti id: 'repo-1', name: 'integry/propr', enabled: true, - autoFollowupOnFailedCi: false + autoFollowupOnFailedCi: false, + visualPreview: { enabled: false, types: ['image'] } }] }); }); @@ -86,6 +87,7 @@ test('POST repository config persists an enabled option without enabling other r name: 'integry/propr', enabled: true, autoFollowupOnFailedCi: true, + visualPreview: { enabled: false, types: ['image'] }, alias: undefined, baseBranch: undefined, defaultBranch: undefined @@ -95,6 +97,7 @@ test('POST repository config persists an enabled option without enabling other r name: 'integry/other', enabled: true, autoFollowupOnFailedCi: false, + visualPreview: { enabled: false, types: ['image'] }, alias: undefined, baseBranch: undefined, defaultBranch: undefined @@ -102,6 +105,48 @@ test('POST repository config persists an enabled option without enabling other r ]); }); +test('POST repository config synchronizes visual previews across branch entries', async () => { + const saveMonitoredRepos = mock.fn(async () => true); + const routes = createConfigRoutes({ + redisClient: { + set: mock.fn(async () => 'OK'), + eval: mock.fn(async () => 1), + publish: mock.fn(async () => 1), + lPush: mock.fn(async () => 1), + lTrim: mock.fn(async () => 'OK') + } as never, + configStore: { + loadMonitoredReposRaw: async () => [], + saveMonitoredRepos, + clearRemovedRepositoryIndexData: async () => {} + }, + database: { + transaction: async (callback: (transaction: never) => Promise) => callback({} as never) + } as never + }); + const response = createResponse(); + const visualPreview = { + enabled: true, + types: ['image', 'video'], + instructions: 'Show desktop and mobile.' + }; + + await routes.postRepos({ + body: { + repos_to_monitor: [ + { id: 'repo-main', name: 'integry/propr', enabled: true, baseBranch: 'main', visualPreview }, + { id: 'repo-release', name: 'integry/propr', enabled: true, baseBranch: 'release' } + ] + } + } as never, response as never); + + assert.equal(response.statusCode, 200); + assert.deepEqual( + saveMonitoredRepos.mock.calls[0]?.arguments[0].map(repo => repo.visualPreview), + [visualPreview, visualPreview] + ); +}); + test('POST repository config preserves an omitted option for existing repositories', async () => { const saveMonitoredRepos = mock.fn(async () => true); const routes = createConfigRoutes({ diff --git a/packages/api/test/configRepoValidation.test.ts b/packages/api/test/configRepoValidation.test.ts index 19dc67e89..c6b4c8280 100644 --- a/packages/api/test/configRepoValidation.test.ts +++ b/packages/api/test/configRepoValidation.test.ts @@ -12,6 +12,49 @@ test('repository config defaults missing automatic failed-CI follow-up to false' assert.equal(normalized.ok, true); if (normalized.ok) { assert.equal(normalized.value.autoFollowupOnFailedCi, false); + assert.deepEqual(normalized.value.visualPreview, { enabled: false, types: ['image'] }); + } +}); + +test('repository config accepts visual preview types and trims instructions', () => { + const normalized = normalizeRepoConfig({ + id: 'repo-1', + name: 'integry/propr', + enabled: true, + visualPreview: { + enabled: true, + types: ['video', 'image', 'video'], + instructions: ' Capture desktop and mobile views. ' + } + }); + + assert.equal(normalized.ok, true); + if (normalized.ok) { + assert.deepEqual(normalized.value.visualPreview, { + enabled: true, + types: ['video', 'image'], + instructions: 'Capture desktop and mobile views.' + }); + } +}); + +test('repository config rejects invalid visual preview settings', () => { + const invalidValues = [ + { enabled: 'true', types: ['image'] }, + { enabled: true, types: [] }, + { enabled: true, types: ['animation'] }, + { enabled: true, types: ['image'], instructions: 42 } + ]; + + for (const visualPreview of invalidValues) { + const normalized = normalizeRepoConfig({ + id: 'repo-1', + name: 'integry/propr', + enabled: true, + visualPreview + }); + assert.equal(normalized.ok, false); + if (!normalized.ok) assert.match(normalized.error, /visualPreview/); } }); diff --git a/packages/core/src/config/configManager.ts b/packages/core/src/config/configManager.ts index d246724fb..cf7fe45a9 100644 --- a/packages/core/src/config/configManager.ts +++ b/packages/core/src/config/configManager.ts @@ -18,11 +18,40 @@ export interface RepoToMonitor { name: string; // owner/repo enabled: boolean; autoFollowupOnFailedCi?: boolean; // Defaults to false for legacy configurations + visualPreview?: VisualPreviewSettings; // Defaults to disabled for legacy configurations alias?: string; // Optional display name baseBranch?: string; // Optional specific branch to monitor defaultBranch?: string; // Optional repository default branch for demo metadata } +export type VisualPreviewType = 'image' | 'video'; + +export interface VisualPreviewSettings { + enabled: boolean; + types: VisualPreviewType[]; + instructions?: string; +} + +export function normalizeStoredVisualPreviewSettings(value: unknown): VisualPreviewSettings { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return { enabled: false, types: ['image'] }; + } + + const candidate = value as Partial; + const types = Array.isArray(candidate.types) + ? [...new Set(candidate.types.filter((type): type is VisualPreviewType => type === 'image' || type === 'video'))] + : []; + const instructions = typeof candidate.instructions === 'string' && candidate.instructions.trim() + ? candidate.instructions.trim() + : undefined; + + return { + enabled: candidate.enabled === true, + types: types.length > 0 ? types : ['image'], + ...(instructions ? { instructions } : {}) + }; +} + interface ConfigSettings { worker_concurrency?: number; analysis_model_fast?: string; @@ -120,6 +149,35 @@ export async function loadMonitoredReposRaw(): Promise { return rawRepos; } +/** + * Resolve the branch-independent visual-preview policy for a repository. + * Multiple branch entries may exist for one repository; an explicitly enabled + * entry wins over disabled or legacy entries until the next synchronized save. + */ +export function resolveRepositoryVisualPreviewSettings( + repos: readonly RepoToMonitor[], + repository: string +): VisualPreviewSettings { + const normalizedRepository = repository.trim().toLowerCase(); + if (!normalizedRepository) return { enabled: false, types: ['image'] }; + + const matching = repos.filter(repo => repo.name.trim().toLowerCase() === normalizedRepository); + const configured = matching.find(repo => normalizeStoredVisualPreviewSettings(repo.visualPreview).enabled) + ?? matching.find(repo => repo.visualPreview !== undefined); + return normalizeStoredVisualPreviewSettings(configured?.visualPreview); +} + +export async function loadRepositoryVisualPreviewSettings(repository: string): Promise { + try { + const settings = resolveRepositoryVisualPreviewSettings(await loadMonitoredReposRaw(), repository); + logger.info({ repository, enabled: settings.enabled, types: settings.types }, 'Loaded repository visual-preview settings'); + return settings; + } catch (error) { + logger.warn({ repository, error: (error as Error).message }, 'Failed to load visual-preview settings; treating previews as disabled'); + return { enabled: false, types: ['image'] }; + } +} + export async function saveMonitoredRepos(repos: RepoToMonitor[], client?: Knex | Knex.Transaction): Promise { await saveConfig('repos_to_monitor', repos, client); logger.info({ repos }, 'Successfully saved monitored repositories'); diff --git a/packages/core/test/visualPreviewConfig.test.ts b/packages/core/test/visualPreviewConfig.test.ts new file mode 100644 index 000000000..96255bc16 --- /dev/null +++ b/packages/core/test/visualPreviewConfig.test.ts @@ -0,0 +1,51 @@ +import assert from 'node:assert/strict'; +import { after, test } from 'node:test'; +import { + normalizeStoredVisualPreviewSettings, + resolveRepositoryVisualPreviewSettings, + type RepoToMonitor +} from '../src/config/configManager.js'; +import { db } from '../src/db/connection.js'; + +after(async () => { + await db.destroy(); +}); + +test('stored visual preview settings are backward compatible and sanitized', () => { + assert.deepEqual(normalizeStoredVisualPreviewSettings(undefined), { + enabled: false, + types: ['image'] + }); + assert.deepEqual(normalizeStoredVisualPreviewSettings({ + enabled: true, + types: ['video', 'invalid', 'video'], + instructions: ' Focus the changed dialog. ' + }), { + enabled: true, + types: ['video'], + instructions: 'Focus the changed dialog.' + }); +}); + +test('repository visual preview settings are branch independent', () => { + const repos: RepoToMonitor[] = [ + { id: 'main', name: 'integry/propr', enabled: true, baseBranch: 'main' }, + { + id: 'release', + name: 'INTEGRY/PROPR', + enabled: true, + baseBranch: 'release', + visualPreview: { enabled: true, types: ['image', 'video'], instructions: 'Show both breakpoints.' } + } + ]; + + assert.deepEqual(resolveRepositoryVisualPreviewSettings(repos, 'integry/propr'), { + enabled: true, + types: ['image', 'video'], + instructions: 'Show both breakpoints.' + }); + assert.deepEqual(resolveRepositoryVisualPreviewSettings(repos, 'integry/other'), { + enabled: false, + types: ['image'] + }); +}); diff --git a/propr-ui/src/api/proprTypes.ts b/propr-ui/src/api/proprTypes.ts index 07cbfb8f6..9f37c899e 100644 --- a/propr-ui/src/api/proprTypes.ts +++ b/propr-ui/src/api/proprTypes.ts @@ -120,6 +120,12 @@ export interface MonitoredRepo { enabled: boolean; /** Whether failed CI triggers an automatic follow-up. Missing legacy values are off. */ autoFollowupOnFailedCi?: boolean; + /** Generated media to embed in PRs when a change has a visible result. */ + visualPreview?: { + enabled: boolean; + types: Array<'image' | 'video'>; + instructions?: string; + }; alias?: string; baseBranch?: string; starred?: boolean; diff --git a/propr-ui/src/components/RepositoryListContent.tsx b/propr-ui/src/components/RepositoryListContent.tsx index 500ad45b3..f4e71994b 100644 --- a/propr-ui/src/components/RepositoryListContent.tsx +++ b/propr-ui/src/components/RepositoryListContent.tsx @@ -33,6 +33,7 @@ interface RepositoryListContentProps { selectedRepoId: string | null; onToggle: (repoId: string) => void; onToggleAutoCiFollowup: (repoId: string) => void; + onUpdateVisualPreview: (repoId: string, settings: NonNullable) => void; onRemove: (repoId: string) => void; onStopIndexing: (repoName: string, baseBranch?: string) => void; onReindex: (repoName: string, baseBranch?: string) => void; @@ -51,6 +52,7 @@ export const RepositoryListContent: React.FC = ({ selectedRepoId, onToggle, onToggleAutoCiFollowup, + onUpdateVisualPreview, onRemove, onStopIndexing, onReindex, @@ -105,6 +107,7 @@ export const RepositoryListContent: React.FC = ({ indexingStatuses={indexingStatuses} onToggle={onToggle} onToggleAutoCiFollowup={onToggleAutoCiFollowup} + onUpdateVisualPreview={onUpdateVisualPreview} onRemove={onRemove} onStopIndexing={onStopIndexing} onReindex={onReindex} diff --git a/propr-ui/src/components/RepositoryListItem.tsx b/propr-ui/src/components/RepositoryListItem.tsx index 38290c357..1e05a4a19 100644 --- a/propr-ui/src/components/RepositoryListItem.tsx +++ b/propr-ui/src/components/RepositoryListItem.tsx @@ -1,5 +1,5 @@ -import React, { useState } from 'react'; -import { Github, RefreshCw, Star, Eye, EyeOff } from 'lucide-react'; +import React, { useEffect, useState } from 'react'; +import { Github, RefreshCw, Star, Eye, EyeOff, Image, Video } from 'lucide-react'; import { DeleteRepoDialog } from './DeleteRepoDialog'; import { RepositoryIndexingStatus, MonitoredRepo } from '../api/proprApi'; import { getRepoStatusKey } from '../api/repoIndexingApi'; @@ -222,11 +222,95 @@ const AutoCiFollowupControl: React.FC<{ ); }; +type VisualPreviewSettings = NonNullable; + +const VisualPreviewControl: React.FC<{ + repo: MonitoredRepo; + onUpdate: (repoId: string, settings: VisualPreviewSettings) => void; + isReadOnly: boolean; +}> = ({ repo, onUpdate, isReadOnly }) => { + const settings: VisualPreviewSettings = repo.visualPreview || { enabled: false, types: ['image'] }; + const [instructions, setInstructions] = useState(settings.instructions || ''); + + useEffect(() => setInstructions(settings.instructions || ''), [settings.instructions]); + + if (isReadOnly) return null; + + const toggleType = (type: 'image' | 'video') => { + const selected = settings.types.includes(type); + if (selected && settings.types.length === 1) return; + onUpdate(repo.id, { + ...settings, + types: selected ? settings.types.filter(candidate => candidate !== type) : [...settings.types, type] + }); + }; + + return ( +
event.stopPropagation()}> + + + {settings.enabled && ( +
event.stopPropagation()}> +
+ + +
+