Skip to content

Add boxel realm sync status command (CS-10621)#4781

Open
FadhlanR wants to merge 2 commits into
mainfrom
cs-10621-reimplement-boxel-realm-sync-status-command
Open

Add boxel realm sync status command (CS-10621)#4781
FadhlanR wants to merge 2 commits into
mainfrom
cs-10621-reimplement-boxel-realm-sync-status-command

Conversation

@FadhlanR
Copy link
Copy Markdown
Contributor

Summary

Ports the status command from the standalone boxel-cli into the monorepo as boxel realm sync status (alias st). It's git status for a Boxel realm sync — a read-only inspector that shows pending changes between a local sync directory and its remote realm.

boxel realm sync status [local-dir]
boxel realm sync st [local-dir]
  • --pull downloads only the safe one-way subset (new + modified remote, conflicts left untouched). Lighter than the existing pull command, which is an unconditional overwrite.
  • --all recursively scans cwd for .boxel-sync.json dirs and prints a one-line summary per workspace.
  • Attached as a subcommand of sync; the existing bidirectional boxel realm sync <dir> <url> invocation is unaffected.
  • Reuses classifyLocal / classifyRemote / determineAction from lib/sync-logic.ts — same diff engine push/pull/sync already use.

See docs/cs-10621-realm-sync-status-plan.md for the full design (classification mapping, pitfalls, exit codes).

Linear: https://linear.app/cardstack/issue/CS-10621

Test plan

  • 16 new integration tests in packages/boxel-cli/tests/integration/realm-sync-status.test.ts covering all 7 file-state categories, --pull (safe / leaves conflicts / no-op), --all (walks, skips node_modules, continues past malformed manifest), --all + --pull rejection, missing manifest error.
  • Existing realm-sync.test.ts regression — passes (no breakage from registerSyncCommand returning Command).
  • Full boxel-cli unit suite (153 tests) — passes.
  • pnpm tsc --noEmit and pnpm lint — clean.
  • Manual smoke: boxel realm sync --help lists status|st; boxel realm sync status --help shows the new flags; boxel realm sync <dir> <url> parent action still works.

🤖 Generated with Claude Code

Port the status command from standalone boxel-cli to the monorepo. It's
git status for a Boxel realm sync — a read-only inspector that shows
pending changes between a local sync directory and its remote realm.

  boxel realm sync status [local-dir]
  boxel realm sync st [local-dir]

Options:
  --pull   Download only the safe one-way subset (new + modified remote,
           never clobbers unsynced local edits or conflicts).
  --all    Recursively scan cwd for .boxel-sync.json dirs and report a
           summary per workspace.

Attached as a subcommand of sync; the existing bidirectional
`boxel realm sync <dir> <url>` invocation is unaffected.

Reuses the existing classifyLocal / classifyRemote / determineAction
shared between push/pull/sync.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@FadhlanR FadhlanR marked this pull request as ready for review May 12, 2026 08:47
@FadhlanR FadhlanR requested a review from a team May 12, 2026 08:47
@FadhlanR
Copy link
Copy Markdown
Contributor Author

Please ignore the lint error it will be fixed once #4751 is merged. I'll improve the boxel-cli CI, so it wont block multiple open PRs.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40bedbcb7a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +143 to +146
[localFilesWithMtimes, this.remoteMtimes] = await Promise.all([
this.getLocalFileListWithMtimes(),
this.getRemoteMtimes(),
]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fall back when _mtimes cannot be fetched

When getRemoteMtimes() returns an empty map (for example on realms that do not expose _mtimes, or when fetch errors are swallowed by the base helper), this code proceeds to classify against manifest entries without any remote existence fallback. That makes previously synced files appear as deleted-remote (or new-local) even though they still exist remotely, so status reports incorrect diffs and --pull can miss real remote files. sync already avoids this by falling back to directory listing; status needs the same guard before classification.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4 — fall back to getRemoteFileList() when _mtimes returns empty, mirroring sync.ts. Files land in classifyRemote's 'known in manifest.files → changed' branch (rendered as modified-remote) instead of deleted-remote.

Comment thread docs/cs-10621-realm-sync-status-plan.md Outdated
@@ -0,0 +1,207 @@
# CS-10621 — Reimplement `boxel realm sync status`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this file needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in 0cf5aa4.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Boxel CLI subcommand to inspect realm sync state (boxel realm sync status / st), including an optional “safe pull” mode and a multi-workspace scanner, ported into the monorepo’s @cardstack/boxel-cli.

Changes:

  • Introduces boxel realm sync status (alias st) with --pull and --all functionality.
  • Adjusts registerSyncCommand to return the created Commander Command so subcommands can be attached.
  • Adds Vitest integration coverage for status categories, safe-pull behavior, and --all scanning.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/boxel-cli/src/commands/realm/status.ts Implements status logic, safe pull, --all directory discovery, and CLI rendering/exit codes.
packages/boxel-cli/src/commands/realm/sync.ts Returns the sync command instance to enable attaching status as a subcommand.
packages/boxel-cli/src/commands/realm/index.ts Wires registerStatusCommand(sync) under realm sync.
packages/boxel-cli/tests/integration/realm-sync-status.test.ts Adds integration tests for all status categories and new flags.
docs/cs-10621-realm-sync-status-plan.md Documents the intended design/behavior and exit codes for the new command.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

manifestMtime,
changes: inspector.changes,
pulled: inspector.pulled.slice().sort(),
inSync: inspector.changes.length === 0,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4inSync is now !inspector.hasError && inspector.changes.length === 0.

Comment on lines +224 to +228
} catch (err) {
this.hasError = true;
const msg = err instanceof Error ? err.message : String(err);
console.error(` ${FG_RED}✗ ${change.file}${RESET} (${msg})`);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4 — failures accumulate into a failures[] array, and this.error is set to a summary of <file> (<message>) entries so programmatic callers see a non-empty message alongside hasError: true.

Comment on lines +366 to +368
const maxDepth = Number(
process.env.BOXEL_STATUS_ALL_MAX_DEPTH ?? DEFAULT_MAX_DEPTH,
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4 — the env var is now validated with Number.isFinite(parsedDepth) && parsedDepth >= 0, falling back to DEFAULT_MAX_DEPTH on invalid input (NaN, negative, non-numeric).

for (const entry of entries) {
if (!entry.isDirectory()) continue;
if (ALL_IGNORED_DIRS.has(entry.name)) continue;
if (entry.name.startsWith('.')) continue;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4 — dropped the blanket entry.name.startsWith('.') skip. The explicit ALL_IGNORED_DIRS list still excludes .git / .boxel-history / .cache / .vscode; other dot-prefixed dirs (e.g. .workspaces/) are now walkable. Added a regression test.

Comment on lines +393 to +407
try {
JSON.parse(rawContent);
} catch {
workspaces.push({
localDir: dir,
realmUrl: '',
changes: [],
pulled: [],
inSync: false,
hasError: true,
skipped: 'malformed',
});
hasError = true;
continue;
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4statusAll now passes the parsed JSON through isValidManifest(), so JSON-valid-but-shape-wrong manifests bucket as skipped: 'malformed' instead of falling through to status() and being reported as fetch-failed. Added a regression test.

return;
}
if (result.workspaces.length === 0) {
console.log('No .boxel-sync.json directories found under cwd.');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0cf5aa4StatusAllResult now includes rootDir, and the empty-result message uses it: No .boxel-sync.json directories found under ${result.rootDir}.

- Drop docs/cs-10621-realm-sync-status-plan.md (jurgenwerk: not needed).
- Fall back to directory listing when _mtimes is unavailable (Codex P1):
  mirrors sync.ts so remote-existing files don't get classified as
  `deleted-remote` when the endpoint doesn't return mtimes.
- Compute inSync as `!hasError && changes.length === 0` so a realm
  fetch failure doesn't masquerade as "in sync" (Copilot).
- Populate `error` with a summary of failed files on partial --pull
  failures, instead of setting `hasError: true` with no message (Copilot).
- Validate BOXEL_STATUS_ALL_MAX_DEPTH with Number.isFinite and clamp
  to >= 0; fall back to DEFAULT_MAX_DEPTH on invalid input so the depth
  limit can't be defeated by a NaN env var (Copilot).
- Drop the blanket `entry.name.startsWith('.')` skip in the --all
  walker. The explicit ignore list already covers .git, .boxel-history,
  .cache, .vscode; other dot-prefixed dirs (e.g. .workspaces/) should
  be walkable (Copilot).
- Use isValidManifest() in statusAll's pre-check so JSON-valid but
  shape-invalid manifests are bucketed as `malformed` instead of
  `fetch-failed` (Copilot).
- Include rootDir in StatusAllResult and use it in the "no dirs found"
  message instead of hard-coding "under cwd" (Copilot).
- Add two regression tests: shape-invalid manifest → malformed, and
  walker discovers sync dirs under non-ignored dot-prefixed dirs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants