Remove MATRIX_* var auth from software-factory and use boxel-cli profile instead#4426
Remove MATRIX_* var auth from software-factory and use boxel-cli profile instead#4426jurgenwerk wants to merge 9 commits intomainfrom
Conversation
Replace the dual auth path (env vars vs Boxel profile) with profile-only auth. The factory now relies solely on the active Boxel profile configured via `boxel profile add`. This eliminates the confusing requirement to set MATRIX_USERNAME even when a profile is configured. Also makes --realm-server-url fall back to the active profile's realmServerUrl instead of hardcoding localhost:4201, so staging and production environments work without extra flags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rofile When the target realm URL origin doesn't match the realm server URL (derived from the active profile), fail early with a clear error explaining the mismatch and suggesting `boxel profile switch`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tell the user to run `boxel profile add` instead of the generic "configure an active Boxel profile" message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve conflicts with main's BoxelCLIClient integration: - Use BoxelCLIClient for profile access instead of local getActiveProfile() - Export resetProfileManager from boxel-cli/api for test support - Update test helper to write to real profiles.json and reset singleton Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Removes the MATRIX_* env var–based authentication path from software-factory and standardizes authentication/target-realm resolution on the active Boxel CLI profile (boxel profile add), updating code paths, tests, docs, and smoke scripts accordingly.
Changes:
- Switch realm auth + factory entrypoint/auth wiring to rely on Boxel CLI profiles (and update error/usage messaging).
- Update target realm resolution to derive owner + default realm server URL from the active profile, with additional mismatch validation.
- Refactor tests/docs/smoke scripts to create/read Boxel profiles instead of MATRIX_* env vars.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/software-factory/src/realm-auth.ts | Drops env-profile fallback; uses active Boxel profile only. |
| packages/software-factory/src/boxel.ts | Removes env fallback in getActiveProfile() and improves guidance message. |
| packages/software-factory/src/factory-target-realm.ts | Resolves owner/server from profile and adds target/server origin mismatch error. |
| packages/software-factory/src/factory-issue-loop-wiring.ts | Removes env fallback for auth resolution; improves login failure guidance. |
| packages/software-factory/src/factory-entrypoint.ts | Updates CLI usage text to reflect profile-only auth. |
| packages/software-factory/tests/realm-auth.test.ts | Updates tests to validate active-profile auth path. |
| packages/software-factory/tests/helpers/test-profile.ts | Adds helper to install a test Boxel profile (currently writes to real homedir). |
| packages/software-factory/tests/factory-target-realm.test.ts | Updates unit tests to depend on Boxel profile; adds new failure-mode tests. |
| packages/software-factory/tests/factory-entrypoint.test.ts | Updates entrypoint tests to depend on Boxel profile. |
| packages/software-factory/tests/factory-target-realm.spec.ts | Updates E2E test to run with a temp HOME containing profiles.json. |
| packages/software-factory/tests/factory-entrypoint.integration.test.ts | Updates integration tests to run with a temp HOME containing profiles.json. |
| packages/software-factory/tests/helpers/browser-auth.ts | Removes MATRIX_URL fallback for browser auth defaults. |
| packages/software-factory/scripts/smoke-tests/smoke-test-realm.ts | Requires active profile; removes env defaulting for auth chain. |
| packages/software-factory/scripts/smoke-tests/smoke-test-factory-scenarios.ts | Requires active profile and derives username/server from it. |
| packages/software-factory/docs/testing-strategy.md | Updates docs to reference active Boxel profile instead of MATRIX_USERNAME. |
| packages/software-factory/docs/phase-1-plan.md | Updates plan language to reference active Boxel profile requirement. |
| packages/software-factory/README.md | Updates setup instructions to use boxel profile add. |
| packages/boxel-cli/api.ts | Exports resetProfileManager for test usage. |
Comments suppressed due to low confidence (1)
packages/software-factory/scripts/smoke-tests/smoke-test-realm.ts:168
- When --target-realm-url is omitted, this defaults to http://localhost:4201/... even if the active Boxel profile points at a different realm server. That can lead to logging into one environment (profile) while trying to create/write to another (localhost), causing confusing failures or writing to the wrong server. Consider defaulting the target realm URL from active.realmServerUrl (and/or validating targetRealmUrl origin matches active.realmServerUrl origin before proceeding).
if (!targetRealmUrl) {
let username = active.matrixId.replace(/^@/, '').replace(/:.*$/, '');
targetRealmUrl = `http://localhost:4201/${username}/smoke-test-realm/`;
log.info(
`No --target-realm-url specified, using default: ${targetRealmUrl}\n`,
);
}
if (!targetRealmUrl.endsWith('/')) {
targetRealmUrl += '/';
}
let testResultsModuleUrl = new URL(
'software-factory/test-results',
new URL(targetRealmUrl).origin + '/',
).href;
let realmServerUrl = new URL(targetRealmUrl).origin + '/';
let realmPath = new URL(targetRealmUrl).pathname
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Installs a fake Boxel CLI profile into the real ~/.boxel-cli/profiles.json. | ||
| * Backs up any existing file and resets the ProfileManager singleton so | ||
| * BoxelCLIClient picks up the test profile. | ||
| * | ||
| * Returns a cleanup function that restores the original file and resets again. | ||
| */ | ||
| export function installTestProfile(options: TestProfileOptions): () => void { | ||
| let configDir = join(homedir(), '.boxel-cli'); | ||
| let profilesFile = join(configDir, 'profiles.json'); |
There was a problem hiding this comment.
installTestProfile writes directly to the current user's ~/.boxel-cli/profiles.json (and may overwrite real credentials). This makes the unit tests non-hermetic and can leave a modified profiles.json (and even create ~/.boxel-cli) on developer machines/CI runners. Refactor the helper/tests to use an isolated temp config (e.g., create a temp HOME/config dir for the test process, or inject a ProfileManager/BoxelCLIClient pointed at a temp dir) rather than touching the real home directory.
| hooks.afterEach(function () { | ||
| restoreEnv('MATRIX_USERNAME', originalMatrixUsername); | ||
| cleanupProfile?.(); | ||
| cleanupProfile = undefined; | ||
| }); | ||
|
|
||
| test('resolveFactoryTargetRealm uses MATRIX_USERNAME and explicit target URL', function (assert) { | ||
| process.env.MATRIX_USERNAME = 'hassan'; | ||
| function setupHassanProfile() { | ||
| cleanupProfile = installTestProfile({ | ||
| username: 'hassan', |
There was a problem hiding this comment.
These tests call installTestProfile(), which currently writes to the real ~/.boxel-cli/profiles.json. That makes the test suite non-hermetic and risks overwriting a developer/CI Boxel profile. Prefer setting up an isolated profile location for the test (temp HOME/config dir) or injecting a BoxelCLIClient/ProfileManager that points at test state.
| error.message.includes('does not match the realm server') && | ||
| error.message.includes('boxel profile switch'), | ||
| ); | ||
| }); | ||
|
|
||
| test('resolveFactoryTargetRealm rejects when no active profile is configured', function (assert) { | ||
| let profilesFile = join(homedir(), '.boxel-cli', 'profiles.json'); | ||
| let backup = existsSync(profilesFile) | ||
| ? readFileSync(profilesFile, 'utf8') | ||
| : undefined; | ||
|
|
||
| try { | ||
| writeFileSync( | ||
| profilesFile, | ||
| JSON.stringify({ profiles: {}, activeProfile: null }), | ||
| ); | ||
| resetProfileManager(); |
There was a problem hiding this comment.
This test manipulates ~/.boxel-cli/profiles.json under os.homedir() (real home), and it also writes the file without ensuring the ~/.boxel-cli directory exists (writeFileSync will throw ENOENT on a fresh environment). Use an isolated temp HOME/config dir for the test and create the directory before writing, so the test is reliable and doesn't mutate real user state.
| function setupHassanProfile() { | ||
| cleanupProfile = installTestProfile({ | ||
| username: 'hassan', | ||
| matrixUrl: 'https://matrix.example.test/', | ||
| realmServerUrl: 'https://realms.example.test/', | ||
| password: 'secret', | ||
| }); | ||
| } |
There was a problem hiding this comment.
setupHassanProfile() uses installTestProfile(), which currently writes to the real ~/.boxel-cli/profiles.json. That makes these unit tests non-hermetic and can overwrite a developer/CI profile. Please switch to a temp HOME/config dir for the test run (or inject a BoxelCLIClient/ProfileManager configured for a temp directory) so no real user files are touched.
Address Copilot review: installTestProfile was writing to the real ~/.boxel-cli/profiles.json. Now uses setProfileManager() to point the singleton at a temp directory, so no real user files are touched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return () => { | ||
| resetProfileManager(); | ||
| }; |
There was a problem hiding this comment.
installTestProfile() creates a temp config directory but the returned cleanup function only resets the singleton; it never removes the temp directory. This can leak many /tmp entries across repeated test runs—consider deleting tempConfigDir (e.g., rmSync with recursive/force) as part of cleanup.
| let tempConfigDir = mkdtempSync(join(tmpdir(), 'boxel-test-empty-')); | ||
| writeFileSync( | ||
| join(tempConfigDir, 'profiles.json'), | ||
| JSON.stringify({ profiles: {}, activeProfile: null }), | ||
| ); |
There was a problem hiding this comment.
This test creates a temporary profiles config dir but never removes it (cleanup only resets the ProfileManager singleton). Please delete tempConfigDir in the cleanup path to avoid leaking temp directories during repeated runs.
| let tempHome = mkdtempSync(join(tmpdir(), 'boxel-test-')); | ||
| let boxelCliDir = join(tempHome, '.boxel-cli'); | ||
| mkdirSync(boxelCliDir, { recursive: true }); | ||
|
|
There was a problem hiding this comment.
createTempProfileHome() allocates a temp HOME directory but the test never cleans it up. Consider returning a cleanup function (or returning { home, cleanup }) and rmSync-ing the temp directory in the existing finally block to prevent temp directory leaks.
| let tempHome = createTempProfileHome({ | ||
| username: 'hassan', | ||
| matrixUrl: `${origin}/`, | ||
| realmServerUrl: `${origin}/`, | ||
| password: 'secret', |
There was a problem hiding this comment.
tempHome is created for the child process HOME but never removed in the finally block. This can accumulate /tmp directories over time—consider rmSync(tempHome, { recursive: true, force: true }) during cleanup (and import rmSync).
|
|
||
| - require `MATRIX_USERNAME` so the target realm owner is explicit before bootstrap starts | ||
| - require an active Boxel profile so the target realm owner is explicit before bootstrap starts | ||
| - infer the target realm server URL from the target realm URL by default, but allow an explicit override when the realm server lives under a subdirectory and the URL shape is ambiguous |
There was a problem hiding this comment.
The plan still says the realm server URL is inferred from --target-realm-url by default, but the current implementation/usage text says it is never inferred and instead comes from --realm-server-url or the active Boxel profile. Please update this bullet to match current behavior.
| - infer the target realm server URL from the target realm URL by default, but allow an explicit override when the realm server lives under a subdirectory and the URL shape is ambiguous | |
| - use `realm-server-url` when explicitly provided; otherwise take the realm server URL from the active Boxel profile rather than inferring it from the target realm URL |
- installTestProfile cleanup now removes the temp config dir - Integration and spec tests clean up their temp HOME dirs in finally blocks - Update phase-1-plan.md to reflect profile-based realm server URL resolution Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
boxel profile add)