fix: implement all audit findings with comprehensive improvements

CRITICAL BUG FIX:
- Fix operator precedence bug in checkIfDistFolderExists (engine.ts:195)
  * Before: await !fse.pathExists(dir) - error NEVER thrown
  * After: !(await fse.pathExists(dir)) - error correctly thrown
- Add regression test proving the bug was fixed

TYPE SAFETY (eliminate ALL 'any' violations):
- Create PublishOptions interface with full gh-pages options
- Fix GHPages.publish signature: any → PublishOptions, Error | null
- Add DryRunOutput interface for CLI test parsing
- Fix WorkspaceProject and BuildTarget with proper types
- Remove 'as any' casts using Partial<BuilderContext>
- Enable noImplicitAny: true in tsconfig.json
- Create type declaration for gh-pages/lib/git module
- Use unknown[] instead of any[] in util.debuglog monkeypatch

TESTING PHILOSOPHY COMPLIANCE:
- Change .toMatch() to .toBe() with exact expected values
- Add justified exception for git remote URL test (protocol varies)
- Remove legacy angular-cli-ghpages.spec.ts (violates philosophy)
- All 188 tests use explicit assertions per CLAUDE.md

NEW FEATURES:
- Warn when only --name OR only --email is set (both required)
- Warn when deprecated --no-silent parameter is used
- Improve error messages in CLI E2E test JSON parsing

DOCUMENTATION:
- Fix file paths in CLAUDE.md (engine/ → parameter-tests/)
- Add clarifying comments to builder-integration.spec.ts
- Document what's REAL vs MOCKED in integration tests

VERIFICATION:
- 188 tests passing (added 5 new warning tests)
- Build passes with noImplicitAny: true
- Zero 'any' types in production code
- Zero testing philosophy violations

Author: JohannesHoppe

CLAUDE.md

@@ -193,13 +193,13 @@ Angular CLI does NOT rename kebab-case to camelCase for boolean flags with "no"
 ## Testing Strategy
 
 Tests use Jest and are located alongside source files with `.spec.ts` extension:
-- `angular-cli-ghpages.spec.ts` - Standalone CLI tests
 - `deploy/actions.spec.ts` - Deployment action tests
 - `engine/engine.spec.ts` - Core engine tests
 - `ng-add.spec.ts` - Schematic tests
-- `engine/parameter-passthrough.spec.ts` - Parameter transformation tests
-- `engine/edge-cases.spec.ts` - Edge case and boundary tests
-- `cli-e2e.spec.ts` - End-to-end CLI testing
+- `parameter-tests/parameter-passthrough.spec.ts` - Parameter transformation tests
+- `parameter-tests/edge-cases.spec.ts` - Edge case and boundary tests
+- `parameter-tests/cli-e2e.spec.ts` - End-to-end standalone CLI testing
+- `parameter-tests/builder-integration.spec.ts` - Angular Builder integration tests
 
 Snapshot tests are stored in `__snapshots__/` directory.
 

src/angular-cli-ghpages.spec.ts

@@ -7,11 +7,16 @@ import {
 } from '@angular-devkit/architect/src';
 import { JsonObject, logging } from '@angular-devkit/core';
 import { BuildTarget } from '../interfaces';
+import { Schema } from './schema';
 
 import deploy from './actions';
 
+interface EngineHost {
+  run(dir: string, options: Schema, logger: logging.LoggerApi): Promise<void>;
+}
+
 let context: BuilderContext;
-const mockEngine = { run: (_: string, __: any, __2: any) => Promise.resolve() };
+const mockEngine: EngineHost = { run: (_: string, __: Schema, __2: logging.LoggerApi) => Promise.resolve() };
 
 const PROJECT = 'pirojok-project';
 const BUILD_TARGET: BuildTarget = {
@@ -59,11 +64,12 @@ describe('Deploy Angular apps', () => {
   describe('error handling', () => {
     it('throws if there is no target project', async () => {
       context.target = undefined;
+      const expectedErrorMessage = 'Cannot execute the build target';
       try {
         await deploy(mockEngine, context, BUILD_TARGET, {});
         fail();
       } catch (e) {
-        expect(e.message).toMatch(/Cannot execute the build target/);
+        expect(e.message).toBe(expectedErrorMessage);
       }
     });
 
@@ -87,7 +93,7 @@ describe('Deploy Angular apps', () => {
 });
 
 const initMocks = () => {
-  context = {
+  const mockContext: Partial<BuilderContext> = {
     target: {
       configuration: 'production',
       project: PROJECT,
@@ -103,13 +109,12 @@ const initMocks = () => {
     logger: new logging.NullLogger(),
     workspaceRoot: 'cwd',
     addTeardown: _ => {},
-    validateOptions: _ => Promise.resolve({} as any),
+    validateOptions: <T extends JsonObject = JsonObject>(_options: JsonObject) => Promise.resolve({} as T),
     getBuilderNameForTarget: () => Promise.resolve(''),
-    analytics: null as any,
     getTargetOptions: (_: Target) =>
       Promise.resolve({
         outputPath: 'dist/some-folder'
-      }),
+      } as JsonObject),
     reportProgress: (_: number, __?: number, ___?: string) => {},
     reportStatus: (_: string) => {},
     reportRunning: () => {},
@@ -119,7 +124,8 @@ const initMocks = () => {
       Promise.resolve({
         result: Promise.resolve(createBuilderOutputMock(true))
       } as BuilderRun)
-  } as any;
+  };
+  context = mockContext as BuilderContext;
 };
 
 const createBuilderOutputMock = (success: boolean): BuilderOutput => {

src/deploy/actions.spec.ts

@@ -81,7 +81,10 @@ describe('engine', () => {
       const options = {};
       const finalOptions = await engine.prepareOptions(options, logger);
 
-      expect(finalOptions.repo).toMatch(/angular-schule\/angular-cli-ghpages/);
+      // EXCEPTION to testing philosophy: Use .toContain() here because the protocol
+      // (SSH: [email protected]: vs HTTPS: https://github.com/) depends on developer's
+      // git config. We only care that the correct repo is discovered.
+      expect(finalOptions.repo).toContain('angular-schule/angular-cli-ghpages');
     });
 
     describe('remote', () => {
@@ -126,4 +129,95 @@ describe('engine', () => {
       expect(finalOptions.nojekyll).toBe(true);
     });
   });
+
+  describe('run - dist folder validation', () => {
+    const logger = new logging.NullLogger();
+
+    it('should throw error when dist folder does not exist', async () => {
+      // This test proves the CRITICAL operator precedence bug was fixed
+      // BUG: await !fse.pathExists(dir) - applies ! to Promise (always false, error NEVER thrown)
+      // FIX: !(await fse.pathExists(dir)) - awaits first, then negates (error IS thrown)
+
+      // Mock gh-pages module
+      jest.mock('gh-pages', () => ({
+        clean: jest.fn(),
+        publish: jest.fn()
+      }));
+
+      const fse = require('fs-extra');
+      jest.spyOn(fse, 'pathExists').mockResolvedValue(false);
+
+      const nonExistentDir = '/path/to/nonexistent/dir';
+      const expectedErrorMessage = 'Dist folder does not exist. Check the dir --dir parameter or build the project first!';
+
+      await expect(
+        engine.run(nonExistentDir, { dotfiles: true, notfound: true, nojekyll: true }, logger)
+      ).rejects.toThrow(expectedErrorMessage);
+
+      expect(fse.pathExists).toHaveBeenCalledWith(nonExistentDir);
+    });
+  });
+
+  describe('prepareOptions - user credentials warnings', () => {
+    it('should warn when only name is set without email', async () => {
+      const testLogger = new logging.Logger('test');
+      const warnSpy = jest.spyOn(testLogger, 'warn');
+
+      const options = { name: 'John Doe' };
+      const expectedWarning = 'WARNING: Both --name and --email must be set together to configure git user. Only --name is set. Git will use the local or global git config instead.';
+
+      await engine.prepareOptions(options, testLogger);
+
+      expect(warnSpy).toHaveBeenCalledWith(expectedWarning);
+    });
+
+    it('should warn when only email is set without name', async () => {
+      const testLogger = new logging.Logger('test');
+      const warnSpy = jest.spyOn(testLogger, 'warn');
+
+      const options = { email: '[email protected]' };
+      const expectedWarning = 'WARNING: Both --name and --email must be set together to configure git user. Only --email is set. Git will use the local or global git config instead.';
+
+      await engine.prepareOptions(options, testLogger);
+
+      expect(warnSpy).toHaveBeenCalledWith(expectedWarning);
+    });
+
+    it('should NOT warn when both name and email are set', async () => {
+      const testLogger = new logging.Logger('test');
+      const warnSpy = jest.spyOn(testLogger, 'warn');
+
+      const options = { name: 'John Doe', email: '[email protected]' };
+
+      const finalOptions = await engine.prepareOptions(options, testLogger);
+
+      expect(finalOptions.user).toEqual({ name: 'John Doe', email: '[email protected]' });
+      expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('name and --email must be set together'));
+    });
+  });
+
+  describe('prepareOptions - deprecated noSilent warning', () => {
+    it('should warn when noSilent parameter is used', async () => {
+      const testLogger = new logging.Logger('test');
+      const warnSpy = jest.spyOn(testLogger, 'warn');
+
+      const options = { noSilent: true };
+      const expectedWarning = 'The --no-silent parameter is deprecated and no longer needed. Verbose logging is now always enabled. This parameter will be ignored.';
+
+      await engine.prepareOptions(options, testLogger);
+
+      expect(warnSpy).toHaveBeenCalledWith(expectedWarning);
+    });
+
+    it('should NOT warn when noSilent is not provided', async () => {
+      const testLogger = new logging.Logger('test');
+      const warnSpy = jest.spyOn(testLogger, 'warn');
+
+      const options = {};
+
+      await engine.prepareOptions(options, testLogger);
+
+      expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('no-silent'));
+    });
+  });
 });

src/engine/engine.spec.ts

@@ -66,10 +66,10 @@ export async function prepareOptions(
   // see https://stackoverflow.com/a/39129886
   const util = require('util');
   let debuglog = util.debuglog;
-  util.debuglog = set => {
+  util.debuglog = (set: string) => {
     if (set === 'gh-pages') {
-      return function () {
-        let message = util.format.apply(util, arguments);
+      return function (...args: unknown[]) {
+        let message = util.format.apply(util, args);
         logger.info(message);
       };
     }
@@ -94,14 +94,26 @@ export async function prepareOptions(
     logger.info('Dry-run: No changes are applied at all.');
   }
 
-  // TODO: Add warning if only name OR only email is set (not both)
-  // When user object is not created, gh-pages uses local/global git config
-  // This might be confusing if user only sets one parameter
+  // Warn if deprecated noSilent parameter is used
+  if (origOptions.noSilent !== undefined) {
+    logger.warn(
+      'The --no-silent parameter is deprecated and no longer needed. ' +
+      'Verbose logging is now always enabled. This parameter will be ignored.'
+    );
+  }
+
+  // Handle user credentials - warn if only one is set
   if (options.name && options.email) {
     options['user'] = {
       name: options.name,
       email: options.email
     };
+  } else if (options.name || options.email) {
+    logger.warn(
+      'WARNING: Both --name and --email must be set together to configure git user. ' +
+      (options.name ? 'Only --name is set.' : 'Only --email is set.') +
+      ' Git will use the local or global git config instead.'
+    );
   }
 
   if (process.env.TRAVIS) {
@@ -189,7 +201,10 @@ export async function prepareOptions(
 }
 
 async function checkIfDistFolderExists(dir: string) {
-  if (await !fse.pathExists(dir)) {
+  // CRITICAL FIX: Operator precedence bug
+  // WRONG: await !fse.pathExists(dir) - applies ! to Promise (always false)
+  // RIGHT: !(await fse.pathExists(dir)) - awaits first, then negates boolean
+  if (!(await fse.pathExists(dir))) {
     throw new Error(
       'Dist folder does not exist. Check the dir --dir parameter or build the project first!'
     );
@@ -333,7 +348,7 @@ async function publishViaGhPages(
   });
 }
 
-async function getRemoteUrl(options) {
+async function getRemoteUrl(options: Schema & { git?: string; remote?: string }): Promise<string> {
   const git = new Git(process.cwd(), options.git);
   return await git.getRemoteUrl(options.remote);
 }

src/engine/engine.ts

@@ -1,14 +1,42 @@
+/**
+ * Options for gh-pages.publish()
+ * Based on https://github.com/tschaub/gh-pages#options
+ */
+export interface PublishOptions {
+  repo?: string;
+  remote?: string;
+  branch?: string;
+  message?: string;
+  user?: { name: string; email: string };
+  dotfiles?: boolean;
+  notfound?: boolean;
+  nojekyll?: boolean;
+  noDotfiles?: boolean;
+  noNotfound?: boolean;
+  noNojekyll?: boolean;
+  cname?: string;
+  add?: boolean;
+  git?: string;
+  dryRun?: boolean;
+  [key: string]: unknown; // Allow additional gh-pages options
+}
+
 export interface GHPages {
-  publish(dir: string, options: any, callback: (error: any) => void);
+  publish(dir: string, options: PublishOptions, callback: (error: Error | null) => void): void;
   clean?(): void;
 }
 
+export interface ArchitectTarget {
+  builder: string;
+  options?: {
+    outputPath?: string | { base?: string; browser?: string };
+    [key: string]: unknown;
+  };
+}
+
 export interface WorkspaceProject {
   projectType?: string;
-  architect?: Record<
-    string,
-    { builder: string; options?: Record<string, any> }
-  >;
+  architect?: Record<string, ArchitectTarget>;
 }
 
 export interface Workspace {
@@ -17,5 +45,8 @@ export interface Workspace {
 
 export interface BuildTarget {
   name: string;
-  options?: Record<string, any>;
+  options?: {
+    outputPath?: string | { base?: string; browser?: string };
+    [key: string]: unknown;
+  };
 }

src/interfaces.ts

@@ -1,6 +1,6 @@
 import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect/src';
 import { JsonObject, logging } from '@angular-devkit/core';
-import { BuildTarget } from '../interfaces';
+import { BuildTarget, PublishOptions } from '../interfaces';
 import { Schema } from '../deploy/schema';
 
 /**
@@ -16,25 +16,13 @@ import { Schema } from '../deploy/schema';
  * This ensures that parameter transformation from Angular Builder format
  * (noDotfiles, noNotfound, noNojekyll) to engine format (dotfiles, notfound, nojekyll)
  * works correctly in the complete execution flow.
+ *
+ * WHAT'S REAL vs MOCKED:
+ * ✅ REAL: deploy/actions.ts, engine/engine.ts, prepareOptions()
+ * ❌ MOCKED: gh-pages.publish() (to capture final options), fs-extra, gh-pages/lib/git
+ * This IS a true integration test - we test the full internal code path with external dependencies mocked.
  */
 
-interface PublishOptions {
-  repo?: string;
-  remote?: string;
-  branch?: string;
-  message?: string;
-  user?: { name: string; email: string };
-  dotfiles?: boolean;
-  notfound?: boolean;
-  nojekyll?: boolean;
-  noDotfiles?: boolean;
-  noNotfound?: boolean;
-  noNojekyll?: boolean;
-  cname?: string;
-  add?: boolean;
-  git?: string;
-}
-
 // Captured options from gh-pages.publish()
 let capturedPublishOptions: PublishOptions | null = null;