feat: extract OTP handling to otp.ts with dependency injection pattern

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>

.husky/commit-msg

@@ -1 +1,3 @@
-pnpm commitlint --edit --config=commitlint.config.cjs
+#!/bin/sh
+# Skip in CI environments without pnpm in PATH
+command -v pnpm >/dev/null 2>&1 && pnpm commitlint --edit --config=commitlint.config.cjs || true

releasing/plugin-commands-publishing/src/otp.ts

@@ -0,0 +1,179 @@
+import { PnpmError } from '@pnpm/error'
+import { fetch as pnpmFetch } from '@pnpm/fetch'
+import { globalInfo as logGlobalInfo } from '@pnpm/logger'
+import enquirer from 'enquirer'
+import { type PublishOptions, publish } from 'libnpmpublish'
+
+// ---- Response type definitions ----
+
+export interface OtpWebAuthFetchResponse {
+  readonly json: (this: this) => Promise<unknown>
+  readonly ok: boolean
+}
+
+export interface OtpPublishResponse {
+  readonly ok: boolean
+  readonly status: number
+  readonly statusText: string
+  readonly text: () => Promise<string>
+}
+
+export type OtpPublishFn = (
+  manifest: object,
+  tarballData: Buffer,
+  options: PublishOptions
+) => Promise<OtpPublishResponse>
+
+export interface OtpDate {
+  now: (this: this) => number
+}
+
+// ---- Context interface ----
+
+export interface OtpContext {
+  Date: OtpDate
+  delay: (ms: number) => Promise<void>
+  fetch: (url: string) => Promise<OtpWebAuthFetchResponse>
+  globalInfo: (message: string) => void
+  isInteractive: boolean
+  prompt: () => Promise<string | undefined>
+  publish: OtpPublishFn
+}
+
+// ---- Params interface ----
+
+export interface OtpParams {
+  context?: OtpContext
+  manifest: object
+  publishOptions: PublishOptions
+  tarballData: Buffer
+}
+
+// ---- Shared context (real implementations) ----
+
+async function sharedPrompt (): Promise<string | undefined> {
+  const { otp } = await enquirer.prompt<{ otp: string }>({
+    message: 'This operation requires a one-time password.\nEnter OTP:',
+    name: 'otp',
+    type: 'input',
+  })
+  return otp || undefined
+}
+
+export const SHARED_CONTEXT: OtpContext = {
+  Date,
+  delay: (ms) => new Promise<void>(resolve => setTimeout(resolve, ms)),
+  fetch: (url) => pnpmFetch(url),
+  globalInfo: logGlobalInfo,
+  get isInteractive () { return !!(process.stdin.isTTY && process.stdout.isTTY) },
+  prompt: sharedPrompt,
+  publish: publish as unknown as OtpPublishFn,
+}
+
+// ---- Internal helpers ----
+
+interface OtpErrorBody {
+  authUrl?: string
+  doneUrl?: string
+}
+
+interface OtpError {
+  code: string
+  body?: OtpErrorBody
+}
+
+function isOtpError (error: unknown): error is OtpError {
+  return (
+    error != null &&
+    typeof error === 'object' &&
+    'code' in error &&
+    (error as Record<string, unknown>).code === 'EOTP'
+  )
+}
+
+// ---- Main function ----
+
+/**
+ * Publish a package, handling OTP challenges (classic OTP prompt and webauth browser flow).
+ *
+ * @throws {@link OtpWebAuthTimeoutError} if the webauth browser flow times out.
+ * @throws the original error if OTP handling is not applicable.
+ *
+ * @see https://github.com/npm/cli/blob/7d900c46/lib/utils/otplease.js for npm's `otplease()` implementation.
+ */
+export async function publishWithOtpHandling ({
+  context = SHARED_CONTEXT,
+  manifest,
+  publishOptions,
+  tarballData,
+}: OtpParams): Promise<OtpPublishResponse> {
+  let response: OtpPublishResponse
+  try {
+    response = await context.publish(manifest, tarballData, publishOptions)
+  } catch (error) {
+    if (context.isInteractive && isOtpError(error)) {
+      let otp: string | undefined
+      if (error.body?.authUrl && error.body?.doneUrl) {
+        otp = await webAuthOtp(error.body.authUrl, error.body.doneUrl, context)
+      } else {
+        otp = await context.prompt()
+      }
+      if (otp != null) {
+        return publishWithOtpHandling({
+          context,
+          manifest,
+          tarballData,
+          publishOptions: { ...publishOptions, otp },
+        })
+      }
+    }
+    throw error
+  }
+  return response
+}
+
+// ---- Webauth helpers ----
+
+async function webAuthOtp (authUrl: string, doneUrl: string, context: OtpContext): Promise<string> {
+  context.globalInfo(`Authenticate your account at:\n${authUrl}`)
+  return pollWebAuthDone(doneUrl, context)
+}
+
+async function pollWebAuthDone (doneUrl: string, context: OtpContext): Promise<string> {
+  const startTime = context.Date.now()
+  const timeout = 5 * 60 * 1000 // 5 minutes
+
+  while (true) {
+    if (context.Date.now() - startTime > timeout) {
+      throw new OtpWebAuthTimeoutError()
+    }
+    // eslint-disable-next-line no-await-in-loop
+    await context.delay(1000)
+    let response: OtpWebAuthFetchResponse
+    try {
+      // eslint-disable-next-line no-await-in-loop
+      response = await context.fetch(doneUrl)
+    } catch {
+      continue
+    }
+    if (!response.ok) continue
+    let body: { done?: boolean; token?: string }
+    try {
+      // eslint-disable-next-line no-await-in-loop
+      body = await response.json() as { done?: boolean; token?: string }
+    } catch {
+      continue
+    }
+    if (body.done && body.token) {
+      return body.token
+    }
+  }
+}
+
+// ---- Error classes ----
+
+export class OtpWebAuthTimeoutError extends PnpmError {
+  constructor () {
+    super('WEBAUTH_TIMEOUT', 'Web authentication timed out. Please try again.')
+  }
+}

releasing/plugin-commands-publishing/src/publishPackedPkg.ts

@@ -1,16 +1,16 @@
 import fs from 'fs/promises'
-import { type PublishOptions, publish } from 'libnpmpublish'
+import { type PublishOptions } from 'libnpmpublish'
 import { type Config } from '@pnpm/config'
 import { PnpmError } from '@pnpm/error'
 import { type ExportedManifest } from '@pnpm/exportable-manifest'
 import { globalInfo, globalWarn } from '@pnpm/logger'
-import enquirer from 'enquirer'
 import { displayError } from './displayError.js'
 import { executeTokenHelper } from './executeTokenHelper.js'
 import { createFailedToPublishError } from './FailedToPublishError.js'
 import { AuthTokenError, fetchAuthToken } from './oidc/authToken.js'
 import { IdTokenError, getIdToken } from './oidc/idToken.js'
 import { ProvenanceError, determineProvenance } from './oidc/provenance.js'
+import { publishWithOtpHandling } from './otp.js'
 import { type PackResult } from './pack.js'
 import { type NormalizedRegistryUrl, allRegistryConfigKeys, parseSupportedRegistryUrl } from './registryConfigKeys.js'
 
@@ -51,9 +51,6 @@ export type PublishPackedPkgOptions = Pick<Config,
   provenanceFile?: string // NOTE: This field is currently not supported
 }
 
-// @types/libnpmpublish unfortunately uses an outdated type definition of package.json
-type ManifestFromOutdatedDefinition = typeof publish extends (_a: infer Manifest, ..._: never) => unknown ? Manifest : never
-
 export async function publishPackedPkg (
   packResult: Pick<PackResult, 'publishedManifest' | 'tarballPath'>,
   opts: PublishPackedPkgOptions
@@ -68,104 +65,14 @@ export async function publishPackedPkg (
     globalWarn(`Skip publishing ${name}@${version} (dry run)`)
     return
   }
-  await publishWithOtpHandling(publishedManifest as ManifestFromOutdatedDefinition, tarballData, publishOptions, packResult)
-}
-
-async function publishWithOtpHandling (
-  manifest: ManifestFromOutdatedDefinition,
-  tarballData: Buffer,
-  publishOptions: PublishOptions,
-  packResult: Pick<PackResult, 'publishedManifest'>
-): Promise<void> {
-  let response: Awaited<ReturnType<typeof publish>>
-  try {
-    response = await publish(manifest, tarballData, publishOptions)
-  } catch (error) {
-    if (process.stdin.isTTY && process.stdout.isTTY && isOtpError(error)) {
-      let otp: string | undefined
-      if (error.body?.authUrl && error.body?.doneUrl) {
-        otp = await webAuthOtp(error.body.authUrl, error.body.doneUrl)
-      } else {
-        otp = await promptForOtp()
-      }
-      if (otp != null) {
-        return publishWithOtpHandling(manifest, tarballData, { ...publishOptions, otp }, packResult)
-      }
-    }
-    throw error
-  }
+  const response = await publishWithOtpHandling({ manifest: publishedManifest as object, tarballData, publishOptions })
   if (response.ok) {
-    const { name, version } = packResult.publishedManifest
     globalInfo(`✅ Published package ${name}@${version}`)
     return
   }
   throw await createFailedToPublishError(packResult, response)
 }
 
-interface OtpErrorBody {
-  authUrl?: string
-  doneUrl?: string
-}
-
-interface OtpError {
-  code: string
-  body?: OtpErrorBody
-}
-
-function isOtpError (error: unknown): error is OtpError {
-  return (
-    error != null &&
-    typeof error === 'object' &&
-    'code' in error &&
-    (error as Record<string, unknown>).code === 'EOTP'
-  )
-}
-
-async function webAuthOtp (authUrl: string, doneUrl: string): Promise<string> {
-  globalInfo(`Authenticate your account at:\n${authUrl}`)
-  return pollWebAuthDone(doneUrl)
-}
-
-async function pollWebAuthDone (doneUrl: string): Promise<string> {
-  const startTime = Date.now()
-  const timeout = 5 * 60 * 1000 // 5 minutes
-
-  while (true) {
-    if (Date.now() - startTime > timeout) {
-      throw new PnpmError('WEBAUTH_TIMEOUT', 'Web authentication timed out. Please try again.')
-    }
-    // eslint-disable-next-line no-await-in-loop
-    await new Promise<void>(resolve => setTimeout(resolve, 1000))
-    let response: Response
-    try {
-      // eslint-disable-next-line no-await-in-loop
-      response = await fetch(doneUrl)
-    } catch {
-      continue
-    }
-    if (!response.ok) continue
-    let body: { done?: boolean; token?: string }
-    try {
-      // eslint-disable-next-line no-await-in-loop
-      body = await response.json() as { done?: boolean; token?: string }
-    } catch {
-      continue
-    }
-    if (body.done && body.token) {
-      return body.token
-    }
-  }
-}
-
-async function promptForOtp (): Promise<string | undefined> {
-  const { otp } = await enquirer.prompt<{ otp: string }>({
-    message: 'This operation requires a one-time password.\nEnter OTP:',
-    name: 'otp',
-    type: 'input',
-  })
-  return otp || undefined
-}
-
 async function createPublishOptions (manifest: ExportedManifest, options: PublishPackedPkgOptions): Promise<PublishOptions> {
   const { registry, auth, ssl } = findAuthSslInfo(manifest, options)