fix: validate rollout input and add comprehensive test coverage for core utilities and CLI logic

Author: sunnylqm

src/api.ts

@@ -98,7 +98,13 @@ async function query(url: string, options: fetch.RequestInit) {
   let json: any;
   try {
     json = JSON.parse(text);
-  } catch (e) {}
+  } catch (e) {
+    if (resp.status === 200 && text) {
+      console.warn(
+        `Warning: API returned 200 with non-JSON body (${text.length} bytes)`,
+      );
+    }
+  }
 
   if (resp.status !== 200) {
     const message = json?.message || resp.statusText || `HTTP ${resp.status}`;

src/app.ts

@@ -112,7 +112,7 @@ export const appCommands = {
     options: { platform: Platform };
   }) => {
     const { platform } = options;
-    const id = args[0] || chooseApp(platform);
+    const id = args[0] || (await chooseApp(platform)).id;
     if (!id) {
       console.log(t('cancelled'));
     }
@@ -139,7 +139,9 @@ export const appCommands = {
       Record<Platform, { appId: number; appKey: string }>
     > = {};
     try {
-      updateInfo = JSON.parse(await fs.promises.readFile('update.json', 'utf8'));
+      updateInfo = JSON.parse(
+        await fs.promises.readFile('update.json', 'utf8'),
+      );
     } catch (e: any) {
       if (e.code !== 'ENOENT') {
         console.error(t('failedToParseUpdateJson'));

src/diff.ts

@@ -19,41 +19,12 @@ import {
 } from './utils/zip-options';
 
 type Diff = (oldSource?: Buffer, newSource?: Buffer) => Buffer;
-type HpatchCover = {
-  oldPos: number | string | bigint;
-  newPos: number | string | bigint;
-  len: number | string | bigint;
-};
-type HpatchCompatiblePlan = {
-  covers?: HpatchCover[];
-};
-type HdiffWithCoversOptions = {
-  mode?: 'replace' | 'merge' | 'native-coalesce';
-};
 type HdiffModule = {
   diff?: Diff;
-  diffWithCovers?: (
-    oldSource: Buffer,
-    newSource: Buffer,
-    covers: HpatchCover[],
-    options?: HdiffWithCoversOptions,
-  ) => { diff?: Buffer };
 };
 type BsdiffModule = {
   diff?: Diff;
 };
-type ChiffModule = {
-  hpatchCompatiblePlanResult?: (
-    oldSource: Buffer,
-    newSource: Buffer,
-  ) => HpatchCompatiblePlan;
-  hpatchApproximatePlanResult?: (
-    oldSource: Buffer,
-    newSource: Buffer,
-  ) => HpatchCompatiblePlan;
-};
-type ChiffHpatchPolicy = 'off' | 'costed';
-type ChiffHpatchExactPolicy = 'off' | 'on';
 type EntryMap = Record<string, { crc32: number; fileName: string }>;
 type CrcMap = Record<number, string>;
 type CopyMap = Record<string, string>;
@@ -86,121 +57,6 @@ const loadModule = <T>(pkgName: string): T | undefined => {
 
 const hdiff = loadModule<HdiffModule>('node-hdiffpatch');
 const bsdiff = loadModule<BsdiffModule>('node-bsdiff');
-const chiff = loadModule<ChiffModule>('@chiff/node');
-
-// Structured covers are experimental and can be expensive on real Hermes input.
-// Keep native hdiff as the default unless the server explicitly opts in.
-function resolveChiffHpatchPolicy(policy?: unknown): ChiffHpatchPolicy {
-  const value = String(
-    policy ?? process.env.RNU_CHIFF_HPATCH_POLICY ?? 'off',
-  ).toLowerCase();
-  if (
-    value === 'costed' ||
-    value === 'on' ||
-    value === 'true' ||
-    value === '1'
-  ) {
-    return 'costed';
-  }
-  return 'off';
-}
-
-function resolveChiffHpatchMinNativeBytes(value?: unknown): number {
-  const raw = value ?? process.env.RNU_CHIFF_HPATCH_MIN_NATIVE_BYTES ?? 4096;
-  const parsed = Number(raw);
-  if (!Number.isFinite(parsed) || parsed < 0) {
-    return 4096;
-  }
-  return Math.floor(parsed);
-}
-
-function resolveChiffHpatchExactPolicy(policy?: unknown): ChiffHpatchExactPolicy {
-  const value = String(
-    policy ?? process.env.RNU_CHIFF_HPATCH_EXACT_COVERS ?? 'off',
-  ).toLowerCase();
-  if (value === 'on' || value === 'true' || value === '1') {
-    return 'on';
-  }
-  return 'off';
-}
-
-function createChiffAwareHdiff(
-  hdiffModule: HdiffModule,
-  chiffModule: ChiffModule | undefined,
-  policy: ChiffHpatchPolicy,
-  minNativeBytes: number,
-  exactPolicy: ChiffHpatchExactPolicy,
-): Diff {
-  const baseDiff = hdiffModule.diff;
-  if (!baseDiff) {
-    throw new Error(t('nodeHdiffpatchRequired', { scriptName }));
-  }
-
-  if (policy === 'off') {
-    return baseDiff;
-  }
-
-  return (oldSource?: Buffer, newSource?: Buffer) => {
-    const nativeDiff = baseDiff(oldSource, newSource);
-    if (!oldSource || !newSource || !hdiffModule.diffWithCovers) {
-      return nativeDiff;
-    }
-
-    let bestDiff = nativeDiff;
-    const tryDiffWithCovers = (
-      covers: HpatchCover[],
-      mode: 'replace' | 'merge' | 'native-coalesce',
-    ) => {
-      try {
-        const result = hdiffModule.diffWithCovers?.(
-          oldSource,
-          newSource,
-          covers,
-          { mode },
-        );
-        if (
-          Buffer.isBuffer(result?.diff) &&
-          result.diff.length < bestDiff.length
-        ) {
-          bestDiff = result.diff;
-        }
-      } catch {}
-    };
-
-    tryDiffWithCovers([], 'native-coalesce');
-
-    if (nativeDiff.length < minNativeBytes) {
-      return bestDiff;
-    }
-
-    try {
-      const approximatePlan = chiffModule?.hpatchApproximatePlanResult?.(
-        oldSource,
-        newSource,
-      );
-      if (Array.isArray(approximatePlan?.covers)) {
-        tryDiffWithCovers(approximatePlan.covers, 'merge');
-      }
-    } catch {}
-
-    if (
-      exactPolicy === 'off' ||
-      !chiffModule?.hpatchCompatiblePlanResult
-    ) {
-      return bestDiff;
-    }
-
-    try {
-      const plan = chiffModule.hpatchCompatiblePlanResult(oldSource, newSource);
-      if (Array.isArray(plan.covers)) {
-        tryDiffWithCovers(plan.covers, 'replace');
-        tryDiffWithCovers(plan.covers, 'merge');
-      }
-    } catch {}
-
-    return bestDiff;
-  };
-}
 
 function basename(fn: string): string | undefined {
   const m = /^(.+\/)[^\/]+\/?$/.exec(fn);
@@ -473,10 +329,6 @@ async function diffFromPackage(
 type DiffCommandOptions = {
   customDiff?: Diff;
   customHdiffModule?: HdiffModule;
-  customChiffModule?: ChiffModule;
-  chiffHpatchPolicy?: ChiffHpatchPolicy;
-  chiffHpatchMinNativeBytes?: number | string;
-  chiffHpatchExactCovers?: ChiffHpatchExactPolicy | boolean | string | number;
   [key: string]: any;
 };
 
@@ -493,13 +345,7 @@ function resolveDiffImplementation(
     if (!hdiffModule?.diff) {
       throw new Error(t('nodeHdiffpatchRequired', { scriptName }));
     }
-    return createChiffAwareHdiff(
-      hdiffModule,
-      options.customChiffModule ?? chiff,
-      resolveChiffHpatchPolicy(options.chiffHpatchPolicy),
-      resolveChiffHpatchMinNativeBytes(options.chiffHpatchMinNativeBytes),
-      resolveChiffHpatchExactPolicy(options.chiffHpatchExactCovers),
-    );
+    return hdiffModule.diff;
   }
 
   if (!bsdiff?.diff) {

src/utils/zip-entries.ts

@@ -15,6 +15,12 @@ export function readEntry(
   const buffers: Buffer[] = [];
   return new Promise((resolve, reject) => {
     zipFile.openReadStream(entry, (err, stream) => {
+      if (err) {
+        return reject(err);
+      }
+      if (!stream) {
+        return reject(new Error(`Unable to read zip entry: ${entry.fileName}`));
+      }
       stream.on('data', (chunk: Buffer) => {
         buffers.push(chunk);
       });

src/utils/zip-options.ts

@@ -1,5 +1,5 @@
-import path from 'path';
 import * as fs from 'fs';
+import path from 'path';
 
 export type ZipEntryOptions = {
   compress?: boolean;
@@ -108,11 +108,7 @@ function hasAlreadyCompressedMagic(bytes: Buffer): boolean {
   if (startsWith(bytes, Buffer.from('ID3', 'ascii'))) {
     return true;
   }
-  if (
-    bytes[0] === 0xff &&
-    bytes.length >= 2 &&
-    (bytes[1] & 0xe0) === 0xe0
-  ) {
+  if (bytes[0] === 0xff && bytes.length >= 2 && (bytes[1] & 0xe0) === 0xe0) {
     return true;
   }
   if (

src/versions.ts

@@ -485,7 +485,8 @@ export const versionCommands = {
   update: async ({ options }: { options: VersionCommandOptions }) => {
     const platform = await getPlatform(options.platform);
     const appId = options.appId || (await getSelectedApp(platform)).appId;
-    let versionId = options.versionId || (await chooseVersion(String(appId))).id;
+    let versionId =
+      options.versionId || (await chooseVersion(String(appId))).id;
     if (versionId === 'null') {
       versionId = undefined;
     }
@@ -498,12 +499,8 @@ export const versionCommands = {
     let rollout: number | undefined = undefined;
 
     if (options.rollout !== undefined) {
-      try {
-        rollout = Number.parseInt(options.rollout);
-      } catch (e) {
-        throw new Error(t('rolloutRangeError'));
-      }
-      if (rollout < 1 || rollout > 100) {
+      rollout = Number.parseInt(options.rollout, 10);
+      if (Number.isNaN(rollout) || rollout < 1 || rollout > 100) {
         throw new Error(t('rolloutRangeError'));
       }
     }
@@ -596,7 +593,8 @@ export const versionCommands = {
   }) => {
     const platform = await getPlatform(options.platform);
     const { appId } = await getSelectedApp(platform);
-    const versionId = options.versionId || (await chooseVersion(String(appId))).id;
+    const versionId =
+      options.versionId || (await chooseVersion(String(appId))).id;
 
     const updateParams: Record<string, string> = {};
     if (options.name) updateParams.name = options.name;