Skip to content

Fix multi-level config merging (default → org → repo) - #538

Merged
timja merged 4 commits into
mainfrom
copilot/fix-configuration-merging
Jul 25, 2026
Merged

Fix multi-level config merging (default → org → repo)#538
timja merged 4 commits into
mainfrom
copilot/fix-configuration-merging

Conversation

CopilotAI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

When both an org-level (.github repo) and repo-level comment-ops.yml exist, the org config was silently skipped — only default → repo was applied instead of the intended default → org → repo.

Root Cause

The @probot/octokit-plugin-config plugin only falls back to the org's .github repo when the requesting repo has no config file. So a single octokit.config.get call can never return both configs simultaneously.

Changes

  • app/router.js: Split config loading into two explicit calls — one for {owner}/.github (org) and one for the actual repo — then merge in the correct precedence order. Extracted into a mergeConfigs helper that correctly handles:
    • Missing configs at any level
    • _extends chains (the plugin returns multiple files per level; only the first file's existence is checked to avoid the plugin's org-fallthrough being double-counted)
// Before: single call — org config skipped when repo config existsconst{ config }=awaitoctokit.config.get({
owner,repo: repoName,path: configPath,defaults: (configs)=>deepmerge.all([defaultConfig, ...configs]),});// After: explicit fetch of both levels, merged in correct orderconst{files: orgFiles}=awaitoctokit.config.get({ owner,repo: ".github",path: configPath});const{files: repoFiles}=awaitoctokit.config.get({ owner,repo: repoName,path: configPath});constconfig=mergeConfigs(defaultConfig,orgFiles,repoFiles);// → deepmerge.all([default, ...orgConfigs, ...repoConfigs])
  • app/router.test.js: Unit tests for mergeConfigs covering all combinations: no config, org-only, repo-only, both (the broken case), precedence ordering, and _extends chains.

CopilotAI changed the title [WIP] Fix multiple levels of configuration mergingFix multi-level config merging (default → org → repo)Jul 25, 2026
CopilotAI requested a review from timjaJuly 25, 2026 08:03
@timja

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

@timjatimja added the bug Something isn't working label Jul 25, 2026
@timja
timja marked this pull request as ready for review July 25, 2026 10:29
@timja
timja enabled auto-merge (squash) July 25, 2026 10:29
auto-merge was automatically disabled July 25, 2026 10:31

Head branch was pushed to by a user without write access

CopilotAI commented Jul 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 826cab9. The conflict was in app/router.js where the validateConfig call (added in #539) was integrated into the new multi-call config loading approach — validateConfig now runs after mergeConfigs produces the final merged config. All 90 tests pass.

@timja
timja merged commit 63c8860 into mainJul 25, 2026
4 checks passed
@timja
timja deleted the copilot/fix-configuration-merging branch July 25, 2026 21:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple levels of configuration don't seem to be working

2 participants

@timja