From fa9f7a69d72da0eb212e831657f9a50c6484379d Mon Sep 17 00:00:00 2001 From: David Arce Date: Sun, 10 May 2026 16:41:03 +0200 Subject: [PATCH 1/2] fix(materialize/renderers): stop consuming REGISTRY for OpenCode/Copilot installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenCode and Copilot have a primary `sdd-orchestrator` agent the user explicitly selects (a custom agent in opencode.json / a `.agent.md` in .github/agents/). The orchestrator playbook lives in that agent's prompt — rendered from ORCHESTRATOR.{opencode,copilot}.md alone. Until now, the renderer was also: - prepending REGISTRY.{opencode,copilot}.md to the orchestrator prompt - capturing the same registry content into r.registryContents for later injection into AGENTS.md / copilot-instructions.md Both paths leaked workflow-specific rules into non-workflow sessions (ambient catalog) and duplicated / contradicted the orchestrator agent's own playbook (prompt prepend). They were a Claude/Factory pattern applied where it does not fit. This change removes both consumers for OpenCode and Copilot: - opencode.go: skip captureRegistryContent + pass registryPath="" to synthesizeAgents. - copilot.go: skip captureRegistryContent + pass registryPath="" to installOrchestratorAgent. buildOrchestratorEntry / installOrchestratorAgent still accept a registryPath argument for backward compatibility — non-empty paths are still read and prepended — but the canonical caller now passes "". Tests updated to assert the new contract (RegistryContents must NOT contain the workflow name for OpenCode/Copilot installs). Renames the two test functions accordingly. Pairs with devrune-starter-catalog change that deletes REGISTRY.opencode.md / REGISTRY.copilot.md (now dead code from the catalog's perspective) and slims ORCHESTRATOR.{opencode,copilot}.md to remove duplication with the deleted REGISTRY blocks. --- internal/materialize/renderers/copilot.go | 38 ++++++------- .../materialize/renderers/copilot_test.go | 29 +++++----- internal/materialize/renderers/opencode.go | 53 ++++++++----------- .../materialize/renderers/opencode_test.go | 27 +++++----- 4 files changed, 63 insertions(+), 84 deletions(-) diff --git a/internal/materialize/renderers/copilot.go b/internal/materialize/renderers/copilot.go index 6cd1192..2ca31db 100644 --- a/internal/materialize/renderers/copilot.go +++ b/internal/materialize/renderers/copilot.go @@ -474,20 +474,15 @@ func (r *CopilotRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath s if variantOrchPath != "" { effectiveSrc = variantOrchPath // use variant if found } - // Resolve the Copilot registry variant (or generic fallback) so the - // orchestrator agent file embeds both ambient rules + playbook — - // Copilot custom agents have no shared catalog context. - registryPath := "" - if wf.Components.Registry != "" { - variantName := strings.TrimSuffix(wf.Components.Registry, ".md") + ".copilot.md" - if _, statErr := os.Stat(filepath.Join(cachePath, variantName)); statErr == nil { - registryPath = filepath.Join(cachePath, variantName) - } else if _, statErr := os.Stat(filepath.Join(cachePath, wf.Components.Registry)); statErr == nil { - registryPath = filepath.Join(cachePath, wf.Components.Registry) - } - } + // REGISTRY blocks are intentionally NOT consumed for Copilot installs. + // Copilot has a primary `.agent.md` the user explicitly invokes; the + // playbook lives there (rendered from ORCHESTRATOR.copilot.md alone). + // Injecting REGISTRY content into the orchestrator body would duplicate + // or contradict the playbook. REGISTRY.copilot.md in the catalog is kept + // as an empty stub to prevent the variant resolver from falling back to + // the generic Claude-flavoured REGISTRY.md. dstPath := filepath.Join(agentsBase, orchRoleName+".agent.md") - if err := r.installOrchestratorAgent(effectiveSrc, registryPath, dstPath, wf, replacements); err != nil { + if err := r.installOrchestratorAgent(effectiveSrc, "", dstPath, wf, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: workflow orchestrator: %w", err) } managedPaths = append(managedPaths, dstPath) @@ -603,16 +598,13 @@ func (r *CopilotRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath s } } - // Capture registry content for catalog injection; apply shared placeholder replacements. - if wf.Components.Registry != "" { - content, err := captureRegistryContent(cachePath, wf.Components.Registry, replacements) - if err != nil { - return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: capture registry: %w", err) - } - if content != "" { - r.registryContents[wf.Metadata.Name] = content - } - } + // REGISTRY blocks are intentionally NOT captured for Copilot installs. + // Copilot has a primary `.agent.md` for the orchestrator (synthesized + // above); injecting the playbook into an ambient catalog would leak + // workflow-specific rules into non-workflow sessions and duplicate the + // orchestrator agent's body. r.registryContents stays empty for this + // workflow so RenderRootCatalog produces no SDD section in the ambient + // instructions file. // Resolve placeholders in all installed .md files under skillsBase and agentsBase. // agentsBase now contains the orchestrator .agent.md and the _shared/ directory. diff --git a/internal/materialize/renderers/copilot_test.go b/internal/materialize/renderers/copilot_test.go index 950fb51..b8b1f7f 100644 --- a/internal/materialize/renderers/copilot_test.go +++ b/internal/materialize/renderers/copilot_test.go @@ -490,11 +490,14 @@ func TestCopilotRenderer_InstallWorkflow_OrchestratorOnlyInAgentsDir(t *testing. } } -// TestCopilotRenderer_InstallWorkflow_RegistryInjectedIntoCatalog verifies that -// registry content is captured (for potential other use) but NOT injected verbatim -// into the catalog — instead a minimal orchestrator pointer is emitted. -// Also verifies no REGISTRY.md file is written anywhere in the workspace. -func TestCopilotRenderer_InstallWorkflow_RegistryInjectedIntoCatalog(t *testing.T) { +// TestCopilotRenderer_InstallWorkflow_RegistryNotCaptured verifies that +// registry content is NOT captured into the renderer's registryContents +// map for Copilot installs and is NOT copied loose anywhere in the +// workspace. Copilot has a primary `.agent.md` for the orchestrator +// (synthesized from ORCHESTRATOR.copilot.md); leaking REGISTRY content +// into the ambient instructions file would inject workflow-specific +// rules into non-workflow sessions. +func TestCopilotRenderer_InstallWorkflow_RegistryNotCaptured(t *testing.T) { workspaceRoot := t.TempDir() def := copilotParityDef(workspaceRoot) r := renderers.NewCopilotRenderer(def) @@ -529,18 +532,12 @@ func TestCopilotRenderer_InstallWorkflow_RegistryInjectedIntoCatalog(t *testing. t.Fatalf("InstallWorkflow: %v", err) } - // Verify registry content is captured (for later use by RenderRootCatalog). + // REGISTRY content must NOT be captured for Copilot — primary `.agent.md` + // owns the playbook; the ambient catalog should not carry workflow-specific + // orchestrator rules. contents := r.RegistryContents() - // Registry content is captured but not verbatim-injected (Copilot emits minimal pointer). - // The workflow name "sdd" must exist as a key. - if _, ok := contents[wf.Metadata.Name]; !ok { - t.Errorf("RegistryContents should contain captured content for workflow 'sdd'; got keys: %v", func() []string { - var keys []string - for k := range contents { - keys = append(keys, k) - } - return keys - }()) + if _, ok := contents[wf.Metadata.Name]; ok { + t.Errorf("RegistryContents must NOT contain workflow %q for Copilot installs; got captured content", wf.Metadata.Name) } // No loose REGISTRY.md should exist anywhere in workspace. diff --git a/internal/materialize/renderers/opencode.go b/internal/materialize/renderers/opencode.go index e38177d..d00b7bf 100644 --- a/internal/materialize/renderers/opencode.go +++ b/internal/materialize/renderers/opencode.go @@ -428,16 +428,22 @@ func (r *OpenCodeRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath // instead of the shared skillsBase path. replacements["{WORKFLOW_DIR}"] = workflowDir - // Capture registry content for catalog injection; apply shared replacements. - if wf.Components.Registry != "" { - content, err := captureRegistryContent(cachePath, wf.Components.Registry, replacements) - if err != nil { - return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: capture registry: %w", err) - } - if content != "" { - r.registryContents[wf.Metadata.Name] = content - } - } + // REGISTRY blocks are intentionally NOT consumed for OpenCode installs. + // + // Why: the REGISTRY pattern injects ambient orchestrator context into + // CLAUDE.md / AGENTS.md catalogs because Claude/Factory have no "primary + // agent" concept — the main session needs the playbook to know when to + // engage the workflow. OpenCode has a primary agent the user explicitly + // selects (synthesized from components.roles below); the playbook lives + // in that agent's prompt (rendered from ORCHESTRATOR.opencode.md alone). + // Injecting REGISTRY content into either AGENTS.md or the orchestrator + // prompt would (a) leak workflow-specific rules into non-workflow sessions + // and (b) duplicate / contradict the orchestrator prompt itself. + // + // We deliberately skip both the catalog capture (r.registryContents) and + // the prompt prepend (registryPath) here. REGISTRY.{opencode,copilot}.md + // in the catalog are kept as empty stubs to prevent the variant resolver + // from falling back to the generic Claude-flavoured REGISTRY.md. // Resolve placeholders in skill .md files under skillsBase. if err := resolvePlaceholders(skillsBase, replacements); err != nil { @@ -450,23 +456,10 @@ func (r *OpenCodeRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath } } - // Resolve registry variant for the orchestrator prompt (registry + - // playbook combined). The OpenCode primary agent has no separate ambient - // catalog, so the registry block is prepended to the orchestrator prompt - // to govern its behaviour. - registryPath := "" - if wf.Components.Registry != "" { - variantName := strings.TrimSuffix(wf.Components.Registry, ".md") + ".opencode.md" - if _, statErr := os.Stat(filepath.Join(cachePath, variantName)); statErr == nil { - registryPath = filepath.Join(cachePath, variantName) - } else if _, statErr := os.Stat(filepath.Join(cachePath, wf.Components.Registry)); statErr == nil { - registryPath = filepath.Join(cachePath, wf.Components.Registry) - } - } - // Synthesize SDD agents into opencode.json from components.roles. + // registryPath is empty by design (see REGISTRY-skip rationale above). if len(wf.Components.Roles) > 0 { - if err := r.synthesizeAgents(wf, orchPath, registryPath, workspaceRoot, skillsBase, replacements); err != nil { + if err := r.synthesizeAgents(wf, orchPath, "", workspaceRoot, skillsBase, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: synthesize agents: %w", err) } managedPaths = append(managedPaths, filepath.Join(workspaceRoot, "opencode.json")) @@ -590,14 +583,14 @@ func (r *OpenCodeRenderer) buildSubagentEntry(role model.WorkflowRole, skillsBas // buildOrchestratorEntry creates an opencode.json agent entry for the orchestrator role. // The prompt is the full content of the ORCHESTRATOR.md file with all placeholders // resolved ({SKILLS_PATH}, {SDD_MODEL_*}) using the provided replacements map. +// +// registryPath is accepted for signature compatibility but should be empty for +// OpenCode installs — REGISTRY blocks are not concatenated into the orchestrator +// prompt (see RenderWorkflow). If a non-empty path is supplied, it is read and +// prepended for backward compatibility, but the canonical caller passes "". func (r *OpenCodeRenderer) buildOrchestratorEntry(wf model.WorkflowManifest, role model.WorkflowRole, orchPath, registryPath string, replacements map[string]string) (map[string]any, error) { var prompt string - // Prepend the registry block (ambient rules: Role Invariant, Evaluation - // Gate, Memory Protocols, Engram Availability Guard, Session close) so the - // orchestrator agent has the same governing rules a CLAUDE.md ambient - // install would provide for the main session. OpenCode primary agents have - // no shared catalog context, so embedding here is the only delivery path. if registryPath != "" { regData, err := os.ReadFile(registryPath) if err != nil { diff --git a/internal/materialize/renderers/opencode_test.go b/internal/materialize/renderers/opencode_test.go index d78e97f..0499409 100644 --- a/internal/materialize/renderers/opencode_test.go +++ b/internal/materialize/renderers/opencode_test.go @@ -641,11 +641,13 @@ func TestOpenCodeRenderer_InstallWorkflow_NoAgentsDirCreated(t *testing.T) { } } -// TestOpenCodeRenderer_InstallWorkflow_RegistryInjectedIntoCatalog verifies that -// a workflow Registry file is NOT copied loose. After the post-review fix, registry -// content is also NOT injected verbatim into the catalog — a minimal orchestrator -// pointer is emitted instead. -func TestOpenCodeRenderer_InstallWorkflow_RegistryInjectedIntoCatalog(t *testing.T) { +// TestOpenCodeRenderer_InstallWorkflow_RegistryNotCaptured verifies that +// a workflow Registry file is NOT copied loose AND NOT captured into the +// renderer's registryContents map for OpenCode installs. OpenCode primary +// agents own their playbook in opencode.json — leaking the REGISTRY block +// into AGENTS.md would inject workflow-specific rules into non-workflow +// sessions and duplicate the orchestrator agent's prompt. +func TestOpenCodeRenderer_InstallWorkflow_RegistryNotCaptured(t *testing.T) { projectRoot := t.TempDir() workspaceRoot := filepath.Join(projectRoot, ".opencode") if err := os.MkdirAll(workspaceRoot, 0o755); err != nil { @@ -706,17 +708,12 @@ components: t.Fatalf("InstallWorkflow: %v", err) } - // Registry content is captured in the renderer for later use by RenderRootCatalog. + // REGISTRY content must NOT be captured for OpenCode — primary agents own + // their playbook in opencode.json; AGENTS.md should not carry the workflow's + // orchestrator rules. contents := r.RegistryContents() - // The workflow name "sdd" must exist as a key. - if _, ok := contents[wf.Metadata.Name]; !ok { - t.Errorf("RegistryContents should contain captured content for workflow 'sdd'; got keys: %v", func() []string { - var keys []string - for k := range contents { - keys = append(keys, k) - } - return keys - }()) + if _, ok := contents[wf.Metadata.Name]; ok { + t.Errorf("RegistryContents must NOT contain workflow %q for OpenCode installs; got captured content", wf.Metadata.Name) } // NEGATIVE: REGISTRY.md must NOT exist in .agents/skills/. From 1dcf7e5ad6039a9c31f2df01c056f984013f8887 Mon Sep 17 00:00:00 2001 From: David Arce Date: Sun, 10 May 2026 17:05:02 +0200 Subject: [PATCH 2/2] refactor(materialize/renderers): introduce {SHARED_DIR} placeholder + move _shared/ for primary-agent installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a {SHARED_DIR} placeholder that the catalog uses to reference workflow-shared assets (launch-templates, advisor-templates, envelope- contract, persistence-contract, recovery). Each renderer resolves it to its idiomatic path so the same template body works for all agents. Path resolution per agent: - claude/factory/codex: workflowDir/_shared/ (legacy — _shared/ stays inside the orchestrator skill dir, which is a real skill on disk). - opencode/copilot: workspaceRoot//_shared/ — a workflow-namespaced top-level directory. The legacy .opencode/sdd-orchestrator/ and .github/skills/sdd-orchestrator/ directories were fantasma containers (no SKILL.md, the orchestrator is a primary agent in opencode.json / a flat .agent.md). Installing _shared/ at the top level removes that fantasma directory entirely. Code changes: - helpers.go: add {SHARED_DIR} to both buildWorkflowPlaceholderReplacements (used by opencode/copilot/claude/factory) and buildWorkflowPathReplacements (used by codex/factory). Each picks the right resolution based on agentName and workflow.Name. - opencode.go: change _shared/ copy destination to workspaceRoot//_shared/. Add resolvePlaceholders pass for the new shared root. Keep legacy workflowDir resolve as a no-op fallback for backward compat. - copilot.go: same pattern, destination .github//_shared/. Tests: - TestOpenCodeRenderer_InstallWorkflow_SkillsUnderSkillsDir / Copilot equivalent: assert _shared/ at the new path AND assert legacy fantasma paths are NOT created. - TestOpenCodeRenderer_InstallWorkflow_SharedVariantSuffixStripping / Copilot equivalent: same path update. - TestBuildWorkflowPathReplacements_OnlyPathKeys: now expects 3 keys ({SKILLS_PATH}, {WORKFLOW_DIR}, {SHARED_DIR}) instead of 2. - TestBuildWorkflowPathReplacements_NoModelKeysEvenWithRoles: allowed key set extended to include {SHARED_DIR}. Pairs with devrune-starter-catalog refactor that updates all templates to use {SHARED_DIR} instead of {WORKFLOW_DIR}/_shared/. --- internal/materialize/renderers/copilot.go | 33 ++++++++++++++----- .../materialize/renderers/copilot_test.go | 25 ++++++++------ internal/materialize/renderers/helpers.go | 22 +++++++++++++ .../materialize/renderers/helpers_test.go | 24 +++++++++++--- internal/materialize/renderers/opencode.go | 30 +++++++++++++---- .../materialize/renderers/opencode_test.go | 21 +++++++++--- 6 files changed, 120 insertions(+), 35 deletions(-) diff --git a/internal/materialize/renderers/copilot.go b/internal/materialize/renderers/copilot.go index 2ca31db..2c48aa5 100644 --- a/internal/materialize/renderers/copilot.go +++ b/internal/materialize/renderers/copilot.go @@ -517,19 +517,26 @@ func (r *CopilotRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath s continue } - // Copy everything else (e.g. _shared/) under skillsBase// - // so that the orchestrator .agent.md can reference them via paths such as - // .github/skills/sdd-orchestrator/_shared/launch-templates.md. - // In Copilot's model, agents/ is flat (.agent.md files only); content lives in skills/. - // Apply variant-suffix stripping for _shared/ so that launch-templates.copilot.md → + // _shared/ is a workflow-level asset (launch-templates, advisor-templates, + // envelope-contract, persistence-contract, recovery). For Copilot it + // installs to .github//_shared/ — outside the legacy + // skills// container which is empty for primary-agent installs + // (Copilot delivers the orchestrator as a flat .agent.md). + // In Copilot's model, agents/ is flat (.agent.md files only); content + // lives in skills/ for actual sub-agent skills. Apply variant-suffix + // stripping for _shared/ so that launch-templates.copilot.md → // launch-templates.md and files for other variants are skipped entirely. - dstPath := filepath.Join(orchSkillDir, name) + var dstPath string if entry.IsDir() && name == "_shared" { + dstPath = filepath.Join(workspaceRoot, wf.Metadata.Name, "_shared") if err := copyDirRecursiveStripVariant(srcPath, dstPath, "copilot"); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: workflow copy %q: %w", name, err) } - } else if err := copyEntry(srcPath, dstPath, entry); err != nil { - return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: workflow copy %q: %w", name, err) + } else { + dstPath = filepath.Join(orchSkillDir, name) + if err := copyEntry(srcPath, dstPath, entry); err != nil { + return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: workflow copy %q: %w", name, err) + } } managedPaths = append(managedPaths, dstPath) } @@ -607,13 +614,21 @@ func (r *CopilotRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath s // instructions file. // Resolve placeholders in all installed .md files under skillsBase and agentsBase. - // agentsBase now contains the orchestrator .agent.md and the _shared/ directory. if err := resolvePlaceholders(skillsBase, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: resolve placeholders (skills): %w", err) } if err := resolvePlaceholders(agentsBase, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: resolve placeholders (agents): %w", err) } + // Resolve placeholders in the workflow's shared assets — _shared/ now lives + // at .github//_shared/, outside the legacy skills// + // container. + workflowSharedRoot := filepath.Join(workspaceRoot, wf.Metadata.Name) + if _, statErr := os.Stat(workflowSharedRoot); statErr == nil { + if err := resolvePlaceholders(workflowSharedRoot, replacements); err != nil { + return matypes.WorkflowInstallResult{}, fmt.Errorf("copilot: resolve workflow shared placeholders: %w", err) + } + } // Remove any lines containing unresolved {SDD_MODEL_*} or {WORKFLOW_MODEL_*} placeholders. // These correspond to phases where the user selected "inherit from session" (no override). diff --git a/internal/materialize/renderers/copilot_test.go b/internal/materialize/renderers/copilot_test.go index b8b1f7f..fb2352b 100644 --- a/internal/materialize/renderers/copilot_test.go +++ b/internal/materialize/renderers/copilot_test.go @@ -398,19 +398,24 @@ func TestCopilotRenderer_InstallWorkflow_SkillsUnderSkillsDir(t *testing.T) { t.Errorf("expected %s to exist: %v", skillMD, err) } - // POSITIVE: _shared directory under skills/sdd-orchestrator/_shared - // (matches the paths the orchestrator .agent.md references, e.g. - // .github/skills/sdd-orchestrator/_shared/launch-templates.md) - sharedDest := filepath.Join(workspaceRoot, "skills", "sdd-orchestrator", "_shared") + // POSITIVE: _shared installed at the workflow-namespaced top-level path + // .github//_shared/. The orchestrator .agent.md references + // it via {SHARED_DIR}, which the renderer resolves to this path. + sharedDest := filepath.Join(workspaceRoot, wf.Metadata.Name, "_shared") if info, err := os.Stat(sharedDest); err != nil || !info.IsDir() { t.Errorf("expected %s to be a directory: err=%v", sharedDest, err) } - // NEGATIVE: _shared must NOT be installed under agents/sdd-orchestrator/ - // (agents/ is flat: only .agent.md files, no subdirectories) - sharedInAgents := filepath.Join(workspaceRoot, "agents", "sdd-orchestrator", "_shared") - if _, err := os.Stat(sharedInAgents); err == nil { - t.Error("_shared must NOT exist under agents/sdd-orchestrator/ for Copilot — it belongs in skills/sdd-orchestrator/") + // NEGATIVE: legacy paths must NOT be created — the orchestrator skill dir + // in skills/ was a fantasma container (no SKILL.md), and agents/ is flat + // (only .agent.md files, no subdirectories). + for _, legacy := range []string{ + filepath.Join(workspaceRoot, "skills", "sdd-orchestrator", "_shared"), + filepath.Join(workspaceRoot, "agents", "sdd-orchestrator", "_shared"), + } { + if _, err := os.Stat(legacy); err == nil { + t.Errorf("legacy fantasma path %s should NOT be created for Copilot installs", legacy) + } } // POSITIVE: orchestrator surfaced as native .agent.md in agents/ @@ -2062,7 +2067,7 @@ func TestCopilotRenderer_InstallWorkflow_SharedVariantSuffixStripping(t *testing t.Fatalf("InstallWorkflow: %v", err) } - installedShared := filepath.Join(workspaceRoot, "skills", "sdd-orchestrator", "_shared") + installedShared := filepath.Join(workspaceRoot, wf.Metadata.Name, "_shared") // launch-templates.md must exist with copilot content (from launch-templates.copilot.md). ltPath := filepath.Join(installedShared, "launch-templates.md") diff --git a/internal/materialize/renderers/helpers.go b/internal/materialize/renderers/helpers.go index 9982132..aee7fcc 100644 --- a/internal/materialize/renderers/helpers.go +++ b/internal/materialize/renderers/helpers.go @@ -798,9 +798,27 @@ func buildWorkflowPlaceholderReplacements( workflowDir = skillsPath } + // {SHARED_DIR} resolves to the directory where the renderer copies the + // workflow's _shared/ assets. For Claude/Factory/Codex this stays inside + // the orchestrator skill (workflowDir/_shared) — `_shared/` legitimately + // belongs to the orchestrator skill there. For OpenCode/Copilot the + // orchestrator is a primary agent (not a skill on disk), so we install + // _shared/ at workspaceDir//_shared/ — a top-level + // workflow-namespaced location with no fantasma orchestrator skill dir. + // The catalog uses {SHARED_DIR} in template references; each renderer + // resolves it to its idiomatic path so the same template works for all. + var sharedDir string + switch agentName { + case "opencode", "copilot": + sharedDir = filepath.Clean(workspaceDir + "/" + wf.Metadata.Name + "/_shared") + default: + sharedDir = filepath.Clean(workflowDir + "/_shared") + } + replacements := map[string]string{ "{SKILLS_PATH}": skillsPath, "{WORKFLOW_DIR}": workflowDir, + "{SHARED_DIR}": sharedDir, } wfName := wf.Metadata.Name @@ -887,6 +905,10 @@ func buildWorkflowPathReplacements(wf model.WorkflowManifest, workspaceDir, skil replacements := map[string]string{ "{SKILLS_PATH}": skillsPath, "{WORKFLOW_DIR}": workflowDir, + // Codex/Factory keep _shared/ inside the orchestrator skill dir + // (their orchestrator IS a real skill on disk). Same path the + // legacy {WORKFLOW_DIR}/_shared/ resolved to. + "{SHARED_DIR}": filepath.Clean(workflowDir + "/_shared"), } // Add subagent placeholders defaulting to "general" for renderers without native agents. wfName := wf.Metadata.Name diff --git a/internal/materialize/renderers/helpers_test.go b/internal/materialize/renderers/helpers_test.go index 3f5e178..b135ffa 100644 --- a/internal/materialize/renderers/helpers_test.go +++ b/internal/materialize/renderers/helpers_test.go @@ -905,13 +905,14 @@ func TestBuildWorkflowPlaceholderReplacements_OpenCodeResolver(t *testing.T) { // Tests for buildWorkflowPathReplacements // --------------------------------------------------------------------------- -// TestBuildWorkflowPathReplacements_OnlySkillsPath verifies that the function -// returns only a {SKILLS_PATH} entry and no {SDD_MODEL_*} entries. +// TestBuildWorkflowPathReplacements_OnlyPathKeys verifies that the function +// returns only path entries ({SKILLS_PATH}, {WORKFLOW_DIR}, {SHARED_DIR}) and +// no {SDD_MODEL_*} entries. func TestBuildWorkflowPathReplacements_OnlyPathKeys(t *testing.T) { result := renderers.BuildWorkflowPathReplacements(model.WorkflowManifest{}, "/ws", "skills") - if len(result) != 2 { - t.Errorf("expected exactly 2 replacements, got %d: %v", len(result), result) + if len(result) != 3 { + t.Errorf("expected exactly 3 replacements, got %d: %v", len(result), result) } got, ok := result["{SKILLS_PATH}"] if !ok { @@ -928,6 +929,15 @@ func TestBuildWorkflowPathReplacements_OnlyPathKeys(t *testing.T) { if gotWD != "/ws/skills" { t.Errorf("{WORKFLOW_DIR} = %q, want %q (empty workingDir should yield skillsPath)", gotWD, "/ws/skills") } + // SHARED_DIR is WORKFLOW_DIR/_shared for renderers without per-agent + // orchestrator pattern (Codex/Factory). + gotSD, ok := result["{SHARED_DIR}"] + if !ok { + t.Fatal("{SHARED_DIR} key missing from result") + } + if gotSD != "/ws/skills/_shared" { + t.Errorf("{SHARED_DIR} = %q, want %q (workflowDir/_shared for non-primary-agent renderers)", gotSD, "/ws/skills/_shared") + } } // TestBuildWorkflowPathReplacements_EmptySkillDir uses workspaceDir directly when skillDir is empty. @@ -955,7 +965,11 @@ func TestBuildWorkflowPathReplacements_TrailingSlashStripped(t *testing.T) { // regardless of the workflow manifest roles provided. func TestBuildWorkflowPathReplacements_NoModelKeysEvenWithRoles(t *testing.T) { result := renderers.BuildWorkflowPathReplacements(model.WorkflowManifest{}, "/project", "agents") - allowedKeys := map[string]bool{"{SKILLS_PATH}": true, "{WORKFLOW_DIR}": true} + allowedKeys := map[string]bool{ + "{SKILLS_PATH}": true, + "{WORKFLOW_DIR}": true, + "{SHARED_DIR}": true, + } for key := range result { if !allowedKeys[key] { t.Errorf("unexpected key %q — buildWorkflowPathReplacements must only produce path keys", key) diff --git a/internal/materialize/renderers/opencode.go b/internal/materialize/renderers/opencode.go index d00b7bf..e771bca 100644 --- a/internal/materialize/renderers/opencode.go +++ b/internal/materialize/renderers/opencode.go @@ -376,17 +376,25 @@ func (r *OpenCodeRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath continue } - // Copy everything else (e.g. _shared/) as-is under workflowDir. - // agents/ and commands/ directories are NOT created — OpenCode uses opencode.json. + // _shared/ is a workflow-level asset (launch-templates, advisor-templates, + // envelope-contract, persistence-contract, recovery). For OpenCode it + // installs to .opencode//_shared/ — outside the legacy + // .opencode// directory which is empty for primary-agent + // installs (no SKILL.md, the orchestrator lives in opencode.json). + // agents/ and commands/ directories are NOT created. // Apply variant-suffix stripping for _shared/ so that launch-templates.opencode.md → // launch-templates.md and files for other variants are skipped entirely. - dstPath := filepath.Join(workflowDir, name) + var dstPath string if entry.IsDir() && name == "_shared" { + dstPath = filepath.Join(workspaceRoot, wf.Metadata.Name, "_shared") if err := copyDirRecursiveStripVariant(srcPath, dstPath, "opencode"); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: workflow copy %q: %w", name, err) } - } else if err := copyEntry(srcPath, dstPath, entry); err != nil { - return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: workflow copy %q: %w", name, err) + } else { + dstPath = filepath.Join(workflowDir, name) + if err := copyEntry(srcPath, dstPath, entry); err != nil { + return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: workflow copy %q: %w", name, err) + } } managedPaths = append(managedPaths, dstPath) } @@ -449,7 +457,17 @@ func (r *OpenCodeRenderer) InstallWorkflow(wf model.WorkflowManifest, cachePath if err := resolvePlaceholders(skillsBase, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: resolve skill placeholders: %w", err) } - // Resolve placeholders in workflow files under the workspace-local workflowDir (if it exists). + // Resolve placeholders in the workflow's shared assets (_shared/ now lives at + // the workflow-namespaced top-level path .opencode//_shared/, + // not inside a per-orchestrator skill dir). + workflowSharedRoot := filepath.Join(workspaceRoot, wf.Metadata.Name) + if _, statErr := os.Stat(workflowSharedRoot); statErr == nil { + if err := resolvePlaceholders(workflowSharedRoot, replacements); err != nil { + return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: resolve workflow shared placeholders: %w", err) + } + } + // Backward-compat: a few catalogs may still drop content under workflowDir + // (e.g. legacy fixtures); resolve placeholders there too if it exists. if _, statErr := os.Stat(workflowDir); statErr == nil { if err := resolvePlaceholders(workflowDir, replacements); err != nil { return matypes.WorkflowInstallResult{}, fmt.Errorf("opencode: resolve workflow placeholders: %w", err) diff --git a/internal/materialize/renderers/opencode_test.go b/internal/materialize/renderers/opencode_test.go index 0499409..9daad5a 100644 --- a/internal/materialize/renderers/opencode_test.go +++ b/internal/materialize/renderers/opencode_test.go @@ -423,7 +423,8 @@ func sddParityManifest() model.WorkflowManifest { } // TestOpenCodeRenderer_InstallWorkflow_SkillsUnderSkillsDir verifies that workflow -// skills are installed under .agents/skills/, _shared/ is also copied there, +// skills are installed under .agents/skills/, _shared/ is installed at the +// workflow-namespaced top-level path .opencode//_shared/, // and the old buggy agents/ path is never created. func TestOpenCodeRenderer_InstallWorkflow_SkillsUnderSkillsDir(t *testing.T) { projectRoot := t.TempDir() @@ -457,8 +458,11 @@ func TestOpenCodeRenderer_InstallWorkflow_SkillsUnderSkillsDir(t *testing.T) { t.Errorf("expected %s to exist: %v", skillMD, err) } - // POSITIVE: _shared/ directory installed under .opencode/sdd-orchestrator/ (workspace-local, not shared) - sharedDir := filepath.Join(workspaceDir, "sdd-orchestrator", "_shared") + // POSITIVE: _shared/ installed at workflow-namespaced top-level path + // .opencode//_shared/ (NOT inside the legacy + // .opencode// container — the orchestrator is a primary agent + // in opencode.json, so there is no fantasma sdd-orchestrator skill dir). + sharedDir := filepath.Join(workspaceDir, wf.Metadata.Name, "_shared") info, err := os.Stat(sharedDir) if err != nil { t.Errorf("expected %s to exist: %v", sharedDir, err) @@ -466,6 +470,12 @@ func TestOpenCodeRenderer_InstallWorkflow_SkillsUnderSkillsDir(t *testing.T) { t.Errorf("expected %s to be a directory", sharedDir) } + // NEGATIVE: the legacy fantasma path must NOT be created. + legacyShared := filepath.Join(workspaceDir, "sdd-orchestrator", "_shared") + if _, err := os.Stat(legacyShared); err == nil { + t.Errorf("legacy fantasma path %s should NOT be created for OpenCode installs", legacyShared) + } + // POSITIVE: opencode.json synthesized (from roles) opencodeJSON := filepath.Join(workspaceDir, "opencode.json") if _, err := os.Stat(opencodeJSON); err != nil { @@ -1437,8 +1447,9 @@ func TestOpenCodeRenderer_InstallWorkflow_SharedVariantSuffixStripping(t *testin t.Fatalf("InstallWorkflow: %v", err) } - // OpenCode installs _shared/ under .opencode/sdd-orchestrator/_shared/. - installedShared := filepath.Join(workspaceDir, "sdd-orchestrator", "_shared") + // OpenCode installs _shared/ at the workflow-namespaced top-level path + // .opencode//_shared/. + installedShared := filepath.Join(workspaceDir, wf.Metadata.Name, "_shared") // launch-templates.md must exist with opencode content (from launch-templates.opencode.md). ltPath := filepath.Join(installedShared, "launch-templates.md")