From bdf9e006c457537d41d2a211bfb1c21e2f3c30e2 Mon Sep 17 00:00:00 2001 From: mbanderas Date: Thu, 18 Jun 2026 02:48:06 +0100 Subject: [PATCH 1/2] refactor(install): block-replace stale maestro doctrine in appendOnlyDoctrine Replace the present-with-sentinel skip path with an in-place block splice: absent -> append below user content; present -> replace between markers, preserving everything outside. Splice safety: LF-normalized canonical body (no perpetual diff), newline style taken from the destination file, abort without writing on ambiguous (>1 begin/end) or corrupt (begin without end) markers. Idempotent: identical doctrine is a no-op. Makes install.cjs the single merge source of truth so a synced repo refreshes doctrine without clobbering user-owned AGENTS.md content. Tests 14-18. --- scripts/install.cjs | 69 ++++++++++++++++++++++++----- scripts/install.test.cjs | 94 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 10 deletions(-) diff --git a/scripts/install.cjs b/scripts/install.cjs index c665401..2b99dd9 100644 --- a/scripts/install.cjs +++ b/scripts/install.cjs @@ -483,8 +483,28 @@ function readPkgFile(rel, log) { } /** - * Install a doctrine/adapter markdown file. Append-only, idempotent, never - * clobbers user content above the maestro block; refuses symlinks. + * Build the canonical maestro doctrine block (markers + normalized body) using + * the given newline style. Deterministic: the body is normalized to `nl` and + * trailing blank lines are trimmed, so re-running produces byte-identical + * output (no perpetual diff). Returns the SENTINEL..SENTINEL_END span only; + * the caller owns the surrounding newlines. + * @param {string} srcContent doctrine source + * @param {string} nl newline style ('\n' or '\r\n') + * @returns {string} + */ +function buildDoctrineBlock(srcContent, nl) { + const body = srcContent.replace(/\r\n/g, '\n').replace(/\n+$/, '').replace(/\n/g, nl); + return `${SENTINEL}${nl}${body}${nl}${SENTINEL_END}`; +} + +/** + * Install a doctrine/adapter markdown file, merge-safe. Never clobbers user + * content outside the maestro block: if the block is absent it is appended + * below existing content; if present it is REPLACED in place (refreshes stale + * doctrine) while preserving everything outside the markers. Idempotent — + * re-running with identical doctrine is a no-op. Refuses symlinks and aborts + * (without writing) on an ambiguous/corrupt marker state. Newline style is + * taken from the destination file. * @param {string} dest absolute destination path * @param {string} srcContent content to install * @param {string} label short name for logs (e.g. "AGENTS.md") @@ -493,8 +513,6 @@ function readPkgFile(rel, log) { * @returns {boolean} true = success (or no-op), false = error */ function appendOnlyDoctrine(dest, srcContent, label, dryRun, log) { - const block = `\n${SENTINEL}\n${srcContent}\n${SENTINEL_END}\n`; - let existsStat; try { existsStat = fs.lstatSync(dest); } catch { existsStat = null; } @@ -510,17 +528,49 @@ function appendOnlyDoctrine(dest, srcContent, label, dryRun, log) { return false; } - if (existing.includes(SENTINEL)) { - log(`[doctrine] ${label} already contains sentinel — skipping`); + const nl = existing.includes('\r\n') ? '\r\n' : '\n'; + const beginCount = existing.split(SENTINEL).length - 1; + const endCount = existing.split(SENTINEL_END).length - 1; + + if (beginCount > 0) { + // Block present — replace it in place, preserving content outside. + if (beginCount > 1 || endCount > 1) { + log(`ERROR: ${label} has ${beginCount} begin / ${endCount} end maestro markers — refusing to splice an ambiguous block: ${dest}`); + return false; + } + const bi = existing.indexOf(SENTINEL); + const ei = existing.indexOf(SENTINEL_END); + if (ei === -1 || ei < bi) { + log(`ERROR: ${label} has a maestro begin marker without a following end marker — refusing to splice a corrupt block: ${dest}`); + return false; + } + const prefix = existing.slice(0, bi); + const suffix = existing.slice(ei + SENTINEL_END.length); + const updated = prefix + buildDoctrineBlock(srcContent, nl) + suffix; + + if (updated === existing) { + log(`[doctrine] ${label} already up to date — skipping`); + return true; + } + if (dryRun) { + log(`[dry-run] would refresh maestro block in ${dest}`); + return true; + } + const res = safeWrite(dest, updated); + if (!res.ok) { + log(`ERROR: failed to refresh ${label}: ${res.reason}`); + return false; + } + log(`[doctrine] refreshed maestro block in ${label}`); return true; } + // Block absent — append below existing user content. if (dryRun) { log(`[dry-run] would append maestro doctrine to existing ${dest}`); return true; } - - const res = safeWrite(dest, existing + block); + const res = safeWrite(dest, existing + nl + buildDoctrineBlock(srcContent, nl) + nl); if (!res.ok) { log(`ERROR: failed to append to ${label}: ${res.reason}`); return false; @@ -540,8 +590,7 @@ function appendOnlyDoctrine(dest, srcContent, label, dryRun, log) { return false; } - const freshContent = SENTINEL + '\n' + srcContent + '\n' + SENTINEL_END + '\n'; - const res = safeWrite(dest, freshContent); + const res = safeWrite(dest, buildDoctrineBlock(srcContent, '\n') + '\n'); if (!res.ok) { log(`ERROR: failed to write ${label}: ${res.reason}`); return false; diff --git a/scripts/install.test.cjs b/scripts/install.test.cjs index 1673b79..71c9fb6 100644 --- a/scripts/install.test.cjs +++ b/scripts/install.test.cjs @@ -393,6 +393,100 @@ function mkTmpTracked() { captured2.join('').includes('preserved user-edited legacy Codex skill')); } +// ---- test 14: block-replace refreshes a stale maestro block, preserves outside ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + const stale = + 'MY NOTES\n\n\nOLD STALE DOCTRINE\n\n\nMORE NOTES\n'; + fs.writeFileSync(agents, stale, 'utf8'); + + run(['--target', 'codex', '--project', TMP]); + + const r = fs.readFileSync(agents, 'utf8'); + check('14a: stale block content is gone', !r.includes('OLD STALE DOCTRINE')); + check('14b: real doctrine now inside the block', r.includes('Decision Gate')); + check('14c: user content above the block preserved', + r.includes('MY NOTES') && r.indexOf('MY NOTES') < r.indexOf('')); + check('14d: user content below the block preserved', + r.includes('MORE NOTES') && r.indexOf('MORE NOTES') > r.indexOf('')); + check('14e: still exactly one begin and one end', + (r.match(//g) || []).length === 1 && + (r.match(//g) || []).length === 1); +} + +// ---- test 15: replace path is idempotent (second run = no change, logs up to date) ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + run(['--target', 'codex', '--project', TMP]); + const after1 = fs.readFileSync(agents, 'utf8'); + + const origWrite = process.stdout.write.bind(process.stdout); + const captured = []; + process.stdout.write = (s) => { captured.push(s); origWrite(s); return true; }; + run(['--target', 'codex', '--project', TMP]); + process.stdout.write = origWrite; + + const after2 = fs.readFileSync(agents, 'utf8'); + check('15a: second run leaves AGENTS.md byte-identical', after2 === after1); + check('15b: second run logs the doctrine block is up to date', + captured.join('').includes('AGENTS.md already up to date')); +} + +// ---- test 16: ambiguous (double-begin) markers -> abort, file unchanged ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + const doubled = + '\nA\n\nUSER\n\nB\n\n'; + fs.writeFileSync(agents, doubled, 'utf8'); + + const origWrite = process.stdout.write.bind(process.stdout); + const captured = []; + process.stdout.write = (s) => { captured.push(s); origWrite(s); return true; }; + const code = run(['--target', 'codex', '--project', TMP]); + process.stdout.write = origWrite; + + check('16a: run returns non-zero on ambiguous markers', code !== 0); + check('16b: AGENTS.md left unchanged', fs.readFileSync(agents, 'utf8') === doubled); + check('16c: logs a refusal mentioning markers', captured.join('').toLowerCase().includes('marker')); +} + +// ---- test 17: begin-without-end -> abort, file unchanged ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + const broken = 'USER\n\nNO END HERE\n'; + fs.writeFileSync(agents, broken, 'utf8'); + + const origWrite = process.stdout.write.bind(process.stdout); + const captured = []; + process.stdout.write = (s) => { captured.push(s); origWrite(s); return true; }; + const code = run(['--target', 'codex', '--project', TMP]); + process.stdout.write = origWrite; + + check('17a: run returns non-zero on begin-without-end', code !== 0); + check('17b: AGENTS.md left unchanged', fs.readFileSync(agents, 'utf8') === broken); + check('17c: logs a refusal', captured.join('').toLowerCase().includes('refus')); +} + +// ---- test 18: CRLF file -> CRLF block, idempotent (no perpetual diff) ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + fs.writeFileSync(agents, 'USER\r\n', 'utf8'); + + run(['--target', 'codex', '--project', TMP]); + const after1 = fs.readFileSync(agents, 'utf8'); + check('18a: block written with CRLF on a CRLF file', /\r\n/.test(after1)); + check('18b: user CRLF content preserved', after1.includes('USER\r\n')); + + run(['--target', 'codex', '--project', TMP]); + const after2 = fs.readFileSync(agents, 'utf8'); + check('18c: re-run on a CRLF file is byte-idempotent', after2 === after1); +} + // ---- cleanup ---- for (const d of tmpDirs) { From 95b50b5d22aa8b478de4240db09d240253d8d866 Mon Sep 17 00:00:00 2001 From: mbanderas Date: Thu, 18 Jun 2026 02:53:37 +0100 Subject: [PATCH 2/2] refactor(install): route sync through install.cjs splice, drop Copy-Item clobber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sync-maestro.ps1 no longer overwrites whole AGENTS.md files. It now invokes `node scripts/install.cjs --project --doctrine-only` per downstream repo, making the marker-splice the single merge path: stale doctrine is refreshed in place while user content outside the block survives. Adds a --doctrine-only mode to install.cjs (splices AGENTS.md only — no engine/adapter/wrapper/skills) and a -ListFile param to the sync for testability. Tests: install test 19 (--doctrine-only splices AGENTS.md, writes nothing else, idempotent); new sync-maestro smoke (real PowerShell: dry-run writes nothing, content outside the block survives a sync, re-run byte-idempotent, missing dir non-fatal; skips if no PowerShell on PATH). --- scripts/install.cjs | 18 +++++- scripts/install.test.cjs | 22 +++++++ scripts/sync-maestro.ps1 | 64 +++++++++---------- scripts/sync-maestro.test.cjs | 113 ++++++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 33 deletions(-) create mode 100644 scripts/sync-maestro.test.cjs diff --git a/scripts/install.cjs b/scripts/install.cjs index 2b99dd9..d799566 100644 --- a/scripts/install.cjs +++ b/scripts/install.cjs @@ -413,7 +413,7 @@ function legacyGenericCodexTemplate(srcContent, legacyName, namespacedName) { /** * @param {string[]} argv - * @returns {{ target: string, project: string, user: boolean, dryRun: boolean, noHooks: boolean }} + * @returns {{ target: string, project: string, user: boolean, dryRun: boolean, noHooks: boolean, doctrineOnly: boolean }} */ function parseArgs(argv) { const opts = { @@ -422,6 +422,7 @@ function parseArgs(argv) { user: false, dryRun: false, noHooks: false, + doctrineOnly: false, }; let i = 0; @@ -437,6 +438,8 @@ function parseArgs(argv) { opts.dryRun = true; } else if (a === '--no-hooks') { opts.noHooks = true; + } else if (a === '--doctrine-only') { + opts.doctrineOnly = true; } i++; } @@ -995,13 +998,24 @@ function installCodexSkills(projectRoot, userGlobal, dryRun, log) { */ function run(argv) { const opts = parseArgs(argv || []); - const { target: rawTarget, project, user: userGlobal, dryRun } = opts; + const { target: rawTarget, project, user: userGlobal, dryRun, doctrineOnly } = opts; const lines = []; const log = (msg) => { lines.push(msg); process.stdout.write(msg + '\n'); }; if (dryRun) log('[dry-run] planning only — no files will be written'); + // Doctrine-only — splice just the AGENTS.md kernel (used by sync-maestro.ps1 + // so the marker-splice is the single merge path; no engine/adapter/wrapper). + if (doctrineOnly) { + if (!installDoctrine(project, dryRun, log)) { + log('doctrine sync completed with errors (see above)'); + return 1; + } + log('doctrine sync complete'); + return 0; + } + // Resolve target let target = rawTarget; if (target === 'auto') { diff --git a/scripts/install.test.cjs b/scripts/install.test.cjs index 71c9fb6..256ada8 100644 --- a/scripts/install.test.cjs +++ b/scripts/install.test.cjs @@ -487,6 +487,28 @@ function mkTmpTracked() { check('18c: re-run on a CRLF file is byte-idempotent', after2 === after1); } +// ---- test 19: --doctrine-only splices AGENTS.md and writes nothing else ---- +{ + const TMP = mkTmpTracked(); + const agents = path.join(TMP, 'AGENTS.md'); + fs.writeFileSync(agents, 'USER OWNS THIS\n', 'utf8'); + + const code = run(['--doctrine-only', '--project', TMP]); + const r = fs.readFileSync(agents, 'utf8'); + + check('19a: --doctrine-only succeeds', code === 0); + check('19b: doctrine block spliced below user content', + r.includes('USER OWNS THIS') && r.includes('') && r.includes('Decision Gate')); + check('19c: no engine installed (frontier/ absent)', !fs.existsSync(path.join(TMP, 'frontier'))); + check('19d: no codex skills installed (.agents/skills absent)', + !fs.existsSync(path.join(TMP, '.agents', 'skills'))); + check('19e: no docs/orchestration.md installed', !fs.existsSync(path.join(TMP, 'docs'))); + + const after1 = fs.readFileSync(agents, 'utf8'); + run(['--doctrine-only', '--project', TMP]); + check('19f: --doctrine-only re-run is byte-idempotent', fs.readFileSync(agents, 'utf8') === after1); +} + // ---- cleanup ---- for (const d of tmpDirs) { diff --git a/scripts/sync-maestro.ps1 b/scripts/sync-maestro.ps1 index 6953b93..c6c3043 100644 --- a/scripts/sync-maestro.ps1 +++ b/scripts/sync-maestro.ps1 @@ -1,17 +1,28 @@ <# .SYNOPSIS - Sync Maestro AGENTS.md to downstream repos listed in scripts/downstream.txt. + Sync Maestro doctrine (the AGENTS.md kernel) to downstream repos listed in + scripts/downstream.txt. .DESCRIPTION - Copies Maestro/AGENTS.md verbatim to each downstream workspace root. - Warns if downstream CLAUDE.md exists but lacks `@AGENTS.md` import. + For each downstream workspace, invokes + `node scripts/install.cjs --project --doctrine-only`, which splices + the Maestro doctrine block between its `` / + `` markers: it refreshes stale doctrine in place while + preserving any user content outside the block. install.cjs is the single + merge source of truth; this script never overwrites a whole AGENTS.md + (no more Copy-Item -Force clobber). + Warns if a downstream CLAUDE.md exists but lacks the `@AGENTS.md` import. Does NOT git-add or commit — review and commit per repo manually. .PARAMETER WorkspaceRoot Parent dir containing all workspaces. Default: C:\Users\mail\Workspaces +.PARAMETER ListFile + File of downstream workspace names (one per line; blank lines and lines + starting with # are ignored). Default: scripts/downstream.txt + .PARAMETER DryRun - Show what would change. No writes. + Show what would change. No writes (passed through to install.cjs). .EXAMPLE pwsh ./scripts/sync-maestro.ps1 @@ -20,23 +31,22 @@ [CmdletBinding()] param( [string]$WorkspaceRoot = 'C:\Users\mail\Workspaces', + [string]$ListFile, [switch]$DryRun ) $ErrorActionPreference = 'Stop' $scriptDir = Split-Path -Parent $MyInvocation.MyCommand.Path -$maestroRoot = Split-Path -Parent $scriptDir -$source = Join-Path $maestroRoot 'AGENTS.md' -$listFile = Join-Path $scriptDir 'downstream.txt' +$installer = Join-Path $scriptDir 'install.cjs' +if (-not $ListFile) { $ListFile = Join-Path $scriptDir 'downstream.txt' } -if (-not (Test-Path $source)) { throw "Missing source: $source" } -if (-not (Test-Path $listFile)) { throw "Missing list: $listFile" } +if (-not (Test-Path $installer)) { throw "Missing installer: $installer" } +if (-not (Test-Path $ListFile)) { throw "Missing list: $ListFile" } -$sourceHash = (Get-FileHash $source).Hash -$targets = Get-Content $listFile | ForEach-Object { $_.Trim() } | +$targets = Get-Content $ListFile | ForEach-Object { $_.Trim() } | Where-Object { $_ -and -not $_.StartsWith('#') } -$copied = 0; $skipped = 0; $missing = 0; $warnings = @() +$synced = 0; $failed = 0; $missing = 0; $warnings = @() foreach ($name in $targets) { $repoDir = Join-Path $WorkspaceRoot $name @@ -45,23 +55,15 @@ foreach ($name in $targets) { $missing++; continue } - $dest = Join-Path $repoDir 'AGENTS.md' - $needsCopy = $true - if (Test-Path $dest) { - if ((Get-FileHash $dest).Hash -eq $sourceHash) { - Write-Host "OK $name" -ForegroundColor DarkGray - $skipped++; $needsCopy = $false - } - } - - if ($needsCopy) { - if ($DryRun) { - Write-Host "DRY $name (would copy)" -ForegroundColor Cyan - } else { - Copy-Item -Path $source -Destination $dest -Force - Write-Host "COPY $name" -ForegroundColor Green - } - $copied++ + $nodeArgs = @($installer, '--project', $repoDir, '--doctrine-only') + if ($DryRun) { $nodeArgs += '--dry-run' } + & node @nodeArgs + if ($LASTEXITCODE -ne 0) { + Write-Host "FAIL $name (installer exit $LASTEXITCODE)" -ForegroundColor Red + $failed++ + } else { + Write-Host "SYNC $name" -ForegroundColor Green + $synced++ } $claudeMd = Join-Path $repoDir 'CLAUDE.md' @@ -72,11 +74,11 @@ foreach ($name in $targets) { } Write-Host '' -Write-Host "Source hash: $sourceHash" -Write-Host "Copied: $copied Up-to-date: $skipped Missing dirs: $missing" +Write-Host "Synced: $synced Failed: $failed Missing dirs: $missing" if ($warnings.Count) { Write-Host '' Write-Host "Warnings:" -ForegroundColor Yellow $warnings | ForEach-Object { Write-Host $_ -ForegroundColor Yellow } } if ($DryRun) { Write-Host ''; Write-Host "(dry-run — no files written)" -ForegroundColor Cyan } +if ($failed) { exit 1 } diff --git a/scripts/sync-maestro.test.cjs b/scripts/sync-maestro.test.cjs new file mode 100644 index 0000000..01ecdcb --- /dev/null +++ b/scripts/sync-maestro.test.cjs @@ -0,0 +1,113 @@ +#!/usr/bin/env node +// Smoke test for scripts/sync-maestro.ps1. Proves the rewritten sync delegates +// to install.cjs (marker-splice): user content OUTSIDE the block survives, the +// block is refreshed, a re-run is byte-idempotent, and a dry-run writes nothing. +// Skips gracefully (passes) if no PowerShell (pwsh/powershell) is on PATH. +// +// Run: node scripts/sync-maestro.test.cjs + +'use strict'; + +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +let failures = 0; +function check(name, cond) { + if (cond) process.stdout.write(` ok ${name}\n`); + else { failures++; process.stderr.write(` FAIL ${name}\n`); } +} + +console.log('sync-maestro smoke tests'); + +// Prefer pwsh (PS7, the project standard); on Windows fall back to Windows +// PowerShell. Skip (pass) if neither is available — keeps npm test green on +// runners without PowerShell while exercising the real script where it exists. +function findShell() { + const candidates = ['pwsh']; + if (process.platform === 'win32') candidates.push('powershell'); + for (const sh of candidates) { + try { + const r = spawnSync(sh, ['-NoProfile', '-Command', '$PSVersionTable.PSVersion.Major'], + { encoding: 'utf8' }); + if (r.status === 0) return sh; + } catch { /* not found — try next */ } + } + return null; +} + +const shell = findShell(); +if (!shell) { + console.log(' SKIP no PowerShell (pwsh/powershell) on PATH — sync smoke not run'); + console.log('\nall tests passed (sync smoke skipped)'); + process.exit(0); +} + +const script = path.join(__dirname, 'sync-maestro.ps1'); + +const tmpDirs = []; +function mkTmp() { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'maestro-sync-test-')); + tmpDirs.push(d); + return d; +} + +function runSync(workspaceRoot, listFile, dryRun) { + const args = ['-NoProfile', '-File', script, + '-WorkspaceRoot', workspaceRoot, '-ListFile', listFile]; + if (dryRun) args.push('-DryRun'); + return spawnSync(shell, args, { encoding: 'utf8' }); +} + +// Downstream repo: user content surrounding a STALE maestro block. +const ws = mkTmp(); +const repo = path.join(ws, 'testrepo'); +fs.mkdirSync(repo); +const dest = path.join(repo, 'AGENTS.md'); +const userContent = + 'MY REPO NOTES\n\n\nOLD STALE DOCTRINE\n\n\nMORE USER NOTES\n'; +fs.writeFileSync(dest, userContent, 'utf8'); + +const listFile = path.join(ws, 'list.txt'); +fs.writeFileSync(listFile, 'testrepo\n', 'utf8'); + +// 1. Dry-run writes nothing. +const dry = runSync(ws, listFile, true); +check('a: dry-run exits 0', dry.status === 0); +check('b: dry-run leaves AGENTS.md byte-unchanged', fs.readFileSync(dest, 'utf8') === userContent); + +// 2. Real sync splices the block, preserving content outside it. +const real = runSync(ws, listFile, false); +check('c: sync exits 0', real.status === 0); +const after1 = fs.readFileSync(dest, 'utf8'); +check('d: stale doctrine replaced with real doctrine', + !after1.includes('OLD STALE DOCTRINE') && after1.includes('Decision Gate')); +check('e: user content ABOVE the block preserved', + after1.includes('MY REPO NOTES') && + after1.indexOf('MY REPO NOTES') < after1.indexOf('')); +check('f: user content BELOW the block preserved', + after1.includes('MORE USER NOTES') && + after1.indexOf('MORE USER NOTES') > after1.indexOf('')); +check('g: exactly one begin and one end marker', + (after1.match(//g) || []).length === 1 && + (after1.match(//g) || []).length === 1); + +// 3. Re-run is byte-idempotent. +const real2 = runSync(ws, listFile, false); +check('h: re-run exits 0', real2.status === 0); +check('i: re-run is byte-idempotent', fs.readFileSync(dest, 'utf8') === after1); + +// 4. A missing workspace dir is reported, not fatal. +const ws2 = mkTmp(); +const list2 = path.join(ws2, 'list.txt'); +fs.writeFileSync(list2, 'does-not-exist\n', 'utf8'); +const miss = runSync(ws2, list2, false); +check('j: missing downstream dir is non-fatal (exit 0)', miss.status === 0); +check('k: missing downstream dir reported as MISS', (miss.stdout || '').includes('MISS')); + +// cleanup +for (const d of tmpDirs) { try { fs.rmSync(d, { recursive: true, force: true }); } catch {} } + +if (failures > 0) { console.error(`\n${failures} failure(s)`); process.exit(1); } +console.log('\nall tests passed');