feat(deploy) - add site deploy command

[kfirstri]

This pr adds a basic site deploy command, it prompts the user to verify to path from which we upload, reads the files and mocks a call to the server.

Screen.Recording.2026-01-13.at.18.39.06.mov

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.3-pr.20.c1a0030

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.3-pr.20.c1a0030"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.3-pr.20.c1a0030"
  }
}

• 📦 Preview Package: @base44-preview/cli@0.0.3-pr.20.c1a0030
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[claude]

Pull Request Review: Site Deploy Command

Overview

This PR adds a site deploy command that reads built frontend files, encodes them as base64, and uploads them to Base44 hosting. The implementation follows the project's architectural patterns well and includes proper documentation updates.

✅ Strengths

Architecture & Code Organization

• Excellent structure: Follows the established pattern with separate schema.ts, config.ts, api.ts, and deploy.ts files
• Good documentation: Updated AGENTS.md with clear site module documentation
• Proper separation: Correctly distinguishes site deployment from the Resource pattern
• Progressive enhancement: Nice addition of progress callbacks for user feedback

User Experience

• Interactive prompts: Good use of @clack/prompts for user confirmation
• Progress feedback: The updateMessage callback in runTask provides real-time feedback during file reading
• Clear error messages: Helpful error when no files are found

Code Quality

• Type safety: Proper TypeScript usage with Zod schemas
• Async generators: Smart use of AsyncGenerator for streaming file reads
• Consistent patterns: Matches existing command structure (e.g., entities push)

🐛 Bugs & Issues

Critical Issues

1. Path Traversal Vulnerability (src/core/site/config.ts:18)

const absolutePath = join(outputDir, relativePath);

The code doesn't validate that the resolved path stays within outputDir. A malicious config.jsonc could specify outputDirectory: "../../../" or globby could match symlinks pointing outside the directory.

Fix: Add path validation:

import { resolve, relative } from "node:path";

async function readSiteFile(outputDir: string, relativePath: string): Promise<SiteFile> {
  const absolutePath = resolve(outputDir, relativePath);
  const normalizedBase = resolve(outputDir);
  
  // Ensure the resolved path is within outputDir
  const rel = relative(normalizedBase, absolutePath);
  if (rel.startsWith('..') || path.isAbsolute(rel)) {
    throw new Error(`Path traversal detected: ${relativePath}`);
  }
  
  // ... rest of implementation
}

2. Missing Directory Existence Check (src/cli/commands/site/deploy.ts:16)

const outputDir = resolve(project.root, project.site.outputDirectory);

The code resolves the path but never checks if the directory exists before attempting to deploy. This will cause a confusing error later when globby tries to read from a non-existent directory.

Fix: Add existence check after line 16:

const outputDir = resolve(project.root, project.site.outputDirectory);

if (!(await pathExists(outputDir))) {
  throw new Error(
    `Output directory does not exist: ${project.site.outputDirectory}. Did you run the build command?`
  );
}

Medium Issues

3. Unbounded Memory Usage (src/core/site/deploy.ts:37-47)
The current implementation loads ALL files into memory before uploading:

const files: SiteFile[] = [];
// ... loads all files
files.push(file);

For large sites (100MB+), this could cause memory issues. The async generator pattern is great, but it's not being used effectively.

Consider: Either document this limitation or implement chunked uploads if the API supports it.

4. Missing Zod Validation (src/core/site/api.ts:9)
The uploadSite function returns a DeployResponse but doesn't validate the API response with Zod, unlike other API functions in the codebase (see src/core/resources/entity/config.ts:8-18).

Fix: Add validation when implementing the real API:

export async function uploadSite(files: SiteFile[]): Promise<DeployResponse> {
  const response = await base44Client.post("api/sites/deploy", { json: { files } });
  const data = await response.json();
  
  const result = DeployResponseSchema.safeParse(data);
  if (!result.success) {
    throw new Error(`Invalid API response: ${result.error.message}`);
  }
  
  return result.data;
}

5. Arbitrary Delay in Production Code (src/core/site/config.ts:13)

await new Promise((resolve) => setTimeout(resolve, 20));

This 20ms delay per file is for demo purposes but shouldn't be in production code. It's easy to forget to remove.

Recommendation: Add a clear TODO comment or remove it:

// TODO: Remove this demo delay before production
await new Promise((resolve) => setTimeout(resolve, 20));

Minor Issues

6. Missing Newline at EOF (src/core/site/index.ts:4)
The file is missing a trailing newline, which violates common conventions and most linters.

7. Inconsistent Error Handling
The deploy command doesn't handle the "no site config" error as gracefully as the entities command handles "no entities found". Consider using log.error() and a more user-friendly message.

🔒 Security Concerns

1. Path Traversal (Critical - see issue Add Claude Code GitHub Workflow #1 above)
2. No File Size Validation: Large files could cause issues. Consider adding validation:

   const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
   if (buffer.length > MAX_FILE_SIZE) {
     throw new Error(`File too large: ${relativePath} (${buffer.length} bytes)`);
   }

3. No Total Size Limit: An attacker could create thousands of small files to DoS the upload
4. Base64 Encoding: Increases payload size by ~33%. Consider if the API could accept multipart/form-data with raw files instead

⚡ Performance Considerations

1. Sequential File Reading (src/core/site/config.ts:48): Files are read one at a time. Consider parallel reading with concurrency limit:

   import pLimit from 'p-limit';
   const limit = pLimit(10);
   const files = await Promise.all(
     filePaths.map(path => limit(() => readSiteFile(outputDir, path)))
   );

2. Memory Usage: See issue more everything to single repo #3 - all files loaded into memory

3. Globby Options: Consider adding ignore patterns for common files that shouldn't be deployed:

   return await globby("**/*", {
     cwd: outputDir,
     onlyFiles: true,
     absolute: false,
     gitignore: true,  // Respect .gitignore
     ignore: ['**/.DS_Store', '**/Thumbs.db', '**/*.map'],  // Skip unwanted files
   });

🧪 Test Coverage

Missing Tests: No tests were added for this feature. Critical paths that need testing:

1. readSiteFilesStream - file reading and encoding
2. getSiteFilePaths - glob pattern matching
3. deploySite - orchestration and error handling
4. Path traversal protection (security test)
5. Progress callback invocation
6. Error cases (no files, missing directory, invalid files)

Recommendation: Add tests in tests/core/site.test.ts following the pattern in tests/core/project.test.ts.

📝 Suggested Improvements

1. Add validation for site config in src/core/project/schema.ts:

   const SiteConfigSchema = z.object({
     buildCommand: z.string().optional(),
     serveCommand: z.string().optional(),
     outputDirectory: z.string().min(1).optional(),  // Ensure not empty
     installCommand: z.string().optional(),
   });

2. Add file type validation: Ensure only web assets are uploaded (html, js, css, images, fonts)

3. Consider adding dry-run option:

   new Command("deploy")
     .description("Deploy built site files to Base44 hosting")
     .option("--dry-run", "Show what would be deployed without uploading")

4. Add build integration: The deploy command could optionally run the build command first if site.buildCommand is configured

Summary

This is a solid first implementation that follows the project's patterns well. The main concerns are:

• Critical: Path traversal vulnerability needs to be fixed before merging
• Important: Add directory existence check and Zod validation
• Nice-to-have: Tests, better memory management, and security hardening

The code quality is good and the architecture is sound. With the security fixes applied, this will be a great addition to the CLI.

---

Recommendation: Request changes for the critical security issue, then approve after fixes.

[claude]

Pull Request Review: Site Deploy Command

Thank you for this contribution! This PR adds a solid foundation for site deployment functionality. Below is my detailed review covering code quality, potential issues, and recommendations.

✅ Strengths

1. Well-structured architecture: Follows the established patterns in the codebase with proper separation between CLI commands (src/cli/commands/site/deploy.ts) and core logic (src/core/site/)

2. Good error handling: Validates output directory existence and checks for empty directories before proceeding

3. User experience: Uses confirmation prompts and proper spinner feedback during deployment

4. Documentation: AGENTS.md is properly updated with the new site module documentation

5. Type safety: Proper use of Zod schemas for API response validation

6. Cleanup: Properly cleans up temporary tar.gz files in finally block

🔧 Code Quality Issues

1. Missing FormData compatibility for Node.js (src/core/site/api.ts:15)

const formData = new FormData();
formData.append("file", archiveBuffer, "dist.tar.gz");

Issue: Node.js doesn't have a native FormData that works with Buffer the same way browsers do. The ky library uses the native FormData, which may not handle Buffer properly.

Fix: You'll need to either:

• Use formdata-node or similar package that works with Node.js
• Or convert the Buffer to a Blob: new Blob([archiveBuffer])

Priority: HIGH - This will likely fail at runtime

2. Incorrect import path (src/core/site/api.ts:1)

import { getAppClient } from "@core/clients/index.js";

Issue: Based on the codebase structure shown in AGENTS.md (line 34), the correct path should be @core/api/index.js, not @core/clients/index.js

Priority: HIGH - This is a compilation error

3. Incorrect AGENTS.md reference (AGENTS.md:66)

The documentation says:

├── config.ts         # readSiteFiles() - glob and encode files

But the function in config.ts is getSiteFilePaths(), not readSiteFiles(). This should be updated for consistency.

Priority: LOW - Documentation accuracy

4. Missing error handling for tar creation (src/core/site/deploy.ts:34)

await createArchive(siteOutputDir, archivePath);

The tarCreate function might fail (disk full, permissions, etc.), but there's no specific error handling or helpful error message for users.

Recommendation: Wrap in try-catch with a user-friendly error message

5. runTask callback signature not used (src/cli/commands/site/deploy.ts:28-36)

const result = await runTask(
  "Creating archive and deploying site...",
  async () => {
    return await deploySite(outputDir);
  },

Issue: The updated runTask signature accepts an updateMessage callback parameter (src/cli/utils/runTask.ts:21), but it's not being used here. During a potentially long deployment, users would benefit from progress updates like:

• "Creating archive..."
• "Uploading to Base44..."
• "Deployment complete"

Recommendation: Use the updateMessage callback for better UX

🐛 Potential Bugs

1. Race condition in temp file cleanup (src/core/site/deploy.ts:36-37)

} finally {
  await deleteFile(archivePath);
}

If uploadSite fails while the file is being read/uploaded, there's a small chance the cleanup could happen while the file is still in use. This is minor but worth noting.

Recommendation: Consider more robust cleanup or at least document this edge case

2. UUID toString() is redundant (src/core/site/deploy.ts:30)

`base44-site-${getBase44ClientId()}-${randomUUID().toString()}.tar.gz`

randomUUID() already returns a string, so .toString() is unnecessary.

Priority: LOW - Works but is redundant

🔒 Security Considerations

1. Path traversal vulnerability check needed

The code resolves project.site.outputDirectory relative to the project root:

const outputDir = resolve(project.root, project.site.outputDirectory);

Question: Is there validation elsewhere that project.site.outputDirectory can't contain ../ sequences that escape the project root? If not, a malicious config.jsonc could deploy arbitrary files from the system.

Recommendation: Add validation to ensure the resolved path is within the project root

2. Temporary file naming includes client ID

`base44-site-${getBase44ClientId()}-${randomUUID().toString()}.tar.gz`

This exposes the client ID in the temp directory. While temp directories typically have restricted permissions, consider if this is necessary or if just the UUID would suffice.

Priority: LOW - Mostly a privacy consideration

⚡ Performance Considerations

1. No file size limits

There's no check for the total size of files being deployed. Very large deployments could:

• Fill up the temp directory
• Cause network timeouts
• Hit server upload limits

Recommendation: Add a size check or at least document expected limitations

2. No upload progress feedback

For large sites, users might think the CLI is frozen during upload. Consider:

• Using the updateMessage callback during different phases
• Adding a progress indicator if possible with the upload library

🧪 Test Coverage

CRITICAL: No tests were added for the new functionality.

Required tests:

1. deploySite() with non-existent directory
2. deploySite() with empty directory
3. getSiteFilePaths() with various file structures
4. Archive creation and cleanup
5. API response validation with DeployResponseSchema
6. Error handling for failed uploads

Recommendation: Add test file at tests/core/site.test.ts following the pattern in tests/core/project.test.ts

📋 Missing Edge Cases

1. Symlinks: What happens if the output directory contains symlinks?
2. Hidden files: Should .git, .DS_Store, etc. be excluded?
3. Large files: Binary assets, videos, etc.
4. Concurrent deploys: What if someone runs base44 site deploy twice?

🎯 Recommendations Summary

Must Fix (Breaking Issues):

1. Fix FormData Buffer compatibility for Node.js
2. Correct the import path from @core/clients to @core/api
3. Add test coverage

Should Fix (User Experience):

4. Use updateMessage callback for deployment progress
5. Add file size validation/limits
6. Add path traversal validation

Nice to Have:

7. Update AGENTS.md to reflect actual function name
8. Remove redundant .toString() on UUID
9. Add file filtering (exclude hidden files, etc.)
10. Better error messages for tar creation failures

Overall Assessment

This is a solid first implementation that follows the codebase conventions well. The architecture is clean and the code is generally well-structured. However, there are two critical issues that will prevent this from working:

1. FormData Buffer handling
2. Incorrect import path

Once these are fixed and tests are added, this will be ready to merge.

Recommendation: Request changes for the must-fix items, then approve after fixes.

---

🤖 Generated with Claude Code

[claude]

PR Review: feat(deploy) - add site deploy command

Summary

This PR adds a basic site deployment command that creates a tar.gz archive from a build output directory and uploads it to Base44 hosting. The implementation follows the established patterns in the codebase and includes good documentation.

✅ Strengths

Code Quality

• Well-structured: Follows the established modular architecture with proper separation between core/site/ and cli/commands/site/
• Consistent patterns: Matches existing patterns (entities push command, resource structure)
• Good documentation: AGENTS.md is properly updated with clear examples
• Type safety: Proper use of Zod schemas for API response validation
• Error messages: Clear, actionable error messages guide users

Architecture

• Proper layering: Core logic in src/core/site/, CLI interaction in src/cli/commands/site/
• Reusable utilities: New readFile() and deleteFile() functions added to fs.ts
• Clean API: Simple public interface with deploySite() orchestrating the workflow

User Experience

• Confirmation prompt: Asks user to confirm deployment before proceeding
• Informative output: Shows deployment URL after success
• Cancellation support: Proper handling of user cancellation with isCancel()

---

🚨 Critical Issues

1. Missing Authentication (CRITICAL)

The deploy command does not require authentication, unlike entities push. This must be fixed.

Location: src/cli/commands/site/deploy.ts:48
Fix: Add requireAuth: true to the runCommand call

2. runTask Signature Breaking Change (HIGH)

The runTask signature was changed to require an updateMessage callback parameter. This is a breaking change for existing code.

Fix: Make the parameter optional or update all existing callers.

---

🔧 Medium Priority Issues

3. Missing Error Handling for Archive Creation

createArchive() does not wrap tar creation in try-catch. Users will get raw errors on failures.

4. UUID toString() is Redundant

randomUUID() already returns a string, toString() call is unnecessary.

5. Missing Newline at End of File

src/core/site/index.ts:4 is missing a newline at EOF.

---

⚠️ Lower Priority Issues

6. Inconsistent Error Handling in uploadSite

Function does not check response.ok before parsing JSON, unlike pushEntities().

7. Missing JSDoc for Public Functions

deploySite() and getSiteFilePaths() lack JSDoc comments for consistency.

---

🧪 Testing Concerns

No tests were added for the new functionality. Recommend adding unit tests for:

• deploySite() with success and error cases
• getSiteFilePaths() with various directory structures
• Temporary file cleanup
• Empty directory handling

---

🔒 Security & Performance

Security - Generally Good

• Path traversal prevented
• Temp files use randomUUID()
• Limited to project output directory

Performance Concerns

• No progress indication for large uploads
• readFile() loads entire archive into memory (problematic for large sites)
• Consider streaming for files over 100MB

---

📝 Recommendation

REQUEST CHANGES - Critical issues must be fixed before merge.

Required:

1. Add requireAuth: true to deploy command
2. Fix runTask breaking change (make updateMessage optional)
3. Add error handling for tar archive creation
4. Add newline at EOF in index.ts

Strongly Recommended:

5. Add error response handling in uploadSite()
6. Add basic unit tests
7. Remove redundant toString() call

Nice to Have:

8. Add JSDoc comments
9. Consider streaming for large files
10. Add file size information

---

Great work overall! The architecture is solid. Once critical issues are addressed, this will be a solid addition.

[claude]

Code Review: Site Deploy Command

Thanks for this PR! The implementation is generally solid and follows the project's conventions well. Here are my findings across different areas:

✅ Positives

• Good architecture: Clear separation between CLI command layer (src/cli/commands/site/deploy.ts) and core logic (src/core/site/)
• Consistent patterns: Follows existing command patterns (entities push) with runCommand and runTask wrappers
• Type safety: Proper use of Zod schemas for API response validation
• Error handling: Good error messages and validation checks
• Documentation: Excellent JSDoc comments and updated AGENTS.md

🐛 Issues Found

1. Missing Authentication (Critical)

Location: src/cli/commands/site/deploy.ts:48

The deploy command doesn't require authentication, but it makes an API call to deploy-dist. Looking at similar commands like entities push, this should require auth.

// Current
await runCommand(deployAction);

// Should be
await runCommand(deployAction, { requireAuth: true });

2. Potential Resource Leak (Medium)

Location: src/core/site/deploy.ts:36-37

If deleteFile() fails in the finally block, it will throw and potentially mask the original error. The file deletion should be more defensive:

} finally {
  try {
    await deleteFile(archivePath);
  } catch {
    // Ignore cleanup errors - the file is in tmpdir and will be cleaned up eventually
  }
}

3. Missing newline in index.ts (Minor)

Location: src/core/site/index.ts:4

File missing trailing newline (inconsistent with other files in codebase).

4. Unused runTask updateMessage parameter (Minor)

Location: src/cli/commands/site/deploy.ts:30

The PR updates runTask to support progress messages via updateMessage callback, but the deploy command doesn't use it. Consider adding progress updates during archive creation:

async (updateMessage) => {
  updateMessage("Creating archive...");
  // create archive
  updateMessage("Uploading to Base44...");
  return await deploySite(outputDir);
}

Or if not needed, remove the unused parameter.

5. API Error Handling (Medium)

Location: src/core/site/api.ts:19-22

The API call doesn't handle HTTP errors before attempting to parse JSON. If the server returns 4xx/5xx, this will fail cryptically. Consider:

const response = await appClient.post("deploy-dist", {
  body: formData,
});

if (!response.ok) {
  throw new Error(`Deploy failed: ${response.statusText}`);
}

const result = DeployResponseSchema.parse(await response.json());

🔒 Security Considerations

• Path traversal: The code uses resolve() to build paths, which is good
• Temp file naming: Uses UUID which prevents collisions - good
• File cleanup: Temp files are cleaned up (though error handling could be better)
• No sensitive data exposure: Error messages don't leak sensitive info

⚡ Performance Considerations

• Archive creation: Using streaming tar creation is efficient for large sites
• Memory usage: The tar library handles streaming, so large sites won't OOM
• API upload: Entire archive is loaded into memory as Buffer for FormData. For very large sites (100MB+), this could be problematic. Consider streaming uploads in the future if needed.

🧪 Test Coverage

No tests added for this feature. The codebase has at least one test file (tests/core/project.test.ts), so there's a testing infrastructure. Consider adding:

• Unit tests for getSiteFilePaths()
• Unit tests for deploySite() error cases
• Integration test for the full deploy flow (can mock the API call)

📝 Code Quality

• Naming: Clear and consistent
• Structure: Well-organized module structure
• Comments: Excellent JSDoc documentation
• Formatting: Consistent with project style
• Dependencies: The tar library is appropriate for this use case

🎯 Recommendations

Must Fix

1. Add requireAuth: true to the deploy command
2. Fix error handling in API calls
3. Add defensive error handling to file cleanup

Should Fix

4. Add trailing newline to src/core/site/index.ts
5. Either use the updateMessage parameter or remove it from the deploy command

Nice to Have

6. Add test coverage for the new functionality
7. Consider adding a --yes flag to skip confirmation prompt (for CI/CD use)
8. Document the expected site.outputDirectory config in a README or config schema

Summary

This is a well-structured PR that adds useful functionality. The main issues are the missing authentication requirement and some error handling improvements. Once those are addressed, this will be ready to merge.

Great work overall! The code is clean and follows the project's patterns nicely. 🎉

[claude]

Pull Request Review: Site Deploy Command

Summary

This PR adds a site deployment feature that creates a tar.gz archive of built files and uploads them to Base44 hosting. The implementation follows the established patterns in the codebase and is well-structured.

Code Quality ✅

Strengths:

• Good separation of concerns: The site module is properly organized with distinct files for schema, config, API, and deployment logic
• Consistent patterns: Follows the same structure as other modules (entities, functions)
• Good documentation: JSDoc comments are clear and helpful
• Proper TypeScript usage: Strong typing with Zod schemas for API validation
• Error handling: Good validation checks for missing directories and empty output

Style:

• Code follows existing conventions in the codebase
• Import organization is consistent
• Naming conventions are clear and descriptive

Potential Issues 🔍

1. runTask callback signature change (src/cli/utils/runTask.ts:30)

The runTask function signature was changed to pass an updateMessage callback, but the deploy command doesn't use it:

const result = await runTask(
  "Creating archive and deploying site...",
  async () => {  // ❌ Doesn't accept updateMessage parameter
    return await deploySite(outputDir);
  },

Impact: This works but misses an opportunity for progress feedback during long uploads.

Recommendation: Consider either:

• Using the updateMessage callback to show progress (e.g., "Creating archive...", "Uploading...")
• Or make the callback parameter optional to avoid breaking changes

2. Missing newline at end of file (src/core/site/index.ts:4)

The file is missing a trailing newline, which is a common convention.

3. Cleanup on error (src/core/site/deploy.ts:36-37)

The finally block will always attempt to delete the temp file, but if archive creation fails before the file is created, this could cause issues.

try {
  await createArchive(siteOutputDir, archivePath);
  return await uploadSite(archivePath);
} finally {
  await deleteFile(archivePath);  // ❌ May fail if file was never created
}

Impact: Low - deleteFile already checks if file exists before deleting, so this is actually safe. Nice defensive programming!

4. No size limits (src/core/site/deploy.ts)

The deployment doesn't check the archive size before uploading. Very large sites could cause issues.

Recommendation: Consider adding a size check and warning/error for archives over a certain threshold (e.g., 100MB).

Performance Considerations ⚡

1. Archive creation is synchronous-blocking

The tar.create() function blocks the event loop while creating the archive. For large sites, this could be noticeable.

Recommendation: Consider adding progress feedback using the updateMessage callback if tar creation takes a while.

2. Memory usage for large files

The entire archive is loaded into memory as a Buffer (src/core/site/api.ts:14) before uploading.

const archiveBuffer = await readFile(archivePath);  // Entire file in memory
const blob = new Blob([archiveBuffer], { type: "application/gzip" });

Impact: For very large sites (100MB+), this could cause memory issues.

Recommendation: If supporting large deployments, consider streaming the upload instead of buffering the entire file. However, this may require API changes, so it's fine to defer until needed.

Security Concerns 🔒

1. Path traversal validation ✅

Good use of resolve() to normalize paths, but the code doesn't explicitly validate that outputDir stays within expected boundaries.

const outputDir = resolve(project.root, project.site.outputDirectory);

Current risk: Low - project.site.outputDirectory comes from the project's own config file, not user input.

Recommendation: If outputDirectory ever comes from CLI args or environment variables, add validation to prevent path traversal attacks.

2. Temporary file naming ✅

Good use of randomUUID() to prevent collisions:

const archivePath = join(
  tmpdir(),
  `base44-site-${getBase44ClientId()}-${randomUUID().toString()}.tar.gz`
);

Note: The .toString() on randomUUID() is redundant as it already returns a string, but harmless.

3. API response validation ✅

Excellent use of Zod schema validation on the API response to ensure data integrity.

Test Coverage ❌

Missing Tests:

• No test file for the new site module
• Existing test pattern in tests/core/project.test.ts should be followed
• Consider testing:
  • deploySite() with valid/invalid directories
  • Error handling for missing output directory
  • Archive creation and cleanup
  • getSiteFilePaths() with various directory structures

Recommendation: Add tests/core/site.test.ts with unit tests for the core functionality.

Additional Recommendations 💡

1. User feedback improvements

Consider using the updateMessage callback in runTask:

const result = await runTask(
  "Preparing deployment...",
  async (updateMessage) => {
    updateMessage("Creating archive...");
    // archive creation
    updateMessage("Uploading to Base44...");
    return await deploySite(outputDir);
  },

2. Add deployment size information

Show the archive size before uploading for user awareness:

log.info(`Archive size: ${(totalSize / 1024 / 1024).toFixed(2)} MB`);

3. Configuration validation

Consider validating the output directory path in the config schema to catch issues early.

4. Documentation update

The AGENTS.md was updated nicely, but consider updating README.md to document the new base44 site deploy command.

Summary

Overall: This is a solid implementation that follows best practices. The code is clean, well-organized, and follows the existing patterns in the codebase.

Verdict: ✅ Approve with minor suggestions

The issues identified are mostly minor improvements and missing tests. The core functionality is sound and ready to merge, but adding tests would significantly improve the quality.

Priority fixes before merge:

1. Add unit tests for the site module
2. Consider using the updateMessage callback for better UX
3. Add trailing newline to index.ts

Nice-to-have improvements:

• Size limits and warnings for large deployments
• Documentation in README.md
• Archive size display

[claude]

PR Review: Site Deploy Command

Thanks for implementing the site deployment feature! This is a solid implementation with good separation of concerns.

Strengths

Architecture and Code Organization

• Excellent module structure with clean separation (schema, config, api, deploy)
• Consistent with existing entity resource patterns (Zod schemas, separate API layer)
• Well-documented code with JSDoc comments
• Good separation of concerns throughout

Security

• Authentication properly required via requireAuth: true
• User confirmation before deployment (good UX and safety)
• Proper cleanup with try-finally for temporary files

Best Practices

• Strong typing throughout with Zod schema validation
• Clear, actionable error messages
• Proper path handling with resolve

Critical Issues

1. Missing Error Handling in API Upload (src/core/site/api.ts:20-22)

The uploadSite function does not handle HTTP errors. Unlike the entity push API pattern, this could throw unclear errors.

Recommended: Add throwHttpErrors: false and check response.ok, following the pattern from src/core/resources/entity/api.ts:17-28

2. Missing Test Coverage

No tests are included for the new functionality. Tests should cover:

• getSiteFilePaths() with empty/non-existent directories
• deploySite() validation logic and error cases
• Archive creation and cleanup
• API error handling

Suggested test file: tests/core/site.test.ts

Other Issues

3. Redundant UUID call (src/core/site/deploy.ts:30)

randomUUID() already returns a string, so .toString() is redundant.

4. Missing Error Handling in Archive Creation (src/core/site/deploy.ts:45-52)

The tarCreate function could fail but lacks explicit error handling. Add try-catch with validation that archive was created.

5. Progress Feedback Enhancement (src/cli/commands/site/deploy.ts:28-32)

runTask now supports updateMessage parameter for progress updates. Consider using it for better user feedback during large deployments.

6. Missing Newline (src/core/site/index.ts:4)

Index file missing trailing newline for consistency.

7. Performance: Large File Memory Issues

Loading entire archive into memory could be problematic for large sites. Consider streaming or document the limitation with size checks.

Summary

Well-structured implementation following project conventions. Main concerns:

1. CRITICAL: Missing HTTP error handling in upload
2. IMPORTANT: Missing test coverage
3. MINOR: Small robustness improvements

Recommendation: Request changes for error handling and tests before approval.