feat(pipeline): unify platform-specific pipelines with forge template variables

[nextlevelshit]

Summary

Replaces 25 platform-specific pipeline YAML files (gh-*, gl-*, bb-*, gt-*) with 6 unified pipelines that use {{ forge.* }} template variables, resolved at runtime based on git remote detection.

• Forge template variables: Extended ForgeInfo with PRTerm/PRCommand fields; added InjectForgeVariables() to populate forge.prefix, forge.cli_tool, forge.pr_term, forge.pr_command in pipeline context
• Runtime resolution: ResolvePlaceholders() in the executor resolves template variables in step.Persona, step.Exec.SourcePath, and requires.tools before step execution
• Unified pipelines: Created implement, scope, research, rewrite, refresh, and pr-review pipelines with platform-agnostic prompts using forge variables
• Backward compatibility: ResolveDeprecatedName() maps old prefixed names (e.g., gh-implement → implement) with deprecation warnings
• Net deletion: -4,001 lines removed, +1,871 added — 53% reduction in pipeline configuration

Closes #241

Spec

See specs/241-unify-platform-pipelines/ for the full specification, research, implementation plan, and task breakdown.

Key Changes

Area	Files	What
Forge detection	internal/forge/detect.go	Add PRTerm, PRCommand to ForgeInfo
Pipeline context	internal/pipeline/context.go	InjectForgeVariables() for template variable population
Executor	internal/pipeline/executor.go	ResolvePlaceholders() for runtime template resolution
Deprecation	internal/pipeline/deprecated.go	ResolveDeprecatedName() with warning logs
Unified pipelines	internal/defaults/pipelines/*.yaml	6 unified pipelines replacing 25 platform-specific ones
Unified prompts	internal/defaults/prompts/implement/	Platform-agnostic prompt templates
Suggest engine	internal/suggest/engine.go	Prefer unified names over forge-prefixed
Preflight	internal/preflight/preflight.go	Resolve deprecated names before validation

Test Plan

• go test -race ./... — all 29 packages pass
• New unit tests for InjectForgeVariables() (6 test cases covering GitHub, GitLab, Bitbucket, Gitea, unknown, empty remote)
• New unit tests for ResolvePlaceholders() (placeholder resolution in persona, source path, tools)
• New unit tests for ResolveDeprecatedName() (all 10 deprecated → unified mappings + unknown passthrough)
• Updated embed_test.go to validate unified pipeline names
• Updated suggest engine tests for unified pipeline preferences
• Updated forge detection tests for new PRTerm/PRCommand fields

Known Limitations

• Template variables are resolved via simple string replacement, not a full template engine — sufficient for current {{ forge.* }} patterns but may need upgrading if more complex templating is needed later
• Deprecated name resolution logs warnings but does not surface them in the TUI yet

[nextlevelshit]

Code Review (Wave Pipeline)

Verdict: REQUEST_CHANGES

This PR consolidates 25+ forge-specific pipeline YAML files into unified pipelines using {{ forge.* }} template variables — a net deletion of ~2,130 lines. The core design is sound, tests pass, and the migration path is well-considered. However, one security finding and two completeness gaps should be addressed before merge.

---

Critical Issues (must fix)

1. Template-resolved source_path bypasses path validation before os.ReadFile (Security — HIGH)
internal/pipeline/executor.go: After ResolvePlaceholders() expands step.Exec.SourcePath, the result is passed directly to os.ReadFile without pathValidator.ValidatePath(). Other code paths (e.g., loadSecureSchemaContent) already validate paths. A misconfigured project.vars entry containing ../ could resolve to an arbitrary file read. Apply the same pathValidator.ValidatePath() check used elsewhere.

2. Deprecated name resolution not wired at pipeline loading layer (Quality/Security — HIGH)
ResolveDeprecatedName() is only called in cmd/wave/commands/run.go and preflight. Composition sub-pipelines (e.g., composition_test.go:287 references "gh-scope"), programmatic API callers, and saved TUI favorites will fail silently since the 25 deleted YAML files no longer exist. Wire deprecated name resolution at the pipeline loading layer so all entry points benefit, or update all sub-pipeline references to unified names.

3. CLAUDE.md references deprecated pipeline names (Quality — HIGH)
The root CLAUDE.md references gh-implement, gh-pr-review, gh-scope, and gh-research throughout the Pipeline Selection table, PR Review-Then-Merge Protocol, and Issue Triage sections. Since Wave agents read CLAUDE.md for orchestration instructions, they will attempt deprecated names. Update to unified names (implement, pr-review, scope, research) as part of this PR.

---

Suggested Improvements

• ResolveDeprecatedName() is overly broad — it strips any forge prefix (gh-, gl-, bb-, gt-) from any name, not just known deprecated pipelines. A user-created gh-deploy would be incorrectly resolved to deploy. Consider an explicit deprecated-name map or only triggering fallback when the original name fails to load.

• ForgeUnknown produces empty template variables — when forge detection fails, forge.cli_tool resolves to "", forge.prefix to "", causing persona lookups like "-committer" and silently skipped preflight tool checks. Fail fast or log a warning when forge variables are used but detection returned unknown.

• Duplicate replacement block in ResolvePlaceholders() — context.go lines 86-91 and 103-107 replace pipeline_id, pipeline_name, step_id twice. Functionally harmless but clearly accidental duplication — remove the second block.

• Add ForgeUnknown edge-case tests — no test verifies behavior when forge detection fails and template variables resolve to empty strings in persona, source_path, or requires.tools fields.

• Update test data — several test files (chatworkspace_test.go, resume_test.go, doctor_test.go, composition_test.go) reference deprecated forge-prefixed names in test data.

• Missing newline at end of context_test.go.

---

Positive Observations

• Excellent scope reduction — eliminating 25 duplicate pipeline files with a clean template system is a major DX win.
• Thorough test coverage — InjectForgeVariables tests cover all forge types, both {{ spaced }} and {{unspaced}} syntax, and mixed variable resolution.
• Graceful deprecation path — the CLI fallback with stderr warnings gives users a clear migration signal.
• Robust forge detection — tests cover SSH, HTTPS, ssh:// prefix, HTTP upgrade, empty URL, and malformed URL edge cases.
• All tests pass across all affected packages (pipeline, forge, suggest, defaults, preflight, doctor, tui, commands).

---

Generated by Wave gh-pr-review pipeline

[nextlevelshit]

Code Review (Wave Pipeline)

Verdict: REQUEST_CHANGES

This PR makes a strong architectural move — consolidating 25+ forge-specific pipeline/prompt files into 6 unified pipelines with {{ forge.* }} template variable substitution. The forge detection system is well-designed, the backward compatibility via ResolveDeprecatedName() is a nice touch, and test coverage for the new forge detection and variable injection is solid. However, a critical runtime-breaking bug and several high-severity gaps must be addressed before merging.

---

Critical Issues (must fix)

1. Persona naming mismatch breaks all 6 unified pipelines at runtime

Unified pipelines use {{ forge.prefix }}-commenter, {{ forge.prefix }}-analyst, etc. For GitHub, forge.prefix resolves to "gh", producing gh-commenter. But wave.yaml defines personas as github-commenter, github-analyst, gitlab-enhancer, bitbucket-scoper, etc.

Every step using {{ forge.prefix }}-* personas will fail with persona "gh-commenter" not found in manifest. Unit tests don't catch this because they mock the manifest.

Affected files: implement.yaml:100, pr-review.yaml:140, scope.yaml:16,63,127, refresh.yaml:16,73,130, research.yaml:16,208, rewrite.yaml:16,63

Fix options: (a) Add forge.persona_prefix mapping to full names (github, gitlab, bitbucket, gitea), (b) rename personas in wave.yaml to short prefixes, or (c) change forgeMetadata() to return full names as the prefix.

2. source_path template resolution bypasses PathValidator

executor.go:1748-1758 — ResolvePlaceholders is called on step.Exec.SourcePath before os.ReadFile(), but the resolved path skips the PathValidator check that is correctly applied to SchemaPath at line 2471. This is a defense-in-depth gap. Apply PathValidator.ValidatePath() to the resolved sourcePath.

3. Missing requires.tools in 4 of 6 unified pipelines

Only implement.yaml declares requires.tools: ["{{ forge.cli_tool }}", git]. scope.yaml, refresh.yaml, research.yaml, and rewrite.yaml all use {{ forge.cli_tool }} in prompts but don't declare the requirement, so preflight won't verify the CLI is installed.

4. ForgeUnknown silently degrades — no fail-fast behavior

When forge detection returns ForgeUnknown, all forge.* variables become empty strings. Persona resolution produces -commenter (fails), prompts produce bare commands like issue view <N> (broken), and preflight silently skips the empty tool name (preflight.go:86-88). The pipeline should fail early with a clear message when forge detection fails for pipelines that require forge variables.

5. CLAUDE.md references stale forge-prefixed pipeline names

CLAUDE.md:153-204 — The orchestrator reference still uses gh-implement, gh-pr-review throughout the Pipeline Selection table, PR Review-Then-Merge Protocol, Issue Triage table, and Post-Pipeline PR Validation. Since CLAUDE.md governs the Wave swarm orchestrator, stale names will cause deprecation warnings or failures.

---

Suggested Improvements

• Duplicate ResolvePlaceholders logic (context.go:88-92 and 105-107): pipeline_id, pipeline_name, and step_id are replaced twice. Remove the duplicate block at lines 104-107.
• Gitea forge classification is overly broad (detect.go:177): strings.Contains(host, "gitea") matches any hostname containing "gitea". Consider a suffix or more specific match.
• Three copies of the forge prefix list exist at deprecated.go:8, engine.go:330, and detect.go:202. Extract to a shared constant to avoid drift.
• pr-review.yaml:179 still references gh-pr-comment-result.schema.json — inconsistent with unification naming.
• Missing test coverage: No tests for InjectForgeVariables with ForgeUnknown, ForgeBitbucket, or ForgeGitea. No integration test verifying resolved persona names match actual manifest entries.
• executor.go:258: forge.DetectFromGitRemotes() error is silently discarded. Log it in debug mode.
• Bitbucket CLI metadata (detect.go:192): Returns "bb" as CLI tool, but prompts use curl with the REST API. If requires.tools is added, preflight would check for a nonexistent bb binary.

---

Positive Observations

• The forge detection system (internal/forge/detect.go) is well-structured with clear type definitions and comprehensive test coverage (13/13 TestDetect, 5/5 TestFilterPipelinesByForge).
• Template variable injection via PipelineContext is a clean abstraction that centralizes forge-specific logic.
• Backward compatibility through ResolveDeprecatedName() with 11/11 passing tests ensures a smooth transition for existing users.
• The consolidation from 25+ files to 6 unified pipelines is a significant maintenance win.
• All changed package tests pass (pipeline, forge, preflight, suggest, defaults).
• Security posture is generally solid — no critical vulnerabilities found, and the existing PathValidator/permission enforcement patterns are maintained for most code paths.

---

Generated by Wave gh-pr-review pipeline