fix: close VCS workflow acceptance gaps
This commit is contained in:
+133
-44
@@ -303,50 +303,14 @@ export class SvnProvider implements VcsProvider {
|
|||||||
combinedDiff += addDiffRes.stdout.trim() + "\n";
|
combinedDiff += addDiffRes.stdout.trim() + "\n";
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Unversioned file: generate diff manually
|
// Unversioned node: generate diff manually
|
||||||
if (fs.existsSync(fullPath)) {
|
// Recursively collect regular (non-symlink) files; skip symlinks and nodes outside wc
|
||||||
const stats = fs.statSync(fullPath);
|
const unversionedFiles = this.collectUnversionedFiles(fullPath, repoRoot);
|
||||||
if (stats.isDirectory()) {
|
// Sort stably so diff output is deterministic
|
||||||
// Skip directories
|
unversionedFiles.sort();
|
||||||
continue;
|
for (const absFile of unversionedFiles) {
|
||||||
}
|
const relFile = path.relative(repoRoot, absFile);
|
||||||
if (stats.isFile()) {
|
combinedDiff += this.generateUnversionedFileDiff(absFile, relFile);
|
||||||
try {
|
|
||||||
// Try to read as text, but handle binary
|
|
||||||
const content = fs.readFileSync(fullPath);
|
|
||||||
// Check if binary
|
|
||||||
const isBinary = content.includes(0);
|
|
||||||
|
|
||||||
if (isBinary) {
|
|
||||||
combinedDiff += `Index: ${entryPath}\n`;
|
|
||||||
combinedDiff += `===================================================================\n`;
|
|
||||||
combinedDiff += `Cannot display: file marked as a binary type.\n`;
|
|
||||||
combinedDiff += `svn:mime-type = application/octet-stream\n`;
|
|
||||||
} else {
|
|
||||||
const textContent = content.toString("utf8");
|
|
||||||
const lines = textContent.split("\n");
|
|
||||||
const hasTrailingNewline = textContent.endsWith("\n");
|
|
||||||
const lineCount = hasTrailingNewline ? lines.length - 1 : lines.length;
|
|
||||||
|
|
||||||
combinedDiff += `Index: ${entryPath}\n`;
|
|
||||||
combinedDiff += `===================================================================\n`;
|
|
||||||
combinedDiff += `--- ${entryPath}\t(nonexistent)\n`;
|
|
||||||
combinedDiff += `+++ ${entryPath}\t(working copy)\n`;
|
|
||||||
combinedDiff += `@@ -0,0 +1,${lineCount} @@\n`;
|
|
||||||
for (let i = 0; i < lines.length; i++) {
|
|
||||||
if (i === lines.length - 1 && hasTrailingNewline && lines[i] === "") {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
combinedDiff += `+${lines[i]}\n`;
|
|
||||||
}
|
|
||||||
if (!hasTrailingNewline && lineCount > 0) {
|
|
||||||
combinedDiff += "\\ No newline at end of file\n";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} catch (err) {
|
|
||||||
throw new VcsError(`Failed to read unversioned file ${entryPath}: ${(err as Error).message}`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (itemStatus === "missing" || itemStatus === "deleted") {
|
} else if (itemStatus === "missing" || itemStatus === "deleted") {
|
||||||
@@ -496,6 +460,131 @@ export class SvnProvider implements VcsProvider {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Recursively collect regular (non-symlink) files under an unversioned directory.
|
||||||
|
* Uses lstat so directory symlinks are never followed. Only returns nodes that are
|
||||||
|
* within repoRoot and that are regular files (not symlinks).
|
||||||
|
*/
|
||||||
|
private collectUnversionedFiles(rootPath: string, repoRoot: string): string[] {
|
||||||
|
const result: string[] = [];
|
||||||
|
|
||||||
|
// Skip .svn administration directory immediately
|
||||||
|
if (path.basename(rootPath) === ".svn") {
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
let st: fs.Stats;
|
||||||
|
try {
|
||||||
|
st = fs.lstatSync(rootPath);
|
||||||
|
} catch (err) {
|
||||||
|
throw new VcsError(`Failed to lstat ${rootPath}: ${(err as Error).message}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (st.isSymbolicLink()) {
|
||||||
|
// Do not follow symlinks, do not traverse, do not include
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (st.isFile()) {
|
||||||
|
const rel = path.relative(repoRoot, rootPath);
|
||||||
|
if (!rel.startsWith("..") && !path.isAbsolute(rel)) {
|
||||||
|
result.push(rootPath);
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (st.isDirectory()) {
|
||||||
|
let entries: string[];
|
||||||
|
try {
|
||||||
|
entries = fs.readdirSync(rootPath);
|
||||||
|
} catch (err) {
|
||||||
|
throw new VcsError(`Failed to readdir ${rootPath}: ${(err as Error).message}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const entry of entries) {
|
||||||
|
const full = path.join(rootPath, entry);
|
||||||
|
const rel = path.relative(repoRoot, full);
|
||||||
|
if (rel.startsWith("..") || path.isAbsolute(rel)) continue;
|
||||||
|
|
||||||
|
const nested = this.collectUnversionedFiles(full, repoRoot);
|
||||||
|
result.push(...nested);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Generate a review-evidence diff block for a single unversioned regular file.
|
||||||
|
* Always emits Index + header lines even for empty files.
|
||||||
|
*/
|
||||||
|
private generateUnversionedFileDiff(absFile: string, relPath: string): string {
|
||||||
|
let block = "";
|
||||||
|
let fd: number;
|
||||||
|
try {
|
||||||
|
// open with O_NOFOLLOW to ensure we do not follow symlinks at read time
|
||||||
|
fd = fs.openSync(absFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW);
|
||||||
|
} catch (err) {
|
||||||
|
throw new VcsError(`Failed to open unversioned file ${relPath}: ${(err as Error).message}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
const stats = fs.fstatSync(fd);
|
||||||
|
if (!stats.isFile()) {
|
||||||
|
throw new VcsError(`Failed to read unversioned file ${relPath}: not a regular file`);
|
||||||
|
}
|
||||||
|
const buffer = Buffer.alloc(stats.size);
|
||||||
|
let bytesRead = 0;
|
||||||
|
while (bytesRead < stats.size) {
|
||||||
|
const read = fs.readSync(fd, buffer, bytesRead, stats.size - bytesRead, null);
|
||||||
|
if (read === 0) break;
|
||||||
|
bytesRead += read;
|
||||||
|
}
|
||||||
|
const content = buffer.subarray(0, bytesRead);
|
||||||
|
const isBinary = content.includes(0);
|
||||||
|
|
||||||
|
block += `Index: ${relPath}\n`;
|
||||||
|
block += `===================================================================\n`;
|
||||||
|
|
||||||
|
if (isBinary) {
|
||||||
|
block += `Cannot display: file marked as a binary type.\n`;
|
||||||
|
block += `svn:mime-type = application/octet-stream\n`;
|
||||||
|
} else {
|
||||||
|
const textContent = content.toString("utf8");
|
||||||
|
block += `--- ${relPath}\t(nonexistent)\n`;
|
||||||
|
block += `+++ ${relPath}\t(working copy)\n`;
|
||||||
|
|
||||||
|
if (textContent.length === 0) {
|
||||||
|
// Empty file: hunk with zero lines
|
||||||
|
block += `@@ -0,0 +0,0 @@\n`;
|
||||||
|
} else {
|
||||||
|
const lines = textContent.split("\n");
|
||||||
|
const hasTrailingNewline = textContent.endsWith("\n");
|
||||||
|
const lineCount = hasTrailingNewline ? lines.length - 1 : lines.length;
|
||||||
|
block += `@@ -0,0 +1,${lineCount} @@\n`;
|
||||||
|
for (let i = 0; i < lines.length; i++) {
|
||||||
|
if (i === lines.length - 1 && hasTrailingNewline && lines[i] === "") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
block += `+${lines[i]}\n`;
|
||||||
|
}
|
||||||
|
if (!hasTrailingNewline && lineCount > 0) {
|
||||||
|
block += "\\ No newline at end of file\n";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
throw new VcsError(`Failed to read unversioned file ${relPath}: ${(err as Error).message}`);
|
||||||
|
} finally {
|
||||||
|
try {
|
||||||
|
fs.closeSync(fd);
|
||||||
|
} catch {
|
||||||
|
// ignore close error
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return block;
|
||||||
|
}
|
||||||
|
|
||||||
/** Strict validation: reject mixed revisions, switched paths, externals, conflicts, etc. */
|
/** Strict validation: reject mixed revisions, switched paths, externals, conflicts, etc. */
|
||||||
private validateStrictWorkingCopy(repoRoot: string): void {
|
private validateStrictWorkingCopy(repoRoot: string): void {
|
||||||
try {
|
try {
|
||||||
|
|||||||
+12
-4
@@ -278,7 +278,7 @@ export async function resolveVcsSelection(
|
|||||||
// Load config from every distinct candidate root
|
// Load config from every distinct candidate root
|
||||||
const configs = new Map<string, { vcs?: VcsKind; root: string }>();
|
const configs = new Map<string, { vcs?: VcsKind; root: string }>();
|
||||||
|
|
||||||
for (const [vcsType, root] of Object.entries(candidates) as Array<[string, string | undefined]>) {
|
for (const [, root] of Object.entries(candidates) as Array<[string, string | undefined]>) {
|
||||||
if (!root) continue;
|
if (!root) continue;
|
||||||
|
|
||||||
// Skip if we already loaded config from this root
|
// Skip if we already loaded config from this root
|
||||||
@@ -288,15 +288,23 @@ export async function resolveVcsSelection(
|
|||||||
const config = resolveWorkflowConfig(root, {});
|
const config = resolveWorkflowConfig(root, {});
|
||||||
configs.set(root, { vcs: config.vcs, root });
|
configs.set(root, { vcs: config.vcs, root });
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// Re-throw validation errors, tolerate missing config
|
// Re-throw validation/parse errors (WorkflowConfigError) — these are real problems
|
||||||
if (err instanceof WorkflowConfigError) {
|
if (err instanceof WorkflowConfigError) {
|
||||||
throw new WorkflowEngineError(`Invalid agent-workflow.json at ${root}: ${(err as WorkflowConfigError).message}`);
|
throw new WorkflowEngineError(`Invalid agent-workflow.json at ${root}: ${(err as WorkflowConfigError).message}`);
|
||||||
}
|
}
|
||||||
// Config doesn't exist - that's ok
|
// Re-throw file access errors unless the file simply doesn't exist (ENOENT).
|
||||||
configs.set(root, { root });
|
// Permission denied, I/O errors, generic Error/TypeError, etc. must surface rather than be silently ignored.
|
||||||
|
const errCode = (err as any)?.code;
|
||||||
|
if (errCode === "ENOENT") {
|
||||||
|
// Config file doesn't exist — that's ok, treat as no project config
|
||||||
|
configs.set(root, { root });
|
||||||
|
} else {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
// Extract project defaultVcs settings
|
// Extract project defaultVcs settings
|
||||||
const projectDefaults = Array.from(configs.values())
|
const projectDefaults = Array.from(configs.values())
|
||||||
.map(c => c.vcs)
|
.map(c => c.vcs)
|
||||||
|
|||||||
+779
-110
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user