Skip to content

feat: Config Upgrade Command & fab_version Migration#476

Merged
sahil-noon merged 5 commits into
mainfrom
260708-j0qm-config-upgrade-migration
Jul 8, 2026
Merged

feat: Config Upgrade Command & fab_version Migration#476
sahil-noon merged 5 commits into
mainfrom
260708-j0qm-config-upgrade-migration

Conversation

@sahil-noon

Copy link
Copy Markdown
Collaborator

Meta

Change ID Type Confidence Plan Review
j0qm feat 4.6/5.0 30/30 tasks, 25/25 acceptance ✓ ✓ 2 cycles
Impact +/− Net
raw +3984 / −639 +3345
true +2899 / −535 +2364
└ impl +1527 / −248 +1279
└ tests +1372 / −287 +1085

excludes fab/, docs/ · generated by fab-kit v2.13.6

Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr

Summary

config.yaml upgrades are not mechanical today — setFabVersion mashes user comments/order on every fab init/fab upgrade-repo, and every field rename/removal needs a hand-written migration. This change makes fab config upgrade the single, mechanical writer of config.yaml: live fields are kept verbatim, advertise-flagged fields regenerate inside a managed fence, unknown fields are parked (never deleted), and renames carry via renamed_from.

Changes

  • New subcommand: fab config upgrade (fab-go)
  • fab_version moves out of config.yaml → fab/.fab-version; setFabVersion deleted
  • fab upgrade-repo auto-runs the upgrader (fab-kit)
  • Scaffold config.yaml deleted — init generates from the registry
  • Migration file (constitution: user-data restructure)
  • HOME hermeticity for the fab-go test suite (inherited follow-up)
  • Documentation obligations

sahil87 added 2 commits July 8, 2026 16:36
Makes fab config upgrade the sole writer of config.yaml: live fields
are kept verbatim, advertise-flagged fields are regenerated inside a
managed fence, and unknown fields are parked in a comment block rather
than silently dropped. Moves fab_version out of config.yaml into a new
fab/.fab-version sibling file (deleting setFabVersion, the source of
the recurring comment-mashing bug), wires fab upgrade-repo to auto-run
the upgrader, replaces the hand-maintained scaffold config.yaml with
registry-driven init generation, and adds suite-wide HOME isolation
for the config test suite.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the “config-upgrade” effort by making fab config upgrade the single mechanical writer of fab/project/config.yaml, relocating the pinned engine version from config.yaml to fab/.fab-version, and updating fab-kit’s init/upgrade flows, tests, and documentation to match.

Changes:

  • Adds fab config upgrade and fab config init --project (registry-driven project config generation + managed fence reconciliation).
  • Moves the version pin from fab_version in config.yaml to fab/.fab-version, adds compat fallbacks, and ships the 2.14.0-to-2.15.0 migration doc.
  • Updates fab-kit init / upgrade-repo to stamp .fab-version, generate config via pinned fab-go, and auto-run fab config upgrade (fail-open), plus broad doc/test updates.

Reviewed changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/kit/VERSION Bumps kit version to 2.15.0.
src/kit/skills/fab-setup.md Updates setup skill behavior/docs for .fab-version + registry-generated config.
src/kit/skills/_cli-fab.md Updates CLI docs for new config commands and version-pin relocation.
src/kit/scaffold/fab/project/config.yaml Deletes retired project config scaffold template.
src/kit/migrations/2.14.0-to-2.15.0.md Adds migration spec for moving fab_versionfab/.fab-version.
src/go/fab/internal/spawn/main_test.go Isolates $HOME for spawn tests (hermeticity).
src/go/fab/internal/configupgrade/golden_test.go Adds full-document goldens for config fence rendering.
src/go/fab/internal/configupgrade/freeze_test.go Adds idempotence/freeze + entrypoint tests for upgrader behavior.
src/go/fab/internal/configupgrade/configupgrade_test.go Adds behavioral tests over shipped registry for upgrader contracts.
src/go/fab/internal/configscope/configscope.go Removes fab_version from scope table; adds compat-window notes.
src/go/fab/internal/configscope/configscope_test.go Updates scope tests to reflect fab_version no longer scoped.
src/go/fab/internal/configref/configref.go Adds InitSeed, deletes fab_version field row, adds InitSeedKeys().
src/go/fab/internal/config/config.go Overlays .fab-version into loaded config; strips system fab_version.
src/go/fab/internal/config/config_test.go Adds tests for .fab-version precedence and system-layer stripping.
src/go/fab/cmd/fab/main_test.go Isolates $HOME for cmd/fab tests (hermeticity).
src/go/fab/cmd/fab/config.go Adds fab config upgrade and fab config init --project; reuses shared comment-out helper.
src/go/fab/cmd/fab/config_upgrade_test.go Adds end-to-end command tests for config upgrade and init --project.
src/go/fab/cmd/fab/config_test.go Removes scaffold-based reference coverage tests; adds relocated fab_version and init-seed guards.
src/go/fab-kit/internal/upgrade.go Stamps fab/.fab-version post-sync and auto-runs pinned fab-go config upgrade (fail-open).
src/go/fab-kit/internal/upgrade_test.go Updates upgrade tests for .fab-version + fail-open config upgrade run.
src/go/fab-kit/internal/init.go Stamps .fab-version, generates project config via pinned fab-go with stub fallback, adds seed detection.
src/go/fab-kit/internal/init_test.go Replaces setFabVersion tests with .fab-version/generation/seed-detection tests.
src/go/fab-kit/internal/config.go Resolves pinned version from .fab-version first, config.yaml fallback second.
src/go/fab-kit/internal/config_test.go Adds tests for .fab-version precedence and fallback behavior.
fab/changes/260708-j0qm-config-upgrade-migration/.status.yaml Adds change tracking status file for this effort.
fab/changes/260708-j0qm-config-upgrade-migration/.history.jsonl Adds change tracking history file for this effort.
docs/specs/skills/SPEC-fab-setup.md Updates spec to reflect registry generation and .fab-version pin.
docs/specs/skills/SPEC-_cli-fab.md Updates CLI spec for new config commands and semantics.
docs/specs/index.md Updates specs index to reflect expanded config system metadata/behavior.
docs/specs/config.md Updates config spec for Change 3 landing: upgrader, fence, init-seed, .fab-version.
docs/specs/architecture.md Updates architecture docs for .fab-version and managed-fence config.
docs/memory/distribution/setup.md Updates memory doc for new init/config generation flow and version relocation.
docs/memory/distribution/migrations.md Documents the new 2.14.0→2.15.0 migration and rationale.
docs/memory/distribution/index.md Updates distribution index summaries for j0qm changes.
docs/memory/distribution/distribution.md Updates distribution doc for .fab-version, config generation, and auto-upgrade step.
docs/memory/_shared/index.md Updates shared index to include config-upgrade/.fab-version changes.
docs/memory/_shared/configuration.md Updates configuration memory doc for upgrader/init-project and .fab-version relocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +60 to +65
// fab_version is intentionally ABSENT (260708-j0qm): it moved out of config.yaml
// to the plain-text sibling fab/.fab-version and is no longer a config-file key,
// so it carries no scope. A stale `fab_version:` in a not-yet-migrated PROJECT file
// is an unknown top-level key — ignored silently by the loader, exactly like any
// other unrecognized key, until the 2.14.0-to-2.15.0 migration removes it. In the
// SYSTEM file it is handled differently: fab_version is repo-scoped state, so the

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — corrected the comment: a stale fab_version: in a not-yet-migrated PROJECT file is not ignored, it is still parsed by internal/config (Config.FabVersion survives one compat window) and read as a legacy fallback when fab/.fab-version is absent, until the migration removes it. (ab209df)

Comment thread docs/specs/config.md
Comment on lines +116 to +123
Fields the decision-6 taxonomy does not enumerate (`stage_hooks`, `branch_prefix`) default to `project`
— the conservative choice, since system-visibility is opt-in per the same rationale. (`fab_version` was
machine-managed and, as of [Change 3 — landed], left `config.yaml` entirely for the plain-text sibling
`fab/.fab-version`; it is no longer a config-file key and carries no scope — a stale `fab_version:` in a
not-yet-migrated PROJECT file is an unknown key, ignored silently by the loader, while a stale
`fab_version:` in the SYSTEM file is stripped as a named compat-window exception so a machine-global value
never bleeds into a repo's resolved version.) Scope was metadata-only in
Change 1; **as of Change 2 it is enforced**: the cascade

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — reworded the spec: fab_version is no longer a scoped/registry key, but it is still parsed and read as a legacy fallback when fab/.fab-version is absent (one compat window, until the migration removes it) rather than ignored. (ab209df)

Comment on lines +126 to +129
if _, err := os.Stat(configPath); err == nil {
// Already present (e.g. re-run over an existing repo) — leave it untouched.
return nil
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped — the if _, err := os.Stat(x); err == nil existence-guard idiom is the established pattern across this codebase (skills.go, sync.go, scaffold.go, config.go, and init.go:152/187/225). Changing only these two call sites would introduce a distinguish-non-NotExist idiom inconsistent with the rest. Impact is also minimal: on a non-NotExist stat error the very next step writes to the same path (config init / the stub write), so the real error surfaces immediately via cannot write stub config.yaml: %w rather than being masked. Out of scope for this PR; a consistent sweep of the whole idiom would be the right vehicle if we want to tighten it.

Comment on lines +289 to 300
func writeStubConfig(configPath string, seed projectSeed) error {
if _, err := os.Stat(configPath); err == nil {
return nil
}
if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil {
return fmt.Errorf("cannot create fab/project/ directory: %w", err)
}
if err := os.WriteFile(configPath, []byte(renderStubConfig(seed)), 0644); err != nil {
return fmt.Errorf("cannot write stub config.yaml: %w", err)
}
return nil
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped — same rationale as the generateProjectConfig stat guard above: this is the codebase-wide existence-check idiom, and writeStubConfig is only reached via the caller's fail-open path (the comment notes the check is defensive). A non-NotExist stat error falls through to os.WriteFile on the same path, which returns a clear cannot write stub config.yaml: %w, so the filesystem problem is not masked. Tightening this belongs in a consistent sweep of the whole idiom, not two isolated sites in this PR.

@sahil-noon sahil-noon marked this pull request as ready for review July 8, 2026 14:35
@sahil-noon sahil-noon merged commit bd38684 into main Jul 8, 2026
3 checks passed
@sahil-noon sahil-noon deleted the 260708-j0qm-config-upgrade-migration branch July 8, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants