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 <noreply@anthropic.com>
This commit is contained in:
NuklearRabbit
co-authored by
Claude Opus 5
Claude Opus 5
parent
cf1da8a2fa
commit
9260d35957
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
@@ -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: [] },
|
||||
|
||||
Reference in New Issue
Block a user