Kevin/improved errors

CLAUDE.md

@@ -184,4 +184,27 @@ Parent Process              Plugin Process
 3. **Plugin IPC**: Plugins cannot directly read stdin (security isolation)
 4. **Sudo Caching**: Password cached in memory during session unless `--secure` flag used
 5. **File Watcher**: Use `persistent: false` option to prevent hanging processes
-6. **Linting**: ESLint enforces single quotes, specific import ordering, and strict type safety
+6. **Linting**: ESLint enforces single quotes, specific import ordering, and strict type safety
+7. **Reporter display methods are async**: All `Reporter` interface display methods (`displayPlan`, `displayImportResult`, `displayFileModifications`, `displayMessage`, `displayPluginError`) return `Promise<void>`. Always `await` them at call sites — `DefaultReporter.updateRenderState()` has a 50ms sleep, so unawaited calls cause `process.exit(1)` to fire before the UI renders.
+8. **Mock reporter async assertions**: Assertions inside `MockReporter` config callbacks (e.g. `displayFileModifications`) will silently pass if the call isn't awaited. Making display methods async surfaced latent bugs where expected file paths were wrong.
+
+## Plugin Error Handling Architecture
+
+Plugin errors flow as structured `PluginErrorData` over IPC and are caught as `PluginError` instances on the CLI side:
+
+**IPC envelope** (`@codifycli/schemas`):
+```typescript
+interface PluginErrorData {
+  errorType: string;   // 'apply_validation' | 'sudo_error' | 'unknown'
+  message: string;
+  data?: unknown;
+}
+```
+
+**CLI carrier** (`src/common/errors.ts`): `PluginError extends CodifyError` holds `pluginName`, `resourceType`, and `errorData: PluginErrorData`.
+
+**Reporter as view model**: Reporters (not components) decide how to render each `errorType`. `DefaultReporter.displayPluginError()` branches on `errorType` to set the appropriate `RenderStatus` (`APPLY_VALIDATION_ERROR` with a `ResourcePlan` for plan diffs, `PLUGIN_ERROR` with a message string for generic errors). The `DefaultComponent` is purely display.
+
+**Shared formatter**: `src/ui/plugin-error-formatter.ts` exports `formatApplyValidationError(error: PluginError): string` used by both `PlainReporter` and `DefaultComponent`.
+
+**Backward compat**: `plugin.ts#toErrorData()` validates IPC data against `ErrorResponseDataSchema` (AJV); falls back to `{ errorType: 'unknown', message: data }` for old plugins sending bare strings.

package.json

@@ -5,7 +5,7 @@
   },
   "dependencies": {
     "@codifycli/ink-form": "0.0.12",
-    "@codifycli/schemas": "1.1.0-beta5",
+    "@codifycli/schemas": "1.1.0-beta8",
     "@homebridge/node-pty-prebuilt-multiarch": "^0.12.0-beta.5",
     "@mischnic/json-sourcemap": "^0.1.1",
     "@oclif/core": "^4.0.8",
@@ -43,7 +43,7 @@
   },
   "description": "Codify is a configuration-as-code tool that declaratively installs and manages developer tools and applications. Check out https://dashboard.codifycli.com for an editor.",
   "devDependencies": {
-    "@codifycli/plugin-core": "^1.1.0-beta13",
+    "@codifycli/plugin-core": "^1.1.0-beta19",
     "@oclif/prettier-config": "^0.2.1",
     "@types/chalk": "^2.2.0",
     "@types/cors": "^2.8.19",

src/common/base-command.ts

@@ -11,7 +11,7 @@ import { DefaultReporter } from '../ui/reporters/default-reporter.js';
 import { Reporter, ReporterFactory, ReporterType } from '../ui/reporters/reporter.js';
 import { spawnSafe } from '../utils/spawn.js';
 import { SudoUtils } from '../utils/sudo.js';
-import { prettyPrintError } from './errors.js';
+import { PluginError, prettyPrintError } from './errors.js';
 
 export abstract class BaseCommand extends Command {
   static baseFlags = {
@@ -145,6 +145,12 @@ export abstract class BaseCommand extends Command {
   }
 
   protected async catch(err: Error): Promise<void> {
+    if (err instanceof PluginError && this.reporter) {
+      await this.reporter.hide();
+      await this.reporter.displayPluginError(err);
+      process.exit(1);
+    }
+
     prettyPrintError(err);
     process.exit(1);
   }

src/common/errors.ts

@@ -1,5 +1,6 @@
 import { ErrorObject } from 'ajv';
 import chalk from 'chalk';
+import { PluginErrorData } from '@codifycli/schemas';
 
 import { ResourceConfig } from '../entities/resource-config.js';
 import { SourceMapCache } from '../parser/source-maps.js';
@@ -231,6 +232,24 @@ export class SpawnError extends CodifyError {
   }
 }
 
+export class PluginError extends CodifyError {
+  name = 'PluginError';
+  pluginName: string;
+  resourceType: string;
+  errorData: PluginErrorData;
+
+  constructor(pluginName: string, resourceType: string, errorData: PluginErrorData) {
+    super(errorData.message);
+    this.pluginName = pluginName;
+    this.resourceType = resourceType;
+    this.errorData = errorData;
+  }
+
+  formattedMessage(): string {
+    return this.message;
+  }
+}
+
 export function prettyPrintError(error: unknown): void {
   if (error instanceof CodifyError) {
     return console.error(chalk.red(error.formattedMessage()));

src/orchestrators/apply.ts

@@ -50,7 +50,7 @@ export const ApplyOrchestrator = {
     // Need to sleep to wait for the message to display before we exit
     await sleep(100);
 
-    reporter.displayMessage(`
+    await reporter.displayMessage(`
 🎉 Finished applying 🎉
 Open a new terminal or source '.zshrc' for the new changes to be reflected`);
   },

src/orchestrators/destroy.ts

@@ -42,7 +42,7 @@ export class DestroyOrchestrator {
     plan.sortByEvalOrder(project.evaluationOrder);
     destroyProject.removeNoopFromEvaluationOrder(plan);
 
-    reporter.displayPlan(plan);
+    await reporter.displayPlan(plan);
 
     // Short circuit and exit if every change is NOOP
     if (plan.isEmpty()) {

src/orchestrators/import.ts

@@ -116,7 +116,7 @@ export class ImportOrchestrator {
 
     ctx.processFinished(ProcessName.IMPORT)
 
-    reporter.displayImportResult(importResult, false);
+    await reporter.displayImportResult(importResult, false);
 
     resourceInfoList.push(...(await pluginManager.getMultipleResourceInfo(
       project.resourceConfigs.map((r) => r.type)
@@ -194,8 +194,8 @@ export class ImportOrchestrator {
     }
 
     // No writes
-    reporter.displayImportResult(importResult, true);
-    reporter.displayMessage('\n🎉 Imported completed 🎉')
+    await reporter.displayImportResult(importResult, true);
+    await reporter.displayMessage('\n🎉 Imported completed 🎉')
 
     await sleep(100);
   }
@@ -244,17 +244,17 @@ export class ImportOrchestrator {
 
     // No changes to be made
     if (diffs.every((d) => d.modification.diff === '')) {
-      reporter.displayMessage('\nNo changes are needed! Exiting...')
+      await reporter.displayMessage('\nNo changes are needed! Exiting...')
 
       // Wait for the message to display before we exit
       await sleep(100);
       return;
     }
 
-    reporter.displayFileModifications(diffs);
+    await reporter.displayFileModifications(diffs);
     const shouldSave = await reporter.promptConfirmation('Save the changes?');
     if (!shouldSave) {
-      reporter.displayMessage('\nSkipping save! Exiting...');
+      await reporter.displayMessage('\nSkipping save! Exiting...');
 
       // Wait for the message to display before we exit
       await sleep(100);
@@ -265,7 +265,7 @@ export class ImportOrchestrator {
       await FileUpdater.write(diff.file, diff.modification.newFile);
     }
 
-    reporter.displayMessage('\n🎉 Imported completed and saved to file 🎉');
+    await reporter.displayMessage('\n🎉 Imported completed and saved to file 🎉');
 
     // Wait for the message to display before we exit
     await sleep(100);
@@ -448,11 +448,11 @@ ${JSON.stringify(unsupportedTypeIds)}`);
     const newFile = JSON.stringify(importResult.result.map((r) => r.raw), null, 2);
     const diff = prettyFormatFileDiff('', newFile);
 
-    reporter.displayFileModifications([{ file: filePath, modification: { newFile, diff } }]);
+    await reporter.displayFileModifications([{ file: filePath, modification: { newFile, diff } }]);
 
     const shouldSave = await reporter.promptConfirmation(`Save the changes? (${filePath})`);
     if (!shouldSave) {
-      reporter.displayMessage('\nSkipping save! Exiting...');
+      await reporter.displayMessage('\nSkipping save! Exiting...');
 
       // Wait for the message to display before we exit
       await sleep(100);
@@ -461,7 +461,7 @@ ${JSON.stringify(unsupportedTypeIds)}`);
 
     await FileUpdater.write(filePath, newFile);
 
-    reporter.displayMessage('\n🎉 Imported completed and saved to file 🎉');
+    await reporter.displayMessage('\n🎉 Imported completed and saved to file 🎉');
 
     // Wait for the message to display before we exit
     await sleep(100);

src/orchestrators/plan.ts

@@ -48,7 +48,7 @@ export class PlanOrchestrator {
     if (!args.noProgress) ctx.processFinished(ProcessName.PLAN)
 
     await reporter.hide();
-    reporter.displayPlan(plan);
+    await reporter.displayPlan(plan);
 
     return {
       plan,

src/orchestrators/refresh.ts

@@ -35,7 +35,7 @@ export class RefreshOrchestrator {
 
     ctx.processFinished(ProcessName.REFRESH);
 
-    reporter.displayImportResult(importResult, false);
+    await reporter.displayImportResult(importResult, false);
 
 
     // Special handling for remote-file resources. Offer to save them remotely if any changes are detected on import.

src/orchestrators/test.ts

@@ -130,7 +130,7 @@ export const TestOrchestrator = {
 
     // Short circuit and exit if every change is NOOP
     if (!planResult.plan.isEmpty()) {
-      reporter.displayPlan(planResult.plan);
+      await reporter.displayPlan(planResult.plan);
       const confirm = await reporter.promptConfirmation('The following resources will need to be installed (Tart VM - 25gb). Do you want to continue?')
       if (!confirm) {
         return process.exit(0);