From 576a517c59cef63f78975c611904d38069771443 Mon Sep 17 00:00:00 2001 From: beads/crew/emma Date: Fri, 9 Jan 2026 14:12:48 -0800 Subject: [PATCH] feat(multirepo): trust non-built-in types during hydration (bd-9ji4z) Implements federation trust model for multi-repo type validation: - Built-in types are validated (catch typos) - Non-built-in types trusted from source repos Changes: - Add IssueType.IsBuiltIn() method - Add Issue.ValidateForImport() for trust-based validation - Update upsertIssueInTx to use ValidateForImport Closes: bd-dqwuf, bd-alpw2 | Epic: bd-9ji4z Co-Authored-By: Claude Opus 4.5 --- internal/storage/sqlite/multirepo.go | 22 ++-- internal/types/types.go | 42 ++++++- internal/types/types_test.go | 180 +++++++++++++++++++++++++++ 3 files changed, 232 insertions(+), 12 deletions(-) diff --git a/internal/storage/sqlite/multirepo.go b/internal/storage/sqlite/multirepo.go index 71c820c9..7a8ca15f 100644 --- a/internal/storage/sqlite/multirepo.go +++ b/internal/storage/sqlite/multirepo.go @@ -126,15 +126,13 @@ func (s *SQLiteStorage) importJSONLFile(ctx context.Context, jsonlPath, sourceRe } defer file.Close() - // Fetch custom statuses and types for validation (bd-1pj6) + // Fetch custom statuses for validation (bd-1pj6) + // Note: custom types are NOT fetched - we use federation trust model (bd-9ji4z) + // where non-built-in types from child repos are trusted as already validated. customStatuses, err := s.GetCustomStatuses(ctx) if err != nil { return 0, fmt.Errorf("failed to get custom statuses: %w", err) } - customTypes, err := s.GetCustomTypes(ctx) - if err != nil { - return 0, fmt.Errorf("failed to get custom types: %w", err) - } scanner := bufio.NewScanner(file) // Increase buffer size for large issues @@ -187,8 +185,8 @@ func (s *SQLiteStorage) importJSONLFile(ctx context.Context, jsonlPath, sourceRe issue.ContentHash = issue.ComputeContentHash() } - // Insert or update issue (with custom status and type support) - if err := s.upsertIssueInTx(ctx, tx, &issue, customStatuses, customTypes); err != nil { + // Insert or update issue (with federation trust model for types, bd-9ji4z) + if err := s.upsertIssueInTx(ctx, tx, &issue, customStatuses); err != nil { return 0, fmt.Errorf("failed to import issue %s at line %d: %w", issue.ID, lineNum, err) } @@ -254,7 +252,9 @@ func (s *SQLiteStorage) importJSONLFile(ctx context.Context, jsonlPath, sourceRe // upsertIssueInTx inserts or updates an issue within a transaction. // Uses INSERT OR REPLACE to handle both new and existing issues. -func (s *SQLiteStorage) upsertIssueInTx(ctx context.Context, tx *sql.Tx, issue *types.Issue, customStatuses, customTypes []string) error { +// Uses federation trust model for type validation: built-in types are validated, +// non-built-in types are trusted from the source repo (bd-9ji4z). +func (s *SQLiteStorage) upsertIssueInTx(ctx context.Context, tx *sql.Tx, issue *types.Issue, customStatuses []string) error { // Defensive fix for closed_at invariant (GH#523): older versions of bd could // close issues without setting closed_at. Fix by using max(created_at, updated_at) + 1s. if issue.Status == types.StatusClosed && issue.ClosedAt == nil { @@ -276,8 +276,10 @@ func (s *SQLiteStorage) upsertIssueInTx(ctx context.Context, tx *sql.Tx, issue * issue.DeletedAt = &deletedAt } - // Validate issue (with custom status and type support, bd-1pj6) - if err := issue.ValidateWithCustom(customStatuses, customTypes); err != nil { + // Validate issue using federation trust model (bd-9ji4z): + // - Built-in types are validated (catch typos) + // - Non-built-in types are trusted (child repo already validated them) + if err := issue.ValidateForImport(customStatuses); err != nil { return fmt.Errorf("validation failed: %w", err) } diff --git a/internal/types/types.go b/internal/types/types.go index 02467bae..a9b46611 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -307,6 +307,20 @@ func (i *Issue) ValidateWithCustomStatuses(customStatuses []string) error { // ValidateWithCustom checks if the issue has valid field values, // allowing custom statuses and types in addition to built-in ones. func (i *Issue) ValidateWithCustom(customStatuses, customTypes []string) error { + return i.validateInternal(customStatuses, customTypes, false) +} + +// ValidateForImport validates the issue for multi-repo import (federation trust model). +// Built-in types are validated (to catch typos). Non-built-in types are trusted +// since the source repo already validated them when the issue was created. +// This implements "trust the chain below you" from the HOP federation model. +func (i *Issue) ValidateForImport(customStatuses []string) error { + return i.validateInternal(customStatuses, nil, true) +} + +// validateInternal is the shared validation logic. +// If trustCustomTypes is true, non-built-in issue types are trusted (not validated). +func (i *Issue) validateInternal(customStatuses, customTypes []string, trustCustomTypes bool) error { if len(i.Title) == 0 { return fmt.Errorf("title is required") } @@ -319,9 +333,25 @@ func (i *Issue) ValidateWithCustom(customStatuses, customTypes []string) error { if !i.Status.IsValidWithCustom(customStatuses) { return fmt.Errorf("invalid status: %s", i.Status) } - if !i.IssueType.IsValidWithCustom(customTypes) { - return fmt.Errorf("invalid issue type: %s", i.IssueType) + + // Issue type validation: federation trust model (bd-9ji4z) + if trustCustomTypes { + // Multi-repo import: trust non-built-in types from source repo + // Only validate built-in types (catch typos like "tsak" vs "task") + if i.IssueType != "" && !i.IssueType.IsBuiltIn() { + // Non-built-in type - trust it (child repo already validated) + } else if i.IssueType != "" && !i.IssueType.IsValid() { + // This shouldn't happen: IsBuiltIn() == IsValid() for non-empty types + // But guard against edge cases + return fmt.Errorf("invalid issue type: %s", i.IssueType) + } + } else { + // Normal validation: check against built-in + custom types + if !i.IssueType.IsValidWithCustom(customTypes) { + return fmt.Errorf("invalid issue type: %s", i.IssueType) + } } + if i.EstimatedMinutes != nil && *i.EstimatedMinutes < 0 { return fmt.Errorf("estimated_minutes cannot be negative") } @@ -437,6 +467,14 @@ func (t IssueType) IsValid() bool { return false } +// IsBuiltIn returns true if this is a built-in type (not a custom type). +// Used during multi-repo hydration to determine whether to validate or trust: +// - Built-in types: validate (catch typos like "tsak" vs "task") +// - Custom types: trust (child repo already validated) +func (t IssueType) IsBuiltIn() bool { + return t.IsValid() +} + // IsValidWithCustom checks if the issue type is valid, including custom types. // Custom types are user-defined via bd config set types.custom "type1,type2,..." func (t IssueType) IsValidWithCustom(customTypes []string) bool { diff --git a/internal/types/types_test.go b/internal/types/types_test.go index 393c74bf..c2c7b5e4 100644 --- a/internal/types/types_test.go +++ b/internal/types/types_test.go @@ -391,6 +391,149 @@ func TestValidateWithCustomStatuses(t *testing.T) { } } +// TestValidateForImport tests the federation trust model (bd-9ji4z): +// - Built-in types are validated (catch typos) +// - Non-built-in types are trusted (child repo already validated) +func TestValidateForImport(t *testing.T) { + tests := []struct { + name string + issue Issue + wantErr bool + errMsg string + }{ + { + name: "built-in type task passes", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: TypeTask, + }, + wantErr: false, + }, + { + name: "built-in type bug passes", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: TypeBug, + }, + wantErr: false, + }, + { + name: "custom type pm is trusted (not in parent config)", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: IssueType("pm"), // Custom type from child repo + }, + wantErr: false, // Should pass - federation trust model + }, + { + name: "custom type llm is trusted", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: IssueType("llm"), // Custom type from child repo + }, + wantErr: false, // Should pass - federation trust model + }, + { + name: "custom type agent is trusted", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: IssueType("agent"), // Gas Town custom type + }, + wantErr: false, // Should pass - federation trust model + }, + { + name: "empty type defaults to task (handled by SetDefaults)", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: IssueType(""), // Empty is allowed + }, + wantErr: false, + }, + { + name: "other validations still run - missing title", + issue: Issue{ + Title: "", // Missing required field + Status: StatusOpen, + Priority: 1, + IssueType: IssueType("pm"), + }, + wantErr: true, + errMsg: "title is required", + }, + { + name: "other validations still run - invalid priority", + issue: Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 10, // Invalid + IssueType: IssueType("pm"), + }, + wantErr: true, + errMsg: "priority must be between 0 and 4", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.issue.ValidateForImport(nil) // No custom statuses needed for these tests + if tt.wantErr { + if err == nil { + t.Errorf("ValidateForImport() expected error, got nil") + return + } + if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) { + t.Errorf("ValidateForImport() error = %v, want error containing %q", err, tt.errMsg) + } + } else { + if err != nil { + t.Errorf("ValidateForImport() unexpected error = %v", err) + } + } + }) + } +} + +// TestValidateForImportVsValidateWithCustom contrasts the two validation modes +func TestValidateForImportVsValidateWithCustom(t *testing.T) { + // Issue with custom type that's NOT in customTypes list + issue := Issue{ + Title: "Test Issue", + Status: StatusOpen, + Priority: 1, + IssueType: IssueType("pm"), // Custom type not configured in parent + } + + // ValidateWithCustom (normal mode): should fail without pm in customTypes + err := issue.ValidateWithCustom(nil, nil) + if err == nil { + t.Error("ValidateWithCustom() should fail for custom type without config") + } + + // ValidateWithCustom: should pass with pm in customTypes + err = issue.ValidateWithCustom(nil, []string{"pm"}) + if err != nil { + t.Errorf("ValidateWithCustom() with pm config should pass, got: %v", err) + } + + // ValidateForImport (federation trust mode): should pass without any config + err = issue.ValidateForImport(nil) + if err != nil { + t.Errorf("ValidateForImport() should trust custom type, got: %v", err) + } +} + func TestIssueTypeIsValid(t *testing.T) { tests := []struct { issueType IssueType @@ -423,6 +566,43 @@ func TestIssueTypeIsValid(t *testing.T) { } } +func TestIssueTypeIsBuiltIn(t *testing.T) { + // IsBuiltIn should match IsValid - both identify built-in types + // This is used during multi-repo hydration to determine trust: + // - Built-in types: validate (catch typos) + // - Custom types (!IsBuiltIn): trust from source repo + tests := []struct { + issueType IssueType + builtIn bool + }{ + // All built-in types + {TypeBug, true}, + {TypeFeature, true}, + {TypeTask, true}, + {TypeEpic, true}, + {TypeChore, true}, + {TypeMessage, true}, + {TypeMergeRequest, true}, + {TypeMolecule, true}, + {TypeGate, true}, + {TypeEvent, true}, + // Custom types (not built-in) + {IssueType("pm"), false}, // Custom type from child repo + {IssueType("llm"), false}, // Custom type from child repo + {IssueType("agent"), false}, // Gas Town custom type + {IssueType("convoy"), false}, // Gas Town custom type + {IssueType(""), false}, // Empty is not built-in + } + + for _, tt := range tests { + t.Run(string(tt.issueType), func(t *testing.T) { + if got := tt.issueType.IsBuiltIn(); got != tt.builtIn { + t.Errorf("IssueType(%q).IsBuiltIn() = %v, want %v", tt.issueType, got, tt.builtIn) + } + }) + } +} + func TestIssueTypeRequiredSections(t *testing.T) { tests := []struct { issueType IssueType