fix docs scripts for new stucture

[wixysam]

No description provided.

[claude]

Claude finished @wixysam's task —— View job

---

Code Review Complete

I've analyzed the changes in this PR that refactors the documentation scripts for a new structure. Here's my comprehensive feedback:

Review Checklist:

• Analyze the PR changes and understand the refactoring
• Review package.json changes
• Review copy-to-local-docs.js refactoring
• Review file-processing.js improvements
• Analyze the removal of push-to-docs-repo.js
• Provide comprehensive feedback

Summary

This PR appears to refactor the documentation generation scripts to adapt to a new directory structure, moving SDK docs from a root-level sdk-docs directory to developers/references/sdk/docs and simplifying the deployment process by removing the remote push functionality.

Code Quality and Best Practices ✅

Positive Changes:

1. Improved directory structure: Moving SDK docs to developers/references/sdk/docs follows a better organizational hierarchy
2. Code consolidation: Removing push-to-docs-repo.js (272 lines) eliminates duplicate logic that was similar to copy-to-local-docs.js
3. Better error handling: Added helpful error messages with actionable guidance
4. Command-line flexibility: The --target option in copy-to-local-docs.js provides good flexibility
5. Module renaming logic: The dynamic module renaming (PascalCase to kebab-case) is well-implemented

Areas for Improvement:

1. Hardcoded paths: The new path developers/references/sdk/docs is hardcoded in multiple places. Consider extracting to a constant
2. Missing JSDoc comments: The new functions lack documentation
3. Error handling inconsistency: Some functions use process.exit(1) while others use exceptions

Potential Bugs or Issues 🐛

1. Path traversal vulnerability: In copy-to-local-docs.js:42-46, the tilde expansion could potentially be exploited:

   if (target.startsWith("~")) {
     target = path.join(process.env.HOME, target.slice(1));
   }

   Consider validating the resolved path stays within expected boundaries.

2. Race condition risk: The script deletes and recreates directories without checking for concurrent access

3. Silent failures: In file-processing.js, some operations silently continue on error (e.g., lines 726-731)

Performance Considerations ⚡

1. Synchronous operations: All file operations are synchronous, which could be slow for large documentation sets
2. Multiple file reads: Files are read multiple times in file-processing.js for different transformations
3. Regex performance: Complex regex operations in processLinksInFile() could be optimized

Security Concerns 🔒

1. Command injection: No direct command injection risks found
2. File system access: The scripts properly use path.join() to avoid path traversal in most places
3. No credential exposure: The removal of push-to-docs-repo.js eliminates the need for GitHub SSH keys
4. Environment variable usage: HOME environment variable is used without validation

Test Coverage 🧪

Major concern: No test files were found for these scripts. This is problematic because:

• Documentation generation is critical infrastructure
• The scripts have complex logic (regex transformations, file operations)
• Edge cases aren't covered (missing files, malformed JSON, concurrent access)

Recommendations:

1. Add unit tests for individual functions (module renaming, link processing)
2. Add integration tests for the full documentation pipeline
3. Add tests for error scenarios (missing files, invalid paths)

Additional Recommendations

1. Extract constants: Define paths and patterns as constants at the top of files
2. Add logging levels: Replace console.debug = () => {} with a proper logging solution
3. Validate inputs: Add schema validation for JSON files (category-map.json, types-to-expose.json)
4. Atomic operations: Consider using temporary directories and atomic moves to prevent partial states
5. Add dry-run mode: Allow users to preview changes without modifying files

Overall Assessment

This refactoring improves the codebase by simplifying the deployment process and better organizing the documentation structure. However, the lack of tests and some minor security/robustness issues should be addressed before merging.

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/[email protected]

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

npm i "@base44/sdk@npm:@base44-preview/[email protected]"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/[email protected]"
  }
}

• 📦 Preview Package: @base44-preview/[email protected]
• 🔗 View this commit on GitHub

---

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