diff --git a/core/commands/naturalize.md b/core/commands/naturalize.md index c104be0..1a3ef0b 100644 --- a/core/commands/naturalize.md +++ b/core/commands/naturalize.md @@ -22,11 +22,23 @@ Do this: command, invariants, where architecture docs and ADRs live. Everything outside that region is installer-owned and will be regenerated by `engsys update`. -3. **Replace `` placeholders.** Grep the installed tree - (`.mcp.json`, `.claude/hooks/*`, stack fragments echoed into CLAUDE.md) for - `` / `` / `` / `` and substitute real - values. For iOS, that's the Xcode project path, scheme, simulator UDID, bundle - id; for cloud, the resource group / account / naming suffix. +3. **Replace `` placeholders — but only where the edit + survives `engsys update`.** `.mcp.json` is safe to hand-edit in place: + `engsys update` preserves any naturalized (non-placeholder) value already + there and only refreshes fields still carrying the literal `` marker. Grep it for `` / `` / `` / + `` and substitute real values — for iOS that's the Xcode project + path, scheme, simulator UDID, bundle id; for cloud, the resource group / + account / naming suffix. + + The `## Stack` section of `CLAUDE.md` is different: it's the pack's + `claude.fragment.md` text, spliced in **verbatim** and fully regenerated by + every `engsys update` — any `` placeholder comment + left there is an installer-owned example, not a fill-in-the-blank. Never + hand-edit it in place (the edit will silently vanish on the next update). + Instead, capture the resolved facts it's pointing at (project path, scheme, + resource group, etc.) as a bullet in the PROJECT-FACTS region, which *is* + preserved. 4. **Fold in imported AI config (if any).** If `docs/imported-ai-config/` exists, the installer found pre-existing Copilot/Cursor/etc. config and snapshotted it. diff --git a/core/skills/merge-monster/SKILL.md b/core/skills/merge-monster/SKILL.md index 85f2406..58d310d 100644 --- a/core/skills/merge-monster/SKILL.md +++ b/core/skills/merge-monster/SKILL.md @@ -82,7 +82,10 @@ fresh, nothing else merges. Full design: `docs/merge-monster.md` in engsys ## Preflight (verify the enqueuer's claims) -`gh pr view N --json isDraft,mergeable,mergeStateStatus,reviewThreads,baseRefName,body,comments,files,commits` +`/scripts/mm-preflight.sh --repo --pr N` — `gh pr view --json` +has no `reviewThreads` field (thread resolution is GraphQL-only); this script +stitches `isDraft,mergeable,mergeStateStatus,baseRefName,body,comments,files,commits` +from `gh pr view` together with a `reviewThreads` GraphQL query into one object. - all review threads resolved (the ruleset will block otherwise) - `ready_requirements.review_marker` comment present, if configured diff --git a/core/skills/merge-monster/scripts/mm-preflight.sh b/core/skills/merge-monster/scripts/mm-preflight.sh new file mode 100755 index 0000000..b848d4c --- /dev/null +++ b/core/skills/merge-monster/scripts/mm-preflight.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# mm-preflight.sh — one JSON snapshot for the preflight gate on a single PR. +# `gh pr view --json` has no `reviewThreads` field (thread resolution is only +# available over GraphQL) — this stitches the `gh pr view` fields the model +# needs together with a GraphQL reviewThreads query into one JSON object. +# +# Usage: mm-preflight.sh --repo owner/name --pr N +set -euo pipefail + +REPO="" PR="" +while [ $# -gt 0 ]; do + case "$1" in + --repo) REPO="$2"; shift 2 ;; + --pr) PR="$2"; shift 2 ;; + *) echo "unknown arg: $1" >&2; exit 2 ;; + esac +done +[ -n "$REPO" ] && [ -n "$PR" ] || { echo "usage: mm-preflight.sh --repo owner/name --pr N" >&2; exit 2; } + +OWNER="${REPO%%/*}" +NAME="${REPO##*/}" + +VIEW=$(gh pr view "$PR" -R "$REPO" \ + --json isDraft,mergeable,mergeStateStatus,baseRefName,body,comments,files,commits) + +THREADS=$(gh api graphql -f owner="$OWNER" -f name="$NAME" -F number="$PR" -f query=' + query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { id isResolved } + } + } + } + }' --jq '.data.repository.pullRequest.reviewThreads') + +jq -n --argjson view "$VIEW" --argjson threads "$THREADS" '$view + {reviewThreads: $threads}' diff --git a/docs/naturalization.md b/docs/naturalization.md index 8451f48..7116d9b 100644 --- a/docs/naturalization.md +++ b/docs/naturalization.md @@ -21,11 +21,15 @@ engsys verify --into . # confirm nothing drifted - The fenced `PROJECT-FACTS` region of `CLAUDE.md` (between the `ENGSYS:PROJECT-FACTS:START/END` markers). `engsys update` regenerates everything *outside* this region and never overwrites what's inside. -- `` / `` / `` / `` / `` placeholders that - packs left in `.mcp.json`, hooks, and the spliced CLAUDE.md fragments. +- `` / `` / `` / `` / `` placeholders in + `.mcp.json`. This one is safe to hand-edit in place: `engsys update` + deep-merges each mcpServer entry and only refreshes fields still carrying the + literal `` marker, so a naturalized value survives every + future update. - The project's `engsys.config.yaml` `naturalize:` block (model_strategy, hook_patterns, invariants) — edit there and re-run `engsys update` rather than - hand-editing generated files. + hand-editing generated files. `invariants` renders as a bullet list appended + to the PROJECT-FACTS seed on first install. ## What naturalization must NOT touch @@ -35,7 +39,12 @@ engsys verify --into . # confirm nothing drifted differences belong in *packs*, selected via config — not in edits to a persona. If Melvin feels "too generic," the fix is a richer `cloud-architecture-` pack in engsys, not a per-project edit. -- Content outside the PROJECT-FACTS fence in CLAUDE.md. +- Content outside the PROJECT-FACTS fence in CLAUDE.md — this includes the + `## Stack` section, which is a pack's `claude.fragment.md` spliced in + verbatim and fully regenerated on every `engsys update`. Any `` comment there is an installer-owned example, not a fill-in-the-blank + — a hand-edit is silently reverted by the next update. Capture the resolved + facts in the PROJECT-FACTS region instead. ## Good project facts diff --git a/lib/render.js b/lib/render.js index 997466c..3850861 100644 --- a/lib/render.js +++ b/lib/render.js @@ -46,6 +46,11 @@ function renderClaudeMd(engsysRoot, config, plan, existingClaudeMd, seedFacts) { let projectFacts = nat.project_facts || '> TODO (naturalize): describe this project — services, runtimes, build/verify toolchain, hard invariants, key paths. Run `/naturalize` or fill this in by hand.'; + const invariants = Array.isArray(nat.invariants) ? nat.invariants.filter(Boolean) : []; + if (invariants.length) { + projectFacts += '\n\n**Invariants (hard rules).**\n' + invariants.map((i) => `- ${i}`).join('\n'); + } + // A foreign CLAUDE.md (or imported AI config) seeds the facts on first adoption. if (seedFacts) projectFacts = seedFacts; @@ -107,8 +112,34 @@ function renderSettingsLocal(engsysRoot, plan) { return JSON.stringify(base, null, 2) + '\n'; } +const NATURALIZE_PLACEHOLDER_RE = /^$/; + +// Deep-merge a fresh pack mcpServer definition with what's already on disk. +// The pack's own values win (keeps structure current across engsys updates), +// except where the fresh value is still an unfilled "" +// placeholder and the existing value is a real, previously-naturalized +// replacement — that survives `engsys update` instead of being clobbered. +function mergeMcpServer(fresh, existing) { + if (fresh === null || typeof fresh !== 'object' || Array.isArray(fresh)) { + if (typeof fresh === 'string' && NATURALIZE_PLACEHOLDER_RE.test(fresh) && + typeof existing === 'string' && !NATURALIZE_PLACEHOLDER_RE.test(existing)) { + return existing; + } + return fresh; + } + const out = {}; + for (const key of Object.keys(fresh)) { + out[key] = mergeMcpServer(fresh[key], existing && existing[key]); + } + return out; +} + function renderMcpJson(plan, existingMcp) { - const servers = Object.assign({}, (existingMcp && existingMcp.mcpServers) || {}, plan.mcpServers); + const existingServers = (existingMcp && existingMcp.mcpServers) || {}; + const servers = Object.assign({}, existingServers); + for (const [name, fresh] of Object.entries(plan.mcpServers)) { + servers[name] = mergeMcpServer(fresh, existingServers[name]); + } return JSON.stringify({ mcpServers: servers }, null, 2) + '\n'; } diff --git a/lib/selftest.js b/lib/selftest.js index 4daeccf..11c0509 100644 --- a/lib/selftest.js +++ b/lib/selftest.js @@ -1,8 +1,12 @@ 'use strict'; -// Tiny zero-dep test harness for the YAML subset parser. Run: npm test +// Tiny zero-dep test harness for lib/. Run: npm test const assert = require('assert'); +const path = require('path'); const { parseYaml } = require('./yaml'); +const render = require('./render'); + +const ENGSYS_ROOT = path.join(__dirname, '..'); let pass = 0; function check(name, fn) { fn(); pass++; console.log(` ok ${name}`); } @@ -74,6 +78,51 @@ empty: ~ assert.strictEqual(c.empty, null); }); +check('block scalar (project_facts: |)', () => { + const c = parseYaml(` +naturalize: + project_facts: | + - Serve with \`python3 -m http.server 8517\`. + - Architecture: \`data.js\` is the read-only baseline. + model_strategy: "after the block" +`); + assert.strictEqual( + c.naturalize.project_facts, + '- Serve with `python3 -m http.server 8517`.\n- Architecture: `data.js` is the read-only baseline.\n' + ); + assert.strictEqual(c.naturalize.model_strategy, 'after the block'); +}); + +check('block scalar nested under a block-list map (hook_patterns reminder)', () => { + const c = parseYaml(` +naturalize: + hook_patterns: + - glob: "docs/spec.md" + reminder: | + Bump the version header. + Keep cross-refs in sync. +`); + assert.strictEqual( + c.naturalize.hook_patterns[0].reminder, + 'Bump the version header.\nKeep cross-refs in sync.\n' + ); +}); + +check('block scalar chomp indicators (strip/keep)', () => { + const c = parseYaml(` +a: |- + no trailing newline +b: |+ + keep trailing newline +c: > + folded into + one line +`); + assert.strictEqual(c.a, 'no trailing newline'); + assert.strictEqual(c.b, 'keep trailing newline\n'); + assert.strictEqual(c.c, 'folded into one line\n'); +}); + check('inline flow maps (single-line stack/lessons)', () => { const c = parseYaml(` project: {name: SeedTest, description: x} @@ -87,4 +136,64 @@ lessons: {seed: false} assert.strictEqual(c.lessons.seed, false); }); +check('renderClaudeMd appends naturalize.invariants to project facts', () => { + const config = { + project: { name: 'Acme', description: 'desc' }, + naturalize: { + project_facts: 'Base facts.', + invariants: ['Never commit *.secret files.', 'Always run tests before push.'], + }, + }; + const plan = { claudeFragments: [], mcpServers: {}, commands: [] }; + const out = render.renderClaudeMd(ENGSYS_ROOT, config, plan, null, null); + assert.ok(out.includes('Base facts.')); + assert.ok(out.includes('**Invariants (hard rules).**')); + assert.ok(out.includes('- Never commit *.secret files.')); + assert.ok(out.includes('- Always run tests before push.')); +}); + +check('renderClaudeMd omits the invariants heading when none are configured', () => { + const config = { project: {}, naturalize: { project_facts: 'Base facts.' } }; + const plan = { claudeFragments: [], mcpServers: {}, commands: [] }; + const out = render.renderClaudeMd(ENGSYS_ROOT, config, plan, null, null); + assert.ok(!out.includes('Invariants (hard rules)')); +}); + +check('renderMcpJson preserves naturalized values, refreshes unfilled placeholders', () => { + const plan = { + mcpServers: { + xcodebuildmcp: { + type: 'stdio', + command: 'npx', + args: ['-y', 'xcodebuildmcp@latest', 'mcp'], + env: { + XCODEBUILDMCP_PROJECT_PATH: '', + XCODEBUILDMCP_SCHEME: '', + XCODEBUILDMCP_PLATFORM: 'iOS Simulator', + }, + }, + }, + }; + const existing = { + mcpServers: { + xcodebuildmcp: { + type: 'stdio', + command: 'npx', + args: ['-y', 'xcodebuildmcp@latest', 'mcp'], + env: { + XCODEBUILDMCP_PROJECT_PATH: 'App/App.xcodeproj', + XCODEBUILDMCP_SCHEME: 'App', + XCODEBUILDMCP_PLATFORM: 'iOS Simulator', + }, + }, + custom: { type: 'stdio', command: 'my-own-server' }, + }, + }; + const out = JSON.parse(render.renderMcpJson(plan, existing)); + assert.strictEqual(out.mcpServers.xcodebuildmcp.env.XCODEBUILDMCP_PROJECT_PATH, 'App/App.xcodeproj'); + assert.strictEqual(out.mcpServers.xcodebuildmcp.env.XCODEBUILDMCP_SCHEME, 'App'); + assert.strictEqual(out.mcpServers.xcodebuildmcp.env.XCODEBUILDMCP_PLATFORM, 'iOS Simulator'); + assert.strictEqual(out.mcpServers.custom.command, 'my-own-server'); +}); + console.log(`\n${pass} checks passed.`); diff --git a/lib/yaml.js b/lib/yaml.js index 4d5c549..2b4d034 100644 --- a/lib/yaml.js +++ b/lib/yaml.js @@ -7,9 +7,12 @@ // - inline flow lists: key: [a, b, c] // - block lists: key:\n - a\n - b (items indented under the key) // - block lists of maps: key:\n - glob: x\n reminder: y +// - block scalars: key: | / key: > (with optional -/+ chomp indicator) // - full-line (#...) and trailing ( #...) comments outside quotes // It is NOT a general YAML implementation. Block sequences MUST be indented -// under their key. If you need richer config, switch the loader to js-yaml. +// under their key. Block scalars may not contain blank lines (blank lines are +// stripped before parsing). If you need richer config, switch the loader to +// js-yaml. function stripTrailingComment(s) { let inS = false, inD = false; @@ -72,6 +75,29 @@ function parseScalar(v) { return v; } +const BLOCK_SCALAR_RE = /^([|>])([+-]?)$/; + +// Consume a block scalar body (lines more indented than `blockIndent`, +// starting at `lines[idx]`) and return { text, idx } with idx positioned at +// the first line after the block. `style` is '|' (literal) or '>' (folded); +// `chomp` is '' (clip: single trailing newline), '-' (strip), or '+' (keep). +function readBlockScalar(lines, idx, blockIndent, indentOf, style, chomp) { + const raw = []; + let minIndent = null; + while (idx < lines.length) { + const li = indentOf(lines[idx]); + if (li <= blockIndent) break; + if (minIndent === null || li < minIndent) minIndent = li; + raw.push(lines[idx]); + idx++; + } + const body = raw.map((l) => l.slice(minIndent || 0)); + let text = style === '>' ? body.join(' ') : body.join('\n'); + if (chomp === '+') text += '\n'; + else if (chomp !== '-' && body.length) text += '\n'; + return { text, idx }; +} + function parseYaml(text) { const lines = []; for (const raw of text.split(/\r?\n/)) { @@ -142,6 +168,12 @@ function parseYaml(text) { } else { obj[key] = null; } + } else if (BLOCK_SCALAR_RE.test(val)) { + const [, style, chomp] = val.match(BLOCK_SCALAR_RE); + idx++; + const res = readBlockScalar(lines, idx, blockIndent, indentOf, style, chomp); + obj[key] = res.text; + idx = res.idx; } else { obj[key] = parseScalar(val); idx++;