[PR #5047] [MERGED] fix(setup): treat empty COPILOT_HOME as unset #6758

Closed
opened 2026-11-01 21:42:50 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5047
Author: @cpruijsen
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: fix/setup-empty-home-override


📝 Commits (1)

  • fd52ab3 fix(setup): treat empty COPILOT_HOME as unset

📊 Changes

2 files changed (+7 additions, -1 deletions)

View changed files

📝 src/application/SupportedClients.ts (+1 -1)
📝 tests/boundary/cli/clientProfileLifecycle.test.ts (+6 -0)

📄 Description

Summary

With COPILOT_HOME="", rea setup --client copilot_cli writes the GitHub Copilot CLI registration to ./mcp-config.json in the current directory instead of ~/.copilot/mcp-config.json. copilotDirectory used ??, so the empty string won over the default. The sibling client homes (CODEX_HOME, KIMI_CODE_HOME, GROK_HOME, XDG_CONFIG_HOME) already use ||, and #2686 made the same change for DSH_HOME.

No issue is linked. This is the last *_HOME resolver in SupportedClients.ts that still used ??.

Problem and expected behavior

Trigger: COPILOT_HOME= rea setup --client copilot_cli --dry-run --json.

  • Base: the planned configure_client:copilot_cli target is "mcp-config.json", a relative path that resolves against the caller's working directory.
  • Expected: ~/.copilot/mcp-config.json, the same as with COPILOT_HOME unset.

Change and scope

  • src/application/SupportedClients.ts: env.COPILOT_HOME ?? join(home, ".copilot") becomes env.COPILOT_HOME || join(home, ".copilot").
  • What I chose and the alternative: plain || matches every sibling POSIX home resolver, and the readable COPILOT_HOME lookup in the Copilot CLI binary is itself a truthiness check, so || keeps REA aligned with the client. The alternative, the trim-based shape #2686 used for DSH_HOME, was rejected here: it would treat " " as unset while Copilot CLI itself would honor it, and DSH_HOME trimmed only because the released DSH resolver does.
  • No other resolver changes. The Windows APPDATA fallbacks also use ??, but an empty APPDATA is not a normal Windows state, so I left them alone.

Scope: an ordinary fix inside the existing setup boundary. A maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: client location resolution in SupportedClients.ts, before planning.

  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows #2686).

  • CLI and MCP/tool-catalog contract: none. Only the resolved Copilot CLI path changes, and only when COPILOT_HOME is empty.

  • Provider, bridge, target-format, or platform compatibility: GitHub Copilot CLI on every platform.

  • Evidence, artifact, provenance, or reconstruction contract: none.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Change process memory, registers, files, or configuration. (Setup writes the registration to the correct file.)

Evidence and regression coverage

  • Tests added or updated: tests/boundary/cli/clientProfileLifecycle.test.ts gets a copilot_cli row with COPILOT_HOME: "" in "selected client profiles", next to the existing empty CODEX_HOME and GROK_HOME rows. The row runs dry-run setup, setup, doctor and uninstall against .copilot/mcp-config.json.
  • Base reproduction (isolated worktree at the merge-base with only the test file applied, dist/ rebuilt from base source by the focused-test runner): "selected client profiles lifecycle follows the selected roots" fails at the dry-run planned-actions assertion with target: "mcp-config.json" where <home>/.copilot/mcp-config.json is expected; the other 4 tests in the file pass. With the source change, all 5 tests pass.
  • Copilot CLI: in the Copilot CLI 1.0.98 darwin-arm64 binary, the readable COPILOT_HOME lookup is a truthiness check (process.env.COPILOT_HOME&&...), so it also skips an empty value. The binary's other lookups are not readable as text.
  • Remaining proof gaps: I did not run Copilot CLI against the written file.

Validation performed

  • npm run test:focused -- tests/boundary/cli/clientProfileLifecycle.test.ts: 5 passed (38.9s), including the new copilot_cli row.
  • Counterfactual in an isolated worktree at the merge-base with only the test change: 1 failed, 4 passed; the failure is the planned configure_client:copilot_cli target "mcp-config.json".
  • npx turbo run typecheck lint --force: 2 successful, 0 cached.
  • npm run verify:test-discovery: passed (1580 test files discovered exactly once).
  • Platform: macOS (arm64), Node 26.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. An empty COPILOT_HOME now means the default location, which is what Copilot CLI uses.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tests ran on macOS only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the change stops setup from writing a config file into an arbitrary working directory.

Author readiness

Human owner: @cpruijsen

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5047 **Author:** [@cpruijsen](https://github.com/cpruijsen) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/setup-empty-home-override` --- ### 📝 Commits (1) - [`fd52ab3`](https://github.com/morluto/rea/commit/fd52ab39778e605248a7248d0d80bbac079c6f61) fix(setup): treat empty COPILOT_HOME as unset ### 📊 Changes **2 files changed** (+7 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/application/SupportedClients.ts` (+1 -1) 📝 `tests/boundary/cli/clientProfileLifecycle.test.ts` (+6 -0) </details> ### 📄 Description ## Summary With `COPILOT_HOME=""`, `rea setup --client copilot_cli` writes the GitHub Copilot CLI registration to `./mcp-config.json` in the current directory instead of `~/.copilot/mcp-config.json`. `copilotDirectory` used `??`, so the empty string won over the default. The sibling client homes (`CODEX_HOME`, `KIMI_CODE_HOME`, `GROK_HOME`, `XDG_CONFIG_HOME`) already use `||`, and #2686 made the same change for `DSH_HOME`. No issue is linked. This is the last `*_HOME` resolver in `SupportedClients.ts` that still used `??`. ## Problem and expected behavior Trigger: `COPILOT_HOME= rea setup --client copilot_cli --dry-run --json`. - Base: the planned `configure_client:copilot_cli` target is `"mcp-config.json"`, a relative path that resolves against the caller's working directory. - Expected: `~/.copilot/mcp-config.json`, the same as with `COPILOT_HOME` unset. ## Change and scope - `src/application/SupportedClients.ts`: `env.COPILOT_HOME ?? join(home, ".copilot")` becomes `env.COPILOT_HOME || join(home, ".copilot")`. - What I chose and the alternative: plain `||` matches every sibling POSIX home resolver, and the readable `COPILOT_HOME` lookup in the Copilot CLI binary is itself a truthiness check, so `||` keeps REA aligned with the client. The alternative, the trim-based shape #2686 used for `DSH_HOME`, was rejected here: it would treat `" "` as unset while Copilot CLI itself would honor it, and `DSH_HOME` trimmed only because the released DSH resolver does. - No other resolver changes. The Windows `APPDATA` fallbacks also use `??`, but an empty `APPDATA` is not a normal Windows state, so I left them alone. Scope: an ordinary fix inside the existing setup boundary. A maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: client location resolution in `SupportedClients.ts`, before planning. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows #2686). - CLI and MCP/tool-catalog contract: none. Only the resolved Copilot CLI path changes, and only when `COPILOT_HOME` is empty. - Provider, bridge, target-format, or platform compatibility: GitHub Copilot CLI on every platform. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Change process memory, registers, files, or configuration. (Setup writes the registration to the correct file.) ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/clientProfileLifecycle.test.ts` gets a `copilot_cli` row with `COPILOT_HOME: ""` in "selected client profiles", next to the existing empty `CODEX_HOME` and `GROK_HOME` rows. The row runs dry-run setup, setup, doctor and uninstall against `.copilot/mcp-config.json`. - Base reproduction (isolated worktree at the merge-base with only the test file applied, `dist/` rebuilt from base source by the focused-test runner): "selected client profiles lifecycle follows the selected roots" fails at the dry-run planned-actions assertion with `target: "mcp-config.json"` where `<home>/.copilot/mcp-config.json` is expected; the other 4 tests in the file pass. With the source change, all 5 tests pass. - Copilot CLI: in the Copilot CLI 1.0.98 darwin-arm64 binary, the readable `COPILOT_HOME` lookup is a truthiness check (`process.env.COPILOT_HOME&&...`), so it also skips an empty value. The binary's other lookups are not readable as text. - Remaining proof gaps: I did not run Copilot CLI against the written file. ## Validation performed - `npm run test:focused -- tests/boundary/cli/clientProfileLifecycle.test.ts`: 5 passed (38.9s), including the new `copilot_cli` row. - Counterfactual in an isolated worktree at the merge-base with only the test change: 1 failed, 4 passed; the failure is the planned `configure_client:copilot_cli` target `"mcp-config.json"`. - `npx turbo run typecheck lint --force`: 2 successful, 0 cached. - `npm run verify:test-discovery`: passed (1580 test files discovered exactly once). - Platform: macOS (arm64), Node 26. ## Compatibility, safety, and release - Breaking changes or migration steps: none. An empty `COPILOT_HOME` now means the default location, which is what Copilot CLI uses. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tests ran on macOS only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the change stops setup from writing a config file into an arbitrary working directory. ## Author readiness Human owner: @cpruijsen - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:42:50 +00:00
bsakaguchi changed title from [PR #5047] [MERGED] fix(setup): treat empty COPILOT_HOME as unset to [PR #5047] fix(setup): treat empty COPILOT_HOME as unset 2026-11-03 13:34:10 +00:00
bsakaguchi changed title from [PR #5047] fix(setup): treat empty COPILOT_HOME as unset to [PR #5047] [MERGED] fix(setup): treat empty COPILOT_HOME as unset 2026-11-03 14:35:57 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6758
No description provided.