Skip to content

fix(#6678): split scaffold config into base layer and user overlay - #6809

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6678-layered-config-install
Open

fix(#6678): split scaffold config into base layer and user overlay#6809
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6678-layered-config-install

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Split BuildScaffoldFiles config generation into a layered model: config.base.yaml (scaffold-managed defaults) + config.yaml (user-owned overlay), leveraging the existing three-tier config hierarchy from ADR 0069
  • On fresh install (admin install), both files are generated; on re-onboarding, only config.base.yaml is regenerated — config.yaml is never touched, preserving custom agents, roles, runtime overrides, and other user customizations
  • Backward-compatible: existing callers (converge/drift detection, migration) are unaffected — the new behavior is opt-in via the ConfigLayout field on InstallConfig

Changes

  • internal/repos/install.go: Add ConfigLayout type with three variants (Default, Layered, Upgrade) and a minimalOverlayYAML constant for the stub overlay. Modify BuildScaffoldFiles to select file strategy based on layout, with PerRepoConfig override taking precedence
  • internal/cli/admin.go: Set ConfigLayout based on alreadyInstalled detection in both the install and dry-run paths of runPerRepoInstall
  • internal/repos/install_test.go: Add 5 tests covering layered fresh install, upgrade (no config.yaml), upgrade with runtime, default backward compat, and PerRepoConfig override
  • internal/cli/admin_test.go: Add 2 integration tests verifying fresh install generates both files and re-onboarding preserves user config

Testing

  • All TestBuildScaffoldFiles* tests pass (10 tests including 5 new)
  • All TestRunPerRepoInstall* tests pass (9 tests including 2 new)
  • All TestInstall_* tests pass
  • All TestRunGitHubSetup* tests pass (github setup path unaffected)
  • go vet clean
  • Secret scan passed
  • Patch coverage for BuildScaffoldFiles at 90.6% (above 80% threshold)

Closes #6678

Post-script verification

  • Branch is not main/master (agent/6678-layered-config-install)
  • Secret scan passed (gitleaks — 32ad401f884b3fe989d689bab02be30657dce380..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

BuildScaffoldFiles now supports a ConfigLayout field that controls
how per-repo config files are generated during admin install:

- ConfigLayoutLayered (fresh install): generates config.base.yaml
  with scaffold defaults and config.yaml as a minimal user-owned
  overlay
- ConfigLayoutUpgrade (re-onboarding): regenerates config.base.yaml
  with new defaults, leaves config.yaml untouched
- ConfigLayoutDefault (empty, backward compatible): generates a
  single config.yaml with full defaults baked in

The admin install path (runPerRepoInstall) now detects whether
the repo is already installed and sets the appropriate layout.
On re-onboarding, config.yaml is never overwritten, preserving
user customizations (custom agents, roles, runtime overrides).
New scaffold defaults from fullsend upgrades appear automatically
via config.base.yaml fallthrough (ADR 0069 Decision 2).

When PerRepoConfig is explicitly set (migration path), the layout
is ignored and config.yaml is written from the provided config,
preserving existing migration behavior.

Closes #6678
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 31, 2026 16:15
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 31, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:18 PM UTC · Completed 4:55 PM UTC

Commit: f4fe159 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.62

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/admin.go 75.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Focused 4-file bug fix with strong test coverage (50% test files), bot-authored, no protected/security/CI/dependency changes; elevated git history churn and coupling are typical for these high-traffic core files and do not materially increase risk given the narrow, well-tested scope.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [consumer-completeness] internal/repos/uninstall.go:46 — The gitlabScaffoldPaths list includes .fullsend/config.yaml but not .fullsend/config.base.yaml. After a layered install on a GitLab repo, uninstall will leave config.base.yaml behind as an orphan file.
    Remediation: Add .fullsend/config.base.yaml to the gitlabScaffoldPaths slice.

Low

  • [consumer-completeness] internal/repos/drift.go:140CheckOrphanFiles hardcodes .fullsend/config.yaml as the only config-related managed path. After a layered install, .fullsend/config.base.yaml is scaffold-managed but not tracked by orphan detection.
    Remediation: Add .fullsend/config.base.yaml to managed paths in CheckOrphanFiles.

  • [scope-gap] internal/repos/converge.go:1193 — The converge/repair path calls BuildScaffoldFiles with ConfigLayoutDefault (zero value), generating monolithic config.yaml. A converge repair on a layered-config repo could revert it to legacy layout. Pre-existing design limitation not introduced by this PR.
    Remediation: Consider filing a follow-up issue to make converge/drift paths ConfigLayout-aware.

  • [scope-gap] internal/cli/github.go:304 — The PR modifies only the admin install path. The github setup path has its own config generation logic that does not use ConfigLayout; it handles layered config only through --config presets. These are separate install flows with different design intents.
    Remediation: Confirm whether github setup without --config should also adopt the layered split on re-onboarding. If so, file a follow-up issue.

  • [code-organization] internal/cli/admin.go:862 — The configLayout computation is duplicated between the dry-run path and real-install path. This follows a pre-existing pattern in the file where both paths construct separate InstallConfig structs.

  • [stale-procedure] docs/ADRs/0033-per-repo-installation-mode.md:285 — Step 6 lists .fullsend/config.yaml as the only config file generated. After this PR, fresh installs generate both config.base.yaml and config.yaml overlay. ADRs are historical records, but a note could clarify current behavior.
    Remediation: Add a note to step 6 about layered config generation.

  • [stale-procedure] docs/guides/dev/cli-internals.md:334 — Phase 5 (Scaffold) row describes BuildScaffoldFiles() output at a high level without mentioning the ConfigLayout parameter or layered output.
    Remediation: Add a note about ConfigLayout parameter and layered output.

  • [incomplete-teardown] docs/guides/getting-started/operations.md:70 — GitHub per-repo teardown instructions do not mention .fullsend/config.base.yaml (or any .fullsend/ directory files for GitHub repos).
    Remediation: Add config.base.yaml to teardown instructions or note entire .fullsend/ directory removal.

  • [missing-documentation] docs/guides/getting-started/configuring-github.md:71 — Getting-started guide does not explain that fresh installs now generate a layered config by default. Existing text is not wrong but is incomplete.
    Remediation: Add brief note about layered config generation and link to layered-config-reference.md.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

Comment thread internal/cli/admin.go
@@ -862,6 +862,10 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
// VendorBinary, UpstreamRef, UpstreamTag. Extra fields are included to stay aligned

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] code-organization

The configLayout computation is duplicated between the dry-run path and real-install path. This follows a pre-existing pattern in the file where both paths construct separate InstallConfig structs.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/fullsend/fullsend/.fullsend/.fullsend-cache/resources/sha256/e14f2eff53cc1ed5cb9200bf5471721ee531c61b6314e8977d603eae321214cd/scripts/post-fix.sh failed: exit status 1) · Started 4:57 PM UTC · Completed 5:07 PM UTC

Commit: f4fe159 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.29

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/fullsend/actions/runs/33416728975

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Agent PR ready for human review risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-onboarding with fullsend v0.37 removes custom agent configuration

1 participant