Python POC for GitHub-based API Reviews (Phase 1) (#47203)

* Add generate_api_text.py script.

* Add PR creation script.

* Review feedback.

* Add scripts for syncing api.md

* Refactor create_api_review_pr from Python to JS in a module way.

* Remove update process and just have a consistency check.

* Add minor change to azure-template to test pipeline.

* Add API.md to test mismatch.

* Apply actual api.md

* Update GitHub Actions to latest major versions

Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com>

* Add shared JS scripts from rest-api-specs repo.

* Refactor scripts to use shared code and simplify.

* Code review feedback.

* Add generated API.md for azure-template

Co-authored-by: tjprescott <5723682+tjprescott@users.noreply.github.com>

* Refactor to use `azpysdk apistub --md` command.

* Remove Python 3.10 limit on apistub.py.

* Refactor `azpysdk apistub` command. Extract metadata from API.md.

* Add validation for select metadata fields.

* CI fixes.

* Add skill for generating review PR.

* Update review generation logic.

* fix(python adapter): add shell:true on Windows for spawnSync

* fix(python adapter): pass --dest-dir so API.md lands in package dir

* fix(python adapter): skip _generated dirs in readVersion to avoid stale versions

* Make script idempotent and reuse branches with the same API hash.

* Code review feedback.

* Updates.

* Make review PR creation only work for updates, not new packages.

* CI fixes.

* Use shared helper method.

* Add metadata to review PRs.

* Code review feedback.

* Add package-lock.json

* Refactor to use simple-git and octokit

* Code review feedback.

* Remove template APIs.

* Code review feedback.

* Refactor review create script to Python.

* Test consistency refactor.

* Improve failure logging. Fix CI

* Test resovling consistency

* Script fixes.

* Tweaks and remove APIs (more testing)

* Code review feedback.

* Consolidate package.json.

* Code review feedback

* restore chronus

* remove .github/package.json

* remove unnecessary shared src

* add readme with "copied from"

* Fix CI

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com>
Co-authored-by: tjprescott <5723682+tjprescott@users.noreply.github.com>
Co-authored-by: Mike Harder <mharder@microsoft.com>

Author: 3 people

.github/shared/README.md

@@ -0,0 +1,3 @@
+# `.github/shared`
+
+Copied from https://github.com/Azure/azure-rest-api-specs/tree/main/.github/shared

.github/shared/src/cache.js

@@ -0,0 +1,61 @@
+/**
+ * Caches values in memory with a single key of any type.
+ *
+ * @template K, V
+ */
+export class KeyedCache {
+  /** @type {Map<K, V>} */
+  #map = new Map();
+
+  /**
+   * Returns cached value, initializing if necessary
+   *
+   * @param {K} key
+   * @param {() => V} factory
+   * @returns {V} cached value
+   *
+   * @example
+   * const result = cache.getOrCreate(42, async () => await doWork(42));
+   */
+  getOrCreate(key, factory) {
+    let value = this.#map.get(key);
+
+    if (value === undefined) {
+      value = factory();
+      this.#map.set(key, value);
+    }
+
+    return value;
+  }
+}
+
+/**
+ * Caches values in memory with an ordered pair of keys of any types.
+ *
+ * @template K1, K2, V
+ */
+export class KeyedPairCache {
+  // Two-layer nested cache
+  /** @type {KeyedCache<K1, KeyedCache<K2, V>>} */
+  #cache1 = new KeyedCache();
+
+  /**
+   * Returns cached value, initializing if necessary.
+   * Keys are ordered, so (key1, key2) != (key2, key1).
+   *
+   * @param {K1} key1
+   * @param {K2} key2
+   * @param {() => V} factory
+   * @returns {V} cached value
+   *
+   * @example
+  * const result = cache.getOrCreate(42, 7, async () => await doWork(42, 7));
+   */
+  getOrCreate(key1, key2, factory) {
+    // key1 => cache for the next layer
+    const cache2 = this.#cache1.getOrCreate(key1, () => new KeyedCache());
+
+    // key2 => final value
+    return cache2.getOrCreate(key2, factory);
+  }
+}

.github/shared/src/exec.js

@@ -0,0 +1,152 @@
+import child_process from "child_process";
+import { dirname, join } from "path";
+import { promisify } from "util";
+const execFileImpl = promisify(child_process.execFile);
+
+/**
+ * @typedef {Object} ExecOptions
+ * @property {string} [cwd] Current working directory.  Default: process.cwd().
+ * @property {import('./logger.js').ILogger} [logger]
+ * @property {boolean} [logOutput] Log captured stdout/stderr at info level. Default: false.
+ * @property {number} [maxBuffer] Max bytes allowed on stdout or stderr.  Default: 16 * 1024 * 1024.
+ */
+
+/**
+ * @typedef {Object} NpmPrefixOptions
+ * @property {string} [prefix] Prefix to pass to npm via "--prefix".
+ */
+
+/**
+ * @typedef {ExecOptions & NpmPrefixOptions} ExecNpmOptions
+ */
+
+/**
+ * @typedef {Object} ExecResult
+ * @property {string} stdout
+ * @property {string} stderr
+ */
+
+/**
+ * @typedef {Error & { stdout?: string, stderr?: string, code?: number }} ExecError
+ */
+
+/**
+ * Checks whether an unknown error object is an ExecError.
+ * @param {unknown} error
+ * @returns {error is ExecError}
+ */
+export function isExecError(error) {
+  if (!(error instanceof Error)) return false;
+
+  const e = /** @type {ExecError} */ (error);
+  return typeof e.stdout === "string" || typeof e.stderr === "string";
+}
+
+/**
+ * Wraps `child_process.execFile()`, adding logging and a larger default maxBuffer.
+ *
+ * @param {string} file
+ * @param {string[]} [args]
+ * @param {ExecOptions} [options]
+ * @returns {Promise<ExecResult>}
+ * @throws {ExecError}
+ */
+export async function execFile(file, args, options = {}) {
+  const {
+    cwd,
+    logger,
+    logOutput = false,
+    // Node default is 1024 * 1024, which is too small for some git commands returning many entities or large file content.
+    // To support "git show", should be larger than the largest swagger file in the repo (2.5 MB as of 2/28/2025).
+    maxBuffer = 16 * 1024 * 1024,
+  } = options;
+
+  logger?.info(`execFile("${file}", ${JSON.stringify(args)})`);
+
+  try {
+    // execFile(file, args) is more secure than exec(cmd), since the latter is vulnerable to shell injection
+    const result = await execFileImpl(file, args, {
+      cwd,
+      maxBuffer,
+    });
+
+    logger?.debug(`stdout: '${result.stdout}'`);
+    logger?.debug(`stderr: '${result.stderr}'`);
+    if (logOutput) {
+      if (result.stdout) {
+        logger?.info(result.stdout.trimEnd());
+      }
+      if (result.stderr) {
+        logger?.info(result.stderr.trimEnd());
+      }
+    }
+
+    return result;
+  } catch (error) {
+    /* v8 ignore next */
+    logger?.debug(`error: '${JSON.stringify(error)}'`);
+    if (logOutput && isExecError(error)) {
+      if (error.stdout) {
+        logger?.info(error.stdout.trimEnd());
+      }
+      if (error.stderr) {
+        logger?.info(error.stderr.trimEnd());
+      }
+    }
+
+    throw error;
+  }
+}
+
+/**
+ * Calls `execFile()` with appropriate arguments to run `npm` on all platforms
+ *
+ * @param {string[]} args
+ * @param {ExecNpmOptions} [options]
+ * @returns {Promise<ExecResult>}
+ * @throws {ExecError}
+ */
+export async function execNpm(args, options = {}) {
+  const { prefix } = options;
+
+  // Exclude platform-specific code from coverage
+  /* v8 ignore start */
+  const { file, defaultArgs } =
+    process.platform === "win32"
+      ? {
+          // Only way I could find to run "npm" on Windows, without using the shell (e.g. "cmd /c npm ...")
+          //
+          // "node.exe", ["--", "npm-cli.js", ...args]
+          //
+          // The "--" MUST come BEFORE "npm-cli.js", to ensure args are sent to the script unchanged.
+          // If the "--" comes after "npm-cli.js", the args sent to the script will be ["--", ...args],
+          // which is NOT equivalent, and can break if args itself contains another "--".
+
+          // example: "C:\Program Files\nodejs\node.exe"
+          file: process.execPath,
+
+          // example: "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js"
+          defaultArgs: [
+            "--",
+            join(dirname(process.execPath), "node_modules", "npm", "bin", "npm-cli.js"),
+          ],
+        }
+      : { file: "npm", defaultArgs: [] };
+  /* v8 ignore stop */
+
+  const prefixArgs = prefix ? ["--prefix", prefix] : [];
+
+  return await execFile(file, [...defaultArgs, ...prefixArgs, ...args], options);
+}
+
+/**
+ * Calls `execNpm()` with arguments ["exec", "--no", "--"] prepended.
+ *
+ * @param {string[]} args
+ * @param {ExecNpmOptions} [options]
+ * @returns {Promise<ExecResult>}
+ * @throws {ExecError}
+ */
+export async function execNpmExec(args, options = {}) {
+  return await execNpm(["exec", "--no", "--", ...args], options);
+}

.github/shared/src/logger.js

@@ -0,0 +1,64 @@
+/**
+ * @typedef {Object} ILogger
+ * @property {(message:string) => void} debug
+ * @property {(message:string) => void} error
+ * @property {(message:string) => void} info
+ * @property {(message:string) => void} warning
+ * @property {() => boolean} isDebug
+ */
+
+/**
+ * @implements {ILogger}
+ */
+export class ConsoleLogger {
+  /** @type {boolean} */
+  #isDebug;
+
+  /**
+   * @param {boolean} [isDebug] - If true, debug logs will be printed.  Default: false.
+   */
+  constructor(isDebug = false) {
+    this.#isDebug = isDebug;
+  }
+
+  /**
+   * @param {string} message
+   */
+  debug(message) {
+    if (this.isDebug()) {
+      console.debug(message);
+    }
+  }
+
+  /**
+   * @param {string} message
+   */
+  error(message) {
+    console.error(message);
+  }
+
+  /**
+   * @param {string} message
+   */
+  info(message) {
+    console.log(message);
+  }
+
+  /**
+   * @returns {boolean}
+   */
+  isDebug() {
+    return this.#isDebug;
+  }
+
+  /**
+   * @param {string} message
+   */
+  warning(message) {
+    console.warn(message);
+  }
+}
+
+// Singleton loggers
+export const defaultLogger = new ConsoleLogger();
+export const debugLogger = new ConsoleLogger(/*isDebug*/ true);

.github/shared/src/path.js

@@ -0,0 +1,104 @@
+import { basename, dirname, resolve } from "path";
+
+import { KeyedCache, KeyedPairCache } from "./cache.js";
+
+/** @type {KeyedCache<string, string>} */
+const resolveCache = new KeyedCache();
+
+/** @type {KeyedPairCache<string, string, string>} */
+const resolvePairCache = new KeyedPairCache();
+
+/**
+ *
+ * @param {string} path Absolute or relative path
+ * @param {string} segment File or folder
+ * @returns {boolean} True if resolved path contains segment
+ *
+ * @example
+ * includesSegment("stable/2025-01-01/examples/foo.json", "examples")
+ * // -> true
+ */
+export function includesSegment(path, segment) {
+  return untilLastSegment(path, segment) !== "";
+}
+
+/**
+ * Wraps `path.resolve(path)` with a cache to improve performance
+ *
+ * @param {string} path
+ * @returns {string}
+ */
+export function resolveCached(path) {
+  return resolveCache.getOrCreate(path, () => resolve(path));
+}
+
+/**
+ * Wraps `path.resolve(from, to)` with a cache to improve performance
+
+* @param {string} from
+ * @param {string} to
+ * @returns {string}
+ */
+export function resolvePairCached(from, to) {
+  return resolvePairCache.getOrCreate(from, to, () => resolve(from, to));
+}
+
+/**
+ * @param {string} path Absolute or relative path
+ * @param {string} segment File or folder
+ * @returns {string} Portion of resolved path up to (and including) the last occurrence of segment
+ *
+ * @example
+ * untilLastSegment("stable/2025-01-01/examples/foo.json", "examples")
+ * // -> "{cwd}/stable/2025-01-01/examples"
+ */
+export function untilLastSegment(path, segment) {
+  // Shares code with `untilLastSegmentWithParent()`, but not worth refactoring yet
+
+  let current = resolveCached(path);
+
+  while (true) {
+    const parent = dirname(current);
+
+    if (basename(current) === segment) {
+      // Found the target folder.  Return it.
+      return current;
+    } else if (parent === current) {
+      // Reached the filesystem root (folder not found).  Return empty string.
+      return "";
+    } else {
+      // Keep walking upward
+      current = parent;
+    }
+  }
+}
+
+/**
+ * @param {string} path Absolute or relative path
+ * @param {string} segment File or folder
+ * @returns {string} Portion of resolved path up to (and including) the last segment with the specified parent
+ *
+ * @example
+ * untilLastSegmentWithParent("specification/foo/data-plane/stable/2025-01-01/foo.json", "specification")
+ * // -> "{cwd}/specification/foo"
+ */
+export function untilLastSegmentWithParent(path, segment) {
+  // Shares code with `untilLastSegment()`, but not worth refactoring yet
+
+  let current = resolveCached(path);
+
+  while (true) {
+    const parent = dirname(current);
+
+    if (basename(parent) === segment) {
+      // Found the target parent.  Return current;
+      return current;
+    } else if (parent === current) {
+      // Reached the filesystem root (folder not found).  Return empty string.
+      return "";
+    } else {
+      // Keep walking upward
+      current = parent;
+    }
+  }
+}