♻️ refactor: give the remaining shared domain errors a kind (#862)

apps/api/src/app/auth/auth.controller.spec.ts

@@ -354,22 +354,10 @@ describe('AuthController', () => {
         );
       });
 
-      it('throws an HttpException', async () => {
-        await expect(
-          controller.signIn(signInRequest, mockResponse),
-        ).rejects.toThrow(HttpException);
-      });
-
-      it('maps the error to HTTP 401', async () => {
-        await expect(
-          controller.signIn(signInRequest, mockResponse),
-        ).rejects.toMatchObject({ status: HttpStatus.UNAUTHORIZED });
-      });
-
-      it('exposes the domain error message', async () => {
+      it('rethrows the domain error for the filter to answer 401', async () => {
         await expect(
           controller.signIn(signInRequest, mockResponse),
-        ).rejects.toThrow('Invalid email or password');
+        ).rejects.toBeInstanceOf(InvalidEmailOrPasswordError);
       });
 
       it('calls authService.signIn with the request', async () => {

apps/api/src/app/auth/auth.controller.ts

@@ -38,7 +38,6 @@ import {
   RequestPasswordResetCommand,
   RequestPasswordResetResponse,
   TooManyLoginAttemptsError,
-  ExpectedAuthError,
   CreateCliLoginCodeResponse,
   ExchangeCliLoginCodeCommand,
   ExchangeCliLoginCodeResponse,
@@ -152,30 +151,22 @@ export class AuthController {
 
       return result;
     } catch (error) {
-      // Expected auth errors are legitimate user-facing outcomes (wrong
-      // password, rate limit reached) — not application bugs. Log them at
-      // warn level without stack trace so Datadog error dashboards stay
-      // focused on real incidents.
-      // Kept by hand: no kind yields 401, nor 429 with bannedUntil in the
-      // body, so DomainExceptionFilter cannot produce these answers.
-      if (error instanceof ExpectedAuthError) {
+      // Kept by hand: no kind answers 429 with bannedUntil in the body, so
+      // DomainExceptionFilter cannot produce this answer. Invalid credentials
+      // carry `unauthenticated` and are answered 401 by the filter.
+      if (error instanceof TooManyLoginAttemptsError) {
         this.logger.warn(`POST /auth/signin - ${error.name}`, {
           email: maskEmail(signInRequest.email),
           reason: error.message,
         });
 
-        if (error instanceof TooManyLoginAttemptsError) {
-          throw new HttpException(
-            {
-              message: error.message,
-              bannedUntil: error.bannedUntil.toISOString(),
-            },
-            HttpStatus.TOO_MANY_REQUESTS,
-          );
-        }
-
-        // InvalidEmailOrPasswordError (and future ExpectedAuthError subclasses)
-        throw new HttpException(error.message, HttpStatus.UNAUTHORIZED);
+        throw new HttpException(
+          {
+            message: error.message,
+            bannedUntil: error.bannedUntil.toISOString(),
+          },
+          HttpStatus.TOO_MANY_REQUESTS,
+        );
       }
 
       throw error;

apps/cli/src/application/services/GitService.realGit.spec.ts

@@ -19,9 +19,18 @@ describe('GitService against real git', () => {
   let repoPath: string;
   let service: GitService;
 
+  // A git hook exports GIT_DIR and friends; inherited, they point every git
+  // call here at the repository running the hook instead of the temp repo
+  // (`init --bare` then flips its core.bare). Jest sandboxes process.env, so
+  // the clean env has to be passed to each call.
+  const env = Object.fromEntries(
+    Object.entries(process.env).filter(([key]) => !key.startsWith('GIT_')),
+  );
+
   const git = (args: string, cwd: string = repoPath) =>
     execSync(`git ${args}`, {
       cwd,
+      env,
       encoding: 'utf-8',
       stdio: ['pipe', 'pipe', 'pipe'],
     });
@@ -33,6 +42,7 @@ describe('GitService against real git', () => {
     root = mkdtempSync(path.join(tmpdir(), 'packmind-git-service-'));
     repoPath = path.join(root, 'repo');
     execSync(`git init -q -b main "${repoPath}"`, {
+      env,
       stdio: ['pipe', 'pipe', 'pipe'],
     });
     git('config user.email test@packmind.com');
@@ -41,7 +51,14 @@ describe('GitService against real git', () => {
     git('commit -q --allow-empty -m "initial commit"');
     git('branch feature');
 
-    service = new GitService();
+    service = new GitService(undefined, (cmd, opts) => ({
+      stdout: execSync(`git ${cmd}`, {
+        ...opts,
+        env,
+        encoding: 'utf-8',
+        stdio: ['pipe', 'pipe', 'pipe'],
+      }),
+    }));
   });
 
   afterEach(() => {
@@ -93,6 +110,7 @@ describe('GitService against real git', () => {
     it('reports the branch exists', () => {
       const remotePath = path.join(root, 'origin.git');
       execSync(`git init -q --bare "${remotePath}"`, {
+        env,
         stdio: ['pipe', 'pipe', 'pipe'],
       });
       git(`remote add origin "${remotePath}"`);

packages/accounts/src/application/useCases/createInvitations/CreateInvitationsUseCase.spec.ts

@@ -1,4 +1,7 @@
-import { UserNotFoundError } from '@packmind/node-utils';
+import {
+  MembershipOrganizationNotFoundError,
+  UserNotFoundError,
+} from '@packmind/node-utils';
 import {
   mockInterface,
   stubLogger,
@@ -664,7 +667,7 @@ describe('CreateInvitationsUseCase', () => {
     };
 
     await expect(useCase.execute(command)).rejects.toThrow(
-      `Organization ${organizationId} not found`,
+      MembershipOrganizationNotFoundError,
     );
   });
 

packages/accounts/src/domain/errors/AccountsError.spec.ts

@@ -18,6 +18,7 @@ import { CliLoginCodeUserNotFoundError } from './CliLoginCodeUserNotFoundError';
 import { CliLoginCodeMembershipNotFoundError } from './CliLoginCodeMembershipNotFoundError';
 import { CliLoginCodeOrganizationNotFoundError } from './CliLoginCodeOrganizationNotFoundError';
 import { CliLoginCodeApiKeyError } from './CliLoginCodeApiKeyError';
+import { InvalidEmailOrPasswordError } from './InvalidEmailOrPasswordError';
 
 describe('EmailAlreadyExistsError', () => {
   const error = new EmailAlreadyExistsError('test@example.com');
@@ -337,3 +338,23 @@ describe('CliLoginCodeApiKeyError', () => {
     expect(isDomainError(error)).toBe(false);
   });
 });
+
+describe('InvalidEmailOrPasswordError', () => {
+  const error = new InvalidEmailOrPasswordError();
+
+  it('is a domain error', () => {
+    expect(isDomainError(error)).toBe(true);
+  });
+
+  it('answers unauthenticated', () => {
+    expect(error.kind).toBe('unauthenticated');
+  });
+
+  it('carries the invalid_credentials reason', () => {
+    expect(error.reason).toBe('invalid_credentials');
+  });
+
+  it('keeps an empty context, so the log never names the account', () => {
+    expect(error.context).toEqual({});
+  });
+});

packages/accounts/src/domain/errors/AccountsError.ts

@@ -18,7 +18,8 @@ export type AccountsErrorReason =
   | 'user_cannot_change_own_role'
   | 'cannot_demote_last_admin'
   | 'invalid_authentication_type'
-  | 'user_creation_fields_required';
+  | 'user_creation_fields_required'
+  | 'invalid_credentials';
 
 export type AccountsErrorContext = {
   organizationId?: string;

packages/accounts/src/domain/errors/ExpectedAuthError.ts

@@ -1,10 +1,9 @@
 /**
- * Base class for expected authentication errors.
+ * Base class for expected authentication errors that no `kind` can answer yet.
  *
- * These represent legitimate user-facing outcomes (wrong password, rate limit
- * reached, etc.) — NOT application bugs. Callers (e.g. NestJS controllers,
- * exception filters) should log instances of this class at `warn` level
- * without stack traces, and map them to the appropriate HTTP response.
+ * Only `TooManyLoginAttemptsError` is left: it must answer 429 with
+ * `bannedUntil` in the body, which `DomainExceptionFilter` cannot produce, so
+ * the sign-in controller still maps it by hand and logs it at `warn`.
  */
 export abstract class ExpectedAuthError extends Error {
   protected constructor(message: string, name: string) {

packages/accounts/src/domain/errors/InvalidEmailOrPasswordError.ts

@@ -1,7 +1,17 @@
-import { ExpectedAuthError } from './ExpectedAuthError';
+import { AccountsError } from './AccountsError';
 
-export class InvalidEmailOrPasswordError extends ExpectedAuthError {
+/**
+ * One error for an unknown email, a wrong password and a social account with
+ * no password, so the answer never tells which of the three it was.
+ */
+export class InvalidEmailOrPasswordError extends AccountsError {
   constructor() {
-    super('Invalid email or password', 'InvalidEmailOrPasswordError');
+    super(
+      'unauthenticated',
+      'invalid_credentials',
+      {},
+      'Invalid email or password',
+    );
+    this.name = 'InvalidEmailOrPasswordError';
   }
 }

packages/deployments/src/application/jobs/PublishArtifactsDelayedJob.ts

@@ -7,7 +7,12 @@ import {
   SSEEventPublisher,
   WorkerListeners,
 } from '@packmind/node-utils';
-import { DistributionStatus, GitCommit, IGitPort } from '@packmind/types';
+import {
+  DistributionStatus,
+  GitCommit,
+  IGitPort,
+  NoChangesDetectedError,
+} from '@packmind/types';
 import { Job } from 'bullmq';
 import {
   distributionIdsOf,
@@ -91,7 +96,7 @@ export class PublishArtifactsDelayedJob extends AbstractAIDelayedJob<
         filesDeleted: input.fileUpdates.delete.length,
       });
     } catch (error) {
-      if (error instanceof Error && error.message === 'NO_CHANGES_DETECTED') {
+      if (error instanceof NoChangesDetectedError) {
         this.logger.info(
           `[${this.origin}] No changes detected for distributions ${distributionIds.join(', ')}`,
         );

packages/deployments/src/application/useCases/RemovePackageFromTargetsUseCase.spec.ts

@@ -44,6 +44,7 @@ import {
   GitRepo,
   createGitProviderId,
   createGitCommitId,
+  NoChangesDetectedError,
 } from '@packmind/types';
 
 describe('RemovePackageFromTargetsUseCase', () => {
@@ -415,7 +416,7 @@ describe('RemovePackageFromTargetsUseCase', () => {
         beforeEach(() => {
           mockDistributionRepository.listByTargetIds.mockResolvedValue([]);
           mockGitPort.commitToGit.mockRejectedValue(
-            new Error('NO_CHANGES_DETECTED'),
+            new NoChangesDetectedError(),
           );
         });
 

packages/deployments/src/application/useCases/RemovePackageFromTargetsUseCase.ts

@@ -32,6 +32,7 @@ import {
   FileUpdates,
   CodingAgent,
   RenderMode,
+  NoChangesDetectedError,
 } from '@packmind/types';
 import { PackageService } from '../services/PackageService';
 import { TargetService } from '../services/TargetService';
@@ -144,10 +145,7 @@ export class RemovePackageFromTargetsUseCase implements IRemovePackageFromTarget
             firstTargetData.fileUpdates.delete,
           );
         } catch (error) {
-          if (
-            error instanceof Error &&
-            error.message === 'NO_CHANGES_DETECTED'
-          ) {
+          if (error instanceof NoChangesDetectedError) {
             this.logger.info('No changes detected for package removal', {
               repositoryId,
               packageSlug: pkg.slug,

packages/git/src/application/useCases/commitToGit/CommitToGitUseCase.spec.ts

@@ -10,6 +10,7 @@ import {
   GitProviderNotFoundError,
   GitProviderVendor,
   GitProviderVendors,
+  NoChangesDetectedError,
   NoFilesToCommitError,
 } from '@packmind/types';
 import { IGitRepo } from '../../../domain/repositories/IGitRepo';
@@ -200,6 +201,40 @@ describe('CommitToGitUseCase', () => {
       ).rejects.toBeInstanceOf(NoFilesToCommitError);
     });
 
+    describe('when the provider reports no changes', () => {
+      const commit = () =>
+        commitToGit.commitToGit(
+          mockGitRepo,
+          [{ path: 'test/file1.txt', content: 'unchanged' }],
+          'Commit message',
+        );
+
+      beforeEach(() => {
+        mockGitProviderRepository.findById.mockResolvedValue(mockGitProvider);
+        mockGithubRepository.commitFiles.mockResolvedValue({
+          sha: 'no-changes',
+          message: 'Commit message',
+          author: 'test@example.com',
+          url: '',
+        });
+      });
+
+      it('throws NoChangesDetectedError', async () => {
+        await expect(commit()).rejects.toBeInstanceOf(NoChangesDetectedError);
+      });
+
+      // Callers outside this repo still match on the message.
+      it('keeps the NO_CHANGES_DETECTED message', async () => {
+        await expect(commit()).rejects.toThrow(/^NO_CHANGES_DETECTED$/);
+      });
+
+      it('does not record a commit', async () => {
+        await commit().catch(() => undefined);
+
+        expect(mockGitCommitService.addCommit).not.toHaveBeenCalled();
+      });
+    });
+
     describe('when deleteFiles parameter is provided', () => {
       it('passes deleteFiles parameter to commitFiles', async () => {
         const files = [{ path: 'test/file.txt', content: 'test content' }];

packages/git/src/application/useCases/commitToGit/CommitToGitUseCase.ts

@@ -4,6 +4,7 @@ import {
   GitCommit,
   DeleteItem,
   DeleteItemType,
+  NoChangesDetectedError,
   NoFilesToCommitError,
 } from '@packmind/types';
 import { CommitFile } from '../../../domain/repositories/IGitRepo';
@@ -143,9 +144,11 @@ export class CommitToGitUseCase {
         repo: repo.repo,
         fileCount: files.length,
       });
-      // Sentinel: deployment logic catches this rather than treating it as a
-      // failure.
-      throw new Error('NO_CHANGES_DETECTED');
+      throw new NoChangesDetectedError({
+        gitRepoId: repo.id,
+        owner: repo.owner,
+        repo: repo.repo,
+      });
     }
 
     return this.gitCommitService.addCommit(commitData);

packages/linter-ast/src/application/ConsoleLogRemovalService.spec.ts

@@ -1,5 +1,6 @@
 import { ConsoleLogRemovalService } from './ConsoleLogRemovalService';
 import { ProgrammingLanguage } from '@packmind/types';
+import { LinterAstInternalError } from '../core/LinterAstInternalError';
 
 describe('ConsoleLogRemovalService', () => {
   let consoleLogRemovalService: ConsoleLogRemovalService;
@@ -17,9 +18,7 @@ describe('ConsoleLogRemovalService', () => {
           program,
           ProgrammingLanguage.TYPESCRIPT,
         ),
-      ).rejects.toThrow(
-        'ConsoleLogRemovalService only supports JAVASCRIPT, received: TYPESCRIPT',
-      );
+      ).rejects.toBeInstanceOf(LinterAstInternalError);
     });
 
     it('throws error for Python language', async () => {
@@ -30,9 +29,7 @@ describe('ConsoleLogRemovalService', () => {
           program,
           ProgrammingLanguage.PYTHON,
         ),
-      ).rejects.toThrow(
-        'ConsoleLogRemovalService only supports JAVASCRIPT, received: PYTHON',
-      );
+      ).rejects.toBeInstanceOf(LinterAstInternalError);
     });
   });