diff --git a/.github/aw/schemas/agentic-workflow.json b/.github/aw/schemas/agentic-workflow.json index 5dc44b40f29..b77d13c62c4 100644 --- a/.github/aw/schemas/agentic-workflow.json +++ b/.github/aw/schemas/agentic-workflow.json @@ -3102,13 +3102,20 @@ "type": "object", "description": "Repo-memory configuration object", "properties": { + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/default)" + "description": "Git branch name for memory storage (default: {branch-prefix}/default or memory/default if branch-prefix not set)" }, "file-glob": { "oneOf": [ @@ -3172,13 +3179,20 @@ "type": "string", "description": "Memory identifier (required for array notation, default: 'default')" }, + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Applied to all entries in the array. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/{id})" + "description": "Git branch name for memory storage (default: {branch-prefix}/{id} or memory/{id} if branch-prefix not set)" }, "file-glob": { "oneOf": [ diff --git a/.github/workflows/daily-code-metrics.lock.yml b/.github/workflows/daily-code-metrics.lock.yml index b06dc4fe213..7145d617dcf 100644 --- a/.github/workflows/daily-code-metrics.lock.yml +++ b/.github/workflows/daily-code-metrics.lock.yml @@ -151,7 +151,7 @@ jobs: - name: Clone repo-memory branch (default) env: GH_TOKEN: ${{ github.token }} - BRANCH_NAME: memory/code-metrics + BRANCH_NAME: daily/default TARGET_REPO: ${{ github.repository }} MEMORY_DIR: /tmp/gh-aw/repo-memory/default CREATE_ORPHAN: true @@ -1335,7 +1335,7 @@ jobs: You have access to a persistent repo memory folder at `/tmp/gh-aw/repo-memory/default/` where you can read and write files that are stored in a git branch. Historical code quality and health metrics - **Read/Write Access**: You can freely read from and write to any files in this folder - - **Git Branch Storage**: Files are stored in the `memory/code-metrics` branch of the current repository + - **Git Branch Storage**: Files are stored in the `daily/default` branch of the current repository - **Automatic Push**: Changes are automatically committed and pushed after the workflow completes - **Merge Strategy**: In case of conflicts, your changes (current version) win - **Persistence**: Files persist across workflow runs via git branch storage @@ -1984,7 +1984,7 @@ jobs: ARTIFACT_DIR: /tmp/gh-aw/repo-memory/default MEMORY_ID: default TARGET_REPO: ${{ github.repository }} - BRANCH_NAME: memory/code-metrics + BRANCH_NAME: daily/default MAX_FILE_SIZE: 102400 MAX_FILE_COUNT: 100 FILE_GLOB_FILTER: "*.json *.jsonl *.csv *.md" diff --git a/.github/workflows/daily-code-metrics.md b/.github/workflows/daily-code-metrics.md index f928e7f4fd9..7bae87b7c40 100644 --- a/.github/workflows/daily-code-metrics.md +++ b/.github/workflows/daily-code-metrics.md @@ -11,7 +11,7 @@ tracker-id: daily-code-metrics engine: claude tools: repo-memory: - branch-name: memory/code-metrics + branch-prefix: daily description: "Historical code quality and health metrics" file-glob: ["*.json", "*.jsonl", "*.csv", "*.md"] max-file-size: 102400 # 100KB diff --git a/pkg/parser/schemas/included_file_schema.json b/pkg/parser/schemas/included_file_schema.json index 44a9cf8caa6..bf4eeb5cb8c 100644 --- a/pkg/parser/schemas/included_file_schema.json +++ b/pkg/parser/schemas/included_file_schema.json @@ -284,13 +284,20 @@ "type": "object", "description": "Repo-memory configuration object", "properties": { + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/default)" + "description": "Git branch name for memory storage (default: {branch-prefix}/default or memory/default if branch-prefix not set)" }, "file-glob": { "oneOf": [ @@ -350,13 +357,20 @@ "type": "string", "description": "Memory identifier (required for array notation, default: 'default')" }, + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Applied to all entries in the array. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/{id})" + "description": "Git branch name for memory storage (default: {branch-prefix}/{id} or memory/{id} if branch-prefix not set)" }, "file-glob": { "oneOf": [ diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5dc44b40f29..b77d13c62c4 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3102,13 +3102,20 @@ "type": "object", "description": "Repo-memory configuration object", "properties": { + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/default)" + "description": "Git branch name for memory storage (default: {branch-prefix}/default or memory/default if branch-prefix not set)" }, "file-glob": { "oneOf": [ @@ -3172,13 +3179,20 @@ "type": "string", "description": "Memory identifier (required for array notation, default: 'default')" }, + "branch-prefix": { + "type": "string", + "minLength": 4, + "maxLength": 32, + "pattern": "^[a-zA-Z0-9_-]+$", + "description": "Branch prefix for memory storage (default: 'memory'). Must be 4-32 characters, alphanumeric with hyphens/underscores, and cannot be 'copilot'. Applied to all entries in the array. Branch will be named {branch-prefix}/{id}" + }, "target-repo": { "type": "string", "description": "Target repository for memory storage (default: current repository). Format: owner/repo" }, "branch-name": { "type": "string", - "description": "Git branch name for memory storage (default: memory/{id})" + "description": "Git branch name for memory storage (default: {branch-prefix}/{id} or memory/{id} if branch-prefix not set)" }, "file-glob": { "oneOf": [ diff --git a/pkg/workflow/repo_memory.go b/pkg/workflow/repo_memory.go index 3f8410033e0..f5a46a843aa 100644 --- a/pkg/workflow/repo_memory.go +++ b/pkg/workflow/repo_memory.go @@ -18,6 +18,7 @@ package workflow import ( "fmt" + "regexp" "strings" "github.com/githubnext/gh-aw/pkg/logger" @@ -27,7 +28,8 @@ var repoMemoryLog = logger.New("workflow:repo_memory") // RepoMemoryConfig holds configuration for repo-memory functionality type RepoMemoryConfig struct { - Memories []RepoMemoryEntry `yaml:"memories,omitempty"` // repo-memory configurations + BranchPrefix string `yaml:"branch-prefix,omitempty"` // branch prefix (default: "memory") + Memories []RepoMemoryEntry `yaml:"memories,omitempty"` // repo-memory configurations } // RepoMemoryEntry represents a single repo-memory configuration @@ -49,12 +51,40 @@ type RepoMemoryToolConfig struct { Raw any `yaml:"-"` } -// generateDefaultBranchName generates a default branch name for a given memory ID -func generateDefaultBranchName(memoryID string) string { - if memoryID == "default" { - return "memory/default" +// generateDefaultBranchName generates a default branch name for a given memory ID and prefix +func generateDefaultBranchName(memoryID string, branchPrefix string) string { + if branchPrefix == "" { + branchPrefix = "memory" } - return fmt.Sprintf("memory/%s", memoryID) + return fmt.Sprintf("%s/%s", branchPrefix, memoryID) +} + +// validateBranchPrefix validates that the branch prefix meets requirements +func validateBranchPrefix(prefix string) error { + if prefix == "" { + return nil // Empty means use default + } + + // Check length (4-32 characters) + if len(prefix) < 4 { + return fmt.Errorf("branch-prefix must be at least 4 characters long, got %d", len(prefix)) + } + if len(prefix) > 32 { + return fmt.Errorf("branch-prefix must be at most 32 characters long, got %d", len(prefix)) + } + + // Check for alphanumeric and branch-friendly characters (alphanumeric, hyphens, underscores) + validPattern := regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) + if !validPattern.MatchString(prefix) { + return fmt.Errorf("branch-prefix must contain only alphanumeric characters, hyphens, and underscores, got '%s'", prefix) + } + + // Cannot be "copilot" + if strings.ToLower(prefix) == "copilot" { + return fmt.Errorf("branch-prefix cannot be 'copilot' (reserved)") + } + + return nil } // extractRepoMemoryConfig extracts repo-memory configuration from tools section @@ -66,7 +96,9 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor repoMemoryLog.Print("Extracting repo-memory configuration from ToolsConfig") - config := &RepoMemoryConfig{} + config := &RepoMemoryConfig{ + BranchPrefix: "memory", // Default branch prefix + } repoMemoryValue := toolsConfig.RepoMemory.Raw // Handle nil value (simple enable with defaults) - same as true @@ -75,7 +107,7 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor config.Memories = []RepoMemoryEntry{ { ID: "default", - BranchName: generateDefaultBranchName("default"), + BranchName: generateDefaultBranchName("default", config.BranchPrefix), MaxFileSize: 10240, // 10KB MaxFileCount: 100, CreateOrphan: true, @@ -92,7 +124,7 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor config.Memories = []RepoMemoryEntry{ { ID: "default", - BranchName: generateDefaultBranchName("default"), + BranchName: generateDefaultBranchName("default", config.BranchPrefix), MaxFileSize: 10240, // 10KB MaxFileCount: 100, CreateOrphan: true, @@ -109,6 +141,23 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor if memoryArray, ok := repoMemoryValue.([]any); ok { repoMemoryLog.Printf("Processing memory array with %d entries", len(memoryArray)) config.Memories = make([]RepoMemoryEntry, 0, len(memoryArray)) + + // Parse branch-prefix from first item if it's a map with branch-prefix key + // This allows branch-prefix to be set at the top level for all memories + if len(memoryArray) > 0 { + if firstItem, ok := memoryArray[0].(map[string]any); ok { + if branchPrefix, exists := firstItem["branch-prefix"]; exists { + if prefixStr, ok := branchPrefix.(string); ok { + if err := validateBranchPrefix(prefixStr); err != nil { + return nil, err + } + config.BranchPrefix = prefixStr + repoMemoryLog.Printf("Using custom branch-prefix: %s", prefixStr) + } + } + } + } + for _, item := range memoryArray { if memoryMap, ok := item.(map[string]any); ok { entry := RepoMemoryEntry{ @@ -143,7 +192,7 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor } // Set default branch name if not specified if entry.BranchName == "" { - entry.BranchName = generateDefaultBranchName(entry.ID) + entry.BranchName = generateDefaultBranchName(entry.ID, config.BranchPrefix) } // Parse file-glob @@ -228,9 +277,21 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor // Convert to array with single entry if configMap, ok := repoMemoryValue.(map[string]any); ok { repoMemoryLog.Print("Processing object-style repo-memory configuration (backward compatible)") + + // Parse branch-prefix if provided + if branchPrefix, exists := configMap["branch-prefix"]; exists { + if prefixStr, ok := branchPrefix.(string); ok { + if err := validateBranchPrefix(prefixStr); err != nil { + return nil, err + } + config.BranchPrefix = prefixStr + repoMemoryLog.Printf("Using custom branch-prefix: %s", prefixStr) + } + } + entry := RepoMemoryEntry{ ID: "default", - BranchName: generateDefaultBranchName("default"), + BranchName: generateDefaultBranchName("default", config.BranchPrefix), MaxFileSize: 10240, // 10KB default MaxFileCount: 100, // 100 files default CreateOrphan: true, // create orphan by default diff --git a/pkg/workflow/repo_memory_path_consistency_test.go b/pkg/workflow/repo_memory_path_consistency_test.go index 33ac3b5372e..badd625b523 100644 --- a/pkg/workflow/repo_memory_path_consistency_test.go +++ b/pkg/workflow/repo_memory_path_consistency_test.go @@ -347,7 +347,7 @@ func TestRepoMemoryBranchNameGeneration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - branchName := generateDefaultBranchName(tt.memoryID) + branchName := generateDefaultBranchName(tt.memoryID, "memory") assert.Equal(t, tt.expectedBranchName, branchName, "Generated branch name should match expected pattern") }) diff --git a/pkg/workflow/repo_memory_test.go b/pkg/workflow/repo_memory_test.go index 1a1854a3627..61aefdc6484 100644 --- a/pkg/workflow/repo_memory_test.go +++ b/pkg/workflow/repo_memory_test.go @@ -3,6 +3,9 @@ package workflow import ( "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestRepoMemoryConfigDefault tests basic repo-memory configuration with boolean true @@ -706,3 +709,227 @@ func TestRepoMemoryConfigWithCampaignID(t *testing.T) { t.Errorf("Expected file glob 'go-file-size-reduction-project64/**', got %v", memory.FileGlob) } } + +// TestBranchPrefixValidation tests the validateBranchPrefix function +func TestBranchPrefixValidation(t *testing.T) { + tests := []struct { + name string + prefix string + wantErr bool + errMsg string + }{ + { + name: "empty prefix (use default)", + prefix: "", + wantErr: false, + }, + { + name: "valid prefix - alphanumeric", + prefix: "campaigns", + wantErr: false, + }, + { + name: "valid prefix - daily (5 chars)", + prefix: "daily", + wantErr: false, + }, + { + name: "valid prefix - with hyphens", + prefix: "my-memory", + wantErr: false, + }, + { + name: "valid prefix - with underscores", + prefix: "my_memory", + wantErr: false, + }, + { + name: "valid prefix - mixed", + prefix: "test_mem-123", + wantErr: false, + }, + { + name: "valid prefix - exactly 4 chars", + prefix: "mem1", + wantErr: false, + }, + { + name: "valid prefix - 5 chars", + prefix: "mem12", + wantErr: false, + }, + { + name: "valid prefix - exactly 32 chars", + prefix: "12345678901234567890123456789012", + wantErr: false, + }, + { + name: "invalid - too short (3 chars)", + prefix: "mem", + wantErr: true, + errMsg: "must be at least 4 characters long", + }, + { + name: "invalid - too long (33 chars)", + prefix: "123456789012345678901234567890123", + wantErr: true, + errMsg: "must be at most 32 characters long", + }, + { + name: "invalid - contains slash", + prefix: "memory/branch", + wantErr: true, + errMsg: "must contain only alphanumeric characters", + }, + { + name: "invalid - contains space", + prefix: "my memory", + wantErr: true, + errMsg: "must contain only alphanumeric characters", + }, + { + name: "invalid - contains special char", + prefix: "memory@branch", + wantErr: true, + errMsg: "must contain only alphanumeric characters", + }, + { + name: "invalid - reserved word 'copilot'", + prefix: "copilot", + wantErr: true, + errMsg: "cannot be 'copilot' (reserved)", + }, + { + name: "invalid - reserved word 'Copilot' (case-insensitive)", + prefix: "Copilot", + wantErr: true, + errMsg: "cannot be 'copilot' (reserved)", + }, + { + name: "invalid - reserved word 'COPILOT' (case-insensitive)", + prefix: "COPILOT", + wantErr: true, + errMsg: "cannot be 'copilot' (reserved)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateBranchPrefix(tt.prefix) + if tt.wantErr { + require.Error(t, err, "Expected error for prefix: %s", tt.prefix) + assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain: %s", tt.errMsg) + } else { + require.NoError(t, err, "Expected no error for prefix: %s", tt.prefix) + } + }) + } +} + +// TestBranchPrefixInConfig tests branch-prefix in object configuration +func TestBranchPrefixInConfig(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": map[string]any{ + "branch-prefix": "campaigns", + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Failed to parse tools config") + + compiler := NewCompiler(false, "", "test") + config, err := compiler.extractRepoMemoryConfig(toolsConfig) + require.NoError(t, err, "Failed to extract repo-memory config") + require.NotNil(t, config, "Expected non-nil config") + + assert.Equal(t, "campaigns", config.BranchPrefix, "Expected branch-prefix 'campaigns'") + assert.Len(t, config.Memories, 1, "Expected 1 memory") + + memory := config.Memories[0] + assert.Equal(t, "campaigns/default", memory.BranchName, "Expected branch name 'campaigns/default'") +} + +// TestBranchPrefixInArrayConfig tests branch-prefix in array configuration +func TestBranchPrefixInArrayConfig(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": []any{ + map[string]any{ + "id": "session", + "branch-prefix": "my-prefix", + }, + map[string]any{ + "id": "logs", + }, + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Failed to parse tools config") + + compiler := NewCompiler(false, "", "test") + config, err := compiler.extractRepoMemoryConfig(toolsConfig) + require.NoError(t, err, "Failed to extract repo-memory config") + require.NotNil(t, config, "Expected non-nil config") + + assert.Equal(t, "my-prefix", config.BranchPrefix, "Expected branch-prefix 'my-prefix'") + assert.Len(t, config.Memories, 2, "Expected 2 memories") + + // Both memories should use the same prefix + assert.Equal(t, "my-prefix/session", config.Memories[0].BranchName, "Expected branch name 'my-prefix/session'") + assert.Equal(t, "my-prefix/logs", config.Memories[1].BranchName, "Expected branch name 'my-prefix/logs'") +} + +// TestBranchPrefixWithExplicitBranchName tests that explicit branch-name overrides prefix +func TestBranchPrefixWithExplicitBranchName(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": map[string]any{ + "branch-prefix": "campaigns", + "branch-name": "custom/branch", + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Failed to parse tools config") + + compiler := NewCompiler(false, "", "test") + config, err := compiler.extractRepoMemoryConfig(toolsConfig) + require.NoError(t, err, "Failed to extract repo-memory config") + require.NotNil(t, config, "Expected non-nil config") + + assert.Equal(t, "campaigns", config.BranchPrefix, "Expected branch-prefix 'campaigns'") + + memory := config.Memories[0] + // Explicit branch-name should override the prefix + assert.Equal(t, "custom/branch", memory.BranchName, "Expected explicit branch name 'custom/branch'") +} + +// TestInvalidBranchPrefixRejectsConfig tests that invalid prefix causes error +func TestInvalidBranchPrefixRejectsConfig(t *testing.T) { + tests := []struct { + name string + prefix string + }{ + {"too short", "short"}, + {"too long", "this_is_a_very_long_prefix_that_exceeds_32_characters"}, + {"reserved word", "copilot"}, + {"special chars", "my@prefix"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": map[string]any{ + "branch-prefix": tt.prefix, + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Failed to parse tools config") + + compiler := NewCompiler(false, "", "test") + config, err := compiler.extractRepoMemoryConfig(toolsConfig) + require.Error(t, err, "Expected error for invalid branch-prefix: %s", tt.prefix) + assert.Nil(t, config, "Expected nil config on error") + }) + } +}