mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
Close AL development contract gaps
Bound post-implementation review rounds, expose output kinds in Entry dispatch, enforce capability coverage, map BCFIX-HANDOFF v1, clarify no-knowledge behavior, and reject repository-escaping skill paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
This commit is contained in:
Jesper Schulz-Wedde
parent
56b80e6dcf
commit
f6fca1d56d
15 changed files with 356 additions and 52 deletions
65
.github/scripts/validate_frontmatter.py
vendored
65
.github/scripts/validate_frontmatter.py
vendored
|
|
@ -18,7 +18,7 @@ import os
|
|||
import re
|
||||
import sys
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from pathlib import Path, PurePosixPath
|
||||
from typing import Any, Iterable
|
||||
|
||||
try:
|
||||
|
|
@ -39,7 +39,7 @@ ACTION_SKILL_REQUIRED_KEYS = {
|
|||
}
|
||||
ACTION_SKILL_OPTIONAL_KEYS = {
|
||||
"bc-version", "technologies", "countries", "application-area", "sub-skills",
|
||||
"quality-skill", "guidance-skill",
|
||||
"quality-skill", "quality-round-limit", "guidance-skill",
|
||||
}
|
||||
META_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"}
|
||||
ENTRY_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"}
|
||||
|
|
@ -151,6 +151,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]:
|
||||
"""Normalize an optional leading './' and reject absolute/escaping paths."""
|
||||
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[2:] if value.startswith("./") else value
|
||||
segments = normalized.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.
|
||||
|
||||
|
|
@ -384,21 +406,34 @@ 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)
|
||||
report.error(path, "R20", f"invalid sub-skills paths: {bad}", 1)
|
||||
|
||||
if "quality-skill" in fm:
|
||||
quality_skill = fm["quality-skill"]
|
||||
if not isinstance(quality_skill, str) or not quality_skill.endswith(".md"):
|
||||
report.error(path, "R31", "quality-skill must be one repo-relative .md path", 1)
|
||||
_, err = normalize_repo_md_path(quality_skill)
|
||||
if err:
|
||||
report.error(path, "R31", f"quality-skill {err}", 1)
|
||||
if fm.get("outputs") != ["implementation-report"]:
|
||||
report.error(path, "R31", "quality-skill is valid only with outputs: [implementation-report]", 1)
|
||||
if "quality-round-limit" not in fm:
|
||||
report.error(path, "R31", "quality-skill requires quality-round-limit", 1)
|
||||
|
||||
if "quality-round-limit" in fm:
|
||||
limit = fm["quality-round-limit"]
|
||||
if not isinstance(limit, int) or isinstance(limit, bool) or limit <= 0:
|
||||
report.error(path, "R31", "quality-round-limit must be a positive integer", 1)
|
||||
if "quality-skill" not in fm:
|
||||
report.error(path, "R31", "quality-round-limit requires quality-skill", 1)
|
||||
if fm.get("outputs") != ["implementation-report"]:
|
||||
report.error(path, "R31", "quality-round-limit is valid only with outputs: [implementation-report]", 1)
|
||||
|
||||
if "guidance-skill" in fm:
|
||||
guidance_skill = fm["guidance-skill"]
|
||||
if not isinstance(guidance_skill, str) or not guidance_skill.endswith(".md"):
|
||||
report.error(path, "R32", "guidance-skill must be one repo-relative .md path", 1)
|
||||
_, err = normalize_repo_md_path(guidance_skill)
|
||||
if err:
|
||||
report.error(path, "R32", f"guidance-skill {err}", 1)
|
||||
if fm.get("outputs") != ["implementation-report"]:
|
||||
report.error(path, "R32", "guidance-skill is valid only with outputs: [implementation-report]", 1)
|
||||
|
||||
|
|
@ -598,7 +633,11 @@ def validate_sub_skills_registry(path: Path, fm: dict[str, Any], root: Path, rep
|
|||
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 = {
|
||||
|
|
@ -626,10 +665,10 @@ def validate_sub_skills_registry(path: Path, fm: dict[str, Any], root: Path, rep
|
|||
def validate_quality_skill(path: Path, fm: dict[str, Any], root: Path, report: Report) -> None:
|
||||
"""R30: implementation quality-skill paths resolve to a findings producer."""
|
||||
quality_skill = fm.get("quality-skill")
|
||||
if not isinstance(quality_skill, str) or not quality_skill.endswith(".md"):
|
||||
normalized, err = normalize_repo_md_path(quality_skill)
|
||||
if err or normalized is None:
|
||||
return
|
||||
|
||||
normalized = quality_skill.lstrip("./")
|
||||
target = root / normalized
|
||||
if not target.is_file():
|
||||
report.error(path, "R30", f"quality-skill does not exist on disk: {normalized}", 1)
|
||||
|
|
@ -651,10 +690,10 @@ def validate_quality_skill(path: Path, fm: dict[str, Any], root: Path, report: R
|
|||
def validate_guidance_skill(path: Path, fm: dict[str, Any], root: Path, report: Report) -> None:
|
||||
"""R33: implementation guidance-skill paths resolve to a read-only planner."""
|
||||
guidance_skill = fm.get("guidance-skill")
|
||||
if not isinstance(guidance_skill, str) or not guidance_skill.endswith(".md"):
|
||||
normalized, err = normalize_repo_md_path(guidance_skill)
|
||||
if err or normalized is None:
|
||||
return
|
||||
|
||||
normalized = guidance_skill.lstrip("./")
|
||||
target = root / normalized
|
||||
if not target.is_file():
|
||||
report.error(path, "R33", f"guidance-skill does not exist on disk: {normalized}", 1)
|
||||
|
|
|
|||
|
|
@ -150,6 +150,11 @@ Repository-specific orchestrators do not need to delegate implementation to
|
|||
plan, feed its read-only guidance report into their own phases, and retain their
|
||||
specialized environment, test, propagation, and delivery gates.
|
||||
|
||||
`al-development` does not silently fall back to generic generation when no
|
||||
article applies. It returns `no-knowledge` without changing code, making corpus
|
||||
coverage visible; callers can use their normal repository workflow or
|
||||
contribute the missing Business Central-specific guidance.
|
||||
|
||||
Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth.
|
||||
|
||||
## Tracking developer coverage
|
||||
|
|
|
|||
|
|
@ -44,7 +44,11 @@ development 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 identifies
|
||||
read-only review or planning work versus repository-changing implementation
|
||||
before invocation. 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.
|
||||
|
|
|
|||
|
|
@ -14,6 +14,10 @@ one-for-one.
|
|||
- `development-capabilities.json` tracks whether representative Business
|
||||
Central development capabilities have implementation fixtures.
|
||||
|
||||
The capability manifest declares `minimumFixtureCoverage`. CI fails when the
|
||||
share of `fixture` or `validated` capabilities falls below that floor, so a
|
||||
new planned capability cannot silently dilute generation coverage.
|
||||
|
||||
Each tracked unit has a `reviewStatus`:
|
||||
|
||||
- `in-progress` — at least one concern has been identified, but editorial
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
{
|
||||
"version": 1,
|
||||
"minimumFixtureCoverage": 0.5,
|
||||
"capabilities": [
|
||||
{
|
||||
"id": "setup-and-master-data",
|
||||
|
|
@ -82,13 +83,15 @@
|
|||
{
|
||||
"id": "install-and-upgrade",
|
||||
"title": "Installation and data upgrade",
|
||||
"status": "planned",
|
||||
"status": "fixture",
|
||||
"domains": [
|
||||
"breaking-changes",
|
||||
"testing",
|
||||
"upgrade"
|
||||
],
|
||||
"fixtureIds": []
|
||||
"fixtureIds": [
|
||||
"versioned-data-upgrade"
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "external-services",
|
||||
|
|
|
|||
|
|
@ -83,7 +83,8 @@ pwsh ./tools/Test-DevelopmentFixtures.ps1 -Root . -ResultsDirectory ./.developme
|
|||
|
||||
The initial fixtures cover setup-backed master data, document header/line
|
||||
workflows, versioned API integrations, and surgical diagnosis and repair of a
|
||||
batch-processing bug. The broader capability roadmap lives in
|
||||
batch-processing bug, plus a rerunnable data upgrade. The capability manifest
|
||||
enforces a minimum fixture-backed coverage ratio; the broader roadmap lives in
|
||||
`coverage/development-capabilities.json`.
|
||||
|
||||
### Read-only plan guidance
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
{
|
||||
"version": 1,
|
||||
"skill": "microsoft/skills/development/al-development.md",
|
||||
"maximumReviewRounds": 3,
|
||||
"minimumKnowledgeRecall": 1.0,
|
||||
"minimumKnowledgePrecision": 0.5,
|
||||
"cases": [
|
||||
|
|
@ -183,6 +184,54 @@
|
|||
"tests",
|
||||
"review"
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "versioned-data-upgrade",
|
||||
"title": "Implement a rerunnable data upgrade",
|
||||
"capabilities": [
|
||||
"install-and-upgrade"
|
||||
],
|
||||
"expectedKind": "upgrade",
|
||||
"development-request": {
|
||||
"kind": "upgrade",
|
||||
"description": "Upgrade an existing app from a text-based Customer Tier field to a new enum-backed Tier field. Preserve existing customer data, support tenants that skip intermediate app versions, keep fresh installation separate from migration, and add upgrade tests.",
|
||||
"acceptance-criteria": [
|
||||
"Existing tier values are migrated without running field validation triggers.",
|
||||
"The migration is guarded by an upgrade tag and is safe when the upgrade runs again.",
|
||||
"Check and validation triggers do not write data.",
|
||||
"Fresh installation does not run version-upgrade migration.",
|
||||
"Tests cover more than one historical source data version."
|
||||
]
|
||||
},
|
||||
"context": {
|
||||
"technologies": [
|
||||
"al"
|
||||
],
|
||||
"countries": [
|
||||
"w1"
|
||||
],
|
||||
"application-area": [
|
||||
"all"
|
||||
]
|
||||
},
|
||||
"requiredKnowledge": [
|
||||
"microsoft/knowledge/upgrade/appversion-meaning-depends-on-execution-context.md",
|
||||
"microsoft/knowledge/upgrade/install-and-upgrade-codeunits-have-no-order.md",
|
||||
"microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md",
|
||||
"microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md",
|
||||
"microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md"
|
||||
],
|
||||
"optionalKnowledge": [
|
||||
"microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md",
|
||||
"microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md",
|
||||
"microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md",
|
||||
"microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions.md"
|
||||
],
|
||||
"requiredChecks": [
|
||||
"compile",
|
||||
"tests",
|
||||
"review"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,29 +2,37 @@
|
|||
"version": 1,
|
||||
"skill": "microsoft/skills/development/al-development-plan.md",
|
||||
"minimumKnowledgeRecall": 1.0,
|
||||
"minimumKnowledgePrecision": 0.5,
|
||||
"minimumKnowledgePrecision": 0.67,
|
||||
"cases": [
|
||||
{
|
||||
"id": "bcapps-filtered-batch-bug-plan",
|
||||
"title": "Select guidance for a filtered batch bug fix",
|
||||
"title": "Select guidance from a BCFIX-HANDOFF v1 payload",
|
||||
"expectedKind": "bug",
|
||||
"development-plan": {
|
||||
"kind": "bug",
|
||||
"request": "Fix a filtered batch routine that updates only the first matching record.",
|
||||
"root-cause": "The routine calls FindFirst and updates the current record without entering an enumerator loop.",
|
||||
"affected-files": [
|
||||
"src/Batch/UpdateSelectedEntries.Codeunit.al",
|
||||
"format": "BCFIX-HANDOFF",
|
||||
"version": 1,
|
||||
"issue": 4312,
|
||||
"phase": "implement",
|
||||
"status": "paused",
|
||||
"baton": 2,
|
||||
"rootCause": "The routine calls FindFirst and updates the current record without entering an enumerator loop, so only the first record in the supplied filtered set is modified.",
|
||||
"harnessMap": {
|
||||
"testCodeunit": "Update Selected Entries Tests",
|
||||
"libraries": [
|
||||
"Library - Random"
|
||||
],
|
||||
"pages": [],
|
||||
"handlers": []
|
||||
},
|
||||
"iterationsUsed": 1,
|
||||
"filesCommitted": [
|
||||
"test/Batch/UpdateSelectedEntries.Codeunit.al"
|
||||
],
|
||||
"proposed-changes": [
|
||||
"Iterate the supplied filtered record set and update every selected entry.",
|
||||
"Add a regression test with multiple selected entries."
|
||||
"lastTestResult": "1 failing, 4 passing; the red test shows only the first of three selected records is updated.",
|
||||
"deadEnds": [
|
||||
"Changing the page selection did not help because the codeunit discarded the supplied enumerator."
|
||||
],
|
||||
"test-strategy": "Establish a red test where only one of several selected records is updated, then require all selected records to be updated after the fix.",
|
||||
"acceptance-criteria": [
|
||||
"The supplied filters are preserved.",
|
||||
"Every selected record is updated.",
|
||||
"The regression test demonstrates red-to-green behavior."
|
||||
]
|
||||
"nextStep": "Replace the single-record read with an update-safe FindSet/Next loop that preserves the supplied filters, then rerun the red test."
|
||||
},
|
||||
"context": {
|
||||
"technologies": [
|
||||
|
|
@ -45,6 +53,50 @@
|
|||
"optionalKnowledge": [
|
||||
"microsoft/knowledge/performance/pass-var-record-to-preserve-partial-load-enumerator.md"
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "versioned-upgrade-plan-guidance",
|
||||
"title": "Select guidance for a versioned data upgrade",
|
||||
"expectedKind": "upgrade",
|
||||
"development-plan": {
|
||||
"kind": "upgrade",
|
||||
"request": "Migrate existing customer tier text values to a new enum field in an app upgrade.",
|
||||
"root-cause": "The new schema needs an explicit, rerunnable migration for existing tenant data.",
|
||||
"affected-files": [
|
||||
"src/Upgrade/CustomerTierUpgrade.Codeunit.al",
|
||||
"test/Upgrade/CustomerTierUpgradeTests.Codeunit.al"
|
||||
],
|
||||
"proposed-changes": [
|
||||
"Add a tagged upgrade step that copies existing values without validation triggers.",
|
||||
"Add upgrade tests from multiple historical data versions."
|
||||
],
|
||||
"test-strategy": "Run upgrade tests from two prior data versions and verify a second invocation makes no further changes.",
|
||||
"acceptance-criteria": [
|
||||
"Existing values are preserved.",
|
||||
"The migration is rerunnable.",
|
||||
"Fresh installation does not execute upgrade migration."
|
||||
]
|
||||
},
|
||||
"context": {
|
||||
"technologies": [
|
||||
"al"
|
||||
],
|
||||
"countries": [
|
||||
"w1"
|
||||
],
|
||||
"application-area": [
|
||||
"all"
|
||||
]
|
||||
},
|
||||
"requiredKnowledge": [
|
||||
"microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md",
|
||||
"microsoft/knowledge/upgrade/check-only-triggers-do-not-migrate-data.md",
|
||||
"microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md"
|
||||
],
|
||||
"optionalKnowledge": [
|
||||
"microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md",
|
||||
"microsoft/knowledge/upgrade/appversion-meaning-depends-on-execution-context.md"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ When a dimension cannot be resolved, retain conditionally applicable candidates
|
|||
## Worklist
|
||||
|
||||
1. Normalize the plan into: request summary, development kind, assumptions, root cause or design intent, affected files and symbols, proposed changes, test strategy, and acceptance criteria. When the plan has no normalized kind, apply the same categories as `al-development`: 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`. A repository-specific additive event or extensibility request maps to `feature`; retain its original work-item type in the request summary. Do not redesign the repository-specific workflow.
|
||||
- For a `BCFIX-HANDOFF` v1 payload, map `rootCause` to root cause, `harnessMap` to test context, `filesCommitted` to affected files, `lastTestResult` to existing test evidence, `deadEnds` to rejected approaches, and `nextStep` to the immediate proposed change. Preserve `issue`, `phase`, `status`, `baton`, and `iterationsUsed` as workflow context only; they do not create Business Central constraints. A handoff with a non-empty root cause is `bug` unless the surrounding plan identifies an additive Event Request, which maps to `feature`.
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
guidance-skill: microsoft/skills/development/al-development-plan.md
|
||||
quality-skill: microsoft/skills/review/al-code-review.md
|
||||
quality-round-limit: 3
|
||||
---
|
||||
|
||||
# AL development
|
||||
|
|
@ -66,7 +67,7 @@ Record unresolved dimensions in the development plan rather than silently substi
|
|||
6. Invoke the frontmatter `guidance-skill` with that plan, the repository, and the resolved context. It performs Source, Relevance, and knowledge worklisting independently and read-only.
|
||||
7. Require a complete guidance result before editing product code:
|
||||
- `completed` — use every returned constraint and validation consideration.
|
||||
- `no-knowledge` — return `no-knowledge` without implementing a Business Central-specific change.
|
||||
- `no-knowledge` — intentionally refuse to implement: return `no-knowledge` with no request changes, set `outcome-reason` to `No applicable BCQuality knowledge was found for this development plan.`, and add a `remaining` entry directing the caller to use a repository-specific workflow/general coding agent or contribute the missing BC-specific knowledge.
|
||||
- `not-applicable`, `partial`, or `failed` — return the corresponding non-completed outcome without editing product code; preserve its reason in `remaining`.
|
||||
8. Copy the guidance report's selected paths into the eventual implementation report only when the corresponding constraint materially shaped the implementation. Carry its suppression records forward.
|
||||
|
||||
|
|
@ -83,9 +84,15 @@ Record unresolved dimensions in the development plan rather than silently substi
|
|||
4. Implement the request end to end. Include all surfaces required by the mode, acceptance criteria, and repository conventions. Do not create success-shaped stubs.
|
||||
5. Treat the guidance report as design constraints throughout implementation. Adapt its referenced companion samples to the target codebase; never copy demonstration IDs or names blindly.
|
||||
6. Run the smallest existing build, analyzer, and test commands that cover the change. Fix failures caused by the implementation. Record every command and real outcome in `validation`; unavailable checks are `not-run`, never `passed`.
|
||||
7. Invoke the frontmatter `quality-skill` against the final implementation diff. Fix all justified knowledge-backed `blocker` and `major` findings and concrete defects introduced by this work, then rerun affected validation and review. Preserve the last findings-report in `review` and add a `validation` entry with `id: "review"`. If review is disabled or unavailable, record `not-run` and return `partial`.
|
||||
8. Verify the persisted files against the implementation contract, acceptance criteria, and mode-specific evidence. If behavior, validation, or review remains incomplete, return `partial` and list the exact gap in `remaining`.
|
||||
7. Invoke the frontmatter `quality-skill` against the final implementation diff, bounded by `quality-round-limit`:
|
||||
- Record every invocation in `review-rounds`, including its gating `blocker` and `major` IDs.
|
||||
- When no gating finding remains, mark the round `clean` and stop.
|
||||
- Otherwise fix every justified, safely actionable gating finding, rerun affected validation, mark the round `fixing`, and start the next review round.
|
||||
- Stop early as `stalled` when the gating ID set is unchanged from the preceding round, no gating finding can be fixed safely, or validation cannot be restored.
|
||||
- When the final allowed round still has gating findings, mark it `limit-reached`.
|
||||
Preserve the last complete findings-report in `review` and add a `validation` entry with `id: "review"`. A `stalled` or `limit-reached` loop returns `partial` with the unresolved gating findings in `remaining`. If review is disabled or unavailable, record `not-run` and return `partial`.
|
||||
8. Verify the persisted files against the implementation contract, acceptance criteria, and mode-specific evidence. If behavior, validation, guidance, or review remains incomplete, return `partial` and list the exact gap in `remaining`.
|
||||
|
||||
## Output
|
||||
|
||||
Return one `implementation-report` conforming to DO. Set `plan.kind` to the classified execution mode. `knowledge` lists only articles opened in full and materially used. `changes` lists only files changed by this skill. `completed` requires a persisted implementation, passing required validation, and no unresolved `blocker` or `major` finding in `review`.
|
||||
Return one `implementation-report` conforming to DO. Set `plan.kind` to the classified execution mode. `knowledge` lists only articles opened in full and materially used. `changes` lists only files changed by this skill. `completed` requires a persisted implementation, passing required validation, and no unresolved `blocker` or `major` finding in `review`. `no-knowledge` is a visible coverage decision, not an error or silent fallback.
|
||||
|
|
|
|||
|
|
@ -9,6 +9,11 @@ This is BCQuality's host-native adapter for standalone plugin installations. It
|
|||
|
||||
When the target repository exposes a more specific local workflow for the request, such as an end-to-end bug-fix skill with its own environment and delivery gates, prefer that repository workflow unless the caller explicitly asks to use BCQuality's generic development skill.
|
||||
|
||||
This adapter is deliberately knowledge-backed: if BCQuality has no applicable
|
||||
guidance, it returns a visible `no-knowledge` result without changing code. Use
|
||||
the repository's normal coding workflow for unbacked requests, or add the
|
||||
missing BC-specific knowledge before expecting this skill to implement them.
|
||||
|
||||
## Execute
|
||||
|
||||
1. Resolve `PLUGIN_ROOT` to the directory containing this plugin's root `plugin.json`. This file is `PLUGIN_ROOT/skills/al-development/SKILL.md`; when the host does not expose the plugin root, resolve it two levels above this file.
|
||||
|
|
|
|||
11
skills/do.md
11
skills/do.md
|
|
@ -66,7 +66,7 @@ application-area: [all]
|
|||
|
||||
`sub-skills` is an optional field. When present and non-empty, the skill is a **super-skill** that composes other action skills; see *Composition* below. Values are repo-relative paths to action-skill files.
|
||||
|
||||
`quality-skill` is optional on an action skill that emits an `implementation-report`. It names one repo-relative review action skill to run over the completed diff. It is a post-implementation gate, not a composed sub-skill: Entry does not route through it, and its complete findings-report is returned in `review`. Consumer configuration still applies; if the named quality skill is disabled or unavailable, record its validation as `not-run` and do not claim `completed`.
|
||||
`quality-skill` is optional on an action skill that emits an `implementation-report`. It names one repo-relative review action skill to run over the completed diff. It is a post-implementation gate, not a composed sub-skill: Entry does not route through it, and its final complete findings-report is returned in `review`. `quality-round-limit` is the required positive maximum number of review/fix rounds when a quality skill is declared. Consumer configuration still applies; if the named quality skill is disabled or unavailable, record its validation as `not-run` and do not claim `completed`.
|
||||
|
||||
`guidance-skill` is optional on an action skill that emits an `implementation-report`. It names one repo-relative read-only action skill that accepts a `development-plan` and emits a `development-guidance-report`. The implementation skill invokes it after forming its plan and before editing product code. Consumer configuration still applies; when guidance is disabled or unavailable, the implementation skill must not claim knowledge-backed development.
|
||||
|
||||
|
|
@ -346,6 +346,13 @@ An action skill with `outputs: [implementation-report]` emits one JSON document:
|
|||
}
|
||||
],
|
||||
"review": { "...full findings-report from the post-implementation review..." : null },
|
||||
"review-rounds": [
|
||||
{
|
||||
"round": 1,
|
||||
"outcome": "clean | fixing | stalled | limit-reached",
|
||||
"gating-finding-ids": ["string"]
|
||||
}
|
||||
],
|
||||
"suppressed": [
|
||||
{
|
||||
"reference": { "path": "string", "sha": "string" },
|
||||
|
|
@ -378,6 +385,8 @@ An action skill with `outputs: [implementation-report]` emits one JSON document:
|
|||
|
||||
**`review`** is optional for generic implementation skills and required when a skill's instructions mandate post-implementation review. When present, it is the complete findings-report returned by that review skill, not a rewritten summary.
|
||||
|
||||
**`review-rounds`** records every quality-skill invocation in order. `gating-finding-ids` contains the `blocker` and `major` IDs from that round. `clean` ends successfully; `fixing` means the skill applied justified fixes before another round; `stalled` means the same gating set persisted or no safe progress was possible; `limit-reached` means the configured round cap was exhausted. The array length MUST NOT exceed `quality-round-limit`. `stalled` or `limit-reached` requires implementation outcome `partial`, the final findings-report in `review`, and every unresolved gating item in `remaining`.
|
||||
|
||||
**`suppressed`** has the same semantics as in a findings-report and records applicable knowledge excluded by layer precedence or configuration.
|
||||
|
||||
**`remaining`** contains concrete unfinished work only. It is empty for `completed`.
|
||||
|
|
|
|||
|
|
@ -99,7 +99,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": [
|
||||
|
|
@ -126,6 +127,7 @@ Emit a single JSON document conforming to the output contract below. Entry does
|
|||
- `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 read-only `findings-report` and `development-guidance-report` work from repository-changing `implementation-report` work before invoking the skill. An orchestrator MAY require an additional write confirmation for `implementation-report`; it MUST NOT infer side effects from the skill ID or title.
|
||||
|
||||
Ordering of `dispatch[]` is not significant.
|
||||
|
||||
|
|
@ -163,7 +165,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": [
|
||||
|
|
@ -177,7 +180,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 the report kind declared by that skill's single `outputs` value.
|
||||
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 an unexpectedly mutating skill.
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -52,6 +52,22 @@ if ($manifest.version -ne 1) {
|
|||
if ($capabilityManifest.version -ne 1) {
|
||||
$problems.Add("Unsupported capability manifest version: $($capabilityManifest.version)") | Out-Null
|
||||
}
|
||||
$minimumFixtureCoverage = if ($capabilityManifest.PSObject.Properties.Name -contains 'minimumFixtureCoverage') {
|
||||
[double]$capabilityManifest.minimumFixtureCoverage
|
||||
} else {
|
||||
-1
|
||||
}
|
||||
if ($minimumFixtureCoverage -lt 0 -or $minimumFixtureCoverage -gt 1) {
|
||||
$problems.Add("minimumFixtureCoverage must be between 0 and 1.") | Out-Null
|
||||
}
|
||||
$maximumReviewRounds = if ($manifest.PSObject.Properties.Name -contains 'maximumReviewRounds') {
|
||||
[int]$manifest.maximumReviewRounds
|
||||
} else {
|
||||
0
|
||||
}
|
||||
if ($maximumReviewRounds -le 0) {
|
||||
$problems.Add("maximumReviewRounds must be a positive integer.") | Out-Null
|
||||
}
|
||||
foreach ($thresholdName in @('minimumKnowledgeRecall', 'minimumKnowledgePrecision')) {
|
||||
$threshold = [double]$manifest.$thresholdName
|
||||
if ($threshold -lt 0 -or $threshold -gt 1) {
|
||||
|
|
@ -62,6 +78,12 @@ foreach ($thresholdName in @('minimumKnowledgeRecall', 'minimumKnowledgePrecisio
|
|||
$skillPath = [string]$manifest.skill
|
||||
if (-not (Test-Path -LiteralPath (Join-Path $Root $skillPath) -PathType Leaf)) {
|
||||
$problems.Add("Development skill does not exist: $skillPath") | Out-Null
|
||||
} else {
|
||||
$skillText = Get-Content -LiteralPath (Join-Path $Root $skillPath) -Raw
|
||||
$limitMatch = [regex]::Match($skillText, '(?m)^quality-round-limit:\s*(\d+)\s*$')
|
||||
if (-not $limitMatch.Success -or [int]$limitMatch.Groups[1].Value -ne $maximumReviewRounds) {
|
||||
$problems.Add("maximumReviewRounds must match the development skill quality-round-limit.") | Out-Null
|
||||
}
|
||||
}
|
||||
|
||||
$validChecks = @('compile', 'tests', 'review')
|
||||
|
|
@ -173,6 +195,16 @@ foreach ($case in @($manifest.cases)) {
|
|||
}
|
||||
}
|
||||
|
||||
$fixtureBackedCount = @(
|
||||
$capabilityManifest.capabilities |
|
||||
Where-Object status -in @('fixture', 'validated')
|
||||
).Count
|
||||
$capabilityCount = @($capabilityManifest.capabilities).Count
|
||||
$fixtureCoverage = if ($capabilityCount) { $fixtureBackedCount / $capabilityCount } else { 0.0 }
|
||||
if ($fixtureCoverage -lt $minimumFixtureCoverage) {
|
||||
$problems.Add("Fixture-backed capability coverage $fixtureCoverage is below minimumFixtureCoverage $minimumFixtureCoverage.") | Out-Null
|
||||
}
|
||||
|
||||
if ($problems.Count) {
|
||||
Write-Host "Development fixture validation FAILED ($($problems.Count) problem(s)):" -ForegroundColor Red
|
||||
$problems | ForEach-Object { Write-Host " - $_" -ForegroundColor Red }
|
||||
|
|
@ -269,6 +301,11 @@ if ($PrepareDirectory) {
|
|||
findings = @()
|
||||
suppressed = @()
|
||||
}
|
||||
'review-rounds' = @([ordered]@{
|
||||
round = 1
|
||||
outcome = 'clean | fixing | stalled | limit-reached'
|
||||
'gating-finding-ids' = @()
|
||||
})
|
||||
suppressed = @()
|
||||
remaining = @()
|
||||
}
|
||||
|
|
@ -303,7 +340,7 @@ if ($ResultsDirectory) {
|
|||
continue
|
||||
}
|
||||
$report = $result.implementationReport
|
||||
foreach ($requiredField in @('skill', 'outcome', 'summary', 'plan', 'knowledge', 'changes', 'validation', 'review', 'suppressed', 'remaining')) {
|
||||
foreach ($requiredField in @('skill', 'outcome', 'summary', 'plan', 'knowledge', 'changes', 'validation', 'review', 'review-rounds', 'suppressed', 'remaining')) {
|
||||
if ($report.PSObject.Properties.Name -notcontains $requiredField) {
|
||||
$failures.Add("$($case.id): implementation report is missing '$requiredField'.") | Out-Null
|
||||
}
|
||||
|
|
@ -492,6 +529,41 @@ if ($ResultsDirectory) {
|
|||
$failures.Add("$($case.id): post-implementation review has $($gatingFindings.Count) gating finding(s).") | Out-Null
|
||||
}
|
||||
}
|
||||
[object[]]$reviewRounds = @()
|
||||
if ($report.PSObject.Properties.Name -contains 'review-rounds') {
|
||||
$reviewRounds = @($report.'review-rounds')
|
||||
}
|
||||
if (-not $reviewRounds.Count -or $reviewRounds.Count -gt $maximumReviewRounds) {
|
||||
$failures.Add("$($case.id): review-rounds must contain 1..$maximumReviewRounds entries.") | Out-Null
|
||||
} else {
|
||||
for ($index = 0; $index -lt $reviewRounds.Count; $index++) {
|
||||
$round = $reviewRounds[$index]
|
||||
$roundNumber = if ($round.PSObject.Properties.Name -contains 'round') { [int]$round.round } else { 0 }
|
||||
$roundOutcome = if ($round.PSObject.Properties.Name -contains 'outcome') { [string]$round.outcome } else { '' }
|
||||
if ($roundNumber -ne ($index + 1)) {
|
||||
$failures.Add("$($case.id): review round numbering is not contiguous.") | Out-Null
|
||||
}
|
||||
if ($roundOutcome -notin @('clean', 'fixing', 'stalled', 'limit-reached')) {
|
||||
$failures.Add("$($case.id): review round $roundNumber has invalid outcome '$roundOutcome'.") | Out-Null
|
||||
}
|
||||
[object[]]$roundGatingIds = @()
|
||||
if ($round.PSObject.Properties.Name -contains 'gating-finding-ids') {
|
||||
$roundGatingIds = @($round.'gating-finding-ids')
|
||||
}
|
||||
if ($roundOutcome -eq 'clean' -and $roundGatingIds.Count) {
|
||||
$failures.Add("$($case.id): clean review round $roundNumber must have no gating IDs.") | Out-Null
|
||||
}
|
||||
if ($roundOutcome -in @('fixing', 'stalled', 'limit-reached') -and -not $roundGatingIds.Count) {
|
||||
$failures.Add("$($case.id): review round $roundNumber outcome '$roundOutcome' requires gating IDs.") | Out-Null
|
||||
}
|
||||
if (@($roundGatingIds | Sort-Object -Unique).Count -ne $roundGatingIds.Count) {
|
||||
$failures.Add("$($case.id): review round $roundNumber has duplicate gating IDs.") | Out-Null
|
||||
}
|
||||
}
|
||||
if ([string]$reviewRounds[-1].outcome -ne 'clean') {
|
||||
$failures.Add("$($case.id): completed implementation must end with a clean review round.") | Out-Null
|
||||
}
|
||||
}
|
||||
[object[]]$remaining = @()
|
||||
if ($report.PSObject.Properties.Name -contains 'remaining') {
|
||||
$remaining = @($report.remaining)
|
||||
|
|
@ -508,9 +580,5 @@ if ($ResultsDirectory) {
|
|||
}
|
||||
Write-Host "Development fixture scoring PASSED: $(@($manifest.cases).Count) case(s)."
|
||||
} else {
|
||||
$fixtureBackedCapabilities = @(
|
||||
$capabilityManifest.capabilities |
|
||||
Where-Object status -in @('fixture', 'validated')
|
||||
).Count
|
||||
Write-Host "Development fixture validation PASSED: $(@($manifest.cases).Count) cases; $fixtureBackedCapabilities of $($capabilityIds.Count) capabilities have fixtures."
|
||||
Write-Host "Development fixture validation PASSED: $(@($manifest.cases).Count) cases; $fixtureBackedCount of $($capabilityIds.Count) capabilities have fixtures (minimum $minimumFixtureCoverage)."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,6 +36,22 @@ function Get-ModelCaseId {
|
|||
}
|
||||
}
|
||||
|
||||
function Get-PlanRequest {
|
||||
param([object] $Plan)
|
||||
|
||||
if ($Plan.PSObject.Properties.Name -contains 'request' -and
|
||||
-not [string]::IsNullOrWhiteSpace([string]$Plan.request)) {
|
||||
return [string]$Plan.request
|
||||
}
|
||||
if ($Plan.PSObject.Properties.Name -contains 'format' -and
|
||||
[string]$Plan.format -eq 'BCFIX-HANDOFF' -and
|
||||
$Plan.PSObject.Properties.Name -contains 'nextStep') {
|
||||
$issue = if ($Plan.PSObject.Properties.Name -contains 'issue') { [string]$Plan.issue } else { 'unknown' }
|
||||
return "Continue BCFIX issue #${issue}: $($Plan.nextStep)"
|
||||
}
|
||||
return ''
|
||||
}
|
||||
|
||||
if ($manifest.version -ne 1) {
|
||||
$problems.Add("Unsupported guidance fixture version: $($manifest.version)") | Out-Null
|
||||
}
|
||||
|
|
@ -64,11 +80,49 @@ foreach ($case in @($manifest.cases)) {
|
|||
continue
|
||||
}
|
||||
$plan = $case.'development-plan'
|
||||
if ($validKinds -notcontains [string]$plan.kind) {
|
||||
$expectedKind = if ($case.PSObject.Properties.Name -contains 'expectedKind') {
|
||||
[string]$case.expectedKind
|
||||
} else {
|
||||
''
|
||||
}
|
||||
if ($validKinds -notcontains $expectedKind) {
|
||||
$problems.Add("${id}: expectedKind is invalid.") | Out-Null
|
||||
}
|
||||
$isBcfixHandoff = (
|
||||
$plan.PSObject.Properties.Name -contains 'format' -and
|
||||
[string]$plan.format -eq 'BCFIX-HANDOFF'
|
||||
)
|
||||
if ($isBcfixHandoff) {
|
||||
$requiredHandoffFields = @(
|
||||
'version', 'issue', 'phase', 'status', 'baton', 'rootCause',
|
||||
'harnessMap', 'iterationsUsed', 'filesCommitted', 'lastTestResult',
|
||||
'deadEnds', 'nextStep'
|
||||
)
|
||||
foreach ($field in $requiredHandoffFields) {
|
||||
if ($plan.PSObject.Properties.Name -notcontains $field) {
|
||||
$problems.Add("${id}: BCFIX-HANDOFF is missing '$field'.") | Out-Null
|
||||
}
|
||||
}
|
||||
$handoffVersion = if ($plan.PSObject.Properties.Name -contains 'version') { [int]$plan.version } else { 0 }
|
||||
$handoffPhase = if ($plan.PSObject.Properties.Name -contains 'phase') { [string]$plan.phase } else { '' }
|
||||
$handoffStatus = if ($plan.PSObject.Properties.Name -contains 'status') { [string]$plan.status } else { '' }
|
||||
if ($handoffVersion -ne 1) {
|
||||
$problems.Add("${id}: only BCFIX-HANDOFF version 1 is supported.") | Out-Null
|
||||
}
|
||||
if ($handoffPhase -notin @('plan', 'baseline', 'implement', 'pr')) {
|
||||
$problems.Add("${id}: BCFIX-HANDOFF phase is invalid.") | Out-Null
|
||||
}
|
||||
if ($handoffStatus -notin @('in-progress', 'paused', 'done')) {
|
||||
$problems.Add("${id}: BCFIX-HANDOFF status is invalid.") | Out-Null
|
||||
}
|
||||
} elseif (
|
||||
$plan.PSObject.Properties.Name -notcontains 'kind' -or
|
||||
$validKinds -notcontains [string]$plan.kind
|
||||
) {
|
||||
$problems.Add("${id}: development-plan.kind is invalid.") | Out-Null
|
||||
}
|
||||
if ([string]::IsNullOrWhiteSpace([string]$plan.request)) {
|
||||
$problems.Add("${id}: development-plan.request is required.") | Out-Null
|
||||
if ([string]::IsNullOrWhiteSpace((Get-PlanRequest $plan))) {
|
||||
$problems.Add("${id}: development-plan must provide request or BCFIX nextStep.") | Out-Null
|
||||
}
|
||||
|
||||
$expectedKnowledge = @($case.requiredKnowledge) + @($case.optionalKnowledge)
|
||||
|
|
@ -125,7 +179,7 @@ if ($PrepareDirectory) {
|
|||
skillInstructions = $skillInstructions
|
||||
knowledgeIndex = 'knowledge-index.json'
|
||||
'task-context' = [ordered]@{
|
||||
goal = [string]$case.'development-plan'.request
|
||||
goal = Get-PlanRequest $case.'development-plan'
|
||||
'inputs-available' = @('development-plan', 'repository')
|
||||
technologies = @($case.context.technologies)
|
||||
countries = @($case.context.countries)
|
||||
|
|
@ -216,7 +270,7 @@ if ($ResultsDirectory) {
|
|||
$report.PSObject.Properties.Name -contains 'summary' -and
|
||||
$report.summary.PSObject.Properties.Name -contains 'kind'
|
||||
) { [string]$report.summary.kind } else { '' }
|
||||
if ($kind -ne [string]$case.'development-plan'.kind) {
|
||||
if ($kind -ne [string]$case.expectedKind) {
|
||||
$failures.Add("$($case.id): summary.kind '$kind' does not match the plan.") | Out-Null
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue