From 023509d10a4caaf2864f59c7e638a931f60f04cb Mon Sep 17 00:00:00 2001 From: Codex Date: Sat, 23 May 2026 09:05:23 +0000 Subject: [PATCH] fix(code-queue): reject runner skills typo path --- docs/reference/code-queue-supervision.md | 4 +- .../code-queue-runner-skills-contract-test.ts | 107 +++++++++++++++++- scripts/src/code-queue.ts | 18 ++- scripts/src/microservices.ts | 2 + .../code-queue/src/skill-availability.ts | 44 ++++++- 5 files changed, 167 insertions(+), 8 deletions(-) diff --git a/docs/reference/code-queue-supervision.md b/docs/reference/code-queue-supervision.md index fe426346..97a6d7dc 100644 --- a/docs/reference/code-queue-supervision.md +++ b/docs/reference/code-queue-supervision.md @@ -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 请求必须失败。 diff --git a/scripts/code-queue-runner-skills-contract-test.ts b/scripts/code-queue-runner-skills-contract-test.ts index d21fc10a..ee3f0560 100644 --- a/scripts/code-queue-runner-skills-contract-test.ts +++ b/scripts/code-queue-runner-skills-contract-test.ts @@ -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, diff --git a/scripts/src/code-queue.ts b/scripts/src/code-queue.ts index c4f73451..74de9435 100644 --- a/scripts/src/code-queue.ts +++ b/scripts/src/code-queue.ts @@ -529,6 +529,15 @@ function asRecord(value: unknown): Record | null { return typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record : null; } +function compactObjectFields(record: Record | null, keys: string[]): Record | null { + if (record === null) return null; + const selected: Record = {}; + 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 | 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, }; } diff --git a/scripts/src/microservices.ts b/scripts/src/microservices.ts index 7516261b..5d2f92a9 100644 --- a/scripts/src/microservices.ts +++ b/scripts/src/microservices.ts @@ -642,6 +642,8 @@ function compactSkillAvailability(value: unknown): Record | nul "expectedTarget", "forbiddenPathChecked", "forbiddenPathExists", + "forbiddenPathConfigured", + "forbiddenPathRoles", "forbiddenPathMustNotBeUsed", ]), repairHint: skills.repairHint ?? null, diff --git a/src/components/microservices/code-queue/src/skill-availability.ts b/src/components/microservices/code-queue/src/skill-availability.ts index e030acf4..4a862bd1 100644 --- a/src/components/microservices/code-queue/src/skill-availability.ts +++ b/src/components/microservices/code-queue/src/skill-availability.ts @@ -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 = { "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: {