Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 47 additions & 16 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,31 @@ func (c *orgConfig) DefaultRoles() []string {
// Stored in .fullsend/config.yaml within the target repository.
// Consumer packages should use the PerRepoConfigReader or ConfigWriter
// interfaces rather than referencing this type directly.
//
// The parent field implements a fallback chain per ADR 0069 Decision 2:
// accessors check the local struct first, then fall through to parent
// when the local value is unset. The terminal parent is perRepoDefaults,
// which returns compiled-in code defaults.
type perRepoConfig struct {
Version string `yaml:"version"`
KillSwitch bool `yaml:"kill_switch,omitempty"`
Runtime string `yaml:"runtime,omitempty"`
Roles []string `yaml:"roles,omitempty"`
Agents []AgentEntry `yaml:"agents,omitempty"`
// omitempty so unset version is not marshaled (unlike orgConfig,
// where version is always required). This allows the fallback
// chain to inherit version from the parent layer.
Version string `yaml:"version,omitempty"`
KillSwitch *bool `yaml:"kill_switch,omitempty"`
Runtime string `yaml:"runtime,omitempty"`
Roles []string `yaml:"roles,omitempty"`
Agents []AgentEntry `yaml:"agents,omitempty"`
// NOTE: omitempty means an empty slice (deny-all) is not marshaled.
// After a YAML roundtrip, deny-all becomes nil and AllowedResources()
// falls through to parent defaults. This is a pre-existing
// limitation of the yaml.v3 omitempty behavior.
AllowedRemoteResources []string `yaml:"allowed_remote_resources,omitempty"`
CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"`

// parent is the next layer in the fallback chain. Getters consult
// parent when the local field is unset. Excluded from YAML
// serialization so Marshal emits only locally-set values.
parent PerRepoConfigReader `yaml:"-"`
}

const perRepoConfigHeader = `# fullsend per-repo configuration
Expand All @@ -522,6 +539,7 @@ func NewPerRepoConfig(roles []string, targetRepo string) PerRepoConfigWriter {
Version: "1",
Roles: roles,
AllowedRemoteResources: DefaultAllowedRemoteResources(),
parent: &perRepoDefaults{},
}
if targetRepo != "" {
cfg.CreateIssues = &CreateIssuesConfig{
Expand All @@ -539,6 +557,7 @@ func ParsePerRepoConfig(data []byte) (PerRepoConfigReader, error) {
if err := yaml.Unmarshal(data, &cfg); err != nil {
return nil, fmt.Errorf("parsing per-repo config: %w", err)
}
cfg.parent = &perRepoDefaults{}
return &cfg, nil
}

Expand All @@ -549,6 +568,7 @@ func ParsePerRepoConfigWriter(data []byte) (ConfigWriter, error) {
if err := yaml.Unmarshal(data, &cfg); err != nil {
return nil, fmt.Errorf("parsing per-repo config: %w", err)
}
cfg.parent = &perRepoDefaults{}
return &cfg, nil
}

Expand All @@ -562,22 +582,33 @@ func (c *perRepoConfig) Marshal() ([]byte, error) {
}

// Validate checks the PerRepoConfig for structural correctness.
// Locally-set fields are validated; resolved values (e.g.,
Comment thread
ifireball marked this conversation as resolved.
Resolved
// AllowedResources) are used where validation requires the full
// effective config.
func (c *perRepoConfig) Validate() error {
if c.Version != "1" {
// Version: empty means "inherit from parent"; non-empty must be "1".
if c.Version != "" && c.Version != "1" {
return fmt.Errorf("unsupported version %q: must be \"1\"", c.Version)
}
valid := ValidRoles()
seen := make(map[string]bool, len(c.Roles))
for _, role := range c.Roles {
if !slices.Contains(valid, role) {
return fmt.Errorf("invalid role %q: must be one of %s", role, strings.Join(valid, ", "))
}
if seen[role] {
return fmt.Errorf("duplicate role %q in roles", role)
// Roles: nil means "inherit from parent"; non-nil (including empty)
// is locally set and validated.
if c.Roles != nil {
valid := ValidRoles()
seen := make(map[string]bool, len(c.Roles))
for _, role := range c.Roles {
if !slices.Contains(valid, role) {
return fmt.Errorf("invalid role %q: must be one of %s", role, strings.Join(valid, ", "))
}
if seen[role] {
return fmt.Errorf("duplicate role %q in roles", role)
}
seen[role] = true
}
seen[role] = true
}
if err := ValidateAgentEntries(c.Agents, c.AllowedRemoteResources); err != nil {
// Agents are validated against the resolved allowlist (including
// parent resources) so that URL agents covered by a parent or
// default prefix pass validation.
if err := ValidateAgentEntries(c.Agents, c.AllowedResources()); err != nil {
return err
}
if err := validateCreateIssues(c.CreateIssues); err != nil {
Expand Down
14 changes: 12 additions & 2 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,12 @@ allowed_remote_resources:
require.Len(t, cfg.AgentEntries(), 2)
assert.Contains(t, cfg.AgentEntries()[0].Source, "triage.yaml")
assert.Equal(t, "lint", cfg.AgentEntries()[1].Name)
assert.Equal(t, []string{"https://raw.githubusercontent.com/fullsend-ai/agents/"}, cfg.AllowedResources())
// AllowedResources now unions with parent defaults (code defaults).
resources := cfg.AllowedResources()
assert.Contains(t, resources, "https://raw.githubusercontent.com/fullsend-ai/agents/")
for _, d := range DefaultAllowedRemoteResources() {
assert.Contains(t, resources, d)
}
}

func TestPerRepoConfig_Validate_WithAgents(t *testing.T) {
Expand Down Expand Up @@ -1894,7 +1899,12 @@ func TestPerRepoConfig_RoundTrip_WithAgents(t *testing.T) {
require.Len(t, parsed.AgentEntries(), 2)
assert.Equal(t, original.Agents[0].Source, parsed.AgentEntries()[0].Source)
assert.Equal(t, original.Agents[1].Name, parsed.AgentEntries()[1].Name)
assert.Equal(t, original.AllowedRemoteResources, parsed.AllowedResources())
// Parsed config has a parent so AllowedResources unions with
// code defaults. Verify local resource is present.
resources := parsed.AllowedResources()
assert.Contains(t, resources, "https://example.com/")
// Verify the raw struct field was preserved.
assert.Equal(t, original.AllowedRemoteResources, parsed.(*perRepoConfig).AllowedRemoteResources)
}

func TestOrgConfig_RoundTrip_WithAgents(t *testing.T) {
Expand Down
36 changes: 36 additions & 0 deletions internal/config/defaults.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package config

// perRepoDefaults implements PerRepoConfigReader with compiled-in
// code defaults. It serves as the terminal node in the parent
// fallback chain (ADR 0069 Decision 2).
//
// Lookup order for all accessors: overlay -> base -> code defaults.
// perRepoDefaults is the "code defaults" layer.
type perRepoDefaults struct{}

// Compile-time assertion that perRepoDefaults satisfies PerRepoConfigReader.
var _ PerRepoConfigReader = (*perRepoDefaults)(nil)

// ConfigVersion returns the default schema version.
func (d *perRepoDefaults) ConfigVersion() string { return "1" }

// ConfigRuntime returns the default runtime.
func (d *perRepoDefaults) ConfigRuntime() string { return "claude" }

// IsKillSwitchActive returns the default kill switch state (off).
func (d *perRepoDefaults) IsKillSwitchActive() bool { return false }

// ConfigRoles returns the default agent roles.
func (d *perRepoDefaults) ConfigRoles() []string { return PerRepoDefaultRoles() }

// AgentEntries returns nil — no agents are configured by default.
func (d *perRepoDefaults) AgentEntries() []AgentEntry { return nil }

// AllowedResources returns the default allowed remote resource prefixes.
func (d *perRepoDefaults) AllowedResources() []string { return DefaultAllowedRemoteResources() }

// IssueCreationConfig returns nil — no issue creation config by default.
func (d *perRepoDefaults) IssueCreationConfig() *CreateIssuesConfig { return nil }

// IsOrgMode returns false — per-repo configs are never org mode.
func (d *perRepoDefaults) IsOrgMode() bool { return false }
Loading
Loading