Compare commits

...
Author SHA1 Message Date
TabishB 4a723999c4 fix: prefer native realpath for canonical paths 2026-04-14 17:24:04 +10:00
TabishB 8945ed21ca fix: canonicalize workflow artifact paths 2026-04-14 17:01:41 +10:00
7 changed files with 89 additions and 16 deletions
+1 -1
View File
@@ -250,7 +250,7 @@ export async function generateApplyInstructions(
): Promise<ApplyInstructions> {
// loadChangeContext will auto-detect schema from metadata if not provided
const context = loadChangeContext(projectRoot, changeName, schemaName);
const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName);
const changeDir = context.changeDir;
// Get the full schema to access the apply phase configuration
const schema = resolveSchema(context.schemaName, projectRoot);
+4 -1
View File
@@ -11,6 +11,7 @@ import {
getSchemaDir,
ArtifactGraph,
} from '../../core/artifact-graph/index.js';
import { FileSystemUtils } from '../../utils/file-system.js';
import { validateSchemaExists, DEFAULT_SCHEMA } from './shared.js';
// -----------------------------------------------------------------------------
@@ -68,7 +69,9 @@ export async function templatesCommand(options: TemplatesOptions): Promise<void>
const templates: TemplateInfo[] = graph.getAllArtifacts().map((artifact) => ({
artifactId: artifact.id,
templatePath: path.join(schemaDir, 'templates', artifact.template),
templatePath: FileSystemUtils.canonicalizeExistingPath(
path.join(schemaDir, 'templates', artifact.template)
),
source,
}));
+10 -5
View File
@@ -4,6 +4,7 @@ import { getSchemaDir, resolveSchema } from './resolver.js';
import { ArtifactGraph } from './graph.js';
import { detectCompleted } from './state.js';
import { resolveSchemaForChange } from '../../utils/change-metadata.js';
import { FileSystemUtils } from '../../utils/file-system.js';
import { readProjectConfig, validateConfigRules } from '../project-config.js';
import type { Artifact, CompletedSet } from './types.js';
@@ -137,15 +138,17 @@ export function loadTemplate(
);
}
const fullPath = path.join(schemaDir, 'templates', templatePath);
const templatePathOnDisk = path.join(schemaDir, 'templates', templatePath);
if (!fs.existsSync(fullPath)) {
if (!fs.existsSync(templatePathOnDisk)) {
throw new TemplateLoadError(
`Template not found: ${fullPath}`,
fullPath
`Template not found: ${templatePathOnDisk}`,
templatePathOnDisk
);
}
const fullPath = FileSystemUtils.canonicalizeExistingPath(templatePathOnDisk);
try {
return fs.readFileSync(fullPath, 'utf-8');
} catch (err) {
@@ -175,7 +178,9 @@ export function loadChangeContext(
changeName: string,
schemaName?: string
): ChangeContext {
const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName);
const changeDir = FileSystemUtils.canonicalizeExistingPath(
path.join(projectRoot, 'openspec', 'changes', changeName)
);
// Resolve schema: explicit > metadata > default
const resolvedSchemaName = resolveSchemaForChange(changeDir, schemaName);
+6 -2
View File
@@ -19,14 +19,18 @@ export function resolveArtifactOutputs(changeDir: string, generates: string): st
if (!isGlobPattern(generates)) {
try {
return fs.statSync(fullPattern).isFile() ? [fullPattern] : [];
return fs.statSync(fullPattern).isFile()
? [FileSystemUtils.canonicalizeExistingPath(fullPattern)]
: [];
} catch {
return [];
}
}
const normalizedPattern = FileSystemUtils.toPosixPath(fullPattern);
const matches = fg.sync(normalizedPattern, { onlyFiles: true }).map((match) => path.normalize(match));
const matches = fg
.sync(normalizedPattern, { onlyFiles: true })
.map((match) => FileSystemUtils.canonicalizeExistingPath(path.normalize(match)));
return Array.from(new Set(matches)).sort();
}
+21 -1
View File
@@ -1,6 +1,9 @@
import { promises as fs, constants as fsConstants } from 'fs';
import * as nodeFs from 'fs';
import path from 'path';
const fs = nodeFs.promises;
const { constants: fsConstants } = nodeFs;
function isMarkerOnOwnLine(content: string, markerIndex: number, markerLength: number): boolean {
let leftIndex = markerIndex - 1;
while (leftIndex >= 0 && content[leftIndex] !== '\n') {
@@ -50,6 +53,23 @@ export class FileSystemUtils {
return p.replace(/\\/g, '/');
}
/**
* Returns a canonical absolute path when the target exists.
* Falls back to path.resolve() so callers can still produce a stable absolute path.
*/
static canonicalizeExistingPath(targetPath: string): string {
try {
// Prefer the native resolver so Windows short-path aliases are expanded.
return nodeFs.realpathSync.native(targetPath);
} catch {
try {
return nodeFs.realpathSync(targetPath);
} catch {
return path.resolve(targetPath);
}
}
}
private static isWindowsBasePath(basePath: string): boolean {
return /^[A-Za-z]:[\\/]/.test(basePath) || basePath.startsWith('\\');
}
+29 -5
View File
@@ -20,7 +20,7 @@ describe('artifact-graph/outputs', () => {
const filePath = path.join(tempDir, 'proposal.md');
fs.writeFileSync(filePath, 'content');
expect(resolveArtifactOutputs(tempDir, 'proposal.md')).toEqual([filePath]);
expect(resolveArtifactOutputs(tempDir, 'proposal.md')).toEqual([fs.realpathSync(filePath)]);
expect(artifactOutputExists(tempDir, 'proposal.md')).toBe(true);
});
@@ -38,7 +38,7 @@ describe('artifact-graph/outputs', () => {
fs.mkdirSync(nestedDir, { recursive: true });
fs.writeFileSync(filePath, 'content');
expect(resolveArtifactOutputs(tempDir, 'specs/*/spec.md')).toEqual([filePath]);
expect(resolveArtifactOutputs(tempDir, 'specs/*/spec.md')).toEqual([fs.realpathSync(filePath)]);
expect(artifactOutputExists(tempDir, 'specs/*/spec.md')).toBe(true);
});
@@ -50,7 +50,7 @@ describe('artifact-graph/outputs', () => {
fs.writeFileSync(matching, 'content');
fs.writeFileSync(nonMatching, 'content');
expect(resolveArtifactOutputs(tempDir, 'specs/foo*.md')).toEqual([matching]);
expect(resolveArtifactOutputs(tempDir, 'specs/foo*.md')).toEqual([fs.realpathSync(matching)]);
});
it('supports question-mark glob patterns', () => {
@@ -60,7 +60,7 @@ describe('artifact-graph/outputs', () => {
fs.writeFileSync(matching, 'content');
fs.writeFileSync(path.join(specsDir, 'a10.md'), 'content');
expect(resolveArtifactOutputs(tempDir, 'specs/a?.md')).toEqual([matching]);
expect(resolveArtifactOutputs(tempDir, 'specs/a?.md')).toEqual([fs.realpathSync(matching)]);
});
it('supports character class glob patterns', () => {
@@ -72,7 +72,31 @@ describe('artifact-graph/outputs', () => {
fs.writeFileSync(bPath, 'content');
fs.writeFileSync(path.join(specsDir, 'c.md'), 'content');
expect(resolveArtifactOutputs(tempDir, 'specs/[ab].md')).toEqual([aPath, bPath]);
expect(resolveArtifactOutputs(tempDir, 'specs/[ab].md')).toEqual([
fs.realpathSync(aPath),
fs.realpathSync(bPath),
]);
});
it('canonicalizes resolved paths when the change directory is accessed through an alias', () => {
const rootDir = path.join(tempDir, 'workspace');
const realChangeDir = path.join(rootDir, 'real-change');
const aliasChangeDir = path.join(rootDir, 'alias-change');
const specDir = path.join(realChangeDir, 'specs', 'change-a');
const proposalPath = path.join(realChangeDir, 'proposal.md');
const specPath = path.join(specDir, 'spec.md');
fs.mkdirSync(specDir, { recursive: true });
fs.writeFileSync(proposalPath, 'content');
fs.writeFileSync(specPath, 'content');
fs.symlinkSync(realChangeDir, aliasChangeDir, process.platform === 'win32' ? 'junction' : 'dir');
expect(resolveArtifactOutputs(aliasChangeDir, 'proposal.md')).toEqual([
fs.realpathSync(proposalPath),
]);
expect(resolveArtifactOutputs(aliasChangeDir, 'specs/*/spec.md')).toEqual([
fs.realpathSync(specPath),
]);
});
it('returns an empty list when no files match the artifact output', () => {
+18 -1
View File
@@ -1,4 +1,5 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import * as nodeFs from 'fs';
import { promises as fs } from 'fs';
import path from 'path';
import os from 'os';
@@ -92,6 +93,22 @@ describe('FileSystemUtils', () => {
});
});
describe('canonicalizeExistingPath', () => {
it('should prefer the native realpath resolver when available', async () => {
const filePath = path.join(testDir, 'canonical.txt');
await fs.writeFile(filePath, 'content');
const nativeSpy = vi.spyOn(nodeFs.realpathSync, 'native');
const resolved = FileSystemUtils.canonicalizeExistingPath(filePath);
expect(nativeSpy).toHaveBeenCalledWith(filePath);
expect(resolved).toBe(nodeFs.realpathSync.native(filePath));
nativeSpy.mockRestore();
});
});
describe('writeFile', () => {
it('should write content to file', async () => {
const filePath = path.join(testDir, 'output.txt');