From 9260d35957541c9879f8569ced8a791445e7dac5 Mon Sep 17 00:00:00 2001 From: NuklearRabbit <145918611+NuklearRabbit@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:31:58 +0200 Subject: [PATCH] fix: repair broken IPC wiring and cut the cost of repository polling Three handlers referenced a dependency they were never given, which made them throw a ReferenceError as soon as they ran: - deployment:preflight for Gitea Actions profiles (`preflight` was passed to registerOperationsIpc but not to registerDeploymentIpc) - Unraid write-access repair (`safeRelativeRemoteFile` was missing from createUnraidAccessMethods) - a dead reference of the same name in unraid-state-methods no-undef and no-unused-vars were disabled for every file, which is why none of these were caught. Both are now enabled for src/main and src/shared, where the dependency graph is explicit. The renderer keeps them off because its functions are deliberately cross-script globals. Performance: - git.status() spawned three processes (rev-parse, status, remote get-url) per call. A directory holding its own .git is by definition the work tree root, so rev-parse is unnecessary, and the remote URL is cached against the mtime of .git/config, including the failure for a repository without that remote. - git status runs with --no-optional-locks so a read no longer rewrites the index. That stops it fighting a concurrent Git command for the index lock, and is what makes filesystem watching viable at all. - One commit issued four `git status` reads; callers that already hold the status now pass it on, leaving two. - The repository monitor is event driven. A watched repository is read on filesystem activity, with a 30s safety net for watchers that stop delivering and a 1s floor so a busy tree cannot drive a read per event. Repositories that cannot be watched keep using the interval. Idle cost for one repository over 35s: 24 git processes before, 3 after. - Resolving one repository by name no longer refreshes the whole workspace. - Concurrent configuration saves share a single write of the latest state. - Repository discovery follows directory junctions again. The filter that skipped them made the realpath cycle guard dead code, and hid any project folder reached through a junction. Renderer: - render() replaced the whole shell on every poll, discarding focus, caret and scroll position while the user was typing. Those are preserved now, and an unchanged render leaves the DOM alone entirely. - The four sections that enhanceRenderedUi() injected after render moved into the views, so the rendered markup is the single source of truth. - The monitor no longer keeps a repository paused forever when it is unlinked mid-mutation, scheduleAutoRefresh honours its delay argument, the demo bridges no longer block startup, and #app is no longer an aria-live region announcing the entire UI on every render. IPC channel plumbing moved to src/main/ipc/channel.cjs, replacing a module-level mutable diagnostics singleton with an argument. Co-Authored-By: Claude Opus 5 --- eslint.config.js | 28 ++++ src/main/config-store.cjs | 19 ++- src/main/deployment-service.cjs | 4 - src/main/git-service.cjs | 77 +++++++++-- src/main/ipc.cjs | 82 ++--------- src/main/ipc/channel.cjs | 77 +++++++++++ src/main/ipc/deployment-handlers.cjs | 1 + src/main/repository-monitor.cjs | 104 +++++++++++++- src/main/repository-service.cjs | 41 +++++- src/main/ssh-service.cjs | 1 - src/main/unraid-access-methods.cjs | 2 +- src/main/unraid-deployment-service.cjs | 2 +- src/main/unraid-state-methods.cjs | 3 - src/renderer/app.js | 4 +- src/renderer/dialogs.js | 183 +++++++++++++++++++------ src/renderer/index.html | 8 +- src/renderer/views.js | 1 + tests/browser/forgeflow.spec.mjs | 79 +++++++++++ tests/dependency-wiring.test.mjs | 165 ++++++++++++++++++++++ tests/repository-monitor.test.mjs | 53 +++++++ tests/repository-service.test.mjs | 71 ++++++++++ 21 files changed, 858 insertions(+), 147 deletions(-) create mode 100644 src/main/ipc/channel.cjs create mode 100644 tests/dependency-wiring.test.mjs diff --git a/eslint.config.js b/eslint.config.js index 8d6acec..12c320c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -29,6 +29,34 @@ export default [ eqeqeq: ["error", "always", { null: "ignore" }], }, }, + { + // The main process and shared modules are plain CommonJS with an explicit + // dependency graph, so undefined identifiers there are always real bugs + // (missing require, missing injected dependency) rather than a global that + // another script tag happens to define. + files: ["src/main/**/*.cjs", "src/shared/**/*.cjs", "main.cjs", "preload.cjs"], + languageOptions: { + sourceType: "commonjs", + globals: { + require: "readonly", module: "writable", exports: "writable", + __dirname: "readonly", __filename: "readonly", + Buffer: "readonly", process: "readonly", console: "readonly", + setTimeout: "readonly", clearTimeout: "readonly", + setInterval: "readonly", clearInterval: "readonly", setImmediate: "readonly", + queueMicrotask: "readonly", structuredClone: "readonly", globalThis: "readonly", + URL: "readonly", URLSearchParams: "readonly", fetch: "readonly", + FormData: "readonly", Blob: "readonly", + AbortController: "readonly", AbortSignal: "readonly", + TextEncoder: "readonly", TextDecoder: "readonly", + }, + }, + rules: { + "no-undef": "error", + // Also catches code that a refactor left behind, such as a value computed + // from a dependency that is no longer injected. + "no-unused-vars": ["error", { args: "none", caughtErrors: "none", ignoreRestSiblings: true }], + }, + }, { files: ["tests/**/*.mjs"], rules: { diff --git a/src/main/config-store.cjs b/src/main/config-store.cjs index ce38ee2..e29ec9e 100644 --- a/src/main/config-store.cjs +++ b/src/main/config-store.cjs @@ -56,6 +56,8 @@ class ConfigStore { this.sessionToken = null; this.data = structuredClone(DEFAULT_CONFIG); this.saveQueue = Promise.resolve(); + this.pendingSave = null; + this.lastWrittenSnapshot = null; } migrate(parsed) { @@ -138,16 +140,27 @@ class ConfigStore { } async save() { - const snapshot = JSON.stringify(this.data, null, 2); + // Several callers persist in quick succession (a server scan writes deployment + // state per workload). Serializing the configuration once per call is the + // expensive part, so saves that are still queued share a single write of the + // latest data. That is equivalent because every caller asks for "persist the + // current configuration", not "persist the snapshot I saw". + if (this.pendingSave) return this.pendingSave; const operation = async () => { + this.pendingSave = null; + const snapshot = JSON.stringify(this.data, null, 2); + if (snapshot === this.lastWrittenSnapshot + && await fs.access(this.filePath).then(() => true).catch(() => false)) return; await fs.mkdir(path.dirname(this.filePath), { recursive: true }); const temporary = `${this.filePath}.${process.pid}.${Date.now()}.${crypto.randomUUID()}.tmp`; await fs.writeFile(temporary, snapshot, { mode: 0o600 }); await fs.rename(temporary, this.filePath); try { await fs.chmod(this.filePath, 0o600); } catch {} + this.lastWrittenSnapshot = snapshot; }; - this.saveQueue = this.saveQueue.then(operation, operation); - return this.saveQueue; + this.pendingSave = this.saveQueue.then(operation, operation); + this.saveQueue = this.pendingSave.catch(() => {}); + return this.pendingSave; } getGitValidatorState(fullName) { diff --git a/src/main/deployment-service.cjs b/src/main/deployment-service.cjs index 9ea1412..7e72474 100644 --- a/src/main/deployment-service.cjs +++ b/src/main/deployment-service.cjs @@ -36,10 +36,6 @@ function isRunningStatus(value) { return ['running', 'in_progress', 'processing'].includes(String(value || '').toLowerCase()); } -function isQueuedStatus(value) { - return ['pending', 'queued', 'waiting', 'blocked', 'requested'].includes(String(value || '').toLowerCase()); -} - class DeploymentService { constructor(store, giteaService, gitService, diagnostics = null) { this.store = store; diff --git a/src/main/git-service.cjs b/src/main/git-service.cjs index 328deba..966e580 100644 --- a/src/main/git-service.cjs +++ b/src/main/git-service.cjs @@ -29,6 +29,13 @@ function parseUnifiedDiff(diffText) { } class GitService { + constructor() { + // `git remote get-url` is only re-run when the repository configuration file + // itself changed. Status polling asks for the remote URL of every repository + // every few seconds, and on Windows the child process dominates that cost. + this.remoteUrlCache = new Map(); + } + async isAvailable() { try { const result = await run('git', ['--version'], { timeout: 10_000 }); @@ -42,13 +49,24 @@ class GitService { const resolved = assertSafeRepositoryPath(repoPath); const stat = await fs.stat(resolved).catch(() => null); if (!stat?.isDirectory()) throw new Error('The linked local folder no longer exists.'); + // A directory that carries its own `.git` entry is by definition the top level + // of that working tree, for plain repositories as well as for submodules and + // linked worktrees where `.git` is a file. Spawning `git rev-parse` to learn + // that again is pure overhead, and every status poll passes an already + // resolved repository root back in. + const marker = await fs.stat(path.join(resolved, '.git')).catch(() => null); + if (marker) return resolved; const result = await run('git', ['rev-parse', '--show-toplevel'], { cwd: resolved, timeout: 15_000 }); return path.resolve(result.stdout.trim()); } async status(repoPath) { const root = await this.ensureRepository(repoPath); - const result = await run('git', ['status', '--porcelain=v2', '--branch', '-z', '--untracked-files=all'], { + // `--no-optional-locks` keeps a status read from refreshing and rewriting the + // index. Without it every read writes inside .git, which both fights a + // concurrent Git command for the index lock and retriggers the filesystem + // watcher that asked for this read in the first place. + const result = await run('git', ['--no-optional-locks', 'status', '--porcelain=v2', '--branch', '-z', '--untracked-files=all'], { cwd: root, timeout: 30_000 }); @@ -66,9 +84,34 @@ class GitService { }); } + remoteUrlCacheKey(repoPath, remote) { + return JSON.stringify([path.resolve(repoPath), remote]); + } + async getRemoteUrl(repoPath, remote = 'origin') { - const result = await run('git', ['remote', 'get-url', remote], { cwd: repoPath, timeout: 15_000 }); - return result.stdout.trim(); + const cacheKey = this.remoteUrlCacheKey(repoPath, remote); + const config = await fs.stat(path.join(repoPath, '.git', 'config')).catch(() => null); + const cached = this.remoteUrlCache.get(cacheKey); + if (config && cached && cached.mtimeMs === config.mtimeMs && cached.size === config.size) { + if (cached.error) throw cached.error; + return cached.url; + } + const remember = (entry) => { + if (config) this.remoteUrlCache.set(cacheKey, { ...entry, mtimeMs: config.mtimeMs, size: config.size }); + else this.remoteUrlCache.delete(cacheKey); + }; + try { + const result = await run('git', ['remote', 'get-url', remote], { cwd: repoPath, timeout: 15_000 }); + const url = result.stdout.trim(); + remember({ url, error: null }); + return url; + } catch (error) { + // A repository that has no such remote keeps failing until its configuration + // changes, so the failure is remembered too. Without this, every status poll + // of an unmatched local repository spawns a child process that cannot succeed. + remember({ url: '', error }); + throw error; + } } @@ -279,6 +322,7 @@ class GitService { const name = String(remote || 'origin').trim(); if (!/^[A-Za-z0-9._-]+$/.test(name)) throw new Error('Invalid Git remote name.'); await run('git', ['remote', 'set-url', name, safeRemote], { cwd: root, timeout: 30_000 }); + this.remoteUrlCache.delete(this.remoteUrlCacheKey(root, name)); return this.status(root); } @@ -354,8 +398,7 @@ class GitService { return { selected, matches }; } - async expandSelectedPaths(root, files, { unstagedOnly = false } = {}) { - const status = await this.status(root); + expandStatusPaths(status, files, { unstagedOnly = false } = {}) { const { selected, matches } = this.selectedStatusFiles(status, files); if (!selected.length) return []; const expanded = new Set(); @@ -367,12 +410,17 @@ class GitService { return [...expanded]; } - async stage(repoPath, files) { - const root = await this.ensureRepository(repoPath); + async expandSelectedPaths(root, files, options = {}) { + return this.expandStatusPaths(await this.status(root), files, options); + } + + // Callers that already read the status pass it in. Reading it again costs a + // child process, and a commit used to pay for four of them. + async applyStage(root, files, knownStatus = null) { const requested = assertRepositoryRelativePaths(files); if (!requested.length) { await run('git', ['add', '--all'], { cwd: root, timeout: 60_000 }); - return this.status(root); + return; } // Only stage records that still have a worktree-side change. Re-running @@ -380,10 +428,16 @@ class GitService { // Git fail with "pathspec did not match any files" because the file no // longer exists in either the worktree or HEAD. Staged-only deletions and // renames are already ready for commit and must therefore be left alone. - const selected = await this.expandSelectedPaths(root, requested, { unstagedOnly: true }); + const status = knownStatus || await this.status(root); + const selected = this.expandStatusPaths(status, requested, { unstagedOnly: true }); if (selected.length) { await this.runWithPathspec(root, ['add', '-A'], selected, { timeout: 120_000 }); } + } + + async stage(repoPath, files) { + const root = await this.ensureRepository(repoPath); + await this.applyStage(root, files); return this.status(root); } @@ -403,8 +457,9 @@ class GitService { async prepareSelectedStage(root, files) { const selected = assertRepositoryRelativePaths(files); + let current = null; if (selected.length) { - const current = await this.status(root); + current = await this.status(root); const excludedStaged = current.files .filter((file) => file.staged) .filter((file) => !selected.includes(file.path) && !(file.originalPath && selected.includes(file.originalPath))) @@ -413,7 +468,7 @@ class GitService { throw new Error(`Some staged files are not selected (${excludedStaged.slice(0, 3).join(', ')}${excludedStaged.length > 3 ? ', …' : ''}). Select them or unstage them first.`); } } - await this.stage(root, selected); + await this.applyStage(root, selected, current); const stagedCheck = await run('git', ['diff', '--cached', '--quiet'], { cwd: root, allowExitCodes: [1] }); if (stagedCheck.exitCode === 0) throw new Error('There are no staged changes to commit.'); return selected; diff --git a/src/main/ipc.cjs b/src/main/ipc.cjs index f3ae7ec..2050aed 100644 --- a/src/main/ipc.cjs +++ b/src/main/ipc.cjs @@ -1,13 +1,17 @@ "use strict"; const path = require("node:path"); const fs = require("node:fs/promises"); -const { fileURLToPath } = require("node:url"); -const { ipcMain, dialog, shell, app } = require("electron"); +const { dialog, shell, app } = require("electron"); const { matchRemoteToRepository } = require("../shared/repository-match.cjs"); const { cloneDirectoryName, resolveCloneTarget, } = require("../shared/clone-target.cjs"); +const { + createChannelRegistrar, + assertTrustedSender, + toErrorPayload, +} = require("./ipc/channel.cjs"); const { registerRepositoryIpc } = require("./ipc/repository-handlers.cjs"); const { registerDeploymentIpc } = require("./ipc/deployment-handlers.cjs"); const { registerOperationsIpc } = require("./ipc/operations-handlers.cjs"); @@ -16,66 +20,6 @@ const { readEncryptedBackup, } = require("./configuration-backup.cjs"); const { evaluateDeploymentPolicy } = require("../shared/deployment-policy.cjs"); -let diagnosticsService = null; -const TRUSTED_RENDERER_PATH = path.resolve( - __dirname, - "..", - "renderer", - "index.html", -); - -function toErrorPayload(error) { - return { - message: error?.message || "Unknown error", - code: error?.code || null, - status: error?.status || null, - recoverable: Boolean(error?.recoverable), - commitSha: error?.commitSha || null, - }; -} - -function assertTrustedSender(event) { - const url = event?.senderFrame?.url || event?.sender?.getURL?.() || ""; - try { - const parsed = new URL(url); - if (parsed.protocol !== "file:") throw new Error("not a file URL"); - const senderPath = path.resolve(fileURLToPath(parsed)); - const normalize = (value) => - process.platform === "win32" ? value.toLowerCase() : value; - if (normalize(senderPath) !== normalize(TRUSTED_RENDERER_PATH)) - throw new Error("unexpected renderer file"); - } catch { - throw new Error("Rejected IPC request from an untrusted renderer origin."); - } -} - -function register(channel, handler) { - ipcMain.handle(channel, async (event, payload) => { - const started = Date.now(); - try { - assertTrustedSender(event); - const data = await handler(payload || {}, event); - await diagnosticsService?.debug("ipc.completed", { - channel, - durationMs: Date.now() - started, - }); - return { ok: true, data }; - } catch (error) { - await diagnosticsService?.error("ipc.failed", { - channel, - durationMs: Date.now() - started, - error: { - name: error?.name, - message: error?.message, - code: error?.code, - status: error?.status, - stack: error?.stack, - }, - }); - return { ok: false, error: toErrorPayload(error) }; - } - }); -} function registerIpc({ store, git, @@ -95,7 +39,7 @@ function registerIpc({ monitor, onPreferencesChanged, }) { - diagnosticsService = diagnostics; + const register = createChannelRegistrar(diagnostics); const repositoryMutations = new Map(); const withRepositoryPause = async (localPath, action) => { monitor?.pause(localPath); @@ -161,8 +105,11 @@ function registerIpc({ await repositories.refresh(); knownPaths = repositories.getWatchPaths(); } - const canonicalKnown = await Promise.all(knownPaths.map(canonicalPath)); - if (!canonicalKnown.some((known) => known === candidate)) + // Watch paths are already canonical, so re-resolving all of them on every + // guarded call is only needed when the cheap comparison finds no match. + const matched = knownPaths.some((known) => path.resolve(known) === candidate) + || (await Promise.all(knownPaths.map(canonicalPath))).some((known) => known === candidate); + if (!matched) throw new Error( "The requested local repository is not linked or discovered by ForgeFlow.", ); @@ -172,9 +119,7 @@ function registerIpc({ const resolveRepository = async (repositoryPayload) => { const fullName = String(repositoryPayload?.fullName || "").trim(); if (!fullName) throw new Error("Repository identity is required."); - const current = (await repositories.refresh()).find( - (item) => item.fullName === fullName, - ); + const current = await repositories.resolveByFullName(fullName); if (!current) throw new Error( "The repository is no longer available through the configured Gitea account.", @@ -732,6 +677,7 @@ function registerIpc({ registerDeploymentIpc({ register, store, resolveRepository, unraid, deployments, evaluateDeploymentPolicy, audit, deployKeys, repositories, inventoryReviews, diagnostics, git, gitea, ssh, + preflight, }); registerOperationsIpc({ register, store, unraid, deployments, diagnostics, shell, dialog, path, app, diff --git a/src/main/ipc/channel.cjs b/src/main/ipc/channel.cjs new file mode 100644 index 0000000..ee8a252 --- /dev/null +++ b/src/main/ipc/channel.cjs @@ -0,0 +1,77 @@ +"use strict"; + +const path = require("node:path"); +const { fileURLToPath } = require("node:url"); +const { ipcMain } = require("electron"); + +const TRUSTED_RENDERER_PATH = path.resolve( + __dirname, + "..", + "..", + "renderer", + "index.html", +); + +function toErrorPayload(error) { + return { + message: error?.message || "Unknown error", + code: error?.code || null, + status: error?.status || null, + recoverable: Boolean(error?.recoverable), + commitSha: error?.commitSha || null, + }; +} + +function assertTrustedSender(event) { + const url = event?.senderFrame?.url || event?.sender?.getURL?.() || ""; + try { + const parsed = new URL(url); + if (parsed.protocol !== "file:") throw new Error("not a file URL"); + const senderPath = path.resolve(fileURLToPath(parsed)); + const normalize = (value) => + process.platform === "win32" ? value.toLowerCase() : value; + if (normalize(senderPath) !== normalize(TRUSTED_RENDERER_PATH)) + throw new Error("unexpected renderer file"); + } catch { + throw new Error("Rejected IPC request from an untrusted renderer origin."); + } +} + +// Built per registerIpc() call so the diagnostics sink is an argument instead of +// module-level mutable state that every handler silently depends on. +function createChannelRegistrar(diagnostics) { + return function register(channel, handler) { + ipcMain.handle(channel, async (event, payload) => { + const started = Date.now(); + try { + assertTrustedSender(event); + const data = await handler(payload || {}, event); + await diagnostics?.debug("ipc.completed", { + channel, + durationMs: Date.now() - started, + }); + return { ok: true, data }; + } catch (error) { + await diagnostics?.error("ipc.failed", { + channel, + durationMs: Date.now() - started, + error: { + name: error?.name, + message: error?.message, + code: error?.code, + status: error?.status, + stack: error?.stack, + }, + }); + return { ok: false, error: toErrorPayload(error) }; + } + }); + }; +} + +module.exports = { + createChannelRegistrar, + assertTrustedSender, + toErrorPayload, + TRUSTED_RENDERER_PATH, +}; diff --git a/src/main/ipc/deployment-handlers.cjs b/src/main/ipc/deployment-handlers.cjs index 5969774..ee8bca1 100644 --- a/src/main/ipc/deployment-handlers.cjs +++ b/src/main/ipc/deployment-handlers.cjs @@ -3,6 +3,7 @@ function registerDeploymentIpc({ register, store, resolveRepository, unraid, deployments, evaluateDeploymentPolicy, audit, deployKeys, repositories, inventoryReviews, diagnostics, git, gitea, ssh, + preflight, }) { register("deployment:save-profile", async ({ fullName, profile }) => { const saved = await store.saveDeploymentProfile(fullName, profile); diff --git a/src/main/repository-monitor.cjs b/src/main/repository-monitor.cjs index 08fdc3e..9528e6e 100644 --- a/src/main/repository-monitor.cjs +++ b/src/main/repository-monitor.cjs @@ -1,5 +1,16 @@ 'use strict'; +const fs = require('node:fs'); + +// A watched repository is only re-read when the filesystem reports activity. The +// interval below stays as a safety net for watchers that silently stop +// delivering, which happens on network shares and removed folders. +const SAFETY_CHECK_INTERVAL_MS = 30_000; +const WATCH_DEBOUNCE_MS = 250; +// Busy trees (a build, an install, a fetch) produce a continuous event stream. +// This bounds how often that can turn into a Git read. +const MIN_WATCH_CHECK_INTERVAL_MS = 1_000; + class RepositoryMonitor { constructor({ store, git, onChange, diagnostics = null }) { this.store = store; @@ -11,13 +22,87 @@ class RepositoryMonitor { this.timer = null; this.running = false; this.paused = new Set(); + this.active = false; + this.watchers = new Map(); + this.changed = new Set(); + this.lastCheckedAt = new Map(); + this.watchTimer = null; } setPaths(paths) { this.paths = [...new Set((paths || []).filter(Boolean))]; + const watched = new Set(this.paths); for (const existing of [...this.fingerprints.keys()]) { - if (!this.paths.includes(existing)) this.fingerprints.delete(existing); + if (!watched.has(existing)) this.fingerprints.delete(existing); } + // A repository that is unlinked while a mutation holds it paused would keep + // that pause forever, silently freezing its status once it is watched again. + for (const existing of [...this.paused]) { + if (!watched.has(existing)) this.paused.delete(existing); + } + for (const existing of [...this.changed]) { + if (!watched.has(existing)) this.changed.delete(existing); + } + for (const existing of [...this.lastCheckedAt.keys()]) { + if (!watched.has(existing)) this.lastCheckedAt.delete(existing); + } + this.syncWatchers(); + } + + syncWatchers() { + for (const [localPath, watcher] of [...this.watchers]) { + if (this.active && this.paths.includes(localPath)) continue; + this.closeWatcher(localPath, watcher); + } + if (!this.active) return; + for (const localPath of this.paths) { + if (this.watchers.has(localPath)) continue; + try { + const watcher = fs.watch( + localPath, + { recursive: true, persistent: false }, + () => this.noteFilesystemChange(localPath) + ); + watcher.on('error', () => this.dropWatcher(localPath)); + this.watchers.set(localPath, watcher); + } catch { + // Watching is unavailable for this folder. Leaving it unwatched makes + // shouldCheck() fall back to the interval for that repository only. + } + } + } + + closeWatcher(localPath, watcher = this.watchers.get(localPath)) { + if (!watcher) return; + try { watcher.close(); } catch { /* already closed */ } + this.watchers.delete(localPath); + } + + dropWatcher(localPath) { + this.closeWatcher(localPath); + this.changed.add(localPath); + } + + noteFilesystemChange(localPath) { + this.changed.add(localPath); + this.scheduleWatchTick(); + } + + scheduleWatchTick() { + if (this.watchTimer) return; + this.watchTimer = setTimeout(() => { + this.watchTimer = null; + this.tick().catch((error) => this.diagnostics?.warning('repository-monitor.tick.failed', error)); + }, WATCH_DEBOUNCE_MS); + this.watchTimer.unref?.(); + } + + shouldCheck(localPath, now) { + if (this.paused.has(localPath)) return false; + if (!this.watchers.has(localPath)) return true; + const sinceLastCheck = now - (this.lastCheckedAt.get(localPath) || 0); + if (this.changed.has(localPath)) return sinceLastCheck >= MIN_WATCH_CHECK_INTERVAL_MS; + return sinceLastCheck >= SAFETY_CHECK_INTERVAL_MS; } pause(localPath) { if (localPath) this.paused.add(localPath); } @@ -26,6 +111,8 @@ class RepositoryMonitor { restart() { this.stop(); if (!this.store.data.preferences.autoRefresh) return; + this.active = true; + this.syncWatchers(); const seconds = Math.min(Math.max(Number(this.store.data.preferences.repositoryPollSeconds) || 4, 2), 60); this.timer = setInterval(() => this.tick().catch((error) => this.diagnostics?.warning('repository-monitor.tick.failed', error)), seconds * 1000); this.timer.unref?.(); @@ -34,17 +121,23 @@ class RepositoryMonitor { stop() { if (this.timer) clearInterval(this.timer); this.timer = null; + if (this.watchTimer) clearTimeout(this.watchTimer); + this.watchTimer = null; + this.active = false; + this.syncWatchers(); } async tick() { if (this.running || !this.paths.length) return; this.running = true; try { - const queue = [...this.paths]; + const now = Date.now(); + const queue = this.paths.filter((localPath) => this.shouldCheck(localPath, now)); const workers = Array.from({ length: Math.min(4, queue.length) }, async () => { while (queue.length) { const localPath = queue.shift(); - if (this.paused.has(localPath)) continue; + this.changed.delete(localPath); + this.lastCheckedAt.set(localPath, Date.now()); try { const status = await this.git.status(localPath); const next = this.git.statusFingerprint(status); @@ -68,8 +161,11 @@ class RepositoryMonitor { await Promise.all(workers); } finally { this.running = false; + // Activity that arrived while the check was running keeps its flag set, so + // it must not wait for the safety interval. + if (this.active && this.changed.size) this.scheduleWatchTick(); } } } -module.exports = { RepositoryMonitor }; +module.exports = { RepositoryMonitor, SAFETY_CHECK_INTERVAL_MS, WATCH_DEBOUNCE_MS, MIN_WATCH_CHECK_INTERVAL_MS }; diff --git a/src/main/repository-service.cjs b/src/main/repository-service.cjs index 00569ec..797f3dd 100644 --- a/src/main/repository-service.cjs +++ b/src/main/repository-service.cjs @@ -35,6 +35,7 @@ class RepositoryService { this.lastDiscoveredPaths = []; this.lastDiscoveryAtMs = 0; this.refreshPromise = null; + this.lastResult = null; } async discoverInRoot(root, maxDepth = 4) { @@ -57,8 +58,13 @@ class RepositoryService { let entries; try { entries = await fs.readdir(real, { withFileTypes: true }); } catch { return; } + // Directory entries report as a symbolic link instead of a directory, which + // is how Windows junctions surface. Skipping those made a project folder + // that is mapped through a junction invisible; visit() resolves each entry + // and the `seen` set above keeps links that point back into the tree from + // being scanned twice. await mapLimit(entries - .filter((entry) => entry.isDirectory() && !entry.isSymbolicLink() && !SKIP_DIRECTORIES.has(entry.name)), 12, + .filter((entry) => (entry.isDirectory() || entry.isSymbolicLink()) && !SKIP_DIRECTORIES.has(entry.name)), 12, (entry) => visit(path.join(real, entry.name), depth + 1)); }; @@ -133,6 +139,38 @@ class RepositoryService { return paths; } + // Resolving a single repository used to go through a full refresh, which runs + // `git status` for every discovered repository. Handlers that act on one + // repository only need that one, so its local state is read directly. Anything + // this cannot answer confidently still falls back to the full scan. + async resolveByFullName(fullName) { + const name = String(fullName || '').trim(); + if (!name) return null; + const fromFullRefresh = async () => (await this.refresh()).find((item) => item.fullName === name) || null; + + const remoteResult = await this.getRemoteRepositories({}); + const remote = remoteResult.repositories.find((item) => item.full_name === name); + if (!remote) return fromFullRefresh(); + + const explicitPath = this.store.data.repositoryMappings[repositoryKey(remote)]; + const knownPath = explicitPath || (this.lastResult || []).find((item) => item.fullName === name)?.localPath || null; + // Without a known path the link can still exist through remote-URL matching, + // which only the discovery pass can establish. + if (!knownPath && !this.lastResult) return fromFullRefresh(); + + const local = knownPath ? (await this.getLocalDescriptors([knownPath]))[0] : null; + const profiles = this.store.getDeploymentProfiles(remote.full_name).map((profile) => ({ + ...profile, + state: this.store.getDeploymentState(profile.id) + })); + return { + ...this.decorate(remote, local, profiles), + remoteStale: remoteResult.stale, + remoteRefreshError: remoteResult.error, + remoteLastRefreshedAt: this.lastSuccessfulRemoteRefreshAt + }; + } + async refresh(options = {}) { if (this.refreshPromise) return this.refreshPromise; this.refreshPromise = this.performRefresh(options).finally(() => { this.refreshPromise = null; }); @@ -215,6 +253,7 @@ class RepositoryService { attentionCount: sorted.filter((item) => item.attention).length, readyToDeployCount: sorted.filter((item) => item.readyToDeploy).length }); + this.lastResult = sorted; return sorted; } diff --git a/src/main/ssh-service.cjs b/src/main/ssh-service.cjs index 051d2e8..386575f 100644 --- a/src/main/ssh-service.cjs +++ b/src/main/ssh-service.cjs @@ -1,6 +1,5 @@ 'use strict'; -const fs = require('node:fs'); const fsp = require('node:fs/promises'); const crypto = require('node:crypto'); const path = require('node:path').posix; diff --git a/src/main/unraid-access-methods.cjs b/src/main/unraid-access-methods.cjs index d2b7e61..7e5489c 100644 --- a/src/main/unraid-access-methods.cjs +++ b/src/main/unraid-access-methods.cjs @@ -1,6 +1,6 @@ "use strict"; -function createUnraidAccessMethods({ shellQuote, path, bash, inventoryRemoteIdentity, checksSummary, crypto, parsePermissionInspection }) { +function createUnraidAccessMethods({ shellQuote, path, bash, inventoryRemoteIdentity, checksSummary, crypto, parsePermissionInspection, safeRelativeRemoteFile }) { class UnraidAccessMethods { serverGitRemote(repository, profile) { const candidates = [ diff --git a/src/main/unraid-deployment-service.cjs b/src/main/unraid-deployment-service.cjs index 11faa5e..b1b980a 100644 --- a/src/main/unraid-deployment-service.cjs +++ b/src/main/unraid-deployment-service.cjs @@ -490,7 +490,7 @@ for (const name of Object.getOwnPropertyNames(preflightMethods)) { if (name !== "constructor") Object.defineProperty(UnraidDeploymentService.prototype, name, Object.getOwnPropertyDescriptor(preflightMethods, name)); } -const accessMethods = createUnraidAccessMethods({ shellQuote, path, bash, inventoryRemoteIdentity, checksSummary, crypto, parsePermissionInspection }); +const accessMethods = createUnraidAccessMethods({ shellQuote, path, bash, inventoryRemoteIdentity, checksSummary, crypto, parsePermissionInspection, safeRelativeRemoteFile }); for (const name of Object.getOwnPropertyNames(accessMethods)) { if (name !== "constructor") Object.defineProperty(UnraidDeploymentService.prototype, name, Object.getOwnPropertyDescriptor(accessMethods, name)); } diff --git a/src/main/unraid-state-methods.cjs b/src/main/unraid-state-methods.cjs index 0b79bcc..0ace1f9 100644 --- a/src/main/unraid-state-methods.cjs +++ b/src/main/unraid-state-methods.cjs @@ -127,9 +127,6 @@ function createUnraidStateMethods({ path, bash, shellQuote, inventoryRemoteIdent // profile hint no longer matches the real Compose service keys. return this.refreshProfileState(repository.fullName, profileId); } - const composeFile = profile.generatedCompose - ? ".forgeflow/compose.forgeflow.yml" - : safeRelativeRemoteFile(profile.composeFile || "docker-compose.yml"); const iconReference = await this.prepareIcon(profile, repository, server); const metadata = this.metadataCompose(profile, repository, iconReference); const compose = this.composeInvocation(profile, repository); diff --git a/src/renderer/app.js b/src/renderer/app.js index 422e59e..b38fab2 100644 --- a/src/renderer/app.js +++ b/src/renderer/app.js @@ -413,7 +413,7 @@ async function bootstrap() { } } -function scheduleAutoRefresh() { +function scheduleAutoRefresh(delay = 450) { if ( ui.loading || ui.autoRefreshPending || @@ -424,7 +424,7 @@ function scheduleAutoRefresh() { setTimeout(async () => { ui.autoRefreshPending = false; await refreshRepositories(false, true); - }, 450); + }, delay); } async function refreshRepositories(withLoader = true, silent = false) { diff --git a/src/renderer/dialogs.js b/src/renderer/dialogs.js index bcd92ef..4ef50f1 100644 --- a/src/renderer/dialogs.js +++ b/src/renderer/dialogs.js @@ -1,3 +1,25 @@ +// These three sections used to be pushed into the DOM after render() had already +// written the shell. Keeping them in the markup makes the rendered output the +// single source of truth, so an unchanged render can be skipped safely. +function renderDeploymentPolicyFields(policy) { + const windows = (policy.maintenanceWindows || []) + .map((window) => `${window.days.join(",")}:${window.start}-${window.end}`) + .join(" | "); + return `

Deployment policy

Day 0 is Sunday. Separate windows with |.
`; +} + +function renderReleaseNoteFields(profile) { + return `
`; +} + +function renderWorkloadClassificationFields(workload) { + const type = workload?.classification?.type || "ambiguous"; + const recommended = type === "duplicate" ? "select-authoritative" : type === "stale-link" ? "archive-link" : type === "historical-compose" ? "mark-historical" : type === "orphan-container" ? "monitor-only" : "manual-link"; + const actions = [["manual-link", "Confirm selected repository match"], ["select-authoritative", "Select as authoritative instance"], ["mark-historical", "Mark historical definition"], ["archive-link", "Archive stale link"], ["monitor-only", "Keep for monitoring only"], ["manual-exclude", "Exclude this workload"], ["ignore", "Ignore with reason"]]; + const options = actions.map(([value, label]) => ``).join(""); + return `

Classify without touching containers

${icon("info")}
${escapeHtml(type)}

${escapeHtml(workload?.classification?.reason || "ForgeFlow needs an explicit decision for this workload.")}

The decision is tied to current evidence and becomes stale automatically when server truth changes.

`; +} + function renderModal() { if (!ui.modal) return ""; const repository = @@ -59,7 +81,7 @@ function renderModal() { .map((container) => container.name) .filter(Boolean) .join(", "); - return ``; + return ``; } if (ui.modal.type === "deployment-config") { const storedProfile = @@ -106,7 +128,7 @@ function renderModal() {
` - }
${icon("shield")}${ssh ? "Server pull fetches the exact selected Gitea commit with a repository-scoped read-only key, validates Compose and services, then promotes atomically with rollback protection." : "ForgeFlow sends only controlled workflow inputs: environment, exact SHA and a unique request ID."}
`; + }${renderDeploymentPolicyFields(storedProfile.deploymentPolicy || {})}
${icon("shield")}${ssh ? "Server pull fetches the exact selected Gitea commit with a repository-scoped read-only key, validates Compose and services, then promotes atomically with rollback protection." : "ForgeFlow sends only controlled workflow inputs: environment, exact SHA and a unique request ID."}
`; } if (ui.modal.type === "inventory-review-plan") { const plan = ui.inventoryReviewPlan; @@ -133,7 +155,7 @@ function renderModal() { (item) => item.id === ui.modal.profileId, ) || selectedProfile(repository); const targetSha = deploymentTargetSha(repository, profile); - return ``; + return ``; } if (ui.modal.type === "rollback-confirm") { const profile = repository?.deploymentProfiles?.find( @@ -239,46 +261,6 @@ function renderCommandPalette() { } function enhanceRenderedUi() { - const repository = selectedRepository(); - if (ui.modal?.type === "deployment-config") { - const profile = - repository?.deploymentProfiles?.find( - (item) => item.id === ui.modal.profileId, - ) || {}; - const policy = profile.deploymentPolicy || {}; - document - .querySelector(".modal-body .form-grid") - ?.insertAdjacentHTML( - "beforeend", - `

Deployment policy

Day 0 is Sunday. Separate windows with |.
`, - ); - } - if (ui.modal?.type === "workload-link") { - const workload = (ui.serverDiscovery || []).find((server) => server.serverId === ui.modal.serverId)?.workloads?.find((item) => item.workloadId === ui.modal.workloadId); - const type = workload?.classification?.type || "ambiguous"; - const recommended = type === "duplicate" ? "select-authoritative" : type === "stale-link" ? "archive-link" : type === "historical-compose" ? "mark-historical" : type === "orphan-container" ? "monitor-only" : "manual-link"; - const actions = [["manual-link", "Confirm selected repository match"], ["select-authoritative", "Select as authoritative instance"], ["mark-historical", "Mark historical definition"], ["archive-link", "Archive stale link"], ["monitor-only", "Keep for monitoring only"], ["manual-exclude", "Exclude this workload"], ["ignore", "Ignore with reason"]]; - const options = actions.map(([value, label]) => ``).join(""); - document.querySelector(".modal-body")?.insertAdjacentHTML("beforeend", `

Classify without touching containers

${icon("info")}
${escapeHtml(type)}

${escapeHtml(workload?.classification?.reason || "ForgeFlow needs an explicit decision for this workload.")}

The decision is tied to current evidence and becomes stale automatically when server truth changes.

`); - } - if (ui.modal?.type === "deploy-confirm") { - const profile = repository?.deploymentProfiles?.find( - (item) => item.id === ui.modal.profileId, - ); - document - .querySelector(".modal-body") - ?.insertAdjacentHTML( - "beforeend", - `
`, - ); - } - if (ui.currentView === "diagnostics") { - const container = document.querySelector(".diagnostics-page"); - container?.insertAdjacentHTML( - "beforeend", - `

Operational audit log

Append-only release, pull-request and recovery events
${ui.auditEvents.length ? `${ui.auditEvents.map((item) => ``).join("")}
TimeEventRepositoryResult
${formatDate(item.timestamp)}${escapeHtml(item.event)}${escapeHtml(item.details?.repository || "—")}${escapeHtml(item.details?.result || item.details?.note || "—")}
` : '

Load the operational audit log.

'}
`, - ); - } document.querySelectorAll("button.icon-button:not([aria-label])").forEach((button) => { const action = String(button.title || button.dataset.action || "Action").replaceAll("-", " "); button.setAttribute("aria-label", action.charAt(0).toUpperCase() + action.slice(1)); @@ -296,6 +278,110 @@ function enhanceRenderedUi() { }); } +// A render replaces the complete application shell. Without this, a background +// repository poll or deployment poll destroys the element the user is typing in, +// discarding the caret position and every scroll offset on screen. +function elementRenderPath(element) { + const parts = []; + let node = element; + while (node && node !== app) { + const parent = node.parentElement; + if (!parent) return null; + parts.push(`${node.tagName}.${Array.prototype.indexOf.call(parent.children, node)}`); + node = parent; + } + return node === app ? parts.reverse().join(">") : null; +} + +function elementAtRenderPath(renderPath) { + let node = app; + for (const part of renderPath.split(">")) { + const separator = part.lastIndexOf("."); + node = node?.children?.[Number(part.slice(separator + 1))]; + // The shell can be structurally different after a view change, in which case + // the old offset belongs to an unrelated element and must be dropped. + if (!node || node.tagName !== part.slice(0, separator)) return null; + } + return node; +} + +// enhanceRenderedUi() re-injects these controls empty on every render, so a +// background refresh would otherwise discard a release note or review reason +// while the user is still writing it. +const INJECTED_FIELD_IDS = [ + "deployment-note", + "deployment-override", + "deployment-override-reason", + "inventory-review-action", + "inventory-review-reason", + "profile-policy-frozen", + "profile-policy-note", + "profile-policy-freeze-reason", + "profile-policy-windows", +]; + +function captureInjectedFieldValues() { + const values = []; + for (const id of INJECTED_FIELD_IDS) { + const element = document.getElementById(id); + if (!element) continue; + if (element.type === "checkbox") values.push({ id, checked: element.checked }); + else if (element.value) values.push({ id, value: element.value }); + } + return values; +} + +function restoreInjectedFieldValues(values) { + for (const entry of values) { + const element = document.getElementById(entry.id); + if (!element) continue; + // Never overwrite a value the freshly rendered control already carries; only + // fill back in what the injection left empty. + if ("checked" in entry) { + if (!element.checked) element.checked = entry.checked; + } else if (!element.value) element.value = entry.value; + } +} + +function captureInteractionState() { + const scroll = []; + for (const element of app.querySelectorAll("*")) { + if (!element.scrollTop && !element.scrollLeft) continue; + const renderPath = elementRenderPath(element); + if (renderPath) scroll.push({ renderPath, top: element.scrollTop, left: element.scrollLeft }); + } + const injectedFields = captureInjectedFieldValues(); + const active = document.activeElement; + if (!active?.id || !app.contains(active)) return { scroll, injectedFields, focus: null }; + const focus = { id: active.id, start: null, end: null, direction: "none" }; + try { + focus.start = active.selectionStart; + focus.end = active.selectionEnd; + focus.direction = active.selectionDirection || "none"; + } catch {} + return { scroll, injectedFields, focus }; +} + +function restoreInteractionState(state) { + restoreInjectedFieldValues(state.injectedFields); + for (const entry of state.scroll) { + const element = elementAtRenderPath(entry.renderPath); + if (!element) continue; + element.scrollTop = entry.top; + element.scrollLeft = entry.left; + } + if (!state.focus) return; + const element = document.getElementById(state.focus.id); + if (!element || !app.contains(element)) return; + element.focus({ preventScroll: true }); + if (state.focus.start === null) return; + try { + element.setSelectionRange(state.focus.start, state.focus.end, state.focus.direction); + } catch {} +} + +let lastRenderedMarkup = null; + function render() { if (!ui.boot) return; const repository = selectedRepository(); @@ -314,8 +400,17 @@ function render() { ? renderRepositoryWorkspace(repository) : renderOverview(); const withPanel = ui.currentView === "repository" && repository; - app.innerHTML = `
${renderTitlebar()}
${renderSidebar()}
${main}
${withPanel ? renderActionPanel(repository) : ""}${ui.loading ? `
${escapeHtml(ui.loadingMessage || "Working…")}
` : ""}
${renderStatusbar()}
${ui.boot.state.setupComplete ? "" : renderSetup()}${renderModal()}`; + const markup = `
${renderTitlebar()}
${renderSidebar()}
${main}
${withPanel ? renderActionPanel(repository) : ""}${ui.loading ? `
${escapeHtml(ui.loadingMessage || "Working…")}
` : ""}
${renderStatusbar()}
${ui.boot.state.setupComplete ? "" : renderSetup()}${renderModal()}`; + // Most renders are triggered by a poll that found nothing new. Rebuilding an + // identical shell would only cost layout work and interrupt the user. The + // markup is the complete rendered state, so comparing it is sufficient: + // enhanceRenderedUi() only derives labels and ids from what is already there. + if (markup === lastRenderedMarkup) return; + const interaction = captureInteractionState(); + app.innerHTML = markup; enhanceRenderedUi(); + restoreInteractionState(interaction); + lastRenderedMarkup = markup; if (ui.modal?.type === "command-palette") requestAnimationFrame(() => document.querySelector("#palette-input")?.focus(), diff --git a/src/renderer/index.html b/src/renderer/index.html index 510f1c1..653968f 100644 --- a/src/renderer/index.html +++ b/src/renderer/index.html @@ -9,7 +9,7 @@ -
+
Starting ForgeFlow @@ -17,9 +17,9 @@
- - - + + + diff --git a/src/renderer/views.js b/src/renderer/views.js index 47494dc..33ec63f 100644 --- a/src/renderer/views.js +++ b/src/renderer/views.js @@ -687,6 +687,7 @@ function renderDiagnostics() {

One-click troubleshooter

Git locks, interrupted operations, branch synchronization and deployment/server inconsistencies
${trouble?.issues?.some((item) => item.repairable && item.safe) ? `` : ""}
${trouble ? `${trouble.summary.total ? `${trouble.summary.total} issue(s)` : "Healthy"}${trouble.summary.errors} errors · ${trouble.summary.warnings} warnings · ${trouble.summary.repairable} repairable` : "Run the troubleshooter to inspect all linked repositories and deployments."}
${troubleRows || '

No problems detected.

'}

System preflight

Git, writable storage, credential protection, folders and Gitea
${report ? `${report.summary.ready ? "Ready" : `${report.summary.blocking.length} blocking`}${report.summary.counts.pass} passed · ${report.summary.counts.warning} warnings · ${report.summary.counts.fail} failed` : "Not run in this session"}
${renderPreflightChecks(report)}

Export support bundle

Configuration summary, repository states, operations, preflight and redacted JSONL logs
${ui.lastDiagnosticBundle ? `
${icon("check")}
${escapeHtml(ui.lastDiagnosticBundle.size)} bundle created

SHA-256 ${escapeHtml(ui.lastDiagnosticBundle.sha256)}

` : ""}
+

Operational audit log

Append-only release, pull-request and recovery events
${ui.auditEvents.length ? `${ui.auditEvents.map((item) => ``).join("")}
TimeEventRepositoryResult
${formatDate(item.timestamp)}${escapeHtml(item.event)}${escapeHtml(item.details?.repository || "—")}${escapeHtml(item.details?.result || item.details?.note || "—")}
` : '

Load the operational audit log.

'}
`; } diff --git a/tests/browser/forgeflow.spec.mjs b/tests/browser/forgeflow.spec.mjs index 92b6cb0..102315b 100644 --- a/tests/browser/forgeflow.spec.mjs +++ b/tests/browser/forgeflow.spec.mjs @@ -215,3 +215,82 @@ test("inventory, deployment safety and failure evidence dialogs are reviewable", } await assertSurface(page); }); + +// A repository or deployment poll renders the whole shell again. Changing an +// unrelated part of the state is what a poll effectively does, and it must not +// take the caret or the scroll position away from the user. +async function forceUnrelatedRerender(page) { + await page.evaluate(() => { + ui.diagnosticsStatus = { ...(ui.diagnosticsStatus || {}), enabled: !(ui.diagnosticsStatus?.enabled === false) }; + render(); + }); +} + +test("a background refresh keeps typing and caret position intact", async ({ page }) => { + const search = page.locator("#global-search"); + await search.click(); + await search.fill("Forge"); + // Typing schedules a debounced render. Wait for it, otherwise the caret below + // can land on the element that render is about to replace. + await expect.poll(() => page.evaluate(() => ui.inputRenderTimer === null)).toBe(true); + await search.evaluate((element) => element.setSelectionRange(1, 3)); + + await forceUnrelatedRerender(page); + + await expect(search).toBeFocused(); + expect(await search.inputValue()).toBe("Forge"); + expect(await search.evaluate((element) => [element.selectionStart, element.selectionEnd])).toEqual([1, 3]); +}); + +test("a background refresh keeps scroll offsets intact", async ({ page }) => { + await page.locator('.nav-button[data-action="navigate"][data-view="settings"]').click(); + const canvas = page.locator(".main-canvas"); + const scrolled = await canvas.evaluate((element) => { + element.scrollTop = Math.min(120, Math.max(0, element.scrollHeight - element.clientHeight)); + return element.scrollTop; + }); + expect(scrolled).toBeGreaterThan(0); + + await forceUnrelatedRerender(page); + + expect(await canvas.evaluate((element) => element.scrollTop)).toBe(scrolled); +}); + +test("sections that used to be injected after render are part of the rendered markup", async ({ page }) => { + await page.locator('.nav-button[data-action="navigate"][data-view="diagnostics"]').click(); + const auditPanel = page.locator(".diagnostics-page .section-block", { hasText: "Operational audit log" }); + await expect(auditPanel).toBeVisible(); + await expect(auditPanel).toContainText("Load the operational audit log"); + + // The audit rows are state the shell renders itself now, so a plain render has + // to pick them up without any post-render injection step. + await page.evaluate(() => { + ui.auditEvents = [{ timestamp: new Date().toISOString(), event: "deployment.requested", details: { repository: "Jens/Probe", result: "queued" } }]; + render(); + }); + await expect(auditPanel.locator("table.data-table")).toContainText("Jens/Probe"); + await expect(auditPanel.locator("table.data-table")).toContainText("deployment.requested"); +}); + +test("an unchanged render leaves the existing DOM in place", async ({ page }) => { + await page.locator('[data-action="select-repo"]').first().click(); + const marked = await page.evaluate(() => { + // Relative timestamps ("just now" turning into "1m ago") and pending async + // state legitimately change the markup between two renders that are seconds + // apart. Rendering twice inside one synchronous block removes that window, + // so the second render can only be skipped because nothing changed. + render(); + document.querySelector(".repo-list").dataset.renderProbe = "kept"; + render(); + return document.querySelector(".repo-list")?.dataset.renderProbe || null; + }); + expect(marked).toBe("kept"); + + const replaced = await page.evaluate(() => { + document.querySelector(".repo-list").dataset.renderProbe = "kept"; + ui.repoSearch = `probe-${Date.now()}`; + render(); + return document.querySelector(".repo-list")?.dataset.renderProbe || null; + }); + expect(replaced).toBe(null); +}); diff --git a/tests/dependency-wiring.test.mjs b/tests/dependency-wiring.test.mjs new file mode 100644 index 0000000..b26d5a7 --- /dev/null +++ b/tests/dependency-wiring.test.mjs @@ -0,0 +1,165 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { createRequire } from "node:module"; + +const require = createRequire(import.meta.url); +const { registerDeploymentIpc } = require("../src/main/ipc/deployment-handlers.cjs"); +const { UnraidDeploymentService } = require("../src/main/unraid-deployment-service.cjs"); + +const REPOSITORY = { fullName: "Jens/ForgeFlow", owner: { login: "Jens" }, localPath: "C:/Projects/ForgeFlow" }; +const WORKLOAD = { workloadId: "workload-1", classification: { type: "ambiguous" } }; + +// Every collaborator answers, so a channel can only fail on a dependency the +// module references but never receives. +function harness(overrides = {}) { + const calls = []; + const record = (name, result) => async (...args) => { calls.push({ name, args }); return typeof result === "function" ? result(...args) : result; }; + const handlers = new Map(); + const profile = overrides.profile || { id: "profile-1", provider: "ssh-unraid", branch: "main", name: "Production" }; + + const dependencies = { + register: (channel, handler) => handlers.set(channel, handler), + store: { + data: { servers: [{ id: "server-1", name: "Unraid" }], operations: [] }, + getDeploymentProfile: () => profile, + getPublicState: () => ({ ok: true }), + saveDeploymentProfile: record("store.saveDeploymentProfile", profile), + deleteDeploymentProfile: record("store.deleteDeploymentProfile", []), + addOperation: record("store.addOperation", null), + }, + resolveRepository: record("resolveRepository", REPOSITORY), + unraid: { + preflight: record("unraid.preflight", { ok: true }), + repairWriteAccess: record("unraid.repairWriteAccess", { changed: true, after: {}, before: {} }), + deploy: record("unraid.deploy", { id: "operation-1" }), + rollback: record("unraid.rollback", { id: "operation-2" }), + linkServerWorkload: record("unraid.linkServerWorkload", { linked: true }), + configureServerGitAccess: record("unraid.configureServerGitAccess", { keyFingerprint: "a", hostFingerprint: "b" }), + verifyServerGitProfile: record("unraid.verifyServerGitProfile", { readiness: "ready", ready: true, checkedAt: "now" }), + discoverServerWorkloads: record("unraid.discoverServerWorkloads", { serverId: "server-1", workloads: [] }), + planServerInventoryReconciliation: record("unraid.planServerInventoryReconciliation", { plan: { id: "plan-1", summary: {} } }), + reconcileServerInventory: record("unraid.reconcileServerInventory", { adopted: 0, refreshed: 0, retired: 0 }), + scanServerInventory: record("unraid.scanServerInventory", { workloads: [WORKLOAD] }), + refreshProfileState: record("unraid.refreshProfileState", { liveSha: null }), + applyDockerManMetadata: record("unraid.applyDockerManMetadata", { applied: true }), + refreshOperation: record("unraid.refreshOperation", null), + reconcileRecordedOperations: record("unraid.reconcileRecordedOperations", []), + }, + deployments: { + deploy: record("deployments.deploy", { id: "operation-3" }), + rollback: record("deployments.rollback", { id: "operation-4" }), + checkHealth: record("deployments.checkHealth", { healthy: true }), + refreshProfileState: record("deployments.refreshProfileState", { liveSha: null }), + }, + evaluateDeploymentPolicy: () => ({ note: "", overridden: false, reason: "", violations: [] }), + audit: { append: record("audit.append", null) }, + deployKeys: { + inventory: record("deployKeys.inventory", { keys: [] }), + planRotation: record("deployKeys.planRotation", { id: "rotation-1" }), + rotate: record("deployKeys.rotate", { rotated: true }), + planRevocation: record("deployKeys.planRevocation", { id: "revocation-1" }), + revoke: record("deployKeys.revoke", { revoked: true }), + restore: record("deployKeys.restore", { restored: true }), + }, + repositories: { refresh: record("repositories.refresh", [REPOSITORY]) }, + inventoryReviews: { + preview: (...args) => { calls.push({ name: "inventoryReviews.preview", args }); return { id: "review-1" }; }, + apply: record("inventoryReviews.apply", { applied: true }), + }, + diagnostics: { info: record("diagnostics.info"), warning: record("diagnostics.warning"), error: record("diagnostics.error"), debug: record("diagnostics.debug") }, + git: {}, + gitea: { getBranch: record("gitea.getBranch", { commit: { id: "c".repeat(40) } }) }, + ssh: {}, + preflight: { runDeployment: record("preflight.runDeployment", { ok: "actions" }) }, + ...overrides.dependencies, + }; + + registerDeploymentIpc(dependencies); + return { handlers, calls, names: () => calls.map((item) => item.name) }; +} + +const PAYLOAD = { + repository: REPOSITORY, + fullName: REPOSITORY.fullName, + profileId: "profile-1", + sha: "a".repeat(40), + serverId: "server-1", + workloadId: WORKLOAD.workloadId, + planId: "plan-1", + action: "manual-link", + url: "https://app.example/health", + profile: { name: "Production" }, + targetSha: "b".repeat(40), +}; + +// Both provider paths have to run: a dependency that only the Gitea Actions +// branch reads stays invisible while every channel is exercised as SSH/Unraid. +for (const provider of ["ssh-unraid", "gitea-actions"]) { + test(`every deployment IPC channel runs with the dependencies it is given (${provider})`, async () => { + const { handlers } = harness({ profile: { id: "profile-1", provider, branch: "main", name: "Production" } }); + assert.ok(handlers.size >= 20, "expected the complete deployment channel surface"); + + const failures = []; + for (const [channel, handler] of handlers) { + try { + await handler({ ...PAYLOAD }); + } catch (error) { + // A refusal is a decision the handler made; a missing dependency is not. + if (error instanceof ReferenceError || error instanceof TypeError) { + failures.push(`${channel}: ${error.name}: ${error.message}`); + } + } + } + assert.deepEqual(failures, []); + }); +} + +test("deployment preflight routes by provider", async () => { + const actions = harness({ profile: { id: "profile-1", provider: "gitea-actions" } }); + assert.deepEqual(await actions.handlers.get("deployment:preflight")({ ...PAYLOAD }), { ok: "actions" }); + assert.ok(actions.names().includes("preflight.runDeployment")); + + const unraid = harness(); + assert.deepEqual(await unraid.handlers.get("deployment:preflight")({ ...PAYLOAD }), { ok: true }); + assert.ok(unraid.names().includes("unraid.preflight")); + assert.ok(!unraid.names().includes("preflight.runDeployment")); +}); + +test("write-access repair is refused for anything but an SSH/Unraid profile", async () => { + const actions = harness({ profile: { id: "profile-1", provider: "gitea-actions" } }); + await assert.rejects( + () => actions.handlers.get("deployment:repair-write-access")({ ...PAYLOAD }), + /available only for SSH \/ Unraid/, + ); +}); + +test("a stale workload blocks an inventory review instead of guessing", async () => { + const { handlers } = harness({ + dependencies: { unraid: { scanServerInventory: async () => ({ workloads: [] }) } }, + }); + for (const channel of ["deployment:plan-inventory-review", "deployment:apply-inventory-review"]) { + await assert.rejects(() => handlers.get(channel)({ ...PAYLOAD }), (error) => { + assert.equal(error.code, "INVENTORY_REVIEW_WORKLOAD_STALE"); + return true; + }); + } +}); + +test("write-access repair builds a repair script that preserves runtime paths", () => { + const service = new UnraidDeploymentService({}); + const profile = { + id: "profile-3", + provider: "ssh-unraid", + remoteFolder: "portfolio", + composeFiles: ["docker-compose.yml"], + preservePaths: ["data/uploads"], + }; + const server = { id: "server-1", basePath: "/mnt/user/appdata" }; + + const script = service.permissionRepairScript(profile, server, "/mnt/user/appdata/portfolio"); + + assert.equal(typeof script, "string"); + assert.match(script, /data\/uploads/); + assert.match(script, /node_modules/); + assert.match(script, /ForgeFlow repaired project write access/); +}); diff --git a/tests/repository-monitor.test.mjs b/tests/repository-monitor.test.mjs index 5367f18..8d89ffb 100644 --- a/tests/repository-monitor.test.mjs +++ b/tests/repository-monitor.test.mjs @@ -49,3 +49,56 @@ test('repository monitor checks multiple repositories concurrently with a bounde assert.equal(active, 0); assert.equal(monitor.fingerprints.size, 10); }); + +test('a watched repository is read on filesystem activity instead of on every interval', async (context) => { + const { mkdtemp, mkdir, writeFile, rm } = await import('node:fs/promises'); + const os = await import('node:os'); + const path = await import('node:path'); + const root = await mkdtemp(path.join(os.tmpdir(), 'forgeflow-watch-')); + context.after(() => rm(root, { recursive: true, force: true })); + await mkdir(path.join(root, '.git'), { recursive: true }); + + let revision = 1; + const reads = []; + const changes = []; + const git = { + status: async (localPath) => { reads.push(localPath); return { localPath, revision }; }, + statusFingerprint: (status) => String(status.revision) + }; + const store = { data: { preferences: { autoRefresh: true, repositoryPollSeconds: 2 } } }; + const monitor = new RepositoryMonitor({ store, git, onChange: (change) => changes.push(change) }); + context.after(() => monitor.stop()); + + monitor.restart(); + monitor.setPaths([root]); + if (!monitor.watchers.has(root)) { + context.skip('this platform does not support recursive directory watching'); + return; + } + + await monitor.tick(); + assert.equal(reads.length, 1, 'the baseline is established once'); + + // Without filesystem activity the interval must not spawn another read. + await monitor.tick(); + assert.equal(reads.length, 1); + + revision = 2; + await writeFile(path.join(root, 'feature.txt'), 'changed\n'); + // The watcher debounce and the per-repository cooldown both apply here. + const deadline = Date.now() + 5_000; + while (changes.length === 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 100)); + } + + assert.ok(reads.length > 1, 'filesystem activity triggers a read'); + assert.equal(changes.length, 1); + assert.equal(changes[0].reason, 'working-tree-changed'); + + const readsAfterChange = reads.length; + await new Promise((resolve) => setTimeout(resolve, 800)); + assert.equal(reads.length, readsAfterChange, 'a quiet repository is not read again'); + + monitor.stop(); + assert.equal(monitor.watchers.size, 0, 'stopping releases every watcher'); +}); diff --git a/tests/repository-service.test.mjs b/tests/repository-service.test.mjs index 658a96b..5e7622a 100644 --- a/tests/repository-service.test.mjs +++ b/tests/repository-service.test.mjs @@ -80,6 +80,24 @@ test('repository discovery is bounded, skips generated trees and ignores inacces assert.ok(all.includes(found[0])); }); +test('a repository reached through a directory junction is discovered once', async (context) => { + const root = await mkdtemp(path.join(os.tmpdir(), 'forgeflow-junction-')); + context.after(() => import('node:fs/promises').then(({ rm }) => rm(root, { recursive: true, force: true }))); + const elsewhere = path.join(root, 'elsewhere', 'service'); + await mkdir(path.join(elsewhere, '.git'), { recursive: true }); + await mkdir(path.join(root, 'workspace'), { recursive: true }); + try { + await symlink(elsewhere, path.join(root, 'workspace', 'linked-service'), 'junction'); + } catch { + context.skip('this platform does not allow creating directory links'); + return; + } + + const { realpath } = await import('node:fs/promises'); + const found = await service().discoverInRoot(path.join(root, 'workspace'), 3); + assert.deepEqual(found, [await realpath(elsewhere)]); +}); + test('local descriptors preserve Git failures and watch paths are defensive copies', async () => { const instance = new RepositoryService({ data: {} }, { status: async (localPath) => { @@ -133,6 +151,59 @@ test('refresh links explicit and remote-matched repositories and retains unmatch assert.equal(diagnostics[0][0], 'repositories.refresh.completed'); }); +test('resolving one repository reads only that repository, not the whole workspace', async () => { + const scanned = []; + const store = { + data: { + gitea: { baseUrl: 'https://gitea.example' }, + workspaceRoots: ['root'], + repositoryMappings: { 'jens/portfolio': 'C:/explicit' }, + preferences: { preferredCloneProtocol: 'https' }, + favorites: [] + }, + getToken: () => 'token', + getDeploymentProfiles: () => [{ id: 'prod', branch: 'main' }], + getDeploymentState: () => ({ liveSha: null, healthy: null }) + }; + const instance = new RepositoryService(store, { + status: async (localPath) => { + scanned.push(localPath); + return { ...status(), root: localPath, remoteUrl: remote.clone_url }; + } + }, { listRepositories: async () => [remote, { ...remote, id: 2, full_name: 'Jens/Other', name: 'Other' }] }); + instance.discoverAll = async () => ['C:/explicit', 'C:/other', 'C:/third']; + + await instance.refresh(); + const duringRefresh = scanned.length; + assert.equal(duringRefresh, 3); + + scanned.length = 0; + const resolved = await instance.resolveByFullName(remote.full_name); + assert.equal(resolved.fullName, remote.full_name); + assert.equal(resolved.localPath, 'C:/explicit'); + assert.equal(resolved.deploymentProfiles[0].id, 'prod'); + assert.deepEqual(scanned, ['C:/explicit']); + + assert.equal(await instance.resolveByFullName(''), null); +}); + +test('resolving an unknown repository still falls back to a full refresh', async () => { + const store = { + data: { gitea: { baseUrl: 'https://gitea.example' }, workspaceRoots: [], repositoryMappings: {}, preferences: { preferredCloneProtocol: 'https' }, favorites: [] }, + getToken: () => 'token', + getDeploymentProfiles: () => [], + getDeploymentState: () => null + }; + const instance = new RepositoryService(store, { + status: async (localPath) => ({ ...status(), root: localPath, remoteUrl: '' }) + }, { listRepositories: async () => [remote] }); + instance.discoverAll = async () => ['C:/loose-checkout']; + + const local = await instance.resolveByFullName('loose-checkout'); + assert.equal(local.linkState, 'unmatched-local'); + assert.equal(await instance.resolveByFullName('Jens/Missing'), null); +}); + test('refresh remains local-only without configured Gitea credentials', async () => { const store = { data: { gitea: { baseUrl: '' }, workspaceRoots: [], repositoryMappings: {}, preferences: { preferredCloneProtocol: 'ssh' }, favorites: [] },