Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,478 changes: 0 additions & 1,478 deletions packages/core/src/services/readiness.ts

This file was deleted.

194 changes: 182 additions & 12 deletions packages/core/src/services/readiness/checkers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import path from "path";

import { fileExists, safeReadDir, readJson } from "../../utils/fs";

import type { InstructionConsistencyResult, ReadinessContext } from "./types";
import type {
InstructionConsistencyResult,
ReadinessContext,
VscodeLocationSettings
} from "./types";

export function hasAnyFile(files: string[], candidates: string[]): boolean {
return candidates.some((candidate) => files.includes(candidate));
Expand Down Expand Up @@ -98,6 +102,111 @@ export async function hasArchitectureDoc(repoPath: string): Promise<boolean> {
return fileExists(path.join(repoPath, "docs", "architecture.md"));
}

function validateAndNormalize(raw: string): string | undefined {
const trimmed = raw.trim();
if (!trimmed || path.isAbsolute(trimmed)) return undefined;
const segments = trimmed.split(/[/\\]+/u);
if (segments.some((s) => s === "..")) return undefined;
let normalized = path.normalize(trimmed).replace(/\\/gu, "/");
normalized = normalized.replace(/\/+$/u, "");
if (!normalized || normalized === ".") return undefined;
Comment on lines +107 to +112
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateAndNormalize() can incorrectly accept Windows-style absolute paths when running on POSIX. Example: a setting path like \\etc\\agents is not considered absolute by path.isAbsolute() on POSIX, but the later replace(/\\/g, "/") turns it into /etc/agents (absolute). That would make subsequent path.join(repoPath, dir) ignore repoPath and allow filesystem access outside the repo. Consider normalizing separators before the absolute-path check and/or rejecting any normalized path that starts with / or \\, and add a test case for leading-backslash inputs.

Suggested change
if (!trimmed || path.isAbsolute(trimmed)) return undefined;
const segments = trimmed.split(/[/\\]+/u);
if (segments.some((s) => s === "..")) return undefined;
let normalized = path.normalize(trimmed).replace(/\\/gu, "/");
normalized = normalized.replace(/\/+$/u, "");
if (!normalized || normalized === ".") return undefined;
if (!trimmed) return undefined;
// Normalize separators first so absolute-path detection is reliable across platforms.
const sepNormalized = trimmed.replace(/\\/gu, "/");
// Reject absolute or root-based paths (POSIX, Windows drive letters, UNC, etc.).
if (path.isAbsolute(sepNormalized) || sepNormalized.startsWith("/")) return undefined;
const segments = sepNormalized.split(/\/+/u);
if (segments.some((s) => s === "..")) return undefined;
let normalized = path.normalize(sepNormalized).replace(/\\/gu, "/");
normalized = normalized.replace(/\/+$/u, "");
if (!normalized || normalized === "." || normalized.startsWith("/")) return undefined;

Copilot uses AI. Check for mistakes.
return normalized;
}

function extractLocationPaths(entries: unknown): string[] {
if (!entries || typeof entries !== "object") return [];
const paths: string[] = [];

// Array format: [{ path: "dir" }, "dir2"]
if (Array.isArray(entries)) {
for (const entry of entries) {
let raw: string | undefined;
if (typeof entry === "string") {
raw = entry;
} else if (entry && typeof entry === "object" && !Array.isArray(entry)) {
const obj = entry as Record<string, unknown>;
if (typeof obj.path === "string") {
raw = obj.path;
}
}
if (raw) {
const normalized = validateAndNormalize(raw);
if (normalized) paths.push(normalized);
}
}
return paths;
}

// Object/map format: { "dir": true, "dir2": false }
for (const [key, value] of Object.entries(entries as Record<string, unknown>)) {
if (value !== true) continue;
const normalized = validateAndNormalize(key);
if (normalized) paths.push(normalized);
}
return paths;
}

function extractLocationsFromSettings(settings: Record<string, unknown>): VscodeLocationSettings {
return {
instructionsLocations: extractLocationPaths(settings["chat.instructionsFilesLocations"]),
agentLocations: extractLocationPaths(settings["chat.agentFilesLocations"]),
skillsLocations: extractLocationPaths(settings["chat.agentSkillsLocations"])
};
}

function mergeLocations(
a: VscodeLocationSettings,
b: VscodeLocationSettings
): VscodeLocationSettings {
return {
instructionsLocations: [...a.instructionsLocations, ...b.instructionsLocations],
agentLocations: [...a.agentLocations, ...b.agentLocations],
skillsLocations: [...a.skillsLocations, ...b.skillsLocations]
};
}

/**
* Read `chat.instructionsFilesLocations`, `chat.agentFilesLocations`, and
* `chat.agentSkillsLocations` from `.vscode/settings.json` and `*.code-workspace`
* files in the repo root. Paths are validated to be relative and free of traversal.
*/
export async function parseVscodeLocations(
repoPath: string,
rootFiles: string[]
): Promise<VscodeLocationSettings> {
const empty: VscodeLocationSettings = {
instructionsLocations: [],
agentLocations: [],
skillsLocations: []
};
let result = { ...empty };

// Read from .vscode/settings.json (JSONC — may contain comments)
const settings = await readJson(path.join(repoPath, ".vscode", "settings.json"));
if (settings) {
result = mergeLocations(result, extractLocationsFromSettings(settings));
}

// Read from *.code-workspace files in the repo root (JSONC format)
const workspaceFiles = rootFiles.filter((f) => f.endsWith(".code-workspace"));
for (const wsFile of workspaceFiles) {
const ws = await readJson(path.join(repoPath, wsFile));
if (ws?.settings && typeof ws.settings === "object" && !Array.isArray(ws.settings)) {
result = mergeLocations(
result,
extractLocationsFromSettings(ws.settings as Record<string, unknown>)
);
}
}

// Deduplicate
return {
instructionsLocations: [...new Set(result.instructionsLocations)],
agentLocations: [...new Set(result.agentLocations)],
skillsLocations: [...new Set(result.skillsLocations)]
};
}

export async function hasCustomInstructions(repoPath: string): Promise<string[]> {
const found: string[] = [];
const candidates = [
Expand All @@ -120,16 +229,35 @@ export async function hasCustomInstructions(repoPath: string): Promise<string[]>
return found;
}

export async function hasFileBasedInstructions(repoPath: string): Promise<string[]> {
const instructionsDir = path.join(repoPath, ".github", "instructions");
export async function hasFileBasedInstructions(
repoPath: string,
extraDirs?: string[]
): Promise<string[]> {
const found: string[] = [];
const defaultDir = path.join(repoPath, ".github", "instructions");
try {
const entries = await fs.readdir(instructionsDir);
return entries
.filter((e) => e.endsWith(".instructions.md"))
.map((e) => `.github/instructions/${e}`);
const entries = await fs.readdir(defaultDir);
found.push(
...entries
.filter((e) => e.endsWith(".instructions.md"))
.map((e) => `.github/instructions/${e}`)
);
} catch {
return [];
// directory doesn't exist or not readable
}
for (const dir of extraDirs ?? []) {
const fullDir = path.join(repoPath, dir);
const normalizedDir = dir.replace(/\\/gu, "/").replace(/\/+$/u, "");
try {
const entries = await fs.readdir(fullDir);
found.push(
...entries.filter((e) => e.endsWith(".instructions.md")).map((e) => `${normalizedDir}/${e}`)
);
} catch {
// directory doesn't exist or not readable
}
}
return [...new Set(found)];
}

/**
Expand Down Expand Up @@ -237,7 +365,7 @@ export async function hasMcpConfig(repoPath: string): Promise<string[]> {
return found;
}

export async function hasCustomAgents(repoPath: string): Promise<string[]> {
export async function hasCustomAgents(repoPath: string, extraDirs?: string[]): Promise<string[]> {
const found: string[] = [];
const agentDirs = [".github/agents", ".copilot/agents", ".github/copilot/agents"];
for (const dir of agentDirs) {
Expand All @@ -252,10 +380,15 @@ export async function hasCustomAgents(repoPath: string): Promise<string[]> {
found.push(agentFile);
}
}
return found;
for (const dir of extraDirs ?? []) {
if (await fileExists(path.join(repoPath, dir))) {
found.push(dir);
}
}
return [...new Set(found)];
}

export async function hasCopilotSkills(repoPath: string): Promise<string[]> {
export async function hasCopilotSkills(repoPath: string, extraDirs?: string[]): Promise<string[]> {
const found: string[] = [];
const skillDirs = [
".copilot/skills",
Expand All @@ -268,7 +401,44 @@ export async function hasCopilotSkills(repoPath: string): Promise<string[]> {
found.push(dir);
}
}
return found;
for (const dir of extraDirs ?? []) {
if (await fileExists(path.join(repoPath, dir))) {
found.push(dir);
}
}
return [...new Set(found)];
}

// ── APM (Agent Package Manager) helpers ──

export async function hasApmConfig(repoPath: string): Promise<boolean> {
return fileExists(path.join(repoPath, "apm.yml"));
}

export async function hasApmLockfile(repoPath: string): Promise<boolean> {
return fileExists(path.join(repoPath, "apm.lock.yaml"));
}

export async function hasApmInWorkflows(repoPath: string): Promise<boolean> {
const workflowDir = path.join(repoPath, ".github", "workflows");
let files: string[];
try {
files = await fs.readdir(workflowDir);
} catch {
return false;
}
for (const file of files) {
if (!file.endsWith(".yml") && !file.endsWith(".yaml")) continue;
try {
const content = await fs.readFile(path.join(workflowDir, file), "utf8");
if (/\bmicrosoft\/apm-action\b/.test(content) || /\bapm\s+(audit|install)\b/.test(content)) {
return true;
}
} catch {
// skip unreadable files
}
}
return false;
}

export async function readAllDependencies(context: ReadinessContext): Promise<string[]> {
Expand Down
92 changes: 89 additions & 3 deletions packages/core/src/services/readiness/criteria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
hasMcpConfig,
hasCustomAgents,
hasCopilotSkills,
hasApmConfig,
hasApmLockfile,
hasApmInWorkflows,
readAllDependencies,
checkInstructionConsistency
} from "./checkers";
Expand Down Expand Up @@ -302,7 +305,10 @@ export function buildCriteria(): ReadinessCriterion[] {
}

// Check for area instructions (.github/instructions/*.instructions.md)
const fileBasedInstructions = await hasFileBasedInstructions(context.repoPath);
const fileBasedInstructions = await hasFileBasedInstructions(
context.repoPath,
context.vscodeLocations?.instructionsLocations
);
const areas = context.analysis.areas ?? [];

// For monorepos or repos with detected areas, check coverage
Expand All @@ -321,6 +327,15 @@ export function buildCriteria(): ReadinessCriterion[] {
};
Comment on lines 307 to 327
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In custom-instructions, fileBasedInstructions now includes .instructions.md files from VS Code chat.instructionsFilesLocations, but the surrounding comment and success reason still describe these as "area instruction(s)". This can be misleading when the extra locations are not area-specific. Consider updating the wording (and possibly the variable name) to reflect "file-based instructions" rather than "area instructions" when non-.github/instructions/ locations are included.

Copilot uses AI. Check for mistakes.
}

// File-based instructions found (e.g. from VS Code location settings) without areas
if (fileBasedInstructions.length > 0) {
return {
status: "pass",
reason: `Root + ${fileBasedInstructions.length} file-based instruction(s) found.`,
evidence: [...rootFound, ...fileBasedInstructions]
};
}

// For monorepos without areas, check per-app instructions (legacy behavior)
if (context.analysis.isMonorepo && context.apps.length > 1) {
const appsMissing: string[] = [];
Expand Down Expand Up @@ -405,7 +420,10 @@ export function buildCriteria(): ReadinessCriterion[] {
impact: "medium",
effort: "medium",
check: async (context) => {
const found = await hasCustomAgents(context.repoPath);
const found = await hasCustomAgents(
context.repoPath,
context.vscodeLocations?.agentLocations
);
return {
status: found.length > 0 ? "pass" : "fail",
reason: "No custom AI agents configured (e.g. .github/agents/, .copilot/agents/).",
Expand All @@ -425,7 +443,10 @@ export function buildCriteria(): ReadinessCriterion[] {
impact: "medium",
effort: "medium",
check: async (context) => {
const found = await hasCopilotSkills(context.repoPath);
const found = await hasCopilotSkills(
context.repoPath,
context.vscodeLocations?.skillsLocations
);
return {
status: found.length > 0 ? "pass" : "fail",
reason: "No Copilot or Claude skills found (e.g. .copilot/skills/, .github/skills/).",
Expand All @@ -434,6 +455,71 @@ export function buildCriteria(): ReadinessCriterion[] {
};
}
},
{
id: "apm-config",
title: "APM package manifest present",
pillar: "ai-tooling",
level: 2,
scope: "repo",
impact: "medium",
effort: "low",
check: async (context) => {
const found = await hasApmConfig(context.repoPath);
return {
status: found ? "pass" : "fail",
reason: found
? undefined
: "No apm.yml found. Use APM to install shared agent packages and keep instructions in sync across repos. See: https://github.com/microsoft/apm",
evidence: ["apm.yml"]
};
}
},
{
id: "apm-locked-deps",
title: "APM dependencies locked",
pillar: "ai-tooling",
level: 3,
scope: "repo",
impact: "medium",
effort: "low",
check: async (context) => {
const hasConfig = await hasApmConfig(context.repoPath);
if (!hasConfig) {
return { status: "skip", reason: "No apm.yml found \u2014 skipping lockfile check." };
}
const found = await hasApmLockfile(context.repoPath);
return {
status: found ? "pass" : "fail",
reason: found
? undefined
: "apm.yml found but dependencies are not locked. Run `apm install` to generate apm.lock.yaml.",
evidence: ["apm.lock.yaml"]
};
}
},
{
id: "apm-ci-integration",
title: "APM integrated in CI pipeline",
pillar: "ai-tooling",
level: 4,
scope: "repo",
impact: "high",
effort: "medium",
check: async (context) => {
const hasConfig = await hasApmConfig(context.repoPath);
if (!hasConfig) {
return { status: "skip", reason: "No apm.yml found \u2014 skipping CI check." };
}
const found = await hasApmInWorkflows(context.repoPath);
return {
status: found ? "pass" : "fail",
reason: found
? undefined
: "No APM step found in CI. Add `microsoft/apm-action` to your workflow to audit agent packages on every PR. See: https://github.com/microsoft/apm-action",
evidence: [".github/workflows/*.{yml,yaml}"]
};
}
},
// ── Area-scoped criteria (only run when areaPath is set) ──
{
id: "area-readme",
Expand Down
Loading
Loading