fix(init): forge-template pipeline personas, missing contract, validate --all

[nextlevelshit]

Summary

• Missing contract: epic-children.schema.json was referenced by ops-implement-epic but never shipped — users hit contract validation failure at runtime
• Broken forge filter: filterPersonasByForge used stale prefixes (gh-, gl-) that never matched actual persona names (github-, gitlab-), making the filter a no-op
• Hardcoded GitHub personas: All 21 persona: github-* refs across 12 pipelines now use {{ forge.type }}-* templates so non-GitHub forges (GitLab, Gitea, Bitbucket) resolve to the correct persona at runtime
• Init template expansion: filterTransitiveDeps now expands {{ forge.type }} to all 4 forge variants before filtering, so wave init includes the right persona configs
• TUI empty parentheses: Composition steps (pipeline/branch/gate) showed scan () — now shows scan (pipeline:audit-security), triage (branch), etc.
• wave validate --all: New flag scans every pipeline for missing personas, contracts, sub-pipelines, prompt files, and broken dependencies

Test plan

• go test ./... all green
• go build ./... clean
• wave validate --all catches missing personas/contracts on a fresh init
• wave init on a GitLab/Gitea project produces correct forge-specific personas in wave.yaml
• TUI pipeline detail shows step type labels for composition steps

[nextlevelshit]

Code Review (Wave Pipeline)

Verdict: REQUEST_CHANGES

This PR makes a solid architectural move — replacing hardcoded github- persona prefixes with {{ forge.type }} templates to enable forge-agnostic pipelines. The template expansion approach is sound and the runtime resolution via ResolvePlaceholders works correctly. However, two correctness bugs in the executor mean templated personas leak unresolved strings into events, metrics, and scope validation, and the new validation logic has a false-positive bug and no test coverage.

---

Critical Issues (must fix)

1. Unresolved persona template in events, metrics, and logs

internal/pipeline/executor.go — ~15 call sites after L1183

ResolvePlaceholders correctly resolves step.Persona into resolvedPersona at L1183, and the manifest lookup at L1185 uses it. But every subsequent reference — event emissions, performance metrics, audit logs, and the AdapterRunConfig.Persona field — still uses the raw step.Persona (e.g., {{ forge.type }}-commenter). This means the TUI displays template strings, SQLite metrics contain unresolved placeholders, and the adapter receives a template instead of the actual persona name.

Fix: Replace all step.Persona references after L1183 in runStepExecution with resolvedPersona.

2. Persona scope validation skips all templated personas

internal/pipeline/executor.go:351-352

m.GetPersona(step.Persona) is called with the unresolved template string, which always returns nil, silently skipping token scope validation for every forge-templated persona. If any forge-specific persona defines token_scopes, those scopes are never validated.

Fix: Resolve template variables before the GetPersona call.

3. Dependency validation produces false positives on valid pipelines

cmd/wave/commands/validate.go:328-332

The stepIDs map is populated during iteration, so a step listed before its dependency triggers a spurious "unknown dependency" error. The executor does topological sorting at runtime, so YAML ordering is irrelevant.

Fix: Two-pass validation — first pass collects all step IDs, second pass validates dependencies.

4. No test coverage for new validation logic

cmd/wave/commands/validate.go, cmd/wave/commands/init.go

validatePipelineFull, resolveForgeTemplate, isCompositionStep, and forge template expansion in filterTransitiveDeps are all untested. These are critical code paths that determine correctness of wave validate and wave init.

Fix: Add table-driven tests covering known forge, unknown forge, no-template, composition step types, dependency ordering, and missing persona/sub-pipeline scenarios.

---

Suggested Improvements

• Path traversal in validatePipelineFull (validate.go:268): The --pipeline flag value is used unsanitized in filepath.Join. Validate that it contains no path separators or .. components. Similarly, SubPipeline and SchemaPath from YAML should be confined to the project directory. Low practical risk (CLI access implies local access) but violates defense-in-depth. (Security: MEDIUM)

• Hardcoded pipeline path ignores --manifest location (validate.go:268): validatePipelineFull resolves pipelines relative to CWD, not relative to the manifest directory. The --all codepath correctly uses filepath.Dir(opts.ManifestPath) but --pipeline does not. Pass the manifest directory through.

• Duplicated forge template expansion: resolveForgeTemplate in validate.go and inline expansion in filterTransitiveDeps (init.go) duplicate the same pattern. Extract a shared helper like expandForgeTemplates(persona string) []string.

• Inconsistent template whitespace handling: Both {{ forge.type }} and {{forge.type}} are handled, but variants like {{ forge.type}} would pass through unresolved. Consider a regex pattern \{\{\s*forge\.type\s*\}\} for robust matching.

• Silent forge detection error (validate.go:123): forge.DetectFromGitRemotes() error is discarded. Log it in verbose mode so users can diagnose unexpected "all-variants" fallback behavior.

• TUI stepTypeLabel returns empty string for unknown types (pipeline_detail_provider.go:38): Return a fallback like "step" for unrecognized composition types.

---

Positive Observations

• The forge-agnostic template approach is well-designed — clean separation between template resolution at validation time (expand to all variants) vs. runtime (resolve to detected forge)
• resolveForgeTemplate correctly handles the ForgeUnknown case by expanding to all four variants, ensuring validation doesn't miss personas
• The isCompositionStep detection and TUI handling of persona-less steps is a nice UX improvement
• Pipeline YAML changes are consistent and mechanical — no missed substitutions across 11 pipeline files
• The structured validatePipelineFull function is a significant improvement over the previous validation approach

---

Generated by Wave pr-review pipeline

[nextlevelshit]

Review Response

All critical findings addressed in commits 634f764 and e11a236:

Critical Issues — Fixed

1. Unresolved persona template in events/metrics/logs — 634f764: Replaced all 13 step.Persona references after resolvedPersona computation with resolvedPersona in events, metrics, audit logs, and AdapterRunConfig.Persona.

2. Persona scope validation skipping templated personas — 634f764: Added pipelineContext.ResolvePlaceholders(step.Persona) before GetPersona() call in scope validation loop.

3. Dependency validation false positives — 634f764: Two-pass validation — first pass collects all step IDs, second pass validates dependencies and persona refs.

4. No test coverage — 634f764: Added table-driven tests for resolveForgeTemplate, isCompositionStep, validatePipelineFull (including forward dependency regression test), filterTransitiveDeps forge expansion, and stepTypeLabel.

Suggested Improvements — Addressed

• TUI stepTypeLabel fallback — 634f764: Returns "step" instead of empty string for unrecognized types.
• golangci-lint failures — e11a236: Fixed 2x errcheck (LogStepStart/LogToolCall) and 3x staticcheck QF1012 (fmt.Fprintf instead of WriteString(fmt.Sprintf(...))).

Noted but deferred

• Path traversal in --pipeline flag: Low practical risk (CLI = local access). Can add validation in a follow-up.
• --manifest location for --pipeline path: Noted, will align in follow-up.
• Shared forge template expansion helper: Both callsites are small; extracting now would be premature. Will consolidate if a third callsite appears.
• Whitespace-variant templates ({{ forge.type}}): The pipeline YAML is generated, not hand-edited — consistent spacing is enforced. Regex matching would be over-engineering.
• Silent forge detection error: Forge detection returning ForgeUnknown is the happy path for repos without remotes — logging would be noise.