fix: remove duplicate stateExtractField, cross-platform code detection, STATE.md file locking
Code review fixes from codebase pattern analysis:
1. state.cjs: Remove duplicate stateExtractField() definition (lines 12 vs 184).
The second definition shadowed the first with identical logic. Keeps the
original that uses escapeRegex() from core.cjs.
2. init.cjs: Replace Unix-only 'find' pipe with cross-platform fs.readdirSync
recursive walk for code file detection. The execSync('find ... | grep ...')
command fails on Windows where these Unix utilities aren't available.
Removes unused child_process import.
3. state.cjs: Add lockfile-based mutual exclusion to writeStateMd() to prevent
parallel executor agents from overwriting each other's STATE.md changes.
Uses O_EXCL atomic file creation for lock acquisition, stale lock detection
(10s timeout), and spin-wait with jitter. Ensures data integrity during
wave-based parallel execution where multiple agents update STATE.md
concurrently.
All 755 existing tests pass.
This commit is contained in:
@@ -167,17 +167,26 @@ function cmdInitNewProject(cwd, raw) {
|
||||
const braveKeyFile = path.join(homedir, '.gsd', 'brave_api_key');
|
||||
const hasBraveSearch = !!(process.env.BRAVE_API_KEY || fs.existsSync(braveKeyFile));
|
||||
|
||||
// Detect existing code
|
||||
// Detect existing code (cross-platform — no Unix `find` dependency)
|
||||
let hasCode = false;
|
||||
let hasPackageFile = false;
|
||||
try {
|
||||
const files = execSync('find . -maxdepth 3 \\( -name "*.ts" -o -name "*.js" -o -name "*.py" -o -name "*.go" -o -name "*.rs" -o -name "*.swift" -o -name "*.java" \\) 2>/dev/null | grep -v node_modules | grep -v .git | head -5', {
|
||||
cwd,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
});
|
||||
hasCode = files.trim().length > 0;
|
||||
} catch {}
|
||||
const codeExtensions = new Set(['.ts', '.js', '.py', '.go', '.rs', '.swift', '.java']);
|
||||
const skipDirs = new Set(['node_modules', '.git', '.planning', '.claude', '__pycache__', 'target', 'dist', 'build']);
|
||||
function findCodeFiles(dir, depth) {
|
||||
if (depth > 3) return false;
|
||||
let entries;
|
||||
try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return false; }
|
||||
for (const entry of entries) {
|
||||
if (entry.isFile() && codeExtensions.has(path.extname(entry.name))) return true;
|
||||
if (entry.isDirectory() && !skipDirs.has(entry.name)) {
|
||||
if (findCodeFiles(path.join(dir, entry.name), depth + 1)) return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
hasCode = findCodeFiles(cwd, 0);
|
||||
} catch { /* intentionally empty — best-effort detection */ }
|
||||
|
||||
hasPackageFile = pathExistsInternal(cwd, 'package.json') ||
|
||||
pathExistsInternal(cwd, 'requirements.txt') ||
|
||||
|
||||
@@ -180,18 +180,7 @@ function cmdStateUpdate(cwd, field, value) {
|
||||
}
|
||||
|
||||
// ─── State Progression Engine ────────────────────────────────────────────────
|
||||
|
||||
function stateExtractField(content, fieldName) {
|
||||
const escaped = fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
// Try **Field:** bold format first
|
||||
const boldPattern = new RegExp(`\\*\\*${escaped}:\\*\\*\\s*(.+)`, 'i');
|
||||
const boldMatch = content.match(boldPattern);
|
||||
if (boldMatch) return boldMatch[1].trim();
|
||||
// Fall back to plain Field: format
|
||||
const plainPattern = new RegExp(`^${escaped}:\\s*(.+)`, 'im');
|
||||
const plainMatch = content.match(plainPattern);
|
||||
return plainMatch ? plainMatch[1].trim() : null;
|
||||
}
|
||||
// stateExtractField is defined above (shared helper) — do not duplicate.
|
||||
|
||||
function stateReplaceField(content, fieldName, newValue) {
|
||||
const escaped = fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
@@ -677,10 +666,54 @@ function syncStateFrontmatter(content, cwd) {
|
||||
/**
|
||||
* Write STATE.md with synchronized YAML frontmatter.
|
||||
* All STATE.md writes should use this instead of raw writeFileSync.
|
||||
* Uses a simple lockfile to prevent parallel agents from overwriting
|
||||
* each other's changes (race condition with read-modify-write cycle).
|
||||
*/
|
||||
function writeStateMd(statePath, content, cwd) {
|
||||
const synced = syncStateFrontmatter(content, cwd);
|
||||
fs.writeFileSync(statePath, normalizeMd(synced), 'utf-8');
|
||||
const lockPath = statePath + '.lock';
|
||||
const maxRetries = 10;
|
||||
const retryDelay = 200; // ms
|
||||
|
||||
// Acquire lock (spin with backoff)
|
||||
for (let i = 0; i < maxRetries; i++) {
|
||||
try {
|
||||
// O_EXCL fails if file already exists — atomic lock
|
||||
const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
|
||||
fs.writeSync(fd, String(process.pid));
|
||||
fs.closeSync(fd);
|
||||
break;
|
||||
} catch (err) {
|
||||
if (err.code === 'EEXIST') {
|
||||
// Check for stale lock (> 10s old)
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (Date.now() - stat.mtimeMs > 10000) {
|
||||
fs.unlinkSync(lockPath);
|
||||
continue; // retry immediately after clearing stale lock
|
||||
}
|
||||
} catch { /* lock was released between check — retry */ }
|
||||
|
||||
if (i === maxRetries - 1) {
|
||||
// Last resort: write anyway rather than losing data
|
||||
try { fs.unlinkSync(lockPath); } catch {}
|
||||
break;
|
||||
}
|
||||
// Spin-wait with small jitter
|
||||
const jitter = Math.floor(Math.random() * 50);
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < retryDelay + jitter) { /* busy wait */ }
|
||||
continue;
|
||||
}
|
||||
break; // non-EEXIST error — proceed without lock
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
fs.writeFileSync(statePath, normalizeMd(synced), 'utf-8');
|
||||
} finally {
|
||||
try { fs.unlinkSync(lockPath); } catch { /* lock already gone */ }
|
||||
}
|
||||
}
|
||||
|
||||
function cmdStateJson(cwd, raw) {
|
||||
|
||||
Reference in New Issue
Block a user