Skip to content
Open
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
58 changes: 57 additions & 1 deletion .github/scripts/Test-SkillIndex.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down Expand Up @@ -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'
Expand Down
52 changes: 35 additions & 17 deletions .github/scripts/validate_frontmatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = {
Expand Down
53 changes: 52 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.

Expand Down
Loading
Loading