fix(skills): stop ambient env leaking into generated SKILL.md output

changelog.d/fixes/15948-agent-skills-determinism.md

@@ -0,0 +1,5 @@
+- Generated Agent Skill API examples now use the canonical local base URL unless a caller
+  explicitly supplies another host, preventing ambient environment variables from changing
+  committed documentation. Explicit configured-host examples remain supported, and
+  regression tests cover both behaviors, including isolation and restoration of
+  `NEXT_PUBLIC_BASE_URL`. (#15948 — thanks @BenjaminAronsson)

changelog.d/maintenance/15945-skills-sync-regen.md

@@ -0,0 +1 @@
+- **fix(skills):** regenerate the 20 `SKILL.md` files that had drifted from their generator (#15945). `check:agent-skills-sync` was failing on the `release/v3.8.52` tip, which blocked every PR into the branch rather than just one: `.mergify.yml` names `Merge integrity (changelog + generated skills)` as the always-on anchor that must be an affirmative `check-success`, and the only failure it tolerates is `Build (advisory)`. The cause was #14381 (`4df9ceae03`), which extracted the curl examples out of `src/lib/agentSkills/generator.ts` into the new `apiOperationExample.ts` and replaced the hardcoded `https://localhost:20128` with `resolveOmniRouteBaseUrl()`, whose default is `http://localhost:20128`; the committed files were never regenerated, so every generated example disagreed with the generator from that commit onward. This is the mechanical output of `node --import tsx/esm scripts/skills/generate-agent-skills.mjs --apply` with no base-url env vars set — the documented default and the environment CI runs in — so the diff is the `https:` → `http:` scheme change plus the endpoint-reference content that moved with it. No instruction text is added or edited, which matters because `skills/**/SKILL.md` is an agent-instruction surface. The gate goes from 20 drifted to 0 (`Generated: 0 · Unchanged: 46 · Errors: 0`), with `check:changelog-integrity` and `check:docs-all` also clean. One latent cause is deliberately left alone: `apiOperationExample.ts` and `cliConfigurationExample.ts` resolve the base URL from `process.env` (`OMNIROUTE_BASE_URL`, `BASE_URL`, `NEXT_PUBLIC_BASE_URL`, `PORT`, …), so anyone running `--apply` with one of those set re-bakes their own host into the committed artifact and re-opens this drift. Making generation env-independent touches shared runtime code used by the A2A skills, the MCP internal fetch and two API routes, so it belongs in its own PR rather than a base-red drain.

src/lib/agentSkills/apiOperationExample.ts

@@ -1,13 +1,12 @@
 import { buildCliConfigurationExample } from "./cliConfigurationExample";
-import { resolveOmniRouteBaseUrl } from "@/shared/utils/resolveOmniRouteBaseUrl";
+import { DEFAULT_OMNIROUTE_BASE_URL } from "@/shared/utils/resolveOmniRouteBaseUrl";
 
 interface Operation {
   path: string;
   method: string;
 }
 
-function dashboardExample({ path, method }: Operation): string[] {
-  const baseUrl = resolveOmniRouteBaseUrl();
+function dashboardExample({ path, method }: Operation, baseUrl: string): string[] {
   if (path === "/api/auth/login" && method === "POST") {
     return [
       `curl -X POST ${baseUrl}${path} \\`,
@@ -29,8 +28,7 @@ function dashboardExample({ path, method }: Operation): string[] {
   ];
 }
 
-function bearerExample({ path, method }: Operation): string[] {
-  const baseUrl = resolveOmniRouteBaseUrl();
+function bearerExample({ path, method }: Operation, baseUrl: string): string[] {
   const curlMethod = method === "GET" ? "" : `-X ${method} `;
   const hasJsonBody = ["POST", "PUT", "PATCH"].includes(method);
   return [
@@ -40,13 +38,25 @@ function bearerExample({ path, method }: Operation): string[] {
   ];
 }
 
-/** Preserve the authentication model while supplying a valid configuration example. */
+/**
+ * Preserve the authentication model while supplying a valid configuration example.
+ *
+ * `baseUrl` is an explicit input, not something resolved from `process.env` in here.
+ * The only consumer is the SKILL.md generator, whose output is committed to the repo
+ * and then diff-gated by `check:agent-skills-sync`. Resolving the environment at this
+ * depth made that committed artifact a function of whoever last ran the generator:
+ * anyone with `PORT`/`BASE_URL`/`OMNIROUTE_BASE_URL` set wrote their own host into the
+ * files, and CI — which sets none of them — then reported drift. Callers that genuinely
+ * want the configured host pass `resolveOmniRouteBaseUrl()` themselves; the default
+ * stays canonical so regeneration is reproducible on any machine.
+ */
 export function buildApiOperationExample(
   operation: Operation,
-  dashboardSession: boolean
+  dashboardSession: boolean,
+  baseUrl: string = DEFAULT_OMNIROUTE_BASE_URL
 ): string[] {
   return (
-    buildCliConfigurationExample(operation) ??
-    (dashboardSession ? dashboardExample(operation) : bearerExample(operation))
+    buildCliConfigurationExample(operation, baseUrl) ??
+    (dashboardSession ? dashboardExample(operation, baseUrl) : bearerExample(operation, baseUrl))
   );
 }

src/lib/agentSkills/cliConfigurationExample.ts

@@ -1,13 +1,18 @@
 /** Examples for the configuration API keep credentials out of URLs and apply dry-run-only. */
-import { resolveOmniRouteBaseUrl } from "@/shared/utils/resolveOmniRouteBaseUrl";
+import { DEFAULT_OMNIROUTE_BASE_URL } from "@/shared/utils/resolveOmniRouteBaseUrl";
 
-export function buildCliConfigurationExample(operation: {
-  path: string;
-  method: string;
-}): string[] | undefined {
+export function buildCliConfigurationExample(
+  operation: {
+    path: string;
+    method: string;
+  },
+  // Callers pass the base URL explicitly so generated output is a pure function of
+  // its inputs. Reading process.env here made committed SKILL.md files depend on the
+  // machine that generated them — see apiOperationExample.ts for the full rationale.
+  baseUrl: string = DEFAULT_OMNIROUTE_BASE_URL
+): string[] | undefined {
   const { path, method } = operation;
   if (path !== "/api/cli-tools/config" && path !== "/api/cli-tools/apply") return;
-  const baseUrl = resolveOmniRouteBaseUrl();
   if (method === "GET" && path === "/api/cli-tools/config") {
     return [
       `curl ${baseUrl}${path} \\`,

tests/unit/cli-configuration-consumer-contract.test.ts

@@ -3,7 +3,10 @@ import test from "node:test";
 import { buildSkillMarkdown } from "../../src/lib/agentSkills/generator.ts";
 import { parseOpenapi } from "../../src/lib/agentSkills/openapiParser.ts";
 import { buildApiOperationExample } from "../../src/lib/agentSkills/apiOperationExample.ts";
-import { resolveOmniRouteBaseUrl } from "../../src/shared/utils/resolveOmniRouteBaseUrl.ts";
+import {
+  DEFAULT_OMNIROUTE_BASE_URL,
+  resolveOmniRouteBaseUrl,
+} from "../../src/shared/utils/resolveOmniRouteBaseUrl.ts";
 
 test("generated CLI skill uses header previews and original-input dry-run application", () => {
   const { body, references } = buildSkillMarkdown("omni-cli-tools", {
@@ -30,7 +33,7 @@ test("generated CLI skill uses header previews and original-input dry-run applic
 });
 
 test("configuration examples preserve unrelated bearer and dashboard-session authentication", () => {
-  const baseUrl = resolveOmniRouteBaseUrl();
+  const baseUrl = DEFAULT_OMNIROUTE_BASE_URL;
   assert.deepEqual(buildApiOperationExample({ path: "/api/models", method: "GET" }, false), [
     `curl ${baseUrl}/api/models \\`,
     '  -H "Authorization: Bearer $OMNIROUTE_TOKEN"',
@@ -53,25 +56,96 @@ test("configuration examples preserve unrelated bearer and dashboard-session aut
   assert.ok(!read.includes("CSRF_TOKEN"));
 });
 
-test("configuration examples track the configured loopback port instead of a hardcoded 20128", () => {
-  const savedPort = process.env.PORT;
-  const savedBaseUrl = process.env.BASE_URL;
-  const savedOmniBaseUrl = process.env.OMNIROUTE_BASE_URL;
-  delete process.env.BASE_URL;
-  delete process.env.OMNIROUTE_BASE_URL;
+/** Runs `fn` with the base-url env vars forced to `vars`, restoring them afterwards. */
+function withBaseUrlEnv(vars: Record<string, string | undefined>, fn: () => void): void {
+  const keys = [
+    "PORT",
+    "API_PORT",
+    "DASHBOARD_PORT",
+    "BASE_URL",
+    "OMNIROUTE_BASE_URL",
+    "NEXT_PUBLIC_BASE_URL",
+  ];
+  const saved = new Map(keys.map((key) => [key, process.env[key]]));
   try {
-    process.env.PORT = "37128";
-    const example = buildApiOperationExample({ path: "/api/models", method: "GET" }, false).join(
-      "\n"
-    );
-    assert.ok(example.includes("http://localhost:37128/api/models"), example);
-    assert.ok(!example.includes("20128"), example);
+    for (const key of keys) {
+      const value = vars[key];
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+    fn();
   } finally {
-    if (savedPort === undefined) delete process.env.PORT;
-    else process.env.PORT = savedPort;
-    if (savedBaseUrl === undefined) delete process.env.BASE_URL;
-    else process.env.BASE_URL = savedBaseUrl;
-    if (savedOmniBaseUrl === undefined) delete process.env.OMNIROUTE_BASE_URL;
-    else process.env.OMNIROUTE_BASE_URL = savedOmniBaseUrl;
+    for (const [key, value] of saved) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
   }
+}
+
+// A caller that wants the operator's configured host still gets it (#15778) — but it
+// now has to ask for it, by passing the resolver's result as the baseUrl argument.
+test("configuration examples track the configured loopback port when given one", () => {
+  // Exercise isolation even on machines where this higher-priority input is unset.
+  const savedPublicBaseUrl = process.env.NEXT_PUBLIC_BASE_URL;
+  process.env.NEXT_PUBLIC_BASE_URL = "https://ambient.invalid";
+  try {
+    withBaseUrlEnv({ PORT: "37128" }, () => {
+      const example = buildApiOperationExample(
+        { path: "/api/models", method: "GET" },
+        false,
+        resolveOmniRouteBaseUrl()
+      ).join("\n");
+      assert.ok(example.includes("http://localhost:37128/api/models"), example);
+      assert.ok(!example.includes("20128"), example);
+    });
+    assert.equal(process.env.NEXT_PUBLIC_BASE_URL, "https://ambient.invalid");
+  } finally {
+    if (savedPublicBaseUrl === undefined) delete process.env.NEXT_PUBLIC_BASE_URL;
+    else process.env.NEXT_PUBLIC_BASE_URL = savedPublicBaseUrl;
+  }
+});
+
+// The generator commits its output and `check:agent-skills-sync` diff-gates it, so the
+// default must not vary with the machine. Reading process.env down in the example
+// builders is what drifted all 20 SKILL.md files and blocked every PR into the branch.
+test("generated examples ignore ambient base-url env so committed output is reproducible", () => {
+  const canonical = buildApiOperationExample({ path: "/api/models", method: "GET" }, false).join(
+    "\n"
+  );
+  assert.ok(canonical.includes(`${DEFAULT_OMNIROUTE_BASE_URL}/api/models`), canonical);
+
+  for (const vars of [
+    { PORT: "37128" },
+    { API_PORT: "41999" },
+    { DASHBOARD_PORT: "42000" },
+    { BASE_URL: "https://ops.internal.example" },
+    { OMNIROUTE_BASE_URL: "https://omni.example:8443" },
+    { NEXT_PUBLIC_BASE_URL: "https://public.example" },
+  ]) {
+    withBaseUrlEnv(vars, () => {
+      const actual = buildApiOperationExample({ path: "/api/models", method: "GET" }, false).join(
+        "\n"
+      );
+      assert.equal(actual, canonical, `env ${JSON.stringify(vars)} leaked into the example`);
+    });
+  }
+});
+
+// The /api/cli-tools/* examples take the same baseUrl argument, so pin them too.
+test("configuration-API examples are reproducible and still accept an explicit host", () => {
+  withBaseUrlEnv({ OMNIROUTE_BASE_URL: "https://omni.example:8443" }, () => {
+    const pinned = buildApiOperationExample(
+      { path: "/api/cli-tools/config", method: "GET" },
+      false
+    ).join("\n");
+    assert.ok(pinned.includes(`${DEFAULT_OMNIROUTE_BASE_URL}/api/cli-tools/config`), pinned);
+    assert.ok(!pinned.includes("omni.example"), pinned);
+
+    const explicit = buildApiOperationExample(
+      { path: "/api/cli-tools/config", method: "GET" },
+      false,
+      "https://omni.example:8443"
+    ).join("\n");
+    assert.ok(explicit.includes("https://omni.example:8443/api/cli-tools/config"), explicit);
+  });
 });