fix(code-queue): reject runner skills typo path

2026-05-23 09:05:23 +00:00
parent a0fb63c098
commit 023509d10a
5 changed files with 167 additions and 8 deletions
@@ -228,9 +228,9 @@ bun scripts/code-queue-pr-preflight-example.ts --repo pikasTech/unidesk --base m

 ### Runner Skill 可用性

-D601 Code Queue runner 的长期 skills source of truth 是宿主 `/home/ubuntu/.agents/skills`，生产和 dev Code Queue Pod 都必须只读挂载到容器内 `/root/.agents/skills`，并设置 `UNIDESK_SKILLS_PATH=/root/.agents/skills`。不要使用或传播任何拼错的 skills 路径；诊断中只能把这类问题作为 forbidden path risk 暴露。
+D601 Code Queue runner 的长期 skills source of truth 是宿主 `/home/ubuntu/.agents/skills`，生产和 dev Code Queue Pod 都必须只读挂载到容器内 `/root/.agents/skills`，并设置 `UNIDESK_SKILLS_PATH=/root/.agents/skills`。不要使用或传播任何拼错的 skills 路径；如果配置的 source 或 target 命中拼写错误路径，health/preflight 必须直接返回 `blocker=forbidden-skills-path-configured`，并在 `pathSpelling.forbiddenPathConfigured=true` 与 `pathSpelling.forbiddenPathRoles` 中标明是 source、target 或两者，而不是降级成普通 missing path。

-执行面 `/health`、`/api/dev-ready` 和 `/api/runtime-preflight` 必须输出同一份只读 skill availability report。稳定字段包括 `source`、`target`、`requiredSkills`、`missingSkills`、`degraded`、`blocker` 和 `valuesPrinted=false`。`requiredSkills` 至少覆盖 `docs-spec`、`cli-spec`、`frontend-design` 与 `playwright-cli`；如果目标目录缺失、不是只读挂载、必需 skill 缺失，或拼写错误路径存在，报告必须显示 `ok=false` 和结构化 `blocker`，不能把 runner 能力缺口伪装成业务任务失败。
+执行面 `/health`、`/api/dev-ready` 和 `/api/runtime-preflight` 必须输出同一份只读 skill availability report。稳定字段包括 `source`、`target`、`requiredSkills`、`missingSkills`、`degraded`、`blocker`、`pathSpelling` 和 `valuesPrinted=false`。`requiredSkills` 至少覆盖 `docs-spec`、`cli-spec`、`frontend-design` 与 `playwright-cli`；如果目标目录缺失、不是只读挂载、必需 skill 缺失、拼写错误路径被配置或拼写错误路径已存在，报告必须显示 `ok=false` 和结构化 `blocker`，不能把 runner 能力缺口伪装成业务任务失败。

 执行面还必须提供 dry-run skills sync/preflight 合同：稳定入口是 `GET /api/skills-sync?dryRun=1`，CLI 入口是 `bun scripts/cli.ts codex skills-sync --dry-run [--full]`。该合同只描述受控 hostPath 生命周期，不复制文件、不从任意路径静默加载、不重启服务、不 rollout Pod、不读取 Secret。默认输出保持紧凑，必须报告 source、target、expected env/mount、required skill 列表、source/target skill counts、missing source/target skills、permission failure count、plannedActions 和修复指令；逐 skill 细节、完整 permission failure 和原始报告只能通过 `--full` 显式展开。非 dry-run 请求必须失败。

@@ -29,6 +29,8 @@ const helpSource = readFileSync("scripts/src/help.ts", "utf8");
 const promptSource = readFileSync("src/components/microservices/code-queue/src/prompts.ts", "utf8");
 const docsReference = readFileSync("docs/reference/code-queue-supervision.md", "utf8");
 const forbiddenPathLiteral = [".ag", "nets/skills"].join("");
+const forbiddenTargetPath = [`/root/${[".ag", "nets"].join("")}`, "skills"].join("/");
+const forbiddenSourcePath = [`/home/ubuntu/${[".ag", "nets"].join("")}`, "skills"].join("/");

 assertCondition(!productionManifest.includes(forbiddenPathLiteral), "production manifest must not propagate misspelled skills path");
 assertCondition(!devManifest.includes(forbiddenPathLiteral), "dev manifest must not propagate misspelled skills path");
@@ -74,6 +76,18 @@ assertCondition(missing.blocker === "skills-target-missing", "missing target sho
 assertCondition(missing.missingSkills.includes("docs-spec") && missing.missingSkills.includes("cli-spec"), "missing target should list required missing skills", missing);
 assertCondition(missing.valuesPrinted === false, "missing report must also declare valuesPrinted=false");

+const typoTarget = collectSkillAvailability({
+  source: "/home/ubuntu/.agents/skills",
+  target: forbiddenTargetPath,
+  requiredSkills: ["docs-spec", "cli-spec"],
+});
+assertCondition(typoTarget.ok === false, "misspelled target should fail", typoTarget);
+assertCondition(typoTarget.degraded === true, "misspelled target should be degraded", typoTarget);
+assertCondition(typoTarget.blocker === "forbidden-skills-path-configured", "misspelled target should expose dedicated blocker", typoTarget);
+assertCondition(asRecord(typoTarget.pathSpelling, "typoTarget.pathSpelling").forbiddenPathConfigured === true, "misspelled target should mark configured typo", typoTarget.pathSpelling);
+assertCondition(JSON.stringify(asRecord(typoTarget.pathSpelling, "typoTarget.pathSpelling").forbiddenPathRoles).includes("target"), "misspelled target should classify target role", typoTarget.pathSpelling);
+assertCondition(typoTarget.valuesPrinted === false, "misspelled target report must declare valuesPrinted=false");
+
 const syncDryRun = collectSkillSyncPreflight({
  source: "/home/ubuntu/.agents/skills",
  target: "/path/that/does/not/exist/for-code-queue-skills-test",
@@ -99,6 +113,17 @@ assertCondition(!JSON.stringify(syncDryRun).includes(forbiddenPathLiteral), "ski
 const missingTargetSync = collectSkillSyncPreflight({ target: "/path/that/does/not/exist/for-code-queue-skills-test" });
 assertCondition(missingTargetSync.blocker === "unapproved-target", "non-default target must be rejected as unapproved", missingTargetSync);

+const typoTargetSync = collectSkillSyncPreflight({ target: forbiddenTargetPath });
+assertCondition(typoTargetSync.blocker === "forbidden-skills-path-configured", "misspelled sync target must be rejected as a typo before generic target approval", typoTargetSync);
+assertCondition(asRecord(typoTargetSync.pathSpelling, "typoTargetSync.pathSpelling").forbiddenPathConfigured === true, "misspelled sync target should mark configured typo", typoTargetSync.pathSpelling);
+assertCondition(JSON.stringify(asRecord(typoTargetSync.pathSpelling, "typoTargetSync.pathSpelling").forbiddenPathRoles).includes("target"), "misspelled sync target should classify target role", typoTargetSync.pathSpelling);
+assertCondition(typoTargetSync.valuesPrinted === false, "misspelled sync target must declare valuesPrinted=false");
+
+const typoSourceSync = collectSkillSyncPreflight({ source: forbiddenSourcePath });
+assertCondition(typoSourceSync.blocker === "forbidden-skills-path-configured", "misspelled sync source must be rejected as a typo before generic source approval", typoSourceSync);
+assertCondition(asRecord(typoSourceSync.pathSpelling, "typoSourceSync.pathSpelling").forbiddenPathConfigured === true, "misspelled sync source should mark configured typo", typoSourceSync.pathSpelling);
+assertCondition(JSON.stringify(asRecord(typoSourceSync.pathSpelling, "typoSourceSync.pathSpelling").forbiddenPathRoles).includes("source"), "misspelled sync source should classify source role", typoSourceSync.pathSpelling);
+
 assertCondition(runtimePreflight.includes("skills: SkillAvailabilityReport"), "runtime preflight type must include skills report");
 assertCondition(runtimePreflight.includes("skillsSync: SkillSyncPreflightReport"), "runtime preflight type must include skills sync report");
 assertCondition(runtimePreflight.includes("collectSkillAvailability"), "runtime preflight must collect skills availability");
@@ -114,9 +139,12 @@ assertCondition(codeQueueCli.includes("Code Queue skills sync dry-run could not
 assertCondition(codeQueueCli.includes("compact-skills-sync-control-plane-failure"), "codex skills-sync CLI must keep control-plane failure output compact");
 assertCondition(codeQueueCli.includes("compactSkillsSyncStatus"), "codex CLI must compact skills sync output");
 assertCondition(codeQueueCli.includes("runner-skills-blocker"), "codex preflight must classify skill lifecycle blockers");
+assertCondition(codeQueueCli.includes("forbiddenPathConfigured"), "codex CLI must preserve configured typo classification in compact output");
 assertCondition(microserviceCli.includes("compactSkillSync"), "microservice health summary must compact skills sync output");
+assertCondition(microserviceCli.includes("forbiddenPathConfigured"), "microservice health summary must preserve configured typo classification");
 assertCondition(helpSource.includes("codex skills-sync --dry-run"), "CLI help must document the skills sync dry-run command");
 assertCondition(docsReference.includes("codex skills-sync --dry-run"), "reference docs must document the skills sync dry-run command");
+assertCondition(docsReference.includes("forbidden-skills-path-configured"), "reference docs must document configured typo blocker");

 const skillsPreflightTransport = {
  config: null,
@@ -190,6 +218,68 @@ assertCondition(asRecord(preflightSkillsSync.plannedActions, "preflight.skillsSy
 assertCondition(preflightSkillsSync.valuesPrinted === false, "full preflight skills sync must declare valuesPrinted=false", preflightSkillsSync);
 assertCondition(!JSON.stringify(preflightSkillsSync).includes(forbiddenPathLiteral), "full preflight must not propagate misspelled path literal");

+const typoPreflightTransport = {
+  config: null,
+  coreFetch: () => ({
+    ok: true,
+    status: 200,
+    body: {
+      runtimePreflight: {
+        ok: false,
+        checkedAt: "2026-05-23T00:00:00.000Z",
+        cwd: "/workspace/unidesk",
+        pid: 601,
+        skills: typoTarget,
+        skillsSync: typoTargetSync,
+        ports: {},
+        pullRequestDelivery: {
+          ok: true,
+          checkedAt: "2026-05-23T00:00:00.000Z",
+          tools: {},
+          unideskGhCli: { ok: true, path: "/workspace/unidesk/scripts/cli.ts", present: true },
+          authBroker: { ok: true, configured: true, source: "auth-broker" },
+          credentials: {
+            ghTokenPresent: true,
+            githubTokenPresent: false,
+            ghHostsConfigPresent: false,
+            gitCredentialsPresent: false,
+          },
+          git: {
+            insideWorktree: true,
+            branch: "code-queue/issue-68-runner-skills-lifecycle",
+            head: "abc1234",
+            originMaster: "def5678",
+            remoteOrigin: "git@github.com:pikasTech/unidesk.git",
+            home: "/root",
+            homeWritable: true,
+            knownHostsPresent: true,
+            privateKeyPresent: true,
+          },
+          githubContext: {
+            host: "github.com",
+            apiBaseUrl: "https://api.github.com",
+            repo: "pikasTech/unidesk",
+            issueProbeNumber: 68,
+          },
+          egress: { proxy: {} },
+          remote: null,
+          limitations: [],
+          risks: [],
+        },
+      },
+    },
+  }),
+};
+const typoPreflightSummary = asRecord(codexPrPreflightQueryForTest(["--remote"], typoPreflightTransport), "typo preflight summary");
+assertCondition(typoPreflightSummary.failureKind === "runner-skills-blocker", "typo preflight should classify configured typo as runner skills blocker", typoPreflightSummary);
+assertCondition(typoPreflightSummary.degradedReason === "forbidden-skills-path-configured", "typo preflight degraded reason should preserve configured typo blocker", typoPreflightSummary);
+assertCondition(typoPreflightSummary.preflight === undefined, "typo default preflight should remain bounded", typoPreflightSummary);
+const typoFullPreflightSummary = asRecord(codexPrPreflightQueryForTest(["--remote", "--full"], typoPreflightTransport), "typo full preflight summary");
+const typoFullPreflight = asRecord(typoFullPreflightSummary.preflight, "typoFullPreflightSummary.preflight");
+const typoPreflightSkills = asRecord(typoFullPreflight.skills, "typoFullPreflight.skills");
+const typoPreflightPathSpelling = asRecord(typoPreflightSkills.pathSpelling, "typoFullPreflight.skills.pathSpelling");
+assertCondition(typoPreflightPathSpelling.forbiddenPathConfigured === true, "typo full preflight output must expose configured typo classification", typoPreflightPathSpelling);
+
 const healthSummary = asRecord(summarizeMicroserviceObservation("health", "code-queue", {
  ok: true,
  status: 200,
@@ -210,6 +300,21 @@ assertCondition(asRecord(healthSkillsSync.counts, "microservice.summary.skillsSy
 assertCondition(asRecord(healthSkillsSync.plannedActions, "microservice.summary.skillsSync.plannedActions").copyFromArbitraryPath === false, "compact health must show arbitrary copy is blocked", healthSkillsSync);
 assertCondition(!JSON.stringify(healthSkillsSync).includes(forbiddenPathLiteral), "compact health must not propagate misspelled path literal");

+const typoHealthSummary = asRecord(summarizeMicroserviceObservation("health", "code-queue", {
+  ok: true,
+  status: 200,
+  body: {
+    ok: false,
+    service: "code-queue",
+    skills: typoTarget,
+    skillsSync: typoTargetSync,
+  },
+}, []), "microservice typo health summary");
+const typoHealthSkills = asRecord(asRecord(asRecord(typoHealthSummary.microservice, "typoHealthSummary.microservice").summary, "typoHealthSummary.microservice.summary").skills, "typoHealthSummary.skills");
+const typoHealthPathSpelling = asRecord(typoHealthSkills.pathSpelling, "typoHealthSummary.skills.pathSpelling");
+assertCondition(typoHealthSkills.blocker === "forbidden-skills-path-configured", "compact health must preserve configured typo blocker", typoHealthSkills);
+assertCondition(typoHealthPathSpelling.forbiddenPathConfigured === true, "compact health must expose configured typo classification", typoHealthPathSpelling);
+
 process.stdout.write(`${JSON.stringify({
  ok: true,
  checks: [
@@ -218,7 +323,7 @@ process.stdout.write(`${JSON.stringify({
    "skills sync dry-run reports source, target, counts, missing skills, permission failures, instructions and no-copy actions",
    "runtime-preflight, dev-ready, health and PR preflight use the same structured skill and sync reports",
    "default health/preflight summaries expose bounded skills lifecycle evidence and --full expansion",
-    "misspelled skills paths are only surfaced as a forbidden diagnostic risk",
+    "misspelled skills paths are rejected with forbidden-skills-path-configured before generic missing/unapproved path blockers",
  ],
  observedRunner: {
    source: available.source,
@@ -529,6 +529,15 @@ function asRecord(value: unknown): Record<string, unknown> | null {
  return typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record<string, unknown> : null;
 }

+function compactObjectFields(record: Record<string, unknown> | null, keys: string[]): Record<string, unknown> | null {
+  if (record === null) return null;
+  const selected: Record<string, unknown> = {};
+  for (const key of keys) {
+    if (record[key] !== undefined) selected[key] = record[key];
+  }
+  return Object.keys(selected).length > 0 ? selected : null;
+}
+
 function asArray(value: unknown): unknown[] {
  return Array.isArray(value) ? value : [];
 }
@@ -4654,7 +4663,14 @@ function compactSkillsStatus(value: unknown): Record<string, unknown> | null {
    requiredSkills: Array.isArray(record.requiredSkills) ? record.requiredSkills : [],
    missingSkills: Array.isArray(record.missingSkills) ? record.missingSkills : [],
    valuesPrinted: record.valuesPrinted ?? false,
-    pathSpelling: record.pathSpelling ?? null,
+    pathSpelling: compactObjectFields(asRecord(record.pathSpelling), [
+      "expectedTarget",
+      "forbiddenPathChecked",
+      "forbiddenPathExists",
+      "forbiddenPathConfigured",
+      "forbiddenPathRoles",
+      "forbiddenPathMustNotBeUsed",
+    ]),
    repairHint: record.repairHint ?? null,
  };
 }
@@ -642,6 +642,8 @@ function compactSkillAvailability(value: unknown): Record<string, unknown> | nul
      "expectedTarget",
      "forbiddenPathChecked",
      "forbiddenPathExists",
+      "forbiddenPathConfigured",
+      "forbiddenPathRoles",
      "forbiddenPathMustNotBeUsed",
    ]),
    repairHint: skills.repairHint ?? null,
@@ -30,6 +30,8 @@ export interface SkillAvailabilityReport {
    expectedTarget: string;
    forbiddenPathChecked: true;
    forbiddenPathExists: boolean;
+    forbiddenPathConfigured: boolean;
+    forbiddenPathRoles: Array<"source" | "target">;
    forbiddenPathMustNotBeUsed: true;
  };
  expectedMount: string;
@@ -75,6 +77,7 @@ export interface SkillSyncPreflightReport {
    | "skills-target-not-readonly"
    | "required-source-skills-missing"
    | "required-target-skills-missing"
+    | "forbidden-skills-path-configured"
    | "forbidden-skills-path-present"
    | null;
  checkedAt: string;
@@ -128,6 +131,7 @@ export const defaultRequiredSkills = ["docs-spec", "cli-spec", "frontend-design"
 export const defaultSource = "/home/ubuntu/.agents/skills";
 export const expectedTarget = "/root/.agents/skills";
 const forbiddenSkillsDirName = [".ag", "nets"].join("");
+const forbiddenRelativeSkillsPath = `${forbiddenSkillsDirName}/skills`;
 const forbiddenTargets = [`/root/${forbiddenSkillsDirName}/skills`, `/home/ubuntu/${forbiddenSkillsDirName}/skills`];
 const skillDirectoryAliases: Record<string, string[]> = {
  "playwright-cli": ["playwright-cli", "playwright"],
@@ -151,6 +155,18 @@ function decodeMountInfoPath(value: string): string {
  return value.replace(/\\([0-7]{3})/gu, (_match, octal: string) => String.fromCharCode(Number.parseInt(octal, 8)));
 }

+function normalizedPathText(path: string): string {
+  return path.replace(/\\/gu, "/").replace(/\/+$/u, "");
+}
+
+function isForbiddenSkillsPath(path: string): boolean {
+  const normalized = normalizedPathText(path);
+  return normalized === forbiddenRelativeSkillsPath
+    || normalized === `~/${forbiddenRelativeSkillsPath}`
+    || normalized.endsWith(`/${forbiddenRelativeSkillsPath}`)
+    || forbiddenTargets.includes(resolve(path));
+}
+
 function mountInfoForPath(path: string): { mountPoint: string | null; readonly: boolean | null } {
  try {
    const target = resolve(path);
@@ -280,6 +296,9 @@ export function collectSkillAvailability(options: SkillAvailabilityOptions): Ski
  const checkedAt = new Date().toISOString();
  const mountInfo = mountInfoForPath(target);
  const exists = existsSync(target);
+  const forbiddenSourceConfigured = isForbiddenSkillsPath(source);
+  const forbiddenTargetConfigured = isForbiddenSkillsPath(target);
+  const forbiddenPathConfigured = forbiddenSourceConfigured || forbiddenTargetConfigured;
  const forbiddenPathExists = forbiddenTargets.some((path) => existsSync(path));
  let directory = false;
  let skillCount = 0;
@@ -311,7 +330,9 @@ export function collectSkillAvailability(options: SkillAvailabilityOptions): Ski

  const missingSkills = skills.filter((skill) => !skill.present || !skill.skillMdPresent).map((skill) => skill.name);
  const available = exists && directory;
-  const blocker = !available
+  const blocker = forbiddenPathConfigured
+    ? "forbidden-skills-path-configured"
+    : !available
    ? "skills-target-missing"
    : !readonly
      ? "skills-target-not-readonly"
@@ -342,12 +363,17 @@ export function collectSkillAvailability(options: SkillAvailabilityOptions): Ski
      expectedTarget,
      forbiddenPathChecked: true,
      forbiddenPathExists,
+      forbiddenPathConfigured,
+      forbiddenPathRoles: [
+        ...(forbiddenSourceConfigured ? ["source" as const] : []),
+        ...(forbiddenTargetConfigured ? ["target" as const] : []),
+      ],
      forbiddenPathMustNotBeUsed: true,
    },
-    expectedMount: `${source} mounted read-only to ${target}`,
+    expectedMount: `${defaultSource} mounted read-only to ${expectedTarget}`,
    repairHint: ok
      ? null
-      : `Mount ${source} read-only at ${target}, set UNIDESK_SKILLS_PATH=${target}, and remove any forbidden skills path spelling.`,
+      : `Mount ${defaultSource} read-only at ${expectedTarget}, set UNIDESK_SKILLS_PATH=${expectedTarget}, and remove any forbidden skills path spelling.`,
    error,
    valuesPrinted: false,
  };
@@ -360,9 +386,14 @@ export function collectSkillSyncPreflight(options: SkillSyncPreflightOptions = {
  const checkedAt = new Date().toISOString();
  const source = collectSyncPathReport(sourcePath, sourcePath === defaultSource, requiredSkills);
  const target = collectSyncPathReport(targetPath, targetPath === expectedTarget, requiredSkills);
+  const forbiddenSourceConfigured = isForbiddenSkillsPath(sourcePath);
+  const forbiddenTargetConfigured = isForbiddenSkillsPath(targetPath);
+  const forbiddenPathConfigured = forbiddenSourceConfigured || forbiddenTargetConfigured;
  const forbiddenPathExists = forbiddenTargets.some((path) => existsSync(path));
  const permissionFailures = [...source.permissionFailures, ...target.permissionFailures];
-  const blocker = !source.report.approved
+  const blocker = forbiddenPathConfigured
+    ? "forbidden-skills-path-configured"
+    : !source.report.approved
    ? "unapproved-source"
    : !target.report.approved
      ? "unapproved-target"
@@ -423,6 +454,11 @@ export function collectSkillSyncPreflight(options: SkillSyncPreflightOptions = {
      expectedTarget,
      forbiddenPathChecked: true,
      forbiddenPathExists,
+      forbiddenPathConfigured,
+      forbiddenPathRoles: [
+        ...(forbiddenSourceConfigured ? ["source" as const] : []),
+        ...(forbiddenTargetConfigured ? ["target" as const] : []),
+      ],
      forbiddenPathMustNotBeUsed: true,
    },
    plannedActions: {