Skip to content

sync.sh --validate reports undefined-placeholder errors for tokens that only appear inside {{#if KEY}} conditional blocks #78

Description

@sebastientaggart

Summary

sync.sh --validate fails with "undefined placeholder" errors for any {{KEY}} token that only appears inside a {{#if KEY}}...{{/if}} conditional block when KEY is not defined in the consumer's .codecannon.yaml. The normal write path
(sync.sh without flags) handles the same config correctly — it strips the inactive conditional blocks before substituting
placeholders, so the output is clean.

This means --validate is stricter than the actual writer, which inverts the usual contract of a preflight check ("if
validate passes, sync will succeed" is still true, but "if validate fails, sync will fail" is not, which is misleading in
practice).

Reproduction

  1. Copy templates/codecannon.yaml to a consumer project as .codecannon.yaml.

  2. Use the default two-branch mode: leave # BRANCH_TEST: staging commented and leave # DEFAULT_MILESTONE: "Sprint 4"
    commented (both are commented by default in the shipped template).

  3. Run CodeCannon/sync.sh — succeeds, generates clean skill files with no literal placeholders.

  4. Run CodeCannon/sync.sh --validate — fails with errors like:

    Validation failed — undefined placeholders:
    
      deploy.md: {{BRANCH_TEST}} not defined in config
      deploy.md: {{BRANCH_TEST}} not defined in config
      ...
      start.md: {{DEFAULT_MILESTONE}} not defined in config
      ...
      submit-for-review.md: {{BRANCH_TEST}} not defined in config
    

    In my case this produced 12 errors across deploy.md, start.md, and submit-for-review.md.

Expected behavior

--validate should evaluate conditional blocks first (the same way the normal write path does) and only report tokens that
would genuinely be present in the final rendered output. If a {{KEY}} reference is inside a {{#if KEY}}...{{/if}}
block and KEY is undefined, the block is stripped, so KEY never needs to be defined and should not be reported as
missing.

Root cause

sync.sh's validate_placeholders (around line 344) reads each skill's body and scans it with find_unresolved without
calling apply_conditionals first:

def validate_placeholders(skill_files, project_config):
    """Scan all skills for {{PLACEHOLDER}} tokens; error if any are undefined."""
    errors = []
    for skill_path in skill_files:
        raw = skill_path.read_text()
        fm, body = parse_frontmatter(raw)
        text_to_check = body
        if fm.get('description'):
            text_to_check += '\n' + fm['description']
        missing = [p for p in find_unresolved(text_to_check) if p not in project_config]
        for p in missing:
            errors.append(f"  {skill_path.name}: {{{{{p}}}}} not defined in config")
    return errors

Meanwhile, the normal write path uses apply_conditionals (which, per its implementation around line 149, treats
bool(values.get(key, '').strip()) — so undefined and empty-string are equivalent when evaluating {{#if KEY}}). This
divergence is the bug.

Proposed fix

One-line change: evaluate conditionals before scanning for undefined placeholders:

def validate_placeholders(skill_files, project_config):
    """Scan all skills for {{PLACEHOLDER}} tokens; error if any are undefined."""
    errors = []
    for skill_path in skill_files:
        raw = skill_path.read_text()
        fm, body = parse_frontmatter(raw)
        # Evaluate conditionals first — placeholders inside stripped blocks
        # are not part of the final output and should not be reported.
        text_to_check = apply_conditionals(body, project_config)
        if fm.get('description'):
            text_to_check += '\n' + apply_conditionals(fm['description'], project_config)
        missing = [p for p in find_unresolved(text_to_check) if p not in project_config]
        for p in missing:
            errors.append(f"  {skill_path.name}: {{{{{p}}}}} not defined in config")
    return errors

This makes --validate the conservative preview of the actual writer, which is the usual contract for preflight checks.

Affected placeholders in the current templates

Any placeholder that only appears inside a {{#if KEY}} block. At minimum (based on a fresh grep of skills/*.md on
main):

  • {{BRANCH_TEST}}skills/deploy.md, skills/submit-for-review.md
  • {{DEFAULT_MILESTONE}}skills/start.md

Other placeholders wrapped in conditional blocks by #19
(BRANCH_DEV, PLATFORM_COMPLIANCE_NOTES, CONVENTIONS_NOTES) do not currently trigger the error because they happen to
also be referenced outside any conditional, or the .codecannon.yaml template's default values satisfy them. But they'd be
exposed to the same class of bug the moment anyone removes the unconditional references, so the fix is the right
structural answer rather than a per-placeholder patch.

Workaround for consumers until fixed

Add the affected keys to .codecannon.yaml with empty-string values:

BRANCH_TEST: ""
DEFAULT_MILESTONE: ""

This produces byte-identical output to leaving them undefined (confirmed via sync.sh --dry-run before and after) but
satisfies the strict validator. The downside is that the commented-out example hint (# BRANCH_TEST: staging) that the
template ships with is lost in the process, so future maintainers of the consumer config have to cross-reference
config.schema.yaml to discover the expected value shape.

Impact / severity

Low-severity correctness issue, but high visibility — any two-branch-mode consumer (the most common workflow profile) who
runs --validate on a fresh copy of templates/codecannon.yaml will hit this immediately. It also makes CI integration of
sync.sh --validate as a pre-commit or pre-sync check more painful than it should be for new adopters.

Metadata

Metadata

Assignees

No one assigned

    Labels

    qa-passedQA completed, passes

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions