Skip to content

Commit 2c52756

Browse files
authored
Merge pull request #123 from LennartvdM/codex/use-fs.createpath-for-mkdirp-safe-mirroring
mirror: use FS.createPath (mkdirp) and verify parents; surface ENOENT cleanly
2 parents c942516 + 8ca251c commit 2c52756

1 file changed

Lines changed: 149 additions & 29 deletions

File tree

web/workers/pyRunner.js

Lines changed: 149 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ let repoRootPromise = null;
340340
let repoRelocation = null;
341341
let repoRepairs = [];
342342
let lastRepoProbeResult = null;
343+
let repoCommonParentsPrepared = false;
343344

344345
function toErrorMessage(error) {
345346
if (!error) {
@@ -435,6 +436,107 @@ function safeAnalyzePath(instance, path) {
435436
return { exists: false, name: path };
436437
}
437438

439+
function createRecursivePath(instance, targetPath) {
440+
if (!instance?.FS?.createPath || !targetPath || typeof targetPath !== 'string') {
441+
return;
442+
}
443+
const isAbsolute = targetPath.startsWith('/');
444+
const base = isAbsolute ? '/' : '.';
445+
const relative = isAbsolute ? targetPath.slice(1) : targetPath;
446+
if (!relative && isAbsolute) {
447+
return;
448+
}
449+
instance.FS.createPath(base, relative, true, true);
450+
}
451+
452+
function getParentDirectory(targetPath) {
453+
if (typeof targetPath !== 'string') {
454+
return '';
455+
}
456+
const slashIndex = targetPath.lastIndexOf('/');
457+
if (slashIndex < 0) {
458+
return '';
459+
}
460+
if (slashIndex === 0) {
461+
return '/';
462+
}
463+
return targetPath.slice(0, slashIndex);
464+
}
465+
466+
function ensureParentDirectory(instance, destPath, relPath) {
467+
if (!instance?.FS || typeof destPath !== 'string') {
468+
return { parent: '', analysis: null };
469+
}
470+
const parent = getParentDirectory(destPath);
471+
if (parent && parent !== '/') {
472+
try {
473+
createRecursivePath(instance, parent);
474+
} catch (error) {
475+
throw createStageError('mirror', 'ParentDirCreateFailed', 'Failed to create parent directory', error, {
476+
parent,
477+
destPath,
478+
forPath: relPath,
479+
});
480+
}
481+
} else if (parent === '/') {
482+
// Root directory always exists in the virtual FS, nothing to create.
483+
}
484+
485+
let analysis = null;
486+
try {
487+
analysis = instance.FS.analyzePath(parent || '.');
488+
} catch (error) {
489+
analysis = safeAnalyzePath(instance, parent || '.');
490+
}
491+
const exists = analysis?.exists === true;
492+
const isFolder = analysis?.object?.isFolder === true;
493+
const normalizedAnalysis =
494+
analysis && typeof analysis === 'object' ? { ...analysis, exists: !!analysis.exists, isFolder } : null;
495+
if (!exists || !isFolder) {
496+
const parentError = {
497+
stage: 'mirror',
498+
type: 'ParentDirMissing',
499+
message: `Parent directory missing for ${relPath}`,
500+
parent,
501+
forPath: relPath,
502+
};
503+
if (normalizedAnalysis) {
504+
parentError.analysis = normalizedAnalysis;
505+
}
506+
throw parentError;
507+
}
508+
return { parent, analysis: normalizedAnalysis || analysis };
509+
}
510+
511+
function prepareRepoCommonParents(instance, rootPath) {
512+
if (repoCommonParentsPrepared || !instance?.FS?.createPath) {
513+
return;
514+
}
515+
516+
const targets = ['/repo/engines', '/repo/modules', '/repo/rigs'];
517+
try {
518+
for (const target of targets) {
519+
try {
520+
createRecursivePath(instance, target);
521+
} catch (error) {
522+
// If creation fails we still want to surface on verification during writes.
523+
}
524+
}
525+
if (rootPath && rootPath !== '/repo') {
526+
for (const segment of ['engines', 'modules', 'rigs']) {
527+
try {
528+
const relocated = joinRepoPath(rootPath, segment);
529+
createRecursivePath(instance, relocated);
530+
} catch (error) {
531+
// Ignore relocated creation errors; they'll be handled later if relevant.
532+
}
533+
}
534+
}
535+
} finally {
536+
repoCommonParentsPrepared = true;
537+
}
538+
}
539+
438540
function describeFsError(error) {
439541
if (!error || typeof error !== 'object') {
440542
return null;
@@ -582,9 +684,9 @@ async function ensureRepoRoot(instance) {
582684

583685
if (!repoStatus.exists) {
584686
try {
585-
instance.FS.mkdirTree(DEFAULT_REPO_ROOT);
687+
createRecursivePath(instance, DEFAULT_REPO_ROOT);
586688
} catch (error) {
587-
// if mkdirTree fails because the path exists as a file, we'll handle below via relocation
689+
// if createPath fails because the path exists as a file, we'll handle below via relocation
588690
}
589691
probe = await runRepoFsProbe(instance);
590692
lastRepoProbeResult = probe;
@@ -615,7 +717,7 @@ async function ensureRepoRoot(instance) {
615717
}
616718

617719
try {
618-
instance.FS.mkdirTree(repoRoot);
720+
createRecursivePath(instance, repoRoot);
619721
} catch (error) {
620722
throw createStageError('mirror', 'MirrorSetupFailed', 'Failed to prepare /repo directory', error, {
621723
manifestSize: lastManifestSize,
@@ -627,6 +729,8 @@ async function ensureRepoRoot(instance) {
627729
});
628730
}
629731

732+
prepareRepoCommonParents(instance, repoRoot);
733+
630734
if (!repoRelocation) {
631735
const nodes = (lastRepoProbeResult && lastRepoProbeResult.nodes) || {};
632736
for (const [name, entry] of Object.entries(nodes)) {
@@ -650,7 +754,7 @@ async function ensureRepoRoot(instance) {
650754
});
651755
}
652756
try {
653-
instance.FS.mkdirTree(targetPath);
757+
createRecursivePath(instance, targetPath);
654758
} catch (error) {
655759
const fsError = describeFsError(error);
656760
throw createStageError('mirror', 'MirrorSetupFailed', 'Failed to repair repository node directory', error, {
@@ -1319,6 +1423,8 @@ async function mirrorRepoFiles(instance) {
13191423
let sourceText = typeof cached?.text === 'string' ? cached.text : undefined;
13201424
let actualBytes = typeof cached?.bytes === 'number' ? cached.bytes : 0;
13211425

1426+
let lastWriteContext = null;
1427+
13221428
try {
13231429
if (!cached) {
13241430
const response = await fetch(url, { cache: 'no-store' });
@@ -1361,32 +1467,26 @@ async function mirrorRepoFiles(instance) {
13611467
throw new Error('ByteMismatch');
13621468
}
13631469

1364-
const directory = path.includes('/') ? path.slice(0, path.lastIndexOf('/')) : '';
1365-
if (directory) {
1366-
try {
1367-
instance.FS.mkdirTree(directory);
1368-
} catch (error) {
1369-
if (!/exists/i.test(String(error?.message || ''))) {
1370-
throw error;
1371-
}
1372-
}
1373-
}
1374-
1470+
const cacheParentPath = getParentDirectory(path);
1471+
lastWriteContext = {
1472+
destPath: path,
1473+
relPath: path,
1474+
parent: cacheParentPath || '',
1475+
analysis: null,
1476+
};
1477+
const cacheParentInfo = ensureParentDirectory(instance, path, path);
1478+
lastWriteContext.analysis = cacheParentInfo?.analysis || null;
13751479
instance.FS.writeFile(path, sourceText);
13761480

1377-
const repoDirectory = repoTargetPath.includes('/')
1378-
? repoTargetPath.slice(0, repoTargetPath.lastIndexOf('/'))
1379-
: '';
1380-
if (repoDirectory) {
1381-
try {
1382-
instance.FS.mkdirTree(repoDirectory);
1383-
} catch (error) {
1384-
if (!/exists/i.test(String(error?.message || ''))) {
1385-
throw error;
1386-
}
1387-
}
1388-
}
1389-
1481+
const repoParentPath = getParentDirectory(repoTargetPath);
1482+
lastWriteContext = {
1483+
destPath: repoTargetPath,
1484+
relPath: path,
1485+
parent: repoParentPath || '',
1486+
analysis: null,
1487+
};
1488+
const repoParentInfo = ensureParentDirectory(instance, repoTargetPath, path);
1489+
lastWriteContext.analysis = repoParentInfo?.analysis || null;
13901490
instance.FS.writeFile(repoTargetPath, sourceText);
13911491

13921492
reports.push({
@@ -1402,6 +1502,19 @@ async function mirrorRepoFiles(instance) {
14021502
okCount += 1;
14031503
} catch (error) {
14041504
const fsError = describeFsError(error);
1505+
const writeContext = (error && error.mirrorWriteContext) || lastWriteContext;
1506+
let parentAnalysis = writeContext?.analysis || null;
1507+
if (!parentAnalysis && error && typeof error === 'object' && error.analysis) {
1508+
parentAnalysis = error.analysis;
1509+
}
1510+
if (!parentAnalysis && writeContext?.parent) {
1511+
parentAnalysis = safeAnalyzePath(instance, writeContext.parent);
1512+
}
1513+
const parentExists = parentAnalysis?.exists === true;
1514+
const parentIsFolder =
1515+
parentAnalysis?.object?.isFolder === true || parentAnalysis?.isFolder === true;
1516+
const failedDestPath = writeContext?.destPath || repoTargetPath;
1517+
14051518
let failureMessage = toErrorMessage(error) || 'Unknown mirror failure';
14061519
if (fsError && fsError.label) {
14071520
const detailMessage = fsError.message && fsError.message !== fsError.label ? fsError.message : '';
@@ -1410,14 +1523,21 @@ async function mirrorRepoFiles(instance) {
14101523
const failureEntry = {
14111524
path,
14121525
repoPath: repoTargetPath,
1413-
destPath: repoTargetPath,
1526+
destPath: failedDestPath,
14141527
url,
14151528
status: typeof status === 'number' ? status : undefined,
14161529
bytes: actualBytes,
14171530
expectedBytes: bytes,
14181531
finalURL: finalUrl !== url ? finalUrl : undefined,
14191532
error: failureMessage,
14201533
reason: failureMessage,
1534+
parent: writeContext?.parent || undefined,
1535+
parentAnalysis: parentAnalysis
1536+
? {
1537+
exists: parentExists,
1538+
isFolder: parentIsFolder,
1539+
}
1540+
: undefined,
14211541
};
14221542
if (fsError) {
14231543
failureEntry.fsError = {

0 commit comments

Comments
 (0)