diff --git a/docs/plans/2026-04-20-001-fix-ce-compound-yaml-safety-plan.md b/docs/plans/2026-04-20-001-fix-ce-compound-yaml-safety-plan.md new file mode 100644 index 000000000..6cfefd471 --- /dev/null +++ b/docs/plans/2026-04-20-001-fix-ce-compound-yaml-safety-plan.md @@ -0,0 +1,162 @@ +--- +title: "fix(ce-compound): quote YAML array items starting with reserved indicators" +type: fix +status: active +date: 2026-04-20 +--- + +# fix(ce-compound): quote YAML array items starting with reserved indicators + +## Overview + +`/ce-compound` emits invalid YAML frontmatter when an array item in any +frontmatter array-of-strings field (primarily `symptoms:`, `applies_when:`, +`tags:`, `related_components:`) starts with a backtick (`` ` ``) or other YAML +1.2 reserved indicator. Strict parsers (`yq`, `js-yaml` strict, PyYAML) reject +the resulting file. The existing angle-bracket-token guardrail (issue #602, +fixed in #603) does not generalize to array-item scalars. Teach the +`ce-compound` and `ce-compound-refresh` skills to quote unsafe array items, and +add a regression test so future prompt edits do not silently drop the rule. + +## Problem Frame + +YAML 1.2 reserves `` ` `` as an indicator character at the start of a scalar. When +the frontmatter-writing subagent (or the Lightweight-mode orchestrator) writes +markdown-style backtick-wrapped shell commands as array items, the output is +visually correct markdown but syntactically invalid YAML. Strict parsers reject +the file; `ce-learnings-researcher`'s grep-first retrieval still matches on +substrings, which masks the problem — users silently accumulate unparseable +files. Issue #606 provides the reproduction, impact, and suggested fix. + +## Requirements Trace + +- R1. New `ce-compound` output (Full and Lightweight modes) produces frontmatter + that parses under strict YAML 1.2 even when array items begin with reserved + indicator characters. +- R2. `ce-compound-refresh` Replace-flow subagent output meets the same bar. +- R3. The YAML-safety rule is captured as a durable contract in the authoritative + schema files (not only in prompt prose). +- R4. A regression test fails if the rule is removed from the prompts or the + schema contract, preventing silent drift. +- R5. Existing broken files already under `docs/solutions/` are out of scope. + +## Scope Boundaries + +- Do not auto-repair existing invalid frontmatter in users' repos. +- Do not add a runtime YAML validator step to `ce-compound`. +- Do not change frontmatter schema fields, enum values, or track rules. +- Do not extend quoting guidance to `description:` or other scalar fields + beyond what #603 already covered. + +### Deferred to Separate Tasks + +- A one-shot cleanup utility for repairing existing broken files in + `docs/solutions/`. +- Broader YAML-safety audit of other skills that write frontmatter. + +## Context & Research + +### Relevant Code and Patterns + +- `plugins/compound-engineering/skills/ce-compound/SKILL.md` — Phase 2 step 5 + validates frontmatter; Lightweight mode step 3 writes in a single pass. +- `plugins/compound-engineering/skills/ce-compound/references/schema.yaml` — + authoritative frontmatter contract with `validation_rules` list. +- `plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md` — + human-readable quick reference. +- `plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md` — + concrete frontmatter examples for both tracks. +- `plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md` — Replace + flow dispatches a subagent with the three support files as the source of + truth. +- `tests/compound-support-files.test.ts` — enforces byte-identical copies of + the three support files across the two skills. **Edits must be applied to + both skill copies.** +- `tests/frontmatter.test.ts` — validates strict YAML parseability of plugin + `SKILL.md` frontmatter. + +### Institutional Learnings + +- Issue #602 / PR #603 fixed an analogous bug in `description:` with (a) a + sentence in the skill prompt and (b) a regression test. Apply the same shape. +- Per plugin `AGENTS.md` Rationale Discipline: rule body lives in on-demand + reference files, not `SKILL.md`. + +## Key Technical Decisions + +- **Authoritative rule lives in `schema.yaml` `validation_rules` and a new + `yaml-schema.md` "YAML Safety Rules" section.** Subagents read these at write + time. +- **SKILL.md files get one-line pointers** at the frontmatter-writing spots. +- **Template files get a preamble comment** above each frontmatter block so + pattern-matching subagents see it. +- **Regression test asserts prompt-surface presence** (not runtime output + validity), mirroring the #603 pattern. +- **Mirror discipline:** all three support files are byte-identical across + the two skills. + +## Open Questions + +### Resolved During Planning + +- *Where does the rule live?* → Support files (contract surface). +- *Which reserved characters?* → `` ` ``, `[`, `*`, `&`, `!`, `|`, `>`, `%`, + `@`, `?` plus the `": "` substring trap. +- *Test strategy?* → Prompt presence, not runtime output. +- *Field scope?* → Field-agnostic ("any array-of-strings frontmatter field"). + +## Implementation Units + +- [ ] **Unit 1: Add YAML-safety rule to `schema.yaml` and `yaml-schema.md`** + +**Files:** +- Modify: `plugins/compound-engineering/skills/ce-compound/references/schema.yaml` +- Modify: `plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md` +- Modify: `plugins/compound-engineering/skills/ce-compound-refresh/references/schema.yaml` +- Modify: `plugins/compound-engineering/skills/ce-compound-refresh/references/yaml-schema.md` + +**Approach:** Append one entry to `schema.yaml` `validation_rules`. Add a new +"## YAML Safety Rules" section to `yaml-schema.md` with indicator-character +list, `": "` trap, and before/after example. Mirror to both skills. + +**Verification:** `bun test tests/compound-support-files.test.ts tests/frontmatter.test.ts` passes. + +- [ ] **Unit 2: Add frontmatter-writing pointers to `ce-compound/SKILL.md`** + +**Files:** `plugins/compound-engineering/skills/ce-compound/SKILL.md` + +**Approach:** Add one-line pointer to `references/yaml-schema.md > YAML Safety +Rules` in Phase 2 step 5 and Lightweight mode step 3. + +- [ ] **Unit 3: Add pointer to `ce-compound-refresh/SKILL.md` + template preambles** + +**Files:** +- Modify: `plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md` +- Modify: `plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md` +- Modify: `plugins/compound-engineering/skills/ce-compound-refresh/assets/resolution-template.md` + +**Approach:** Add one-line reminder to Replace-flow subagent dispatch. Add +HTML comment preamble above each frontmatter block in both template copies. + +- [ ] **Unit 4: Add regression test for YAML-safety rule presence** + +**Files:** `tests/compound-support-files.test.ts` (extend) + +**Approach:** Add `describe("ce-compound YAML safety rule presence", ...)` +block asserting: `validation_rules` contains YAML-safety entry, `yaml-schema.md` +has "YAML Safety Rules" heading, `resolution-template.md` references the rule, +both `SKILL.md` files point to the rule. + +## Risks & Dependencies + +| Risk | Mitigation | +|------|------------| +| LLM ignores the rule. | Three complementary surfaces (schema, yaml-schema, template preamble). | +| Future edits drop the rule. | Regression test (Unit 4). | +| Mirror drift. | Existing `compound-support-files.test.ts` enforces byte-identity. | + +## Sources & References + +- Issue: EveryInc/compound-engineering-plugin#606 +- Prior art: PR #603 (`fix(ce-release-notes): backtick-wrap token`) +- Related tests: `tests/frontmatter.test.ts`, `tests/compound-support-files.test.ts` diff --git a/plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md b/plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md index ea46304f2..7724f93fd 100644 --- a/plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md +++ b/plugins/compound-engineering/skills/ce-compound-refresh/SKILL.md @@ -516,7 +516,7 @@ Do not let replacement subagents invent frontmatter fields, enum values, or sect - A summary of the investigation evidence (what changed, what the current code does, why the old guidance is misleading) - The target path and category (same category as the old learning unless the category itself changed) - The relevant contents of the three support files listed above -2. The subagent writes the new learning using the support files as the source of truth: `references/schema.yaml` for frontmatter fields and enum values, `references/yaml-schema.md` for category mapping, and `assets/resolution-template.md` for section order. It should use dedicated file search and read tools if it needs additional context beyond what was passed. +2. The subagent writes the new learning using the support files as the source of truth: `references/schema.yaml` for frontmatter fields and enum values, `references/yaml-schema.md` for category mapping and YAML-safety rules for array items, and `assets/resolution-template.md` for section order. It should use dedicated file search and read tools if it needs additional context beyond what was passed. 3. After the subagent completes, the orchestrator deletes the old learning file. The new learning's frontmatter may include `supersedes: [old learning filename]` for traceability, but this is optional — the git history and commit message provide the same information. **When evidence is insufficient:** diff --git a/plugins/compound-engineering/skills/ce-compound-refresh/assets/resolution-template.md b/plugins/compound-engineering/skills/ce-compound-refresh/assets/resolution-template.md index c7a03d48d..f9924be67 100644 --- a/plugins/compound-engineering/skills/ce-compound-refresh/assets/resolution-template.md +++ b/plugins/compound-engineering/skills/ce-compound-refresh/assets/resolution-template.md @@ -8,6 +8,8 @@ Choose the template matching the problem_type track (see `references/schema.yaml Use for: `build_error`, `test_failure`, `runtime_error`, `performance_issue`, `database_issue`, `security_issue`, `ui_bug`, `integration_issue`, `logic_error` + + ```markdown --- title: [Clear problem title] @@ -54,6 +56,8 @@ tags: [keyword-one, keyword-two] Use for: `best_practice`, `documentation_gap`, `workflow_issue`, `developer_experience` + + ```markdown --- title: [Clear, descriptive title] diff --git a/plugins/compound-engineering/skills/ce-compound-refresh/references/schema.yaml b/plugins/compound-engineering/skills/ce-compound-refresh/references/schema.yaml index ad68b27f9..cd0bbc723 100644 --- a/plugins/compound-engineering/skills/ce-compound-refresh/references/schema.yaml +++ b/plugins/compound-engineering/skills/ce-compound-refresh/references/schema.yaml @@ -220,3 +220,4 @@ validation_rules: - "date must match YYYY-MM-DD format" - "rails_version, if provided, must match X.Y.Z format and only applies to bug-track docs" - "tags should be lowercase and hyphen-separated" + - "Array-of-strings frontmatter items (symptoms, applies_when, tags, related_components, or any future array field) must be wrapped in double quotes when the value starts with a YAML reserved indicator (`, [, *, &, !, |, >, %, @, ?) or contains the substring `: ` — otherwise strict YAML parsers reject the file" diff --git a/plugins/compound-engineering/skills/ce-compound-refresh/references/yaml-schema.md b/plugins/compound-engineering/skills/ce-compound-refresh/references/yaml-schema.md index ca6cd71be..328d000f1 100644 --- a/plugins/compound-engineering/skills/ce-compound-refresh/references/yaml-schema.md +++ b/plugins/compound-engineering/skills/ce-compound-refresh/references/yaml-schema.md @@ -85,3 +85,30 @@ Docs created before the track system may have `symptoms`/`root_cause`/`resolutio 7. Array fields must respect min/max item counts. 8. `date` must match `YYYY-MM-DD`. 9. `rails_version`, if present, must match `X.Y.Z` and only applies to bug-track docs. + +## YAML Safety Rules + +Strict YAML 1.2 parsers (`yq`, `js-yaml` strict, PyYAML) reject array items +that start with a reserved indicator character as unquoted scalars. When +writing items for any array-of-strings field (`symptoms`, `applies_when`, +`tags`, `related_components`, or any future array field), wrap the value in +double quotes if it starts with any of: + +`` ` ``, `[`, `*`, `&`, `!`, `|`, `>`, `%`, `@`, `?` + +Also quote if the value contains the substring `": "` — that punctuation +confuses flow-style parsers. + +Example — before (breaks strict YAML): + + symptoms: + - `sudo dscacheutil -flushcache` does not restore in-container mDNS + +Example — after (parses cleanly): + + symptoms: + - "`sudo dscacheutil -flushcache` does not restore in-container mDNS" + +This rule applies to all array-of-strings frontmatter fields. Scalar string +fields like `description:` have their own quoting rules (see plugin +`AGENTS.md` under "YAML Frontmatter"). diff --git a/plugins/compound-engineering/skills/ce-compound/SKILL.md b/plugins/compound-engineering/skills/ce-compound/SKILL.md index 2684f6152..9ee88fb46 100644 --- a/plugins/compound-engineering/skills/ce-compound/SKILL.md +++ b/plugins/compound-engineering/skills/ce-compound/SKILL.md @@ -216,7 +216,7 @@ The orchestrating agent (main conversation) performs these steps: - Tag session-sourced content with "(session history)" so its origin is clear to future readers - If findings are thin or "no relevant prior sessions," proceed without session context 4. Assemble complete markdown file from the collected pieces, reading `assets/resolution-template.md` for the section structure of new docs -5. Validate YAML frontmatter against `references/schema.yaml` +5. Validate YAML frontmatter against `references/schema.yaml`, including the YAML-safety quoting rule for array items (see `references/yaml-schema.md` > YAML Safety Rules) 6. Create directory if needed: `mkdir -p docs/solutions/[category]/` 7. Write the file: either the updated existing doc or the new `docs/solutions/[category]/[filename].md` @@ -340,7 +340,7 @@ The orchestrator (main conversation) performs ALL of the following in one sequen 1. **Extract from conversation**: Identify the problem and solution from conversation history. Also scan the "user's auto-memory" block injected into your system prompt, if present (Claude Code only) -- use any relevant notes as supplementary context alongside conversation history. Tag any memory-sourced content incorporated into the final doc with "(auto memory [claude])" 2. **Classify**: Read `references/schema.yaml` and `references/yaml-schema.md`, then determine track (bug vs knowledge), category, and filename 3. **Write minimal doc**: Create `docs/solutions/[category]/[filename].md` using the appropriate track template from `assets/resolution-template.md`, with: - - YAML frontmatter with track-appropriate fields + - YAML frontmatter with track-appropriate fields, applying the YAML-safety quoting rule for array items (see `references/yaml-schema.md` > YAML Safety Rules) - Bug track: Problem, root cause, solution with key code snippets, one prevention tip - Knowledge track: Context, guidance with key examples, one applicability note 4. **Skip specialized agent reviews** (Phase 3) to conserve context diff --git a/plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md b/plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md index c7a03d48d..f9924be67 100644 --- a/plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md +++ b/plugins/compound-engineering/skills/ce-compound/assets/resolution-template.md @@ -8,6 +8,8 @@ Choose the template matching the problem_type track (see `references/schema.yaml Use for: `build_error`, `test_failure`, `runtime_error`, `performance_issue`, `database_issue`, `security_issue`, `ui_bug`, `integration_issue`, `logic_error` + + ```markdown --- title: [Clear problem title] @@ -54,6 +56,8 @@ tags: [keyword-one, keyword-two] Use for: `best_practice`, `documentation_gap`, `workflow_issue`, `developer_experience` + + ```markdown --- title: [Clear, descriptive title] diff --git a/plugins/compound-engineering/skills/ce-compound/references/schema.yaml b/plugins/compound-engineering/skills/ce-compound/references/schema.yaml index ad68b27f9..cd0bbc723 100644 --- a/plugins/compound-engineering/skills/ce-compound/references/schema.yaml +++ b/plugins/compound-engineering/skills/ce-compound/references/schema.yaml @@ -220,3 +220,4 @@ validation_rules: - "date must match YYYY-MM-DD format" - "rails_version, if provided, must match X.Y.Z format and only applies to bug-track docs" - "tags should be lowercase and hyphen-separated" + - "Array-of-strings frontmatter items (symptoms, applies_when, tags, related_components, or any future array field) must be wrapped in double quotes when the value starts with a YAML reserved indicator (`, [, *, &, !, |, >, %, @, ?) or contains the substring `: ` — otherwise strict YAML parsers reject the file" diff --git a/plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md b/plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md index ca6cd71be..328d000f1 100644 --- a/plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md +++ b/plugins/compound-engineering/skills/ce-compound/references/yaml-schema.md @@ -85,3 +85,30 @@ Docs created before the track system may have `symptoms`/`root_cause`/`resolutio 7. Array fields must respect min/max item counts. 8. `date` must match `YYYY-MM-DD`. 9. `rails_version`, if present, must match `X.Y.Z` and only applies to bug-track docs. + +## YAML Safety Rules + +Strict YAML 1.2 parsers (`yq`, `js-yaml` strict, PyYAML) reject array items +that start with a reserved indicator character as unquoted scalars. When +writing items for any array-of-strings field (`symptoms`, `applies_when`, +`tags`, `related_components`, or any future array field), wrap the value in +double quotes if it starts with any of: + +`` ` ``, `[`, `*`, `&`, `!`, `|`, `>`, `%`, `@`, `?` + +Also quote if the value contains the substring `": "` — that punctuation +confuses flow-style parsers. + +Example — before (breaks strict YAML): + + symptoms: + - `sudo dscacheutil -flushcache` does not restore in-container mDNS + +Example — after (parses cleanly): + + symptoms: + - "`sudo dscacheutil -flushcache` does not restore in-container mDNS" + +This rule applies to all array-of-strings frontmatter fields. Scalar string +fields like `description:` have their own quoting rules (see plugin +`AGENTS.md` under "YAML Frontmatter"). diff --git a/tests/compound-support-files.test.ts b/tests/compound-support-files.test.ts index 0b13bf292..1aa0a6c5a 100644 --- a/tests/compound-support-files.test.ts +++ b/tests/compound-support-files.test.ts @@ -1,6 +1,7 @@ import { readFile } from "fs/promises" import path from "path" import { describe, expect, test } from "bun:test" +import { load } from "js-yaml" const PLUGIN_ROOT = path.join(process.cwd(), "plugins", "compound-engineering", "skills") @@ -28,3 +29,86 @@ describe("ce-compound support file drift", () => { }) } }) + +/** + * Regression tests for the YAML-safety quoting rule for array items. + * + * Array items in frontmatter fields like `symptoms:` that start with a YAML + * reserved indicator (`, [, *, &, !, |, >, %, @, ?) or contain `: ` must be + * wrapped in double quotes — otherwise strict YAML parsers reject the file. + * See issue #606. + */ +describe("ce-compound YAML safety rule presence", () => { + for (const skill of SKILLS_WITH_COPIES) { + test(`${skill}/references/schema.yaml validation_rules includes YAML-safety entry`, async () => { + const raw = await readFile( + path.join(PLUGIN_ROOT, skill, "references/schema.yaml"), + "utf8", + ) + const parsed = load(raw) as { validation_rules?: string[] } | null + expect(parsed).not.toBeNull() + expect(Array.isArray(parsed?.validation_rules)).toBe(true) + const hasSafetyRule = (parsed?.validation_rules ?? []).some((rule) => + /array.*(quote|reserved indicator)|reserved indicator.*quote|YAML[- ]safety/i.test(rule), + ) + expect(hasSafetyRule).toBe(true) + }) + + test(`${skill}/references/yaml-schema.md contains YAML Safety Rules section`, async () => { + const raw = await readFile( + path.join(PLUGIN_ROOT, skill, "references/yaml-schema.md"), + "utf8", + ) + expect(/^##\s+YAML\s+Safety\s+Rules/mi.test(raw)).toBe(true) + // Concrete example stays present so the rule remains actionable. + expect(raw).toMatch(/"`sudo dscacheutil/) + }) + + test(`${skill}/assets/resolution-template.md references YAML safety rules`, async () => { + const raw = await readFile( + path.join(PLUGIN_ROOT, skill, "assets/resolution-template.md"), + "utf8", + ) + expect(/YAML[- ]safety/i.test(raw)).toBe(true) + expect(raw).toMatch(/yaml-schema\.md/) + }) + } + + test("ce-compound/SKILL.md points at YAML Safety Rules in both frontmatter-writing spots", async () => { + const raw = await readFile( + path.join(PLUGIN_ROOT, "ce-compound", "SKILL.md"), + "utf8", + ) + // Match the distinctive write-path pointer phrase, not generic yaml-schema.md + // references (which also appear in the support-files list and inputs section). + // Both Full-mode Phase 2 step 5 and Lightweight mode step 3 must carry the + // pointer so dropping either one is caught. + const pointer = /YAML[- ]safety\s+quoting\s+rule\s+for\s+array\s+items/gi + const pointerMatches = raw.match(pointer) ?? [] + expect(pointerMatches.length).toBeGreaterThanOrEqual(2) + + // Each pointer must sit in the frontmatter-write step (step 5 of Full mode, + // step 3 of Lightweight mode), not drift to an unrelated location. Both + // steps carry the "YAML frontmatter" phrase adjacent to the pointer. + const frontmatterAdjacent = raw.match( + /YAML\s+frontmatter[\s\S]{0,400}?YAML[- ]safety\s+quoting\s+rule\s+for\s+array\s+items/gi, + ) ?? [] + expect(frontmatterAdjacent.length).toBeGreaterThanOrEqual(2) + }) + + test("ce-compound-refresh/SKILL.md points at YAML-safety rules in the Replace flow", async () => { + const raw = await readFile( + path.join(PLUGIN_ROOT, "ce-compound-refresh", "SKILL.md"), + "utf8", + ) + // Anchor to the Replace Flow section so a drifted or deleted pointer is + // caught even if the phrase still appears elsewhere in the file. + const replaceFlowMatch = raw.match( + /###\s+Replace\s+Flow\b([\s\S]*?)(?=\n###\s+\w|\n##\s+\w|$)/, + ) + expect(replaceFlowMatch).not.toBeNull() + const replaceFlow = replaceFlowMatch?.[1] ?? "" + expect(/YAML[- ]safety/i.test(replaceFlow)).toBe(true) + expect(replaceFlow).toMatch(/yaml-schema\.md/) + }) +})