diff --git a/.github/scripts/Test-SkillIndex.ps1 b/.github/scripts/Test-SkillIndex.ps1 index b694a5b1..5e8ddeed 100644 --- a/.github/scripts/Test-SkillIndex.ps1 +++ b/.github/scripts/Test-SkillIndex.ps1 @@ -32,7 +32,8 @@ function Assert-ThrowsLike { $generator = Join-Path $Root 'tools/Build-SkillIndex.ps1' $indexSchema = Join-Path $Root 'schemas/skill-index.schema.json' $reportSchema = Join-Path $Root 'schemas/findings-report.schema.json' -foreach ($path in $generator, $indexSchema, $reportSchema) { +$guidanceReportSchema = Join-Path $Root 'schemas/development-guidance-report.schema.json' +foreach ($path in $generator, $indexSchema, $reportSchema, $guidanceReportSchema) { if (-not (Test-Path -LiteralPath $path -PathType Leaf)) { throw "Required contract file not found: $path" } @@ -111,6 +112,61 @@ try { } } + $guidance = @($skills | Where-Object id -eq 'al-development-plan') + if ($guidance.Count -ne 1) { + throw "Expected exactly one al-development-plan record, found $($guidance.Count)." + } + if ((@($guidance[0].inputs) -join "`n") -cne ("development-plan`nrepository")) { + throw 'al-development-plan inputs were not indexed in declared order.' + } + if ((@($guidance[0].outputs) -join "`n") -cne 'development-guidance-report') { + throw 'al-development-plan output kind was not preserved in the skill index.' + } + if (@($guidance[0].subSkills).Count) { + throw 'al-development-plan must remain a leaf action skill.' + } + + $minimalGuidanceReport = @{ + skill = @{ id = 'al-development-plan'; version = 1 } + outcome = 'completed' + summary = @{ + request = 'Enrich the existing plan.' + kind = 'feature' + candidates = 1 + selected = 1 + } + context = @{ + 'bc-version' = '28' + technologies = @('al') + countries = @('w1') + 'application-area' = @('all') + unknown = @() + } + knowledge = @(@{ + path = 'microsoft/knowledge/performance/apply-filters-before-iterating.md' + 'used-for' = 'Constrain filtered iteration.' + constraints = @('Apply filters before iterating.') + 'sample-paths' = @() + }) + 'validation-considerations' = @() + suppressed = @() + unresolved = @() + } | ConvertTo-Json -Depth 10 + if (-not ($minimalGuidanceReport | Test-Json -SchemaFile $guidanceReportSchema -ErrorAction Stop)) { + throw 'Minimal development-guidance report does not satisfy schemas/development-guidance-report.schema.json.' + } + + $failedGuidanceReport = $minimalGuidanceReport | ConvertFrom-Json + $failedGuidanceReport.outcome = 'failed' + $failedGuidanceReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Retrieval failed.' + if (($failedGuidanceReport | ConvertTo-Json -Depth 10) | Test-Json -SchemaFile $guidanceReportSchema -ErrorAction SilentlyContinue) { + throw 'A failed development-guidance report with knowledge must not satisfy its JSON schema.' + } + $failedGuidanceReport.knowledge = @() + if (-not (($failedGuidanceReport | ConvertTo-Json -Depth 10) | Test-Json -SchemaFile $guidanceReportSchema -ErrorAction Stop)) { + throw 'A failed development-guidance report with empty knowledge must satisfy its JSON schema.' + } + $minimalReport = @{ skill = @{ id = 'al-style-review'; version = 1 } outcome = 'completed' diff --git a/.github/scripts/validate_frontmatter.py b/.github/scripts/validate_frontmatter.py index 20f33d69..95df11a9 100644 --- a/.github/scripts/validate_frontmatter.py +++ b/.github/scripts/validate_frontmatter.py @@ -18,7 +18,7 @@ import re import sys from dataclasses import dataclass, field -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any, Iterable try: @@ -46,8 +46,9 @@ STANDARD_INPUTS = { "pr-diff", "object-list", "file-path", "folder-path", "repository", "telemetry-query", + "development-plan", } -ALLOWED_OUTPUTS = {"findings-report"} +ALLOWED_OUTPUTS = {"findings-report", "development-guidance-report"} VALID_SAMPLE_KINDS = {"good", "bad"} ACTION_SKILL_SECTIONS = ["Source", "Relevance", "Worklist", "Action", "Output"] @@ -147,6 +148,28 @@ def is_non_empty_list_of_str(value: Any) -> bool: return isinstance(value, list) and len(value) > 0 and all(isinstance(v, str) and v for v in value) +def normalize_repo_md_path(value: Any) -> tuple[str | None, str | None]: + """Validate a canonical repo-relative Markdown path.""" + if not isinstance(value, str) or not value: + return None, "must be a non-empty string" + if "\\" in value: + return None, "must use forward slashes" + + normalized = value + segments = value.split("/") + if ( + not normalized + or normalized.startswith("/") + or re.match(r"^[A-Za-z]:", normalized) + or any(segment in ("", ".", "..") for segment in segments) + or PurePosixPath(normalized).is_absolute() + ): + return None, "must be a repository-relative path that does not escape the repository" + if not normalized.endswith(".md"): + return None, "must end in '.md'" + return normalized, None + + def expand_bc_version(value: Any) -> tuple[list[int] | str | None, str | None]: """Return (expanded, error-message). One of the two is None. @@ -340,9 +363,11 @@ def validate_action_skill(path: Path, parsed: Parsed, report: Report) -> None: if not is_non_empty_list_of_str(out): report.error(path, "R18", "outputs must be a non-empty list of strings", 1) else: + if len(out) != 1: + report.error(path, "R18", "outputs must contain exactly one output kind", 1) bad = [x for x in out if x not in ALLOWED_OUTPUTS] if bad: - report.error(path, "R18", f"outputs contains non-allowed values {bad}; currently only {sorted(ALLOWED_OUTPUTS)} is defined", 1) + report.error(path, "R18", f"outputs contains non-allowed values {bad}; allowed values are {sorted(ALLOWED_OUTPUTS)}", 1) # R19 optional filter dimensions, if present if "bc-version" in fm: @@ -378,20 +403,9 @@ def validate_action_skill(path: Path, parsed: Parsed, report: Report) -> None: if not is_non_empty_list_of_str(ss): report.error(path, "R20", "sub-skills must be a non-empty list of repo-relative paths", 1) else: - bad = [x for x in ss if not x.endswith(".md")] + bad = [f"{x}: {err}" for x in ss if (err := normalize_repo_md_path(x)[1])] if bad: - report.error(path, "R20", f"sub-skills entries must end in '.md': {bad}", 1) - non_canonical = [ - x for x in ss - if "\\" in x or x.startswith("/") or ".." in Path(x).parts or x.startswith("./") - ] - if non_canonical: - report.error( - path, - "R20", - f"sub-skills entries must be canonical repo-relative paths: {non_canonical}", - 1, - ) + report.error(path, "R20", f"invalid sub-skills paths: {bad}", 1) duplicates = sorted({x for x in ss if ss.count(x) > 1}) if duplicates: report.error(path, "R20", f"sub-skills contains duplicate paths: {duplicates}", 1) @@ -598,7 +612,11 @@ def validate_sub_skills_registry( if not is_non_empty_list_of_str(ss): return - declared = {s.lstrip("./") for s in ss} + declared = { + normalized + for s in ss + if (normalized := normalize_repo_md_path(s)[0]) is not None + } # Sibling leaves on disk, excluding the super-skill file itself. leaves = { diff --git a/README.md b/README.md index 9aaea9a6..45893ef0 100644 --- a/README.md +++ b/README.md @@ -26,10 +26,25 @@ copilot plugin install microsoft/BCQuality copilot plugin list ``` -The list should include `bcquality`. The plugin currently exposes the +The list should include `bcquality`. The plugin exposes the [`al-code-review`](skills/al-code-review/SKILL.md) skill. Installation and skill discovery are the general pattern; reviewing an app is one example of using it. +The adapter is intentionally not a second implementation: + +```text +standalone host skill: skills/al-code-review/SKILL.md + -> routing contract: skills/entry.md + -> review coordinator: microsoft/skills/review/al-code-review.md + -> domain review leaves +``` + +Only the files under `skills/*/SKILL.md` follow the host's packaging format. +The remaining files are BCQuality's internal protocol and layered action +skills. Entry remains the single owner of routing and index preparation. This +separation keeps standalone installation available without duplicating policy +in the adapter. + ### Example: Review a complete app folder Start a **new** CLI session in your own app folder, replacing the example path: @@ -48,6 +63,10 @@ Approve access only to a project you trust, then ask: The folder should contain `app.json` and your AL source; it does **not** need to be a Git repository. On macOS or Linux, use your app's local path instead. +The host adapter and internal action skill intentionally share a name: they +expose the same operation in two different skill formats. Their paths make the +boundary explicit. + Expect a report for each selected review, with findings, source locations, severity, confidence, and references to the relevant guidance. Some hosts show the structured JSON directly. `completed` with no findings means nothing was @@ -83,12 +102,38 @@ available domains and the difference between a folder review and a comparison. Mechanical issues already enforced by the AL compiler or standard analyzers are intentionally left to those deterministic tools rather than duplicated here. +BCQuality defines a provisional internal, read-only `al-development-plan` +action-skill contract that selects relevant constraints before a consumer +implements its own existing plan. It is not registered as a standalone plugin +skill and does not change the plugin version. Consumer-owner agreement and a +runtime pilot are required before treating it as a stable public surface. + +Repository-specific orchestrators retain planning, implementation, approvals, +tests, environment, propagation, and delivery ownership. The intended flow is +consumer analysis and normalized plan -> read-only BCQuality guidance -> +existing implementation phases -> independent final BCQuality review -> +delivery. Consumer uptake and a real runtime pilot are follow-up work, not +implemented integrations or demonstrated authoring improvements. + +`no-knowledge` means no additional applicable BCQuality constraints, with empty +`knowledge`; it does not make a plan unsafe or prevent the consumer from using +its ordinary gates. Retrieval failures and materially unresolved conditional +guidance are distinct outcomes, not empty knowledge. Do not add generic advice +just to avoid a `no-knowledge` result. + The [SCM domain](microsoft/knowledge/scm/) covers selected inventory, costing, reservation, tracking, and warehouse/posting workflows, not exhaustive supply chain validation. Broader functional coverage such as Finance, Manufacturing, Jobs, and Service, and technologies such as PowerShell, pipelines, and Power Platform, remain valid future scope, **not current coverage claims**. +## Plan-enrichment follow-up scope + +Consumer agreement, consumer-owned persistence and phase injection, a pinned +baseline comparison, and a runtime pilot remain follow-up work. BCQuality does +not claim improved repairs or authoring effectiveness from this provisional +contract alone. + ## What's in this repo Knowledge articles cover one concern each. Skills tell an agent how to find @@ -100,6 +145,12 @@ and apply the relevant knowledge. Both live in three layers: | [Community](community/) | Community-owned skills and their knowledge. | | [Custom](custom/) | Organization-specific additions and overrides in your own fork. | +Review skills emit a `findings-report`; plan enrichment emits a read-only +`development-guidance-report`. Both contracts are defined in +[`skills/do.md`](skills/do.md). See +[how agents consume BCQuality](docs/agent-consumption.md) for the integration +flow. + All three are enabled by default; Custom is empty upstream. You do not need to configure layers to get started. diff --git a/docs/agent-consumption.md b/docs/agent-consumption.md index 06858c0a..75e6262d 100644 --- a/docs/agent-consumption.md +++ b/docs/agent-consumption.md @@ -78,8 +78,9 @@ Add scheduling, retries, and rendering only when needed, using the - **Layer content** in `/microsoft/`, `/community/`, and `/custom/` — knowledge files and action skills grouped by authority. When BCQuality is installed as a standalone plugin, it additionally exposes -`skills/al-code-review/SKILL.md`. This is a host-format adapter, not another -action skill: it creates the task context and enters the same flow at Entry. +`skills/al-code-review/SKILL.md`. This is a host-format adapter, not an +additional action skill: it creates the task context and enters the same flow +at Entry. ## Repository structure @@ -109,7 +110,7 @@ flowchart LR E -->|3 dispatch record| A A -->|4 invoke dispatched skill| S[Action skill
e.g. al-code-review] S -->|5 execute| P[Source → Relevance
→ Worklist → Action
reading READ · DO on demand] - P -->|6 emit| R[Findings · Domain labels
· References · Confidence] + P -->|6 emit| R[Findings report
or read-only guidance report] R -->|7 integrate| O ``` @@ -119,14 +120,17 @@ The orchestrator has a URL setting that points at BCQuality (default: `github.co ### 2. Agent invokes Entry The agent reads `/skills/entry.md` and runs it against the task context. Entry applies its Source → Relevance → Worklist → Action steps over the action skills under `*/skills/**/*.md` and returns a **dispatch record**: the set of action skills to invoke, plus a list of candidates it skipped (with reasons). Routing is a skill, not orchestrator logic. -For a standalone plugin installation, the host activates the -`skills/al-code-review/SKILL.md` adapter first. That adapter preserves the -caller's actual goal, constructs the task context, and invokes Entry. It does -not select the internal `microsoft/skills/review/al-code-review.md` action skill +For a standalone plugin installation, the host activates the review adapter +first. The adapter preserves the caller's actual goal, constructs the task +context, and invokes Entry. It does not select the internal review action skill itself or duplicate Entry's preparation, routing, and failure semantics. ### 3. Agent consumes the dispatch record -The dispatch record names one or more action skills and the subset of inputs each should receive. If the outcome is `no-match` or `failed`, the agent returns the record to the orchestrator unchanged. +The dispatch record names one or more action skills, the subset of inputs each +should receive, and each skill's output kind. The output kind distinguishes +findings from read-only plan guidance before invocation; it is not proof of +runtime side effects. If the outcome is `no-match` or `failed`, the agent returns +the record to the orchestrator unchanged. ### 4. Agent invokes each dispatched action skill Action skills live inside the layers — `/microsoft/skills/`, `/community/skills/`, `/custom/skills/` — so their authority is carried by their location. For a PR review, Entry typically dispatches `microsoft/skills/review/al-code-review.md`. The agent reads the file and executes it. @@ -165,19 +169,87 @@ security boundary. See [layer selection](customizing-bcquality.md#select-layers- The index changes only *how candidates are discovered*, never *which are selected*. The Worklist predicate is unchanged — `keywords` still drive selection — and the agent still opens each worklisted article **in full** to read its `## Best Practice` / `## Anti Pattern` rule bodies; the index is discovery metadata only and never substitutes for the article body. When no index is present, skills fall back to path-based discovery (collect by domain folder), so review still works. ### 6. Agent emits structured output -The output contract is defined in the DO meta-skill so that every action skill — today's and next year's — produces the same shape: +The output contracts are defined in the DO meta-skill: -- **Outcome** — `completed`, `not-applicable`, `no-knowledge`, `partial`, or `failed`. An orchestrator can distinguish a clean run from a no-op from a failure without guessing. -- **Findings** — what the skill observed (severity, message, optional location). -- **Domain** — the producer-owned, human-readable display label on each review finding. -- **References** — structured objects (`path` plus optional commit `sha`) pointing to the knowledge files that informed each finding. -- **Confidence** — per-finding evidence strength. -- **Suppressed** — knowledge files that were discarded by layer precedence or configuration, so reviewers can see what was overridden. +- A **findings report** carries review findings, domain labels, references, confidence, and suppressions. +- A **development guidance report** carries read-only knowledge constraints and validation considerations for an existing plan. The orchestrator parses this **without skill-specific logic**. This is the point of the contract: orchestrators and action skills evolve independently. +For plan enrichment, the skill reads the existing plan and target repository, +selects applicable knowledge, and returns constraints without changing the +target. It does not generate a replacement plan, run tests, implement code, or +drive a review/fix loop. Implementation stays in the consuming workflow. + ### 7. Orchestrator integrates -The orchestrator turns findings into PR comments, build gates, or IDE diagnostics, and links the references back to the knowledge files so the PR author — human or agent — can read the guidance. +The orchestrator turns findings into PR comments, build gates, or IDE diagnostics. It can feed read-only guidance into its own implementation phases, preserving all existing approvals and delivery gates. + +## Provisional repository-specific development integration + +A repository-specific workflow can consume this read-only foundation before +authoring while retaining its independent final review. This is the intended +integration boundary proposed by `al-development-plan`, not a shipped consumer +integration or a stable plugin surface: + +1. Investigate and produce the consumer's normal initial plan. Normalize any + consumer-specific format outside BCQuality. A full serialized plan document + containing metadata plus a markdown body (root cause or design intent, + proposed changes, affected files, test strategy, acceptance criteria) is a + valid boundary. A continuation/checkpoint payload is not a substitute for + initial-plan coverage; workflow identifiers and state stay with the consumer. +2. Resolve and record an immutable BCQuality checkout and filtering policy. + Invoke Entry with a read-only enrichment goal, the existing + `development-plan`, `repository`, and established applicability dimensions. + Keep index, guidance, and runner artifacts outside the target repository. +3. Execute the dispatched `al-development-plan` skill. Persist the unchanged + report and provenance **after** any consumer state initialization or cleanup + that could erase them. BCQuality does not own the state directory or lifecycle. +4. Inject relevant constraints and validation considerations into the existing + Baseline, Implement, propagation (such as MiApp), and Critique phases, or + equivalents. Re-enrich on material plan or applicability changes; preserve + the relationship between plan version, guidance, and implementation attempt. +5. Run an independent final BCQuality review against the completed diff using + the **same recorded immutable checkout** used for enrichment. Review the + actual changes, not the guidance report as proof of correctness, then apply + the consumer's ordinary delivery gates. + +The consumer owns analysis, normalization, persistence, per-phase injection, +approvals, TDD and runtime execution, propagation, retries, commits, and PR +delivery. BCQuality supplies additional referenced product knowledge, not a +replacement orchestrator. + +Consumer owners must agree the input/output contract and insertion point before +adoption. Runtime mutation-proofing and effectiveness measurement belong to the +consumer pilot rather than the BCQuality content repository. + +### Outcomes are additive, not a universal coding gate + +`no-knowledge` with empty `knowledge` means no additional applicable BCQuality +constraints. The consumer may proceed under its ordinary gates. It must not be +conflated with failed retrieval/reference integrity (`failed`), incomplete +evaluation or materially unresolved conditional guidance (`partial`), or absent +required inputs (`not-applicable`). Consumers own the policy for handling those +outcomes and recorded unknowns: seek missing context, re-enrich, escalate, or +apply their existing risk controls without relabeling the report as successful. +Do not fill gaps with generic articles simply to unlock implementation. + +### Pinning and pilot evidence + +A configured tag or ref alone does not establish runtime pinning. Record the +resolved commit and actual checkout/content identity used at invocation, along +with enabled layers, pruning policy, index identity, plan version, and runner +provenance. Verify that identity at both enrichment and final review; fetching +default HEAD into an explicitly supplied checkout can bypass a configured ref. +Use an isolated checkout that cannot drift during the run. + +Consumer rollout and an external pilot remain follow-up work. A pilot must +compare a pinned independent baseline run of the existing workflow without +enrichment against a matched enriched run, keeping starting code, task, model, +tools, runtime, and gates controlled and recording the actual BCQuality +checkout. Retain external logs, diffs, test/compile outcomes, and independent +final reviews, including failures and unresolved results. Credential-free +fixture preparation and scorer regressions do not demonstrate improved repair +quality, compilation, runtime success, or production integration. ## Knowledge-backed and agent findings diff --git a/microsoft/skills/development/al-development-plan.md b/microsoft/skills/development/al-development-plan.md new file mode 100644 index 00000000..d3d65d57 --- /dev/null +++ b/microsoft/skills/development/al-development-plan.md @@ -0,0 +1,74 @@ +--- +kind: action-skill +id: al-development-plan +version: 1 +title: AL development plan guidance +description: Produces a read-only BCQuality knowledge bundle for an existing Business Central AL development plan. +inputs: [development-plan, repository] +outputs: [development-guidance-report] +bc-version: [all] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# AL development plan guidance + +Selects the BCQuality knowledge that should constrain an existing AL development plan. It does not implement, edit, stage, commit, or publish anything in the target repository. Repository-specific orchestrators can consume this skill before their own test and implementation phases while retaining ownership of workflow, tooling, and delivery. + +A non-empty `development-plan` is required. A readable `repository` is optional but recommended: use it to confirm affected symbols and applicability context when supplied. When it is absent, preserve repository-dependent facts as unknown and return `partial` only when that missing context materially prevents reliable selection. Return `not-applicable` without changing files when the plan is absent or does not identify the intended change. + +The caller supplies its existing plan, not a request to generate one. Consumer-specific formats must be normalized by the consumer before invocation. This skill does not interpret issue records, continuation markers, batons, retries, or workflow state. A serialized document containing plan metadata and a markdown body is acceptable when it states the intended change, affected surfaces, proposed approach, test strategy, and acceptance criteria. Missing details remain unknown; do not invent them. + +## Source + +Read the BCQuality knowledge index once, using the external path supplied by Entry when present. If no index is available, use READ's path-based discovery across enabled layers; inability to read the corpus is `failed`, not `no-knowledge`. Use entries from every enabled layer and domain. The index supplies candidate paths, applicability dimensions, keywords, titles, and descriptions; it never substitutes for opening selected articles in full. + +When supplied, inspect the target repository read-only for `app.json`, affected files and symbols named by the plan, relevant tests, permission sets, dependencies, target/runtime versions, countries, application areas, and repository conventions. Do not create scratch or generated files inside the target repository. + +## Relevance + +Apply READ's matching semantics using: + +- `bc-version` from the plan, target application, or supplied context; for upgrades, distinguish source and target versions. +- `technologies` from the affected files, beginning with `[al]`. +- `countries` from the plan, `app.json`, or workspace configuration. +- `application-area` from the plan and affected objects. + +When a dimension cannot be resolved, retain conditionally applicable candidates only when they can materially constrain the plan. Record the dimension in `context.unknown` and explain it in `unresolved`; do not silently treat it as a match. + +## Worklist + +1. Read the supplied plan for its request summary, development kind, assumptions, root cause or design intent, affected files and symbols, proposed changes, test strategy, and acceptance criteria. Preserve its intent; do not generate a replacement plan. If kind is not explicit, classify the stated intent: new or expanded behavior is `feature`, a defect correction is `bug`, behavior-preserving restructuring is `refactor`, migration is `upgrade`, and other bounded work is `maintenance`. Do not redesign the consumer's workflow. +2. Build retrieval vocabulary from the plan and confirmed repository symbols. Give exact object types, properties, methods, analyzers, errors, and affected domains more weight than broad business nouns. +3. Search the index in separate passes: + - data ownership, keys, setup, numbering, validation, transactions, and upgrade; + - behavior, events, interfaces, errors, permissions, privacy, and telemetry; + - pages, reports, APIs, integrations, localization, and accessibility; + - tests, analyzers, packaging, and deployment constraints. +4. Add an article when its keywords or indexed topic match a concrete planned change, affected symbol, acceptance criterion, or validation obligation. Applicability alone is not enough. +5. Open every selected article in full. Read any referenced `.good.*` and `.bad.*` sibling needed to make the constraint concrete. Never cite an index row that was not opened. +6. Resolve contradictory normative guidance with READ's layer precedence and record losing candidates in `suppressed`. +7. Check the resulting worklist across the whole plan. A bug fix may require testing, data, performance, and upgrade guidance at once; a feature plan may require security and lifecycle constraints that are not named in its title. + +Keep the worklist focused. Do not include generic engineering advice, an entire domain, or an article that would not change implementation or validation. + +This skill deliberately does not dispatch the review leaves. Review leaves inspect existing source and emit defects through domain-specific code signals; plan enrichment runs before that source exists and must collect constraints that cross several domains. Both paths consume the same indexed article metadata and full normative article bodies, so new knowledge is automatically eligible for plan retrieval. Review-leaf token maps remain code-detection precision rules, not a second registry that plan retrieval must duplicate. + +## Action + +For each worklist article: + +1. Copy its exact path and optional commit SHA. +2. State `used-for` as the concrete plan decision or affected surface. +3. Translate its normative Best Practice and Anti Pattern into short implementation constraints without adding facts or weakening conditions. +4. Include only opened, existing sibling samples in `sample-paths`. +5. Derive validation considerations only where the plan or selected knowledge requires observable evidence. Describe the evidence to obtain; do not claim it already exists or passed. + +Do not change the target repository. Before emitting, verify every knowledge and sample path exists in the live BCQuality checkout and was opened during this run. If reference integrity cannot be established, return `failed` rather than fabricating guidance. + +## Output + +Return one `development-guidance-report` conforming to DO. `completed` requires that every selected article was opened and faithfully converted into constraints, with no material unresolved applicability. `no-knowledge` means there are no additional applicable BCQuality constraints for this plan; emit empty `knowledge`. It does not mean the work is unsafe or unimplementable, and the consumer can proceed under its ordinary gates. Never add generic or filler guidance to avoid this outcome. + +Return `partial` for incomplete evaluation or materially unresolved conditional guidance, naming every gap. Failed retrieval or reference-integrity checks are `failed`, never `no-knowledge`. Consumers own handling of partial, failed, and unresolved results, including clarification and re-enrichment; this read-only interface does not define a universal implementation gate. diff --git a/schemas/development-guidance-report.schema.json b/schemas/development-guidance-report.schema.json new file mode 100644 index 00000000..b438e3c0 --- /dev/null +++ b/schemas/development-guidance-report.schema.json @@ -0,0 +1,183 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://github.com/microsoft/BCQuality/schemas/development-guidance-report.schema.json", + "title": "BCQuality development-guidance report", + "type": "object", + "additionalProperties": false, + "required": [ + "skill", + "outcome", + "summary", + "context", + "knowledge", + "validation-considerations", + "suppressed", + "unresolved" + ], + "properties": { + "skill": { + "type": "object", + "additionalProperties": false, + "required": ["id", "version"], + "properties": { + "id": { "type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$" }, + "version": { "type": "integer", "minimum": 1 } + } + }, + "outcome": { + "enum": ["completed", "not-applicable", "no-knowledge", "partial", "failed"] + }, + "outcome-reason": { "type": "string", "minLength": 1 }, + "summary": { + "type": "object", + "additionalProperties": false, + "required": ["request", "kind", "candidates", "selected"], + "properties": { + "request": { "type": "string", "minLength": 1 }, + "kind": { "enum": ["feature", "bug", "refactor", "upgrade", "maintenance"] }, + "candidates": { "type": "integer", "minimum": 0 }, + "selected": { "type": "integer", "minimum": 0 } + } + }, + "context": { + "type": "object", + "additionalProperties": false, + "required": [ + "bc-version", + "technologies", + "countries", + "application-area", + "unknown" + ], + "properties": { + "bc-version": { "type": "string", "minLength": 1 }, + "technologies": { "$ref": "#/definitions/stringArray" }, + "countries": { "$ref": "#/definitions/stringArray" }, + "application-area": { "$ref": "#/definitions/stringArray" }, + "unknown": { + "type": "array", + "uniqueItems": true, + "items": { + "enum": ["bc-version", "technologies", "countries", "application-area"] + } + } + } + }, + "knowledge": { + "type": "array", + "uniqueItems": true, + "items": { "$ref": "#/definitions/knowledge" } + }, + "validation-considerations": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["id", "reason", "evidence"], + "properties": { + "id": { "type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$" }, + "reason": { "type": "string", "minLength": 1 }, + "evidence": { "type": "string", "minLength": 1 } + } + } + }, + "suppressed": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["reference", "reason"], + "properties": { + "reference": { "$ref": "#/definitions/reference" }, + "reason": { "enum": ["layer-precedence", "configuration"] } + } + } + }, + "unresolved": { "$ref": "#/definitions/stringArray" } + }, + "allOf": [ + { + "if": { + "properties": { + "outcome": { "enum": ["partial", "failed"] } + }, + "required": ["outcome"] + }, + "then": { "required": ["outcome-reason"] } + }, + { + "if": { + "properties": { + "outcome": { "const": "completed" } + }, + "required": ["outcome"] + }, + "then": { + "properties": { + "knowledge": { "minItems": 1 } + } + } + }, + { + "if": { + "properties": { + "outcome": { "enum": ["not-applicable", "no-knowledge", "failed"] } + }, + "required": ["outcome"] + }, + "then": { + "properties": { + "knowledge": { "maxItems": 0 } + } + } + } + ], + "definitions": { + "stringArray": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 } + }, + "reference": { + "type": "object", + "additionalProperties": false, + "required": ["path"], + "properties": { + "path": { + "type": "string", + "pattern": "^(microsoft|community|custom)/knowledge/[a-z0-9-]+/[a-z0-9-]+\\.md$" + }, + "sha": { "type": "string", "pattern": "^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$" } + } + }, + "knowledge": { + "type": "object", + "additionalProperties": false, + "required": ["path", "used-for", "constraints", "sample-paths"], + "properties": { + "path": { + "type": "string", + "pattern": "^(microsoft|community|custom)/knowledge/[a-z0-9-]+/[a-z0-9-]+\\.md$" + }, + "sha": { "type": "string", "pattern": "^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$" }, + "used-for": { "type": "string", "minLength": 1 }, + "constraints": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 } + }, + "sample-paths": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^(microsoft|community|custom)/knowledge/[a-z0-9-]+/[a-z0-9-]+\\.(good|bad)\\.[a-zA-Z0-9]+$" + } + } + } + } + } +} diff --git a/schemas/skill-index.schema.json b/schemas/skill-index.schema.json index 01944bb9..8e11cca1 100644 --- a/schemas/skill-index.schema.json +++ b/schemas/skill-index.schema.json @@ -48,7 +48,9 @@ "type": "array", "minItems": 1, "maxItems": 1, - "items": { "const": "findings-report" } + "items": { + "enum": ["findings-report", "development-guidance-report"] + } }, "filters": { "type": "object", diff --git a/skills/do.md b/skills/do.md index e67faf66..3d367458 100644 --- a/skills/do.md +++ b/skills/do.md @@ -7,7 +7,7 @@ title: Action Skill — the template every action skill follows # DO -An action skill is a markdown file that tells an agent how to do one concrete job — review a pull request, audit telemetry usage, generate a skeleton — using knowledge files from BCQuality. This document is the template every action skill follows. Orchestrators rely on the template to consume any skill without skill-specific parsing. +An action skill is a markdown file that tells an agent how to do one concrete job — review a pull request, audit telemetry usage, enrich an existing plan — using knowledge files from BCQuality. This document is the template every action skill follows. Orchestrators rely on the template to consume any skill without skill-specific parsing. This contract is stable. Changes require a PR approved by both maintainers. @@ -23,7 +23,7 @@ Action skills do not live at the repo root. Layer-independent files in `/skills/` contain the three meta-skill contracts (READ, DO, WRITE), the entry-point skill (`entry.md`, `kind: entry-point`), and host-format adapters. Adapters are not action skills. Entry structurally follows the same -four-step pattern but produces a dispatch record rather than a findings-report; +four-step pattern but produces a dispatch record rather than an action-skill report; see [entry.md](entry.md) for its contract. ## Skills hold mechanics; knowledge files hold BC facts @@ -62,12 +62,21 @@ application-area: [all] `bc-version`, `technologies`, `countries`, `application-area` are optional filters that let an orchestrator pre-select applicable skills for a task. They follow the same semantics as in READ. `inputs` is a list of abstract input types the skill **accepts**. Standard values: -`pr-diff`, `object-list`, `file-path`, `folder-path`, `repository`, and -`telemetry-query`. Semantics are any-of: the orchestrator supplies whichever -listed input types it has, and the skill is invoked with a non-empty subset of -its declared `inputs`. A skill that cannot proceed with the supplied subset -MUST return `outcome: "not-applicable"`. `outputs` is always a single-element -list naming the output kind; today only `findings-report` is defined. +`pr-diff`, `object-list`, `file-path`, `folder-path`, `repository`, +`telemetry-query`, and `development-plan`. Semantics are any-of: the +orchestrator supplies whichever listed input types it has, and the skill is +invoked with a non-empty subset of its declared `inputs`. A skill that cannot +proceed with the supplied subset MUST return `outcome: "not-applicable"`. + +`outputs` is always a single-element list naming the output kind: + +- `findings-report` — evaluates an input and reports defects or observations. +- `development-guidance-report` — selects and summarizes applicable BCQuality knowledge for an existing development plan without changing the target repository. + +`development-plan` and `development-guidance-report` are provisional contract +extensions. They are intentionally not exposed by the standalone plugin in this +release. Consumer-owner agreement and pilot evidence are required before they +are treated as frozen public integration surfaces. `file-path` is one file. `folder-path` is a directory whose recursively contained files form the complete current-state input, such as a Business @@ -98,13 +107,23 @@ Every action skill MUST contain these five sections, in order: **Relevance.** Apply frontmatter filters to the candidates. Typical filters: match `bc-version` against the target environment, match `technologies` against the languages in scope, match `countries` and `application-area` against the consuming codebase's context. The exact matching rules are defined in READ (*Frontmatter matching semantics*). Files that do not match are discarded. -**Worklist.** Narrow the relevant candidates to the subset that applies to the current task. This is where the task-specific signal enters: the objects changed in the PR, the queries being audited, the skeleton being generated. Typical moves: match `keywords` against task vocabulary, match file topics against changed objects, deduplicate by concern. +**Worklist.** Narrow the relevant candidates to the subset that applies to the current task. This is where the task-specific signal enters: the objects changed in the PR, the queries being audited, the existing plan being enriched. Typical moves: match `keywords` against task vocabulary, match file topics against changed objects, deduplicate by concern. **Action.** Execute the skill's work against the worklist. Evaluate each item in the worklist against the task input and emit findings. The action step is where skill behavior differs; the preceding three steps are uniform. -## Output contract +Review and plan enrichment use different Worklist signals by design. Review +leaves inspect existing source and use domain-specific code tokens to decide +which rules can produce findings. Plan enrichment precedes implementation and +selects cross-domain constraints from plan vocabulary and confirmed repository +symbols. Both use the same knowledge index, applicability semantics, layer +precedence, and full normative article bodies; review-leaf cue lists are not a +second knowledge registry. -Every action skill emits a single JSON document that conforms to this schema: + + +## Findings-report contract + +An action skill with `outputs: [findings-report]` emits a single JSON document that conforms to this schema: The machine-readable structural schema is [`schemas/findings-report.schema.json`](../schemas/findings-report.schema.json). @@ -335,6 +354,81 @@ Severity taxonomy: - `minor` — quality concern; worth flagging but not a gate. - `info` — observation or context; not actionable on its own. +## Development-guidance-report contract + +An action skill with `outputs: [development-guidance-report]` emits one JSON document: + +The provisional machine-readable structural schema is +[`schemas/development-guidance-report.schema.json`](../schemas/development-guidance-report.schema.json). +The semantic rules below remain authoritative for count arithmetic, exact +reference existence, applicability, and normative constraint fidelity. + +```json +{ + "skill": { "id": "string", "version": 1 }, + "outcome": "completed | not-applicable | no-knowledge | partial | failed", + "outcome-reason": "string", + "summary": { + "request": "string", + "kind": "feature | bug | refactor | upgrade | maintenance", + "candidates": 0, + "selected": 0 + }, + "context": { + "bc-version": "string", + "technologies": ["string"], + "countries": ["string"], + "application-area": ["string"], + "unknown": ["bc-version | technologies | countries | application-area"] + }, + "knowledge": [ + { + "path": "string", + "sha": "string", + "used-for": "string", + "constraints": ["string"], + "sample-paths": ["string"] + } + ], + "validation-considerations": [ + { + "id": "string", + "reason": "string", + "evidence": "string" + } + ], + "suppressed": [ + { + "reference": { "path": "string", "sha": "string" }, + "reason": "layer-precedence | configuration" + } + ], + "unresolved": ["string"] +} +``` + +The skill is read-only with respect to the target repository: no edits, generated files, staging, commits, or publication. Keep index, report, and scratch artifacts outside that repository. The report is strict JSON with no surrounding commentary. The caller supplies an existing plan and may supply a readable repository; consumer-specific input normalization and workflow state are outside this contract. + +### Guidance outcome semantics + +- `completed` — evaluation finished, at least one article was selected, every selected article was opened and faithfully converted into constraints, and no materially unresolved conditional guidance remains. +- `not-applicable` — the required existing plan is absent, does not identify the intended change, or is outside the skill's applicability. No constraints are claimed. +- `no-knowledge` — evaluation finished and there are **no additional applicable BCQuality constraints** for this plan. `knowledge` is empty. This is not a statement that the work is unsafe or unimplementable; the consuming workflow can proceed under its ordinary gates. Do not add generic or filler articles to avoid this outcome. +- `partial` — evaluation is incomplete or conditional guidance remains materially unresolved. Name each gap in `outcome-reason` and `unresolved`; do not silently treat an unknown dimension as a match. +- `failed` — retrieval, reference integrity, or another error prevents a reliable report. Set `outcome-reason`; consumers must not treat the result as reliable constraints or as `no-knowledge`. + +`outcome-reason` is required for `partial` and `failed`, optional otherwise. These outcomes describe enrichment only, not permission to implement or deliver. The consumer owns handling of partial, failed, and unresolved guidance, including escalation, clarification, and re-enrichment; BCQuality does not impose a universal implementation gate. + +### Guidance field semantics + +`summary.request` preserves the planned intent and `kind` classifies it without replacing the plan. `candidates` and `selected` are non-negative integer counts: selected equals the number of distinct article paths in `knowledge` and cannot exceed candidates. Each article path appears at most once; combine its uses and constraints into that entry. Counts are retrieval diagnostics, not capability or authoring-quality scores. + +`knowledge[].constraints` is a non-empty list summarizing only normative `## Best Practice` and `## Anti Pattern` content from the referenced article. It must not introduce a Business Central fact absent from that article. `used-for` names the concrete plan decision. `sample-paths` contains only sibling samples that exist and were opened. All paths use forward slashes, are repository-relative, and must resolve inside the recorded BCQuality checkout; absolute paths, traversal, and links escaping that checkout are invalid. Every reference is subject to the reference-integrity gate. + +`validation-considerations` states evidence the implementation workflow should obtain; it does not claim that a command or test has run. `suppressed` has the same shape and semantics as in a findings-report. `unresolved` records missing repository context or plan decisions that prevent a reliable constraint. Unknown applicability dimensions must appear in both `context.unknown` and a relevant unresolved entry, explaining whether they materially affect a candidate. An unknown dimension is not itself a failure or proof that relevant knowledge exists. + +Reference SHAs, when present, identify the files read; they do not prove runtime pinning on their own. The consumer records and verifies the actual immutable BCQuality checkout used for both enrichment and final review, plus its filtering policy and run provenance outside the target repository. See [agent-consumption.md](../docs/agent-consumption.md). + ## Composition (super-skills) A **super-skill** is an action skill whose frontmatter declares a non-empty `sub-skills: [...]`. A super-skill does not evaluate knowledge files directly; it invokes other action skills and composes their output. @@ -441,4 +535,4 @@ Conforms to the DO output contract. ## How orchestrators consume output -An orchestrator invokes an action skill with an input appropriate to the skill's declared `inputs`, receives the JSON output, and maps findings to its delivery surface (PR comments, build gates, IDE diagnostics). The orchestrator MUST NOT interpret skill-specific fields beyond the schema above. Skills that need richer semantics MUST encode them within the schema (for example, by adding structured `message` text) rather than extending the output shape. +An orchestrator invokes an action skill with an input appropriate to the skill's declared `inputs` and uses the single output kind declared in frontmatter. It maps a `findings-report` to PR comments, build gates, or IDE diagnostics, and a `development-guidance-report` to additional constraints for its existing implementation workflow. These are the two output schemas defined by this contract; the consumer retains ownership of implementation and delivery. diff --git a/skills/entry.md b/skills/entry.md index efa84a15..e7c76131 100644 --- a/skills/entry.md +++ b/skills/entry.md @@ -21,6 +21,8 @@ The agent invokes Entry with a **task context** supplied by the orchestrator: task-context: goal: string # free-text description of what needs doing inputs-available: # values the orchestrator has ready to pass to a chosen skill + - development-plan + - repository - pr-diff - file-path - folder-path @@ -34,9 +36,14 @@ task-context: `goal` and `inputs-available` are required. Filter dimensions (`technologies`, `bc-version`, `countries`, `application-area`) are optional; omitting a dimension is equivalent to "unconstrained" — see Relevance for the exact matching rule. `enabled-layers` defaults to all three. `disabled-skills` defaults to empty. +`development-plan` and the corresponding `development-guidance-report` output +kind are provisional. They are available to explicit integrations for review +and pilot use, but are not registered as standalone plugin capabilities in +this release. + ## Preparation — knowledge index -Before routing, ensure the knowledge index is current for the **live** clone. The dispatched review skills read `knowledge-index.json` (at the clone root) at their Source step instead of opening every knowledge file — see READ's [Retrieval workflow](read.md). When a consumer prunes its clone to policy *before* the agent runs, the index MUST be built over the clone as it exists now, so it lists exactly the articles that survived pruning and never an article the consumer denied: +Before routing, ensure the knowledge index is current for the **live** clone. The dispatched skills read `knowledge-index.json` (by default at the clone root) at their Source step instead of opening every knowledge file — see READ's [Retrieval workflow](read.md). When a consumer prunes its clone to policy *before* the agent runs, the index MUST be built over the clone as it exists now, so it lists exactly the articles that survived pruning and never an article the consumer denied: - If `knowledge-index.json` is absent — or you cannot confirm it reflects the current knowledge tree — regenerate it by running, from the checkout root: @@ -46,6 +53,7 @@ Before routing, ensure the knowledge index is current for the **live** clone. Th It defaults to indexing this checkout and writes `knowledge-index.json` at the root in well under a second. When in doubt, rebuild: a sub-second rebuild is always cheaper than a stale or over-listing index, which is a correctness risk. - The paths above assume the checkout root is the current directory. A caller that enters Entry from elsewhere — a plugin host, whose working directory is the user's own project — MUST resolve them against the BCQuality root it already knows instead. The generator resolves its own root, so invoking it by absolute path indexes and writes the right tree. +- For read-only plan enrichment, generated artifacts MUST remain outside the target repository. When the target contains the BCQuality checkout, or that checkout is immutable, pass the generator's `-IndexPath` to an external runner-owned artifact location and supply that resolved index path to the dispatched skill. Do not regenerate inside the target or modify the immutable checkout. If generation is unavailable, use READ's path-based discovery; retrieval failure is not an empty corpus. - Pruning is the consumer's job, not Entry's, and not every consumer does it: an installation that ships the whole tree gets no deny guarantee from this step. There, `enabled-layers` narrows discovery only, and the unlisted layers' files remain on disk. - This is a side step. It MUST NOT change Entry's output — the dispatch record below is the only thing Entry emits, and build logs are never part of the dispatch JSON. @@ -97,7 +105,8 @@ Emit a single JSON document conforming to the output contract below. Entry does "path": "microsoft/skills/review/al-code-review.md" }, "rationale": "string", - "inputs": ["pr-diff"] + "inputs": ["pr-diff"], + "outputs": ["findings-report"] } ], "skipped": [ @@ -120,10 +129,11 @@ Emit a single JSON document conforming to the output contract below. Entry does **`dispatch[]`** — each entry names one action skill to invoke. -- `skill.path` — repo-relative, forward slashes. The agent fetches and executes the file directly from this path. +- `skill.path` — repo-relative, forward slashes, copied from discovery. Resolve it inside the live BCQuality checkout; reject absolute paths, traversal, and links escaping that checkout. The agent reads the action skill at this exact path rather than constructing a plausible filename. - `skill.version` — copied from the dispatched skill's frontmatter so the orchestrator can detect drift between dispatch time and execution. - `rationale` — short human-readable string, for logs and traceability. - `inputs` — the intersection of `task-context.inputs-available` and the skill's declared `inputs`. The agent MUST pass exactly this subset when invoking the skill. Sending a strict intersection avoids accidental information leakage between skills. +- `outputs` — the dispatched skill's complete, single-element `outputs` value copied from frontmatter. This lets an orchestrator distinguish `findings-report` from read-only `development-guidance-report` before invocation. Check the declared contract and actual skill; output metadata is not a sandbox or proof of side effects. Unknown output kinds must not be silently treated as a supported report. Ordering of `dispatch[]` is not significant. @@ -161,7 +171,8 @@ Populated example (PR review on a repo where only `al-performance-review` is ena { "skill": { "id": "al-performance-review", "version": 1, "path": "microsoft/skills/review/al-performance-review.md" }, "rationale": "Goal 'review pull request' matched; inputs-available contains pr-diff.", - "inputs": ["pr-diff"] + "inputs": ["pr-diff"], + "outputs": ["findings-report"] } ], "skipped": [ @@ -175,7 +186,7 @@ Populated example (PR review on a repo where only `al-performance-review` is ena 1. Invoke Entry with the orchestrator-supplied task context. 2. Receive the dispatch record. -3. For each entry in `dispatch[]`, read the referenced action skill, execute its Source → Relevance → Worklist → Action steps per DO, and produce a findings-report. -4. Return the findings-reports to the orchestrator. When `outcome` is `no-match` or `failed`, return the dispatch record itself so the orchestrator can log the reason. +3. For each entry in `dispatch[]`, inspect `outputs` before invocation, read the referenced action skill, execute its Source → Relevance → Worklist → Action steps per DO, and produce the declared report kind. Verify the file's frontmatter output still equals the dispatch value; return `failed` on drift rather than executing a different contract. +4. Return the action-skill reports to the orchestrator. When Entry's `outcome` is `no-match` or `failed`, return the dispatch record itself so the orchestrator can log the reason. READ and DO are the contracts that govern what the dispatched skills do. An agent that has not yet read READ and DO reads them when it executes the first dispatched skill — they are not prerequisites for invoking Entry.