feat(OCLIF): refactored the CLI package to use the OCLIF framework

[kfirstri]

I'm considering to start using OCLIF for this CLI, we get many features out of the box and a known framework

[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:

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

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "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!}

[claude]

Pull Request Review: OCLIF Migration

Overview

This PR migrates the Base44 CLI from Commander.js to OCLIF, a production-grade CLI framework. The migration is well-executed with thoughtful architectural decisions.

✅ Strengths

Architecture & Design

• Excellent framework choice: OCLIF is a mature, battle-tested framework used by Salesforce, Heroku, and others
• Smart bundling strategy: Using explicit command discovery allows bundling with tsdown while keeping @oclif/core external
• Proper separation of concerns: The prerun hook pattern cleanly handles cross-cutting concerns (banner, auth, env loading)
• Consistent patterns: BaseCommand class provides a clean abstraction for shared behavior

Code Quality

• Well-structured commands: Clean migration from functional to class-based command pattern
• Good use of TypeScript: Proper types for command options, flags, and args
• Comprehensive documentation: AGENTS.md is thoroughly updated with migration details
• Maintained existing utilities: @clack/prompts integration preserved for consistent UX

🔍 Issues & Recommendations

1. Critical: Command Discovery Mismatch

Severity: High

The documentation and configuration are inconsistent:

• package.json line 73-75 declares explicit strategy with commands exported from COMMANDS object
• src/cli/index.ts line 28 exports command IDs with colons: "entities:push", "site:deploy"
• But package.json line 83 sets topicSeparator: " " (space, not colon)

Impact: The commands may not work as expected. OCLIF will look for space-separated topics but the keys use colons.

Recommendation: Make the command keys match the topic separator:
```typescript
// src/cli/index.ts
export const COMMANDS = {
login: Login,
logout: Logout,
whoami: Whoami,
create: Create,
"entities push": EntitiesPush, // Use space to match topicSeparator
"site deploy": SiteDeploy, // Use space to match topicSeparator
};
```

Or alternatively, use the colon separator in package.json:
```json
"topicSeparator": ":"
```

2. Console.log in Production Code

Severity: Medium
Location: src/cli/lib/banner.ts:16

```typescript
export function printBanner(): void {
console.log(BANNER);
}
```

Issue: Direct console.log bypasses the custom logger infrastructure (@clack/prompts)

Recommendation:
```typescript
import { log } from "@clack/prompts";

export function printBanner(): void {
log.info(BANNER);
}
```

3. Process.exit Usage

Severity: Medium
Location: src/cli/commands/create.ts:61

```typescript
onCancel: () => {
cancel("Operation cancelled.");
process.exit(0);
}
```

Issue: Direct process.exit() bypasses OCLIF's exit handling and lifecycle hooks

Recommendation: Use OCLIF's exit method:
```typescript
onCancel: () => {
cancel("Operation cancelled.");
this.exit(0);
}
```

Note: This requires passing context to the onCancel handler. Alternative: throw an error and let BaseCommand's catch handler manage the exit.

4. Missing Error Handling in Hook

Severity: Low
Location: src/cli/hooks/prerun.ts:25

```typescript
await loadProjectEnv();
```

Issue: No error handling if env loading fails

Recommendation: Add try-catch with appropriate error handling:
```typescript
try {
await loadProjectEnv();
} catch (error) {
// Only log if it's an unexpected error, not just "no .env.local found"
if (process.env.DEBUG) {
log.warn(`Could not load environment: ${error.message}`);
}
}
```

5. Test Coverage Gap

Severity: Medium

Finding: Only one test file (tests/core/project.test.ts) exists. No tests for:

• New command classes
• Prerun hook behavior
• BaseCommand functionality
• Command flag/arg parsing

Recommendation: Add tests for the new OCLIF integration:

• Unit tests for each command
• Integration tests for hook execution
• Tests verifying flag/arg parsing works correctly

6. Documentation: Stale References

Severity: Low
Locations: AGENTS.md lines 115-116, 437-445

The documentation says commands are "discovered by file path" (line 115) but then correctly explains the explicit strategy (line 437-445). This is confusing.

Recommendation: Remove or clarify the file path discovery mention since explicit strategy doesn't discover by path.

🛡️ Security Review

✅ No security concerns identified:

• OAuth flow implementation looks correct
• Token refresh logic properly handles expired tokens
• No credential leakage in code
• Proper use of environment variables for sensitive data

🚀 Performance Considerations

✅ Good performance characteristics:

• Bundle size is reasonable (~750KB)
• Minimal runtime dependency (@oclif/core only)
• Token refresh is proactive (checks before requests)
• No obvious performance bottlenecks

📊 Migration Completeness

What was migrated well:

• ✅ All 6 commands converted to OCLIF classes
• ✅ Auth flow preserved
• ✅ Utility functions maintained
• ✅ Documentation updated
• ✅ Build configuration adapted

What needs attention:

• ⚠️ Command key naming (issue Add Claude Code GitHub Workflow #1 above)
• ⚠️ Test coverage for new patterns
• ⚠️ Minor code quality issues (chore(npm): change npm to and fix workflows #2, more everything to single repo #3)

🎯 Recommendations Summary

Must fix before merge:

1. ✅ Fix command discovery naming (entities:push vs "entities push")

Should fix before merge:
2. Replace console.log with log.info in banner.ts
3. Replace process.exit with this.exit in create.ts
4. Add error handling to loadProjectEnv in prerun hook

Can be addressed in follow-up:
5. Add test coverage for OCLIF integration
6. Clean up documentation inconsistencies

📝 Final Verdict

This is a well-executed migration that maintains the existing functionality while adopting a more robust framework. The architecture is sound, the code is clean, and the documentation is thorough.

The main blocker is the command discovery naming inconsistency which could cause runtime issues. Once that's resolved, this PR is ready to merge.

Recommendation: ✅ Approve with changes requested (fix issue #1)

---

Great work on this migration! The explicit command discovery strategy is a smart choice for bundling compatibility, and the prerun hook pattern is exactly how OCLIF is meant to be used. 🎉