From 6db0bead44ab931b1edb69458b2e1c3489341048 Mon Sep 17 00:00:00 2001 From: code-infra-01 Date: Thu, 20 Aug 2026 20:20:33 +0000 Subject: [PATCH] fix(#1327): sentinel-managed PATH block, default-home-only profile writes (#1330) Co-authored-by: code-infra-01 --- .../mosaic/src/stages/finalize-path.spec.ts | 152 ++++++++++++++++++ packages/mosaic/src/stages/finalize.ts | 88 ++++++++-- 2 files changed, 224 insertions(+), 16 deletions(-) create mode 100644 packages/mosaic/src/stages/finalize-path.spec.ts diff --git a/packages/mosaic/src/stages/finalize-path.spec.ts b/packages/mosaic/src/stages/finalize-path.spec.ts new file mode 100644 index 00000000..cc71028f --- /dev/null +++ b/packages/mosaic/src/stages/finalize-path.spec.ts @@ -0,0 +1,152 @@ +/** + * setupPath profile management (issue #1327, MOSAIC-IMPROVEMENTS 4c / D25). + * + * The profile append used to be guarded on the binDir value it was about to + * write, which is blind to accumulation across different Mosaic homes: every + * wizard run against a fresh temp home appended a permanent block to the + * operator's real shell profile (1,061 measured appends on sb-it-1-dt). + * + * Arms below map to the requirements: + * S1 sentinel-managed block, rewritten in place + * S2 a non-default target home never touches the operator profile + * S3 byte-identical profile across repeated runs + * S4 legacy unmarked `# Mosaic` blocks collapse into the managed block + * S5 the Windows ($env:Path) arm shares the same block logic + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir, homedir } from 'node:os'; + +let profilePathMock: string | null = null; + +vi.mock('../platform/detect.js', () => ({ + getShellProfilePath: (): string | null => profilePathMock, +})); + +import { setupPath, managedBlockFor, stripLegacyPathBlocks } from './finalize.js'; + +// The real resolved default on this host. Tests use it as the comparator a +// non-default home must fail against, exactly as the wizard would. +const REAL_DEFAULT_HOME = join(homedir(), '.config', 'mosaic'); + +function tempHome(prefix: string): string { + const dir = join(tmpdir(), prefix); + mkdirSync(join(dir, 'bin'), { recursive: true }); + return dir; +} + +describe('setupPath profile management (#1327)', () => { + let workDir: string; + let profileFile: string; + let defaultLikeHome: string; + let otherHome: string; + const baseline = '# existing operator content\nexport EDITOR=vim\n'; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), 'setuppath-spec-')); + profileFile = join(workDir, '.bashrc'); + writeFileSync(profileFile, baseline, 'utf-8'); + profilePathMock = profileFile; + defaultLikeHome = tempHome(join(workDir, 'home-a', '.config', 'mosaic')); + otherHome = tempHome(join(workDir, 'home-b', '.config', 'mosaic')); + }); + + afterEach(() => { + profilePathMock = null; + rmSync(workDir, { recursive: true, force: true }); + }); + + // S2 — the arm that MUST fail against the pre-fix code: a home that is not + // the resolved default may not modify the operator profile at all. + it('does not touch the operator profile when the target home is not the resolved default', () => { + const action = setupPath(otherHome, REAL_DEFAULT_HOME); + expect(action).toBe('skipped'); + expect(readFileSync(profileFile, 'utf-8')).toBe(baseline); + }); + + it('returns skipped when no shell profile can be resolved', () => { + profilePathMock = null; + const action = setupPath(defaultLikeHome, defaultLikeHome); + expect(action).toBe('skipped'); + }); + + // S1 + S3 — two distinct homes (each run as the resolved default in turn, + // the shape of two legitimate installs against one operator profile) and + // repeated runs against the same home both leave exactly one block. + it('leaves exactly one managed block after runs against two distinct homes', () => { + const first = setupPath(defaultLikeHome, defaultLikeHome); + expect(first).toBe('added'); + + const second = setupPath(otherHome, otherHome); + expect(second).toBe('added'); + + const content = readFileSync(profileFile, 'utf-8'); + const beginCount = content.split('# >>> mosaic begin >>>').length - 1; + const endCount = content.split('# <<< mosaic end <<<').length - 1; + expect(beginCount).toBe(1); + expect(endCount).toBe(1); + expect(content).toContain(join(otherHome, 'bin')); + expect(content).toContain(baseline); + }); + + it('is byte-identical across repeated runs against the same home', () => { + setupPath(defaultLikeHome, defaultLikeHome); + const afterFirst = readFileSync(profileFile, 'utf-8'); + + const again = setupPath(defaultLikeHome, defaultLikeHome); + expect(again).toBe('already'); + expect(readFileSync(profileFile, 'utf-8')).toBe(afterFirst); + }); + + // S4 — pre-existing unmarked blocks from the old append logic collapse + // into the single managed block instead of accumulating beside it. + it('collapses legacy unmarked # Mosaic blocks into the managed block', () => { + const legacy = + '# existing operator content\n' + + '# Mosaic\n' + + 'export PATH="/tmp/mosaic-dead-wizard-1/bin:$PATH"\n' + + 'export EDITOR=vim\n' + + '# Mosaic\n' + + 'export PATH="/tmp/mosaic-dead-wizard-2/bin:$PATH"\n'; + writeFileSync(profileFile, legacy, 'utf-8'); + + const action = setupPath(defaultLikeHome, defaultLikeHome); + expect(action).toBe('added'); + + const content = readFileSync(profileFile, 'utf-8'); + expect(content).not.toContain('/tmp/mosaic-dead-wizard-1/bin'); + expect(content).not.toContain('/tmp/mosaic-dead-wizard-2/bin'); + expect(content).toContain('export EDITOR=vim'); + expect(content.split('# >>> mosaic begin >>>').length - 1).toBe(1); + expect(content).toContain(join(defaultLikeHome, 'bin')); + }); +}); + +describe('managed block helpers (#1327)', () => { + // S5 — the Windows arm shares markers and shape with the POSIX arm. + it('builds the $env:Path variant inside the same markers', () => { + const block = managedBlockFor('C:\\Users\\op\\.config\\mosaic\\bin', true); + expect(block).toContain('# >>> mosaic begin >>>'); + expect(block).toContain('# <<< mosaic end <<<'); + expect(block).toContain('$env:Path = "C:\\Users\\op\\.config\\mosaic\\bin;$env:Path"'); + }); + + it('builds the POSIX export variant inside the same markers', () => { + const block = managedBlockFor('/home/op/.config/mosaic/bin', false); + expect(block).toContain('# >>> mosaic begin >>>'); + expect(block).toContain('export PATH="/home/op/.config/mosaic/bin:$PATH"'); + expect(block).toContain('# <<< mosaic end <<<'); + }); + + it('strips legacy $env:Path pairs on the Windows arm', () => { + const legacy = + '# Mosaic\n$env:Path = "C:\\tmp\\dead\\bin;$env:Path"\n' + + '# Mosaic\n$env:Path = "C:\\tmp\\dead2\\bin;$env:Path"\n' + + 'Write-Host hi\n'; + const stripped = stripLegacyPathBlocks(legacy, true); + expect(stripped).not.toContain('C:\\tmp\\dead'); + expect(stripped).toContain('Write-Host hi'); + }); +}); diff --git a/packages/mosaic/src/stages/finalize.ts b/packages/mosaic/src/stages/finalize.ts index a24fe545..5f564da3 100644 --- a/packages/mosaic/src/stages/finalize.ts +++ b/packages/mosaic/src/stages/finalize.ts @@ -1,11 +1,12 @@ import { spawnSync } from 'node:child_process'; -import { existsSync, readFileSync, appendFileSync } from 'node:fs'; +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { platform } from 'node:os'; import type { WizardPrompter } from '../prompter/interface.js'; import type { ConfigService } from '../config/config-service.js'; import type { WizardState } from '../types.js'; import { getShellProfilePath } from '../platform/detect.js'; +import { DEFAULT_MOSAIC_HOME } from '../constants.js'; import { ManifestError } from '../framework/manifest.js'; import { getDefaultSkillPaths, @@ -144,32 +145,87 @@ function runDoctor(mosaicHome: string): DoctorResult { type PathAction = 'already' | 'added' | 'skipped'; -function setupPath(mosaicHome: string, _p: WizardPrompter): PathAction { - const binDir = join(mosaicHome, 'bin'); - const currentPath = process.env['PATH'] ?? ''; +const PATH_BLOCK_BEGIN = '# >>> mosaic begin >>>'; +const PATH_BLOCK_END = '# <<< mosaic end <<<'; +const PATH_BLOCK_NOTE = '# Managed by the Mosaic installer; this block is rewritten on install.'; - if (currentPath.includes(binDir)) { - return 'already'; +/** + * The managed PATH block written into the operator's shell profile. + * + * The block is delimited by begin/end sentinels so any number of installs, + * against any homes, collapse to exactly one block: the writer replaces the + * region between the sentinels instead of appending a second copy (#1327). + */ +export function managedBlockFor(binDir: string, isWindows: boolean): string { + const exportLine = isWindows + ? `$env:Path = "${binDir};$env:Path"` + : `export PATH="${binDir}:$PATH"`; + return `${PATH_BLOCK_BEGIN}\n${PATH_BLOCK_NOTE}\n${exportLine}\n${PATH_BLOCK_END}\n`; +} + +/** + * Remove legacy unmarked `# Mosaic` PATH pairs appended by pre-#1327 + * installs. Only the exact two-line shape this installer used to write is + * removed; any other `# Mosaic` comment line is left alone. + */ +export function stripLegacyPathBlocks(content: string, isWindows: boolean): string { + const legacyExport = isWindows ? /^\$env:Path = ".*;\$env:Path"$/ : /^export PATH=".*:\$PATH"$/; + const lines = content.split('\n'); + const kept: string[] = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i] ?? ''; + const next = i + 1 < lines.length ? lines[i + 1] : undefined; + if (line === '# Mosaic' && next !== undefined && legacyExport.test(next)) { + i += 1; + continue; + } + kept.push(line); + } + return kept.join('\n'); +} + +/** Drop the region between the managed-block sentinels, first occurrence. */ +function withoutManagedBlock(content: string): string { + const beginIdx = content.indexOf(PATH_BLOCK_BEGIN); + if (beginIdx < 0) return content; + const endIdx = content.indexOf(PATH_BLOCK_END, beginIdx); + if (endIdx < 0) return content; + return content.slice(0, beginIdx) + content.slice(endIdx + PATH_BLOCK_END.length); +} + +export function setupPath(mosaicHome: string, resolvedDefaultHome: string): PathAction { + // Never write outside the home under test (#1327 S2): a wizard run against + // a non-default home (test harnesses, throwaway installs) must not mutate + // the operator's real shell profile. + if (mosaicHome !== resolvedDefaultHome) { + return 'skipped'; } + const binDir = join(mosaicHome, 'bin'); const profilePath = getShellProfilePath(); if (!profilePath) return 'skipped'; const isWindows = platform() === 'win32'; - const exportLine = isWindows - ? `\n# Mosaic\n$env:Path = "${binDir};$env:Path"\n` - : `\n# Mosaic\nexport PATH="${binDir}:$PATH"\n`; + const block = managedBlockFor(binDir, isWindows); - // Check if already in profile + let content = ''; if (existsSync(profilePath)) { - const content = readFileSync(profilePath, 'utf-8'); - if (content.includes(binDir)) { - return 'already'; - } + content = readFileSync(profilePath, 'utf-8'); + } + + // Migration (#1327 S4): legacy unmarked blocks collapse into the managed + // block, and an existing managed block is rewritten in place rather than + // appended beside itself (S1/S3). + const base = stripLegacyPathBlocks(withoutManagedBlock(content), isWindows); + const trimmed = base.replace(/\n+$/, ''); + const next = trimmed.length === 0 ? block : `${trimmed}\n${block}`; + + if (next === content) { + return 'already'; } try { - appendFileSync(profilePath, exportLine, 'utf-8'); + writeFileSync(profilePath, next, 'utf-8'); return 'added'; } catch { return 'skipped'; @@ -286,7 +342,7 @@ export async function finalizeStage( } // 7. PATH setup - const pathAction = setupPath(state.mosaicHome, p); + const pathAction = setupPath(state.mosaicHome, DEFAULT_MOSAIC_HOME); let summaryShown = false; const showSummary = () => {