feat(FN-4018): isolate split merger temp workspaces with settings persisten

Merged FN-4014 to fix dual-scope GitHub tracking settings persistence and validation (mission vs task scope), scoped repo saves to the relevant section, and documented the dual defaults. Also landed FN-4018 to isolate split merger temp workspaces and add regression checks for split-suite test isolat

Fusion-Task-Id: FN-4018
This commit is contained in:
Fusion
2026-05-11 11:36:39 -07:00
committed by gsxdsm
parent c41d49f28b
commit 98aa09f822
16 changed files with 160 additions and 25 deletions

View File

@@ -4,6 +4,7 @@ import {
DEFAULT_PROJECT_SETTINGS,
GLOBAL_SETTINGS_KEYS,
PROJECT_SETTINGS_KEYS,
isGlobalOnlySettingsKey,
isGlobalSettingsKey,
isProjectSettingsKey,
} from "../types.js";
@@ -95,6 +96,8 @@ describe("settings key parity", () => {
expect(isGlobalSettingsKey("githubAuthToken")).toBe(false);
expect(isProjectSettingsKey("githubTrackingDefaultRepo")).toBe(true);
expect(isGlobalSettingsKey("githubTrackingDefaultRepo")).toBe(true);
expect(isGlobalOnlySettingsKey("githubTrackingDefaultRepo")).toBe(false);
expect(isGlobalOnlySettingsKey("themeMode")).toBe(true);
});
it("keeps remoteAccess scoped to global settings only", () => {

View File

@@ -554,6 +554,33 @@ describe("TaskStore", () => {
// ── Scope Separation Regression Tests (FN-1729) ───────────────────────────
describe("scope separation regression", () => {
it("keeps dual-scope githubTrackingDefaultRepo in project scope while excluding global-only keys", async () => {
await harness.store().updateGlobalSettings({
githubTrackingDefaultRepo: "global/default",
themeMode: "light",
});
await harness.store().updateSettings({
githubTrackingDefaultRepo: "project/default",
themeMode: "dark",
});
const merged = await harness.store().getSettings();
const mergedFast = await harness.store().getSettingsFast();
const { global, project } = await harness.store().getSettingsByScope();
const scopedFast = await harness.store().getSettingsByScopeFast();
expect(merged.githubTrackingDefaultRepo).toBe("project/default");
expect(mergedFast.githubTrackingDefaultRepo).toBe("project/default");
expect(project.githubTrackingDefaultRepo).toBe("project/default");
expect(scopedFast.project.githubTrackingDefaultRepo).toBe("project/default");
expect(global.githubTrackingDefaultRepo).toBe("global/default");
expect(scopedFast.global.githubTrackingDefaultRepo).toBe("global/default");
expect((project as Record<string, unknown>).themeMode).toBeUndefined();
expect((scopedFast.project as Record<string, unknown>).themeMode).toBeUndefined();
});
it("getSettingsByScope: global lane keys never appear in project scope", async () => {
await harness.store().updateGlobalSettings({
executionGlobalProvider: "anthropic",

View File

@@ -346,3 +346,7 @@ export function isGlobalSettingsKey(key: string): key is keyof GlobalSettings {
export function isProjectSettingsKey(key: string): key is keyof ProjectSettings {
return (PROJECT_SETTINGS_KEYS as readonly string[]).includes(key);
}
export function isGlobalOnlySettingsKey(key: string): key is keyof GlobalSettings {
return isGlobalSettingsKey(key) && !isProjectSettingsKey(key);
}

View File

@@ -5,7 +5,7 @@ import { join } from "node:path";
import { existsSync, watch, type FSWatcher } from "node:fs";
import type { Task, TaskDetail, TaskCreateInput, TaskAttachment, AgentLogEntry, BoardConfig, Column, MergeResult, Settings, GlobalSettings, ProjectSettings, ActivityLogEntry, ActivityEventType, TaskDocument, TaskDocumentRevision, TaskDocumentCreateInput, TaskDocumentWithTask, InboxTask, TaskLogEntry, RunMutationContext, RunAuditEvent, RunAuditEventInput, RunAuditEventFilter, ArchivedTaskEntry, ArchiveAgentLogMode, TaskPriority, SourceType, WorkflowStepTemplate, Agent, AutostashOrphanRecord, TaskCommitAssociation, TaskCommitAssociationMatchSource, TaskCommitAssociationConfidence } from "./types.js";
import { createActivityLogSnapshot, createRunAuditSnapshot, createTaskMetadataSnapshot, toTaskMetadataRecord, validateSnapshotEnvelope, type ActivityLogSnapshot, type RunAuditSnapshot, type TaskMetadataSnapshot } from "./shared-mesh-state.js";
import { VALID_TRANSITIONS, DEFAULT_SETTINGS, isGlobalSettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js";
import { VALID_TRANSITIONS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js";
import { normalizeTaskPriority } from "./task-priority.js";
import { canAgentTakeImplementationTask } from "./agent-role-policy.js";
import { GlobalSettingsStore } from "./global-settings.js";
@@ -1783,7 +1783,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
// Strip global-only keys from project-level settings so stale project-scoped
// values don't override the correct global value during the spread merge.
const projectSettings = Object.fromEntries(
Object.entries(config.settings ?? {}).filter(([key]) => !isGlobalSettingsKey(key)),
Object.entries(config.settings ?? {}).filter(([key]) => !isGlobalOnlySettingsKey(key)),
);
return canonicalizeSettings({
...DEFAULT_SETTINGS,
@@ -1818,7 +1818,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
// always done this; getSettingsFast() was missing the filter.
const projectSettings: Partial<Settings> | undefined = raw
? (Object.fromEntries(
Object.entries(raw).filter(([key]) => !isGlobalSettingsKey(key)),
Object.entries(raw).filter(([key]) => !isGlobalOnlySettingsKey(key)),
) as Partial<Settings>)
: undefined;
@@ -1846,7 +1846,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
const projectSettings: Partial<ProjectSettings> = {};
if (config.settings) {
for (const key of Object.keys(config.settings)) {
if (!isGlobalSettingsKey(key)) {
if (!isGlobalOnlySettingsKey(key)) {
(projectSettings as Record<string, unknown>)[key] = (config.settings as Record<string, unknown>)[key];
}
}
@@ -1881,7 +1881,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
const projectScoped: Partial<ProjectSettings> = {};
if (projectSettings) {
for (const key of Object.keys(projectSettings)) {
if (!isGlobalSettingsKey(key)) {
if (!isGlobalOnlySettingsKey(key)) {
(projectScoped as Record<string, unknown>)[key] = (projectSettings as Record<string, unknown>)[key];
}
}
@@ -1904,7 +1904,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
// Filter out global-only fields — they should go through updateGlobalSettings()
const projectPatch: Partial<Settings> = {};
for (const [key, value] of Object.entries(patch)) {
if (!isGlobalSettingsKey(key)) {
if (!isGlobalOnlySettingsKey(key)) {
(projectPatch as Record<string, unknown>)[key] = value;
}
}

View File

@@ -2469,6 +2469,7 @@ export {
DEFAULT_SETTINGS,
GLOBAL_SETTINGS_KEYS,
PROJECT_SETTINGS_KEYS,
isGlobalOnlySettingsKey,
isGlobalSettingsKey,
isProjectSettingsKey,
} from "./settings-schema.js";

View File

@@ -1619,6 +1619,9 @@ export function SettingsModal({
const globalPatch: Partial<GlobalSettings> = {};
for (const [key, value] of Object.entries(payload)) {
if (key === "githubTrackingDefaultRepo" && activeSection !== "global-general") {
continue;
}
if (isGlobalSettingsKey(key)) {
// Implement null-as-delete semantics for global settings:
// - undefined values are dropped during JSON serialization
@@ -1639,6 +1642,7 @@ export function SettingsModal({
const projectPatch: Partial<Settings> = {};
for (const [key, value] of Object.entries(payload)) {
if (key === "githubTokenConfigured" || key === "prAuthAvailable") continue; // server-only fields
if (key === "githubTrackingDefaultRepo" && activeSection === "global-general") continue;
if (!isProjectSettingsKey(key)) continue;
// Get the initial project-scoped value (null if not set)
@@ -1689,7 +1693,7 @@ export function SettingsModal({
} catch (err) {
addToast(getErrorMessage(err), "error");
}
}, [form, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, onClose, addToast, projectId]);
}, [form, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, onClose, addToast, projectId, activeSection]);
const handleSaveMemory = useCallback(async () => {
try {

View File

@@ -540,6 +540,26 @@ describe("SettingsModal", () => {
expect(input.value).toBe("");
expect(screen.getByText(/Projects inherit this value when they do not set a project default tracking repo/i)).toBeInTheDocument();
});
it("saves global default tracking repo via global settings payload only", async () => {
renderModal({ initialSection: "global-general" });
await waitForSettingsModalReady();
await userEvent.type(screen.getByLabelText("Global default tracking repo"), "octo/global-default");
await userEvent.click(screen.getByRole("button", { name: "Save" }));
await waitFor(() => {
expect(mockUpdateGlobalSettings).toHaveBeenCalled();
});
const globalPayload = mockUpdateGlobalSettings.mock.calls[0]?.[0] as Record<string, unknown>;
expect(globalPayload.githubTrackingDefaultRepo).toBe("octo/global-default");
if (mockUpdateSettings.mock.calls.length > 0) {
const projectPayload = mockUpdateSettings.mock.calls[0]?.[0] as Record<string, unknown>;
expect(projectPayload.githubTrackingDefaultRepo).toBeUndefined();
}
});
});
describe("Project General", () => {
@@ -1984,6 +2004,11 @@ describe("SettingsModal", () => {
expect(payload.githubTrackingDefaultRepo).toBe("octo/repo");
expect(payload.githubAuthMode).toBe("token");
expect(payload.githubAuthToken).toBe("ghp_test_token");
if (mockUpdateGlobalSettings.mock.calls.length > 0) {
const globalPayload = mockUpdateGlobalSettings.mock.calls[0]?.[0] as Record<string, unknown>;
expect(globalPayload.githubTrackingDefaultRepo).toBeUndefined();
}
});
});

View File

@@ -687,6 +687,22 @@ describe("PUT /settings", () => {
expect(res.body.error).toContain("must include both provider and modelId or neither");
});
it("accepts dual-scope githubTrackingDefaultRepo on project settings endpoint", async () => {
const updatedSettings = { ...DEFAULT_SETTINGS, githubTrackingDefaultRepo: "octo/project-default" };
(store.updateSettings as ReturnType<typeof vi.fn>).mockResolvedValue(updatedSettings);
const res = await REQUEST(
buildApp(),
"PUT",
"/api/settings",
JSON.stringify({ githubTrackingDefaultRepo: "octo/project-default" }),
{ "Content-Type": "application/json" },
);
expect(res.status).toBe(200);
expect(store.updateSettings).toHaveBeenCalledWith({ githubTrackingDefaultRepo: "octo/project-default" });
});
it("rejects global-only fields with 400 error and helpful message", async () => {
const res = await REQUEST(
buildApp(),

View File

@@ -1,6 +1,7 @@
import {
DEFAULT_GLOBAL_SETTINGS,
GLOBAL_SETTINGS_KEYS,
PROJECT_SETTINGS_KEYS,
QMD_INSTALL_COMMAND,
MemoryBackendError,
buildInsightExtractionPrompt,
@@ -415,7 +416,8 @@ export function registerSettingsMemoryRoutes(ctx: ApiRoutesContext, deps: Settin
// Reject global-only fields with a helpful error pointing to the correct endpoint
const globalKeySet = new Set<string>(GLOBAL_SETTINGS_KEYS);
const globalFieldsFound = Object.keys(clientSettings).filter((k) => globalKeySet.has(k));
const projectKeySet = new Set<string>(PROJECT_SETTINGS_KEYS);
const globalFieldsFound = Object.keys(clientSettings).filter((k) => globalKeySet.has(k) && !projectKeySet.has(k));
if (globalFieldsFound.length > 0) {
throw badRequest(`Cannot update global settings via this endpoint. Use PUT /settings/global instead. Global fields found: ${globalFieldsFound.join(", ")}`);
}

View File

@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { commitOrAmendMergeWithFixes } from "../merger.js";
@@ -20,6 +20,12 @@ function initRepo(dir: string): void {
git(dir, 'git commit -m "chore: initial"');
}
function assertIsolatedWorkspace(dir: string): void {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
if (!repoRoot) return;
expect(resolve(dir).startsWith(resolve(repoRoot))).toBe(false);
}
function runFinalize(dir: string, taskId: string, branch: string, preAttemptHeadSha: string) {
return commitOrAmendMergeWithFixes(
dir,
@@ -45,7 +51,8 @@ describe("commitOrAmendMergeWithFixes ancestor/equivalent-content short-circuit"
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-ancestor-shortcircuit-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-ancestor-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});

View File

@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import type { TaskStore } from "@fusion/core";
@@ -44,6 +44,12 @@ function stashList(dir: string): string {
return git(dir, 'git stash list --format="%H %gd %s"');
}
function assertIsolatedWorkspace(dir: string): void {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
if (!repoRoot) return;
expect(resolve(dir).startsWith(resolve(repoRoot))).toBe(false);
}
function makeStore(tasks: Record<string, string>, opts?: { throwOnGetTask?: boolean }): TaskStore {
return {
getTask: async (taskId: string) => {
@@ -69,7 +75,8 @@ describe("sweepStaleAutostashes", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-autostash-stale-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-autostash-stale-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});
@@ -145,7 +152,8 @@ describe("sweepAutostashOrphans", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-autostash-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-autostash-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});

View File

@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { __test__ } from "../merger.js";
@@ -38,11 +38,18 @@ function createAutostash(dir: string, label: string, content: string): string {
return sha;
}
function assertIsolatedWorkspace(dir: string): void {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
if (!repoRoot) return;
expect(resolve(dir).startsWith(resolve(repoRoot))).toBe(false);
}
describe("autostash orphan surface", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-autostash-surface-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-autostash-surface-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});

View File

@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { mkdtempSync, rmSync, writeFileSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { commitOrAmendMergeWithFixes } from "../merger.js";
@@ -33,6 +33,12 @@ function stageSquashThenClear(dir: string, branch: string, file: string, content
return preAttemptSha;
}
function assertIsolatedWorkspace(dir: string): void {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
if (!repoRoot) return;
expect(resolve(dir).startsWith(resolve(repoRoot))).toBe(false);
}
const STUB_SETTINGS = {
...DEFAULT_SETTINGS,
commitAuthorEnabled: false,
@@ -42,7 +48,8 @@ describe("commitOrAmendMergeWithFixes no-op finalize", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-noop-finalize-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-noop-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});

View File

@@ -17,7 +17,7 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { snapshotDirtyFiles, commitOrAmendMergeWithFixes } from "../merger.js";
@@ -74,6 +74,12 @@ function squashBranch(dir: string, branchName: string, fileName: string, content
// Minimal stub settings / args used by commitOrAmendMergeWithFixes
// ---------------------------------------------------------------------------
function assertIsolatedWorkspace(dir: string): void {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
if (!repoRoot) return;
expect(resolve(dir).startsWith(resolve(repoRoot))).toBe(false);
}
const STUB_SETTINGS = {
...DEFAULT_SETTINGS,
commitAuthorEnabled: false, // skip --author flag to avoid user config issues
@@ -87,7 +93,8 @@ describe("snapshotDirtyFiles", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-snapshot-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-snapshot-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});
@@ -135,7 +142,8 @@ describe("snapshotDirtyFiles", () => {
});
it("returns empty set when rootDir is not a git repo (error swallowed)", async () => {
const nonRepo = mkdtempSync(join(tmpdir(), "fn-non-repo-"));
const nonRepo = mkdtempSync(join(tmpdir(), "fusion-test-merger-non-repo-"));
assertIsolatedWorkspace(nonRepo);
try {
const snapshot = await snapshotDirtyFiles(nonRepo);
expect(snapshot.size).toBe(0);
@@ -150,7 +158,8 @@ describe("commitOrAmendMergeWithFixes — staging allowlist", () => {
let warnSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-allowlist-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-allowlist-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
warnSpy = vi.spyOn(mergerLog, "warn");
});
@@ -480,7 +489,8 @@ describe("snapshotDirtyFiles — paths with embedded spaces", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-snapshot-spaces-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-snapshot-spaces-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
});
@@ -532,7 +542,8 @@ describe("commitOrAmendMergeWithFixes — embedded-space paths round-trip", () =
let warnSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-allowlist-spaces-"));
dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-allowlist-spaces-"));
assertIsolatedWorkspace(dir);
initRepo(dir);
warnSpy = vi.spyOn(mergerLog, "warn");
});

View File

@@ -1,6 +1,8 @@
import { describe, expect, it } from "vitest";
import { tmpdir } from "node:os";
import { getFusionAuthPath } from "../auth-storage.js";
import { mkdtempSync, rmSync } from "node:fs";
import { join, resolve } from "node:path";
describe("test isolation guard", () => {
it("overrides HOME to a temp fn-test-home directory", () => {
@@ -20,4 +22,15 @@ describe("test isolation guard", () => {
expect(authPath.startsWith(home!)).toBe(true);
expect(authPath).toContain(".fusion");
});
it("creates temp workspaces outside the real repo root", () => {
const workspace = mkdtempSync(join(tmpdir(), "fusion-test-guard-workspace-"));
try {
const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
expect(repoRoot).toBeDefined();
expect(resolve(workspace).startsWith(resolve(repoRoot!))).toBe(false);
} finally {
rmSync(workspace, { recursive: true, force: true });
}
});
});