diff --git a/forge-cli/build/channels_stage.go b/forge-cli/build/channels_stage.go new file mode 100644 index 0000000..2be0d2b --- /dev/null +++ b/forge-cli/build/channels_stage.go @@ -0,0 +1,77 @@ +package build + +import ( + "context" + "fmt" + "sort" + + clichannels "github.com/initializ/forge/forge-cli/channels" + "github.com/initializ/forge/forge-core/agentspec" + "github.com/initializ/forge/forge-core/pipeline" +) + +// ChannelsStage unions env var names declared by the project's configured +// communication channels into Spec.Requirements.EnvRequired so the generated +// Kubernetes secrets and deployment manifests include them alongside skill +// env vars. +// +// The canonical source is the per-channel YAML (workDir/-config.yaml) +// — every setting key ending in "_env" declares an env-var name. Adding a new +// channel adapter that ships its own config template will pick up here with +// no edits to this file. +type ChannelsStage struct{} + +func (s *ChannelsStage) Name() string { return "channel-env-vars" } + +func (s *ChannelsStage) Execute(ctx context.Context, bc *pipeline.BuildContext) error { + if bc.Config == nil || len(bc.Config.Channels) == 0 { + return nil + } + + channelEnv, missing, err := clichannels.EnvVarsFromConfig(bc.Opts.WorkDir, bc.Config.Channels) + if err != nil { + return fmt.Errorf("reading channel env vars: %w", err) + } + for _, name := range missing { + bc.AddWarning(fmt.Sprintf("channel %q is configured but %s-config.yaml is missing; its env vars will not be included in the generated manifests", name, name)) + } + if len(channelEnv) == 0 { + return nil + } + + if bc.Spec == nil { + return nil + } + if bc.Spec.Requirements == nil { + bc.Spec.Requirements = &agentspec.AgentRequirements{} + } + + // Union with existing skill-required env vars, dedup, sort. + seen := make(map[string]bool, len(bc.Spec.Requirements.EnvRequired)+len(channelEnv)) + merged := make([]string, 0, len(bc.Spec.Requirements.EnvRequired)+len(channelEnv)) + for _, v := range bc.Spec.Requirements.EnvRequired { + if seen[v] { + continue + } + seen[v] = true + merged = append(merged, v) + } + // Skill-declared optional env vars stay optional even if a channel marks + // them required — but in practice channel and skill env-var namespaces + // don't overlap, so this is just defense-in-depth. + optional := make(map[string]bool, len(bc.Spec.Requirements.EnvOptional)) + for _, v := range bc.Spec.Requirements.EnvOptional { + optional[v] = true + } + for _, v := range channelEnv { + if seen[v] || optional[v] { + continue + } + seen[v] = true + merged = append(merged, v) + } + sort.Strings(merged) + bc.Spec.Requirements.EnvRequired = merged + + return nil +} diff --git a/forge-cli/build/channels_stage_test.go b/forge-cli/build/channels_stage_test.go new file mode 100644 index 0000000..1ac2fd8 --- /dev/null +++ b/forge-cli/build/channels_stage_test.go @@ -0,0 +1,176 @@ +package build + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/initializ/forge/forge-core/agentspec" + "github.com/initializ/forge/forge-core/pipeline" + "github.com/initializ/forge/forge-core/types" +) + +func writeChannelYAML(t *testing.T, dir, name, content string) { + t.Helper() + path := filepath.Join(dir, name+"-config.yaml") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("writing %s: %v", path, err) + } +} + +func TestChannelsStage_UnionsWithSkillEnvRequired(t *testing.T) { + dir := t.TempDir() + writeChannelYAML(t, dir, "slack", ` +adapter: slack +settings: + app_token_env: SLACK_APP_TOKEN + bot_token_env: SLACK_BOT_TOKEN +`) + writeChannelYAML(t, dir, "telegram", ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN +`) + + bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: dir}) + bc.Config = &types.ForgeConfig{Channels: []string{"slack", "telegram"}} + bc.Spec = &agentspec.AgentSpec{ + Requirements: &agentspec.AgentRequirements{ + EnvRequired: []string{"SKILL_API_KEY"}, + }, + } + + if err := (&ChannelsStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("ChannelsStage.Execute: %v", err) + } + + want := []string{"SKILL_API_KEY", "SLACK_APP_TOKEN", "SLACK_BOT_TOKEN", "TELEGRAM_BOT_TOKEN"} + if !slices.Equal(bc.Spec.Requirements.EnvRequired, want) { + t.Errorf("EnvRequired = %v, want %v", bc.Spec.Requirements.EnvRequired, want) + } +} + +func TestChannelsStage_PopulatesRequirementsWhenNil(t *testing.T) { + // Project with channels but no skills — Spec.Requirements starts nil + // because RequirementsStage early-returns. ChannelsStage must still + // surface channel env vars to the manifests. + dir := t.TempDir() + writeChannelYAML(t, dir, "slack", ` +adapter: slack +settings: + bot_token_env: SLACK_BOT_TOKEN +`) + + bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: dir}) + bc.Config = &types.ForgeConfig{Channels: []string{"slack"}} + bc.Spec = &agentspec.AgentSpec{} // Requirements nil + + if err := (&ChannelsStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if bc.Spec.Requirements == nil { + t.Fatal("Requirements should be created when channels declare env vars") + } + if !slices.Equal(bc.Spec.Requirements.EnvRequired, []string{"SLACK_BOT_TOKEN"}) { + t.Errorf("EnvRequired = %v, want [SLACK_BOT_TOKEN]", bc.Spec.Requirements.EnvRequired) + } +} + +func TestChannelsStage_NoChannels(t *testing.T) { + bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: t.TempDir()}) + bc.Config = &types.ForgeConfig{} + bc.Spec = &agentspec.AgentSpec{} + + if err := (&ChannelsStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if bc.Spec.Requirements != nil { + t.Error("expected Requirements to stay nil for project with no channels") + } +} + +// TestChannelsStage_FlowsThroughToK8sManifests is the end-to-end regression +// for issue #50: channel env vars must appear in the generated K8s deployment +// and secret manifests, not only in docker-compose. +func TestChannelsStage_FlowsThroughToK8sManifests(t *testing.T) { + workDir := t.TempDir() + outDir := t.TempDir() + writeChannelYAML(t, workDir, "slack", ` +adapter: slack +settings: + app_token_env: SLACK_APP_TOKEN + bot_token_env: SLACK_BOT_TOKEN +`) + writeChannelYAML(t, workDir, "telegram", ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN +`) + + bc := pipeline.NewBuildContext(pipeline.PipelineOptions{ + WorkDir: workDir, + OutputDir: outDir, + }) + bc.Config = &types.ForgeConfig{Channels: []string{"slack", "telegram"}} + bc.Spec = &agentspec.AgentSpec{ + AgentID: "test-agent", + Version: "0.1.0", + Runtime: &agentspec.RuntimeConfig{ + Image: "python:3.12-slim", + Entrypoint: []string{"python", "agent.py"}, + Port: 8080, + }, + } + + if err := (&ChannelsStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("ChannelsStage.Execute: %v", err) + } + if err := (&K8sStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("K8sStage.Execute: %v", err) + } + + dep := readFile(t, filepath.Join(outDir, "k8s", "deployment.yaml")) + sec := readFile(t, filepath.Join(outDir, "k8s", "secrets.yaml")) + + for _, want := range []string{"SLACK_APP_TOKEN", "SLACK_BOT_TOKEN", "TELEGRAM_BOT_TOKEN"} { + if !strings.Contains(dep, want) { + t.Errorf("deployment.yaml missing channel env var %q", want) + } + if !strings.Contains(sec, want) { + t.Errorf("secrets.yaml missing channel env var %q", want) + } + } + if !strings.Contains(dep, "secretKeyRef:") { + t.Error("deployment.yaml should reference channel env vars via secretKeyRef") + } +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", path, err) + } + return string(b) +} + +func TestChannelsStage_MissingConfigWarns(t *testing.T) { + // channels: [slack] declared, but no slack-config.yaml on disk. + // The stage must surface a warning and not fail the build. + bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: t.TempDir()}) + bc.Config = &types.ForgeConfig{Channels: []string{"slack"}} + bc.Spec = &agentspec.AgentSpec{} + + if err := (&ChannelsStage{}).Execute(context.Background(), bc); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(bc.Warnings) != 1 || !strings.Contains(bc.Warnings[0], "slack") { + t.Errorf("expected one warning mentioning slack, got %v", bc.Warnings) + } + if bc.Spec.Requirements != nil { + t.Error("Requirements should stay nil when no channel env vars discovered") + } +} diff --git a/forge-cli/channels/env.go b/forge-cli/channels/env.go new file mode 100644 index 0000000..87ce54c --- /dev/null +++ b/forge-cli/channels/env.go @@ -0,0 +1,55 @@ +package channels + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// EnvVarsFromConfig returns the sorted, deduped union of env-var names that +// the configured channel adapters require. The canonical source is the +// project's per-channel YAML — every setting key ending in "_env" declares +// an env var name (e.g. "bot_token_env: SLACK_BOT_TOKEN"), matching the +// runtime contract used by channels.ResolveEnvVars. +// +// channelNames are the values from forge.yaml's `channels:` list. For each +// name, the file workDir/-config.yaml is consulted. A missing file is +// reported via missing[] and produces no env vars; parse errors are returned. +// +// This is the single canonical source — build stages, container packaging, +// and any other tooling that needs to know "which env vars do my channels +// require" should call this. Adding a new channel adapter requires no edits +// here: it ships its own *-config.yaml template and the helper picks it up. +func EnvVarsFromConfig(workDir string, channelNames []string) (envVars []string, missing []string, err error) { + seen := make(map[string]bool) + for _, name := range channelNames { + name = strings.TrimSpace(name) + if name == "" { + continue + } + path := filepath.Join(workDir, name+"-config.yaml") + if _, statErr := os.Stat(path); os.IsNotExist(statErr) { + missing = append(missing, name) + continue + } + cfg, loadErr := LoadChannelConfig(path) + if loadErr != nil { + return nil, missing, fmt.Errorf("channel %q: %w", name, loadErr) + } + for k, v := range cfg.Settings { + base, ok := strings.CutSuffix(k, "_env") + if !ok || base == "" { + continue + } + if v == "" || seen[v] { + continue + } + seen[v] = true + envVars = append(envVars, v) + } + } + sort.Strings(envVars) + return envVars, missing, nil +} diff --git a/forge-cli/channels/env_test.go b/forge-cli/channels/env_test.go new file mode 100644 index 0000000..fbff235 --- /dev/null +++ b/forge-cli/channels/env_test.go @@ -0,0 +1,127 @@ +package channels + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +func writeChannelConfig(t *testing.T, dir, name, content string) { + t.Helper() + path := filepath.Join(dir, name+"-config.yaml") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("writing %s: %v", path, err) + } +} + +func TestEnvVarsFromConfig_ExtractsEnvSuffixSettings(t *testing.T) { + dir := t.TempDir() + writeChannelConfig(t, dir, "slack", ` +adapter: slack +settings: + app_token_env: SLACK_APP_TOKEN + bot_token_env: SLACK_BOT_TOKEN +`) + writeChannelConfig(t, dir, "telegram", ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN + mode: polling +`) + + got, missing, err := EnvVarsFromConfig(dir, []string{"slack", "telegram"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(missing) != 0 { + t.Errorf("missing = %v, want none", missing) + } + want := []string{"SLACK_APP_TOKEN", "SLACK_BOT_TOKEN", "TELEGRAM_BOT_TOKEN"} + if !slices.Equal(got, want) { + t.Errorf("EnvVarsFromConfig = %v, want %v", got, want) + } +} + +func TestEnvVarsFromConfig_IgnoresNonEnvSettings(t *testing.T) { + dir := t.TempDir() + writeChannelConfig(t, dir, "telegram", ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN + mode: polling + webhook_path: /tg +`) + + got, _, err := EnvVarsFromConfig(dir, []string{"telegram"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Only the _env-suffix key should be picked up; mode/webhook_path are not env names. + if !slices.Equal(got, []string{"TELEGRAM_BOT_TOKEN"}) { + t.Errorf("got %v, want [TELEGRAM_BOT_TOKEN] (non-env settings should be ignored)", got) + } +} + +func TestEnvVarsFromConfig_DedupsAcrossChannels(t *testing.T) { + dir := t.TempDir() + // Two channels declaring the same env var name (e.g. two slack-like + // adapters sharing a credential). The union must dedup. + writeChannelConfig(t, dir, "slack", ` +adapter: slack +settings: + bot_token_env: SHARED_TOKEN +`) + writeChannelConfig(t, dir, "slack-replica", ` +adapter: slack +settings: + bot_token_env: SHARED_TOKEN +`) + + got, _, err := EnvVarsFromConfig(dir, []string{"slack", "slack-replica"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !slices.Equal(got, []string{"SHARED_TOKEN"}) { + t.Errorf("dedup failed: got %v", got) + } +} + +func TestEnvVarsFromConfig_MissingFileReported(t *testing.T) { + dir := t.TempDir() + writeChannelConfig(t, dir, "telegram", ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN +`) + + got, missing, err := EnvVarsFromConfig(dir, []string{"slack", "telegram"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !slices.Equal(missing, []string{"slack"}) { + t.Errorf("missing = %v, want [slack]", missing) + } + if !slices.Equal(got, []string{"TELEGRAM_BOT_TOKEN"}) { + t.Errorf("got %v, want only telegram's env (slack file missing)", got) + } +} + +func TestEnvVarsFromConfig_EmptyChannels(t *testing.T) { + got, missing, err := EnvVarsFromConfig(t.TempDir(), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 0 || len(missing) != 0 { + t.Errorf("expected empty results for no channels, got env=%v missing=%v", got, missing) + } +} + +func TestEnvVarsFromConfig_ParseError(t *testing.T) { + dir := t.TempDir() + writeChannelConfig(t, dir, "slack", "this is: not: valid: yaml: [") + _, _, err := EnvVarsFromConfig(dir, []string{"slack"}) + if err == nil { + t.Fatal("expected parse error for invalid YAML") + } +} diff --git a/forge-cli/cmd/build.go b/forge-cli/cmd/build.go index 8ebfbb3..eb8a24d 100644 --- a/forge-cli/cmd/build.go +++ b/forge-cli/cmd/build.go @@ -114,6 +114,7 @@ func runBuild(cmd *cobra.Command, args []string) error { &build.SkillsStage{}, &build.SecurityAnalysisStage{}, &build.RequirementsStage{}, + &build.ChannelsStage{}, &build.PolicyStage{}, &build.EgressStage{}, &build.DockerfileStage{}, diff --git a/forge-cli/cmd/package.go b/forge-cli/cmd/package.go index 758a0b0..a42a66e 100644 --- a/forge-cli/cmd/package.go +++ b/forge-cli/cmd/package.go @@ -9,6 +9,7 @@ import ( "text/template" "time" + clichannels "github.com/initializ/forge/forge-cli/channels" "github.com/initializ/forge/forge-cli/config" "github.com/initializ/forge/forge-cli/container" "github.com/initializ/forge/forge-cli/templates" @@ -203,7 +204,7 @@ func runPackage(cmd *cobra.Command, args []string) error { // Generate docker-compose.yaml if --with-channels is set if withChannels && len(cfg.Channels) > 0 { composePath := filepath.Join(outDir, "docker-compose.yaml") - if err := generateDockerCompose(composePath, imageTag, cfg, 8080); err != nil { + if err := generateDockerCompose(composePath, imageTag, cfg, filepath.Dir(cfgPath), 8080); err != nil { return fmt.Errorf("generating docker-compose.yaml: %w", err) } fmt.Printf("Generated %s\n", composePath) @@ -267,7 +268,7 @@ type composeData struct { Channels []channelComposeData } -func generateDockerCompose(path string, imageTag string, cfg *types.ForgeConfig, port int) error { +func generateDockerCompose(path string, imageTag string, cfg *types.ForgeConfig, workDir string, port int) error { if port == 0 { port = 8080 } @@ -282,22 +283,21 @@ func generateDockerCompose(path string, imageTag string, cfg *types.ForgeConfig, return fmt.Errorf("parsing docker-compose template: %w", err) } + // Build per-channel env var lists from the same canonical source the + // build pipeline uses for the K8s manifests — each project channel's + // *-config.yaml. Channels without a config file contribute no env vars. var channels []channelComposeData for _, ch := range cfg.Channels { - if ch != "slack" && ch != "telegram" { + envVars, _, envErr := clichannels.EnvVarsFromConfig(workDir, []string{ch}) + if envErr != nil { + return fmt.Errorf("reading channel %q env vars: %w", ch, envErr) + } + if len(envVars) == 0 { continue } - cd := channelComposeData{Name: ch} - switch ch { - case "slack": - cd.EnvVars = []string{ - "SLACK_SIGNING_SECRET=${SLACK_SIGNING_SECRET}", - "SLACK_BOT_TOKEN=${SLACK_BOT_TOKEN}", - } - case "telegram": - cd.EnvVars = []string{ - "TELEGRAM_BOT_TOKEN=${TELEGRAM_BOT_TOKEN}", - } + cd := channelComposeData{Name: ch, EnvVars: make([]string, 0, len(envVars))} + for _, name := range envVars { + cd.EnvVars = append(cd.EnvVars, fmt.Sprintf("%s=${%s}", name, name)) } channels = append(channels, cd) } diff --git a/forge-cli/cmd/package_test.go b/forge-cli/cmd/package_test.go index 79dab4f..df44b17 100644 --- a/forge-cli/cmd/package_test.go +++ b/forge-cli/cmd/package_test.go @@ -130,6 +130,19 @@ func TestWithChannelsFlagDefault(t *testing.T) { func TestGenerateDockerCompose(t *testing.T) { dir := t.TempDir() + // Seed the per-channel config files that the generator now reads to + // derive env vars. These mirror the templates shipped by `forge init`. + writeFile(t, filepath.Join(dir, "slack-config.yaml"), ` +adapter: slack +settings: + app_token_env: SLACK_APP_TOKEN + bot_token_env: SLACK_BOT_TOKEN +`) + writeFile(t, filepath.Join(dir, "telegram-config.yaml"), ` +adapter: telegram +settings: + bot_token_env: TELEGRAM_BOT_TOKEN +`) path := filepath.Join(dir, "docker-compose.yaml") cfg := &types.ForgeConfig{ @@ -147,7 +160,7 @@ func TestGenerateDockerCompose(t *testing.T) { }, } - err := generateDockerCompose(path, "my-agent:0.1.0", cfg, 8080) + err := generateDockerCompose(path, "my-agent:0.1.0", cfg, dir, 8080) if err != nil { t.Fatalf("generateDockerCompose() error: %v", err) } @@ -186,26 +199,34 @@ func TestGenerateDockerCompose(t *testing.T) { t.Error("missing egress mode label") } - // Check slack adapter + // Slack adapter env vars come from slack-config.yaml. The current Slack + // adapter is Socket Mode (app_token + bot_token), so signing_secret is + // not declared in the YAML and therefore not injected — switching to the + // YAML-driven source corrects a long-standing inaccuracy in the prior + // hardcoded map. if !strings.Contains(content, "slack-adapter:") { t.Error("missing slack-adapter service") } - if !strings.Contains(content, "SLACK_SIGNING_SECRET") { - t.Error("missing SLACK_SIGNING_SECRET env var") + if !strings.Contains(content, "SLACK_APP_TOKEN=${SLACK_APP_TOKEN}") { + t.Error("missing SLACK_APP_TOKEN env var") } - if !strings.Contains(content, "SLACK_BOT_TOKEN") { + if !strings.Contains(content, "SLACK_BOT_TOKEN=${SLACK_BOT_TOKEN}") { t.Error("missing SLACK_BOT_TOKEN env var") } + if strings.Contains(content, "SLACK_SIGNING_SECRET") { + t.Error("SLACK_SIGNING_SECRET should not be injected: not declared in slack-config.yaml and unused by Socket Mode adapter") + } // Check telegram adapter if !strings.Contains(content, "telegram-adapter:") { t.Error("missing telegram-adapter service") } - if !strings.Contains(content, "TELEGRAM_BOT_TOKEN") { + if !strings.Contains(content, "TELEGRAM_BOT_TOKEN=${TELEGRAM_BOT_TOKEN}") { t.Error("missing TELEGRAM_BOT_TOKEN env var") } - // Non-adapter channels (a2a) should be skipped + // Channels without a *-config.yaml (a2a here) contribute no env vars + // and so produce no adapter service. if strings.Contains(content, "a2a-adapter") { t.Error("a2a should not generate an adapter service") } @@ -230,7 +251,7 @@ func TestGenerateDockerCompose_NoAdapters(t *testing.T) { Channels: []string{"a2a", "http"}, } - err := generateDockerCompose(path, "my-agent:0.1.0", cfg, 8080) + err := generateDockerCompose(path, "my-agent:0.1.0", cfg, dir, 8080) if err != nil { t.Fatalf("generateDockerCompose() error: %v", err) } @@ -243,3 +264,10 @@ func TestGenerateDockerCompose_NoAdapters(t *testing.T) { t.Error("should not generate adapter services for non-adapter channels") } } + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("writing %s: %v", path, err) + } +}