Skip to content

Replace pullRealmFiles with BoxelCLIClient.pull() #4422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jurgenwerk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace pullRealmFiles with BoxelCLIClient.pull() #4422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/boxel-cli/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ export {
BoxelCLIClient,
type CreateRealmOptions,
type CreateRealmResult,
type PullOptions,
type PullResult,
} from './src/lib/boxel-cli-client';
50 changes: 47 additions & 3 deletions packages/boxel-cli/src/commands/realm/pull.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface PullOptions extends SyncOptions {

class RealmPuller extends RealmSyncBase {
hasError = false;
downloadedFiles: string[] = [];

constructor(
private pullOptions: PullOptions,
Expand Down Expand Up @@ -106,7 +107,7 @@ class RealmPuller extends RealmSyncBase {
}),
),
);
const downloadedFiles = downloadResults.filter(
this.downloadedFiles = downloadResults.filter(
(f): f is string => f !== null,
);

Expand Down Expand Up @@ -138,10 +139,10 @@ class RealmPuller extends RealmSyncBase {

if (
!this.options.dryRun &&
downloadedFiles.length + deletedFiles.length > 0
this.downloadedFiles.length + deletedFiles.length > 0
) {
const pullChanges: CheckpointChange[] = [
...downloadedFiles.map((f) => ({
...this.downloadedFiles.map((f) => ({
file: f,
status: 'modified' as const,
})),
Expand Down Expand Up @@ -232,3 +233,46 @@ export async function pullCommand(
process.exit(1);
}
}

export async function pull(
Copy link
Copy Markdown
Contributor

@FadhlanR FadhlanR Apr 16, 2026

Choose a reason for hiding this comment

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

I think we can drop the pullCommand function and use this function in registerPullCommand and move the console.log and process.exit that previously handled in pullCommand to registerPullCommand.

realmUrl: string,
localDir: string,
options: PullCommandOptions,
): Promise<{ files: string[]; error?: string }> {
let pm = options.profileManager ?? getProfileManager();
let active = pm.getActiveProfile();
if (!active) {
return {
files: [],
error: 'No active profile. Run `boxel profile add` to create one.',
};
Comment on lines +237 to +248
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

New programmatic pull() is introduced to return { files, error? } instead of calling process.exit(), but existing integration coverage in this repo appears to exercise pullCommand() only. Add a test that calls pull() directly (and/or BoxelCLIClient.pull()) to verify it returns an error string on failure conditions (e.g., no active profile / partial download errors) and does not terminate the process.

Copilot uses AI. Check for mistakes.
}

try {
const puller = new RealmPuller(
{
realmUrl,
localDir,
deleteLocal: options.delete,
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

pull() accepts PullCommandOptions (including dryRun), but it does not pass options.dryRun into RealmPuller. As written, a caller providing { dryRun: true } would still perform real filesystem writes/checkpoints.

Pass dryRun: options.dryRun when constructing RealmPuller (or remove dryRun from this API if it’s intentionally unsupported).

Suggested change
deleteLocal: options.delete,
deleteLocal: options.delete,
dryRun: options.dryRun,

Copilot uses AI. Check for mistakes.
},
Comment on lines +253 to +257
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 16, 2026

Choose a reason for hiding this comment

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

P2 Badge Respect dry-run option in programmatic pull helper

The new exported pull() API accepts PullCommandOptions (which includes dryRun), but it does not pass dryRun into RealmPuller. As a result, pull(..., { dryRun: true }) will still write files and perform real mutations, which violates the expected no-side-effects behavior used by the CLI path.

Useful? React with 👍 / 👎.

pm,
);

await puller.sync();

if (puller.hasError) {
return {
files: puller.downloadedFiles.sort(),
error:
'Pull completed with errors. Some files may not have been downloaded.',
};
}

return { files: puller.downloadedFiles.sort() };
} catch (error) {
return {
files: [],
error: `Pull failed: ${error instanceof Error ? error.message : String(error)}`,
};
}
}
23 changes: 23 additions & 0 deletions packages/boxel-cli/src/lib/boxel-cli-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createRealm as coreCreateRealm } from '../commands/realm/create';
import { pull as realmPull } from '../commands/realm/pull';
import { getProfileManager, type ProfileManager } from './profile-manager';

export interface CreateRealmOptions {
Expand All @@ -21,6 +22,17 @@ export interface CreateRealmResult {
authorization: string;
}

export interface PullOptions {
/** Delete local files that don't exist in the realm (default: false). */
delete?: boolean;
}

export interface PullResult {
/** Relative file paths that were downloaded. */
files: string[];
error?: string;
}

export class BoxelCLIClient {
private pm: ProfileManager;

Expand Down Expand Up @@ -59,6 +71,17 @@ export class BoxelCLIClient {
};
}

async pull(
realmUrl: string,
localDir: string,
options?: PullOptions,
): Promise<PullResult> {
return realmPull(realmUrl, localDir, {
delete: options?.delete,
profileManager: this.pm,
});
}

async createRealm(options: CreateRealmOptions): Promise<CreateRealmResult> {
let result = await coreCreateRealm(options.realmName, options.displayName, {
background: options.backgroundURL,
Expand Down
90 changes: 7 additions & 83 deletions packages/software-factory/src/realm-operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
* refactor to boxel-cli tool calls (CS-10529).
*/

import { mkdirSync, writeFileSync } from 'node:fs';
import { dirname, join } from 'node:path';

import { BoxelCLIClient } from '@cardstack/boxel-cli/api';
import type { LooseSingleCardDocument } from '@cardstack/runtime-common';

import { SupportedMimeType } from '@cardstack/runtime-common/supported-mime-type';
Expand Down Expand Up @@ -877,91 +875,17 @@ export async function fetchRealmFilenames(
// ---------------------------------------------------------------------------

/**
* Download all files from a remote realm to a local directory using the
* `_mtimes` endpoint to discover file paths.
*
* TODO: Replace with `boxel pull` once CS-10529 is implemented.
* Download all files from a remote realm to a local directory.
* Delegates to boxel-cli's pull implementation which handles auth
* via the active profile.
*
* Returns the list of relative file paths that were downloaded.
*/
export async function pullRealmFiles(
realmUrl: string,
localDir: string,
options?: RealmFetchOptions,
_options?: RealmFetchOptions,
): Promise<{ files: string[]; error?: string }> {
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

pullRealmFiles() keeps the RealmFetchOptions parameter but now ignores it entirely (including authorization and injected fetch). This is a silent behavior change for any caller that still passes these options.

Consider either (a) updating the signature/type to match the new behavior (e.g. a PullOptions that maps to boxel-cli), or (b) explicitly returning an error when _options contains unsupported fields so callers don’t think auth injection is still honored.

Suggested change
): Promise<{ files: string[]; error?: string }> {
): Promise<{ files: string[]; error?: string }> {
if (_options && Object.keys(_options).length > 0) {
return {
files: [],
error:
'pullRealmFiles no longer supports RealmFetchOptions such as authorization or injected fetch; it now uses boxel-cli pull with active-profile authentication.',
};
}

Copilot uses AI. Check for mistakes.
let fetchImpl = options?.fetch ?? globalThis.fetch;
let normalizedRealmUrl = ensureTrailingSlash(realmUrl);

let headers = buildAuthHeaders(
options?.authorization,
SupportedMimeType.JSONAPI,
);

// Fetch mtimes to discover all file paths.
let mtimesUrl = `${normalizedRealmUrl}_mtimes`;
let mtimesResponse: Response;
try {
mtimesResponse = await fetchImpl(mtimesUrl, { method: 'GET', headers });
} catch (err) {
return {
files: [],
error: `Failed to fetch _mtimes: ${err instanceof Error ? err.message : String(err)}`,
};
}

if (!mtimesResponse.ok) {
let body = await mtimesResponse.text();
return {
files: [],
error: `_mtimes returned HTTP ${mtimesResponse.status}: ${body.slice(0, 300)}`,
};
}

let mtimes: Record<string, number>;
try {
let json = await mtimesResponse.json();
// _mtimes returns JSON:API format: { data: { attributes: { mtimes: {...} } } }
mtimes =
(json as { data?: { attributes?: { mtimes?: Record<string, number> } } })
?.data?.attributes?.mtimes ?? json;
} catch {
return { files: [], error: 'Failed to parse _mtimes response as JSON' };
}

// Download each file.
let downloadedFiles: string[] = [];
for (let fullUrl of Object.keys(mtimes)) {
if (!fullUrl.startsWith(normalizedRealmUrl)) {
continue;
}
let relativePath = fullUrl.slice(normalizedRealmUrl.length);
if (!relativePath || relativePath.endsWith('/')) {
continue;
}

let localPath = join(localDir, relativePath);
mkdirSync(dirname(localPath), { recursive: true });

try {
let fileResponse = await fetchImpl(fullUrl, {
method: 'GET',
headers: buildAuthHeaders(
options?.authorization,
SupportedMimeType.CardSource,
),
});

if (!fileResponse.ok) {
continue;
}

let rawText = await fileResponse.text();
writeFileSync(localPath, rawText);
downloadedFiles.push(relativePath);
} catch {
continue;
}
}

return { files: downloadedFiles.sort() };
let client = new BoxelCLIClient();
return client.pull(realmUrl, localDir);
Comment on lines +887 to +890
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 16, 2026

Choose a reason for hiding this comment

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

P2 Badge Forward pullRealmFiles options to delegated client pull

pullRealmFiles still accepts RealmFetchOptions, but the new delegation ignores _options entirely and always uses a fresh BoxelCLIClient profile. Any caller that passes authorization (or a mocked fetch for controlled execution) now gets silently different behavior—most critically, private realm pulls can fail with "No active profile" even when a valid bearer token was provided.

Useful? React with 👍 / 👎.

}
125 changes: 2 additions & 123 deletions packages/software-factory/tests/factory-test-realm.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
resolveTestRun,
type TestRunAttributes,
} from '../src/factory-test-realm';
import { pullRealmFiles } from '../src/realm-operations';

// ---------------------------------------------------------------------------
// Shared helpers
Expand Down Expand Up @@ -837,128 +836,8 @@ module('factory-test-realm > resolveTestRun', function () {
});
});

// ---------------------------------------------------------------------------
// pullRealmFiles
// ---------------------------------------------------------------------------

module('factory-test-realm > pullRealmFiles', function () {
test('downloads files listed by _mtimes', async function (assert) {
let realmUrl = 'https://realms.example.test/user/personal/';
let capturedUrls: string[] = [];

let mockFetch = (async (url: string | URL | Request) => {
let urlStr = String(url);
capturedUrls.push(urlStr);

if (urlStr.includes('_mtimes')) {
return new Response(
JSON.stringify({
[`${realmUrl}hello.gts`]: 1000,
[`${realmUrl}HelloCard/sample.json`]: 2000,
}),
{ status: 200, headers: { 'Content-Type': SupportedMimeType.JSON } },
);
}

// File downloads
return new Response('file-content', { status: 200 });
}) as typeof globalThis.fetch;

let tmpDir = `/tmp/sf-test-pull-${Date.now()}`;
let result = await pullRealmFiles(realmUrl, tmpDir, { fetch: mockFetch });

assert.strictEqual(result.error, undefined);
assert.strictEqual(result.files.length, 2);
assert.true(result.files.includes('hello.gts'));
assert.true(result.files.includes('HelloCard/sample.json'));

// Should have fetched _mtimes + 2 files = 3 requests
assert.strictEqual(capturedUrls.length, 3);
assert.true(capturedUrls[0].includes('_mtimes'));
});

test('passes authorization header', async function (assert) {
let capturedHeaders: Record<string, string>[] = [];

let mockFetch = (async (
_url: string | URL | Request,
init?: RequestInit,
) => {
capturedHeaders.push((init?.headers as Record<string, string>) ?? {});
// Return empty mtimes so no file downloads happen
return new Response(JSON.stringify({}), {
status: 200,
headers: { 'Content-Type': SupportedMimeType.JSON },
});
}) as typeof globalThis.fetch;

await pullRealmFiles('https://example.test/realm/', '/tmp/unused', {
fetch: mockFetch,
authorization: 'Bearer my-token',
});

assert.strictEqual(capturedHeaders[0]['Authorization'], 'Bearer my-token');
});

test('returns error on _mtimes HTTP failure', async function (assert) {
let mockFetch = (async () => {
return new Response('Forbidden', { status: 403 });
}) as typeof globalThis.fetch;

let result = await pullRealmFiles(
'https://example.test/realm/',
'/tmp/unused',
{ fetch: mockFetch },
);

assert.strictEqual(result.files.length, 0);
assert.true(result.error?.includes('403'));
});

test('returns error on network failure', async function (assert) {
let mockFetch = (async () => {
throw new Error('ECONNREFUSED');
}) as typeof globalThis.fetch;

let result = await pullRealmFiles(
'https://example.test/realm/',
'/tmp/unused',
{ fetch: mockFetch },
);

assert.strictEqual(result.files.length, 0);
assert.true(result.error?.includes('ECONNREFUSED'));
});

test('skips files outside the realm URL', async function (assert) {
let realmUrl = 'https://realms.example.test/user/personal/';

let mockFetch = (async (url: string | URL | Request) => {
let urlStr = String(url);
if (urlStr.includes('_mtimes')) {
return new Response(
JSON.stringify({
[`${realmUrl}hello.gts`]: 1000,
['https://other.test/evil.gts']: 2000, // outside realm
}),
{ status: 200, headers: { 'Content-Type': SupportedMimeType.JSON } },
);
}
return new Response('content', { status: 200 });
}) as typeof globalThis.fetch;

let result = await pullRealmFiles(
realmUrl,
`/tmp/sf-test-pull-${Date.now()}`,
{
fetch: mockFetch,
},
);

assert.strictEqual(result.files.length, 1);
assert.strictEqual(result.files[0], 'hello.gts');
});
});
// pullRealmFiles tests removed — pullRealmFiles now delegates to
// BoxelCLIClient.pull() which is tested in the boxel-cli package.
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This comment says BoxelCLIClient.pull() is tested in the boxel-cli package, but the current test suite appears to cover pullCommand() (integration) rather than the BoxelCLIClient wrapper or the new programmatic pull() return-value API. Consider rewording this to match what’s actually covered, or add a targeted test for the client/pull wrapper so the statement stays true.

Suggested change
// BoxelCLIClient.pull() which is tested in the boxel-cli package.
// BoxelCLIClient.pull(), and pull behavior is covered in the boxel-cli
// package.

Copilot uses AI. Check for mistakes.

// ---------------------------------------------------------------------------
// formatTestResultSummary
Expand Down