diff --git a/CHANGELOG.md b/CHANGELOG.md index baa2820..ee3c250 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,11 +33,6 @@ Theme: Test Locally, Secure by Default. Catch workflow problems before pushing, - CLI restructured with standalone commands that don't require the app - Improved help text with examples for all commands -## [0.2.1] - 2025-12-26 - -### Fixed -- Minor bug fixes - ## [0.2.0] - 2025-12-26 Core improvements to architecture to enable multiple targets. @@ -57,7 +52,6 @@ Core improvements to architecture to enable multiple targets. Initial release of localmost, a Mac app which manages GitHub Actions runners. -[0.3.0]: https://github.com/bfulton/localmost/compare/v0.2.1...HEAD -[0.2.1]: https://github.com/bfulton/localmost/compare/v0.2.0...v0.2.1 +[0.3.0]: https://github.com/bfulton/localmost/compare/v0.2.0...HEAD [0.2.0]: https://github.com/bfulton/localmost/compare/v0.1.0...v0.2.0 [0.1.0]: https://github.com/bfulton/localmost/releases/tag/v0.1.0 diff --git a/e2e/app.spec.ts b/e2e/app.spec.ts index ae00220..d221e04 100644 --- a/e2e/app.spec.ts +++ b/e2e/app.spec.ts @@ -20,6 +20,20 @@ test.describe('localmost App', () => { expect(page).toBeDefined(); }); + test('should load preload script successfully', async () => { + // Verify window.localmost API is exposed by preload script + const hasLocalmostAPI = await page.evaluate(() => { + return typeof (window as any).localmost !== 'undefined'; + }); + expect(hasLocalmostAPI).toBe(true); + + // Verify window.zubridge API is exposed by preload script + const hasZubridgeAPI = await page.evaluate(() => { + return typeof (window as any).zubridge !== 'undefined'; + }); + expect(hasZubridgeAPI).toBe(true); + }); + test('should display the app title', async () => { const title = await page.locator('.titlebar h1').textContent(); expect(title).toBe('localmost'); diff --git a/scripts/process-shim.js b/scripts/process-shim.js new file mode 100644 index 0000000..f0037ef --- /dev/null +++ b/scripts/process-shim.js @@ -0,0 +1,22 @@ +/** + * Minimal process shim for sandboxed Electron preload scripts. + * + * The 'debug' library (bundled in @zubridge/electron) uses 'supports-color' + * which needs: + * - process.argv for hasFlag() checks + * - process.stderr.fd for useColors check + * - process.env for various environment checks + * + * DEBUG_COLORS=false disables color detection to avoid the process.stderr.fd check + */ +module.exports = { + argv: [], + env: { + DEBUG_COLORS: 'false', + }, + platform: 'browser', + version: '', + versions: {}, + stdout: { isTTY: false, fd: 1 }, + stderr: { isTTY: false, fd: 2 }, +}; diff --git a/scripts/tty-shim.js b/scripts/tty-shim.js new file mode 100644 index 0000000..d70405f --- /dev/null +++ b/scripts/tty-shim.js @@ -0,0 +1,9 @@ +/** + * Minimal tty shim for sandboxed Electron preload scripts. + * + * The 'supports-color' library (bundled in @zubridge/electron via debug) + * calls tty.isatty() to check if output is a terminal. + */ +module.exports = { + isatty: () => false, +}; diff --git a/src/cli/env.test.ts b/src/cli/env.test.ts index da0f44b..f4372b8 100644 --- a/src/cli/env.test.ts +++ b/src/cli/env.test.ts @@ -1,5 +1,5 @@ -import { jest, describe, it, expect, beforeEach } from '@jest/globals'; -import { parseEnvArgs, EnvOptions } from './env'; +import { describe, it, expect } from '@jest/globals'; +import { parseEnvArgs } from './env'; describe('CLI env command', () => { describe('parseEnvArgs', () => { diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 4f2f8cf..99af123 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -22,7 +22,6 @@ jest.mock('../shared/paths', () => ({ })); // Import after mocks are set up -import { spawn } from 'child_process'; describe('CLI index', () => { beforeEach(() => { diff --git a/src/cli/policy.test.ts b/src/cli/policy.test.ts index 5234e50..e45bc7d 100644 --- a/src/cli/policy.test.ts +++ b/src/cli/policy.test.ts @@ -1,5 +1,5 @@ -import { jest, describe, it, expect, beforeEach } from '@jest/globals'; -import { parsePolicyArgs, PolicyOptions } from './policy'; +import { describe, it, expect } from '@jest/globals'; +import { parsePolicyArgs } from './policy'; describe('CLI policy command', () => { describe('parsePolicyArgs', () => { diff --git a/src/cli/test.test.ts b/src/cli/test.test.ts index 2489a64..6cb2b1a 100644 --- a/src/cli/test.test.ts +++ b/src/cli/test.test.ts @@ -1,5 +1,5 @@ -import { jest, describe, it, expect, beforeEach } from '@jest/globals'; -import { parseTestArgs, TestOptions } from './test'; +import { describe, it, expect } from '@jest/globals'; +import { parseTestArgs } from './test'; describe('CLI test command', () => { describe('parseTestArgs', () => { diff --git a/src/cli/test.ts b/src/cli/test.ts index 7a5b75d..d91f630 100644 --- a/src/cli/test.ts +++ b/src/cli/test.ts @@ -23,6 +23,11 @@ import { ParsedWorkflow, WorkflowJob, MatrixCombination, + isReusableWorkflowJob, + parseReusableWorkflow, + resolveReusableWorkflowPath, + resolveReusableWorkflowInputs, + ReusableWorkflow, } from '../shared/workflow-parser'; import { executeStep, @@ -34,7 +39,6 @@ import { findLocalmostrc, parseLocalmostrc, getEffectivePolicy, - getRequiredSecrets, LocalmostrcConfig, serializeLocalmostrc, LOCALMOSTRC_VERSION, @@ -92,6 +96,13 @@ export interface JobResult { steps: StepResult[]; status: 'success' | 'failure' | 'skipped'; duration: number; + /** Outputs from this job (for use by dependent jobs) */ + outputs?: Record; +} + +/** Tracks outputs from completed jobs for dependency resolution */ +interface JobOutputs { + [jobId: string]: Record; } // ============================================================================= @@ -269,12 +280,37 @@ export async function runTest(options: TestOptions = {}): Promise { // Run jobs const jobResults: JobResult[] = []; + const jobOutputs: JobOutputs = {}; for (const jobId of jobsToRun) { const job = workflow.workflow.jobs[jobId]; const jobName = job.name || jobId; - // Determine matrix combinations + // Check if this is a reusable workflow call + if (isReusableWorkflowJob(job)) { + console.log(`${colors.bold}\u25B6 ${jobName}${colors.reset} ${colors.dim}(reusable workflow)${colors.reset}`); + + const reusableResult = await runReusableWorkflowJob( + jobId, + job, + workflowPath, + { ...context, jobEnv: { ...context.jobEnv, GITHUB_JOB: jobId } }, + jobOutputs, + options + ); + + jobResults.push(reusableResult); + + // Store outputs for dependent jobs + if (reusableResult.outputs) { + jobOutputs[jobId] = reusableResult.outputs; + } + + console.log(); + continue; + } + + // Regular job - determine matrix combinations const combinations = generateMatrixCombinations(job.strategy); let combinationsToRun: MatrixCombination[]; @@ -305,10 +341,17 @@ export async function runTest(options: TestOptions = {}): Promise { job, matrix, { ...context, matrix, jobEnv: { ...context.jobEnv, GITHUB_JOB: jobId, ...(job.env || {}) } }, + jobOutputs, options ); jobResults.push(jobResult); + + // Store outputs for dependent jobs + if (jobResult.outputs) { + jobOutputs[jobId] = jobResult.outputs; + } + console.log(); } } @@ -328,12 +371,22 @@ export async function runTest(options: TestOptions = {}): Promise { console.log(formatEnvironmentInfo(localEnv)); console.log(); - // Compare to first job's runs-on - const firstJob = workflow.workflow.jobs[jobsToRun[0]]; - const runsOn = Array.isArray(firstJob['runs-on']) ? firstJob['runs-on'][0] : firstJob['runs-on']; - const diffs = compareEnvironments(localEnv, runsOn); - environmentDiffs = formatEnvironmentDiff(diffs); - console.log(environmentDiffs); + // Compare to first non-reusable job's runs-on + let runsOn: string | undefined; + for (const jobId of jobsToRun) { + const job = workflow.workflow.jobs[jobId]; + if (job['runs-on']) { + runsOn = Array.isArray(job['runs-on']) ? job['runs-on'][0] : job['runs-on']; + break; + } + } + if (runsOn) { + const diffs = compareEnvironments(localEnv, runsOn); + environmentDiffs = formatEnvironmentDiff(diffs); + console.log(environmentDiffs); + } else { + console.log(' (No runs-on to compare - all jobs are reusable workflows)'); + } } // Show summary @@ -373,20 +426,27 @@ async function runJob( job: WorkflowJob, matrix: MatrixCombination, context: ExecutionContext, + jobOutputs: JobOutputs, options: TestOptions ): Promise { const startTime = Date.now(); const stepResults: StepResult[] = []; let jobStatus: 'success' | 'failure' | 'skipped' = 'success'; - for (const step of job.steps) { + // Create context with job outputs available for expression substitution + const jobContext = { + ...context, + needs: jobOutputs, + }; + + for (const step of job.steps!) { if (options.dryRun) { const stepName = step.name || step.id || (step.uses ? `Run ${step.uses}` : 'Run script'); console.log(` ${pending(stepName)} (dry run)`); continue; } - const result = await executeStep(step, context, job); + const result = await executeStep(step, jobContext, job); stepResults.push(result); // Print step result @@ -407,6 +467,9 @@ async function runJob( } } + // Extract job outputs from step outputs + const outputs = extractJobOutputs(job, context.stepOutputs); + return { jobId, jobName: job.name || jobId, @@ -414,9 +477,200 @@ async function runJob( steps: stepResults, status: jobStatus, duration: Date.now() - startTime, + outputs, + }; +} + +/** + * Run a reusable workflow job. + */ +async function runReusableWorkflowJob( + jobId: string, + job: WorkflowJob, + callerWorkflowPath: string, + context: ExecutionContext, + jobOutputs: JobOutputs, + options: TestOptions +): Promise { + const startTime = Date.now(); + + // Resolve the workflow path + const workflowPath = resolveReusableWorkflowPath(job.uses!, callerWorkflowPath); + if (!workflowPath) { + console.log(` ${colors.yellow}Skipping: Remote reusable workflows not supported${colors.reset}`); + console.log(` ${colors.dim}uses: ${job.uses}${colors.reset}`); + return { + jobId, + jobName: job.name || jobId, + steps: [], + status: 'skipped', + duration: Date.now() - startTime, + }; + } + + // Load and parse the reusable workflow + let reusableWorkflow: ReusableWorkflow; + try { + reusableWorkflow = parseReusableWorkflow(workflowPath); + console.log(` ${colors.dim}Loading: ${path.basename(workflowPath)}${colors.reset}`); + } catch (err) { + console.log(` ${colors.red}Error loading workflow: ${(err as Error).message}${colors.reset}`); + return { + jobId, + jobName: job.name || jobId, + steps: [], + status: 'failure', + duration: Date.now() - startTime, + }; + } + + // Resolve inputs + const inputs = resolveReusableWorkflowInputs(job.with, reusableWorkflow.inputs); + if (Object.keys(inputs).length > 0) { + console.log(` ${colors.dim}Inputs: ${Object.entries(inputs).map(([k, v]) => `${k}=${v}`).join(', ')}${colors.reset}`); + } + + // Run all jobs in the called workflow + const allStepResults: StepResult[] = []; + let overallStatus: 'success' | 'failure' | 'skipped' = 'success'; + const calledJobOutputs: JobOutputs = {}; + + for (const calledJobId of reusableWorkflow.jobOrder) { + const calledJob = reusableWorkflow.workflow.jobs[calledJobId]; + + // Skip reusable workflow jobs within reusable workflows (nested not supported yet) + if (isReusableWorkflowJob(calledJob)) { + console.log(` ${colors.yellow}Skipping nested reusable workflow: ${calledJobId}${colors.reset}`); + continue; + } + + const calledJobName = calledJob.name || calledJobId; + console.log(` ${colors.cyan}▸ ${calledJobName}${colors.reset}`); + + // Create context with inputs available + const calledContext: ExecutionContext = { + ...context, + workflowEnv: { + ...context.workflowEnv, + ...(reusableWorkflow.workflow.env || {}), + }, + jobEnv: { + ...context.jobEnv, + GITHUB_JOB: calledJobId, + ...(calledJob.env || {}), + }, + // Make inputs available as inputs.* context + inputs, + stepOutputs: {}, + needs: { ...jobOutputs, ...calledJobOutputs }, + }; + + // Run steps in the called job + for (const step of calledJob.steps!) { + if (options.dryRun) { + const stepName = step.name || step.id || (step.uses ? `Run ${step.uses}` : 'Run script'); + console.log(` ${pending(stepName)} (dry run)`); + continue; + } + + const result = await executeStep(step, calledContext, calledJob); + allStepResults.push(result); + + if (!options.verbose) { + console.log(` ${formatStepStatus(result.status, result.name, result.duration)}`); + } + + if (result.status === 'failure') { + overallStatus = 'failure'; + if (result.error) { + console.log(` ${colors.red}Error: ${result.error}${colors.reset}`); + } + if (!step['continue-on-error']) { + break; + } + } + } + + // Extract outputs from this job + const calledOutputs = extractJobOutputs(calledJob, calledContext.stepOutputs); + if (calledOutputs) { + calledJobOutputs[calledJobId] = calledOutputs; + } + + if (overallStatus === 'failure') { + break; + } + } + + // Map workflow-level outputs from job outputs + const workflowOutputs = extractWorkflowOutputs(reusableWorkflow, calledJobOutputs); + + return { + jobId, + jobName: job.name || jobId, + steps: allStepResults, + status: overallStatus, + duration: Date.now() - startTime, + outputs: workflowOutputs, }; } +/** + * Extract job outputs from step outputs using the job's output definitions. + */ +function extractJobOutputs( + job: WorkflowJob, + stepOutputs: Record> +): Record | undefined { + if (!job.outputs) { + return undefined; + } + + const result: Record = {}; + + for (const [outputName, expression] of Object.entries(job.outputs)) { + // Parse expressions like ${{ steps.check.outputs.runner }} + const match = expression.match(/\$\{\{\s*steps\.(\w+)\.outputs\.(\w+)\s*\}\}/); + if (match) { + const [, stepId, outputKey] = match; + const value = stepOutputs[stepId]?.[outputKey]; + if (value !== undefined) { + result[outputName] = value; + } + } + } + + return Object.keys(result).length > 0 ? result : undefined; +} + +/** + * Extract workflow-level outputs from job outputs. + */ +function extractWorkflowOutputs( + workflow: ReusableWorkflow, + jobOutputs: JobOutputs +): Record | undefined { + if (Object.keys(workflow.outputs).length === 0) { + return undefined; + } + + const result: Record = {}; + + for (const [outputName, outputDef] of Object.entries(workflow.outputs)) { + // Parse expressions like ${{ jobs.check.outputs.runner }} + const match = outputDef.value.match(/\$\{\{\s*jobs\.(\w+)\.outputs\.(\w+)\s*\}\}/); + if (match) { + const [, jobId, outputKey] = match; + const value = jobOutputs[jobId]?.[outputKey]; + if (value !== undefined) { + result[outputName] = value; + } + } + } + + return Object.keys(result).length > 0 ? result : undefined; +} + // ============================================================================= // Helpers // ============================================================================= @@ -507,7 +761,7 @@ async function resolveSecrets( async function handleUpdateRc( cwd: string, workflow: ParsedWorkflow, - context: ExecutionContext + _context: ExecutionContext ): Promise { console.log(); console.log(`${colors.bold}Discovery mode:${colors.reset}`); diff --git a/src/main/broker-proxy-service.ts b/src/main/broker-proxy-service.ts index 3a36468..b86d1c6 100644 --- a/src/main/broker-proxy-service.ts +++ b/src/main/broker-proxy-service.ts @@ -182,6 +182,8 @@ export interface GitHubJobInfo { githubJobId?: number; githubRepo?: string; githubActor?: string; // Username who triggered the workflow + githubSha?: string; // Commit SHA that triggered the workflow + githubRef?: string; // Branch/tag ref (e.g., refs/heads/main) } export interface BrokerProxyEvents { @@ -430,6 +432,8 @@ export class BrokerProxyService extends EventEmitter { let githubJobId: number | undefined; let githubRepo: string | undefined; let githubActor: string | undefined; + let githubSha: string | undefined; + let githubRef: string | undefined; if (runServiceUrl) { const jobDetails = await this.acquireJobUpstream(state, instance, jobId, runServiceUrl, billingOwnerId); if (jobDetails) { @@ -437,7 +441,7 @@ export class BrokerProxyService extends EventEmitter { this.acquiredJobDetails.set(messageId, jobDetails); log()?.info(`[BrokerProxy] Acquired job ${jobId} (messageId=${messageId}) upstream, stored details`); - // Extract run_id, job ID, and actor from job details + // Extract run_id, job ID, actor, sha, ref from job details try { const parsed = JSON.parse(jobDetails); @@ -448,6 +452,8 @@ export class BrokerProxyService extends EventEmitter { if (item.k === 'run_id') githubRunId = parseInt(item.v, 10); if (item.k === 'repository') githubRepo = item.v; if (item.k === 'actor') githubActor = item.v; + if (item.k === 'sha') githubSha = item.v; + if (item.k === 'ref') githubRef = item.v; } } @@ -459,7 +465,7 @@ export class BrokerProxyService extends EventEmitter { } } - log()?.info(`[BrokerProxy] Extracted: run_id=${githubRunId}, job_id=${githubJobId}, repo=${githubRepo}, actor=${githubActor}`); + log()?.info(`[BrokerProxy] Extracted: run_id=${githubRunId}, job_id=${githubJobId}, repo=${githubRepo}, actor=${githubActor}, sha=${githubSha?.slice(0, 7)}`); } catch (e) { log()?.warn(`[BrokerProxy] Failed to parse job details for IDs: ${(e as Error).message}`); } @@ -504,6 +510,8 @@ export class BrokerProxyService extends EventEmitter { githubJobId, githubRepo, githubActor, + githubSha, + githubRef, }); this.emitStatusUpdate(); } else { @@ -560,10 +568,15 @@ export class BrokerProxyService extends EventEmitter { ); // Wait for all deletions with a timeout to not block shutdown forever + let timeoutId: NodeJS.Timeout | undefined; + const timeoutPromise = new Promise(resolve => { + timeoutId = setTimeout(resolve, 5000); + }); await Promise.race([ Promise.allSettled(deletionPromises), - new Promise(resolve => setTimeout(resolve, 5000)), // 5 second timeout + timeoutPromise, ]); + if (timeoutId) clearTimeout(timeoutId); return new Promise((resolve) => { // Force close all connections immediately - don't wait for long-polls to timeout diff --git a/src/main/cli-install.ts b/src/main/cli-install.ts index 2d4ee93..40bdeb1 100644 --- a/src/main/cli-install.ts +++ b/src/main/cli-install.ts @@ -54,7 +54,7 @@ export async function getCliInstallStatus(): Promise<{ const pointsToThisApp = target === expectedTarget; return { installed: true, pointsToThisApp, currentTarget: target }; - } catch (error) { + } catch { // File doesn't exist or can't be read return { installed: false, pointsToThisApp: false }; } diff --git a/src/main/contributor-cache.ts b/src/main/contributor-cache.ts new file mode 100644 index 0000000..5a6ad3d --- /dev/null +++ b/src/main/contributor-cache.ts @@ -0,0 +1,171 @@ +/** + * Contributor Cache + * + * Caches repository contributors to efficiently check if all code authors + * are trusted. Uses SHA-based cache invalidation instead of TTL: + * - Store contributors with the default branch SHA when fetched + * - On subsequent checks, fetch commits since that SHA and add their authors + * - This is deterministic and never stale + */ + +import { GitHubAuth } from './github-auth'; + +/** Cache entry for a repository */ +interface RepoCacheEntry { + /** Set of contributor logins (lowercase) */ + contributors: Set; + /** SHA of default branch when contributors were fetched */ + defaultBranchSha: string; + /** When the cache entry was created */ + fetchedAt: Date; +} + +/** Logger function type */ +type LogFn = (message: string) => void; + +/** + * Cache for repository contributors. + * Provides efficient lookup of all authors who have contributed to a repo. + */ +export class ContributorCache { + private cache: Map = new Map(); + private githubAuth: GitHubAuth; + private log: LogFn; + + constructor(githubAuth: GitHubAuth, log?: LogFn) { + this.githubAuth = githubAuth; + this.log = log || (() => {}); + } + + /** + * Get cache key for a repo + */ + private getCacheKey(owner: string, repo: string): string { + return `${owner.toLowerCase()}/${repo.toLowerCase()}`; + } + + /** + * Get all authors for a repository at a given commit SHA. + * + * This combines: + * 1. Cached contributors (from API at time of cache) + * 2. Commit authors since the cached SHA + * + * @param accessToken GitHub access token + * @param owner Repository owner + * @param repo Repository name + * @param jobSha The commit SHA for the job being checked + * @returns Set of all author logins (lowercase) + */ + async getAllAuthors( + accessToken: string, + owner: string, + repo: string, + jobSha: string + ): Promise> { + const cacheKey = this.getCacheKey(owner, repo); + let entry = this.cache.get(cacheKey); + + if (!entry) { + // Cache miss - fetch contributors and current default branch SHA + this.log(`[ContributorCache] Cache miss for ${owner}/${repo}, fetching contributors...`); + entry = await this.fetchAndCache(accessToken, owner, repo); + } + + // Start with cached contributors + const authors = new Set(entry.contributors); + + // If the job SHA is different from cached SHA, fetch commits since then + if (jobSha !== entry.defaultBranchSha) { + this.log(`[ContributorCache] Fetching commits from ${entry.defaultBranchSha.slice(0, 7)} to ${jobSha.slice(0, 7)}`); + try { + const newAuthors = await this.githubAuth.getCommitAuthors( + accessToken, + owner, + repo, + entry.defaultBranchSha, + jobSha + ); + + for (const author of newAuthors) { + authors.add(author); + } + + if (newAuthors.length > 0) { + this.log(`[ContributorCache] Found ${newAuthors.length} new author(s) in commits`); + } + } catch (error) { + // If we can't get commits (e.g., SHA not found after force push), + // we should re-fetch the full contributor list to be safe + this.log(`[ContributorCache] Failed to get commits, re-fetching contributors: ${(error as Error).message}`); + entry = await this.fetchAndCache(accessToken, owner, repo); + return new Set(entry.contributors); + } + } + + return authors; + } + + /** + * Fetch contributors and cache them with the current default branch SHA. + */ + private async fetchAndCache( + accessToken: string, + owner: string, + repo: string + ): Promise { + const cacheKey = this.getCacheKey(owner, repo); + + // Fetch contributors and default branch info in parallel + const [contributors, branchInfo] = await Promise.all([ + this.githubAuth.getContributors(accessToken, owner, repo), + this.githubAuth.getDefaultBranch(accessToken, owner, repo), + ]); + + const entry: RepoCacheEntry = { + contributors: new Set(contributors), + defaultBranchSha: branchInfo.sha, + fetchedAt: new Date(), + }; + + this.cache.set(cacheKey, entry); + this.log(`[ContributorCache] Cached ${contributors.length} contributors for ${owner}/${repo} at ${branchInfo.sha.slice(0, 7)}`); + + return entry; + } + + /** + * Invalidate cache for a repository. + * Call this when a target is removed. + */ + invalidate(owner: string, repo: string): void { + const cacheKey = this.getCacheKey(owner, repo); + if (this.cache.delete(cacheKey)) { + this.log(`[ContributorCache] Invalidated cache for ${owner}/${repo}`); + } + } + + /** + * Clear all cached entries. + */ + clear(): void { + this.cache.clear(); + this.log('[ContributorCache] Cleared all cache entries'); + } + + /** + * Get cache statistics for debugging. + */ + getStats(): { repoCount: number; entries: Array<{ repo: string; contributorCount: number; age: number }> } { + const entries = Array.from(this.cache.entries()).map(([key, entry]) => ({ + repo: key, + contributorCount: entry.contributors.size, + age: Date.now() - entry.fetchedAt.getTime(), + })); + + return { + repoCount: this.cache.size, + entries, + }; + } +} diff --git a/src/main/github-auth.ts b/src/main/github-auth.ts index 31965e0..784c92e 100644 --- a/src/main/github-auth.ts +++ b/src/main/github-auth.ts @@ -595,4 +595,108 @@ export class GitHubAuth { return usersWithNames; } + + /** + * Get all contributors for a repository. + * Returns array of contributor logins (paginated). + */ + async getContributors( + accessToken: string, + owner: string, + repo: string + ): Promise { + const client = new GitHubClient(accessToken); + const contributors: string[] = []; + let page = 1; + const perPage = 100; + + while (true) { + const data = await client.get>( + `/repos/${owner}/${repo}/contributors`, + { params: { per_page: String(perPage), page: String(page), anon: '0' } } + ); + + if (!data || data.length === 0) { + break; + } + + for (const contributor of data) { + if (contributor.login) { + contributors.push(contributor.login.toLowerCase()); + } + } + + if (data.length < perPage) { + break; + } + page++; + } + + return contributors; + } + + /** + * Get commit authors between two SHAs. + * Returns array of author logins for commits from baseSha to headSha. + */ + async getCommitAuthors( + accessToken: string, + owner: string, + repo: string, + baseSha: string, + headSha: string + ): Promise { + const client = new GitHubClient(accessToken); + const authors = new Set(); + + try { + // Use compare API to get commits between two refs + const data = await client.get<{ + commits: Array<{ + author: { login: string } | null; + commit: { author: { name: string } | null }; + }>; + }>(`/repos/${owner}/${repo}/compare/${baseSha}...${headSha}`); + + for (const commit of data.commits || []) { + // Prefer the GitHub user login if available + if (commit.author?.login) { + authors.add(commit.author.login.toLowerCase()); + } + } + } catch (error) { + // If compare fails (e.g., baseSha not found), return empty + // This can happen if the repo was force-pushed + console.error(`Failed to get commit authors: ${(error as Error).message}`); + } + + return Array.from(authors); + } + + /** + * Get default branch info for a repository. + * Returns the branch name and current HEAD SHA. + */ + async getDefaultBranch( + accessToken: string, + owner: string, + repo: string + ): Promise<{ name: string; sha: string }> { + const client = new GitHubClient(accessToken); + + // Get repo info to find default branch name + const repoData = await client.get<{ default_branch: string }>( + `/repos/${owner}/${repo}` + ); + + // Get the branch to find HEAD SHA + const branchData = await client.get<{ commit: { sha: string } }>( + `/repos/${owner}/${repo}/branches/${repoData.default_branch}` + ); + + return { + name: repoData.default_branch, + sha: branchData.commit.sha, + }; + } } diff --git a/src/main/index.ts b/src/main/index.ts index 03d0b85..8f731af 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -10,6 +10,7 @@ import { RunnerDownloader } from './runner-downloader'; import { HeartbeatManager } from './heartbeat-manager'; import { BrokerProxyService } from './broker-proxy-service'; import { TargetManager } from './target-manager'; +import { ContributorCache } from './contributor-cache'; // State management import { @@ -97,7 +98,7 @@ import { } from './runner-state-service'; // Zustand store -import { initStore, connectWindow, cleanupStore } from './store/init'; +import { initStore, connectWindow, cleanupStore, store } from './store/init'; // ============================================================================ // App Initialization @@ -200,6 +201,9 @@ app.whenReady().then(async () => { const githubAuth = new GitHubAuth(); setGitHubAuth(githubAuth); + // Contributor cache for user filtering + const contributorCache = new ContributorCache(githubAuth, (msg) => logger?.debug(msg)); + const runnerManager = new RunnerManager({ onLog: sendLog, onStatusChange: sendStatusUpdate, @@ -231,6 +235,13 @@ app.whenReady().then(async () => { } return auth.getJobConclusion(accessToken, owner, repo, jobId); }, + getAllContributors: async (owner: string, repo: string, sha: string) => { + const accessToken = await getValidAccessToken(); + if (!accessToken) { + throw new Error('Not authenticated'); + } + return contributorCache.getAllAuthors(accessToken, owner, repo, sha); + }, onJobEvent: (event: JobEvent) => { logger?.info(`Job event: ${event.type} ${event.jobName}`); @@ -310,7 +321,7 @@ app.whenReady().then(async () => { // Wire up broker proxy to runner manager: when a job is received, spawn a worker brokerProxyService.on('job-received', async (targetId: string, jobId: string, _registeredRunnerName: string, githubInfo) => { - getLogger()?.info(`[job-received event] targetId=${targetId}, jobId=${jobId}, runId=${githubInfo.githubRunId}, actor=${githubInfo.githubActor}`); + getLogger()?.info(`[job-received event] targetId=${targetId}, jobId=${jobId}, runId=${githubInfo.githubRunId}, actor=${githubInfo.githubActor}, sha=${githubInfo.githubSha?.slice(0, 7)}`); const target = targetManager.getTargets().find(t => t.id === targetId); if (target) { getLogger()?.info(`Spawning worker for job ${jobId} from ${target.displayName}...`); @@ -320,7 +331,7 @@ app.whenReady().then(async () => { actionsUrl = `https://github.com/${githubInfo.githubRepo}/actions/runs/${githubInfo.githubRunId}/job/${githubInfo.githubJobId}`; getLogger()?.info(`Constructed actions URL: ${actionsUrl}`); } - runnerManager.setPendingTargetContext('next', targetId, target.displayName, actionsUrl, githubInfo.githubRunId, githubInfo.githubJobId, githubInfo.githubActor); + runnerManager.setPendingTargetContext('next', targetId, target.displayName, actionsUrl, githubInfo.githubRunId, githubInfo.githubJobId, githubInfo.githubActor, githubInfo.githubSha, githubInfo.githubRef); // Spawn a worker to handle this job try { @@ -356,6 +367,10 @@ app.whenReady().then(async () => { if (config.auth) { setAuthState(config.auth); + // Also update store so zubridge syncs user to renderer + if (config.auth.user) { + store.getState().setUser(config.auth.user); + } } if (config.sleepProtection) { setSleepProtectionSetting(config.sleepProtection as SleepProtection); @@ -428,9 +443,18 @@ app.whenReady().then(async () => { // Start monitoring (will evaluate conditions and emit events as needed) resourceMonitor.start(); + // Determine if window should be hidden on start + // Only hide if setting is enabled AND runner is configured (don't hide setup wizard) + const isRunnerConfigured = runnerManager.isConfigured(); + logger?.info(`hideOnStart check: hideOnStart=${config.hideOnStart}, isConfigured=${isRunnerConfigured}`); + const shouldHideOnStart = config.hideOnStart && isRunnerConfigured; + if (shouldHideOnStart) { + logger?.info('Window will be hidden on start (hideOnStart enabled)'); + } + // Create UI createMenu(); - createWindow(); + createWindow({ show: !shouldHideOnStart }); initTray(); setDockIcon(); setupIpcHandlers(); @@ -441,6 +465,20 @@ app.whenReady().then(async () => { connectWindow(newMainWindow); } + // Initialize store with current state so renderer has data immediately + const isDownloaded = runnerDownloader.isDownloaded(); + store.getState().setIsDownloaded(isDownloaded); + if (isDownloaded) { + const version = runnerDownloader.getVersion(); + const url = runnerDownloader.getVersionUrl(); + store.getState().setRunnerVersion({ version, url }); + } + store.getState().setIsConfigured(runnerManager.isConfigured()); + store.getState().setTargets(config.targets || []); + + // Mark initial loading as complete so renderer shows the UI + store.getState().setIsInitialLoading(false); + // Initialize auto-updater const mainWindow = getMainWindow(); if (mainWindow) { diff --git a/src/main/ipc-handlers/auth.test.ts b/src/main/ipc-handlers/auth.test.ts new file mode 100644 index 0000000..218bd87 --- /dev/null +++ b/src/main/ipc-handlers/auth.test.ts @@ -0,0 +1,183 @@ +import { jest } from '@jest/globals'; + +// Mock electron +const mockHandle = jest.fn<(channel: string, handler: (...args: any[]) => any) => void>(); +jest.mock('electron', () => ({ + ipcMain: { + handle: mockHandle, + }, + shell: { + openExternal: jest.fn(), + }, + clipboard: { + writeText: jest.fn(), + }, +})); + +// Mock store +const mockSetUser = jest.fn(); +const mockSetDeviceCode = jest.fn(); +const mockSetIsAuthenticating = jest.fn(); +const mockLogout = jest.fn(); +const mockSetRepos = jest.fn(); +const mockSetOrgs = jest.fn(); +jest.mock('../store/init', () => ({ + store: { + getState: () => ({ + setUser: mockSetUser, + setDeviceCode: mockSetDeviceCode, + setIsAuthenticating: mockSetIsAuthenticating, + logout: mockLogout, + setRepos: mockSetRepos, + setOrgs: mockSetOrgs, + }), + }, +})); + +// Mock GitHubAuth +const mockWaitForAuth = jest.fn<() => Promise>(); +const mockStartDeviceFlow = jest.fn<() => Promise>(); +jest.mock('../github-auth', () => ({ + GitHubAuth: jest.fn().mockImplementation(() => ({ + startDeviceFlow: mockStartDeviceFlow, + abortPolling: jest.fn(), + })), + DEFAULT_CLIENT_ID: 'test-client-id', +})); + +// Mock other dependencies +jest.mock('../config', () => ({ + loadConfig: jest.fn(() => ({})), + saveConfig: jest.fn(), +})); + +jest.mock('../auth-tokens', () => ({ + getValidAccessToken: jest.fn(), +})); + +jest.mock('../app-state', () => ({ + getGitHubAuth: jest.fn(), + setGitHubAuth: jest.fn(), + getAuthState: jest.fn(), + setAuthState: jest.fn(), + getLogger: jest.fn(() => ({ + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + })), +})); + +jest.mock('../tray-init', () => ({ + updateTrayMenu: jest.fn(), +})); + +import { registerAuthHandlers } from './auth'; +import { clipboard } from 'electron'; + +describe('auth IPC handlers', () => { + let handlers: Record any>; + + beforeEach(() => { + jest.clearAllMocks(); + handlers = {}; + + // Capture the handlers when registered + mockHandle.mockImplementation((channel, handler) => { + handlers[channel] = handler; + }); + + registerAuthHandlers(); + }); + + describe('github:auth-device-flow', () => { + const mockUser = { login: 'testuser', id: 123, avatar_url: 'https://example.com/avatar.png' }; + + beforeEach(() => { + mockStartDeviceFlow.mockResolvedValue({ + status: { + userCode: 'ABCD-1234', + verificationUri: 'https://github.com/login/device', + }, + waitForAuth: mockWaitForAuth, + }); + }); + + it('should set user in store after successful auth', async () => { + mockWaitForAuth.mockResolvedValue({ + user: mockUser, + accessToken: 'test-token', + }); + + const result = await handlers['github:auth-device-flow'](); + + expect(result.success).toBe(true); + expect(result.user).toEqual(mockUser); + expect(mockSetUser).toHaveBeenCalledWith(mockUser); + }); + + it('should copy device code to clipboard', async () => { + mockWaitForAuth.mockResolvedValue({ + user: mockUser, + accessToken: 'test-token', + }); + + await handlers['github:auth-device-flow'](); + + expect(clipboard.writeText).toHaveBeenCalledWith('ABCD-1234'); + }); + + it('should set device code with copiedToClipboard flag', async () => { + mockWaitForAuth.mockResolvedValue({ + user: mockUser, + accessToken: 'test-token', + }); + + await handlers['github:auth-device-flow'](); + + expect(mockSetDeviceCode).toHaveBeenCalledWith({ + userCode: 'ABCD-1234', + verificationUri: 'https://github.com/login/device', + copiedToClipboard: true, + }); + }); + + it('should clear device code after successful auth', async () => { + mockWaitForAuth.mockResolvedValue({ + user: mockUser, + accessToken: 'test-token', + }); + + await handlers['github:auth-device-flow'](); + + // First call sets the device code, second call clears it + expect(mockSetDeviceCode).toHaveBeenLastCalledWith(null); + }); + + it('should clear auth state on error', async () => { + mockWaitForAuth.mockRejectedValue(new Error('Auth failed')); + + const result = await handlers['github:auth-device-flow'](); + + expect(result.success).toBe(false); + expect(mockSetDeviceCode).toHaveBeenLastCalledWith(null); + expect(mockSetIsAuthenticating).toHaveBeenLastCalledWith(false); + }); + }); + + describe('github:auth-logout', () => { + it('should call logout on store', () => { + handlers['github:auth-logout'](); + + expect(mockLogout).toHaveBeenCalled(); + }); + }); + + describe('github:auth-cancel', () => { + it('should clear device code and auth state', () => { + handlers['github:auth-cancel'](); + + expect(mockSetDeviceCode).toHaveBeenCalledWith(null); + expect(mockSetIsAuthenticating).toHaveBeenCalledWith(false); + }); + }); +}); diff --git a/src/main/ipc-handlers/auth.ts b/src/main/ipc-handlers/auth.ts index 8648eac..c12537d 100644 --- a/src/main/ipc-handlers/auth.ts +++ b/src/main/ipc-handlers/auth.ts @@ -2,13 +2,12 @@ * IPC handlers for GitHub authentication. */ -import { ipcMain, shell } from 'electron'; +import { ipcMain, shell, clipboard } from 'electron'; import { GitHubAuth, DEFAULT_CLIENT_ID } from '../github-auth'; import { toUserError } from '../user-error'; import { loadConfig, saveConfig } from '../config'; import { getValidAccessToken } from '../auth-tokens'; import { - getMainWindow, getGitHubAuth, setGitHubAuth, getAuthState, @@ -17,6 +16,7 @@ import { } from '../app-state'; import { updateTrayMenu } from '../tray-init'; import { IPC_CHANNELS, GitHubRepo, GitHubUserSearchResult } from '../../shared/types'; +import { store } from '../store/init'; /** * Validate that a URL is a legitimate GitHub URL before opening externally. @@ -35,11 +35,11 @@ function isValidGitHubUrl(url: string): boolean { * Safely open a GitHub verification URL in the user's browser. * Validates the URL is actually GitHub before opening. */ -function openGitHubVerificationUrl(url: string, logger?: { error: (msg: string) => void }): void { +function openGitHubVerificationUrl(url: string): void { if (isValidGitHubUrl(url)) { shell.openExternal(url); } else { - logger?.error(`Refusing to open suspicious verification URL: ${url}`); + getLogger()?.error(`Refusing to open suspicious verification URL: ${url}`); } } @@ -86,7 +86,7 @@ export const registerAuthHandlers = (): void => { const { status, waitForAuth } = await githubAuth.startDeviceFlow(); // Open the verification URL (with validation) - openGitHubVerificationUrl(status.verificationUri, logger); + openGitHubVerificationUrl(status.verificationUri); // Wait for user to complete auth const result = await waitForAuth(); @@ -107,22 +107,37 @@ export const registerAuthHandlers = (): void => { const clientId = config.githubClientId || DEFAULT_CLIENT_ID; try { + // Mark auth as in progress + store.getState().setIsAuthenticating(true); + const githubAuth = new GitHubAuth(clientId); setGitHubAuth(githubAuth); const { status, waitForAuth } = await githubAuth.startDeviceFlow(); - // Send the code to the renderer to display - const mainWindow = getMainWindow(); - mainWindow?.webContents.send(IPC_CHANNELS.GITHUB_DEVICE_CODE, { + // Copy code to clipboard for easy pasting + clipboard.writeText(status.userCode); + + // Set device code in store (syncs to renderer via zubridge) + store.getState().setDeviceCode({ userCode: status.userCode, verificationUri: status.verificationUri, + copiedToClipboard: true, }); + // Wait briefly so user can see the code was copied before browser opens + await new Promise(resolve => setTimeout(resolve, 1500)); + // Open the verification URL in the browser (with validation) - openGitHubVerificationUrl(status.verificationUri, logger); + openGitHubVerificationUrl(status.verificationUri); // Wait for user to complete auth const result = await waitForAuth(); + + // Update store with user and clear auth flow state + store.getState().setUser(result.user); + store.getState().setDeviceCode(null); + store.getState().setIsAuthenticating(false); + setAuthState(result); updateTrayMenu(); @@ -133,6 +148,10 @@ export const registerAuthHandlers = (): void => { return { success: true, user: result.user }; } catch (error) { + // Clear device code and auth state on error + store.getState().setDeviceCode(null); + store.getState().setIsAuthenticating(false); + const { userMessage, technicalDetails } = toUserError(error, 'Authentication'); logger?.error(technicalDetails); return { success: false, error: userMessage }; @@ -142,11 +161,18 @@ export const registerAuthHandlers = (): void => { ipcMain.handle(IPC_CHANNELS.GITHUB_AUTH_CANCEL, () => { const githubAuth = getGitHubAuth(); githubAuth?.abortPolling(); + // Clear device code and auth state when cancelled + store.getState().setDeviceCode(null); + store.getState().setIsAuthenticating(false); return { success: true }; }); ipcMain.handle(IPC_CHANNELS.GITHUB_AUTH_STATUS, () => { const authState = getAuthState(); + // Update store so zubridge syncs to renderer + if (authState?.user) { + store.getState().setUser(authState.user); + } return { isAuthenticated: !!authState, user: authState?.user, @@ -154,6 +180,8 @@ export const registerAuthHandlers = (): void => { }); ipcMain.handle(IPC_CHANNELS.GITHUB_AUTH_LOGOUT, () => { + // Clear user from store + store.getState().logout(); setAuthState(null); // Clear saved auth const config = loadConfig(); @@ -173,6 +201,8 @@ export const registerAuthHandlers = (): void => { try { const repos = await githubAuth.getInstalledRepos(accessToken); + // Update store so zubridge syncs to renderer + store.getState().setRepos(repos); return { success: true, repos }; } catch (error) { const { userMessage, technicalDetails } = toUserError(error, 'Fetching repositories'); @@ -191,6 +221,8 @@ export const registerAuthHandlers = (): void => { try { const orgs = await githubAuth.getInstalledOrgs(accessToken); + // Update store so zubridge syncs to renderer + store.getState().setOrgs(orgs); return { success: true, orgs }; } catch (error) { const { userMessage, technicalDetails } = toUserError(error, 'Fetching organizations'); diff --git a/src/main/ipc-handlers/runner.ts b/src/main/ipc-handlers/runner.ts index 5ffb692..621c250 100644 --- a/src/main/ipc-handlers/runner.ts +++ b/src/main/ipc-handlers/runner.ts @@ -23,6 +23,7 @@ import { import { getRunnerProxyManager } from '../runner-proxy-manager'; import { sendRunnerEvent } from '../runner-state-service'; import { updateTrayMenu } from '../tray-init'; +import { store } from '../store'; import { getSnapshot, selectRunnerStatus } from '../runner-state-service'; import { IPC_CHANNELS, @@ -71,22 +72,40 @@ export const registerRunnerHandlers = (): void => { // Runner download ipcMain.handle(IPC_CHANNELS.RUNNER_IS_DOWNLOADED, () => { const runnerDownloader = getRunnerDownloader(); - return runnerDownloader?.isDownloaded() ?? false; + const downloaded = runnerDownloader?.isDownloaded() ?? false; + // Update store so zubridge syncs to renderer + store.getState().setIsDownloaded(downloaded); + return downloaded; }); ipcMain.handle(IPC_CHANNELS.RUNNER_GET_VERSION, () => { const runnerDownloader = getRunnerDownloader(); - return { + const versionInfo = { version: runnerDownloader?.getVersion() ?? null, url: runnerDownloader?.getVersionUrl() ?? null, }; + // Update store so zubridge syncs to renderer + store.getState().setRunnerVersion(versionInfo); + return versionInfo; }); ipcMain.handle(IPC_CHANNELS.RUNNER_GET_AVAILABLE_VERSIONS, async () => { const runnerDownloader = getRunnerDownloader(); try { const versions = await runnerDownloader?.getAvailableVersions(); - return { success: true, versions: versions || [] }; + const versionList = versions || []; + // Update store so zubridge syncs to renderer + store.getState().setAvailableVersions(versionList); + // Set initial selected version if not already set + const currentSelected = store.getState().runner.selectedVersion; + if (!currentSelected && versionList.length > 0) { + const installedVersion = runnerDownloader?.getInstalledVersion(); + const defaultVersion = installedVersion && versionList.some(v => v.version === installedVersion) + ? installedVersion + : versionList[0].version; + store.getState().setSelectedVersion(defaultVersion); + } + return { success: true, versions: versionList }; } catch (error) { const { userMessage, technicalDetails } = toUserError(error, 'Fetching versions'); logger()?.error(technicalDetails); @@ -97,6 +116,10 @@ export const registerRunnerHandlers = (): void => { ipcMain.handle(IPC_CHANNELS.RUNNER_SET_DOWNLOAD_VERSION, (_event, version: string | null) => { const runnerDownloader = getRunnerDownloader(); runnerDownloader?.setDownloadVersion(version); + // Update store so zubridge syncs to renderer + if (version) { + store.getState().setSelectedVersion(version); + } return { success: true }; }); @@ -104,14 +127,26 @@ export const registerRunnerHandlers = (): void => { const runnerDownloader = getRunnerDownloader(); const mainWindow = getMainWindow(); try { + // Set initial progress in store + store.getState().setDownloadProgress({ phase: 'downloading', percent: 0, message: 'Starting...' }); + const progressCallback = (progress: DownloadProgress) => { + // Update zubridge store so renderer sees progress + store.getState().setDownloadProgress(progress); + // Also send via IPC for fallback mainWindow?.webContents.send(IPC_CHANNELS.RUNNER_DOWNLOAD_PROGRESS, progress); }; await runnerDownloader?.download(progressCallback); + // Update store: download complete, clear progress + store.getState().setDownloadProgress(null); + store.getState().setIsDownloaded(true); + return { success: true }; } catch (error) { + // Clear progress on error + store.getState().setDownloadProgress(null); const { userMessage, technicalDetails } = toUserError(error, 'Download'); logger()?.error(technicalDetails); return { success: false, error: userMessage }; @@ -121,7 +156,10 @@ export const registerRunnerHandlers = (): void => { // Runner configuration ipcMain.handle(IPC_CHANNELS.RUNNER_IS_CONFIGURED, () => { const runnerManager = getRunnerManager(); - return runnerManager?.isConfigured() ?? false; + const configured = runnerManager?.isConfigured() ?? false; + // Update store so zubridge syncs to renderer + store.getState().setIsConfigured(configured); + return configured; }); ipcMain.handle(IPC_CHANNELS.RUNNER_GET_DISPLAY_NAME, () => { @@ -429,7 +467,10 @@ export const registerRunnerHandlers = (): void => { // Job history ipcMain.handle(IPC_CHANNELS.JOB_HISTORY_GET, () => { const runnerManager = getRunnerManager(); - return runnerManager?.getJobHistory() ?? []; + const history = runnerManager?.getJobHistory() ?? []; + // Update store so zubridge syncs to renderer + store.getState().setJobHistory(history); + return history; }); ipcMain.handle(IPC_CHANNELS.JOB_HISTORY_SET_MAX, (_event, max: number) => { diff --git a/src/main/ipc-handlers/settings.ts b/src/main/ipc-handlers/settings.ts index de44c13..49d6e94 100644 --- a/src/main/ipc-handlers/settings.ts +++ b/src/main/ipc-handlers/settings.ts @@ -86,6 +86,9 @@ export const registerSettingsHandlers = (): void => { if (settings.launchAtLogin !== undefined) { storeState.setLaunchAtLogin(settings.launchAtLogin); } + if (settings.hideOnStart !== undefined) { + storeState.setHideOnStart(settings.hideOnStart); + } if (settings.targets !== undefined) { storeState.setTargets(settings.targets); } diff --git a/src/main/ipc-handlers/targets.ts b/src/main/ipc-handlers/targets.ts index f6c82ab..61b4353 100644 --- a/src/main/ipc-handlers/targets.ts +++ b/src/main/ipc-handlers/targets.ts @@ -7,6 +7,7 @@ import { IPC_CHANNELS, Target, Result, RunnerProxyStatus } from '../../shared/ty import { getTargetManager } from '../target-manager'; import { getRunnerProxyManager } from '../runner-proxy-manager'; import { getLogger, getBrokerProxyService } from '../app-state'; +import { store } from '../store/init'; /** * Register all target-related IPC handlers. @@ -16,7 +17,10 @@ export const registerTargetHandlers = (): void => { // List all targets ipcMain.handle(IPC_CHANNELS.TARGETS_LIST, (): Target[] => { - return getTargetManager().getTargets(); + const targets = getTargetManager().getTargets(); + // Update store so zubridge syncs to renderer + store.getState().setTargets(targets); + return targets; }); // Add a new target @@ -80,19 +84,24 @@ export const registerTargetHandlers = (): void => { ipcMain.handle( IPC_CHANNELS.TARGETS_GET_STATUS, (): RunnerProxyStatus[] => { + let status: RunnerProxyStatus[]; const brokerProxy = getBrokerProxyService(); if (brokerProxy) { - return brokerProxy.getStatus(); + status = brokerProxy.getStatus(); + } else { + // Fallback: return placeholder status based on targets + const targets = getTargetManager().getTargets(); + status = targets.map(t => ({ + targetId: t.id, + registered: true, + sessionActive: false, + lastPoll: null, + jobsAssigned: 0, + })); } - // Fallback: return placeholder status based on targets - const targets = getTargetManager().getTargets(); - return targets.map(t => ({ - targetId: t.id, - registered: true, - sessionActive: false, - lastPoll: null, - jobsAssigned: 0, - })); + // Update store so zubridge syncs to renderer + store.getState().setTargetStatus(status); + return status; } ); }; diff --git a/src/main/log-file.ts b/src/main/log-file.ts index 30e6641..b1eff0a 100644 --- a/src/main/log-file.ts +++ b/src/main/log-file.ts @@ -13,13 +13,16 @@ const logsDir = getLogsDir(); /** * Early boot logger for errors before the main logger is initialized. - * Writes directly to stderr since log file may not be ready. + * Writes to log file, and to stderr only for warnings/errors. */ -export const bootLog = (level: 'info' | 'warn' | 'error', message: string): void => { +export const bootLog = (level: 'info' | 'warn' | 'error' | 'debug', message: string): void => { const timestamp = new Date().toISOString(); const line = `[BOOT] ${timestamp} [${level.toUpperCase()}] ${message}\n`; - process.stderr.write(line); - // Also try to write to log file if stream is available + // Only write warnings and errors to stderr (not routine info/debug) + if (level === 'warn' || level === 'error') { + process.stderr.write(line); + } + // Always write to log file if stream is available if (logFileStream) { logFileStream.write(line); } diff --git a/src/main/policy-cache.ts b/src/main/policy-cache.ts index 6de7ca6..e413507 100644 --- a/src/main/policy-cache.ts +++ b/src/main/policy-cache.ts @@ -49,7 +49,6 @@ export type PolicyApprovalCallback = (request: PolicyApprovalRequest) => Promise // ============================================================================= const POLICY_CACHE_DIR = 'policies'; -const POLICY_INDEX_FILE = 'policy-index.json'; /** * Get the policies cache directory. diff --git a/src/main/runner-manager.test.ts b/src/main/runner-manager.test.ts index ba119a4..f66e3db 100644 --- a/src/main/runner-manager.test.ts +++ b/src/main/runner-manager.test.ts @@ -295,12 +295,12 @@ describe('RunnerManager', () => { }); describe('user filtering', () => { - it('should allow all users when filter mode is everyone', () => { + it('should allow all users when filter scope is everyone', () => { const manager = new RunnerManager({ onLog: mockOnLog, onStatusChange: mockOnStatusChange, onJobHistoryUpdate: mockOnJobHistoryUpdate, - getUserFilter: () => ({ mode: 'everyone', allowlist: [] }), + getUserFilter: () => ({ scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }), getCurrentUserLogin: () => 'testuser', }); const helper = new RunnerManagerTestHelper(manager); @@ -320,12 +320,12 @@ describe('RunnerManager', () => { expect(helper.isUserAllowed('anyuser')).toBe(true); }); - it('should only allow current user when filter mode is just-me', () => { + it('should only allow current user when trigger scope with just-me', () => { const manager = new RunnerManager({ onLog: mockOnLog, onStatusChange: mockOnStatusChange, onJobHistoryUpdate: mockOnJobHistoryUpdate, - getUserFilter: () => ({ mode: 'just-me', allowlist: [] }), + getUserFilter: () => ({ scope: 'trigger', allowedUsers: 'just-me', allowlist: [] }), getCurrentUserLogin: () => 'testuser', }); const helper = new RunnerManagerTestHelper(manager); @@ -335,13 +335,14 @@ describe('RunnerManager', () => { expect(helper.isUserAllowed('otheruser')).toBe(false); }); - it('should only allow users in allowlist when filter mode is allowlist', () => { + it('should only allow users in allowlist when trigger scope with allowlist', () => { const manager = new RunnerManager({ onLog: mockOnLog, onStatusChange: mockOnStatusChange, onJobHistoryUpdate: mockOnJobHistoryUpdate, getUserFilter: () => ({ - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [ { login: 'user1', avatar_url: '', name: null }, { login: 'user2', avatar_url: '', name: null }, @@ -356,12 +357,12 @@ describe('RunnerManager', () => { expect(helper.isUserAllowed('user3')).toBe(false); }); - it('should allow user when just-me mode but no current user is set', () => { + it('should allow user when just-me but no current user is set', () => { const manager = new RunnerManager({ onLog: mockOnLog, onStatusChange: mockOnStatusChange, onJobHistoryUpdate: mockOnJobHistoryUpdate, - getUserFilter: () => ({ mode: 'just-me', allowlist: [] }), + getUserFilter: () => ({ scope: 'trigger', allowedUsers: 'just-me', allowlist: [] }), getCurrentUserLogin: () => undefined, }); const helper = new RunnerManagerTestHelper(manager); @@ -375,7 +376,7 @@ describe('RunnerManager', () => { onLog: mockOnLog, onStatusChange: mockOnStatusChange, onJobHistoryUpdate: mockOnJobHistoryUpdate, - getUserFilter: () => ({ mode: 'allowlist', allowlist: [] }), + getUserFilter: () => ({ scope: 'trigger', allowedUsers: 'allowlist', allowlist: [] }), }); const helper = new RunnerManagerTestHelper(manager); diff --git a/src/main/runner-manager.ts b/src/main/runner-manager.ts index 70d3b29..0929b13 100644 --- a/src/main/runner-manager.ts +++ b/src/main/runner-manager.ts @@ -10,6 +10,7 @@ import { ProxyServer, ProxyLogEntry } from './proxy-server'; import { RunnerDownloader } from './runner-downloader'; import { getConfigPath, getJobHistoryPath, getRunnerDir } from './paths'; import { loadConfig } from './config'; +import { normalizeFilterConfig, isUserAllowed, areAllUsersAllowed, parseRepository } from './runner/user-filter'; /** * Get the hostname without .local suffix (common on macOS). @@ -32,6 +33,8 @@ interface RunnerInstance { githubRunId?: number; // GitHub workflow run ID (for cancellation) githubJobId?: number; // GitHub job ID (for querying conclusion) githubActor?: string; // Username who triggered the workflow + githubSha?: string; // Commit SHA that triggered the workflow + githubRef?: string; // Branch/tag ref (e.g., refs/heads/main) } | null; name: string; jobsCompleted: number; @@ -65,6 +68,8 @@ interface RunnerManagerOptions { cancelWorkflowRun?: (owner: string, repo: string, runId: number) => Promise; /** Get job conclusion from GitHub API */ getJobConclusion?: (owner: string, repo: string, jobId: number) => Promise; + /** Get all contributors/authors for a repo at a given commit SHA (for contributor filtering) */ + getAllContributors?: (owner: string, repo: string, sha: string) => Promise>; /** Called when a job starts or completes (for notifications) */ onJobEvent?: (event: JobEvent) => void; } @@ -85,6 +90,7 @@ export class RunnerManager { private getCurrentUserLogin?: () => string | undefined; private cancelWorkflowRun?: (owner: string, repo: string, runId: number) => Promise; private getJobConclusion?: (owner: string, repo: string, jobId: number) => Promise; + private getAllContributors?: (owner: string, repo: string, sha: string) => Promise>; private onJobEvent?: (event: JobEvent) => void; private jobHistory: JobHistoryEntry[] = []; private jobIdCounter = 0; @@ -119,7 +125,7 @@ export class RunnerManager { // Pending target context for jobs received from broker // Maps runner name (or 'next') to target context - private pendingTargetContext: Map = new Map(); + private pendingTargetContext: Map = new Map(); /** * Validate that a child path stays within the expected base directory. @@ -152,6 +158,7 @@ export class RunnerManager { this.getCurrentUserLogin = options.getCurrentUserLogin; this.cancelWorkflowRun = options.cancelWorkflowRun; this.getJobConclusion = options.getJobConclusion; + this.getAllContributors = options.getAllContributors; this.onJobEvent = options.onJobEvent; this.downloader = new RunnerDownloader(); @@ -277,17 +284,19 @@ export class RunnerManager { * @param githubRunId The GitHub workflow run ID (for cancellation) * @param githubJobId The GitHub job ID (for querying conclusion) * @param githubActor The username who triggered the workflow (for user filtering) + * @param githubSha The commit SHA that triggered the workflow + * @param githubRef The branch/tag ref (e.g., refs/heads/main) */ - setPendingTargetContext(runnerName: string, targetId: string, targetDisplayName: string, actionsUrl?: string, githubRunId?: number, githubJobId?: number, githubActor?: string): void { - this.pendingTargetContext.set(runnerName, { targetId, targetDisplayName, actionsUrl, githubRunId, githubJobId, githubActor }); - this.log('debug', `Set pending target context for ${runnerName}: ${targetDisplayName} (runId=${githubRunId}, jobId=${githubJobId}, actor=${githubActor})`); + setPendingTargetContext(runnerName: string, targetId: string, targetDisplayName: string, actionsUrl?: string, githubRunId?: number, githubJobId?: number, githubActor?: string, githubSha?: string, githubRef?: string): void { + this.pendingTargetContext.set(runnerName, { targetId, targetDisplayName, actionsUrl, githubRunId, githubJobId, githubActor, githubSha, githubRef }); + this.log('debug', `Set pending target context for ${runnerName}: ${targetDisplayName} (runId=${githubRunId}, jobId=${githubJobId}, actor=${githubActor}, sha=${githubSha?.slice(0, 7)})`); } /** * Consume pending target context for a runner. * Returns and removes the context if found. */ - private consumePendingTargetContext(runnerName: string): { targetId: string; targetDisplayName: string; actionsUrl?: string; githubRunId?: number; githubJobId?: number; githubActor?: string } | undefined { + private consumePendingTargetContext(runnerName: string): { targetId: string; targetDisplayName: string; actionsUrl?: string; githubRunId?: number; githubJobId?: number; githubActor?: string; githubSha?: string; githubRef?: string } | undefined { // Try exact match first, then fall back to 'next' let context = this.pendingTargetContext.get(runnerName); if (context) { @@ -1137,6 +1146,8 @@ export class RunnerManager { githubRunId: targetContext?.githubRunId, githubJobId: targetContext?.githubJobId, githubActor: targetContext?.githubActor, + githubSha: targetContext?.githubSha, + githubRef: targetContext?.githubRef, }; this.log('debug', `[instance ${instanceNum}] Job started: ${jobName} (id: ${instance.currentJob.id})${targetContext ? ` from ${targetContext.targetDisplayName}` : ''}${instance.currentJob.actionsUrl ? ` url=${instance.currentJob.actionsUrl}` : ''}`); @@ -1237,63 +1248,103 @@ export class RunnerManager { } /** - * Check if a user is allowed to trigger jobs based on the user filter configuration. + * Check if a single user is allowed by the trigger-based filter. + * Used for testing and as a fallback in checkJobUserFilter. */ private isUserAllowed(actorLogin: string): boolean { - const userFilter = this.getUserFilter?.(); - - // Default to everyone if no filter is set - if (!userFilter || userFilter.mode === 'everyone') { - return true; - } - - if (userFilter.mode === 'just-me') { - const currentUser = this.getCurrentUserLogin?.(); - return currentUser ? actorLogin.toLowerCase() === currentUser.toLowerCase() : true; - } - - if (userFilter.mode === 'allowlist') { - return userFilter.allowlist.some( - (user) => user.login.toLowerCase() === actorLogin.toLowerCase() - ); - } - - return true; + return isUserAllowed(actorLogin, this.getUserFilter?.(), this.getCurrentUserLogin?.()); } /** * Check if a job should be allowed based on user filter. * If not allowed, attempts to cancel the workflow run. - * Uses stored job info (actor, runId) instead of API lookups. + * + * Supports three scopes: + * - 'everyone': No filtering, all jobs allowed + * - 'trigger': Check the workflow trigger author only + * - 'contributors': Check all repository contributors/commit authors */ private async checkJobUserFilter(instanceNum: number, _runnerName: string): Promise { const instance = this.instances.get(instanceNum); if (!instance?.currentJob) return; - const { githubActor, githubRunId, targetDisplayName } = instance.currentJob; + const { githubActor, githubRunId, targetDisplayName, githubSha } = instance.currentJob; if (!githubActor || !githubRunId || !targetDisplayName) { this.log('debug', `checkJobUserFilter: missing info (actor=${githubActor}, runId=${githubRunId}, target=${targetDisplayName})`); return; } const userFilter = this.getUserFilter?.(); - if (!userFilter || userFilter.mode === 'everyone') { - return; // No filtering needed + const { scope } = normalizeFilterConfig(userFilter); + + // No filtering in everyone scope + if (scope === 'everyone') { + return; } - // Check if the actor is allowed - const isAllowed = this.isUserAllowed(githubActor); + // Parse owner/repo from target display name (e.g., "owner/repo") + const repoInfo = parseRepository(targetDisplayName); + if (!repoInfo) { + this.log('warn', `Cannot parse owner/repo from target: ${targetDisplayName}`); + return; + } + const { owner, repo } = repoInfo; + const currentUser = this.getCurrentUserLogin?.(); - if (!isAllowed) { - this.log('info', `Job triggered by '${githubActor}' is not in allowed users list. Cancelling workflow run.`); + let shouldCancel = false; + let reason = ''; - // Parse owner/repo from target display name (e.g., "owner/repo") - const parts = targetDisplayName.split('/'); - if (parts.length !== 2) { - this.log('warn', `Cannot parse owner/repo from target: ${targetDisplayName}`); - return; + if (scope === 'trigger') { + // Check if the trigger author is allowed + if (!isUserAllowed(githubActor, userFilter, currentUser)) { + shouldCancel = true; + reason = `trigger author '${githubActor}' not in allowed users`; + } else { + this.log('debug', `Job triggered by '${githubActor}' is allowed`); } - const [owner, repo] = parts; + } else if (scope === 'contributors') { + // Check all repository contributors + if (!githubSha) { + this.log('warn', `checkJobUserFilter: no SHA for contributors check, falling back to trigger check`); + // Fall back to trigger check if no SHA available + if (!isUserAllowed(githubActor, userFilter, currentUser)) { + shouldCancel = true; + reason = `trigger author '${githubActor}' not in allowed users (no SHA for contributors check)`; + } + } else if (!this.getAllContributors) { + this.log('warn', 'checkJobUserFilter: getAllContributors not available, falling back to trigger check'); + if (!isUserAllowed(githubActor, userFilter, currentUser)) { + shouldCancel = true; + reason = `trigger author '${githubActor}' not in allowed users (contributors check unavailable)`; + } + } else { + try { + // Get all contributors/authors for the repo at this SHA + const contributors = await this.getAllContributors(owner, repo, githubSha); + this.log('debug', `checkJobUserFilter: found ${contributors.size} contributors for ${owner}/${repo} at ${githubSha.slice(0, 7)}`); + + // Check if all contributors are allowed + const result = areAllUsersAllowed(contributors, userFilter, currentUser); + + if (!result.allowed) { + shouldCancel = true; + const disallowedList = result.disallowedUsers.slice(0, 3).join(', '); + const suffix = result.disallowedUsers.length > 3 ? ` and ${result.disallowedUsers.length - 3} more` : ''; + reason = `repo has disallowed contributors: ${disallowedList}${suffix}`; + } else { + this.log('debug', `All ${contributors.size} contributors for ${owner}/${repo} are allowed`); + } + } catch (err) { + // If contributor check fails, fail safe (cancel the job) + this.log('warn', `Failed to get contributors: ${(err as Error).message}, cancelling job for safety`); + shouldCancel = true; + reason = `contributor check failed: ${(err as Error).message}`; + } + } + } + + if (shouldCancel) { + this.log('info', `Job not allowed: ${reason}. Cancelling workflow run.`); if (!this.cancelWorkflowRun) { this.log('warn', 'Cannot cancel: cancelWorkflowRun not available'); @@ -1302,12 +1353,10 @@ export class RunnerManager { try { await this.cancelWorkflowRun(owner, repo, githubRunId); - this.log('info', `Cancelled workflow run ${githubRunId} triggered by '${githubActor}'`); + this.log('info', `Cancelled workflow run ${githubRunId}: ${reason}`); } catch (cancelErr) { this.log('warn', `Failed to cancel workflow run ${githubRunId}: ${(cancelErr as Error).message}`); } - } else { - this.log('debug', `Job triggered by '${githubActor}' is allowed`); } } diff --git a/src/main/runner/user-filter.ts b/src/main/runner/user-filter.ts index 46e9a89..ac77c32 100644 --- a/src/main/runner/user-filter.ts +++ b/src/main/runner/user-filter.ts @@ -2,37 +2,129 @@ * User Filter * * Handles user filtering for job acceptance. - * Supports everyone, just-me, and allowlist modes. + * + * Two-dimensional model: + * - scope: What to check (everyone, trigger author, or all contributors) + * - allowedUsers: Who is allowed (just me, or explicit allowlist) + * + * Also supports legacy 'mode' field for backwards compatibility. */ -import type { UserFilterConfig } from '../../shared/types'; +import type { UserFilterConfig, FilterScope, AllowedUsers } from '../../shared/types'; + +/** + * Normalize a filter config, handling legacy 'mode' field. + * Returns the effective scope and allowedUsers. + */ +export function normalizeFilterConfig( + userFilter: UserFilterConfig | undefined +): { scope: FilterScope; allowedUsers: AllowedUsers; allowlist: string[] } { + if (!userFilter) { + return { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }; + } + + // Handle new format + if (userFilter.scope) { + return { + scope: userFilter.scope, + allowedUsers: userFilter.allowedUsers || 'just-me', + allowlist: (userFilter.allowlist || []).map(u => u.login.toLowerCase()), + }; + } + + // Handle legacy 'mode' field + if (userFilter.mode) { + switch (userFilter.mode) { + case 'everyone': + return { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }; + case 'just-me': + return { scope: 'trigger', allowedUsers: 'just-me', allowlist: [] }; + case 'allowlist': + return { + scope: 'trigger', + allowedUsers: 'allowlist', + allowlist: (userFilter.allowlist || []).map(u => u.login.toLowerCase()), + }; + } + } + + return { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }; +} /** - * Check if a user is allowed based on filter configuration. + * Check if a single user is allowed based on filter configuration. + * Used for 'trigger' scope to check the workflow trigger. */ export function isUserAllowed( - actorLogin: string, + login: string, userFilter: UserFilterConfig | undefined, currentUserLogin: string | undefined ): boolean { - // No filter or everyone mode - allow all - if (!userFilter || userFilter.mode === 'everyone') { + const { scope, allowedUsers, allowlist } = normalizeFilterConfig(userFilter); + + // Everyone scope - allow all + if (scope === 'everyone') { return true; } - // Just-me mode - only allow current user - if (userFilter.mode === 'just-me') { + // Check the user against allowedUsers setting + return isLoginAllowed(login, allowedUsers, allowlist, currentUserLogin); +} + +/** + * Check if ALL users in a set are allowed. + * Used for 'contributors' scope to check all repo contributors. + */ +export function areAllUsersAllowed( + logins: Set, + userFilter: UserFilterConfig | undefined, + currentUserLogin: string | undefined +): { allowed: boolean; disallowedUsers: string[] } { + const { scope, allowedUsers, allowlist } = normalizeFilterConfig(userFilter); + + // Everyone scope - allow all + if (scope === 'everyone') { + return { allowed: true, disallowedUsers: [] }; + } + + const disallowedUsers: string[] = []; + + for (const login of logins) { + if (!isLoginAllowed(login, allowedUsers, allowlist, currentUserLogin)) { + disallowedUsers.push(login); + } + } + + return { + allowed: disallowedUsers.length === 0, + disallowedUsers, + }; +} + +/** + * Check if a login is allowed by the allowedUsers setting. + */ +function isLoginAllowed( + login: string, + allowedUsers: AllowedUsers, + allowlist: string[], + currentUserLogin: string | undefined +): boolean { + const loginLower = login.toLowerCase(); + + if (allowedUsers === 'just-me') { // If we don't know who we are, allow (fail open) if (!currentUserLogin) return true; - return actorLogin.toLowerCase() === currentUserLogin.toLowerCase(); + return loginLower === currentUserLogin.toLowerCase(); } - // Allowlist mode - check if user is in list - if (userFilter.mode === 'allowlist') { - const allowlist = userFilter.allowlist ?? []; - return allowlist.some(allowed => - allowed.toLowerCase() === actorLogin.toLowerCase() - ); + if (allowedUsers === 'allowlist') { + // Check if user is in the allowlist + // Also include current user automatically + if (currentUserLogin && loginLower === currentUserLogin.toLowerCase()) { + return true; + } + return allowlist.includes(loginLower); } return true; @@ -104,14 +196,15 @@ export class UserFilterManager { } const userFilter = this.getFilter(); + const { scope } = normalizeFilterConfig(userFilter); - // No filtering in everyone mode - if (!userFilter || userFilter.mode === 'everyone') { + // No filtering in everyone scope + if (scope === 'everyone') { return true; } if (!this.isAllowed(actorLogin)) { - log(`Job from ${actorLogin} not allowed by filter (mode: ${userFilter.mode}), cancelling...`); + log(`Job from ${actorLogin} not allowed by filter (scope: ${scope}), cancelling...`); const repoInfo = parseRepository(repository); if (!repoInfo) { diff --git a/src/main/store/index.ts b/src/main/store/index.ts index e68ba46..efe7a88 100644 --- a/src/main/store/index.ts +++ b/src/main/store/index.ts @@ -37,7 +37,7 @@ import { // Create the store export const store = createStore()( - subscribeWithSelector((set, get) => ({ + subscribeWithSelector((set, _get) => ({ // Initial state ...defaultAppState, diff --git a/src/main/store/middleware/persist.ts b/src/main/store/middleware/persist.ts index eec45b9..45d85c7 100644 --- a/src/main/store/middleware/persist.ts +++ b/src/main/store/middleware/persist.ts @@ -9,7 +9,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as yaml from 'js-yaml'; import { getAppDataDir, getConfigPath } from '../../paths'; -import { encryptValue, decryptValue } from '../../encryption'; +import { decryptValue } from '../../encryption'; import { bootLog } from '../../log-file'; import { store, getState } from '../index'; import { ConfigSlice, defaultConfigState } from '../types'; @@ -107,14 +107,37 @@ export function loadPersistedConfig(): void { configUpdates.preserveWorkDir = diskConfig.preserveWorkDir; } - // User filter + // User filter - supports both old 'mode' format and new 'scope/allowedUsers' format if (diskConfig.userFilter) { const filter = diskConfig.userFilter; - if (filter.mode && ['everyone', 'just-me', 'allowlist'].includes(filter.mode)) { + const allowlist = Array.isArray(filter.allowlist) ? filter.allowlist : []; + + // Check for new format first + if (filter.scope && ['everyone', 'trigger', 'contributors'].includes(filter.scope)) { configUpdates.userFilter = { - mode: filter.mode, - allowlist: Array.isArray(filter.allowlist) ? filter.allowlist : [], + scope: filter.scope, + allowedUsers: filter.allowedUsers || 'just-me', + allowlist, }; + } else if (filter.mode && ['everyone', 'just-me', 'allowlist'].includes(filter.mode)) { + // Migrate from old format to new format + // Old format mapping: + // 'everyone' -> scope: 'everyone', allowedUsers: 'just-me' (doesn't matter, not used) + // 'just-me' -> scope: 'trigger', allowedUsers: 'just-me' + // 'allowlist' -> scope: 'trigger', allowedUsers: 'allowlist' + let scope: 'everyone' | 'trigger' | 'contributors' = 'everyone'; + let allowedUsers: 'just-me' | 'allowlist' = 'just-me'; + + if (filter.mode === 'just-me') { + scope = 'trigger'; + allowedUsers = 'just-me'; + } else if (filter.mode === 'allowlist') { + scope = 'trigger'; + allowedUsers = 'allowlist'; + } + + configUpdates.userFilter = { scope, allowedUsers, allowlist }; + bootLog('info', `Migrated userFilter from mode='${filter.mode}' to scope='${scope}', allowedUsers='${allowedUsers}'`); } } @@ -175,10 +198,11 @@ export function loadPersistedConfig(): void { // Handle auth tokens separately (with decryption) if (diskConfig.auth) { try { - const accessToken = decryptValue(diskConfig.auth.accessToken); - const refreshToken = diskConfig.auth.refreshToken - ? decryptValue(diskConfig.auth.refreshToken) - : undefined; + // Validate tokens can be decrypted (actual token storage handled by auth-tokens module) + decryptValue(diskConfig.auth.accessToken); + if (diskConfig.auth.refreshToken) { + decryptValue(diskConfig.auth.refreshToken); + } store.setState((state) => ({ auth: { @@ -187,9 +211,6 @@ export function loadPersistedConfig(): void { isAuthenticated: true, }, })); - - // Store decrypted tokens in a separate location (not in Zustand for security) - // The auth-tokens module handles this } catch (e) { bootLog('warn', `Failed to decrypt auth tokens: ${(e as Error).message}`); } @@ -206,6 +227,7 @@ export function loadPersistedConfig(): void { /** * Save current config state to disk. * Uses atomic write (temp file + rename) to prevent corruption. + * Preserves auth section from existing config (auth is saved separately by auth module). */ export function savePersistedConfig(): void { if (isLoading) { @@ -228,8 +250,19 @@ export function savePersistedConfig(): void { } } - // Auth is handled separately by auth-tokens module - // We don't save it here to avoid conflicts + // Preserve auth from existing config file (auth is saved separately by auth module) + // We must read and preserve it to avoid overwriting encrypted tokens + if (fs.existsSync(configPath)) { + try { + const existingContent = fs.readFileSync(configPath, 'utf-8'); + const existingConfig = (yaml.load(existingContent, { schema: yaml.JSON_SCHEMA }) as AppConfig) || {}; + if (existingConfig.auth) { + configToSave.auth = existingConfig.auth; + } + } catch (readErr) { + bootLog('warn', `Failed to read existing config for auth preservation: ${(readErr as Error).message}`); + } + } // Ensure directory exists fs.mkdirSync(configDir, { recursive: true }); diff --git a/src/main/store/types.ts b/src/main/store/types.ts index 35310bb..f553777 100644 --- a/src/main/store/types.ts +++ b/src/main/store/types.ts @@ -273,7 +273,7 @@ export const defaultConfigState: ConfigSlice = { sleepProtectionConsented: false, preserveWorkDir: 'never', toolCacheLocation: 'persistent', - userFilter: { mode: 'just-me', allowlist: [] }, + userFilter: { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }, power: DEFAULT_POWER_CONFIG, notifications: DEFAULT_NOTIFICATIONS_CONFIG, launchAtLogin: false, diff --git a/src/main/window.ts b/src/main/window.ts index c2b0d41..b0069c7 100644 --- a/src/main/window.ts +++ b/src/main/window.ts @@ -15,16 +15,24 @@ import { REPOSITORY_URL, PRIVACY_POLICY_URL } from '../shared/constants'; declare const MAIN_WINDOW_WEBPACK_ENTRY: string; declare const MAIN_WINDOW_PRELOAD_WEBPACK_ENTRY: string; +interface CreateWindowOptions { + show?: boolean; +} + /** * Create the main application window. + * @param options.show - Whether to show the window immediately (default: true) */ -export const createWindow = (): void => { +export const createWindow = (options: CreateWindowOptions = {}): void => { + const { show = true } = options; + const mainWindow = new BrowserWindow({ title: 'localmost', width: 900, height: 700, minWidth: 600, minHeight: 500, + show, titleBarStyle: 'hiddenInset', trafficLightPosition: { x: 16, y: 16 }, backgroundColor: '#1a1a1a', diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 2ae227e..0ff87df 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -30,10 +30,16 @@ const AppContent: React.FC = () => { // Check if setup is needed and redirect to settings // Must wait for BOTH contexts to finish loading before checking setup state + // Only redirect from status page - don't interrupt if user is in setup flow (settings/targets) useEffect(() => { const isLoading = isConfigLoading || isRunnerLoading; if (!isLoading && (!user || !isDownloaded || !isConfigured)) { - setView('settings'); + setView((currentView) => { + if (currentView === 'status') { + return 'settings'; + } + return currentView; + }); } }, [isConfigLoading, isRunnerLoading, user, isDownloaded, isConfigured]); diff --git a/src/renderer/components/SettingsPage.module.css b/src/renderer/components/SettingsPage.module.css index 77ed9d8..b7eddca 100644 --- a/src/renderer/components/SettingsPage.module.css +++ b/src/renderer/components/SettingsPage.module.css @@ -137,6 +137,27 @@ letter-spacing: 2px; } +.codeWithCopied { + display: flex; + align-items: center; + gap: 12px; +} + +.copiedBadge { + font-size: 12px; + font-weight: 500; + color: var(--success); + background: rgba(34, 197, 94, 0.15); + padding: 4px 10px; + border-radius: 4px; + animation: fadeIn 0.2s ease-out; +} + +@keyframes fadeIn { + from { opacity: 0; transform: translateX(-8px); } + to { opacity: 1; transform: translateX(0); } +} + /* Download Section */ .downloadSection { display: flex; diff --git a/src/renderer/components/SettingsPage.tsx b/src/renderer/components/SettingsPage.tsx index b411fb2..61c7e18 100644 --- a/src/renderer/components/SettingsPage.tsx +++ b/src/renderer/components/SettingsPage.tsx @@ -77,6 +77,18 @@ const SettingsPage: React.FC = ({ onBack, scrollToSection, on const [avatarError, setAvatarError] = useState(false); const [launchAtLogin, setLaunchAtLogin] = useState(false); const [hideOnStart, setHideOnStart] = useState(false); + const [showCopiedNotice, setShowCopiedNotice] = useState(false); + + // Show copied notice when device code is copied, auto-hide after 4s + useEffect(() => { + if (deviceCode?.copiedToClipboard) { + setShowCopiedNotice(true); + const timer = setTimeout(() => setShowCopiedNotice(false), 4000); + return () => clearTimeout(timer); + } else { + setShowCopiedNotice(false); + } + }, [deviceCode?.copiedToClipboard]); // Load startup settings useEffect(() => { @@ -219,11 +231,22 @@ const SettingsPage: React.FC = ({ onBack, scrollToSection, on {deviceCode && (

Enter code on GitHub:

- {deviceCode.userCode} +
+ {deviceCode.userCode} + {showCopiedNotice && ( + Copied! + )} +
Waiting...
+
)} diff --git a/src/renderer/components/UserFilterSettings.test.tsx b/src/renderer/components/UserFilterSettings.test.tsx index f0910ec..e4deee1 100644 --- a/src/renderer/components/UserFilterSettings.test.tsx +++ b/src/renderer/components/UserFilterSettings.test.tsx @@ -5,7 +5,7 @@ import { mockLocalmost } from '../../../test/setup-renderer'; import { UserFilterConfig } from '../../shared/types'; describe('UserFilterSettings', () => { - const defaultFilter: UserFilterConfig = { mode: 'everyone', allowlist: [] }; + const defaultFilter: UserFilterConfig = { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }; const mockOnFilterChange = jest.fn(); beforeEach(() => { @@ -13,8 +13,8 @@ describe('UserFilterSettings', () => { mockLocalmost.github.searchUsers.mockResolvedValue({ success: true, users: [] }); }); - describe('Mode Selection', () => { - it('should render with everyone mode selected by default', () => { + describe('Scope Selection', () => { + it('should render with everyone scope selected by default', () => { render( { /> ); - // Verify all mode buttons are rendered + // Verify all scope buttons are rendered expect(screen.getByRole('button', { name: 'Everyone' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Just me' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Specific users' })).toBeInTheDocument(); - // Verify hint text for everyone mode + expect(screen.getByRole('button', { name: 'Trigger author' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'All contributors' })).toBeInTheDocument(); + // Verify hint text for everyone scope expect(screen.getByText(/Accept workflow jobs triggered by any user/)).toBeInTheDocument(); }); - it('should switch to just-me mode when clicked', async () => { + it('should switch to trigger scope when clicked', async () => { render( { ); await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Just me' })); + fireEvent.click(screen.getByRole('button', { name: 'Trigger author' })); }); expect(mockOnFilterChange).toHaveBeenCalledWith({ - mode: 'just-me', + scope: 'trigger', + allowedUsers: 'just-me', allowlist: [], }); }); - it('should switch to allowlist mode when clicked', async () => { + it('should switch to contributors scope when clicked', async () => { render( { ); await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Specific users' })); + fireEvent.click(screen.getByRole('button', { name: 'All contributors' })); }); expect(mockOnFilterChange).toHaveBeenCalledWith({ - mode: 'allowlist', + scope: 'contributors', + allowedUsers: 'just-me', allowlist: [], }); }); - it('should show correct hint for each mode', () => { + it('should show correct hint for each scope', () => { const { rerender } = render( @@ -82,30 +84,78 @@ describe('UserFilterSettings', () => { rerender( ); - expect(screen.getByText(/Only accept jobs triggered by @testuser/)).toBeInTheDocument(); + expect(screen.getByText(/Check who triggered the workflow/)).toBeInTheDocument(); rerender( + ); + + expect(screen.getByText(/Check all contributors to the repository/)).toBeInTheDocument(); + }); + }); + + describe('AllowedUsers Selection', () => { + it('should show allowedUsers selector when scope is trigger', () => { + render( + + ); + + expect(screen.getByRole('button', { name: 'Just me' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Allowlist' })).toBeInTheDocument(); + }); + + it('should not show allowedUsers selector when scope is everyone', () => { + render( + + ); + + // Only the scope buttons should be visible, not the allowedUsers buttons + expect(screen.queryAllByRole('button', { name: 'Just me' })).toHaveLength(0); + }); + + it('should switch to allowlist when clicked', async () => { + render( + ); - expect(screen.getByText(/Only accept jobs triggered by users in the list below/)).toBeInTheDocument(); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Allowlist' })); + }); + + expect(mockOnFilterChange).toHaveBeenCalledWith({ + scope: 'trigger', + allowedUsers: 'allowlist', + allowlist: [], + }); }); }); describe('Allowlist Mode', () => { - const allowlistFilter: UserFilterConfig = { mode: 'allowlist', allowlist: [] }; + const allowlistFilter: UserFilterConfig = { scope: 'trigger', allowedUsers: 'allowlist', allowlist: [] }; - it('should show search input in allowlist mode', () => { + it('should show search input when allowedUsers is allowlist and scope is not everyone', () => { render( { expect(screen.getByPlaceholderText('Search GitHub users...')).toBeInTheDocument(); }); - it('should not show search input in other modes', () => { + it('should not show search input in everyone scope', () => { render( @@ -155,9 +205,9 @@ describe('UserFilterSettings', () => { fireEvent.change(searchInput, { target: { value: 'user' } }); }); - // Advance timers for debounce + // Advance timers for debounce (1 second) await act(async () => { - jest.advanceTimersByTime(300); + jest.advanceTimersByTime(1000); }); expect(mockLocalmost.github.searchUsers).toHaveBeenCalledWith('user'); @@ -191,7 +241,7 @@ describe('UserFilterSettings', () => { }); await act(async () => { - jest.advanceTimersByTime(300); + jest.advanceTimersByTime(1000); }); await waitFor(() => { @@ -228,7 +278,7 @@ describe('UserFilterSettings', () => { }); await act(async () => { - jest.advanceTimersByTime(300); + jest.advanceTimersByTime(1000); }); await waitFor(() => { @@ -240,7 +290,8 @@ describe('UserFilterSettings', () => { }); expect(mockOnFilterChange).toHaveBeenCalledWith({ - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [{ login: 'newuser', avatar_url: 'https://example.com/avatar.png', name: 'New User' }], }); @@ -249,7 +300,8 @@ describe('UserFilterSettings', () => { it('should display existing allowlist users', () => { const filterWithUsers: UserFilterConfig = { - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [ { login: 'existinguser', avatar_url: '', name: 'Existing User' }, ], @@ -269,7 +321,8 @@ describe('UserFilterSettings', () => { it('should remove user from allowlist when remove button clicked', async () => { const filterWithUsers: UserFilterConfig = { - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [ { login: 'usertoremove', avatar_url: '', name: 'User To Remove' }, ], @@ -290,7 +343,8 @@ describe('UserFilterSettings', () => { }); expect(mockOnFilterChange).toHaveBeenCalledWith({ - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [], }); }); @@ -311,7 +365,8 @@ describe('UserFilterSettings', () => { jest.useFakeTimers(); const filterWithUsers: UserFilterConfig = { - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [ { login: 'existinguser', avatar_url: '', name: 'Existing User' }, ], @@ -341,7 +396,7 @@ describe('UserFilterSettings', () => { }); await act(async () => { - jest.advanceTimersByTime(300); + jest.advanceTimersByTime(1000); }); // Should only show newuser in search results, not existinguser diff --git a/src/renderer/components/UserFilterSettings.tsx b/src/renderer/components/UserFilterSettings.tsx index a4cf991..ad95393 100644 --- a/src/renderer/components/UserFilterSettings.tsx +++ b/src/renderer/components/UserFilterSettings.tsx @@ -1,7 +1,7 @@ import React, { useState, useEffect, useRef, useCallback } from 'react'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faXmark } from '@fortawesome/free-solid-svg-icons'; -import { UserFilterMode, AllowlistUser, UserFilterConfig } from '../../shared/types'; +import { FilterScope, AllowedUsers, AllowlistUser, UserFilterConfig } from '../../shared/types'; import styles from './UserFilterSettings.module.css'; import shared from '../styles/shared.module.css'; @@ -22,6 +22,7 @@ const UserFilterSettings: React.FC = ({ const [showDropdown, setShowDropdown] = useState(false); const searchTimeoutRef = useRef(null); const dropdownRef = useRef(null); + const currentSearchRef = useRef(''); // Track current search to prevent stale results // Close dropdown when clicking outside useEffect(() => { @@ -42,20 +43,32 @@ const UserFilterSettings: React.FC = ({ return; } + // Track this search so we can ignore stale results + currentSearchRef.current = query; setIsSearching(true); + try { const result = await window.localmost.github.searchUsers(query); + + // Only update results if this is still the current search + if (currentSearchRef.current !== query) { + return; // Stale result, ignore + } + if (result.success && result.users) { - // Filter out users already in the allowlist - const filtered = result.users.filter( - (user: AllowlistUser) => !userFilter.allowlist.some((u) => u.login === user.login) - ); + // Filter out users already in the allowlist and limit to 10 results + const filtered = result.users + .filter((user: AllowlistUser) => !userFilter.allowlist.some((u) => u.login === user.login)) + .slice(0, 10); setSearchResults(filtered); } } catch (error) { console.error('User search failed:', error); } finally { - setIsSearching(false); + // Only clear loading if this is still the current search + if (currentSearchRef.current === query) { + setIsSearching(false); + } } }, [userFilter.allowlist]); @@ -70,16 +83,30 @@ const UserFilterSettings: React.FC = ({ clearTimeout(searchTimeoutRef.current); } - // Debounce search + // Clear results immediately when query changes to avoid showing stale results + if (!query.trim()) { + setSearchResults([]); + currentSearchRef.current = ''; + return; + } + + // Debounce search - wait 1 second for user to pause typing searchTimeoutRef.current = setTimeout(() => { searchUsers(query); - }, 300); + }, 1000); + }; + + const handleScopeChange = (scope: FilterScope) => { + onFilterChange({ + ...userFilter, + scope, + }); }; - const handleModeChange = (mode: UserFilterMode) => { + const handleAllowedUsersChange = (allowedUsers: AllowedUsers) => { onFilterChange({ ...userFilter, - mode, + allowedUsers, }); }; @@ -100,43 +127,83 @@ const UserFilterSettings: React.FC = ({ }); }; + // Get hint text for scope + const getScopeHint = () => { + switch (userFilter.scope) { + case 'everyone': + return 'Accept workflow jobs triggered by any user.'; + case 'trigger': + return 'Check who triggered the workflow and filter based on that user.'; + case 'contributors': + return 'Check all contributors to the repository and ensure all are trusted.'; + default: + return ''; + } + }; + + // Get hint text for allowedUsers + const getAllowedUsersHint = () => { + if (userFilter.allowedUsers === 'just-me') { + return currentUserLogin + ? `Only @${currentUserLogin}. Jobs involving other users will be cancelled.` + : 'Only you. Jobs involving other users will be cancelled.'; + } + return 'Only users in the list below. Jobs involving other users will be cancelled.'; + }; + return (
+ {/* Scope selector: What to check */}
- +
-

- {userFilter.mode === 'everyone' && 'Accept workflow jobs triggered by any user.'} - {userFilter.mode === 'just-me' && ( - currentUserLogin - ? `Only accept jobs triggered by @${currentUserLogin}. Jobs from other users will be cancelled.` - : 'Only accept jobs you trigger. Jobs from other users will be cancelled.' - )} - {userFilter.mode === 'allowlist' && 'Only accept jobs triggered by users in the list below.'} -

+

{getScopeHint()}

- {userFilter.mode === 'allowlist' && ( + {/* AllowedUsers selector: Who is allowed (shown when scope is not 'everyone') */} + {userFilter.scope !== 'everyone' && ( +
+ +
+ + +
+

{getAllowedUsersHint()}

+
+ )} + + {/* Allowlist user search and list (shown when allowedUsers is 'allowlist' and scope is not 'everyone') */} + {userFilter.scope !== 'everyone' && userFilter.allowedUsers === 'allowlist' && (
{ {String(config.sleepProtectionConsented)} {config.preserveWorkDir} {config.toolCacheLocation} - {config.userFilter.mode} + {config.userFilter.scope} + {config.userFilter.allowedUsers} {config.userFilter.allowlist.length} {String(config.isOnline)} {String(config.isLoading)} @@ -36,8 +37,8 @@ const TestConsumer: React.FC = () => { - - + +
); }; @@ -547,7 +548,7 @@ describe('AppConfigContext', () => { }); describe('User Filter', () => { - it('should have default user filter mode as just-me', async () => { + it('should have default user filter scope as everyone', async () => { render( @@ -558,14 +559,16 @@ describe('AppConfigContext', () => { expect(screen.getByTestId('is-loading').textContent).toBe('false'); }); - expect(screen.getByTestId('user-filter-mode').textContent).toBe('just-me'); + expect(screen.getByTestId('user-filter-scope').textContent).toBe('everyone'); + expect(screen.getByTestId('user-filter-allowed-users').textContent).toBe('just-me'); expect(screen.getByTestId('user-filter-allowlist-count').textContent).toBe('0'); }); it('should load user filter from settings', async () => { mockLocalmost.settings.get.mockResolvedValue({ userFilter: { - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [ { login: 'user1', avatar_url: '', name: null }, { login: 'user2', avatar_url: '', name: null }, @@ -580,12 +583,13 @@ describe('AppConfigContext', () => { ); await waitFor(() => { - expect(screen.getByTestId('user-filter-mode').textContent).toBe('allowlist'); + expect(screen.getByTestId('user-filter-scope').textContent).toBe('trigger'); + expect(screen.getByTestId('user-filter-allowed-users').textContent).toBe('allowlist'); expect(screen.getByTestId('user-filter-allowlist-count').textContent).toBe('2'); }); }); - it('should set user filter to just-me', async () => { + it('should set user filter to trigger scope with just-me', async () => { render( @@ -600,9 +604,10 @@ describe('AppConfigContext', () => { screen.getByTestId('set-user-filter-just-me').click(); }); - expect(screen.getByTestId('user-filter-mode').textContent).toBe('just-me'); + expect(screen.getByTestId('user-filter-scope').textContent).toBe('trigger'); + expect(screen.getByTestId('user-filter-allowed-users').textContent).toBe('just-me'); expect(mockLocalmost.settings.set).toHaveBeenCalledWith({ - userFilter: { mode: 'just-me', allowlist: [] }, + userFilter: { scope: 'trigger', allowedUsers: 'just-me', allowlist: [] }, }); }); @@ -621,20 +626,23 @@ describe('AppConfigContext', () => { screen.getByTestId('set-user-filter-allowlist').click(); }); - expect(screen.getByTestId('user-filter-mode').textContent).toBe('allowlist'); + expect(screen.getByTestId('user-filter-scope').textContent).toBe('trigger'); + expect(screen.getByTestId('user-filter-allowed-users').textContent).toBe('allowlist'); expect(screen.getByTestId('user-filter-allowlist-count').textContent).toBe('1'); expect(mockLocalmost.settings.set).toHaveBeenCalledWith({ userFilter: { - mode: 'allowlist', + scope: 'trigger', + allowedUsers: 'allowlist', allowlist: [{ login: 'testuser', avatar_url: '', name: null }], }, }); }); - it('should handle invalid user filter mode in settings', async () => { + it('should handle invalid user filter scope in settings', async () => { mockLocalmost.settings.get.mockResolvedValue({ userFilter: { - mode: 'invalid-mode', + scope: 'invalid-scope', + allowedUsers: 'just-me', allowlist: [], }, }); @@ -650,7 +658,7 @@ describe('AppConfigContext', () => { }); // Should fall back to default - expect(screen.getByTestId('user-filter-mode').textContent).toBe('just-me'); + expect(screen.getByTestId('user-filter-scope').textContent).toBe('everyone'); }); }); }); diff --git a/src/renderer/contexts/AppConfigContext.tsx b/src/renderer/contexts/AppConfigContext.tsx index 7bd3d15..332acc7 100644 --- a/src/renderer/contexts/AppConfigContext.tsx +++ b/src/renderer/contexts/AppConfigContext.tsx @@ -148,7 +148,7 @@ export const AppConfigProvider: React.FC = ({ children } sleepProtectionConsented: false, preserveWorkDir: 'never', toolCacheLocation: 'persistent', - userFilter: { mode: 'just-me', allowlist: [] }, + userFilter: { scope: 'everyone', allowedUsers: 'just-me', allowlist: [] }, power: DEFAULT_POWER_CONFIG, notifications: DEFAULT_NOTIFICATIONS_CONFIG, isOnline: true, @@ -217,7 +217,7 @@ export const AppConfigProvider: React.FC = ({ children } sleepProtectionConsented: settings.sleepProtectionConsented || prev.sleepProtectionConsented, preserveWorkDir: (settings.preserveWorkDir as 'never' | 'session' | 'always') || prev.preserveWorkDir, toolCacheLocation: (settings.toolCacheLocation as ToolCacheLocation) || prev.toolCacheLocation, - userFilter: settings.userFilter && ['just-me', 'allowlist', 'anyone'].includes((settings.userFilter as UserFilterConfig).mode) + userFilter: settings.userFilter && ['everyone', 'trigger', 'contributors'].includes((settings.userFilter as UserFilterConfig).scope) ? (settings.userFilter as UserFilterConfig) : prev.userFilter, power: settings.power ? { ...DEFAULT_POWER_CONFIG, ...(settings.power as PowerConfig) } : prev.power, diff --git a/src/renderer/store/index.ts b/src/renderer/store/index.ts index 197d3d8..7873597 100644 --- a/src/renderer/store/index.ts +++ b/src/renderer/store/index.ts @@ -6,7 +6,7 @@ */ import { createUseStore, useDispatch as useZubridgeDispatch } from '@zubridge/electron'; -import type { AppState, AppStore } from '../../main/store/types'; +import type { AppState } from '../../main/store/types'; // Create the store hook - this connects to the main process store via zubridge export const useStore = createUseStore(); diff --git a/src/renderer/styles/shared.module.css b/src/renderer/styles/shared.module.css index 39c9e5d..df56b14 100644 --- a/src/renderer/styles/shared.module.css +++ b/src/renderer/styles/shared.module.css @@ -157,6 +157,10 @@ } /* Spinner */ +@keyframes spin { + to { transform: rotate(360deg); } +} + .spinner { width: 20px; height: 20px; diff --git a/src/shared/action-fetcher.ts b/src/shared/action-fetcher.ts index 853397d..d187ee3 100644 --- a/src/shared/action-fetcher.ts +++ b/src/shared/action-fetcher.ts @@ -7,8 +7,6 @@ import * as fs from 'fs'; import * as path from 'path'; -import * as https from 'https'; -import * as os from 'os'; import { getAppDataDirWithoutElectron } from './paths'; // ============================================================================= @@ -194,47 +192,6 @@ export function isInterceptedAction(uses: string): boolean { // Fetching // ============================================================================= -/** - * Fetch JSON from a URL. - */ -function fetchJson(url: string): Promise { - return new Promise((resolve, reject) => { - const options = { - headers: { - 'User-Agent': 'localmost', - Accept: 'application/vnd.github.v3+json', - }, - }; - - https - .get(url, options, (res) => { - if (res.statusCode === 302 || res.statusCode === 301) { - // Handle redirects - if (res.headers.location) { - fetchJson(res.headers.location).then(resolve).catch(reject); - return; - } - } - - if (res.statusCode !== 200) { - reject(new Error(`HTTP ${res.statusCode}: ${url}`)); - return; - } - - let data = ''; - res.on('data', (chunk) => (data += chunk)); - res.on('end', () => { - try { - resolve(JSON.parse(data) as T); - } catch (err) { - reject(new Error(`Invalid JSON from ${url}`)); - } - }); - }) - .on('error', reject); - }); -} - /** * Download a tarball and extract it. */ @@ -303,7 +260,7 @@ export async function fetchAction(ref: ActionRef): Promise { try { await downloadAndExtract(tarballUrl, actionDir); - } catch (err) { + } catch { // Try as a branch const branchUrl = `https://github.com/${ref.owner}/${ref.repo}/archive/refs/heads/${ref.version}.tar.gz`; await downloadAndExtract(branchUrl, actionDir); diff --git a/src/shared/environment.ts b/src/shared/environment.ts index df83e7a..df2c263 100644 --- a/src/shared/environment.ts +++ b/src/shared/environment.ts @@ -7,7 +7,6 @@ import { execSync } from 'child_process'; import * as os from 'os'; -import * as fs from 'fs'; // ============================================================================= // Types diff --git a/src/shared/localmostrc.test.ts b/src/shared/localmostrc.test.ts index 4a7a21a..0a02c65 100644 --- a/src/shared/localmostrc.test.ts +++ b/src/shared/localmostrc.test.ts @@ -3,7 +3,6 @@ */ import * as fs from 'fs'; -import * as path from 'path'; import { findLocalmostrc, parseLocalmostrc, @@ -15,7 +14,6 @@ import { diffConfigs, formatPolicyDiff, LocalmostrcConfig, - LOCALMOSTRC_VERSION, } from './localmostrc'; import { SandboxPolicy } from './sandbox-profile'; diff --git a/src/shared/localmostrc.ts b/src/shared/localmostrc.ts index b88499b..04e95ee 100644 --- a/src/shared/localmostrc.ts +++ b/src/shared/localmostrc.ts @@ -69,9 +69,6 @@ export function findLocalmostrc(repoRoot: string): string | null { * Parse a .localmostrc file. */ export function parseLocalmostrc(filePath: string): ParseResult { - const errors: ParseError[] = []; - const warnings: string[] = []; - if (!fs.existsSync(filePath)) { return { success: false, diff --git a/src/shared/sandbox-profile.test.ts b/src/shared/sandbox-profile.test.ts index 93107f4..7d434c9 100644 --- a/src/shared/sandbox-profile.test.ts +++ b/src/shared/sandbox-profile.test.ts @@ -6,7 +6,6 @@ import { generateSandboxProfile, generateDiscoveryProfile, DEFAULT_SANDBOX_POLICY, - SandboxPolicy, } from './sandbox-profile'; // Mock os module diff --git a/src/shared/step-executor.ts b/src/shared/step-executor.ts index c2b01dc..60a538a 100644 --- a/src/shared/step-executor.ts +++ b/src/shared/step-executor.ts @@ -8,7 +8,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { spawn, ChildProcess, SpawnOptions } from 'child_process'; +import { spawn, SpawnOptions } from 'child_process'; import { WorkflowStep, WorkflowJob, MatrixCombination } from './workflow-parser'; import { SandboxPolicy, generateSandboxProfile } from './sandbox-profile'; import { parseActionRef, fetchAction, isInterceptedAction, readActionMetadata } from './action-fetcher'; @@ -42,6 +42,10 @@ export interface ExecutionContext { secrets: Record; /** Previous step outputs (step_id -> outputs) */ stepOutputs: Record>; + /** Inputs from workflow_call (for reusable workflows) */ + inputs?: Record; + /** Outputs from jobs this job depends on (needs context) */ + needs?: Record>; /** Sandbox policy to enforce */ policy?: SandboxPolicy; /** Whether running in permissive/discovery mode */ @@ -196,6 +200,20 @@ export function expandExpression( return ctx.stepOutputs[stepId]?.[outputName] || ''; } + // inputs.VAR (for reusable workflows) + if (trimmed.startsWith('inputs.')) { + const inputName = trimmed.slice(7); + const value = ctx.inputs?.[inputName]; + return value !== undefined ? String(value) : ''; + } + + // needs.JOB_ID.outputs.VAR + const needsMatch = trimmed.match(/^needs\.([^.]+)\.outputs\.(.+)$/); + if (needsMatch) { + const [, jobId, outputName] = needsMatch; + return ctx.needs?.[jobId]?.[outputName] || ''; + } + // github.* context if (trimmed.startsWith('github.')) { const prop = trimmed.slice(7); @@ -1058,7 +1076,7 @@ function parseGitHubOutputFile(filePath: string): Record { * Evaluate a step condition. * This is a simplified implementation - full expression support would require more work. */ -function evaluateCondition(condition: string, ctx: ExecutionContext): boolean { +function evaluateCondition(condition: string, _ctx: ExecutionContext): boolean { // Always run conditions if (condition === 'always()') { return true; diff --git a/src/shared/types.ts b/src/shared/types.ts index 19e11b6..643e547 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -225,12 +225,13 @@ export interface GitHubUser { export interface GitHubRepo { id: number; + name?: string; full_name: string; private: boolean; html_url: string; owner: { login: string; - type: string; + avatar_url: string; }; } @@ -286,6 +287,7 @@ export interface AppSettings { export interface DeviceCodeInfo { userCode: string; verificationUri: string; + copiedToClipboard?: boolean; } export interface HeartbeatStatus { @@ -297,9 +299,15 @@ export interface HeartbeatStatus { // User Filter Types // ============================================================================= -/** Mode for filtering jobs by triggering user */ +/** @deprecated Use FilterScope instead */ export type UserFilterMode = 'everyone' | 'just-me' | 'allowlist'; +/** What to check when filtering jobs */ +export type FilterScope = 'everyone' | 'trigger' | 'contributors'; + +/** Who is allowed when scope is not 'everyone' */ +export type AllowedUsers = 'just-me' | 'allowlist'; + /** A GitHub user for the filter allowlist */ export interface AllowlistUser { login: string; @@ -307,11 +315,25 @@ export interface AllowlistUser { name: string | null; } -/** User filter configuration */ +/** + * User filter configuration. + * + * Two-dimensional model: + * - scope: What to check (everyone, trigger author, or all contributors) + * - allowedUsers: Who is allowed (just me, or explicit allowlist) + * + * For backwards compatibility, also supports legacy 'mode' field. + */ export interface UserFilterConfig { - mode: UserFilterMode; - /** List of allowed users (only used when mode is 'allowlist') */ + /** What to check when filtering */ + scope: FilterScope; + /** Who is allowed (when scope is not 'everyone') */ + allowedUsers: AllowedUsers; + /** List of allowed users (used when allowedUsers is 'allowlist') */ allowlist: AllowlistUser[]; + + /** @deprecated Legacy field - use scope/allowedUsers instead */ + mode?: UserFilterMode; } /** Search result for GitHub users */ diff --git a/src/shared/workflow-parser.ts b/src/shared/workflow-parser.ts index 8d8e23e..3582d62 100644 --- a/src/shared/workflow-parser.ts +++ b/src/shared/workflow-parser.ts @@ -39,7 +39,14 @@ export interface MatrixStrategy { export interface WorkflowJob { name?: string; - 'runs-on': string | string[]; + /** Runner label(s) - required for regular jobs, absent for reusable workflow calls */ + 'runs-on'?: string | string[]; + /** Reference to a reusable workflow (e.g., "./.github/workflows/check.yaml") */ + uses?: string; + /** Inputs to pass to a reusable workflow */ + with?: Record; + /** Secrets to pass to a reusable workflow ('inherit' or explicit mapping) */ + secrets?: 'inherit' | Record; needs?: string | string[]; if?: string; strategy?: MatrixStrategy; @@ -50,7 +57,8 @@ export interface WorkflowJob { 'working-directory'?: string; }; }; - steps: WorkflowStep[]; + /** Steps to execute - required for regular jobs, absent for reusable workflow calls */ + steps?: WorkflowStep[]; outputs?: Record; 'timeout-minutes'?: number; 'continue-on-error'?: boolean; @@ -66,9 +74,38 @@ export interface WorkflowTrigger { schedule?: { cron: string }[]; } +/** Input definition for workflow_call trigger */ +export interface WorkflowCallInput { + description?: string; + required?: boolean; + type?: 'string' | 'boolean' | 'number'; + default?: string | boolean | number; +} + +/** Output definition for workflow_call trigger */ +export interface WorkflowCallOutput { + description?: string; + value: string; +} + +/** workflow_call trigger configuration */ +export interface WorkflowCallTrigger { + inputs?: Record; + outputs?: Record; + secrets?: Record; +} + +/** Parsed reusable workflow with its inputs/outputs */ +export interface ReusableWorkflow extends ParsedWorkflow { + /** Input definitions from workflow_call trigger */ + inputs: Record; + /** Output definitions from workflow_call trigger */ + outputs: Record; +} + export interface Workflow { name?: string; - on: string | string[] | Record; + on: string | string[] | Record; env?: Record; defaults?: { run?: { @@ -137,6 +174,14 @@ export function parseWorkflowContent(content: string, filePath: string): ParsedW // Validate each job has required fields for (const [jobId, job] of Object.entries(workflow.jobs)) { + // Check if this is a reusable workflow call + if (job.uses) { + // Reusable workflow jobs don't have runs-on or steps + // They reference another workflow file + continue; + } + + // Regular job requires runs-on and steps if (!job['runs-on']) { throw new Error(`Job "${jobId}" is missing required 'runs-on' field`); } @@ -369,6 +414,9 @@ export function extractSecretReferences(workflow: Workflow): string[] { extractFromValue(job.env); } + // Skip reusable workflow jobs (they have no steps) + if (!job.steps) continue; + for (const step of job.steps) { if (step.env) { extractFromValue(step.env); @@ -406,6 +454,9 @@ export function extractEnvReferences(workflow: Workflow): string[] { } for (const job of Object.values(workflow.jobs)) { + // Skip reusable workflow jobs (they have no steps) + if (!job.steps) continue; + for (const step of job.steps) { if (step.run) { extractFromValue(step.run); @@ -421,3 +472,101 @@ export function extractEnvReferences(workflow: Workflow): string[] { return Array.from(envVars).sort(); } + +// ============================================================================= +// Reusable Workflow Functions +// ============================================================================= + +/** + * Check if a job is a reusable workflow call. + */ +export function isReusableWorkflowJob(job: WorkflowJob): boolean { + return !!job.uses; +} + +/** + * Check if a workflow is a reusable workflow (has workflow_call trigger). + */ +export function isReusableWorkflow(workflow: Workflow): boolean { + if (typeof workflow.on === 'string') { + return workflow.on === 'workflow_call'; + } + if (Array.isArray(workflow.on)) { + return workflow.on.includes('workflow_call'); + } + return 'workflow_call' in workflow.on; +} + +/** + * Parse a reusable workflow file, extracting its inputs and outputs. + */ +export function parseReusableWorkflow(filePath: string): ReusableWorkflow { + const parsed = parseWorkflowFile(filePath); + + if (!isReusableWorkflow(parsed.workflow)) { + throw new Error(`Workflow "${filePath}" is not a reusable workflow (missing workflow_call trigger)`); + } + + // Extract inputs and outputs from workflow_call trigger + let inputs: Record = {}; + let outputs: Record = {}; + + const on = parsed.workflow.on; + if (typeof on === 'object' && !Array.isArray(on) && on.workflow_call) { + const callTrigger = on.workflow_call as WorkflowCallTrigger; + inputs = callTrigger.inputs || {}; + outputs = callTrigger.outputs || {}; + } + + return { + ...parsed, + inputs, + outputs, + }; +} + +/** + * Resolve the file path for a reusable workflow reference. + * Handles local references like "./.github/workflows/check.yaml". + */ +export function resolveReusableWorkflowPath( + uses: string, + callerWorkflowPath: string +): string | null { + // Local workflow reference: ./.github/workflows/workflow.yaml + if (uses.startsWith('./')) { + const repoRoot = path.dirname(path.dirname(path.dirname(callerWorkflowPath))); + return path.join(repoRoot, uses.slice(2)); + } + + // Remote workflow reference: owner/repo/.github/workflows/workflow.yaml@ref + // Not supported yet + if (uses.includes('/') && uses.includes('@')) { + return null; + } + + return null; +} + +/** + * Resolve inputs for a reusable workflow call. + * Combines caller's `with` values with workflow defaults. + */ +export function resolveReusableWorkflowInputs( + callerInputs: Record | undefined, + workflowInputDefs: Record +): Record { + const resolved: Record = {}; + + for (const [name, def] of Object.entries(workflowInputDefs)) { + if (callerInputs && name in callerInputs) { + resolved[name] = callerInputs[name]; + } else if (def.default !== undefined) { + resolved[name] = def.default; + } else if (def.required) { + throw new Error(`Required input "${name}" not provided for reusable workflow`); + } + } + + return resolved; +} diff --git a/src/shared/workspace.test.ts b/src/shared/workspace.test.ts index ba6eea0..ea9279e 100644 --- a/src/shared/workspace.test.ts +++ b/src/shared/workspace.test.ts @@ -3,12 +3,9 @@ */ import * as fs from 'fs'; -import * as path from 'path'; -import * as os from 'os'; import { execSync } from 'child_process'; import { getWorkspacesDir, - createWorkspace, listWorkspaces, removeWorkspace, cleanupWorkspaces, diff --git a/src/shared/workspace.ts b/src/shared/workspace.ts index 714a2b7..eccc477 100644 --- a/src/shared/workspace.ts +++ b/src/shared/workspace.ts @@ -7,7 +7,6 @@ import * as fs from 'fs'; import * as path from 'path'; -import * as os from 'os'; import { spawn, execSync } from 'child_process'; import { getAppDataDirWithoutElectron } from './paths'; diff --git a/webpack.config.js b/webpack.config.js index 9d86679..c6c5789 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -67,6 +67,9 @@ const mainConfig = { }; // Preload script configuration +// Note: Electron 20+ sandboxes preload scripts by default, so Node.js modules +// are not available. We provide shims for modules used by dependencies +// like the 'debug' library (bundled in @zubridge/electron). const preloadConfig = { ...commonConfig, target: 'electron-preload', @@ -75,6 +78,45 @@ const preloadConfig = { path: path.resolve(__dirname, 'dist'), filename: 'preload.js', }, + // Disable automatic Node.js externalization for sandboxed preload + // electron-preload target uses node externals preset under the hood + externalsPresets: { node: false, electronPreload: false }, + // Only externalize electron (which is available in preload) + externals: { + electron: 'commonjs electron', + }, + resolve: { + ...commonConfig.resolve, + // Provide empty fallbacks for Node.js modules used by zubridge's debug dependency + fallback: { + fs: false, + path: false, + os: false, + crypto: false, + util: false, + // Handle node: prefix scheme (used by uuid package) + 'node:crypto': false, + }, + // Alias process and tty to our minimal shims (supports-color needs them) + alias: { + process: path.resolve(__dirname, 'scripts/process-shim.js'), + tty: path.resolve(__dirname, 'scripts/tty-shim.js'), + }, + }, + plugins: [ + // Handle node: scheme imports by aliasing them to non-prefixed versions + new webpack.NormalModuleReplacementPlugin( + /^node:/, + (resource) => { + resource.request = resource.request.replace(/^node:/, ''); + } + ), + // Replace global process and tty references with our shims + new webpack.ProvidePlugin({ + process: path.resolve(__dirname, 'scripts/process-shim.js'), + tty: path.resolve(__dirname, 'scripts/tty-shim.js'), + }), + ], }; // Renderer process configuration