Skip to content

Config-key renames hard-crash all existing user configs at startup — no retired-key mechanism, no migration enforcement #4303

Description

@Trecek

Incident

PR #4301 (merge d1d02e338, shipped in 0.10.885/0.10.886) renamed the config key diagnostics.post_run_analysisdiagnostics.pipeline_health. Any pre-existing ~/.autoskillit/config.yaml (or project .autoskillit/config.yaml) containing the formerly-valid key now raises at every load_config() call:

autoskillit.config._config_dataclasses.ConfigSchemaError: Invalid configuration in
'~/.autoskillit/config.yaml': unrecognized key 'diagnostics.post_run_analysis' in
section 'diagnostics'.

Blast radius is near-total: serve (the default command — the MCP server cannot start, so open_kitchen and every downstream skill are unreachable), cook, order, doctor, and all fleet/workspace/feature/session subcommands crash identically. doctor dies at cli/doctor/__init__.py:85 before any of its 39 checks run — the diagnostic tool cannot diagnose this failure. Survivors (at minimum): update, migrate, --version, likely install, and init (guarded at cli/app.py:224-231 for unrelated backend-fallback reasons, commit e5a57c977).

The difflib.get_close_matches "did you mean" hint provably never fires: SequenceMatcher('post_run_analysis', 'pipeline_health').ratio() = 0.25, below the 0.6 cutoff (settings.py:583-584) — the user gets zero remediation guidance.

The immediate outage has been fixed manually (one-line edit of the user config), so the reporter's machine no longer reproduces it. A fixer can reproduce in seconds with a synthetic config in an isolated HOME:

mkdir -p /tmp/repro-home/.autoskillit
printf 'diagnostics:\n  post_run_analysis: false\n' > /tmp/repro-home/.autoskillit/config.yaml
HOME=/tmp/repro-home autoskillit doctor   # → ConfigSchemaError

This issue is for the architectural flaw, which will produce a fourth outage on the next config-key rename unless fixed.

Root cause (architectural)

validate_layer_keys (src/autoskillit/config/settings.py:563-613) is fail-closed against the schema derived from the current dataclasses (_build_config_schema, settings.py:525-560) and has no concept of a formerly-valid key. It conflates two classes that industry practice keeps distinct:

Why the shipped migration note didn't help

A migration note exists — src/autoskillit/migrations/0.10.884-to-0.10.885.yaml, added reactively during review (commit 421602423, ~26 min before the merge landed) — and it even carries machine-readable detect.key and example_after fields. But it is advisory prose:

  • default_migration_engine() (src/autoskillit/migration/engine.py:379-383) registers recipe/contract/diagram adapters only — no config adapter exists anywhere.
  • Nothing reads detect.key for config files; the field is only re-serialized into an LLM prompt for recipe migration.
  • autoskillit update (src/autoskillit/cli/update/_update.py:19-85) never invokes the migration engine at all.
  • The note's own test (tests/migration/test_pipeline_health_migration_note.py) asserts the YAML's prose content only — it never loads a config, creating false confidence that "the migration is covered."

Why tests structurally could not catch this

The failure requires combining current code with a config file written by a prior version; every test tier excludes that combination by construction:

  1. Self-referential fixtures_CONFIG_SCHEMA is generated from the live dataclasses; every test config is written against the current schema. The rename commit (592a9dfcc) mechanically substituted the old key across 17 test files, so the suite validates the new world against the new world.
  2. HOME isolation — 20+ tests replace $HOME with a temp dir, so the exact file that crashes real users is structurally invisible to the entire suite.
  3. Prose-only migration test (above).
  4. No installed-executable / golden-corpus tier — nothing installs the wheel and runs the CLI against a realistic prior-version HOME.

Smoking gun: during the #4301 work, test_sigterm_writes_scenario_json did fail because its subprocess loaded the developer's real config with the stale key. Commit 16afef492 fixed it by HOME-isolating the test — its message explicitly cites "stale pre-rename keys" in "the developer's real ~/.autoskillit/config.yaml". The suite caught this exact bug class once, and the signal was suppressed rather than recognized as the compatibility break it predicted.

Recurring class — third occurrence

Rename Version Outcome
features.franchisefeatures.fleet (#1206, 23527284a) 0.9.134→135 Done right at the time: additive deprecation shims shipped first ("safety net that the rename commit can rely on"). The shims were since removed — franchise is absent from today's FEATURE_REGISTRY (verified against installed 0.10.886), so pre-0.9.135 configs crash again. A temporary shim without a permanent registry decays back into the same break.
quota_guard.threshold → split thresholds (5a5d20fab) 0.8.38→39 Same hard break — commit message admits users "get a ConfigSchemaError at startup... but no migration note existed"
diagnostics.post_run_analysispipeline_health (#4301) 0.10.884→885 Same hard break — note added reactively, no shim

Config is the only schema-bearing subsystem with no rename guard: hooks have RETIRED_SCRIPT_BASENAMES + guard test, skills have RETIRED_SKILL_NAMES + guard test, recipes have versioned load-time auto-migration with backups. Config keys have nothing.

Proposed remediation (converged; blast radius LOW; adversarially validated)

  1. RETIRED_CONFIG_KEYS: dict[tuple[str, str], str] in config/settings.py beside _YAML_KEY_ALIASES — append-only, mirroring the RETIRED_SKILL_NAMES idiom. Seed with all three documented config-key renames (verified: these are the only three across all 14 migrations/*.yaml notes):

    • ("diagnostics", "post_run_analysis"): "pipeline_health" — the current incident.
    • ("quota_guard", "threshold"): "short_window_threshold"scoping caveat: 0.8.38 was a one-to-two split (short_window_threshold + long_window_threshold, dataclass defaults 85.0/95.0). The one-to-one remap carries the user's old value to short_window_threshold only; long_window_threshold keeps its default. The deprecation warning for this entry must name both new fields so users review the second one. This is deliberate lossy-but-safe scoping, not an oversight.
    • ("features", "franchise"): "fleet" — the feat(T1): fleet deprecation shims for franchise→fleet rename #1206 shims were later removed, so this key crashes today too. Note: the features section is validated by a separate path (_build_features_dict, settings.py:394-425, raising on unknown feature names against FEATURE_REGISTRY), not by validate_layer_keys sub-key checks — the per-layer remap still covers it because it rewrites the raw layer dict upstream of both validators.

    Design rules to state in the registry docstring: (a) retired names may never be reused; (b) entries must always map to the current name — chained renames (old1→old2→new) get a direct old1→new entry, enforced automatically by the arch invariant "new key ∈ _CONFIG_SCHEMA"; (c) the (section, key) shape covers sub-key renames only — a future top-level section rename (rejected separately at settings.py:585-588) would need a dotted-path extension of the registry; state this limitation rather than speculatively building it.

  2. Shared helper remap_retired_keys() applied per-layer in the load loop (_config_loader.py:143-149) before validate_layer_keys, gated to validated non-secrets layers (should_validate=True, is_secrets_layer=False): move value old→new when the new key is not explicitly set in that layer (explicit new key wins; per-layer remap preserves dynaconf merge precedence), delete the old key, logger.warning old→new + migration note. This covers both the user-global (~/.autoskillit/config.yaml) and project (.autoskillit/config.yaml) layers — both are marked should_validate=True in the layer table (_config_loader.py:136-141) — and therefore also heals fleet-spawned clones/worktrees, which load their project configs through this same loader. Never-valid keys still hard-fail — the Implementation Plan: Strict Schema Validation for config.yaml #454/Rectify: Config Schema Contamination & Misplaced Secrets Immunity — PART A ONLY #476 typo-protection and contamination-closure intent is untouched. Warning channel is logging (architecturally stderr-only per core/logging.py) — safe for MCP stdio on serve.

  3. Write path unchangedwrite_config_layer keeps rejecting retired keys; programmatic writers must use current spellings.

  4. Doctor: wrap load_config at cli/doctor/__init__.py:85 in try/except ConfigSchemaError → synthetic failing DoctorResult (doctor must never die of the disease it diagnoses); route _check_config_layers_for_secrets (_doctor_config.py:27-66) through the same helper so doctor and loader agree on retired keys.

  5. Guard tests (the immunity):

    • Arch invariant: every retired old key ∉ _CONFIG_SCHEMA; every new key ∈ _CONFIG_SCHEMA; neither side is a SECRETS_ONLY_KEYS path (a remap target must never mask the secrets-placement check); every entry has a matching migrations/*.yaml note whose detect.key matches — binding registry ↔ notes so they cannot drift.
    • Behavior: old-key config loads, populates the new field, warns (caplog).
    • Golden corpus: frozen prior-version config fixtures under tests/config/ — the first tier combining current code with prior-version configs.

Killed alternatives (documented so they aren't re-litigated)

  • Deterministic ConfigMigrationAdapter (file rewrite): cannot fix already-broken users (every discovery surface — serve/doctor/MCP — crashes before it could run; only update/migrate survive); destroys YAML comments (plain-PyYAML dump_yaml_str, core/io.py); misses sibling worktree/clone configs; ~3× effort. Possible later as an optional explicit cleanup command on top of the same registry.
  • _YAML_KEY_ALIASES entry: it is a redirect (one accepted spelling per field, settings.py:337-338), not a fallback — an entry would break the new spelling; and it never runs for a doomed config (validation raises before _build_subconfig).
  • Softening validation globally: destroys the typo-protection intent of Implementation Plan: Strict Schema Validation for config.yaml #454/Rectify: Config Schema Contamination & Misplaced Secrets Immunity — PART A ONLY #476.
  • Doc-only migration notes: the status quo that already failed twice.

External precedent

No surveyed mainstream tool (ruff, Cargo, VS Code, Black, pydantic AliasChoices, ESLint) hard-fails startup on a key it renamed itself. Consensus: warn-and-map for formerly-valid keys through a deprecation window; hard-fail only never-valid keys; file rewriting only as an explicit user-invoked command with backup.

References

Investigation

Prior investigation completed interactively (deep mode: 2 batches, adversarial challenge, blast-radius + breakage analysis, 2 validators, plus post-filing adversarial issue audit — corrections applied). Full report below.

Investigation: ConfigSchemaError Startup Crash — diagnostics.post_run_analysis Rejected After Rename

Date: 2026-07-20
Scope: Runtime crash of every config-loading autoskillit CLI command (observed via autoskillit cook) on installed 0.10.886 with a pre-rename user config; root cause, test-gap architecture, and fix approach
Mode: Deep Analysis

Summary

autoskillit cook (and nearly every other CLI command) crashes at startup because PR #4301 ("Pipeline Health Mechanism Naming Alignment", merge d1d02e338, 2026-07-20, shipped in 0.10.885/0.10.886) renamed the config key diagnostics.post_run_analysis to diagnostics.pipeline_health without any runtime compatibility mechanism. The user's ~/.autoskillit/config.yaml still contains the formerly-valid key; validate_layer_keys (fail-closed schema validation derived from the current dataclasses) rejects it as unrecognized and raises ConfigSchemaError before any command logic runs. A migration note YAML was shipped, but it is advisory prose that no code applies to config files — the migration engine only has recipe/contract/diagram adapters. The test suite could not catch this because its fixtures are self-referentially generated from the current schema and 20+ tests actively HOME-isolate away the real user-config layer; during the #4301 work a subprocess test did fail on the developer's real stale config, and the failing signal was suppressed by adding HOME isolation to the test rather than treated as a compatibility bug. This failure class has occurred before (quota_guard.threshold, 0.8.38→0.8.39) and the correct additive-shim pattern exists in-repo (franchisefleet, #1206) but was not applied.

Root Cause

Immediate cause~/.autoskillit/config.yaml:5 contains post_run_analysis: false under diagnostics:. The installed 0.10.886 schema (DiagnosticsConfig, src/autoskillit/config/_config_dataclasses.py:308) defines only pipeline_health. _make_dynaconf (src/autoskillit/config/_config_loader.py:117-186) validates the user-global layer with should_validate=True at line 149, and validate_layer_keys (src/autoskillit/config/settings.py:563-613) hard-raises ConfigSchemaError on the unrecognized sub-key (lines 610-613). The difflib.get_close_matches "did you mean" hint never fires: similarity of post_run_analysis vs pipeline_health is ~0.25, below the 0.6 cutoff, so the user gets zero remediation guidance. [SUPPORTED]

Underlying cause — the rename (9b2b425e6, squashed into d1d02e338) shipped with no compatibility bridge for formerly-valid keys:

  1. No alias: _YAML_KEY_ALIASES (settings.py:272-275) covers only two model.* spellings, and it is a redirect (one accepted YAML spelling per field, settings.py:337-338), not a dual-key fallback — it could not express a deprecation window even if an entry had been added. [SUPPORTED]
  2. No tombstone/retired-key registry: config is the only schema-bearing subsystem without one (hooks have RETIRED_SCRIPT_BASENAMES, skills have RETIRED_SKILL_NAMES, recipes have versioned auto-migration). [SUPPORTED]
  3. The shipped migration note (src/autoskillit/migrations/0.10.884-to-0.10.885.yaml, added reactively during review as 421602423 "fix(review): add pipeline health config migration note") is machine-readable in principle (detect.key, example_after) but nothing consumes it for config files: default_migration_engine() (src/autoskillit/migration/engine.py:379-383) registers recipe/contract/diagram adapters only, and neither _config_loader.py nor settings.py references migration/ at all. [SUPPORTED]
  4. No upgrade-time check: autoskillit update (src/autoskillit/cli/update/_update.py:19-85) runs the upgrade subprocess and autoskillit install, never invoking the migration engine or config validation. [SUPPORTED]

Blast radius — near-total outage: serve (the default command — the MCP server itself cannot start, so open_kitchen and every downstream skill are unreachable), cook, order, doctor, and all fleet/workspace/feature/session subcommands call load_config() and crash identically. doctor crashes at cli/doctor/__init__.py:85 before any of its 39 checks run — the diagnostic tool cannot diagnose this failure. Survivors (at minimum): update, migrate, --version (cyclopts built-in short-circuits), likely install (its registration path avoids load_config), and init (whose try/except ConfigSchemaError at cli/app.py:224-231 was added in e5a57c977 for unrelated backend-selection fallback). [SUPPORTED]

Affected Components

  • ~/.autoskillit/config.yaml (line 5): stale formerly-valid key — the triggering data [SUPPORTED]
  • src/autoskillit/config/settings.py:563-613 (validate_layer_keys): fail-closed rejection with no formerly-valid-key handling [SUPPORTED]
  • src/autoskillit/config/settings.py:525-560 (_build_config_schema / _CONFIG_SCHEMA): allowed-key set derived solely from current dataclass fields [SUPPORTED]
  • src/autoskillit/config/_config_dataclasses.py:306-308 (DiagnosticsConfig): renamed field, sole surviving spelling [SUPPORTED]
  • src/autoskillit/config/_config_loader.py:117-186 (_make_dynaconf): user-global layer validated at line 149; crash site [SUPPORTED]
  • src/autoskillit/migrations/0.10.884-to-0.10.885.yaml: advisory note describing this exact break; unconsumed for configs [SUPPORTED]
  • src/autoskillit/migration/engine.py (adapter registry): no config adapter exists [SUPPORTED]
  • src/autoskillit/cli/doctor/__init__.py:85: unguarded load_config — doctor dies before its checks [SUPPORTED]
  • tests/execution/test_recording_sigterm.py:36-46: HOME isolation added by 16afef492 that suppressed the real-world failure signal [SUPPORTED]

Data Flow

autoskillit cookcli/app.py:_cook_cmdcli/session/_session_cook.py:84 load_config()config/_config_loader.py:193 from_dynaconf(_make_dynaconf(project_dir))_make_dynaconf iterates four layers (defaults unvalidated → user-global ~/.autoskillit/config.yaml validated → project config validated → secrets validated), calling validate_layer_keys(data, path) per layer at _config_loader.py:149settings.py:610 raises on the first formerly-valid key it no longer knows. The exception propagates uncaught to the CLI entry point. The value never reaches _build_subconfig — validation rejects the layer before value resolution.

Test Gap Analysis

Four structural properties made this failure class untestable-by-construction, and one process event actively discarded the only real-world signal:

  1. Self-referential fixtures. _CONFIG_SCHEMA is generated from the live dataclasses (settings.py:525-560); every test config in tests/config/ is hand-written against the current schema. When a key is renamed, tests are renamed with it in the same commit (592a9dfcc mechanically substituted the old token across 17 test files), so the suite always validates the new world against the new world. No fixture corpus of prior-version configs exists. [SUPPORTED]
  2. HOME isolation blinds every test to the user-global layer. _make_dynaconf reads Path.home()/.autoskillit/config.yaml; tests substitute a temp HOME (20+ files follow this pattern), so the exact file that crashes real users is structurally absent from every test's data path. [SUPPORTED]
  3. The migration note test asserts prose, not behavior. tests/migration/test_pipeline_health_migration_note.py checks the YAML exists, has matching from/to versions, and contains expected substrings. It never feeds the old key through load_config. It creates false confidence that "the migration is covered." [SUPPORTED]
  4. No installed-executable tier. No test installs the wheel and invokes the console script against a realistic HOME; smoke tests run run_python callables in-process and never exercise load_config. [SUPPORTED]
  5. The signal was caught and suppressed. During the Implementation Plan: Pipeline Health Mechanism Naming Alignment #4301 work, test_sigterm_writes_scenario_json failed because its subprocess loaded the developer's real config containing the stale key. Commit 16afef492 (2026-07-20 08:34, same PR stream, ~1.5h before the merge landed) explicitly states the subprocess "does not load the developer's real ~/.autoskillit/config.yaml, which can contain stale pre-rename keys and fail schema validation" and fixed it with HOME isolation — cementing the blindness pattern instead of recognizing the compatibility bug it had just demonstrated. [SUPPORTED — commit message quoted verbatim by verification agent]

The deepest gap: the failure requires combining "current code" with "a config file written by a prior version," and every tier of the suite structurally excludes that combination. No CI gate, pre-commit hook, or release checklist ties "config schema change" to "compatibility mechanism must exist" (verified across .github/workflows, Taskfile.yml, pre-commit config).

Similar Patterns

  • Hooks: RETIRED_SCRIPT_BASENAMES (hook_registry.py:377) + test_no_retired_name_has_a_live_file — rename guard enforced in the same commit. [SUPPORTED]
  • Skills: RETIRED_SKILL_NAMES (core/types/_type_constants.py:192-201, append-only, import-time assertion) + test_no_retired_skill_name_has_a_live_directory. [SUPPORTED]
  • Recipes: versioned migration engine with load-time auto-migration, .yaml.bak backups via atomic_write (engine.py:150-153), and the migrate_recipe MCP tool. [SUPPORTED]
  • Enums/parsers: sealed-enum immunity for external schema drift (bb07f6c7b, Rectify: Codex NDJSON Parser Schema Drift — Sealed Enum Immunity #3765) — the project's established idiom is "closed set + architectural guard test in the same commit."
  • Config keys: none of the above. The only prior correct config rename, features.franchisefeatures.fleet (feat(T1): fleet deprecation shims for franchise→fleet rename #1206, 23527284a, 2026-04-24), shipped additive deprecation shims first ("purely additive... safety net that the rename commit (T2) can rely on") — proof the right pattern was known in-repo three months before Implementation Plan: Pipeline Health Mechanism Naming Alignment #4301. [SUPPORTED]

Design Intent Findings

  • validate_layer_keys (strict fail-closed validation): introduced bc3594b76 (2026-03-21 UTC, Implementation Plan: Strict Schema Validation for config.yaml #454, "Strict Schema Validation for config.yaml") — security-motivated: defaults.yaml is the sole trusted layer; all user-writable layers are treated as error-adjacent input, with secrets-placement enforcement. Reinforced by Rectify 4c03177da (Rectify: Config Schema Contamination & Misplaced Secrets Immunity — PART A ONLY #476): when strictness itself caused an outage (system-written key not in schema), the fix routed state to a never-validated .state.yaml and kept the gate hard. Callers: _config_loader.py:149, settings.py:631 (write_config_layer), cli/doctor/_doctor_config.py:55. Design intent: reject never-valid input loudly. The mechanism has no concept of formerly-valid input. [SUPPORTED]
  • Migration note system + engine: introduced 9902cf8cb/0bea2a6ec (2026-02-23, Feature: LLM-assisted migration system for user pipeline scripts #14) as an "LLM migration system" for pipeline scripts/recipes — advisory, suppressible, WARNING-severity semantics. Later reorganized into src/autoskillit/migration/ with three adapters (recipe=headless-LLM, contract=deterministic, diagram=advisory). Its AGENTS.md label "versioned config migration" is documentation drift — no config adapter exists. Consumers: server/_factory.py:53, cli/app.py:287, _doctor_fleet.py:22. The design never intended validate_layer_keys to consult migration notes: zero imports in either direction; the two mechanisms were built independently, three weeks apart, for different domains. [SUPPORTED]
  • Tension: humans use migration notes to describe breaks that validate_layer_keys enforces (the 0.10.884 note literally says the old key "will raise a ConfigSchemaError at startup"), but no code bridges description to enforcement — the note rides on infrastructure that cannot parse it for configs. [SUPPORTED]

Historical Context

This is a recurring failure class, third occurrence, first time investigated:

  • quota_guard.threshold → split thresholds (0.8.38→0.8.39, 5a5d20fab): commit message states users with the old key "get a ConfigSchemaError at startup... but no migration note existed" — identical hard-break shape, patched reactively with a prose-only note. [SUPPORTED]
  • features.franchisefeatures.fleet (0.9.134→0.9.135, feat(T1): fleet deprecation shims for franchise→fleet rename #1206): shipped correctly with additive deprecation shims before the rename. [SUPPORTED]
  • diagnostics.post_run_analysisdiagnostics.pipeline_health (0.10.884→0.10.885, Implementation Plan: Pipeline Health Mechanism Naming Alignment #4301): today's case — hard break, note added as a review-cycle afterthought (421602423), no shim.

No prior /investigate run or fix commit addresses this root cause (session-log matches for ConfigSchemaError were exception-catalog listings, not incidents). No dedicated Rectify for config-key rename safety has ever shipped despite two prior occurrences of the drift shape.

This is a recurring pattern — consider running /rectify for architectural immunity after resolving the immediate issue.

External Research

Industry consensus (sources gathered via web research; URLs in agent evidence):

  • No surveyed mainstream tool hard-fails startup on a key it renamed itself. Ruff maps old→new with deprecation warnings across several minor versions (docs.astral.sh/ruff/settings, astral.sh/blog/ruff-v0.2.0); Cargo warns on unused/renamed manifest keys, never fails (github.com/Optional dependencies with links cause failure even when not used rust-lang/cargo#10066); VS Code grays out obsolete settings; Black v24.1 warns on invalid pyproject keys; pydantic v2 AliasChoices accepts old and new names simultaneously (docs.pydantic.dev/latest/concepts/alias). ESLint's flat-config migration ran across two major versions with an explicit opt-in codemod (@eslint/migrate-config) that generates a new file rather than overwriting (eslint.org/docs/latest/use/configure/migration-guide).
  • The strict/lenient distinction that matters is "never-valid" (reject — typo protection, cf. pytest --strict-config) vs "formerly-valid" (map + warn, remove at a major boundary). Current validate_layer_keys conflates the two.
  • Dynaconf 3.x has no built-in renamed-key/deprecation mechanism (dynaconf.com/validation) — aliasing must be app-side, as the repo already does narrowly with _YAML_KEY_ALIASES.
  • Auto-rewriting user configs is the minority pattern and carries documented pitfalls: PyYAML destroys comments (Load comments yaml/pyyaml#90 — the user's real config contains meaningful comments); even ruamel.yaml round-trip has edge cases; atomic-write/permission/cross-filesystem hazards are well documented (e.g. .claude.json becomes corrupted (Unexpected EOF) during tool use — non-atomic config writes anthropics/claude-code#28809 for .claude.json corruption). Explicit user-invoked migrate commands with backups (Terraform's undisablable .tfstate.backup, npm lockfile migration, K8s StorageVersionMigration) are the accepted shapes.

Scope Boundary

Investigated: config validation code path and schema derivation; migration engine architecture and all 14 migration notes; complete CLI blast radius; update/install/doctor pathways; test suite structure for config validation incl. HOME isolation patterns and the #4301 test-change history; design intent of both mechanisms (introducing commits, callers, docs); prior rename precedents in-repo; external industry patterns; session-log recurrence check.

Not yet explored: whether other stale keys beyond diagnostics.post_run_analysis exist in this user's config or other users' configs (only this one triggers today because validation raises on the first offender per layer — the same file may fail again on the next key after manual fix if others exist); the full content of the other 11 migration notes for additional latent renames; Codex-backend config surfaces; whether project-level .autoskillit/config.yaml files in existing worktrees/clones also carry the stale key (same crash applies to any of them).

Recommendations

Immediate unblock (user action, one line): edit ~/.autoskillit/config.yaml line 5, replacing post_run_analysis: false with pipeline_health: false under diagnostics:. Every command recovers. (Verified: the file's quota_guard section already uses current key names — post_run_analysis is the only stale key in this file.)

Single converged recommendation — retired-key registry with load-time warn-and-remap (Candidate A):

  1. RETIRED_CONFIG_KEYS: dict[tuple[str, str], str] in src/autoskillit/config/settings.py beside _YAML_KEY_ALIASES (line 272) — append-only, mirroring the RETIRED_SKILL_NAMES idiom. Seed with all three documented config-key renames (adversarial issue audit verified these are the only three across all 14 migration notes): ("diagnostics", "post_run_analysis"): "pipeline_health"; ("quota_guard", "threshold"): "short_window_threshold" (the 0.8.38 break — strictly a split into short_window_threshold/long_window_threshold, dataclass defaults 85.0/95.0; map to the semantic successor and have the warning name both fields — deliberately lossy-but-safe for the second field); and ("features", "franchise"): "fleet" (the feat(T1): fleet deprecation shims for franchise→fleet rename #1206 shims were later removed — franchise is absent from today's FEATURE_REGISTRY, so pre-0.9.135 configs crash via _build_features_dict at settings.py:394-425; the pre-validation remap covers this path too since it rewrites the raw layer dict upstream of both validators).
  2. Remap in the per-layer load loop (_config_loader.py:143-149), before validate_layer_keys, gated on validated non-secrets layers only (should_validate=True, is_secrets_layer=False — skips the trusted defaults layer and secrets): move the retired key's value to the new key if the new key is not explicitly set in that layer (explicit new key wins), delete the old key, and emit logger.warning naming old key → new key → migration note. Implement as a single shared helper in settings.py (e.g. remap_retired_keys(data, *, is_secrets_layer) -> list[RemappedKey]) imported by both the loader and doctor's layer check, so the two provably share one implementation. Document in the registry docstring that retired key names may never be reused for future fields (the arch test enforces this by asserting old keys stay absent from _CONFIG_SCHEMA). Per-layer remap preserves dynaconf merge precedence exactly (a project-layer explicit pipeline_health still beats a user-layer remapped value). validate_layer_keys stays a pure validator; never-valid keys keep the hard ConfigSchemaError (the typo-protection design intent of Implementation Plan: Strict Schema Validation for config.yaml #454 is untouched). The warning channel is logging, not stdout — safe for MCP stdio on serve and consistent with existing _config_loader.py:41-49 idiom.
  3. Write path unchanged: write_config_layer continues to hard-fail retired keys — programmatic writers must use current spellings; a retired key reaching the write gateway is a programming error.
  4. Doctor guard: wrap cfg = load_config(...) at cli/doctor/__init__.py:85 in try/except ConfigSchemaError → synthetic failing DoctorResult, then run the config-independent checks. The diagnostic tool must never die of the disease it exists to diagnose. (Read-only; tests/arch/test_doctor_readonly.py unaffected.) Route _check_config_layers_for_secrets (cli/doctor/_doctor_config.py:27-66) through the same shared remap helper so doctor and the loader agree on retired keys — otherwise doctor would flag configs the loader accepts.
  5. Guard tests (the architectural immunity):
    • Arch invariant test (mirroring test_no_retired_skill_name_has_a_live_directory): every RETIRED_CONFIG_KEYS old key is absent from _CONFIG_SCHEMA, every new key is present in it, no old or new key is a SECRETS_ONLY_KEYS dotted path (a remap target must never mask the secrets-placement check), and every entry has a migrations/*.yaml note whose detect.key matches. This last clause is the explicit reconciliation between the two systems: the registry governs runtime behavior, the note remains the human-readable record, and the test binds them so neither can drift from the other.
    • Behavior test: a config dict with the old key loads, populates diagnostics.pipeline_health, and warns (caplog).
    • Golden prior-version fixture corpus seed under tests/config/ — the first test tier in the repo that combines "current code" with "a config written by a prior version". Add cases to the existing parametrized unrecognized-key tests (tests/config/test_config.py:246-266) distinguishing never-valid (raise) from retired (remap+warn).
  6. Process guard (AGENTS.md): add a config-key rename rule parallel to the existing Hook/Skill Rename rules — renaming a config field requires the RETIRED_CONFIG_KEYS entry and migration note in the same commit, enforced by the arch test above. (Per CLAUDE.md, adding this section requires explicit user instruction — flagged here as a recommendation, not applied.)

Killed alternatives:

  • Candidate B — deterministic ConfigMigrationAdapter with file rewrite (wired into autoskillit update/migrate): killed as the primary fix. It cannot restore startup for already-broken users (every discovery surface — serve, doctor, MCP — crashes before it could run; only update/migrate survive), it destroys YAML comments (plain-PyYAML dump_yaml_str; the user's real config carries meaningful commented-out overrides; ruamel.yaml would be a new dependency with its own round-trip caveats), it misses project configs in sibling worktrees/clones by construction, and it is ~2-3× the effort of A. External research confirms explicit file rewriting is the minority pattern; warn-and-map at load is the consensus. May be revisited later as an optional explicit autoskillit migrate --config cleanup command layered on the same registry.
  • Alias-table entry alone (_YAML_KEY_ALIASES): killed — the table is a redirect (one accepted YAML spelling per field, settings.py:337-338), so an entry would swap which spelling works rather than accept both.
  • Softening validate_layer_keys to warn on all unknown keys: killed — destroys the security/typo-protection intent of Implementation Plan: Strict Schema Validation for config.yaml #454/Rectify: Config Schema Contamination & Misplaced Secrets Immunity — PART A ONLY #476; the correct distinction is formerly-valid (map+warn) vs never-valid (reject), which the registry encodes.
  • Doc-only fix (rely on migration notes): killed — this is the status quo that failed twice before (quota_guard 0.8.38, today); notes no code reads are not a mechanism.

Breakage Analysis

  • Recommendation: retired-key registry with load-time warn-and-remap (Candidate A above), which modifies the behavior of the strict config-validation mechanism for the enumerated formerly-valid keys only.
    • Breakage surface: no hard breaks found. One consistency gap: _check_config_layers_for_secrets (cli/doctor/_doctor_config.py:27-66) independently re-validates raw on-disk layers and would report ERROR for a config the loader now accepts — it must use the same shared remap helper (trivial). Existing hard-fail tests (tests/config/test_config_schema_validation.py, test_config.py:246-266) all use never-valid keys, so no carve-outs are needed — only additions. init's ConfigSchemaError fallback (cli/app.py:224-231) stops triggering for retired-key-only configs and reads the real backend instead — a behavior improvement. tests/arch/test_feature_registry.py:563-565 and test_doctor_readonly.py unaffected. write_config_layer still cleanly rejects retired keys (correction from adversarial issue audit: its only callers are cli/_init_helpers.py:525,532 — no config_set MCP tool exists; server/tools/tools_config.py holds the unrelated configure_fleet/configure_order tools). Cosmetic drift: config_show displays new names while the file on disk still has the old key until edited — the deprecation warning covers this.
    • Prior reverts: none — git log --grep=revert on settings.py/_config_loader.py shows the strictness mechanism has never been rolled back.
    • Downstream contract violations: none. Implementation Plan: Strict Schema Validation for config.yaml #454 typo-protection holds (never-valid keys still hard-fail); Rectify: Config Schema Contamination & Misplaced Secrets Immunity — PART A ONLY #476 contamination closure holds (system-written junk keys are not in the append-only registry and still fail; no retired key overlaps SECRETS_ONLY_KEYS; secrets layers are skipped by the remap); doctor read-only AST guard holds (in-memory remap, no FS writes); MCP stdio holds (warning via logging, never stdout).
    • Additional guards required by the adversarial pass: (i) the arch invariant must assert no RETIRED_CONFIG_KEYS entry (old or new side) is a SECRETS_ONLY_KEYS dotted path, since the remap runs before validation and an unguarded remap target could otherwise bypass the secrets-placement check; (ii) the registry-vs-migration-note duplication is resolved by the arch test cross-reference (registry = runtime behavior, note = human record, test binds them) — without that clause the two systems would silently drift.
    • Risk level: LOW once conditioned — MEDIUM as originally sketched, reduced to LOW by three amendments now folded into the recommendation: the shared remap helper in _doctor_config.py's layer check, the secrets-path invariant, and the registry↔note binding test.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions