Merge pull request #8 from cardstack/feature/sync-cleanup-prompt

Add failed download cleanup prompt and documentation updates

Author: christse

.claude/CLAUDE.md

@@ -97,6 +97,39 @@ npx boxel profile switch username   # Switch by partial match
 
 ---
 
+## Local Workspace Organization
+
+When syncing multiple workspaces locally, organize them by **domain/username/realm** to mirror the Matrix ID structure (`@username:domain`):
+
+```
+boxel-workspaces/
+├── boxel.ai/                      # Production domain
+│   └── acme-corp/                 # Username
+│       ├── personal/              # Realm
+│       ├── project-atlas/
+│       └── inventory-tracker/
+└── stack.cards/                   # Staging domain
+    └── acme-corp/
+        └── sandbox/
+```
+
+**Benefits:**
+- Clear separation between production and staging environments
+- Matches the `@username:domain` profile ID format
+- Easy to identify which profile/environment a workspace belongs to
+- Supports multiple users on the same machine
+
+**First-time sync to this structure:**
+```bash
+# Production workspace
+boxel pull https://app.boxel.ai/acme-corp/project-atlas/ ./boxel-workspaces/boxel.ai/acme-corp/project-atlas
+
+# Staging workspace
+boxel pull https://realms-staging.stack.cards/acme-corp/sandbox/ ./boxel-workspaces/stack.cards/acme-corp/sandbox
+```
+
+---
+
 ## Available Skills
 
 ### `/track` - Track Local Edits
@@ -136,15 +169,40 @@ boxel status . --pull             # Auto-pull remote changes
 boxel check ./file.json --sync    # Check single file
 ```
 
-### Sync
+### Pull, Push, Sync (Command Relationship)
+
+| Command | Direction | Purpose | Deletes Local | Deletes Remote |
+|---------|-----------|---------|---------------|----------------|
+| `pull` | Remote → Local | Fresh download | with `--delete` | never |
+| `push` | Local → Remote | Deploy changes | never | with `--delete` |
+| `sync` | Both ways | Stay in sync | with `--prefer-remote` | with `--prefer-local` |
+
 ```bash
 boxel sync .                      # Interactive sync
 boxel sync . --prefer-local       # Keep local + sync deletions
 boxel sync . --prefer-remote      # Keep remote
 boxel sync . --prefer-newest      # Keep newest version
 boxel sync . --delete             # Sync deletions both ways
 boxel sync . --dry-run            # Preview only
+
+boxel push ./local <url>          # One-way push (local → remote)
+boxel push ./local <url> --delete # Push and remove orphaned remote files
+boxel pull <url> ./local          # One-way pull (remote → local)
+```
+
+**Failed download cleanup:** When `sync` encounters files that return 500 errors (broken/corrupted on server), it will prompt you to delete them:
 ```
+⚠️  3 file(s) failed to download (server error):
+   - Staff/broken-card.json
+   - Student/corrupted.json
+
+These files may be broken on the server. Delete them from remote? [y/N]
+```
+
+> **Safety tip:** Before any destructive operation, create a checkpoint with a descriptive message:
+> ```bash
+> boxel history . -m "Before cleanup: removing broken server files"
+> ```
 
 ### Track ⇆ (Local File Watching)
 ```bash
@@ -348,6 +406,21 @@ boxel realms --llm
 
 ## Critical Patterns
 
+### ⚠️ SAFETY FIRST: Checkpoint Before Destructive Operations
+**Always create a checkpoint with a descriptive message before:**
+- Deleting files from server (`--prefer-local`, `push --delete`)
+- Restoring to an earlier checkpoint
+- Bulk cleanup operations
+- Removing card definitions or instances
+
+```bash
+boxel history . -m "Before cleanup: removing sample data and unused definitions"
+# Now safe to proceed with destructive operation
+boxel sync . --prefer-local
+```
+
+This ensures you can always recover if something goes wrong. The checkpoint message helps identify what state to restore to.
+
 ### 0. ALWAYS Write Source Code, Never Compiled Output
 When editing `.gts` files, **always write clean idiomatic source code**:
 ```gts
@@ -563,3 +636,9 @@ Headers:
 ### Switching environments (prod/staging)
 - Add profiles for each environment
 - Switch with: `boxel profile switch <username>`
+
+### "500 Internal Server Error" on specific files
+- These files are broken/corrupted on the server
+- Sync will prompt you to delete them after completion
+- Or use `boxel push . <url> --delete` to remove all orphaned remote files
+- Check if card definitions have errors in Boxel web UI

README.md

@@ -186,6 +186,14 @@ MATRIX_PASSWORD=your-password
 
 ### Sync Operations
 
+**Pull, Push, and Sync relationship:**
+
+| Command | Direction | Purpose | Deletes Local | Deletes Remote |
+|---------|-----------|---------|---------------|----------------|
+| `pull` | Remote → Local | Fresh download | with `--delete` | never |
+| `push` | Local → Remote | Deploy changes | never | with `--delete` |
+| `sync` | Both ways | Stay in sync | with `--prefer-remote` | with `--prefer-local` |
+
 ```bash
 boxel sync .                      # Bidirectional sync (interactive)
 boxel sync . --prefer-local       # Keep local on conflicts, sync deletions to server
@@ -195,9 +203,24 @@ boxel sync . --delete             # Sync deletions both ways
 boxel sync . --dry-run            # Preview only
 
 boxel push ./local <url>          # One-way push (local → remote)
+boxel push ./local <url> --delete # Push and remove orphaned remote files
 boxel pull <url> ./local          # One-way pull (remote → local)
 ```
 
+**Failed download cleanup:** When `sync` encounters files that return 500 errors (broken on server), it will prompt you to delete them:
+```
+⚠️  3 file(s) failed to download (server error):
+   - Staff/broken-card.json
+   - Student/corrupted.json
+
+These files may be broken on the server. Delete them from remote? [y/N]
+```
+
+> **Safety tip:** Before any destructive operation (deleting files, restoring checkpoints), create a checkpoint with a descriptive message:
+> ```bash
+> boxel history . -m "Before cleanup: removing broken server files"
+> ```
+
 ### Track & Watch
 
 ```bash
@@ -348,6 +371,20 @@ boxel sync . --prefer-local       # Push to Boxel server
 
 ## Critical Patterns
 
+### 0. Checkpoint Before Destructive Operations
+**Always create a checkpoint with a descriptive message before:**
+- Deleting files from server (`--prefer-local`, `push --delete`)
+- Restoring to an earlier checkpoint
+- Bulk cleanup operations
+
+```bash
+boxel history . -m "Before cleanup: removing sample data"
+# Now safe to proceed with destructive operation
+boxel sync . --prefer-local
+```
+
+This ensures you can always recover if something goes wrong.
+
 ### 1. Always Use `--prefer-local` After Restore
 ```bash
 boxel history . -r 3              # Deletes files locally
@@ -388,6 +425,38 @@ export class MyCard extends CardDef {
 
 ---
 
+## Local Workspace Organization
+
+When syncing multiple workspaces locally, organize them by **domain/username/realm** to mirror the Matrix ID structure (`@username:domain`):
+
+```
+boxel-workspaces/
+├── boxel.ai/                      # Production domain
+│   └── acme-corp/                 # Username
+│       ├── personal/              # Realm
+│       ├── project-atlas/
+│       └── inventory-tracker/
+└── stack.cards/                   # Staging domain
+    └── acme-corp/
+        └── sandbox/
+```
+
+**Benefits:**
+- Clear separation between production and staging environments
+- Matches the `@username:domain` profile ID format
+- Easy to identify which profile/environment a workspace belongs to
+
+**First-time sync to this structure:**
+```bash
+# Production workspace
+boxel pull https://app.boxel.ai/username/realm/ ./boxel-workspaces/boxel.ai/username/realm
+
+# Staging workspace
+boxel pull https://realms-staging.stack.cards/username/realm/ ./boxel-workspaces/stack.cards/username/realm
+```
+
+---
+
 ## File Structure
 
 ```
@@ -508,6 +577,7 @@ cat ./Type/card-id.json
 | Files reverting after restore | Stop watch first, use `--prefer-local` after |
 | Watch not detecting changes | Check interval, verify workspace URL |
 | Definition changes not reflected | `boxel touch . Instance/file.json` |
+| "500 Internal Server Error" on files | Broken on server - sync will prompt to delete, or use `push --delete` |
 
 ---
 

src/commands/milestone.ts

@@ -1,6 +1,5 @@
 import { CheckpointManager } from '../lib/checkpoint-manager.js';
 import * as path from 'path';
-import * as fs from 'fs';
 
 // ANSI color codes
 const FG_GREEN = '\x1b[32m';
@@ -23,17 +22,11 @@ export async function milestoneCommand(
   // Resolve workspace path
   const workspaceDir = path.resolve(workspace);
 
-  // Check if it's a synced workspace
-  const manifestPath = path.join(workspaceDir, '.boxel-sync.json');
-  if (!fs.existsSync(manifestPath)) {
-    console.error('Error: No .boxel-sync.json found. Run sync first to establish tracking.');
-    process.exit(1);
-  }
-
   const manager = new CheckpointManager(workspaceDir);
 
+  // Check if checkpoint history exists (created by pull, push, sync, or watch)
   if (!manager.isInitialized()) {
-    console.error('Error: No checkpoint history found. Checkpoints are created during sync/watch.');
+    console.error('Error: No checkpoint history found. Run pull, sync, or watch first to create checkpoints.');
     process.exit(1);
   }
 

src/commands/sync.ts

@@ -111,8 +111,24 @@ async function promptUser(question: string, options: string[]): Promise<string>
   });
 }
 
+// Helper function to prompt yes/no
+async function promptYesNo(question: string): Promise<boolean> {
+  const rl = readline.createInterface({
+    input: process.stdin,
+    output: process.stdout,
+  });
+
+  return new Promise((resolve) => {
+    rl.question(question + ' ', (answer) => {
+      rl.close();
+      resolve(answer.toLowerCase() === 'y' || answer.toLowerCase() === 'yes');
+    });
+  });
+}
+
 class RealmSyncer extends RealmSyncBase {
   hasError = false;
+  failedPulls: { relativePath: string; error: string }[] = [];
 
   constructor(
     private syncOptions: SyncCommandOptions,
@@ -123,6 +139,11 @@ class RealmSyncer extends RealmSyncBase {
     super(syncOptions, matrixUrl, username, password);
   }
 
+  // Public method to delete a file from the remote server
+  async deleteRemoteFile(relativePath: string): Promise<void> {
+    return this.deleteFile(relativePath);
+  }
+
   private getConflictStrategy(): ConflictStrategy {
     if (this.syncOptions.preferLocal) return 'local';
     if (this.syncOptions.preferRemote) return 'remote';
@@ -265,6 +286,7 @@ class RealmSyncer extends RealmSyncBase {
     }
 
     // Execute pulls
+    const failedPulls: { relativePath: string; error: string }[] = [];
     if (pullActions.length > 0) {
       console.log(`\nPulling ${pullActions.length} files from remote...`);
       for (const action of pullActions) {
@@ -278,11 +300,19 @@ class RealmSyncer extends RealmSyncBase {
           };
         } catch (error) {
           this.hasError = true;
+          const errorMsg = error instanceof Error ? error.message : String(error);
           console.error(`Error pulling ${action.relativePath}:`, error);
+          // Track 500 errors for potential cleanup
+          if (errorMsg.includes('500') || errorMsg.includes('Internal Server Error')) {
+            failedPulls.push({ relativePath: action.relativePath, error: errorMsg });
+          }
         }
       }
     }
 
+    // Store failed pulls for post-sync cleanup prompt
+    this.failedPulls = failedPulls;
+
     // Handle local deletions (files deleted on server) - always sync these
     // Create checkpoint BEFORE deleting so we can recover
     if (deleteLocalActions.length > 0) {
@@ -604,14 +634,10 @@ export async function syncCommand(
   explicitUrl: string,
   options: SyncCommandOptionsInput,
 ): Promise<void> {
-  const matrixUrl = process.env.MATRIX_URL;
-  const matrixUsername = process.env.MATRIX_USERNAME;
-  const matrixPassword = process.env.MATRIX_PASSWORD;
-
-  if (!matrixUrl || !matrixUsername || !matrixPassword) {
-    console.error('Missing Matrix credentials in environment variables');
-    process.exit(1);
-  }
+  // Determine workspace URL for profile detection (use explicit URL or resolve later)
+  const urlForProfile = explicitUrl || (workspaceRef.startsWith('http') ? workspaceRef : '');
+  const { matrixUrl, username: matrixUsername, password: matrixPassword } =
+    await validateMatrixEnvVars(urlForProfile);
 
   let localDir: string;
   let workspaceUrl: string;
@@ -675,6 +701,31 @@ export async function syncCommand(
     await syncer.initialize();
     await syncer.sync();
 
+    // Handle failed pulls - offer to delete broken files from server
+    if (syncer.failedPulls.length > 0) {
+      console.log(`\n⚠️  ${syncer.failedPulls.length} file(s) failed to download (server error):`);
+      for (const failed of syncer.failedPulls) {
+        console.log(`   - ${failed.relativePath}`);
+      }
+
+      const shouldDelete = await promptYesNo(
+        '\nThese files may be broken on the server. Delete them from remote? [y/N]'
+      );
+
+      if (shouldDelete) {
+        console.log('\nDeleting broken files from server...');
+        for (const failed of syncer.failedPulls) {
+          try {
+            await syncer.deleteRemoteFile(failed.relativePath);
+            console.log(`  Deleted: ${failed.relativePath}`);
+          } catch (error) {
+            console.error(`  Failed to delete ${failed.relativePath}:`, error);
+          }
+        }
+        console.log('Cleanup completed.');
+      }
+    }
+
     if (syncer.hasError) {
       console.log('Sync completed with errors. View logs for details.');
       process.exit(2);