From 7a6c50bc9e949993c5c369bc5011c3d78cdff93b Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 27 Apr 2026 20:26:01 +0200 Subject: [PATCH 1/8] Add settings schema v2 type definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the new typed settings shape that will replace the legacy EntireSettings struct. No loader, parser, or call sites are touched yet — types only. Key shape changes vs. legacy: - Explicit "schema" version marker - Backend selection via Checkpoints.Primary + Mirrors instead of the untyped strategy_options{checkpoints_version, gmeta, ...} soup - Concerns grouped: Logging, Checkpoints, Hooks, Features - SummaryGeneration consolidates Provider/Model/Timeout - Deprecated knobs (checkpoints_v2 alias, push_v2_refs) absent; legacy synthesizer translates them in a follow-up commit. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 2c8df96fc030 --- cmd/entire/cli/settings/schema_v2.go | 164 +++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 cmd/entire/cli/settings/schema_v2.go diff --git a/cmd/entire/cli/settings/schema_v2.go b/cmd/entire/cli/settings/schema_v2.go new file mode 100644 index 0000000000..5b2c5ac5f6 --- /dev/null +++ b/cmd/entire/cli/settings/schema_v2.go @@ -0,0 +1,164 @@ +// This file defines schema v2 of the Entire settings file: a typed, +// concern-grouped replacement for the legacy EntireSettings struct. +// +// The two shapes coexist during migration. Existing callers continue using +// Load()/EntireSettings; new code can opt into LoadV2()/Settings. A future +// phase migrates call sites and retires the legacy struct. +// +// Key differences from legacy: +// - Explicit "schema" version marker (vs. implicit/unmarked) +// - Typed checkpoints config (vs. untyped strategy_options map) +// - Backend selection is structured: primary + optional mirrors +// - Concerns grouped: logging, checkpoints, hooks, features +// - Deprecated keys (checkpoints_v2 alias, push_v2_refs) are not +// represented in the new shape; the synthesizer translates them. +// +// Migration mapping is implemented in synthesizeFromLegacy (see load_v2.go). + +package settings + +// CurrentSchemaVersion is the schema version emitted by v2 writers. +// Files marked with this version are parsed via the v2 path; older files +// are loaded via the legacy parser and synthesized into Settings on the fly. +const CurrentSchemaVersion = 2 + +// Settings is the schema-v2 representation of .entire/settings.json. +// +// All fields are JSON-omitempty where defaults are well-defined so that +// hand-written settings files stay minimal. Pointer types are used where +// "unset" must be distinguishable from "explicit false" (e.g. Telemetry, +// SignCommits, PushSessions). +type Settings struct { + // Schema identifies the settings file schema version. Always 2 in this struct. + Schema int `json:"schema"` + + // Enabled indicates whether Entire is active. When false, CLI commands + // show a disabled message and hooks exit silently. Defaults to true. + Enabled bool `json:"enabled"` + + // LocalDev indicates whether to use "go run" instead of the "entire" + // binary. Used during development when the binary is not installed. + LocalDev bool `json:"local_dev,omitempty"` + + // Logging configures runtime logging. + Logging LoggingConfig `json:"logging,omitempty"` + + // Checkpoints configures permanent checkpoint storage and related ops. + Checkpoints CheckpointsConfig `json:"checkpoints,omitempty"` + + // Hooks configures git hook behavior. + Hooks HooksConfig `json:"hooks,omitempty"` + + // Features toggles optional behaviors. + Features FeaturesConfig `json:"features,omitempty"` + + // Redaction configures PII redaction beyond the default secret detection. + Redaction *RedactionSettings `json:"redaction,omitempty"` + + // Telemetry controls anonymous usage analytics. + // nil = not asked yet, true = opted in, false = opted out. + Telemetry *bool `json:"telemetry,omitempty"` + + // SummaryGeneration configures provider selection and timeout for + // `entire explain --generate`. + SummaryGeneration *SummaryGenerationConfig `json:"summary_generation,omitempty"` +} + +// LoggingConfig configures runtime logging verbosity. +type LoggingConfig struct { + // Level is the logging verbosity (debug, info, warn, error). + // Can be overridden by ENTIRE_LOG_LEVEL. Defaults to "info" when unset. + Level string `json:"level,omitempty"` +} + +// CheckpointsConfig configures checkpoint storage and related operations. +// +// Primary serves all reads and is the authoritative writer. Mirrors receive +// best-effort fan-out writes (warn on failure, never serve reads). This +// shape replaces the legacy strategy_options{checkpoints_version, gmeta, ...} +// soup with explicit, typed selection. +type CheckpointsConfig struct { + // Primary is the authoritative checkpoint backend. + // Reads always come from Primary. Writes that fail here are fatal. + Primary BackendConfig `json:"primary"` + + // Mirrors are best-effort write targets. + // Mirror failures are logged but do not fail the operation. Mirrors + // never serve reads — they are export targets, not sources of truth. + Mirrors []BackendConfig `json:"mirrors,omitempty"` + + // Remote configures the GitHub remote that hosts the checkpoint + // metadata branch. Optional; when unset, the default origin is used. + Remote *CheckpointRemoteConfig `json:"remote,omitempty"` + + // FullTranscriptRetentionDays is the retention window (in days) for + // archived raw-transcript generations. Zero/negative falls back to + // the documented default (60 days). + FullTranscriptRetentionDays int `json:"full_transcript_retention_days,omitempty"` + + // SignCommits controls whether checkpoint commits are signed. + // nil/true = sign (default), false = skip signing. + SignCommits *bool `json:"sign_commits,omitempty"` + + // FilteredFetches enables --filter=blob:none on checkpoint fetches. + FilteredFetches bool `json:"filtered_fetches,omitempty"` + + // PushSessions controls whether session refs are pushed to remotes. + // nil = push (default), explicit false = do not push. + PushSessions *bool `json:"push_sessions,omitempty"` +} + +// BackendConfig identifies and configures a single checkpoint backend. +// +// The Type field selects the backend implementation (e.g. "v1", "v2", +// "gmeta"). Backend-specific configuration is added as additional fields +// here as backends are introduced (e.g. an "s3" backend would carry +// bucket/region; today every supported backend needs only Type). +type BackendConfig struct { + // Type is the backend identifier. Recognized values: "v1", "v2", "gmeta". + Type string `json:"type"` +} + +// HooksConfig configures git hook behavior. +type HooksConfig struct { + // CommitLinking controls how commits are linked to agent sessions. + // "always" = auto-link without prompting, "prompt" = ask each commit. + // Defaults to "prompt" when unset. + CommitLinking string `json:"commit_linking,omitempty"` + + // AbsoluteGitHookPath embeds the full binary path in git hooks instead + // of bare "entire". Needed for GUI git clients (Xcode, Tower, etc.) + // that don't source shell profiles and can't find "entire" on PATH. + AbsoluteGitHookPath bool `json:"absolute_git_hook_path,omitempty"` +} + +// FeaturesConfig toggles optional product behaviors. +type FeaturesConfig struct { + // Summarize enables AI-generated checkpoint summaries. + Summarize bool `json:"summarize,omitempty"` + + // ExternalAgents enables discovery and registration of external agent + // plugins (entire-agent-* binaries on $PATH). + ExternalAgents bool `json:"external_agents,omitempty"` + + // Vercel marks the repository as using Vercel; the metadata branch + // then includes a vercel.json that disables deployments for Entire branches. + Vercel bool `json:"vercel,omitempty"` +} + +// SummaryGenerationConfig configures `entire explain --generate`. +// +// Replaces legacy SummaryGenerationSettings + the top-level +// SummaryTimeoutSeconds field, grouping all summary-generation knobs. +type SummaryGenerationConfig struct { + // Provider is the agent name for summary generation + // (e.g. "claude-code", "codex", "gemini"). + Provider string `json:"provider,omitempty"` + + // Model is an optional model hint passed to the selected provider. + Model string `json:"model,omitempty"` + + // TimeoutSeconds is an optional hard deadline for summary generation. + // Zero or negative means "unset" — the caller picks the default. + TimeoutSeconds int `json:"timeout_seconds,omitempty"` +} From 39b84a51bf22c0fb49afc2e6f3b06b6830e15824 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 27 Apr 2026 20:36:07 +0200 Subject: [PATCH 2/8] Add LoadV2 with legacy synthesizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the schema-v2 settings loader alongside the legacy Load(). Both coexist: existing call sites continue using Load()/EntireSettings, new code can opt into LoadV2()/Settings as it migrates. Loader behavior: - Files declaring "schema": 2 are parsed natively as Settings. - Older files are loaded via the legacy parser and mapped into Settings via synthesizeFromLegacy. - Mixed-shape settings (one v2, one legacy across main/local) are rejected with a clear error suggesting `entire migrate-config`. The synthesizer is the single source of truth for legacy → v2 mapping and will be reused by `entire migrate-config` to rewrite settings.json. Mappings cover every legacy strategy_options key in current use plus top-level fields. Drops push_v2_refs (no v2 destination; will become a backend property of CheckpointSyncer in a later phase) and the deprecated `strategy` tolerator field. Tests cover the full migration matrix (28 cases across backend selection, checkpoint sub-fields, top-level fields, summary generation), file-level scenarios (missing/legacy/v2/mixed), bytes parsing, and a round-trip from raw legacy JSON through LoadFromBytes into the synthesizer. Adds JSON tags to CheckpointRemoteConfig so the same type can be embedded under Settings.Checkpoints.Remote. Legacy paths read this type from a map and are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 9c9670e3489f --- cmd/entire/cli/settings/load_v2.go | 380 ++++++++++++++ cmd/entire/cli/settings/load_v2_test.go | 653 ++++++++++++++++++++++++ cmd/entire/cli/settings/settings.go | 6 +- 3 files changed, 1037 insertions(+), 2 deletions(-) create mode 100644 cmd/entire/cli/settings/load_v2.go create mode 100644 cmd/entire/cli/settings/load_v2_test.go diff --git a/cmd/entire/cli/settings/load_v2.go b/cmd/entire/cli/settings/load_v2.go new file mode 100644 index 0000000000..a106cc4c14 --- /dev/null +++ b/cmd/entire/cli/settings/load_v2.go @@ -0,0 +1,380 @@ +// This file implements LoadV2 and the legacy → v2 synthesizer. +// +// Loading rules: +// - If .entire/settings.json declares "schema": 2, the file is parsed +// directly as Settings. A local override file, if present, must also +// declare "schema": 2 (mixed-shape settings are rejected with a +// descriptive error). +// - Otherwise, the legacy Load() path runs and its EntireSettings result +// is mapped into Settings via synthesizeFromLegacy. +// +// The synthesizer is the single source of truth for legacy → v2 mapping +// and is the implementation that `entire migrate-config` will reuse to +// rewrite settings.json into the new shape. + +package settings + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + + "github.com/entireio/cli/cmd/entire/cli/paths" +) + +// Backend type constants used in BackendConfig.Type. +const ( + BackendTypeV1 = "v1" + BackendTypeV2 = "v2" + BackendTypeGmeta = "gmeta" +) + +// LoadV2 loads settings as the v2 shape. Files declaring "schema": 2 are +// parsed natively; older files are loaded via the legacy parser and +// synthesized. The local override file is merged the same way. +// +// Returns Settings with sensible defaults populated (Schema=2, Enabled=true) +// if neither settings file exists. +func LoadV2(ctx context.Context) (*Settings, error) { + mainAbs, err := paths.AbsPath(ctx, EntireSettingsFile) + if err != nil { + mainAbs = EntireSettingsFile + } + localAbs, err := paths.AbsPath(ctx, EntireSettingsLocalFile) + if err != nil { + localAbs = EntireSettingsLocalFile + } + + mainData, mainErr := readSettingsFileIfExists(mainAbs) + if mainErr != nil { + return nil, fmt.Errorf("reading settings file: %w", mainErr) + } + localData, localErr := readSettingsFileIfExists(localAbs) + if localErr != nil { + return nil, fmt.Errorf("reading local settings file: %w", localErr) + } + + mainIsV2 := isSchemaV2(mainData) + localIsV2 := isSchemaV2(localData) + + switch { + case mainData == nil && localData == nil: + return defaultSettings(), nil + + case mixedShapesRejected(mainData, localData, mainIsV2, localIsV2): + return nil, errors.New("mixed schema versions in settings files; run `entire migrate-config` to convert all settings to schema 2") + + case mainIsV2 || localIsV2: + return loadFromV2Files(mainData, localData) + + default: + // Both files (if present) are legacy shape. Defer to the legacy + // loader to handle merging, then synthesize into v2. + legacy, err := loadMergedSettings(mainAbs, localAbs) + if err != nil { + return nil, err + } + return synthesizeFromLegacy(legacy), nil + } +} + +// LoadV2FromBytes parses a single settings document from raw bytes. +// Accepts either schema-v2 JSON or legacy JSON; the shape is auto-detected +// via the schema field. No local-override merge is performed. +func LoadV2FromBytes(data []byte) (*Settings, error) { + if len(data) == 0 { + return defaultSettings(), nil + } + if isSchemaV2(data) { + return parseV2Bytes(data) + } + legacy, err := LoadFromBytes(data) + if err != nil { + return nil, err + } + return synthesizeFromLegacy(legacy), nil +} + +// readSettingsFileIfExists returns the file contents, or (nil, nil) if the +// file does not exist. Other I/O errors propagate. +func readSettingsFileIfExists(path string) ([]byte, error) { + data, err := os.ReadFile(path) //nolint:gosec // path is from AbsPath or constant + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err //nolint:wrapcheck // caller wraps with file context + } + return data, nil +} + +// isSchemaV2 reports whether a settings JSON document declares schema >= 2. +// Returns false for nil/empty input or invalid JSON; callers handle those +// cases explicitly downstream. +func isSchemaV2(data []byte) bool { + if len(data) == 0 { + return false + } + var probe struct { + Schema int `json:"schema"` + } + if err := json.Unmarshal(data, &probe); err != nil { + return false + } + return probe.Schema >= CurrentSchemaVersion +} + +// mixedShapesRejected reports the only configuration we treat as an error: +// exactly one of (main, local) declares schema 2 while the other is legacy. +// Two missing files or two same-shape files are both fine. +func mixedShapesRejected(mainData, localData []byte, mainIsV2, localIsV2 bool) bool { + bothPresent := mainData != nil && localData != nil + return bothPresent && (mainIsV2 != localIsV2) +} + +// loadFromV2Files parses one or both files as schema-v2 JSON and applies the +// local override on top of the main settings. At least one of the inputs is +// guaranteed by the caller to be a v2 document. +func loadFromV2Files(mainData, localData []byte) (*Settings, error) { + settings := defaultSettings() + if mainData != nil { + parsed, err := parseV2Bytes(mainData) + if err != nil { + return nil, fmt.Errorf("parsing settings file: %w", err) + } + settings = parsed + } + if localData != nil { + if err := mergeV2Override(settings, localData); err != nil { + return nil, fmt.Errorf("merging local settings: %w", err) + } + } + return settings, nil +} + +// parseV2Bytes parses a single schema-v2 JSON document into Settings. +// Strict decoding rejects unknown fields so typos become loud failures. +func parseV2Bytes(data []byte) (*Settings, error) { + settings := defaultSettings() + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(settings); err != nil { + return nil, fmt.Errorf("parsing settings: %w", err) + } + return settings, nil +} + +// mergeV2Override applies local overrides on top of an existing Settings +// value. Field-level merge semantics: only fields explicitly present in +// the override JSON change the base. +// +// For now this delegates to a full reparse of the local file and replaces +// top-level groups wholesale. That is enough for the local-override use +// cases in the codebase today (toggling log level, enabling local_dev). +// More granular per-field merging can be added when a real call site needs +// it; the legacy code only got more granular for redaction/PII fields, +// and we will revisit that if the ergonomics become a problem. +func mergeV2Override(base *Settings, data []byte) error { + override, err := parseV2Bytes(data) + if err != nil { + return err + } + + var presence map[string]json.RawMessage + if err := json.Unmarshal(data, &presence); err != nil { + return fmt.Errorf("parsing local settings: %w", err) + } + + if _, ok := presence["enabled"]; ok { + base.Enabled = override.Enabled + } + if _, ok := presence["local_dev"]; ok { + base.LocalDev = override.LocalDev + } + if _, ok := presence["logging"]; ok { + base.Logging = override.Logging + } + if _, ok := presence["checkpoints"]; ok { + base.Checkpoints = override.Checkpoints + } + if _, ok := presence["hooks"]; ok { + base.Hooks = override.Hooks + } + if _, ok := presence["features"]; ok { + base.Features = override.Features + } + if _, ok := presence["redaction"]; ok { + base.Redaction = override.Redaction + } + if _, ok := presence["telemetry"]; ok { + base.Telemetry = override.Telemetry + } + if _, ok := presence["summary_generation"]; ok { + base.SummaryGeneration = override.SummaryGeneration + } + return nil +} + +// defaultSettings returns the zero-value Settings with documented defaults +// applied (Schema = 2, Enabled = true). Callers should treat this as the +// starting point before parsing a file or merging overrides. +func defaultSettings() *Settings { + return &Settings{ + Schema: CurrentSchemaVersion, + Enabled: true, + } +} + +// synthesizeFromLegacy maps an EntireSettings into a Settings, encoding the +// legacy → v2 migration. This is the single source of truth for the +// translation and is reused by `entire migrate-config`. +// +// Mappings: +// - strategy_options.checkpoints_version=2 OR checkpoints_v2=true +// → Checkpoints.Primary.Type = "v2" +// - otherwise → Checkpoints.Primary.Type = "v1" +// - strategy_options.gmeta=true → append {type: "gmeta"} to Mirrors +// - strategy_options.checkpoint_remote → Checkpoints.Remote +// - strategy_options.full_transcript_generation_retention_days +// → Checkpoints.FullTranscriptRetentionDays +// - strategy_options.filtered_fetches → Checkpoints.FilteredFetches +// - strategy_options.push_sessions (explicit) → Checkpoints.PushSessions +// - strategy_options.summarize.enabled → Features.Summarize +// - log_level → Logging.Level +// - sign_checkpoint_commits → Checkpoints.SignCommits +// - commit_linking → Hooks.CommitLinking +// - absolute_git_hook_path → Hooks.AbsoluteGitHookPath +// - external_agents → Features.ExternalAgents +// - vercel → Features.Vercel +// - summary_generation.{provider,model} + summary_timeout_seconds +// → SummaryGeneration.{Provider,Model,TimeoutSeconds} +// +// Dropped (no v2 destination): +// - push_v2_refs (was a dual-write transition knob; v2 ref pushes are now +// a property of the v2 backend's CheckpointSyncer, not a settings flag) +// - strategy (deprecated tolerator field) +func synthesizeFromLegacy(s *EntireSettings) *Settings { + if s == nil { + return defaultSettings() + } + + out := &Settings{ + Schema: CurrentSchemaVersion, + Enabled: s.Enabled, + LocalDev: s.LocalDev, + Logging: LoggingConfig{ + Level: s.LogLevel, + }, + Hooks: HooksConfig{ + CommitLinking: s.CommitLinking, + AbsoluteGitHookPath: s.AbsoluteGitHookPath, + }, + Features: FeaturesConfig{ + Summarize: s.IsSummarizeEnabled(), + ExternalAgents: s.ExternalAgents, + Vercel: s.Vercel, + }, + Redaction: s.Redaction, + Telemetry: s.Telemetry, + } + + out.Checkpoints = synthesizeCheckpointsConfig(s) + + if hasSummaryGeneration(s) { + out.SummaryGeneration = &SummaryGenerationConfig{ + TimeoutSeconds: s.SummaryTimeoutSeconds, + } + if s.SummaryGeneration != nil { + out.SummaryGeneration.Provider = s.SummaryGeneration.Provider + out.SummaryGeneration.Model = s.SummaryGeneration.Model + } + } + + return out +} + +// synthesizeCheckpointsConfig builds the v2 Checkpoints group from the +// legacy EntireSettings. Split out for readability since the mapping pulls +// from both top-level fields and the strategy_options soup. +func synthesizeCheckpointsConfig(s *EntireSettings) CheckpointsConfig { + cfg := CheckpointsConfig{ + Primary: BackendConfig{Type: legacyPrimaryBackend(s)}, + Remote: s.GetCheckpointRemote(), + FullTranscriptRetentionDays: legacyFullTranscriptRetention(s), + SignCommits: s.SignCheckpointCommits, + FilteredFetches: s.IsFilteredFetchesEnabled(), + PushSessions: legacyPushSessions(s), + } + + if legacyGmetaEnabled(s) { + cfg.Mirrors = append(cfg.Mirrors, BackendConfig{Type: BackendTypeGmeta}) + } + + return cfg +} + +// legacyPrimaryBackend returns "v2" if either checkpoints_version or +// checkpoints_v2 enables v2 in the legacy settings, "v1" otherwise. +func legacyPrimaryBackend(s *EntireSettings) string { + if s.IsCheckpointsV2Enabled() { + return BackendTypeV2 + } + return BackendTypeV1 +} + +// legacyGmetaEnabled checks the legacy strategy_options.gmeta flag. The +// flag itself does not exist on this branch yet (it ships with the gmeta +// mainport), but the synthesizer recognizes it ahead of time so settings +// files written by that branch round-trip cleanly through Phase 0. +func legacyGmetaEnabled(s *EntireSettings) bool { + if s.StrategyOptions == nil { + return false + } + val, ok := s.StrategyOptions["gmeta"].(bool) + return ok && val +} + +// legacyFullTranscriptRetention returns the configured retention only when +// the legacy key is explicitly set; the legacy accessor returns a default +// when unset, which we suppress here so the v2 file stays minimal. +func legacyFullTranscriptRetention(s *EntireSettings) int { + if s.StrategyOptions == nil { + return 0 + } + if _, ok := s.StrategyOptions["full_transcript_generation_retention_days"]; !ok { + return 0 + } + return s.GetFullTranscriptGenerationRetentionDays() +} + +// legacyPushSessions extracts the explicit push_sessions boolean if set; +// otherwise returns nil so the v2 file stays minimal. The legacy semantics +// were "explicit false disables, otherwise default behavior", which we +// preserve by using a *bool here. +func legacyPushSessions(s *EntireSettings) *bool { + if s.StrategyOptions == nil { + return nil + } + val, ok := s.StrategyOptions["push_sessions"] + if !ok { + return nil + } + b, ok := val.(bool) + if !ok { + return nil + } + return &b +} + +// hasSummaryGeneration reports whether any summary-generation field on the +// legacy settings is non-default. Used to decide whether to allocate the +// v2 SummaryGenerationConfig at all (vs. leaving it nil). +func hasSummaryGeneration(s *EntireSettings) bool { + if s.SummaryGeneration != nil && (s.SummaryGeneration.Provider != "" || s.SummaryGeneration.Model != "") { + return true + } + return s.SummaryTimeoutSeconds > 0 +} diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go new file mode 100644 index 0000000000..92d18e0212 --- /dev/null +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -0,0 +1,653 @@ +package settings + +import ( + "context" + "reflect" + "strings" + "testing" +) + +const ( + debugLevel = "debug" + providerClaudeCC = "claude-code" +) + +func boolPtr(b bool) *bool { return &b } + +type synthCase struct { + name string + legacy *EntireSettings + check func(t *testing.T, got *Settings) +} + +func runSynthCases(t *testing.T, cases []synthCase) { + t.Helper() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := synthesizeFromLegacy(tc.legacy) + if got == nil { + t.Fatal("synthesizeFromLegacy returned nil") + } + if got.Schema != CurrentSchemaVersion { + t.Fatalf("Schema = %d, want %d", got.Schema, CurrentSchemaVersion) + } + tc.check(t, got) + }) + } +} + +// TestSynthesizeFromLegacy_BackendSelection covers how the Primary backend +// type and Mirrors are derived from legacy strategy_options. +func TestSynthesizeFromLegacy_BackendSelection(t *testing.T) { + t.Parallel() + runSynthCases(t, []synthCase{ + { + name: "nil legacy returns defaults", + legacy: nil, + check: func(t *testing.T, got *Settings) { + if !got.Enabled { + t.Fatal("Enabled = false, want true") + } + }, + }, + { + name: "empty legacy → primary v1, no mirrors", + legacy: &EntireSettings{Enabled: true}, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV1 { + t.Fatalf("Primary.Type = %q, want %q", got.Checkpoints.Primary.Type, BackendTypeV1) + } + if len(got.Checkpoints.Mirrors) != 0 { + t.Fatalf("Mirrors = %v, want empty", got.Checkpoints.Mirrors) + } + }, + }, + { + name: "checkpoints_v2 alone → primary v2", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_v2": true}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want %q", got.Checkpoints.Primary.Type, BackendTypeV2) + } + }, + }, + { + name: "checkpoints_version=2 → primary v2", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_version": float64(2)}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want %q", got.Checkpoints.Primary.Type, BackendTypeV2) + } + }, + }, + { + name: "checkpoints_version=1 → primary v1", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_version": float64(1)}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV1 { + t.Fatalf("Primary.Type = %q, want %q", got.Checkpoints.Primary.Type, BackendTypeV1) + } + }, + }, + { + name: "gmeta alone → primary v1, gmeta mirror", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"gmeta": true}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV1 { + t.Fatalf("Primary.Type = %q, want v1", got.Checkpoints.Primary.Type) + } + if len(got.Checkpoints.Mirrors) != 1 || got.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [{gmeta}]", got.Checkpoints.Mirrors) + } + }, + }, + { + name: "checkpoints_v2 + gmeta → primary v2, gmeta mirror", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{ + "checkpoints_v2": true, + "gmeta": true, + }, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } + if len(got.Checkpoints.Mirrors) != 1 || got.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [{gmeta}]", got.Checkpoints.Mirrors) + } + }, + }, + { + name: "push_v2_refs is silently dropped (no v2 destination)", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{ + "checkpoints_v2": true, + "push_v2_refs": true, + }, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } + }, + }, + { + name: "deprecated 'strategy' field is silently dropped", + legacy: &EntireSettings{ + Enabled: true, + Strategy: "auto-commit", + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Primary.Type != BackendTypeV1 { + t.Fatalf("Primary.Type = %q, want v1", got.Checkpoints.Primary.Type) + } + }, + }, + }) +} + +// TestSynthesizeFromLegacy_CheckpointFields covers Checkpoints sub-fields +// other than Primary/Mirrors: remote, retention, signing, filtered fetches, +// push_sessions tri-state. +func TestSynthesizeFromLegacy_CheckpointFields(t *testing.T) { + t.Parallel() + runSynthCases(t, []synthCase{ + { + name: "checkpoint_remote → checkpoints.remote", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{ + "checkpoint_remote": map[string]any{ + "provider": "github", + "repo": "org/checkpoints", + }, + }, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.Remote == nil { + t.Fatal("Remote = nil, want populated") + } + if got.Checkpoints.Remote.Provider != "github" || got.Checkpoints.Remote.Repo != "org/checkpoints" { + t.Fatalf("Remote = %+v, want github/org/checkpoints", got.Checkpoints.Remote) + } + }, + }, + { + name: "filtered_fetches → checkpoints.filtered_fetches", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"filtered_fetches": true}, + }, + check: func(t *testing.T, got *Settings) { + if !got.Checkpoints.FilteredFetches { + t.Fatal("FilteredFetches = false, want true") + } + }, + }, + { + name: "push_sessions=false → explicit false pointer", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"push_sessions": false}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.PushSessions == nil || *got.Checkpoints.PushSessions { + t.Fatalf("PushSessions = %v, want explicit false", got.Checkpoints.PushSessions) + } + }, + }, + { + name: "push_sessions=true → explicit true pointer", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"push_sessions": true}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.PushSessions == nil || !*got.Checkpoints.PushSessions { + t.Fatalf("PushSessions = %v, want explicit true", got.Checkpoints.PushSessions) + } + }, + }, + { + name: "push_sessions absent → nil pointer", + legacy: &EntireSettings{Enabled: true}, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.PushSessions != nil { + t.Fatalf("PushSessions = %v, want nil", got.Checkpoints.PushSessions) + } + }, + }, + { + name: "full_transcript_generation_retention_days set → preserved", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"full_transcript_generation_retention_days": float64(90)}, + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.FullTranscriptRetentionDays != 90 { + t.Fatalf("FullTranscriptRetentionDays = %d, want 90", got.Checkpoints.FullTranscriptRetentionDays) + } + }, + }, + { + name: "full_transcript_generation_retention_days absent → 0", + legacy: &EntireSettings{Enabled: true}, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.FullTranscriptRetentionDays != 0 { + t.Fatalf("FullTranscriptRetentionDays = %d, want 0", got.Checkpoints.FullTranscriptRetentionDays) + } + }, + }, + { + name: "sign_checkpoint_commits=false → checkpoints.sign_commits=false", + legacy: &EntireSettings{ + Enabled: true, + SignCheckpointCommits: boolPtr(false), + }, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.SignCommits == nil || *got.Checkpoints.SignCommits { + t.Fatalf("SignCommits = %v, want explicit false", got.Checkpoints.SignCommits) + } + }, + }, + { + name: "sign_checkpoint_commits unset → checkpoints.sign_commits nil", + legacy: &EntireSettings{Enabled: true}, + check: func(t *testing.T, got *Settings) { + if got.Checkpoints.SignCommits != nil { + t.Fatalf("SignCommits = %v, want nil", got.Checkpoints.SignCommits) + } + }, + }, + }) +} + +// TestSynthesizeFromLegacy_TopLevelFields covers logging, hooks, features, +// redaction, and telemetry mappings. +func TestSynthesizeFromLegacy_TopLevelFields(t *testing.T) { + t.Parallel() + runSynthCases(t, []synthCase{ + { + name: "log_level → logging.level", + legacy: &EntireSettings{ + Enabled: true, + LogLevel: debugLevel, + }, + check: func(t *testing.T, got *Settings) { + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q", got.Logging.Level, debugLevel) + } + }, + }, + { + name: "summarize → features.summarize", + legacy: &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{ + "summarize": map[string]any{"enabled": true}, + }, + }, + check: func(t *testing.T, got *Settings) { + if !got.Features.Summarize { + t.Fatal("Features.Summarize = false, want true") + } + }, + }, + { + name: "external_agents → features.external_agents", + legacy: &EntireSettings{ + Enabled: true, + ExternalAgents: true, + }, + check: func(t *testing.T, got *Settings) { + if !got.Features.ExternalAgents { + t.Fatal("Features.ExternalAgents = false, want true") + } + }, + }, + { + name: "vercel → features.vercel", + legacy: &EntireSettings{ + Enabled: true, + Vercel: true, + }, + check: func(t *testing.T, got *Settings) { + if !got.Features.Vercel { + t.Fatal("Features.Vercel = false, want true") + } + }, + }, + { + name: "commit_linking → hooks.commit_linking", + legacy: &EntireSettings{ + Enabled: true, + CommitLinking: CommitLinkingAlways, + }, + check: func(t *testing.T, got *Settings) { + if got.Hooks.CommitLinking != CommitLinkingAlways { + t.Fatalf("Hooks.CommitLinking = %q, want %q", got.Hooks.CommitLinking, CommitLinkingAlways) + } + }, + }, + { + name: "absolute_git_hook_path → hooks.absolute_git_hook_path", + legacy: &EntireSettings{ + Enabled: true, + AbsoluteGitHookPath: true, + }, + check: func(t *testing.T, got *Settings) { + if !got.Hooks.AbsoluteGitHookPath { + t.Fatal("Hooks.AbsoluteGitHookPath = false, want true") + } + }, + }, + { + name: "redaction passes through", + legacy: &EntireSettings{ + Enabled: true, + Redaction: &RedactionSettings{ + PII: &PIISettings{Enabled: true, Email: boolPtr(true)}, + }, + }, + check: func(t *testing.T, got *Settings) { + if got.Redaction == nil || got.Redaction.PII == nil || !got.Redaction.PII.Enabled { + t.Fatalf("Redaction = %+v, want PII enabled", got.Redaction) + } + }, + }, + { + name: "telemetry pointer passes through", + legacy: &EntireSettings{ + Enabled: true, + Telemetry: boolPtr(true), + }, + check: func(t *testing.T, got *Settings) { + if got.Telemetry == nil || !*got.Telemetry { + t.Fatalf("Telemetry = %v, want explicit true", got.Telemetry) + } + }, + }, + }) +} + +// TestSynthesizeFromLegacy_SummaryGeneration covers the summary_generation +// group consolidation (provider, model, timeout). +func TestSynthesizeFromLegacy_SummaryGeneration(t *testing.T) { + t.Parallel() + runSynthCases(t, []synthCase{ + { + name: "summary_timeout_seconds → summary_generation.timeout_seconds", + legacy: &EntireSettings{ + Enabled: true, + SummaryTimeoutSeconds: 30, + }, + check: func(t *testing.T, got *Settings) { + if got.SummaryGeneration == nil { + t.Fatal("SummaryGeneration = nil, want allocated") + } + if got.SummaryGeneration.TimeoutSeconds != 30 { + t.Fatalf("TimeoutSeconds = %d, want 30", got.SummaryGeneration.TimeoutSeconds) + } + }, + }, + { + name: "provider/model preserved", + legacy: &EntireSettings{ + Enabled: true, + SummaryGeneration: &SummaryGenerationSettings{ + Provider: providerClaudeCC, + Model: "sonnet", + }, + }, + check: func(t *testing.T, got *Settings) { + if got.SummaryGeneration == nil { + t.Fatal("SummaryGeneration = nil") + } + if got.SummaryGeneration.Provider != providerClaudeCC || got.SummaryGeneration.Model != "sonnet" { + t.Fatalf("SummaryGeneration = %+v, want claude-code/sonnet", got.SummaryGeneration) + } + }, + }, + { + name: "no summary fields → summary_generation nil", + legacy: &EntireSettings{Enabled: true}, + check: func(t *testing.T, got *Settings) { + if got.SummaryGeneration != nil { + t.Fatalf("SummaryGeneration = %+v, want nil", got.SummaryGeneration) + } + }, + }, + }) +} + +// TestLoadV2_FileScenarios covers how LoadV2 reads from disk. +// These tests use t.Chdir, so they cannot run in parallel. +func TestLoadV2_NoFiles(t *testing.T) { + setupSettingsDir(t, "", "") + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Schema != CurrentSchemaVersion || !got.Enabled { + t.Fatalf("defaults wrong: %+v", got) + } + if got.Checkpoints.Primary.Type != "" { + t.Fatalf("expected empty primary when no files present, got %q", got.Checkpoints.Primary.Type) + } +} + +func TestLoadV2_LegacyMainOnly(t *testing.T) { + setupSettingsDir(t, `{"enabled": true, "strategy_options": {"checkpoints_v2": true}}`, "") + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } +} + +func TestLoadV2_LegacyMainAndLocal(t *testing.T) { + setupSettingsDir(t, + `{"enabled": true}`, + `{"log_level": "`+debugLevel+`"}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q", got.Logging.Level, debugLevel) + } +} + +func TestLoadV2_V2MainOnly(t *testing.T) { + setupSettingsDir(t, `{ + "schema": 2, + "enabled": true, + "checkpoints": {"primary": {"type": "v2"}, "mirrors": [{"type": "gmeta"}]} + }`, "") + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } + if len(got.Checkpoints.Mirrors) != 1 || got.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [gmeta]", got.Checkpoints.Mirrors) + } +} + +func TestLoadV2_V2MainAndLocalOverride(t *testing.T) { + setupSettingsDir(t, + `{"schema": 2, "enabled": true, "logging": {"level": "info"}}`, + `{"schema": 2, "logging": {"level": "`+debugLevel+`"}}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q (overridden)", got.Logging.Level, debugLevel) + } + if !got.Enabled { + t.Fatal("Enabled should be preserved across override") + } +} + +func TestLoadV2_MixedShapesRejected_V2MainLegacyLocal(t *testing.T) { + setupSettingsDir(t, + `{"schema": 2, "enabled": true}`, + `{"log_level": "`+debugLevel+`"}`, + ) + _, err := LoadV2(context.Background()) + if err == nil { + t.Fatal("expected error for mixed shapes") + } + if !strings.Contains(err.Error(), "mixed schema") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestLoadV2_MixedShapesRejected_LegacyMainV2Local(t *testing.T) { + setupSettingsDir(t, + `{"enabled": true}`, + `{"schema": 2, "logging": {"level": "`+debugLevel+`"}}`, + ) + _, err := LoadV2(context.Background()) + if err == nil { + t.Fatal("expected error for mixed shapes") + } +} + +func TestLoadV2FromBytes_Empty(t *testing.T) { + t.Parallel() + got, err := LoadV2FromBytes(nil) + if err != nil { + t.Fatalf("LoadV2FromBytes(nil): %v", err) + } + if got.Schema != CurrentSchemaVersion || !got.Enabled { + t.Fatalf("defaults wrong: %+v", got) + } +} + +func TestLoadV2FromBytes_V2(t *testing.T) { + t.Parallel() + got, err := LoadV2FromBytes([]byte(`{"schema": 2, "enabled": true, "checkpoints": {"primary": {"type": "v2"}}}`)) + if err != nil { + t.Fatalf("LoadV2FromBytes: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } +} + +func TestLoadV2FromBytes_Legacy(t *testing.T) { + t.Parallel() + got, err := LoadV2FromBytes([]byte(`{"enabled": true, "strategy_options": {"checkpoints_v2": true}}`)) + if err != nil { + t.Fatalf("LoadV2FromBytes: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", got.Checkpoints.Primary.Type) + } +} + +func TestLoadV2FromBytes_UnknownFieldRejected(t *testing.T) { + t.Parallel() + _, err := LoadV2FromBytes([]byte(`{"schema": 2, "totally_unknown": true}`)) + if err == nil { + t.Fatal("expected error for unknown field") + } + if !containsUnknownField(err.Error()) { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestLoadV2FromBytes_InvalidJSON(t *testing.T) { + t.Parallel() + _, err := LoadV2FromBytes([]byte(`{not json`)) + if err == nil { + t.Fatal("expected JSON parse error") + } +} + +// TestSynthesizeFromLegacy_RoundTripFromBytes verifies the fully-loaded +// legacy parser feeds the synthesizer correctly. Catches mismatches between +// accessors and the synthesizer that unit cases on synthesize alone would miss. +func TestSynthesizeFromLegacy_RoundTripFromBytes(t *testing.T) { + t.Parallel() + legacyJSON := `{ + "enabled": true, + "log_level": "` + debugLevel + `", + "commit_linking": "always", + "absolute_git_hook_path": true, + "external_agents": true, + "vercel": true, + "telemetry": true, + "sign_checkpoint_commits": false, + "summary_timeout_seconds": 45, + "summary_generation": {"provider": "` + providerClaudeCC + `", "model": "sonnet"}, + "redaction": {"pii": {"enabled": true}}, + "strategy_options": { + "checkpoints_version": 2, + "gmeta": true, + "summarize": {"enabled": true}, + "checkpoint_remote": {"provider": "github", "repo": "org/repo"}, + "filtered_fetches": true, + "push_sessions": false, + "full_transcript_generation_retention_days": 30 + } + }` + legacy, err := LoadFromBytes([]byte(legacyJSON)) + if err != nil { + t.Fatalf("LoadFromBytes: %v", err) + } + + got := synthesizeFromLegacy(legacy) + + want := &Settings{ + Schema: CurrentSchemaVersion, + Enabled: true, + Logging: LoggingConfig{Level: debugLevel}, + Hooks: HooksConfig{CommitLinking: CommitLinkingAlways, AbsoluteGitHookPath: true}, + Features: FeaturesConfig{Summarize: true, ExternalAgents: true, Vercel: true}, + Checkpoints: CheckpointsConfig{ + Primary: BackendConfig{Type: BackendTypeV2}, + Mirrors: []BackendConfig{{Type: BackendTypeGmeta}}, + Remote: &CheckpointRemoteConfig{Provider: "github", Repo: "org/repo"}, + FullTranscriptRetentionDays: 30, + SignCommits: boolPtr(false), + FilteredFetches: true, + PushSessions: boolPtr(false), + }, + Redaction: &RedactionSettings{PII: &PIISettings{Enabled: true}}, + Telemetry: boolPtr(true), + SummaryGeneration: &SummaryGenerationConfig{ + Provider: providerClaudeCC, + Model: "sonnet", + TimeoutSeconds: 45, + }, + } + + if !reflect.DeepEqual(got, want) { + t.Fatalf("round-trip mismatch:\n got: %+v\nwant: %+v", got, want) + } +} diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index 2d5b23290c..b98654b21f 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -684,9 +684,11 @@ func (s *EntireSettings) IsSummarizeEnabled() bool { // CheckpointRemoteConfig holds the structured checkpoint remote configuration. // Stored in strategy_options.checkpoint_remote as {"provider": "github", "repo": "org/repo"}. +// JSON tags are present so the same type can be serialized directly under +// the schema-v2 Settings.Checkpoints.Remote field. type CheckpointRemoteConfig struct { - Provider string // e.g., "github" - Repo string // e.g., "org/checkpoints-repo" + Provider string `json:"provider"` // e.g., "github" + Repo string `json:"repo"` // e.g., "org/checkpoints-repo" } // Owner returns the owner portion of the repo field (before the slash). From c53260648b9fa2b9133399277ebb458a7ef4d2c6 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 27 Apr 2026 20:47:07 +0200 Subject: [PATCH 3/8] Simplify LoadV2 after review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address findings from a three-agent simplify pass. - Extract legacyKey* constants for the strategy_options keys the synthesizer reads directly. Removes duplicated string literals. - Replace mergeV2Override's two-decode pattern (parseV2Bytes plus a separate map[string]json.RawMessage parse) with a single decode and a decodeOverrideField helper. Less work and clearer. - Drop dead bool params from loadFromV2Files; inline the schema-v2 probes in the LoadV2 switch so each branch derives only what it uses. - Rename mixedShapesRejected → hasMixedShapes (predicate name describes state, not the caller's reaction); the error stays at the call site. - Trim narrative file-header comments and the synthesizer mapping list that duplicated information already visible in the function body. Keep the "Dropped (no v2 destination)" section — that's the non-obvious WHY. - Reuse the new readSettingsFileIfExists helper from loadMergedSettings so the legacy and v2 paths share the local-file read pattern. Behavior unchanged; all tests green. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: f1ac37a4b72a --- cmd/entire/cli/settings/load_v2.go | 174 +++++++++++++-------------- cmd/entire/cli/settings/schema_v2.go | 18 +-- cmd/entire/cli/settings/settings.go | 10 +- 3 files changed, 88 insertions(+), 114 deletions(-) diff --git a/cmd/entire/cli/settings/load_v2.go b/cmd/entire/cli/settings/load_v2.go index a106cc4c14..adb77601a0 100644 --- a/cmd/entire/cli/settings/load_v2.go +++ b/cmd/entire/cli/settings/load_v2.go @@ -1,16 +1,5 @@ -// This file implements LoadV2 and the legacy → v2 synthesizer. -// -// Loading rules: -// - If .entire/settings.json declares "schema": 2, the file is parsed -// directly as Settings. A local override file, if present, must also -// declare "schema": 2 (mixed-shape settings are rejected with a -// descriptive error). -// - Otherwise, the legacy Load() path runs and its EntireSettings result -// is mapped into Settings via synthesizeFromLegacy. -// -// The synthesizer is the single source of truth for legacy → v2 mapping -// and is the implementation that `entire migrate-config` will reuse to -// rewrite settings.json into the new shape. +// Schema-v2 loader and legacy synthesizer. +// See synthesizeFromLegacy for the v1→v2 mapping. package settings @@ -32,6 +21,15 @@ const ( BackendTypeGmeta = "gmeta" ) +// Legacy strategy_options keys read by the synthesizer. The legacy package +// has no constants for these; defining them here keeps the synthesizer's +// references typo-proof without modifying legacy accessors. +const ( + legacyKeyGmeta = "gmeta" + legacyKeyPushSessions = "push_sessions" + legacyKeyFullTranscriptRetentionDays = "full_transcript_generation_retention_days" +) + // LoadV2 loads settings as the v2 shape. Files declaring "schema": 2 are // parsed natively; older files are loaded via the legacy parser and // synthesized. The local override file is merged the same way. @@ -57,17 +55,14 @@ func LoadV2(ctx context.Context) (*Settings, error) { return nil, fmt.Errorf("reading local settings file: %w", localErr) } - mainIsV2 := isSchemaV2(mainData) - localIsV2 := isSchemaV2(localData) - switch { case mainData == nil && localData == nil: return defaultSettings(), nil - case mixedShapesRejected(mainData, localData, mainIsV2, localIsV2): + case hasMixedShapes(mainData, localData): return nil, errors.New("mixed schema versions in settings files; run `entire migrate-config` to convert all settings to schema 2") - case mainIsV2 || localIsV2: + case isSchemaV2(mainData) || isSchemaV2(localData): return loadFromV2Files(mainData, localData) default: @@ -127,12 +122,14 @@ func isSchemaV2(data []byte) bool { return probe.Schema >= CurrentSchemaVersion } -// mixedShapesRejected reports the only configuration we treat as an error: -// exactly one of (main, local) declares schema 2 while the other is legacy. -// Two missing files or two same-shape files are both fine. -func mixedShapesRejected(mainData, localData []byte, mainIsV2, localIsV2 bool) bool { - bothPresent := mainData != nil && localData != nil - return bothPresent && (mainIsV2 != localIsV2) +// hasMixedShapes reports whether main and local settings files declare +// different schema shapes (one v2, one legacy). Returns false when either +// file is missing — only the both-present-different-shape case is mixed. +func hasMixedShapes(mainData, localData []byte) bool { + if mainData == nil || localData == nil { + return false + } + return isSchemaV2(mainData) != isSchemaV2(localData) } // loadFromV2Files parses one or both files as schema-v2 JSON and applies the @@ -167,53 +164,64 @@ func parseV2Bytes(data []byte) (*Settings, error) { return settings, nil } -// mergeV2Override applies local overrides on top of an existing Settings -// value. Field-level merge semantics: only fields explicitly present in -// the override JSON change the base. -// -// For now this delegates to a full reparse of the local file and replaces -// top-level groups wholesale. That is enough for the local-override use -// cases in the codebase today (toggling log level, enabling local_dev). -// More granular per-field merging can be added when a real call site needs -// it; the legacy code only got more granular for redaction/PII fields, -// and we will revisit that if the ergonomics become a problem. +// mergeV2Override applies local overrides on top of an existing Settings. +// Only top-level fields explicitly present in the override JSON are +// replaced; absent fields preserve the base. Per-field merge of nested +// groups is intentionally not done — the legacy code only got more +// granular for redaction/PII, and we will revisit when a real call site +// needs it. func mergeV2Override(base *Settings, data []byte) error { - override, err := parseV2Bytes(data) - if err != nil { - return err - } - var presence map[string]json.RawMessage if err := json.Unmarshal(data, &presence); err != nil { return fmt.Errorf("parsing local settings: %w", err) } + if _, ok := presence["schema"]; !ok { + // Defensive: the v2 path is only entered when at least one file + // declares schema 2; a local-only override without it would have + // taken the legacy branch in LoadV2. + return errors.New("local settings file is missing required schema field") + } - if _, ok := presence["enabled"]; ok { - base.Enabled = override.Enabled + if err := decodeOverrideField(presence, "enabled", &base.Enabled); err != nil { + return err + } + if err := decodeOverrideField(presence, "local_dev", &base.LocalDev); err != nil { + return err + } + if err := decodeOverrideField(presence, "logging", &base.Logging); err != nil { + return err } - if _, ok := presence["local_dev"]; ok { - base.LocalDev = override.LocalDev + if err := decodeOverrideField(presence, "checkpoints", &base.Checkpoints); err != nil { + return err } - if _, ok := presence["logging"]; ok { - base.Logging = override.Logging + if err := decodeOverrideField(presence, "hooks", &base.Hooks); err != nil { + return err } - if _, ok := presence["checkpoints"]; ok { - base.Checkpoints = override.Checkpoints + if err := decodeOverrideField(presence, "features", &base.Features); err != nil { + return err } - if _, ok := presence["hooks"]; ok { - base.Hooks = override.Hooks + if err := decodeOverrideField(presence, "redaction", &base.Redaction); err != nil { + return err } - if _, ok := presence["features"]; ok { - base.Features = override.Features + if err := decodeOverrideField(presence, "telemetry", &base.Telemetry); err != nil { + return err } - if _, ok := presence["redaction"]; ok { - base.Redaction = override.Redaction + if err := decodeOverrideField(presence, "summary_generation", &base.SummaryGeneration); err != nil { + return err } - if _, ok := presence["telemetry"]; ok { - base.Telemetry = override.Telemetry + return nil +} + +// decodeOverrideField decodes the named override field into dst if present +// in the parsed top-level map. No-op when the key is absent, preserving +// the base value. +func decodeOverrideField(presence map[string]json.RawMessage, key string, dst any) error { + raw, ok := presence[key] + if !ok { + return nil } - if _, ok := presence["summary_generation"]; ok { - base.SummaryGeneration = override.SummaryGeneration + if err := json.Unmarshal(raw, dst); err != nil { + return fmt.Errorf("parsing %s override: %w", key, err) } return nil } @@ -232,30 +240,12 @@ func defaultSettings() *Settings { // legacy → v2 migration. This is the single source of truth for the // translation and is reused by `entire migrate-config`. // -// Mappings: -// - strategy_options.checkpoints_version=2 OR checkpoints_v2=true -// → Checkpoints.Primary.Type = "v2" -// - otherwise → Checkpoints.Primary.Type = "v1" -// - strategy_options.gmeta=true → append {type: "gmeta"} to Mirrors -// - strategy_options.checkpoint_remote → Checkpoints.Remote -// - strategy_options.full_transcript_generation_retention_days -// → Checkpoints.FullTranscriptRetentionDays -// - strategy_options.filtered_fetches → Checkpoints.FilteredFetches -// - strategy_options.push_sessions (explicit) → Checkpoints.PushSessions -// - strategy_options.summarize.enabled → Features.Summarize -// - log_level → Logging.Level -// - sign_checkpoint_commits → Checkpoints.SignCommits -// - commit_linking → Hooks.CommitLinking -// - absolute_git_hook_path → Hooks.AbsoluteGitHookPath -// - external_agents → Features.ExternalAgents -// - vercel → Features.Vercel -// - summary_generation.{provider,model} + summary_timeout_seconds -// → SummaryGeneration.{Provider,Model,TimeoutSeconds} -// // Dropped (no v2 destination): -// - push_v2_refs (was a dual-write transition knob; v2 ref pushes are now -// a property of the v2 backend's CheckpointSyncer, not a settings flag) -// - strategy (deprecated tolerator field) +// - push_v2_refs: was a dual-write transition knob; v2 ref pushes will +// be a property of the v2 backend's CheckpointSyncer, not a settings flag. +// - strategy: deprecated tolerator field. +// +// See the function body for field-by-field mapping. func synthesizeFromLegacy(s *EntireSettings) *Settings { if s == nil { return defaultSettings() @@ -325,40 +315,40 @@ func legacyPrimaryBackend(s *EntireSettings) string { return BackendTypeV1 } -// legacyGmetaEnabled checks the legacy strategy_options.gmeta flag. The -// flag itself does not exist on this branch yet (it ships with the gmeta -// mainport), but the synthesizer recognizes it ahead of time so settings -// files written by that branch round-trip cleanly through Phase 0. +// legacyGmetaEnabled checks the strategy_options.gmeta flag. The flag does +// not exist on this branch (it ships with the gmeta mainport), but the +// synthesizer recognizes it ahead of time so settings files written by +// that branch round-trip cleanly. func legacyGmetaEnabled(s *EntireSettings) bool { if s.StrategyOptions == nil { return false } - val, ok := s.StrategyOptions["gmeta"].(bool) + val, ok := s.StrategyOptions[legacyKeyGmeta].(bool) return ok && val } // legacyFullTranscriptRetention returns the configured retention only when -// the legacy key is explicitly set; the legacy accessor returns a default -// when unset, which we suppress here so the v2 file stays minimal. +// the legacy key is explicitly set; the legacy accessor would return a +// default for unset values, which we suppress here so the v2 file stays +// minimal. func legacyFullTranscriptRetention(s *EntireSettings) int { if s.StrategyOptions == nil { return 0 } - if _, ok := s.StrategyOptions["full_transcript_generation_retention_days"]; !ok { + if _, ok := s.StrategyOptions[legacyKeyFullTranscriptRetentionDays]; !ok { return 0 } return s.GetFullTranscriptGenerationRetentionDays() } // legacyPushSessions extracts the explicit push_sessions boolean if set; -// otherwise returns nil so the v2 file stays minimal. The legacy semantics -// were "explicit false disables, otherwise default behavior", which we -// preserve by using a *bool here. +// otherwise returns nil so the v2 file stays minimal. Tri-state preserves +// the legacy "explicit false disables, otherwise default" semantics. func legacyPushSessions(s *EntireSettings) *bool { if s.StrategyOptions == nil { return nil } - val, ok := s.StrategyOptions["push_sessions"] + val, ok := s.StrategyOptions[legacyKeyPushSessions] if !ok { return nil } diff --git a/cmd/entire/cli/settings/schema_v2.go b/cmd/entire/cli/settings/schema_v2.go index 5b2c5ac5f6..474c1aef09 100644 --- a/cmd/entire/cli/settings/schema_v2.go +++ b/cmd/entire/cli/settings/schema_v2.go @@ -1,19 +1,5 @@ -// This file defines schema v2 of the Entire settings file: a typed, -// concern-grouped replacement for the legacy EntireSettings struct. -// -// The two shapes coexist during migration. Existing callers continue using -// Load()/EntireSettings; new code can opt into LoadV2()/Settings. A future -// phase migrates call sites and retires the legacy struct. -// -// Key differences from legacy: -// - Explicit "schema" version marker (vs. implicit/unmarked) -// - Typed checkpoints config (vs. untyped strategy_options map) -// - Backend selection is structured: primary + optional mirrors -// - Concerns grouped: logging, checkpoints, hooks, features -// - Deprecated keys (checkpoints_v2 alias, push_v2_refs) are not -// represented in the new shape; the synthesizer translates them. -// -// Migration mapping is implemented in synthesizeFromLegacy (see load_v2.go). +// Schema-v2 settings types. See synthesizeFromLegacy in load_v2.go for +// the v1→v2 mapping. package settings diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index b98654b21f..c1af15fa30 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -209,13 +209,11 @@ func loadMergedSettings(settingsFileAbs, localSettingsFileAbs string) (*EntireSe } // Apply local overrides if they exist - localData, err := os.ReadFile(localSettingsFileAbs) //nolint:gosec // path is from AbsPath or constant + localData, err := readSettingsFileIfExists(localSettingsFileAbs) if err != nil { - if !os.IsNotExist(err) { - return nil, fmt.Errorf("reading local settings file: %w", err) - } - // Local file doesn't exist, continue without overrides - } else { + return nil, fmt.Errorf("reading local settings file: %w", err) + } + if localData != nil { if err := mergeJSON(settings, localData); err != nil { return nil, fmt.Errorf("merging local settings: %w", err) } From db7f185ca7575a09fa42ab1cba6d25b89cd34b61 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 27 Apr 2026 21:24:40 +0200 Subject: [PATCH 4/8] Fix nested partial override, mixed shapes, and add Validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address three findings from the deeper review: 1. (high) Wholesale top-level group replacement in mergeV2Override could silently erase the primary backend. A local override of just {"checkpoints": {"mirrors": [...]}} would clear Primary, Remote, and the rest of the group. Fix: drop the per-field replacement logic and decode the override directly into the live base struct. Go's json package leaves struct fields not mentioned in the JSON unchanged, including nested fields, so decode-in-place gives field-level granular merge for free. Two regression tests cover this: TestLoadV2_PartialCheckpointsOverride exercises exactly the failure case from the finding; TestLoadV2_PartialNestedFieldsPreserved exercises every nested group. 2. (medium) Hard-rejecting mixed legacy/v2 main+local files made incremental migration of the gitignored local override impossible. Fix: replace rejection with a legacy-to-v2 override applier that translates each legacy key present in the override to its v2 destination on the existing base. Symmetric in both directions — legacy main + v2 local synthesizes main first, then applies the v2 local; v2 main + legacy local parses main, then applies the legacy local field-by-field. Two new tests cover both directions plus a strategy_options-only legacy override. 3. (medium) Schema-v2 settings were decoded but not semantically validated; invalid configs (unknown backend type, model without provider, wrong schema version) leaked through to first use. Fix: add Settings.Validate() with rules for schema version, primary and mirror backend types, and the legacy summary-model-without- provider invariant. LoadV2 and LoadV2FromBytes call it after constructing the result. Validation is table-driven in TestSettings_Validate; TestLoadV2_RejectsInvalidV2 confirms invalid v2 files surface as load-time errors. Bonus: defaultSettings() now sets Primary.Type = "v1" so the no-files case validates without special-casing. Matches the legacy "no version specified means v1" behavior. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 47cec093e3c2 --- cmd/entire/cli/settings/load_v2.go | 302 ++++++++++++++++-------- cmd/entire/cli/settings/load_v2_test.go | 301 +++++++++++++++++++++-- cmd/entire/cli/settings/schema_v2.go | 47 ++++ 3 files changed, 538 insertions(+), 112 deletions(-) diff --git a/cmd/entire/cli/settings/load_v2.go b/cmd/entire/cli/settings/load_v2.go index adb77601a0..b9cbcba33d 100644 --- a/cmd/entire/cli/settings/load_v2.go +++ b/cmd/entire/cli/settings/load_v2.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "os" @@ -28,14 +27,20 @@ const ( legacyKeyGmeta = "gmeta" legacyKeyPushSessions = "push_sessions" legacyKeyFullTranscriptRetentionDays = "full_transcript_generation_retention_days" + legacyKeyCheckpointsV2 = "checkpoints_v2" + legacyKeyCheckpointsVersion = "checkpoints_version" + legacyKeyCheckpointRemote = "checkpoint_remote" + legacyKeyFilteredFetches = "filtered_fetches" + legacyKeySummarize = "summarize" ) // LoadV2 loads settings as the v2 shape. Files declaring "schema": 2 are // parsed natively; older files are loaded via the legacy parser and -// synthesized. The local override file is merged the same way. +// synthesized. Mixed shapes (one file v2, one legacy) are supported: the +// legacy file is translated field-by-field on top of the v2 base. // -// Returns Settings with sensible defaults populated (Schema=2, Enabled=true) -// if neither settings file exists. +// Returns Settings with documented defaults if neither settings file exists. +// Validates the result before returning so semantic errors surface at load time. func LoadV2(ctx context.Context) (*Settings, error) { mainAbs, err := paths.AbsPath(ctx, EntireSettingsFile) if err != nil { @@ -55,44 +60,108 @@ func LoadV2(ctx context.Context) (*Settings, error) { return nil, fmt.Errorf("reading local settings file: %w", localErr) } - switch { - case mainData == nil && localData == nil: - return defaultSettings(), nil - - case hasMixedShapes(mainData, localData): - return nil, errors.New("mixed schema versions in settings files; run `entire migrate-config` to convert all settings to schema 2") + settings, err := buildSettings(mainData, localData, mainAbs, localAbs) + if err != nil { + return nil, err + } + if err := settings.Validate(); err != nil { + return nil, fmt.Errorf("settings invalid: %w", err) + } + return settings, nil +} - case isSchemaV2(mainData) || isSchemaV2(localData): - return loadFromV2Files(mainData, localData) +// LoadV2FromBytes parses a single settings document from raw bytes. +// Accepts either schema-v2 JSON or legacy JSON; the shape is auto-detected +// via the schema field. No local-override merge is performed. +func LoadV2FromBytes(data []byte) (*Settings, error) { + if len(data) == 0 { + return defaultSettings(), nil + } + var ( + settings *Settings + err error + ) + if isSchemaV2(data) { + settings, err = parseV2Bytes(data) + } else { + var legacy *EntireSettings + legacy, err = LoadFromBytes(data) + if err == nil { + settings = synthesizeFromLegacy(legacy) + } + } + if err != nil { + return nil, err + } + if err := settings.Validate(); err != nil { + return nil, fmt.Errorf("settings invalid: %w", err) + } + return settings, nil +} - default: - // Both files (if present) are legacy shape. Defer to the legacy - // loader to handle merging, then synthesize into v2. +// buildSettings constructs the merged Settings from raw main and local +// bytes. Each file may independently be v2 or legacy; this is what makes +// incremental migration of the gitignored local override safe. +// +// When both files are legacy, we delegate to the legacy loadMergedSettings +// so the legacy granular-merge behavior (PII fields, summary_generation +// provider/model interaction) is preserved verbatim before synthesis. +func buildSettings(mainData, localData []byte, mainAbs, localAbs string) (*Settings, error) { + if mainData == nil && localData == nil { + return defaultSettings(), nil + } + if !isSchemaV2(mainData) && !isSchemaV2(localData) { legacy, err := loadMergedSettings(mainAbs, localAbs) if err != nil { return nil, err } return synthesizeFromLegacy(legacy), nil } + + // At least one file is v2. Establish the base from the main file (v2 + // or synthesized legacy), then apply the local file as the right kind + // of override. + base, err := buildBase(mainData) + if err != nil { + return nil, err + } + if localData != nil { + if err := applyOverride(base, localData); err != nil { + return nil, err + } + } + return base, nil } -// LoadV2FromBytes parses a single settings document from raw bytes. -// Accepts either schema-v2 JSON or legacy JSON; the shape is auto-detected -// via the schema field. No local-override merge is performed. -func LoadV2FromBytes(data []byte) (*Settings, error) { - if len(data) == 0 { +// buildBase parses the main file into a Settings, choosing between the v2 +// parser and the legacy synthesizer based on shape. +func buildBase(data []byte) (*Settings, error) { + if data == nil { return defaultSettings(), nil } if isSchemaV2(data) { - return parseV2Bytes(data) + settings, err := parseV2Bytes(data) + if err != nil { + return nil, fmt.Errorf("parsing settings file: %w", err) + } + return settings, nil } legacy, err := LoadFromBytes(data) if err != nil { - return nil, err + return nil, fmt.Errorf("parsing settings file: %w", err) } return synthesizeFromLegacy(legacy), nil } +// applyOverride merges an override file (v2 or legacy) onto an existing v2 +// base. Routes to the appropriate per-shape applier. +func applyOverride(base *Settings, data []byte) error { + if isSchemaV2(data) { + return mergeV2Override(base, data) + } + return applyLegacyOverride(base, data) +} + // readSettingsFileIfExists returns the file contents, or (nil, nil) if the // file does not exist. Other I/O errors propagate. func readSettingsFileIfExists(path string) ([]byte, error) { @@ -122,38 +191,9 @@ func isSchemaV2(data []byte) bool { return probe.Schema >= CurrentSchemaVersion } -// hasMixedShapes reports whether main and local settings files declare -// different schema shapes (one v2, one legacy). Returns false when either -// file is missing — only the both-present-different-shape case is mixed. -func hasMixedShapes(mainData, localData []byte) bool { - if mainData == nil || localData == nil { - return false - } - return isSchemaV2(mainData) != isSchemaV2(localData) -} - -// loadFromV2Files parses one or both files as schema-v2 JSON and applies the -// local override on top of the main settings. At least one of the inputs is -// guaranteed by the caller to be a v2 document. -func loadFromV2Files(mainData, localData []byte) (*Settings, error) { - settings := defaultSettings() - if mainData != nil { - parsed, err := parseV2Bytes(mainData) - if err != nil { - return nil, fmt.Errorf("parsing settings file: %w", err) - } - settings = parsed - } - if localData != nil { - if err := mergeV2Override(settings, localData); err != nil { - return nil, fmt.Errorf("merging local settings: %w", err) - } - } - return settings, nil -} - -// parseV2Bytes parses a single schema-v2 JSON document into Settings. -// Strict decoding rejects unknown fields so typos become loud failures. +// parseV2Bytes parses a single schema-v2 JSON document into a fresh Settings +// (with defaults). Strict decoding rejects unknown fields so typos become +// loud failures. func parseV2Bytes(data []byte) (*Settings, error) { settings := defaultSettings() dec := json.NewDecoder(bytes.NewReader(data)) @@ -164,75 +204,141 @@ func parseV2Bytes(data []byte) (*Settings, error) { return settings, nil } -// mergeV2Override applies local overrides on top of an existing Settings. -// Only top-level fields explicitly present in the override JSON are -// replaced; absent fields preserve the base. Per-field merge of nested -// groups is intentionally not done — the legacy code only got more -// granular for redaction/PII, and we will revisit when a real call site -// needs it. +// mergeV2Override merges a schema-v2 override on top of an existing +// Settings. Decoding into the live struct in place gives us field-level +// granular merge for free: Go's json package leaves struct fields not +// mentioned in the JSON unchanged, including nested fields. So an override +// of {"checkpoints": {"mirrors": [...]}} preserves base.Checkpoints.Primary +// and the rest of base.Checkpoints, only replacing Mirrors. +// +// Caveats: pointer fields and slices are decoded by value-replacement, not +// granular merge. If a user wants to change one field of base.Redaction +// they must spell out enough of the override to reconstruct the desired +// shape. This matches what JSON decoding naturally does and is consistent +// with how config tooling generally behaves. func mergeV2Override(base *Settings, data []byte) error { - var presence map[string]json.RawMessage - if err := json.Unmarshal(data, &presence); err != nil { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(base); err != nil { + return fmt.Errorf("merging local settings: %w", err) + } + return nil +} + +// applyLegacyOverride merges a legacy-shape override on top of an existing +// v2 Settings. Translates each legacy key present in the override to its +// v2 destination. Fields not present in the override are left untouched — +// this is what makes "v2 main + legacy local" usable without forcing the +// user to migrate the local file before the main one. +func applyLegacyOverride(base *Settings, data []byte) error { + legacy, err := LoadFromBytes(data) + if err != nil { return fmt.Errorf("parsing local settings: %w", err) } - if _, ok := presence["schema"]; !ok { - // Defensive: the v2 path is only entered when at least one file - // declares schema 2; a local-only override without it would have - // taken the legacy branch in LoadV2. - return errors.New("local settings file is missing required schema field") + + var present map[string]json.RawMessage + if err := json.Unmarshal(data, &present); err != nil { + return fmt.Errorf("parsing local settings: %w", err) + } + + overlay := synthesizeFromLegacy(legacy) + applyTopLevelLegacyKeys(base, overlay, present) + + soRaw, ok := present["strategy_options"] + if !ok { + return nil + } + var so map[string]json.RawMessage + if err := json.Unmarshal(soRaw, &so); err != nil { + return fmt.Errorf("parsing local strategy_options: %w", err) } + applyStrategyOptionLegacyKeys(base, overlay, so) + return nil +} - if err := decodeOverrideField(presence, "enabled", &base.Enabled); err != nil { - return err +// applyTopLevelLegacyKeys copies overlay → base for top-level legacy keys +// present in the override. Each branch maps one legacy key to its v2 +// destination. +func applyTopLevelLegacyKeys(base, overlay *Settings, present map[string]json.RawMessage) { + if _, ok := present["enabled"]; ok { + base.Enabled = overlay.Enabled } - if err := decodeOverrideField(presence, "local_dev", &base.LocalDev); err != nil { - return err + if _, ok := present["local_dev"]; ok { + base.LocalDev = overlay.LocalDev } - if err := decodeOverrideField(presence, "logging", &base.Logging); err != nil { - return err + if _, ok := present["log_level"]; ok { + base.Logging.Level = overlay.Logging.Level } - if err := decodeOverrideField(presence, "checkpoints", &base.Checkpoints); err != nil { - return err + if _, ok := present["commit_linking"]; ok { + base.Hooks.CommitLinking = overlay.Hooks.CommitLinking } - if err := decodeOverrideField(presence, "hooks", &base.Hooks); err != nil { - return err + if _, ok := present["absolute_git_hook_path"]; ok { + base.Hooks.AbsoluteGitHookPath = overlay.Hooks.AbsoluteGitHookPath } - if err := decodeOverrideField(presence, "features", &base.Features); err != nil { - return err + if _, ok := present["external_agents"]; ok { + base.Features.ExternalAgents = overlay.Features.ExternalAgents } - if err := decodeOverrideField(presence, "redaction", &base.Redaction); err != nil { - return err + if _, ok := present["vercel"]; ok { + base.Features.Vercel = overlay.Features.Vercel } - if err := decodeOverrideField(presence, "telemetry", &base.Telemetry); err != nil { - return err + if _, ok := present["telemetry"]; ok { + base.Telemetry = overlay.Telemetry } - if err := decodeOverrideField(presence, "summary_generation", &base.SummaryGeneration); err != nil { - return err + if _, ok := present["sign_checkpoint_commits"]; ok { + base.Checkpoints.SignCommits = overlay.Checkpoints.SignCommits + } + if _, ok := present["redaction"]; ok { + base.Redaction = overlay.Redaction + } + _, hasSummaryGen := present["summary_generation"] + _, hasSummaryTimeout := present["summary_timeout_seconds"] + if hasSummaryGen || hasSummaryTimeout { + base.SummaryGeneration = overlay.SummaryGeneration } - return nil } -// decodeOverrideField decodes the named override field into dst if present -// in the parsed top-level map. No-op when the key is absent, preserving -// the base value. -func decodeOverrideField(presence map[string]json.RawMessage, key string, dst any) error { - raw, ok := presence[key] - if !ok { - return nil +// applyStrategyOptionLegacyKeys copies overlay → base for strategy_options +// keys present in the override. Each legacy strategy_options key maps to a +// specific v2 destination on base.Checkpoints or base.Features. +func applyStrategyOptionLegacyKeys(base, overlay *Settings, so map[string]json.RawMessage) { + if _, ok := so[legacyKeyCheckpointsV2]; ok { + base.Checkpoints.Primary = overlay.Checkpoints.Primary } - if err := json.Unmarshal(raw, dst); err != nil { - return fmt.Errorf("parsing %s override: %w", key, err) + if _, ok := so[legacyKeyCheckpointsVersion]; ok { + base.Checkpoints.Primary = overlay.Checkpoints.Primary + } + if _, ok := so[legacyKeyGmeta]; ok { + base.Checkpoints.Mirrors = overlay.Checkpoints.Mirrors + } + if _, ok := so[legacyKeyCheckpointRemote]; ok { + base.Checkpoints.Remote = overlay.Checkpoints.Remote + } + if _, ok := so[legacyKeyFullTranscriptRetentionDays]; ok { + base.Checkpoints.FullTranscriptRetentionDays = overlay.Checkpoints.FullTranscriptRetentionDays + } + if _, ok := so[legacyKeyFilteredFetches]; ok { + base.Checkpoints.FilteredFetches = overlay.Checkpoints.FilteredFetches + } + if _, ok := so[legacyKeyPushSessions]; ok { + base.Checkpoints.PushSessions = overlay.Checkpoints.PushSessions + } + if _, ok := so[legacyKeySummarize]; ok { + base.Features.Summarize = overlay.Features.Summarize } - return nil } // defaultSettings returns the zero-value Settings with documented defaults -// applied (Schema = 2, Enabled = true). Callers should treat this as the -// starting point before parsing a file or merging overrides. +// applied (Schema = 2, Enabled = true, Primary = v1). v1 is the default +// primary for parity with the legacy "no version specified means v1" +// behavior; users on a fresh repo see V2 only after `entire enable` writes +// it explicitly. func defaultSettings() *Settings { return &Settings{ Schema: CurrentSchemaVersion, Enabled: true, + Checkpoints: CheckpointsConfig{ + Primary: BackendConfig{Type: BackendTypeV1}, + }, } } diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go index 92d18e0212..ae585b50ec 100644 --- a/cmd/entire/cli/settings/load_v2_test.go +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -447,8 +447,8 @@ func TestLoadV2_NoFiles(t *testing.T) { if got.Schema != CurrentSchemaVersion || !got.Enabled { t.Fatalf("defaults wrong: %+v", got) } - if got.Checkpoints.Primary.Type != "" { - t.Fatalf("expected empty primary when no files present, got %q", got.Checkpoints.Primary.Type) + if got.Checkpoints.Primary.Type != BackendTypeV1 { + t.Fatalf("expected default primary v1 when no files present, got %q", got.Checkpoints.Primary.Type) } } @@ -512,28 +512,44 @@ func TestLoadV2_V2MainAndLocalOverride(t *testing.T) { } } -func TestLoadV2_MixedShapesRejected_V2MainLegacyLocal(t *testing.T) { +// TestLoadV2_V2MainLegacyLocal covers the common migration scenario: +// the tracked settings.json has been migrated to schema 2, but the +// gitignored settings.local.json is still in legacy shape. The legacy +// override translates to v2 fields without forcing the user to migrate +// the local file first. +func TestLoadV2_V2MainLegacyLocal(t *testing.T) { setupSettingsDir(t, - `{"schema": 2, "enabled": true}`, + `{"schema": 2, "enabled": true, "checkpoints": {"primary": {"type": "v2"}}}`, `{"log_level": "`+debugLevel+`"}`, ) - _, err := LoadV2(context.Background()) - if err == nil { - t.Fatal("expected error for mixed shapes") + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) } - if !strings.Contains(err.Error(), "mixed schema") { - t.Fatalf("unexpected error: %v", err) + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (preserved from main)", got.Checkpoints.Primary.Type) + } + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q (from legacy local)", got.Logging.Level, debugLevel) } } -func TestLoadV2_MixedShapesRejected_LegacyMainV2Local(t *testing.T) { +// TestLoadV2_LegacyMainV2Local covers the inverse: legacy main, v2 local. +// Less common but should also work without rejection. +func TestLoadV2_LegacyMainV2Local(t *testing.T) { setupSettingsDir(t, - `{"enabled": true}`, + `{"enabled": true, "strategy_options": {"checkpoints_v2": true}}`, `{"schema": 2, "logging": {"level": "`+debugLevel+`"}}`, ) - _, err := LoadV2(context.Background()) - if err == nil { - t.Fatal("expected error for mixed shapes") + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (synthesized from main)", got.Checkpoints.Primary.Type) + } + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q (from v2 local)", got.Logging.Level, debugLevel) } } @@ -651,3 +667,260 @@ func TestSynthesizeFromLegacy_RoundTripFromBytes(t *testing.T) { t.Fatalf("round-trip mismatch:\n got: %+v\nwant: %+v", got, want) } } + +// TestLoadV2_PartialCheckpointsOverride is the regression test for the +// finding that wholesale top-level group replacement could silently erase +// the primary backend. The local override here only specifies mirrors; +// primary, retention, and other fields must be preserved from the main file. +func TestLoadV2_PartialCheckpointsOverride(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": { + "primary": {"type": "v2"}, + "full_transcript_retention_days": 90, + "filtered_fetches": true + } + }`, + `{ + "schema": 2, + "checkpoints": {"mirrors": [{"type": "gmeta"}]} + }`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (preserved)", got.Checkpoints.Primary.Type) + } + if got.Checkpoints.FullTranscriptRetentionDays != 90 { + t.Fatalf("FullTranscriptRetentionDays = %d, want 90 (preserved)", got.Checkpoints.FullTranscriptRetentionDays) + } + if !got.Checkpoints.FilteredFetches { + t.Fatal("FilteredFetches = false, want true (preserved)") + } + if len(got.Checkpoints.Mirrors) != 1 || got.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [{gmeta}] (added by override)", got.Checkpoints.Mirrors) + } +} + +// TestLoadV2_PartialNestedFieldsPreserved is a stronger version of the +// above that exercises every nested group with a partial override. Acts +// as a sanity check on the assumption that Go's json decoder preserves +// struct fields not mentioned in the JSON. +func TestLoadV2_PartialNestedFieldsPreserved(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "enabled": true, + "logging": {"level": "info"}, + "checkpoints": { + "primary": {"type": "v2"}, + "full_transcript_retention_days": 60 + }, + "hooks": {"commit_linking": "always", "absolute_git_hook_path": true}, + "features": {"summarize": true, "external_agents": true} + }`, + `{ + "schema": 2, + "hooks": {"commit_linking": "prompt"}, + "features": {"vercel": true} + }`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Hooks.CommitLinking != CommitLinkingPrompt { + t.Fatalf("Hooks.CommitLinking = %q, want prompt (overridden)", got.Hooks.CommitLinking) + } + if !got.Hooks.AbsoluteGitHookPath { + t.Fatal("Hooks.AbsoluteGitHookPath = false, want true (preserved)") + } + if !got.Features.Vercel { + t.Fatal("Features.Vercel = false, want true (overridden)") + } + if !got.Features.Summarize { + t.Fatal("Features.Summarize = false, want true (preserved)") + } + if !got.Features.ExternalAgents { + t.Fatal("Features.ExternalAgents = false, want true (preserved)") + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (preserved)", got.Checkpoints.Primary.Type) + } + if got.Checkpoints.FullTranscriptRetentionDays != 60 { + t.Fatalf("FullTranscriptRetentionDays = %d, want 60 (preserved)", got.Checkpoints.FullTranscriptRetentionDays) + } + if got.Logging.Level != "info" { + t.Fatalf("Logging.Level = %q, want info (preserved)", got.Logging.Level) + } +} + +// TestLoadV2_LegacyOverridePartial verifies that a legacy override only +// touches the v2 fields it explicitly mentions, leaving the rest of the v2 +// base intact. Mirror of TestLoadV2_PartialCheckpointsOverride for the +// legacy-shape override path. +func TestLoadV2_LegacyOverridePartial(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": {"primary": {"type": "v2"}, "filtered_fetches": true}, + "hooks": {"commit_linking": "always"} + }`, + `{"log_level": "`+debugLevel+`"}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q, want %q", got.Logging.Level, debugLevel) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (preserved)", got.Checkpoints.Primary.Type) + } + if !got.Checkpoints.FilteredFetches { + t.Fatal("FilteredFetches = false, want true (preserved)") + } + if got.Hooks.CommitLinking != CommitLinkingAlways { + t.Fatalf("Hooks.CommitLinking = %q, want always (preserved)", got.Hooks.CommitLinking) + } +} + +// TestLoadV2_LegacyOverridePartialStrategyOptions is the legacy-override +// counterpart for strategy_options-keyed fields that map onto Checkpoints. +func TestLoadV2_LegacyOverridePartialStrategyOptions(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": { + "primary": {"type": "v1"}, + "full_transcript_retention_days": 30, + "filtered_fetches": true + } + }`, + `{"strategy_options": {"checkpoints_v2": true}}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (overridden)", got.Checkpoints.Primary.Type) + } + if got.Checkpoints.FullTranscriptRetentionDays != 30 { + t.Fatalf("FullTranscriptRetentionDays = %d, want 30 (preserved)", got.Checkpoints.FullTranscriptRetentionDays) + } + if !got.Checkpoints.FilteredFetches { + t.Fatal("FilteredFetches = false, want true (preserved)") + } +} + +// TestSettings_Validate covers the semantic validation rules. +func TestSettings_Validate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + s *Settings + wantErr string + }{ + { + name: "nil settings", + s: nil, + wantErr: "nil", + }, + { + name: "wrong schema", + s: &Settings{ + Schema: 1, + Checkpoints: CheckpointsConfig{Primary: BackendConfig{Type: BackendTypeV1}}, + }, + wantErr: "schema", + }, + { + name: "empty primary type", + s: &Settings{ + Schema: CurrentSchemaVersion, + }, + wantErr: "checkpoints.primary.type", + }, + { + name: "unknown primary type", + s: &Settings{ + Schema: CurrentSchemaVersion, + Checkpoints: CheckpointsConfig{Primary: BackendConfig{Type: "totally-fake"}}, + }, + wantErr: "checkpoints.primary.type", + }, + { + name: "unknown mirror type", + s: &Settings{ + Schema: CurrentSchemaVersion, + Checkpoints: CheckpointsConfig{ + Primary: BackendConfig{Type: BackendTypeV2}, + Mirrors: []BackendConfig{{Type: "unknown-backend"}}, + }, + }, + wantErr: "checkpoints.mirrors[0].type", + }, + { + name: "summary model without provider", + s: &Settings{ + Schema: CurrentSchemaVersion, + Checkpoints: CheckpointsConfig{Primary: BackendConfig{Type: BackendTypeV1}}, + SummaryGeneration: &SummaryGenerationConfig{Model: "sonnet"}, + }, + wantErr: "summary_generation.model", + }, + { + name: "valid: defaults", + s: defaultSettings(), + }, + { + name: "valid: full v2 with gmeta mirror", + s: &Settings{ + Schema: CurrentSchemaVersion, + Checkpoints: CheckpointsConfig{ + Primary: BackendConfig{Type: BackendTypeV2}, + Mirrors: []BackendConfig{{Type: BackendTypeGmeta}}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.s.Validate() + if tt.wantErr == "" { + if err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("Validate() = nil, want error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Validate() error %q, want to contain %q", err.Error(), tt.wantErr) + } + }) + } +} + +// TestLoadV2_RejectsInvalidV2 verifies that LoadV2 surfaces validation +// errors as load-time failures rather than letting them through to first use. +func TestLoadV2_RejectsInvalidV2(t *testing.T) { + setupSettingsDir(t, `{ + "schema": 2, + "checkpoints": {"primary": {"type": "totally-fake"}} + }`, "") + _, err := LoadV2(context.Background()) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "checkpoints.primary.type") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/cmd/entire/cli/settings/schema_v2.go b/cmd/entire/cli/settings/schema_v2.go index 474c1aef09..eabb8fbc0b 100644 --- a/cmd/entire/cli/settings/schema_v2.go +++ b/cmd/entire/cli/settings/schema_v2.go @@ -3,6 +3,11 @@ package settings +import ( + "errors" + "fmt" +) + // CurrentSchemaVersion is the schema version emitted by v2 writers. // Files marked with this version are parsed via the v2 path; older files // are loaded via the legacy parser and synthesized into Settings on the fly. @@ -148,3 +153,45 @@ type SummaryGenerationConfig struct { // Zero or negative means "unset" — the caller picks the default. TimeoutSeconds int `json:"timeout_seconds,omitempty"` } + +// knownBackendTypes is the set of backend type identifiers Validate accepts. +// Kept here next to BackendConfig so adding a new backend type is a single +// place to update. +var knownBackendTypes = map[string]struct{}{ + BackendTypeV1: {}, + BackendTypeV2: {}, + BackendTypeGmeta: {}, +} + +// Validate checks the Settings for semantic correctness beyond what JSON +// decoding catches. Run this after parsing or merging to surface invalid +// configurations at load time rather than at first use. +// +// Current rules: +// - Schema must be CurrentSchemaVersion (loaders accept >= but writers +// emit exactly the current value; older loaders rejecting newer files +// is a separate concern handled by isSchemaV2). +// - Checkpoints.Primary.Type must be a known backend. +// - Each Mirror's Type must be a known backend. +// - SummaryGeneration.Model requires SummaryGeneration.Provider, matching +// the legacy SummaryGenerationSettings.Validate semantics. +func (s *Settings) Validate() error { + if s == nil { + return errors.New("settings: nil") + } + if s.Schema != CurrentSchemaVersion { + return fmt.Errorf("settings: schema = %d, want %d", s.Schema, CurrentSchemaVersion) + } + if _, ok := knownBackendTypes[s.Checkpoints.Primary.Type]; !ok { + return fmt.Errorf("checkpoints.primary.type = %q: must be one of v1, v2, gmeta", s.Checkpoints.Primary.Type) + } + for i, m := range s.Checkpoints.Mirrors { + if _, ok := knownBackendTypes[m.Type]; !ok { + return fmt.Errorf("checkpoints.mirrors[%d].type = %q: must be one of v1, v2, gmeta", i, m.Type) + } + } + if s.SummaryGeneration != nil && s.SummaryGeneration.Model != "" && s.SummaryGeneration.Provider == "" { + return fmt.Errorf("summary_generation.model %q set without summary_generation.provider", s.SummaryGeneration.Model) + } + return nil +} From f94e677abf15b1404477fedccd87ed3e91c7f473 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Tue, 28 Apr 2026 16:03:21 +0200 Subject: [PATCH 5/8] Address PR feedback on schema v2 Three issues from the PR review: - (Cursor Bugbot, high) The legacy-override path on top of a v2 base was wholesale-replacing base.Redaction and base.SummaryGeneration even when the override only mentioned sub-fields. Same class of bug as the original finding, just on a different path. Fix: granular merge for both. Redaction reuses the existing legacy mergeRedaction helper (single source of truth for per-PII-field semantics). SummaryGeneration uses per-field presence checks, so e.g. an override of just summary_timeout_seconds preserves the v2 base's existing provider and model. Three regression tests cover the partial-PII case, timeout-only, and provider-only overrides. - (Copilot) Validate doc comment said "loaders accept >=" but the code rejects anything != CurrentSchemaVersion. Updated comment to match actual behavior, with a note that isSchemaV2 accepts >= as a shape probe but strict decode + Validate enforce the contract. - (Copilot) Validate's error messages hardcoded "v1, v2, gmeta" while the authoritative set is knownBackendTypes. Added knownBackendTypesList helper that formats the map's keys in sorted order, used in both error messages so adding a backend updates one place. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 82bf7b46b87c --- cmd/entire/cli/settings/load_v2.go | 75 ++++++++++++++++--- cmd/entire/cli/settings/load_v2_test.go | 97 +++++++++++++++++++++++++ cmd/entire/cli/settings/schema_v2.go | 26 +++++-- 3 files changed, 183 insertions(+), 15 deletions(-) diff --git a/cmd/entire/cli/settings/load_v2.go b/cmd/entire/cli/settings/load_v2.go index b9cbcba33d..b3df826c85 100644 --- a/cmd/entire/cli/settings/load_v2.go +++ b/cmd/entire/cli/settings/load_v2.go @@ -242,7 +242,9 @@ func applyLegacyOverride(base *Settings, data []byte) error { } overlay := synthesizeFromLegacy(legacy) - applyTopLevelLegacyKeys(base, overlay, present) + if err := applyTopLevelLegacyKeys(base, legacy, overlay, present); err != nil { + return err + } soRaw, ok := present["strategy_options"] if !ok { @@ -258,8 +260,11 @@ func applyLegacyOverride(base *Settings, data []byte) error { // applyTopLevelLegacyKeys copies overlay → base for top-level legacy keys // present in the override. Each branch maps one legacy key to its v2 -// destination. -func applyTopLevelLegacyKeys(base, overlay *Settings, present map[string]json.RawMessage) { +// destination. Pointer-typed nested fields (Redaction, SummaryGeneration) +// merge granularly so an override that mentions only sub-fields preserves +// the rest of the v2 base — wholesale replacement would silently erase +// fields the user did not intend to change. +func applyTopLevelLegacyKeys(base *Settings, legacy *EntireSettings, overlay *Settings, present map[string]json.RawMessage) error { if _, ok := present["enabled"]; ok { base.Enabled = overlay.Enabled } @@ -287,14 +292,66 @@ func applyTopLevelLegacyKeys(base, overlay *Settings, present map[string]json.Ra if _, ok := present["sign_checkpoint_commits"]; ok { base.Checkpoints.SignCommits = overlay.Checkpoints.SignCommits } - if _, ok := present["redaction"]; ok { - base.Redaction = overlay.Redaction + if err := mergeLegacyRedactionOverride(base, present); err != nil { + return err + } + return mergeLegacySummaryGenerationOverride(base, legacy, present) +} + +// mergeLegacyRedactionOverride applies a legacy redaction override to the +// v2 base granularly. Reuses the existing legacy mergeRedaction helper so +// the per-PII-field merge logic stays in a single source of truth. +func mergeLegacyRedactionOverride(base *Settings, present map[string]json.RawMessage) error { + redactRaw, ok := present["redaction"] + if !ok { + return nil + } + if base.Redaction == nil { + base.Redaction = &RedactionSettings{} } - _, hasSummaryGen := present["summary_generation"] - _, hasSummaryTimeout := present["summary_timeout_seconds"] - if hasSummaryGen || hasSummaryTimeout { - base.SummaryGeneration = overlay.SummaryGeneration + if err := mergeRedaction(base.Redaction, redactRaw); err != nil { + return fmt.Errorf("merging redaction override: %w", err) } + return nil +} + +// mergeLegacySummaryGenerationOverride applies a legacy summary-generation +// override to the v2 base granularly. The legacy schema splits the timeout +// into a top-level field and provider/model into a nested object; both +// translate into the unified v2 SummaryGenerationConfig. +// +// Per-field presence preserves base values that the override does not +// mention. For example, an override of just summary_timeout_seconds keeps +// the v2 base's Provider and Model intact. +func mergeLegacySummaryGenerationOverride(base *Settings, legacy *EntireSettings, present map[string]json.RawMessage) error { + sgRaw, hasSg := present["summary_generation"] + _, hasTimeout := present["summary_timeout_seconds"] + if !hasSg && !hasTimeout { + return nil + } + if base.SummaryGeneration == nil { + base.SummaryGeneration = &SummaryGenerationConfig{} + } + if hasTimeout { + base.SummaryGeneration.TimeoutSeconds = legacy.SummaryTimeoutSeconds + } + if !hasSg { + return nil + } + var sg map[string]json.RawMessage + if err := json.Unmarshal(sgRaw, &sg); err != nil { + return fmt.Errorf("merging summary_generation override: %w", err) + } + if legacy.SummaryGeneration == nil { + return nil + } + if _, ok := sg["provider"]; ok { + base.SummaryGeneration.Provider = legacy.SummaryGeneration.Provider + } + if _, ok := sg["model"]; ok { + base.SummaryGeneration.Model = legacy.SummaryGeneration.Model + } + return nil } // applyStrategyOptionLegacyKeys copies overlay → base for strategy_options diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go index ae585b50ec..97e136df56 100644 --- a/cmd/entire/cli/settings/load_v2_test.go +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -924,3 +924,100 @@ func TestLoadV2_RejectsInvalidV2(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +// TestLoadV2_LegacyOverrideRedactionGranular is the regression test for +// the wholesale-replacement bug on the legacy-override path: a legacy +// override that mentions only one PII field should preserve the v2 base's +// other PII fields and the rest of the redaction config. +func TestLoadV2_LegacyOverrideRedactionGranular(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": {"primary": {"type": "v2"}}, + "redaction": { + "pii": {"enabled": true, "email": true, "phone": true, "address": true} + } + }`, + `{"redaction": {"pii": {"address": false}}}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.Redaction == nil || got.Redaction.PII == nil { + t.Fatalf("Redaction = %+v, want populated", got.Redaction) + } + pii := got.Redaction.PII + if !pii.Enabled { + t.Fatal("PII.Enabled = false, want true (preserved from base)") + } + if pii.Email == nil || !*pii.Email { + t.Fatalf("PII.Email = %v, want true (preserved from base)", pii.Email) + } + if pii.Phone == nil || !*pii.Phone { + t.Fatalf("PII.Phone = %v, want true (preserved from base)", pii.Phone) + } + if pii.Address == nil || *pii.Address { + t.Fatalf("PII.Address = %v, want explicit false (overridden)", pii.Address) + } +} + +// TestLoadV2_LegacyOverrideSummaryTimeoutOnly verifies that a legacy +// summary_timeout_seconds-only override preserves the v2 base's provider +// and model. Previously this was wholesale-replaced. +func TestLoadV2_LegacyOverrideSummaryTimeoutOnly(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": {"primary": {"type": "v2"}}, + "summary_generation": {"provider": "`+providerClaudeCC+`", "model": "sonnet", "timeout_seconds": 60} + }`, + `{"summary_timeout_seconds": 30}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.SummaryGeneration == nil { + t.Fatal("SummaryGeneration = nil, want preserved") + } + if got.SummaryGeneration.Provider != providerClaudeCC { + t.Fatalf("Provider = %q, want %q (preserved)", got.SummaryGeneration.Provider, providerClaudeCC) + } + if got.SummaryGeneration.Model != "sonnet" { + t.Fatalf("Model = %q, want sonnet (preserved)", got.SummaryGeneration.Model) + } + if got.SummaryGeneration.TimeoutSeconds != 30 { + t.Fatalf("TimeoutSeconds = %d, want 30 (overridden)", got.SummaryGeneration.TimeoutSeconds) + } +} + +// TestLoadV2_LegacyOverrideSummaryProviderOnly verifies that an override +// of just summary_generation.provider preserves the v2 base's existing +// timeout and only updates the explicitly-mentioned field. +func TestLoadV2_LegacyOverrideSummaryProviderOnly(t *testing.T) { + setupSettingsDir(t, + `{ + "schema": 2, + "checkpoints": {"primary": {"type": "v2"}}, + "summary_generation": {"provider": "`+providerClaudeCC+`", "model": "sonnet", "timeout_seconds": 45} + }`, + `{"summary_generation": {"provider": "codex"}}`, + ) + got, err := LoadV2(context.Background()) + if err != nil { + t.Fatalf("LoadV2: %v", err) + } + if got.SummaryGeneration == nil { + t.Fatal("SummaryGeneration = nil") + } + if got.SummaryGeneration.Provider != "codex" { + t.Fatalf("Provider = %q, want codex (overridden)", got.SummaryGeneration.Provider) + } + if got.SummaryGeneration.Model != "sonnet" { + t.Fatalf("Model = %q, want sonnet (preserved)", got.SummaryGeneration.Model) + } + if got.SummaryGeneration.TimeoutSeconds != 45 { + t.Fatalf("TimeoutSeconds = %d, want 45 (preserved)", got.SummaryGeneration.TimeoutSeconds) + } +} diff --git a/cmd/entire/cli/settings/schema_v2.go b/cmd/entire/cli/settings/schema_v2.go index eabb8fbc0b..a9962b650b 100644 --- a/cmd/entire/cli/settings/schema_v2.go +++ b/cmd/entire/cli/settings/schema_v2.go @@ -6,6 +6,8 @@ package settings import ( "errors" "fmt" + "sort" + "strings" ) // CurrentSchemaVersion is the schema version emitted by v2 writers. @@ -156,21 +158,33 @@ type SummaryGenerationConfig struct { // knownBackendTypes is the set of backend type identifiers Validate accepts. // Kept here next to BackendConfig so adding a new backend type is a single -// place to update. +// place to update — Validate's error messages format from this map. var knownBackendTypes = map[string]struct{}{ BackendTypeV1: {}, BackendTypeV2: {}, BackendTypeGmeta: {}, } +// knownBackendTypesList returns the backend types in sorted order for use +// in error messages. Sorting keeps test assertions deterministic. +func knownBackendTypesList() string { + names := make([]string, 0, len(knownBackendTypes)) + for n := range knownBackendTypes { + names = append(names, n) + } + sort.Strings(names) + return strings.Join(names, ", ") +} + // Validate checks the Settings for semantic correctness beyond what JSON // decoding catches. Run this after parsing or merging to surface invalid // configurations at load time rather than at first use. // // Current rules: -// - Schema must be CurrentSchemaVersion (loaders accept >= but writers -// emit exactly the current value; older loaders rejecting newer files -// is a separate concern handled by isSchemaV2). +// - Schema must be exactly CurrentSchemaVersion. Writers emit only the +// current value, and Validate rejects others. (isSchemaV2 accepts >= +// as a shape probe, but strict decode plus this check enforce the +// actual contract.) // - Checkpoints.Primary.Type must be a known backend. // - Each Mirror's Type must be a known backend. // - SummaryGeneration.Model requires SummaryGeneration.Provider, matching @@ -183,11 +197,11 @@ func (s *Settings) Validate() error { return fmt.Errorf("settings: schema = %d, want %d", s.Schema, CurrentSchemaVersion) } if _, ok := knownBackendTypes[s.Checkpoints.Primary.Type]; !ok { - return fmt.Errorf("checkpoints.primary.type = %q: must be one of v1, v2, gmeta", s.Checkpoints.Primary.Type) + return fmt.Errorf("checkpoints.primary.type = %q: must be one of %s", s.Checkpoints.Primary.Type, knownBackendTypesList()) } for i, m := range s.Checkpoints.Mirrors { if _, ok := knownBackendTypes[m.Type]; !ok { - return fmt.Errorf("checkpoints.mirrors[%d].type = %q: must be one of v1, v2, gmeta", i, m.Type) + return fmt.Errorf("checkpoints.mirrors[%d].type = %q: must be one of %s", i, m.Type, knownBackendTypesList()) } } if s.SummaryGeneration != nil && s.SummaryGeneration.Model != "" && s.SummaryGeneration.Provider == "" { From bd798932bac9d01e40a345d631aa5b49ffa1f507 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Tue, 28 Apr 2026 16:55:50 +0200 Subject: [PATCH 6/8] =?UTF-8?q?Rename=20checkpoints.remote=20=E2=86=92=20c?= =?UTF-8?q?heckpoints.git?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group transport-specific config by transport name instead of using "remote" — which was overloaded with git's origin/upstream vocabulary and stops making sense once non-git backends (S3, etc.) appear. The new shape is: checkpoints: primary: { type: v2 } mirrors: [{ type: gmeta }] git: { provider: github, repo: org/checkpoints } Per-backend overrides remain a future seam: BackendConfig can grow its own optional Git field if a use case appears that needs different destinations for different git backends. Today the single checkpoints.git applies to all of them. Synthesizer maps strategy_options.checkpoint_remote → checkpoints.git (was checkpoints.remote). Tests, doc comments, and the round-trip case updated. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: bf0d230c352c --- cmd/entire/cli/settings/load_v2.go | 4 ++-- cmd/entire/cli/settings/load_v2_test.go | 10 +++++----- cmd/entire/cli/settings/schema_v2.go | 9 ++++++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/entire/cli/settings/load_v2.go b/cmd/entire/cli/settings/load_v2.go index b3df826c85..8e2da286fb 100644 --- a/cmd/entire/cli/settings/load_v2.go +++ b/cmd/entire/cli/settings/load_v2.go @@ -368,7 +368,7 @@ func applyStrategyOptionLegacyKeys(base, overlay *Settings, so map[string]json.R base.Checkpoints.Mirrors = overlay.Checkpoints.Mirrors } if _, ok := so[legacyKeyCheckpointRemote]; ok { - base.Checkpoints.Remote = overlay.Checkpoints.Remote + base.Checkpoints.Git = overlay.Checkpoints.Git } if _, ok := so[legacyKeyFullTranscriptRetentionDays]; ok { base.Checkpoints.FullTranscriptRetentionDays = overlay.Checkpoints.FullTranscriptRetentionDays @@ -455,7 +455,7 @@ func synthesizeFromLegacy(s *EntireSettings) *Settings { func synthesizeCheckpointsConfig(s *EntireSettings) CheckpointsConfig { cfg := CheckpointsConfig{ Primary: BackendConfig{Type: legacyPrimaryBackend(s)}, - Remote: s.GetCheckpointRemote(), + Git: s.GetCheckpointRemote(), FullTranscriptRetentionDays: legacyFullTranscriptRetention(s), SignCommits: s.SignCheckpointCommits, FilteredFetches: s.IsFilteredFetchesEnabled(), diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go index 97e136df56..9dc83d79f8 100644 --- a/cmd/entire/cli/settings/load_v2_test.go +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -180,11 +180,11 @@ func TestSynthesizeFromLegacy_CheckpointFields(t *testing.T) { }, }, check: func(t *testing.T, got *Settings) { - if got.Checkpoints.Remote == nil { - t.Fatal("Remote = nil, want populated") + if got.Checkpoints.Git == nil { + t.Fatal("Git = nil, want populated") } - if got.Checkpoints.Remote.Provider != "github" || got.Checkpoints.Remote.Repo != "org/checkpoints" { - t.Fatalf("Remote = %+v, want github/org/checkpoints", got.Checkpoints.Remote) + if got.Checkpoints.Git.Provider != "github" || got.Checkpoints.Git.Repo != "org/checkpoints" { + t.Fatalf("Git = %+v, want github/org/checkpoints", got.Checkpoints.Git) } }, }, @@ -648,7 +648,7 @@ func TestSynthesizeFromLegacy_RoundTripFromBytes(t *testing.T) { Checkpoints: CheckpointsConfig{ Primary: BackendConfig{Type: BackendTypeV2}, Mirrors: []BackendConfig{{Type: BackendTypeGmeta}}, - Remote: &CheckpointRemoteConfig{Provider: "github", Repo: "org/repo"}, + Git: &CheckpointRemoteConfig{Provider: "github", Repo: "org/repo"}, FullTranscriptRetentionDays: 30, SignCommits: boolPtr(false), FilteredFetches: true, diff --git a/cmd/entire/cli/settings/schema_v2.go b/cmd/entire/cli/settings/schema_v2.go index a9962b650b..d9f88f872b 100644 --- a/cmd/entire/cli/settings/schema_v2.go +++ b/cmd/entire/cli/settings/schema_v2.go @@ -80,9 +80,12 @@ type CheckpointsConfig struct { // never serve reads — they are export targets, not sources of truth. Mirrors []BackendConfig `json:"mirrors,omitempty"` - // Remote configures the GitHub remote that hosts the checkpoint - // metadata branch. Optional; when unset, the default origin is used. - Remote *CheckpointRemoteConfig `json:"remote,omitempty"` + // Git configures the destination for git-based backends (v1, v2, gmeta). + // Optional; when unset, the default origin is used. Per-backend overrides + // can be added by giving BackendConfig its own Git field if a future use + // case needs different destinations for different git backends — today + // this single value applies to all of them. + Git *CheckpointRemoteConfig `json:"git,omitempty"` // FullTranscriptRetentionDays is the retention window (in days) for // archived raw-transcript generations. Zero/negative falls back to From 1f5ebefbb7cc521383c5190c3f3c85eda86a7acf Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Tue, 28 Apr 2026 17:15:17 +0200 Subject: [PATCH 7/8] Add testdata examples for schema v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five canonical settings files plus a README under cmd/entire/cli/settings/testdata/. Loaded by TestLoadV2_TestdataExamples — they double as hand-readable documentation of the v2 shape that you can copy into a real .entire/settings.json: - v2-minimal.json: smallest meaningful file (schema + primary) - v2-with-gmeta-mirror.json: primary v2 + gmeta mirror - v2-with-git-config.json: primary v2 + git destination override - v2-kitchen-sink.json: every documented field populated - legacy-equivalent.json: legacy shape that synthesizes to the exact same struct as v2-kitchen-sink.json The legacy↔v2 pair is verified by TestLoadV2_TestdataLegacyEquivalentMatchesKitchenSink, which loads both files and asserts reflect.DeepEqual on the parsed *Settings. If the synthesizer drifts from the v2 schema (or either file forgets a new field), this test fails. This is the most concrete documentation of the v1→v2 migration map short of reading synthesizeFromLegacy directly. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: d0d1f4afca5a --- cmd/entire/cli/settings/load_v2_test.go | 119 ++++++++++++++++++ cmd/entire/cli/settings/testdata/README.md | 24 ++++ .../settings/testdata/legacy-equivalent.json | 33 +++++ .../settings/testdata/v2-kitchen-sink.json | 42 +++++++ .../cli/settings/testdata/v2-minimal.json | 6 + .../settings/testdata/v2-with-git-config.json | 10 ++ .../testdata/v2-with-gmeta-mirror.json | 9 ++ 7 files changed, 243 insertions(+) create mode 100644 cmd/entire/cli/settings/testdata/README.md create mode 100644 cmd/entire/cli/settings/testdata/legacy-equivalent.json create mode 100644 cmd/entire/cli/settings/testdata/v2-kitchen-sink.json create mode 100644 cmd/entire/cli/settings/testdata/v2-minimal.json create mode 100644 cmd/entire/cli/settings/testdata/v2-with-git-config.json create mode 100644 cmd/entire/cli/settings/testdata/v2-with-gmeta-mirror.json diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go index 9dc83d79f8..92b4aec6c2 100644 --- a/cmd/entire/cli/settings/load_v2_test.go +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -2,6 +2,8 @@ package settings import ( "context" + "os" + "path/filepath" "reflect" "strings" "testing" @@ -1021,3 +1023,120 @@ func TestLoadV2_LegacyOverrideSummaryProviderOnly(t *testing.T) { t.Fatalf("TimeoutSeconds = %d, want 45 (preserved)", got.SummaryGeneration.TimeoutSeconds) } } + +// TestLoadV2_TestdataExamples loads each canonical example file under +// testdata/ via LoadV2FromBytes and asserts the parsed structure. These +// examples double as hand-readable documentation of the v2 shape. +func TestLoadV2_TestdataExamples(t *testing.T) { + t.Parallel() + tests := []struct { + name string + file string + check func(t *testing.T, s *Settings) + }{ + { + name: "minimal", + file: "v2-minimal.json", + check: func(t *testing.T, s *Settings) { + if s.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2", s.Checkpoints.Primary.Type) + } + if len(s.Checkpoints.Mirrors) != 0 { + t.Fatalf("Mirrors = %v, want empty", s.Checkpoints.Mirrors) + } + }, + }, + { + name: "with gmeta mirror", + file: "v2-with-gmeta-mirror.json", + check: func(t *testing.T, s *Settings) { + if len(s.Checkpoints.Mirrors) != 1 || s.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [{gmeta}]", s.Checkpoints.Mirrors) + } + }, + }, + { + name: "with git config", + file: "v2-with-git-config.json", + check: func(t *testing.T, s *Settings) { + if s.Checkpoints.Git == nil { + t.Fatal("Git = nil, want populated") + } + if s.Checkpoints.Git.Provider != "github" || s.Checkpoints.Git.Repo != "myorg/myrepo-checkpoints" { + t.Fatalf("Git = %+v", s.Checkpoints.Git) + } + }, + }, + { + name: "kitchen sink", + file: "v2-kitchen-sink.json", + check: func(t *testing.T, s *Settings) { + if s.Logging.Level != debugLevel { + t.Fatalf("Logging.Level = %q", s.Logging.Level) + } + if s.Checkpoints.Git == nil || s.Checkpoints.FullTranscriptRetentionDays != 90 { + t.Fatalf("Checkpoints = %+v", s.Checkpoints) + } + if !s.Features.Summarize || !s.Features.ExternalAgents { + t.Fatalf("Features = %+v", s.Features) + } + if s.SummaryGeneration == nil || s.SummaryGeneration.Provider != providerClaudeCC { + t.Fatalf("SummaryGeneration = %+v", s.SummaryGeneration) + } + }, + }, + { + name: "legacy equivalent", + file: "legacy-equivalent.json", + check: func(t *testing.T, s *Settings) { + if s.Checkpoints.Primary.Type != BackendTypeV2 { + t.Fatalf("Primary.Type = %q, want v2 (synthesized)", s.Checkpoints.Primary.Type) + } + if len(s.Checkpoints.Mirrors) != 1 || s.Checkpoints.Mirrors[0].Type != BackendTypeGmeta { + t.Fatalf("Mirrors = %v, want [{gmeta}]", s.Checkpoints.Mirrors) + } + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + data := readTestdataFile(t, tt.file) + got, err := LoadV2FromBytes(data) + if err != nil { + t.Fatalf("LoadV2FromBytes(%s): %v", tt.file, err) + } + tt.check(t, got) + }) + } +} + +// TestLoadV2_TestdataLegacyEquivalentMatchesKitchenSink verifies that +// legacy-equivalent.json synthesizes to the same Settings as parsing +// v2-kitchen-sink.json directly. This pair documents the legacy → v2 +// migration map concretely; if a new legacy key gets added to the +// synthesizer, both files should be updated and this test will catch +// drift between them. +func TestLoadV2_TestdataLegacyEquivalentMatchesKitchenSink(t *testing.T) { + t.Parallel() + v2, err := LoadV2FromBytes(readTestdataFile(t, "v2-kitchen-sink.json")) + if err != nil { + t.Fatalf("LoadV2FromBytes(v2-kitchen-sink): %v", err) + } + legacy, err := LoadV2FromBytes(readTestdataFile(t, "legacy-equivalent.json")) + if err != nil { + t.Fatalf("LoadV2FromBytes(legacy-equivalent): %v", err) + } + if !reflect.DeepEqual(v2, legacy) { + t.Fatalf("legacy-equivalent does not match v2-kitchen-sink:\n v2: %+v\n legacy: %+v", v2, legacy) + } +} + +func readTestdataFile(t *testing.T, name string) []byte { + t.Helper() + data, err := os.ReadFile(filepath.Join("testdata", name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + return data +} diff --git a/cmd/entire/cli/settings/testdata/README.md b/cmd/entire/cli/settings/testdata/README.md new file mode 100644 index 0000000000..8f6c3aef00 --- /dev/null +++ b/cmd/entire/cli/settings/testdata/README.md @@ -0,0 +1,24 @@ +# Settings testdata + +Canonical example settings.json files. Loaded by `TestLoadV2_TestdataExamples` +in `load_v2_test.go`; doubles as hand-readable documentation of the v2 shape. + +## Files + +| File | Description | +| --- | --- | +| `v2-minimal.json` | Smallest meaningful schema-v2 file: `schema` + a primary backend. | +| `v2-with-gmeta-mirror.json` | Primary v2 + a gmeta mirror (write-only fan-out). | +| `v2-with-git-config.json` | Primary v2 + a git destination override (separate checkpoint repo). | +| `v2-kitchen-sink.json` | Every documented field populated — useful as a reference. | +| `legacy-equivalent.json` | Legacy-shape settings that synthesizes to the same struct as `v2-kitchen-sink.json`. | + +## Migration mapping + +`legacy-equivalent.json` and `v2-kitchen-sink.json` are paired: loading either +through `LoadV2FromBytes` produces an identical `*Settings` value (modulo +defaults that round-trip correctly). The pairing is the most concrete way +to read the legacy → v2 mapping. + +When adding a new legacy field that the synthesizer should translate, add it +to both files and the equivalence test confirms the round-trip. diff --git a/cmd/entire/cli/settings/testdata/legacy-equivalent.json b/cmd/entire/cli/settings/testdata/legacy-equivalent.json new file mode 100644 index 0000000000..9f54e26730 --- /dev/null +++ b/cmd/entire/cli/settings/testdata/legacy-equivalent.json @@ -0,0 +1,33 @@ +{ + "enabled": true, + "log_level": "debug", + "commit_linking": "always", + "external_agents": true, + "telemetry": true, + "sign_checkpoint_commits": true, + "summary_timeout_seconds": 60, + "summary_generation": { + "provider": "claude-code", + "model": "sonnet" + }, + "redaction": { + "pii": { + "enabled": true, + "email": true, + "phone": true, + "address": false + } + }, + "strategy_options": { + "checkpoints_version": 2, + "gmeta": true, + "checkpoint_remote": { + "provider": "github", + "repo": "myorg/myrepo-checkpoints" + }, + "summarize": {"enabled": true}, + "filtered_fetches": true, + "push_sessions": true, + "full_transcript_generation_retention_days": 90 + } +} diff --git a/cmd/entire/cli/settings/testdata/v2-kitchen-sink.json b/cmd/entire/cli/settings/testdata/v2-kitchen-sink.json new file mode 100644 index 0000000000..8a63c2cc01 --- /dev/null +++ b/cmd/entire/cli/settings/testdata/v2-kitchen-sink.json @@ -0,0 +1,42 @@ +{ + "schema": 2, + "enabled": true, + "logging": { + "level": "debug" + }, + "checkpoints": { + "primary": {"type": "v2"}, + "mirrors": [ + {"type": "gmeta"} + ], + "git": { + "provider": "github", + "repo": "myorg/myrepo-checkpoints" + }, + "full_transcript_retention_days": 90, + "sign_commits": true, + "filtered_fetches": true, + "push_sessions": true + }, + "hooks": { + "commit_linking": "always" + }, + "features": { + "summarize": true, + "external_agents": true + }, + "redaction": { + "pii": { + "enabled": true, + "email": true, + "phone": true, + "address": false + } + }, + "telemetry": true, + "summary_generation": { + "provider": "claude-code", + "model": "sonnet", + "timeout_seconds": 60 + } +} diff --git a/cmd/entire/cli/settings/testdata/v2-minimal.json b/cmd/entire/cli/settings/testdata/v2-minimal.json new file mode 100644 index 0000000000..caee7bda54 --- /dev/null +++ b/cmd/entire/cli/settings/testdata/v2-minimal.json @@ -0,0 +1,6 @@ +{ + "schema": 2, + "checkpoints": { + "primary": {"type": "v2"} + } +} diff --git a/cmd/entire/cli/settings/testdata/v2-with-git-config.json b/cmd/entire/cli/settings/testdata/v2-with-git-config.json new file mode 100644 index 0000000000..7ce59b21a7 --- /dev/null +++ b/cmd/entire/cli/settings/testdata/v2-with-git-config.json @@ -0,0 +1,10 @@ +{ + "schema": 2, + "checkpoints": { + "primary": {"type": "v2"}, + "git": { + "provider": "github", + "repo": "myorg/myrepo-checkpoints" + } + } +} diff --git a/cmd/entire/cli/settings/testdata/v2-with-gmeta-mirror.json b/cmd/entire/cli/settings/testdata/v2-with-gmeta-mirror.json new file mode 100644 index 0000000000..57775df72f --- /dev/null +++ b/cmd/entire/cli/settings/testdata/v2-with-gmeta-mirror.json @@ -0,0 +1,9 @@ +{ + "schema": 2, + "checkpoints": { + "primary": {"type": "v2"}, + "mirrors": [ + {"type": "gmeta"} + ] + } +} From bee8422c726d84c41b323f8db83941df3682f940 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Tue, 28 Apr 2026 21:27:07 +0200 Subject: [PATCH 8/8] Extract modelSonnet constant in settings tests Resolves goconst lint failure on `"sonnet"` (6 occurrences) and removes the now-unused `//nolint:goconst` directive flagged by nolintlint. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 0cbb06180772 --- cmd/entire/cli/settings/load_v2_test.go | 13 +++++++------ cmd/entire/cli/settings/settings_test.go | 10 +++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cmd/entire/cli/settings/load_v2_test.go b/cmd/entire/cli/settings/load_v2_test.go index 92b4aec6c2..db1f46e379 100644 --- a/cmd/entire/cli/settings/load_v2_test.go +++ b/cmd/entire/cli/settings/load_v2_test.go @@ -12,6 +12,7 @@ import ( const ( debugLevel = "debug" providerClaudeCC = "claude-code" + modelSonnet = "sonnet" ) func boolPtr(b bool) *bool { return &b } @@ -414,14 +415,14 @@ func TestSynthesizeFromLegacy_SummaryGeneration(t *testing.T) { Enabled: true, SummaryGeneration: &SummaryGenerationSettings{ Provider: providerClaudeCC, - Model: "sonnet", + Model: modelSonnet, }, }, check: func(t *testing.T, got *Settings) { if got.SummaryGeneration == nil { t.Fatal("SummaryGeneration = nil") } - if got.SummaryGeneration.Provider != providerClaudeCC || got.SummaryGeneration.Model != "sonnet" { + if got.SummaryGeneration.Provider != providerClaudeCC || got.SummaryGeneration.Model != modelSonnet { t.Fatalf("SummaryGeneration = %+v, want claude-code/sonnet", got.SummaryGeneration) } }, @@ -660,7 +661,7 @@ func TestSynthesizeFromLegacy_RoundTripFromBytes(t *testing.T) { Telemetry: boolPtr(true), SummaryGeneration: &SummaryGenerationConfig{ Provider: providerClaudeCC, - Model: "sonnet", + Model: modelSonnet, TimeoutSeconds: 45, }, } @@ -872,7 +873,7 @@ func TestSettings_Validate(t *testing.T) { s: &Settings{ Schema: CurrentSchemaVersion, Checkpoints: CheckpointsConfig{Primary: BackendConfig{Type: BackendTypeV1}}, - SummaryGeneration: &SummaryGenerationConfig{Model: "sonnet"}, + SummaryGeneration: &SummaryGenerationConfig{Model: modelSonnet}, }, wantErr: "summary_generation.model", }, @@ -986,7 +987,7 @@ func TestLoadV2_LegacyOverrideSummaryTimeoutOnly(t *testing.T) { if got.SummaryGeneration.Provider != providerClaudeCC { t.Fatalf("Provider = %q, want %q (preserved)", got.SummaryGeneration.Provider, providerClaudeCC) } - if got.SummaryGeneration.Model != "sonnet" { + if got.SummaryGeneration.Model != modelSonnet { t.Fatalf("Model = %q, want sonnet (preserved)", got.SummaryGeneration.Model) } if got.SummaryGeneration.TimeoutSeconds != 30 { @@ -1016,7 +1017,7 @@ func TestLoadV2_LegacyOverrideSummaryProviderOnly(t *testing.T) { if got.SummaryGeneration.Provider != "codex" { t.Fatalf("Provider = %q, want codex (overridden)", got.SummaryGeneration.Provider) } - if got.SummaryGeneration.Model != "sonnet" { + if got.SummaryGeneration.Model != modelSonnet { t.Fatalf("Model = %q, want sonnet (preserved)", got.SummaryGeneration.Model) } if got.SummaryGeneration.TimeoutSeconds != 45 { diff --git a/cmd/entire/cli/settings/settings_test.go b/cmd/entire/cli/settings/settings_test.go index 5aaad57308..7d68890d5f 100644 --- a/cmd/entire/cli/settings/settings_test.go +++ b/cmd/entire/cli/settings/settings_test.go @@ -133,7 +133,7 @@ func TestLoad_AcceptsValidKeys(t *testing.T) { if settings.SummaryGeneration.Provider != "claude-code" { t.Errorf("expected summary_generation.provider 'claude-code', got %q", settings.SummaryGeneration.Provider) } - if settings.SummaryGeneration.Model != "sonnet" { //nolint:goconst // test literal + if settings.SummaryGeneration.Model != modelSonnet { t.Errorf("expected summary_generation.model 'sonnet', got %q", settings.SummaryGeneration.Model) } if settings.Redaction == nil { @@ -583,7 +583,7 @@ func TestLoadFromFile_AcceptsModelWithoutProvider(t *testing.T) { if err != nil { t.Fatalf("LoadFromFile should accept model-only file, got error: %v", err) } - if s.SummaryGeneration == nil || s.SummaryGeneration.Model != "sonnet" { + if s.SummaryGeneration == nil || s.SummaryGeneration.Model != modelSonnet { t.Fatalf("expected model 'sonnet', got %+v", s.SummaryGeneration) } } @@ -597,8 +597,8 @@ func TestSummaryGenerationSettings_Validate(t *testing.T) { wantErr bool }{ {name: "nil receiver is valid", s: nil, wantErr: false}, - {name: "provider and model is valid", s: &SummaryGenerationSettings{Provider: "claude-code", Model: "sonnet"}, wantErr: false}, - {name: "model without provider is invalid", s: &SummaryGenerationSettings{Model: "sonnet"}, wantErr: true}, + {name: "provider and model is valid", s: &SummaryGenerationSettings{Provider: "claude-code", Model: modelSonnet}, wantErr: false}, + {name: "model without provider is invalid", s: &SummaryGenerationSettings{Model: modelSonnet}, wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -660,7 +660,7 @@ func TestMergeJSON_SummaryGeneration_SameProviderPreservesModel(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if s.SummaryGeneration.Provider != "claude-code" || s.SummaryGeneration.Model != "sonnet" { + if s.SummaryGeneration.Provider != "claude-code" || s.SummaryGeneration.Model != modelSonnet { t.Errorf("Provider/Model = %q/%q, want claude-code/sonnet", s.SummaryGeneration.Provider, s.SummaryGeneration.Model) } }